From: Jason Wang <jasowang@redhat.com>
To: Leonid Bloch <leonid.bloch@ravellosystems.com>
Cc: Dmitry Fleytman <dmitry@daynix.com>,
Leonid Bloch <leonid@daynix.com>,
qemu-devel@nongnu.org,
Shmulik Ladkani <shmulik.ladkani@ravellosystems.com>
Subject: Re: [Qemu-devel] [PATCH 0/6] e1000: Various fixes and registers' implementation
Date: Thu, 22 Oct 2015 15:22:17 +0800 [thread overview]
Message-ID: <56288EA9.5040105@redhat.com> (raw)
In-Reply-To: <CAOuJ2785=KzpduL3JdA_gLmn=zutBGkM1MyHqR7+w0CGi0yfFg@mail.gmail.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.
prev parent reply other threads:[~2015-10-22 7:21 UTC|newest]
Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-10-18 7:53 [Qemu-devel] [PATCH 0/6] e1000: Various fixes and registers' implementation Leonid Bloch
2015-10-18 7:53 ` [Qemu-devel] [PATCH 1/6] e1000: Cosmetic and alignment fixes Leonid Bloch
2015-10-18 7:53 ` [Qemu-devel] [PATCH 2/6] e1000: Trivial implementation of various MAC registers Leonid Bloch
2015-10-20 5:40 ` Jason Wang
2015-10-21 9:13 ` Leonid Bloch
2015-10-22 7:19 ` Jason Wang
2015-10-22 14:05 ` Leonid Bloch
2015-10-23 3:10 ` Jason Wang
2015-10-25 19:39 ` Leonid Bloch
2015-10-18 7:53 ` [Qemu-devel] [PATCH 3/6] e1000: Fixing the received/transmitted packets' counters Leonid Bloch
2015-10-18 7:53 ` [Qemu-devel] [PATCH 4/6] e1000: Fixing the received/transmitted octets' counters Leonid Bloch
2015-10-20 6:16 ` Jason Wang
2015-10-21 12:20 ` Leonid Bloch
2015-10-22 7:20 ` Jason Wang
2015-10-18 7:53 ` [Qemu-devel] [PATCH 5/6] e1000: Fixing the packet address filtering procedure Leonid Bloch
2015-10-18 7:53 ` [Qemu-devel] [PATCH 6/6] e1000: Implementing various counters Leonid Bloch
2015-10-20 6:34 ` Jason Wang
2015-10-21 9:20 ` Leonid Bloch
2015-10-20 6:37 ` [Qemu-devel] [PATCH 0/6] e1000: Various fixes and registers' implementation Jason Wang
2015-10-21 13:32 ` Leonid Bloch
2015-10-22 7:22 ` Jason Wang [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=56288EA9.5040105@redhat.com \
--to=jasowang@redhat.com \
--cc=dmitry@daynix.com \
--cc=leonid.bloch@ravellosystems.com \
--cc=leonid@daynix.com \
--cc=qemu-devel@nongnu.org \
--cc=shmulik.ladkani@ravellosystems.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).