Re: [dpdk-dev] [PATCH] ethdev: fix xstat name of basic stats per queue

["Power, Ciara" <ciara.power@intel.com>]

Hi Ferruh, Thomas,


>-----Original Message-----
>From: Ferruh Yigit <ferruh.yigit@intel.com>
>Sent: Thursday 8 October 2020 16:42
>To: Thomas Monjalon <thomas@monjalon.net>; dev@dpdk.org; Van Haaren,
>Harry <harry.van.haaren@intel.com>; Power, Ciara <ciara.power@intel.com>;
>Laatz, Kevin <kevin.laatz@intel.com>
>Cc: arybchenko@solarflare.com
>Subject: Re: [PATCH] ethdev: fix xstat name of basic stats per queue
>
>On 10/7/2020 10:48 PM, Thomas Monjalon wrote:
>> As described in doc/guides/prog_guide/poll_mode_drv.rst,
>> the naming scheme for the xstats is parts separated with underscore:
>> 	* direction
>> 	* detail 1
>> 	* detail 2
>> 	* detail n
>> 	* unit
>> where detail 1 can be "q" followed with a queue number.
>> It means the name of the stats per queue should be rx_qN_* or tx_qN_*.
>>
>> The second underscore was missing so far.
>> Fixing the basic xstat names may be considered an API change, that's
>> why it should not be backported.
>>
>> While fixing this mistake, some examples of the naming scheme are
>> given as part of the API documentation of rte_eth_xstat_name.
>> More proposals about standardizing statistics:
>>
>> http://fast.dpdk.org/events/slides/DPDK-2019-09-Ethernet_Statistics.pd
>> f
>>
>> Fixes: bd6aa172cf35 ("ethdev: fetch extended statistics with integer
>> ids")
>>
>> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
>
>no objection,
>Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
>
>>   doc/guides/rel_notes/release_20_11.rst | 8 +++++++-
>>   lib/librte_ethdev/rte_ethdev.c         | 4 ++--
>>   lib/librte_ethdev/rte_ethdev.h         | 7 +++++++
>>   3 files changed, 16 insertions(+), 3 deletions(-)
>>
>> diff --git a/doc/guides/rel_notes/release_20_11.rst
>> b/doc/guides/rel_notes/release_20_11.rst
>> index cdf20404c9..d0d77c5d3d 100644
>> --- a/doc/guides/rel_notes/release_20_11.rst
>> +++ b/doc/guides/rel_notes/release_20_11.rst
>> @@ -200,7 +200,13 @@ API Changes
>>
>>   * ethdev: ``rte_eth_rx_descriptor_done()`` API has been deprecated.
>>
>> -* Renamed internal ethdev APIs:
>> +* ethdev: Renamed basic statistics per queue. An underscore is
>> +inserted
>> +  between the queue number and the rest of the xstat name:
>> +
>> +  * ``rx_qN*`` -> ``rx_qN_*``
>> +  * ``tx_qN*`` -> ``tx_qN_*``
>> +
>
>As far as I remember collect plugin was using xstat output, does this rename
>affects it? Or any other telemetry application relying on xstats.
>
>Harry, Ciara, Kevin, do you know anything that will be affected from rename?

I don't think this change will affect anything with telemetry itself, but I am not so familiar with the CollectD plugin, Reshma may be able to verify that. 

When using the new dpdk-telemetry.py, the name is just changed in the output which seems ok to me:
	--> /ethdev/xstats,0
	{"/ethdev/xstats": {... "rx_q0packets": 0,

Changes to:
	--> /ethdev/xstats,0
	{"/ethdev/xstats": {... "rx_q0_packets": 0,


Acked-by: Ciara Power <ciara.power@intel.com>