Re: [Qemu-devel] [PATCH 4/6] e1000: Fixing the received/transmitted octets' counters

[Jason Wang <jasowang@redhat.com>]

On 10/18/2015 03:53 PM, Leonid Bloch wrote:
> Previously, the lower parts of these counters (TORL, TOTL) were
> resetting after reaching their maximal values, and since the continuation
> of counting in the higher parts (TORH, TOTH) was triggered by an
> overflow event of the lower parts, the count was not correct.
>
> Additionally, TORH and TOTH were counting the corresponding frames, and
> not the octets, as they supposed to do.
>
> Additionally, these 64-bit registers did not stick at their maximal
> values when (and if) they reached them.
>
> This fix resolves all the issues mentioned above, and makes the octet
> counters behave according to Intel's specs.
>
> Signed-off-by: Leonid Bloch <leonid.bloch@ravellosystems.com>
> Signed-off-by: Dmitry Fleytman <dmitry.fleytman@ravellosystems.com>
> ---
>  hw/net/e1000.c | 34 ++++++++++++++++++++++++++--------
>  1 file changed, 26 insertions(+), 8 deletions(-)
>
> diff --git a/hw/net/e1000.c b/hw/net/e1000.c
> index 5530285..7f977b6 100644
> --- a/hw/net/e1000.c
> +++ b/hw/net/e1000.c
> @@ -583,6 +583,28 @@ inc_reg_if_not_full(E1000State *s, int index)
>      }
>  }
>  
> +static void
> +grow_8reg_if_not_full(E1000State *s, int index, int size)
> +{
> +    uint32_t lo = s->mac_reg[index];
> +    uint32_t hi = s->mac_reg[index+1];
> +
> +    if (lo == 0xffffffff) {
> +        if ((hi += size) > s->mac_reg[index+1]) {
> +            s->mac_reg[index+1] = hi;
> +        } else if (s->mac_reg[index+1] != 0xffffffff) {
> +            s->mac_reg[index+1] = 0xffffffff;
> +        }
> +    } else {
> +        if (((lo += size) < s->mac_reg[index])
> +            && (s->mac_reg[index] = 0xffffffff)) {  /* setting low to full */
> +            s->mac_reg[index+1] += ++lo;
> +        } else {
> +            s->mac_reg[index] = lo;
> +        }
> +    }
> +}

How about something easier:

uint64_t sum = s->mac_reg[index] | (uint64_t)s->mac_reg[index+1] <<32;
if (sum + size < sum) {
    sum = 0xffffffffffffffff;
} else {
    sum += size;
}
s->max_reg[index] = sum;
s->max_reg[index+1] = sum >> 32;

> +
>  static inline int
>  vlan_enabled(E1000State *s)
>  {
> @@ -632,7 +654,7 @@ static void
>  xmit_seg(E1000State *s)
>  {
>      uint16_t len, *sp;
> -    unsigned int frames = s->tx.tso_frames, css, sofar, n;
> +    unsigned int frames = s->tx.tso_frames, css, sofar;
>      struct e1000_tx *tp = &s->tx;
>  
>      if (tp->tse && tp->cptse) {
> @@ -678,10 +700,8 @@ xmit_seg(E1000State *s)
>      } else
>          e1000_send_packet(s, tp->data, tp->size);
>      inc_reg_if_not_full(s, TPT);
> +    grow_8reg_if_not_full(s, TOTL, s->tx.size);
>      s->mac_reg[GPTC] = s->mac_reg[TPT];
> -    n = s->mac_reg[TOTL];
> -    if ((s->mac_reg[TOTL] += s->tx.size) < n)
> -        s->mac_reg[TOTH]++;
>  }
>  
>  static void
> @@ -1096,11 +1116,9 @@ e1000_receive_iov(NetClientState *nc, const struct iovec *iov, int iovcnt)
>      /* TOR - Total Octets Received:
>       * This register includes bytes received in a packet from the <Destination
>       * Address> field through the <CRC> field, inclusively.
> +     * Always include FCS length (4) in size.
>       */
> -    n = s->mac_reg[TORL] + size + /* Always include FCS length. */ 4;
> -    if (n < s->mac_reg[TORL])
> -        s->mac_reg[TORH]++;
> -    s->mac_reg[TORL] = n;
> +    grow_8reg_if_not_full(s, TORL, size+4);
>  
>      n = E1000_ICS_RXT0;
>      if ((rdt = s->mac_reg[RDT]) < s->mac_reg[RDH])