>-----Original Message-----
>From: dev <dev-boun...@dpdk.org> On Behalf Of Kevin Traynor
>Sent: Thursday, October 8, 2020 12:10 PM
>To: NBU-Contact-Thomas Monjalon <tho...@monjalon.net>; dev@dpdk.org
>Cc: ferruh.yi...@intel.com; arybche...@solarflare.com
>Subject: Re: [dpdk-dev] [PATCH] ethdev: fix xstat name of basic stats per
>queue
>
>On 07/10/2020 22:48, 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:
>>
>> https://nam11.safelinks.protection.outlook.com/?url=http%3A%2F%2Ffast.
>> dpdk.org%2Fevents%2Fslides%2FDPDK-2019-09-
>Ethernet_Statistics.pdf&
>>
>data=02%7C01%7Casafp%40nvidia.com%7C4a551495aff7412fa65108d86b6a22
>2f%7
>>
>C43083d15727340c1b7db39efd9ccc17a%7C0%7C1%7C637377450869384533&a
>mp;sda
>>
>ta=1Nn2If%2BgHcSc4hZS8FtLTBD5eyEyZeDZZP4u0%2FON5lU%3D&reserv
>ed=0
>>
>> Fixes: bd6aa172cf35 ("ethdev: fetch extended statistics with integer
>> ids")
>>
>> Signed-off-by: Thomas Monjalon <tho...@monjalon.net>
>> ---
>
>Acked-by: Kevin Traynor <ktray...@redhat.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_*``
>> +
>> +* ethdev: Renamed internal APIs:
>>
>> * ``_rte_eth_dev_callback_process()`` ->
>``rte_eth_dev_callback_process()``
>> * ``_rte_eth_dev_reset`` -> ``rte_eth_dev_internal_reset()`` diff
>> --git a/lib/librte_ethdev/rte_ethdev.c
>> b/lib/librte_ethdev/rte_ethdev.c index 48d1333b17..286c1b5966 100644
>> --- a/lib/librte_ethdev/rte_ethdev.c
>> +++ b/lib/librte_ethdev/rte_ethdev.c
>> @@ -2549,7 +2549,7 @@ rte_eth_basic_stats_get_names(struct
>rte_eth_dev *dev,
>> for (idx = 0; idx < RTE_NB_RXQ_STATS; idx++) {
>> snprintf(xstats_names[cnt_used_entries].name,
>> sizeof(xstats_names[0].name),
>> - "rx_q%u%s",
>> + "rx_q%u_%s",
>> id_queue, rte_rxq_stats_strings[idx].name);
>> cnt_used_entries++;
>> }
>> @@ -2560,7 +2560,7 @@ rte_eth_basic_stats_get_names(struct
>rte_eth_dev *dev,
>> for (idx = 0; idx < RTE_NB_TXQ_STATS; idx++) {
>> snprintf(xstats_names[cnt_used_entries].name,
>> sizeof(xstats_names[0].name),
>> - "tx_q%u%s",
>> + "tx_q%u_%s",
>> id_queue, rte_txq_stats_strings[idx].name);
>> cnt_used_entries++;
>> }
>> diff --git a/lib/librte_ethdev/rte_ethdev.h
>> b/lib/librte_ethdev/rte_ethdev.h index d2bf74f128..86434c9cae 100644
>> --- a/lib/librte_ethdev/rte_ethdev.h
>> +++ b/lib/librte_ethdev/rte_ethdev.h
>> @@ -1507,6 +1507,13 @@ struct rte_eth_xstat {
>> * An array of this structure is returned by rte_eth_xstats_get_names().
>> * It lists the names of extended statistics for a PMD. The *rte_eth_xstat*
>> * structure references these names by their array index.
>> + *
>> + * The xstats should follow a common naming scheme.
>> + * Some names are standardized in rte_stats_strings.
>> + * Examples:
>> + * - rx_missed_errors
>> + * - tx_q3_bytes
>> + * - tx_size_128_to_255_packets
>> */
>> struct rte_eth_xstat_name {
>> char name[RTE_ETH_XSTATS_NAME_SIZE]; /**< The statistic name.
>*/
>>
Thanks, Thomas, for taking care of such changes 😊
Looks like hns3 pmd should be updated as well, since it uses the same format.
