From: Stephen Hemminger <stephen@networkplumber.org>
To: David Marchand <david.marchand@redhat.com>
Cc: Thomas Monjalon <thomas@monjalon.net>, dev <dev@dpdk.org>,
Ferruh Yigit <ferruh.yigit@intel.com>,
Andrew Rybchenko <arybchenko@solarflare.com>,
"Stokes, Ian" <ian.stokes@intel.com>
Subject: Re: [dpdk-dev] [RFC PATCH 1/2] ethdev: introduce internal rxq/txq stats API
Date: Fri, 12 Apr 2019 09:05:47 -0700 [thread overview]
Message-ID: <20190412090547.72769262@shemminger-XPS-13-9360> (raw)
In-Reply-To: <CAJFAV8xqV1wyTyY3tBi=RK9SxyqX2tFT0JNt8BtbBpkju307hA@mail.gmail.com>
On Fri, 12 Apr 2019 16:32:01 +0200
David Marchand <david.marchand@redhat.com> wrote:
> On Fri, Apr 12, 2019 at 3:29 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> > 26/03/2019 10:29, David Marchand:
> > > On Tue, Mar 19, 2019 at 6:18 PM Ferruh Yigit <ferruh.yigit@intel.com>
> > wrote:
> > >
> > > > On 3/14/2019 3:13 PM, David Marchand wrote:
> > > > > Introduce a new api to retrieve per queue statistics from the
> > drivers.
> > > > > The api objectives:
> > > > > - easily add some common per queue statistics and have it exposed
> > > > > through the user xstats api while the user stats api is left
> > untouched
> > > > > - remove the limitations on the per queue statistics count (inherited
> > > > > from ixgbe) and avoid recurrent bugs on stats array overflow
> >
> > First comment, I think it would be easier to read if renaming the legacy
> > basic stats interface was in a separate patch.
> >
>
> It will be quite artificial, but I can do this yes.
>
>
> > > > The patch is adding two new dev_ops 'rxq_stats_get' & 'txq_stats_get',
> > my
> > > > concern is if it is overkill to have three dev_ops to get stats
> > > > and I am feeling that is making xstat code more complex.
> > >
> > > Having these new (meant to be) internal dev_ops has the avantage of
> > > separating the statistics reported from the drivers from the exported
> > api.
> > > This is also why I did not prefix the structure names with rte_.
> >
> > Yes, and to make it clear, please do not talk about API,
> > as it is only a driver interface.
> >
>
> Ok, so I will describe this as a "driver interface" update.
>
>
>
> > > The "complex" part is in a single place, ethdev and this is when
> > > translating from an internal representation to the exposed bits in the
> > > public apis.
> > >
> > > Would it be simpler to add 'q_ierrors' & 'q_oerrors' to 'struct
> > > > rte_eth_stats'?
> > > >
> > >
> > > It does not solve the problem of drivers that are buggy because of the
> > > limit on RTE_ETHDEV_QUEUE_STAT_CNTRS.
> > > All drivers need to be aware of this limitation of the rte_eth_stats
> > > structure.
> >
> > Yes, this limitation should be dropped.
> > I would like to see the functions rte_eth_dev_set_?x_queue_stats_mapping()
> > deprecated as they were a bad abstraction of ixgbe limitation.
> >
>
> That's a different topic from my pov, but yes, this mapping stuff should go
> away, later.
>
>
> > > And perhaps we can do the 'fix rxq q_errors' patchset [1] after this
> > > > change, so
> > > > fix can be done with less changes, although it will push the fix into
> > next
> > > > release because of the ABI break.
> > >
> > > I am fine with merging this together, we don't want to backport this
> > > anyway, right?
> >
> > No, it would make some behaviours changing in stable releases,
> > so better to not backport it and keep the buggy behaviour in old branches.
> >
>
> Since the time I had posted this RFC, I have worked on a RFC v2, I will
> post this next week, with the drivers I found time to convert.
> We will have to take a decision on what goes to -rc2 between this and the
> q_errors[] patchset.
>
>
It looks like this all about maintaining source compatiablity with older
or out of tree drivers. This is not something DPDK has to worry about.
Why not just do a mondo patch that fixes all the drivers to use the new stats API.
You do need to keep the same ethdev interface for applications, but driver
API can change.
next prev parent reply other threads:[~2019-04-12 16:05 UTC|newest]
Thread overview: 50+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-04 11:18 [dpdk-dev] [PATCH 00/12] rxq q_errors[] statistics fixes David Marchand
2019-03-04 11:18 ` [dpdk-dev] [PATCH 01/12] net/af_packet: fix incorrect rxq errors stat David Marchand
2019-03-04 11:18 ` [dpdk-dev] [PATCH 02/12] net/avp: " David Marchand
2019-03-04 12:18 ` Legacy, Allain
2019-03-04 11:18 ` [dpdk-dev] [PATCH 03/12] net/bnxt: " David Marchand
2019-03-04 11:18 ` [dpdk-dev] [PATCH 04/12] net/cxgbe: " David Marchand
2019-03-04 11:18 ` [dpdk-dev] [PATCH 05/12] net/kni: " David Marchand
2019-03-04 11:18 ` [dpdk-dev] [PATCH 06/12] net/mlx4: " David Marchand
2019-03-05 8:19 ` Shahaf Shuler
2019-03-04 11:18 ` [dpdk-dev] [PATCH 07/12] net/mlx5: " David Marchand
2019-03-05 8:18 ` Shahaf Shuler
2019-03-04 11:18 ` [dpdk-dev] [PATCH 08/12] net/null: " David Marchand
2019-03-04 11:18 ` [dpdk-dev] [PATCH 09/12] net/pcap: " David Marchand
2019-03-04 11:18 ` [dpdk-dev] [PATCH 10/12] net/ring: " David Marchand
2019-03-04 11:18 ` [dpdk-dev] [PATCH 11/12] net/szedata2: " David Marchand
2019-03-04 11:18 ` [dpdk-dev] [PATCH 12/12] net/tap: " David Marchand
2019-03-04 13:58 ` Wiles, Keith
2019-03-11 17:22 ` [dpdk-dev] [PATCH 00/12] rxq q_errors[] statistics fixes Ferruh Yigit
2019-03-11 18:09 ` David Marchand
2019-03-14 15:12 ` David Marchand
2019-03-14 15:12 ` David Marchand
2019-03-14 15:13 ` [dpdk-dev] [RFC PATCH 1/2] ethdev: introduce internal rxq/txq stats API David Marchand
2019-03-14 15:13 ` David Marchand
2019-03-14 15:13 ` [dpdk-dev] [RFC PATCH 2/2] net/af_packet: convert to new " David Marchand
2019-03-14 15:13 ` David Marchand
2019-03-15 13:30 ` [dpdk-dev] [RFC PATCH 1/2] ethdev: introduce internal " David Marchand
2019-03-15 13:30 ` David Marchand
2019-03-19 17:18 ` Ferruh Yigit
2019-03-19 17:18 ` Ferruh Yigit
2019-03-19 17:54 ` Stephen Hemminger
2019-03-19 17:54 ` Stephen Hemminger
2019-04-12 13:18 ` Thomas Monjalon
2019-04-12 13:18 ` Thomas Monjalon
2019-03-26 9:29 ` David Marchand
2019-03-26 9:29 ` David Marchand
2019-04-12 13:29 ` Thomas Monjalon
2019-04-12 13:29 ` Thomas Monjalon
2019-04-12 14:32 ` David Marchand
2019-04-12 14:32 ` David Marchand
2019-04-12 16:05 ` Stephen Hemminger [this message]
2019-04-12 16:05 ` Stephen Hemminger
2019-04-12 15:07 ` [dpdk-dev] [PATCH 00/12] rxq q_errors[] statistics fixes Thomas Monjalon
2019-04-12 15:07 ` Thomas Monjalon
2019-04-12 15:38 ` Ferruh Yigit
2019-04-12 15:38 ` Ferruh Yigit
2019-04-12 15:45 ` Thomas Monjalon
2019-04-12 15:45 ` Thomas Monjalon
2019-04-12 15:57 ` Ferruh Yigit
2019-04-12 15:57 ` Ferruh Yigit
2019-05-28 21:38 ` Yigit, Ferruh
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=20190412090547.72769262@shemminger-XPS-13-9360 \
--to=stephen@networkplumber.org \
--cc=arybchenko@solarflare.com \
--cc=david.marchand@redhat.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=ian.stokes@intel.com \
--cc=thomas@monjalon.net \
/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).