From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:38839) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZosNX-0002jD-Lz for qemu-devel@nongnu.org; Wed, 21 Oct 2015 08:20:28 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZosNS-0006zV-Vq for qemu-devel@nongnu.org; Wed, 21 Oct 2015 08:20:27 -0400 Received: from mail-lb0-f175.google.com ([209.85.217.175]:36704) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZosNS-0006zC-KJ for qemu-devel@nongnu.org; Wed, 21 Oct 2015 08:20:22 -0400 Received: by lbcao8 with SMTP id ao8so36886039lbc.3 for ; Wed, 21 Oct 2015 05:20:21 -0700 (PDT) MIME-Version: 1.0 In-Reply-To: <5625DC52.5010006@redhat.com> References: <1445154799-31083-1-git-send-email-leonid.bloch@ravellosystems.com> <1445154799-31083-5-git-send-email-leonid.bloch@ravellosystems.com> <5625DC52.5010006@redhat.com> Date: Wed, 21 Oct 2015 15:20:21 +0300 Message-ID: From: Leonid Bloch Content-Type: text/plain; charset=UTF-8 Subject: Re: [Qemu-devel] [PATCH 4/6] e1000: Fixing the received/transmitted octets' counters List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Jason Wang Cc: Dmitry Fleytman , Leonid Bloch , qemu-devel@nongnu.org, Shmulik Ladkani On Tue, Oct 20, 2015 at 9:16 AM, Jason Wang wrote: > > > 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 >> Signed-off-by: Dmitry Fleytman >> --- >> 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; Yes, that is better! Few small changes: uint64_t sum = s->mac_reg[index] | (uint64_t)s->mac_reg[index+1] << 32; if (sum + size < sum) { sum = ~0; } else { sum += size; } s->mac_reg[index] = sum; s->mac_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 > * Address> field through the 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]) >