On Tue, Oct 20, 2015 at 9:16 AM, Jason Wang <jasow...@redhat.com> 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 <leonid.bl...@ravellosystems.com>
>> Signed-off-by: Dmitry Fleytman <dmitry.fleyt...@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;
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
>> <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])
>
