Re: [dpdk-dev] [RFC PATCH 1/2] ethdev: introduce internal rxq/txq stats API

[David Marchand <david.marchand@redhat.com>]

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
>
> 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_.

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.

The drivers manipulate queues in rx/tx_queue_setup dev_ops for example,
having rxq/txq_stats_get dev_ops is more consistent to me.


> 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?

But so far, I don't feel the need to break the rte_eth_stats_get API, if we
want to go to this, we can define an entirely new api to expose
standardized bits and still use the rxq/txq_stats_get internal dev_ops I
propose.


Thomas, Andrew, can you provide feedback please ?
rc1 is coming.


-- 
David Marchand