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

[Jason Wang <jasowang@redhat.com>]

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 <jasowang@redhat.com
> <mailto:jasowang@redhat.com>> 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   1KB    2KB   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                                
>     |
>       +     +      +     +     A     +      +     +     +     +      +
>     +
>     0
> A-+---A------A-----A-----+-----+------+-----+-----+-----+------+-----+
>      32B   64B   128B  256B  512B   1KB    2KB   4KB   8KB  16KB  
> 32KB  64KB
>                           +--------Buffer size--------+
>                           Mean-old -- A  Mean-new -- B
>
>
>                         Windows 2012R2 guest -- receive
>   3.5
> +-+------------------------------------------------------------------A
>       |                                                       A      A
>     B
>     3 +-+                                                            
>      |
>       |                                                 A            
>      |
>       |                                                              
>      |
>   2.5 +-+                                                            
>      |
>       |                                           A                  
>      |
> G   2 +-+                                                            
>      |
> b     |                                                              
>      |
> /     |                                     A                        
>      |
> s 1.5 +-+                                                            
>      |
>       |                              B                                
>     |
>     1 +-+                            A                                
>     |
>       |                                                              
>      |
>       |                        A                                      
>     |
>   0.5 +-+                A                                            
>     |
>       +     A      A     +     +     +      +     +     +     +      +
>     +
>     0
> A-+---+------+-----+-----+-----+------+-----+-----+-----+------+-----+
>      32B   64B   128B  256B  512B   1KB    2KB   4KB   8KB  16KB  
> 32KB  64KB
>                           +--------Buffer size--------+
>                           Mean-old -- A  Mean-new -- B
>
>
>                         Windows 2012R2 guest --transmit
>   2.5
> +-+------------------------------------------------------------------+
>       |                                                              A
>     A
>       |                                                       B      
>      |
>     2 +-+                                                     A      
>      |
>       |                                                              
>      |
>       |                                                 A            
>      |
>       |                                                              
>      |
> G 1.5 +-+                                         B                  
>      |
> b     |                                           A                  
>      |
> /     |                                                              
>      |
> s   1 +-+                                                            
>      |
>       |                              B                                
>     |
>       |                              A      A                        
>      |
>       |                        A                                      
>     |
>   0.5 +-+          A     A                                            
>     |
>       |     A                                                        
>      |
>       A     +      +     +     +     +      +     +     +     +      +
>     +
>     0
> +-+---+------+-----+-----+-----+------+-----+-----+-----+------+-----+
>      32B   64B   128B  256B  512B   1KB    2KB   4KB   8KB  16KB  
> 32KB  64KB
>                           +--------Buffer size--------+
>                           Mean-old -- A  Mean-new -- B
>
>
> Regards,
> Leonid.
>

Result looks good.

Thanks for the testing.