From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:40398) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpAC8-0002c7-6C for qemu-devel@nongnu.org; Thu, 22 Oct 2015 03:21:53 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZpAC4-0006u2-W8 for qemu-devel@nongnu.org; Thu, 22 Oct 2015 03:21:52 -0400 Received: from mx1.redhat.com ([209.132.183.28]:33132) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZpAC4-0006ty-Nz for qemu-devel@nongnu.org; Thu, 22 Oct 2015 03:21:48 -0400 References: <1445154799-31083-1-git-send-email-leonid.bloch@ravellosystems.com> <5625E13B.2000009@redhat.com> From: Jason Wang Message-ID: <56288EA9.5040105@redhat.com> Date: Thu, 22 Oct 2015 15:22:17 +0800 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH 0/6] e1000: Various fixes and registers' implementation List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Leonid Bloch Cc: Dmitry Fleytman , Leonid Bloch , qemu-devel@nongnu.org, Shmulik Ladkani 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 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.