From: Kevin Traynor <ktraynor@redhat.com>
To: Thomas Monjalon <thomas@monjalon.net>, dev@dpdk.org
Cc: ferruh.yigit@intel.com, arybchenko@solarflare.com
Subject: Re: [dpdk-dev] [PATCH] ethdev: fix xstat name of basic stats per queue
Date: Thu, 8 Oct 2020 10:10:20 +0100 [thread overview]
Message-ID: <e525afda-830c-fa5c-362d-d4ca2bf34a22@redhat.com> (raw)
In-Reply-To: <20201007214848.249516-1-thomas@monjalon.net>
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 <thomas@monjalon.net>
> ---
Acked-by: Kevin Traynor <ktraynor@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. */
>
next prev parent reply other threads:[~2020-10-08 9:10 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-10-07 21:48 Thomas Monjalon
2020-10-08 9:09 ` Bruce Richardson
2020-10-08 9:10 ` Kevin Traynor [this message]
2020-10-08 10:29 ` Asaf Penso
2020-10-09 21:00 ` Ferruh Yigit
2020-10-09 21:01 ` Ferruh Yigit
2020-10-08 15:41 ` Ferruh Yigit
2020-10-09 8:32 ` Power, Ciara
2020-10-09 8:36 ` Pattan, Reshma
2020-10-09 9:02 ` David Marchand
2020-10-09 9:08 ` Pattan, Reshma
2020-10-09 16:53 ` Kevin Laatz
2020-10-09 19:15 ` Ferruh Yigit
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=e525afda-830c-fa5c-362d-d4ca2bf34a22@redhat.com \
--to=ktraynor@redhat.com \
--cc=arybchenko@solarflare.com \
--cc=dev@dpdk.org \
--cc=ferruh.yigit@intel.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).