On Tue, Mar 19, 2019 at 6:18 PM Ferruh Yigit <ferruh.yi...@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