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:
> http://fast.dpdk.org/events/slides/DPDK-2019-09-Ethernet_Statistics.pdf
>
> 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. */
>
