On Mon, Sep 06, 2010 at 01:29:27PM +0200, Juan Quintela wrote:
> "Michael S. Tsirkin" <m...@redhat.com> wrote:
> > Patch b0b900070c7cb29bbefb732ec00397abe5de6d73 made
> > TOR valuer incorrect: the spec says it should always
> > include the CRC field, while size does not include CRC now.
> > No one seems to use TOR field (which is likely why
> > current code works fine), but better to stick to spec.
> >
> > Lightly tested with a linux guest.
> >
> > Signed-off-by: Michael S. Tsirkin <m...@redhat.com>
> > ---
> > hw/e1000.c | 8 ++++++--
> > 1 files changed, 6 insertions(+), 2 deletions(-)
> >
> > diff --git a/hw/e1000.c b/hw/e1000.c
> > index 80b78bc..eb9faf2 100644
> > --- a/hw/e1000.c
> > +++ b/hw/e1000.c
> > @@ -345,7 +345,7 @@ is_vlan_txd(uint32_t txd_lower)
> >
> > /* FCS aka Ethernet CRC-32. We don't get it from backends and can't
> > * fill it in, just pad descriptor length by 4 bytes unless guest
> > - * told us to trip it off the packet. */
> > + * told us to strip it off the packet. */
> > static inline int
> > fcs_len(E1000State *s)
> > {
> > @@ -690,8 +690,12 @@ e1000_receive(VLANClientState *nc, const uint8_t *buf,
> > size_t size)
> >
> > s->mac_reg[GPRC]++;
> > s->mac_reg[TPR]++;
> > + /* TOR - Total Octets Received:
> > + * This register includes bytes received in a packet from the
> > <Destination
> > + * Address> field through the <CRC> field, inclusively.
> > + */
> > n = s->mac_reg[TORL];
> > - if ((s->mac_reg[TORL] += size) < n)
> > + if ((s->mac_reg[TORL] += size + 4 /* Always include FCS
> > length. */) < n)
>
> once changing this, can we move the assignation out of the if?
> It was already complex enough, but adding a comment inside an if
> condition looks "too much" to me.
>
> Later, Juan.
Sure, I'll do that.
> > s->mac_reg[TORH]++;
> >
> > n = E1000_ICS_RXT0;
