On 4/12/2019 4:45 PM, Thomas Monjalon wrote: > 12/04/2019 17:38, Ferruh Yigit: >> On 4/12/2019 4:07 PM, Thomas Monjalon wrote: >>> 11/03/2019 18:22, Ferruh Yigit: >>>> On 3/4/2019 11:18 AM, David Marchand wrote: >>>>> According to the api, the q_errors[] per queue statistic is for reception >>>>> errors not transmit errors. >>>>> This is a first cleanup on statistics before looking at oerrors. >>>>> >>>> >>>> Yes, the patchset looks aligned with the API documentation [1]. >>>> >>>> What can be the solution after cleanup? We can merge this cleanup and >>>> solution >>>> next to each-other to not leave a gap? >>> >>> I think we should merge those fixes in 19.05-rc2. >>> >>> It seems there is a lot more work to achieve on stats, so better >>> to start without waiting for the full picture. >>> >> >> The problem is "q_errors" is available only for Rx queues, and David's patch >> is >> preventing drivers to put Tx error stats into "q_errors" field. >> >> But it is clear that there is a need for a field for Tx queues errors. David >> has >> another patch to using xstats for this. But I believe xstats is making >> solution >> confusing, and now approach is unbalanced for Rx and Tx queues. >> >> I am for adding a new field for Tx queues "q_errors", and this will make >> getting >> stats and David's patch very simple. >> >> The problem with the new fields is it breaks the ABI, but we already >> increased >> the ABIVER for ethdev this release, I believe this is very good timing for >> this fix. > > If changing the stats API, we should increase the number of stats per queue: > #define RTE_ETHDEV_QUEUE_STAT_CNTRS 16 > What about 128 queues per port? 256?
We have 5 uint64_t using this [1], it will be 6 if we add new one. So having 256 queues, will cost 12K memory, this is not a big number. Is there any other concern of having large array other than possible memory waste? [1] uint64_t q_ipackets[RTE_ETHDEV_QUEUE_STAT_CNTRS]; uint64_t q_opackets[RTE_ETHDEV_QUEUE_STAT_CNTRS]; uint64_t q_ibytes[RTE_ETHDEV_QUEUE_STAT_CNTRS]; uint64_t q_obytes[RTE_ETHDEV_QUEUE_STAT_CNTRS]; uint64_t q_errors[RTE_ETHDEV_QUEUE_STAT_CNTRS];
