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 <jasow...@redhat.com > <mailto:jasow...@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.
