From: "Zhao1, Wei" <wei.zhao1@intel.com>
To: Kyle Larose <eomereadig@gmail.com>
Cc: "Wu, Jingjing" <jingjing.wu@intel.com>, "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
Date: Tue, 9 Aug 2016 02:18:10 +0000 [thread overview]
Message-ID: <A2573D2ACFCADC41BB3BE09C6DE313CA01FB799C@PGSMSX103.gar.corp.intel.com> (raw)
In-Reply-To: <CAMFWN9=Err7M2Z9O92qtiw2-sAcb=ef1=m42N6fi27W=5CEdSw@mail.gmail.com>
Hi ,Kyle Larose&Jingjing
> -----Original Message-----
> From: Kyle Larose [mailto:eomereadig@gmail.com]
> Sent: Wednesday, August 3, 2016 12:22 AM
> To: Zhao1, Wei <wei.zhao1@intel.com>
> Cc: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic inconsistent
>
> Hello Wei,
>
>
> On Tue, Aug 2, 2016 at 2:59 AM, Zhao1, Wei <wei.zhao1@intel.com> wrote:
> > Hi, Wujingjing and Kyle Larose
> >
> >
> >
> >> -----Original Message-----
> >> From: Zhao1, Wei
> >> Sent: Tuesday, August 2, 2016 11:27 AM
> >> To: Wu, Jingjing <jingjing.wu@intel.com>; Lu, Wenzhuo
> >> <wenzhuo.lu@intel.com>
> >> Cc: dev@dpdk.org
> >> Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic
> >> inconsistent
> >>
> >> Hi,Wu jingjing and wenzhuo
> >>
> >> > -----Original Message-----
> >> > From: Zhao1, Wei
> >> > Sent: Monday, August 1, 2016 4:58 PM
> >> > To: 'Kyle Larose' <eomereadig@gmail.com>
> >> > Cc: dev@dpdk.org
> >> > Subject: RE: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic
> >> > inconsistent
> >> >
> >> > Hi, Kyle Larose
> >> > The core problem is i40e has no statistic of discard bytes, that
> >> > means even if when ports are not stopped, the statistic
> >> > rx_good_bytes is consist of discard
> >> > bytes，is that reasonable? In other words, I can just minus discard
> >> > bytes from rx_good_bytes if I can get discard bytes number, that is
> >> > much
> >> better.
> >> >
> >> > -----Original Message-----
> >> > From: Kyle Larose [mailto:eomereadig@gmail.com]
> >> > Sent: Saturday, July 30, 2016 1:17 AM
> >> > To: Zhao1, Wei <wei.zhao1@intel.com>
> >> > Cc: dev@dpdk.org
> >> > Subject: Re: [dpdk-dev] [PATCH v2] net/i40e: fix Rx statistic
> >> > inconsistent
> >> >
> >> > On Fri, Jul 29, 2016 at 4:50 AM, Wei Zhao1 <wei.zhao1@intel.com>
> wrote:
> >> > > rx_good_bytes and rx_good_packets statistic is inconsistent when
> >> > > port stopped,ipackets statistic is minus the discard packets but
> >> > > rx_bytes statistic not.Also,i40e has no statistic of discard
> >> > > bytes, so we have to delete discard packets item from
> rx_good_packets statistic.
> >> > >
> >> > > Fixes: 9aace75fc82e ("i40e: fix statistics")
> >> > >
> >> > > Signed-off-by: Wei Zhao1 <wei.zhao1@intel.com>
> >> > > ---
> >> > > drivers/net/i40e/i40e_ethdev.c | 3 +--
> >> > > 1 file changed, 1 insertion(+), 2 deletions(-)
> >> > >
> >> > > diff --git a/drivers/net/i40e/i40e_ethdev.c
> >> > > b/drivers/net/i40e/i40e_ethdev.c index 11a5804..553dfd9 100644
> >> > > --- a/drivers/net/i40e/i40e_ethdev.c
> >> > > +++ b/drivers/net/i40e/i40e_ethdev.c
> >> > > @@ -2319,8 +2319,7 @@ i40e_dev_stats_get(struct rte_eth_dev
> *dev,
> >> > > struct rte_eth_stats *stats)
> >> > >
> >> > > stats->ipackets = pf->main_vsi->eth_stats.rx_unicast +
> >> > > pf->main_vsi->eth_stats.rx_multicast +
> >> > > - pf->main_vsi->eth_stats.rx_broadcast -
> >> > > - pf->main_vsi->eth_stats.rx_discards;
> >> > > + pf->main_vsi->eth_stats.rx_broadcast;
> >> > > stats->opackets = pf->main_vsi->eth_stats.tx_unicast +
> >> > > pf->main_vsi->eth_stats.tx_multicast +
> >> > > pf->main_vsi->eth_stats.tx_broadcast;
> >> > > --
> >> > > 2.5.5
> >> > >
> >> >
> >> > Is it not worse to report a received packet when no packet was
> >> > actually received by the upper layers under normal operations than
> >> > to ensure that packets and bytes are consistent when an interface
> >> > is stopped? It seems like the first case is much more likely to
> >> > occur than the
> >> second.
> >> >
> >> > Are we just introducing a new issue to fix another?
> >> >
> >> > How does this behaviour compare to other NICs? Does the ixgbe
> >> > report discarded packets in its ipackets? My reading of the driver is that
> it does not.
> >> > In fact, it does something interesting to deal with the
> >> > problem:
> >> >
> >> > from:
> >> > http://dpdk.org/browse/dpdk/tree/drivers/net/ixgbe/ixgbe_ethdev.c
> >> >
> >> > /*
> >> > * An errata states that gprc actually counts good + missed packets:
> >> > * Workaround to set gprc to summated queue packet receives */
> >> > hw_stats-
> >> > >gprc = *total_qprc;
> >> >
> >> > total_gprc is equal to the sum of the qprc per queue. Can we do
> >> > something similar on the i40e instead of adding unicast, mulitcast
> >> > and
> >> broadcast?
> >>
> >>
> >> I have checked ixgbe code about Rx statistic, in function
> >> ixgbe_read_stats_registers() we can find the rx_good_bytes and
> >> rx_good_packets statistic.
> >> It is listed below, we can see rx_good_packets is also just addition
> >> of Queue Packets Received Count and not minused discard packet
> number.
> >> Is there some wrong of understanding?
>
>
>
> My understanding of the problem can be broken into three parts:
> 1) In Unicast/Multicast/Broadcast packet counters are counting packets
> which were discarded
> 2) The corresponding byte counters count packets which were discarded.
> 3) There are no discarded byte counters.
>
> Our in bytes counter consists of the sum of in unicast, in multicast, and in
> broadcast. This sum includes discarded bytes, which we do not want, for two
> reasons. First, it would lead to misleading bitrate
> reports: people expect to see the amount of traffic actually handled.
> Second, it conflicts with the current packet counters (the counters without
> your change).
>
> Obviously if we could count the discarded bytes, we could subtract them.
> Alternatively, if we could count only received bytes which were not
> discarded, then we would not need to subtract discarded bytes from the
> received bytes counter. The ixgbe had a similar problem for packets, but
> presumably not bytes. So, I looked to it for inspiration.
>
> Presumably, the ixgbe per-queue counters count all received packets, per
> queue, and do not count discarded packets.Thus, is you take the sum of
> received packets across all queues, you can find the total number of received
> packets without discards included.
>
> Can we not do the same thing on the i40e, but for bytes? I just took a quick
> look at the datasheet, and I cannot see anything offhand that is equivalent to
> qbrc, which is what I would have used here. That said, I'm not an i40e expert,
> so I may have missed something. That's why I'm asking if it's possible. :)
>
>
>
> >>
> >> for (i = 0; i < IXGBE_QUEUE_STAT_COUNTERS; i++) { ......
> >> *total_qprc += hw_stats->qprc[i];
> >> *total_qbrc += hw_stats->qbrc[i]; ......
> >> }
> >
> > The problem is i40e has no statistic of discard bytes, so it is
> > impossible to minus discard bytes from rx_good_bytes . If you think it's not
> reasonable to Delete rx_discards iterm from rx_good_packets statistic, this
> patch will be superseded. Because I didn't find other way to correct this
> problem at present.
> >
>
> I understand that there is no discard byte counter. Are there per-queue byte
> counters on the i40e, which do not count discarded byte, with which we
> could do a similar thing as the packet counters on the ixgbe?
>
> My concern is that both solutions aside from that are not good: either we
> count less packets received than bytes received, or we count discarded
> packets as received packets. Is there any chance that a firmware update in
> the future could fix this? This sort of inconsistency, or inability to provide
> *good* stats, really sucks for a NIC.
>
> Thanks,
>
> Kyle
According to your suggestion and meeting last week with harry ,I have supply a v3 patch for this problem.
[dpdk-dev,v3] net/i40e: fix Rx statistic inconsistent , Permalink :/dev/patchwork/patch/15161/.
This fix is avoiding statistic work when port is stopped.
next prev parent reply other threads:[~2016-08-09 2:18 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2016-07-29 8:50 Wei Zhao1
2016-07-29 17:16 ` Kyle Larose
2016-08-01 8:58 ` Zhao1, Wei
2016-08-02 3:27 ` Zhao1, Wei
2016-08-02 6:59 ` Zhao1, Wei
2016-08-02 16:22 ` Kyle Larose
2016-08-09 2:18 ` Zhao1, Wei [this message]
2016-08-22 11:10 ` Thomas Monjalon
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=A2573D2ACFCADC41BB3BE09C6DE313CA01FB799C@PGSMSX103.gar.corp.intel.com \
--to=wei.zhao1@intel.com \
--cc=dev@dpdk.org \
--cc=eomereadig@gmail.com \
--cc=jingjing.wu@intel.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).