Re: [Qemu-devel] [PATCH 0/6] e1000: Various fixes and registers' implementation

2015-10-22 Thread Jason Wang 


On 10/21/2015 09:32 PM, Leonid Bloch wrote:
> Hi Jason, thanks again for reviewing,
>
> On Tue, Oct 20, 2015 at 9:37 AM, Jason Wang  > wrote:
> >
> >
> > On 10/18/2015 03:53 PM, Leonid Bloch wrote:
> >> This series fixes several issues with incorrect packet/octet
> counting in
> >> e1000's Statistic registers, fixes a bug in the packet address
> filtering
> >> procedure, and implements many MAC registers that were absent before.
> >> Additionally, some cosmetic changes are made.
> >>
> >> Leonid Bloch (6):
> >>   e1000: Cosmetic and alignment fixes
> >>   e1000: Trivial implementation of various MAC registers
> >>   e1000: Fixing the received/transmitted packets' counters
> >>   e1000: Fixing the received/transmitted octets' counters
> >>   e1000: Fixing the packet address filtering procedure
> >>   e1000: Implementing various counters
> >>
> >>  hw/net/e1000.c  | 313
> ++--
> >>  hw/net/e1000_regs.h |   8 +-
> >>  2 files changed, 236 insertions(+), 85 deletions(-)
> >>
> >
> > Looks good to me overall, just few comments in individual patches.
> >
> > A question here, is there any real user/OSes that tries to use those
> > registers? If not, maintain them sees a burden and it's a little bit
> > hard the test them unless unit-test were implemented for those
> > registers. And I'd like to know the test status of this series. At least
> > both windows and linux guest need to be tested.
> >
> > Thanks
>
> While we did not encounter any actual drivers that malfunction because
> of the lack of these registers, implementing them makes the device
> closer to Intel's specs, and reduces the chances that some OSes,
> currently or in the future, may misbehave because of the missing
> registers. The implementation of these additional registers seems as a
> natural continuation of this series, the main purpose of which is to
> fix several bugs in this device.
>
> As for testing, it was performed, obviously, in several different
> scenarios with Linux (Fedora 22) + Windows (2012R2) guests. No
> regressions (and no statistically significant deviations) were found.
> Please find representative results (TCP, 1 stream) for both Linux and
> Windows guests below:
>
> Fedora 22 guest -- receive
> 5
> +-+--+
>   |  
>  A
>   4.5 +-+   A A  A
> B
> 4 +-+   A A  
>  |
>   |  A  B
>  |
>   3.5 +-+
>  |
>   |A  
> |
> G   3 +-+  B  
> |
> b 2.5 +-+
>  |
> / |  
>  |
> s   2 +-+
>  |
>   |  A
> |
>   1.5 +-+
>  |
>   |  
>  |
> 1 +-+  A  
> |
>   0.5 +-+   A
>  |
>   A +  + + + +  + + + +  +
> +
> 0
> +-+---+--+-+-+-+--+-+-+-+--+-+
>  32B   64B   128B  256B  512B   1KB2KB   4KB   8KB  16KB  
> 32KB  64KB
>   +Buffer size+
>   Mean-old -- A  Mean-new -- B
>
>
> Fedora 22 guest -- transmit
> 2
> +-+--+
>   |  B
> A
>   1.8 +-+A
> |
>   1.6 +-+
>  |
>   |  
>  |
>   1.4 +-+ A  
>  |
>   |  
>  |
> G 1.2 +-+
>  |
> b   1 +-+
>  |
> / | A
>  |
> s 0.8 +-+
>  |
>   |  
>

Re: [Qemu-devel] [PATCH 0/6] e1000: Various fixes and registers' implementation

2015-10-21 Thread Leonid Bloch 
Hi Jason, thanks again for reviewing,

On Tue, Oct 20, 2015 at 9:37 AM, Jason Wang  wrote:
>
>
> On 10/18/2015 03:53 PM, Leonid Bloch wrote:
>> This series fixes several issues with incorrect packet/octet counting in
>> e1000's Statistic registers, fixes a bug in the packet address filtering
>> procedure, and implements many MAC registers that were absent before.
>> Additionally, some cosmetic changes are made.
>>
>> Leonid Bloch (6):
>>   e1000: Cosmetic and alignment fixes
>>   e1000: Trivial implementation of various MAC registers
>>   e1000: Fixing the received/transmitted packets' counters
>>   e1000: Fixing the received/transmitted octets' counters
>>   e1000: Fixing the packet address filtering procedure
>>   e1000: Implementing various counters
>>
>>  hw/net/e1000.c  | 313
++--
>>  hw/net/e1000_regs.h |   8 +-
>>  2 files changed, 236 insertions(+), 85 deletions(-)
>>
>
> Looks good to me overall, just few comments in individual patches.
>
> A question here, is there any real user/OSes that tries to use those
> registers? If not, maintain them sees a burden and it's a little bit
> hard the test them unless unit-test were implemented for those
> registers. And I'd like to know the test status of this series. At least
> both windows and linux guest need to be tested.
>
> Thanks

While we did not encounter any actual drivers that malfunction because of
the lack of these registers, implementing them makes the device closer to
Intel's specs, and reduces the chances that some OSes, currently or in the
future, may misbehave because of the missing registers. The implementation
of these additional registers seems as a natural continuation of this
series, the main purpose of which is to fix several bugs in this device.

As for testing, it was performed, obviously, in several different scenarios
with Linux (Fedora 22) + Windows (2012R2) guests. No regressions (and no
statistically significant deviations) were found. Please find
representative results (TCP, 1 stream) for both Linux and Windows guests
below:

Fedora 22 guest -- receive
5 +-+--+
  |A
  4.5 +-+   A A  A B
4 +-+   A A|
  |  A  B  |
  3.5 +-+  |
  |A   |
G   3 +-+  B   |
b 2.5 +-+  |
/ ||
s   2 +-+  |
  |  A |
  1.5 +-+  |
  ||
1 +-+  A   |
  0.5 +-+   A  |
  A +  + + + +  + + + +  + +
0 +-+---+--+-+-+-+--+-+-+-+--+-+
 32B   64B   128B  256B  512B   1KB2KB   4KB   8KB  16KB   32KB
 64KB
  +Buffer size+
  Mean-old -- A  Mean-new -- B


Fedora 22 guest -- transmit
2 +-+--+
  |  B A
  1.8 +-+A |
  1.6 +-+  |
  ||
  1.4 +-+ A|
  ||
G 1.2 +-+  |
b   1 +-+  |
/ | A  |
s 0.8 +-+  |
  ||
  0.6 +-+ A|
  ||
  0.4 +-+   A  |
  0.2 +-+A

Re: [Qemu-devel] [PATCH 0/6] e1000: Various fixes and registers' implementation

2015-10-20 Thread Jason Wang 


On 10/18/2015 03:53 PM, Leonid Bloch wrote:
> This series fixes several issues with incorrect packet/octet counting in
> e1000's Statistic registers, fixes a bug in the packet address filtering
> procedure, and implements many MAC registers that were absent before.
> Additionally, some cosmetic changes are made.
>
> Leonid Bloch (6):
>   e1000: Cosmetic and alignment fixes
>   e1000: Trivial implementation of various MAC registers
>   e1000: Fixing the received/transmitted packets' counters
>   e1000: Fixing the received/transmitted octets' counters
>   e1000: Fixing the packet address filtering procedure
>   e1000: Implementing various counters
>
>  hw/net/e1000.c  | 313 
> ++--
>  hw/net/e1000_regs.h |   8 +-
>  2 files changed, 236 insertions(+), 85 deletions(-)
>

Looks good to me overall, just few comments in individual patches.

A question here, is there any real user/OSes that tries to use those
registers? If not, maintain them sees a burden and it's a little bit
hard the test them unless unit-test were implemented for those
registers. And I'd like to know the test status of this series. At least
both windows and linux guest need to be tested.

Thanks

[Qemu-devel] [PATCH 0/6] e1000: Various fixes and registers' implementation

2015-10-18 Thread Leonid Bloch 
This series fixes several issues with incorrect packet/octet counting in
e1000's Statistic registers, fixes a bug in the packet address filtering
procedure, and implements many MAC registers that were absent before.
Additionally, some cosmetic changes are made.

Leonid Bloch (6):
  e1000: Cosmetic and alignment fixes
  e1000: Trivial implementation of various MAC registers
  e1000: Fixing the received/transmitted packets' counters
  e1000: Fixing the received/transmitted octets' counters
  e1000: Fixing the packet address filtering procedure
  e1000: Implementing various counters

 hw/net/e1000.c  | 313 ++--
 hw/net/e1000_regs.h |   8 +-
 2 files changed, 236 insertions(+), 85 deletions(-)

-- 
2.4.3