From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 1B57BA04BC; Fri, 9 Oct 2020 23:00:50 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 785C41D542; Fri, 9 Oct 2020 23:00:48 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id BAEB41D525 for ; Fri, 9 Oct 2020 23:00:46 +0200 (CEST) IronPort-SDR: tlNMymB2h/oCSKtJ2S5FXfzmtt/6xVfCQVa+y1NHpYuajiiPd/5kzm6sfNJJIXivyKn/3NYYrW aBCPsvRItbEw== X-IronPort-AV: E=McAfee;i="6000,8403,9769"; a="162899076" X-IronPort-AV: E=Sophos;i="5.77,356,1596524400"; d="scan'208";a="162899076" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by fmsmga104.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Oct 2020 14:00:44 -0700 IronPort-SDR: ngJYogjIN2TdnDC7zeNeHuf84krtz6Ijvv3RJOzwKhnSUQMQyUCP4zSokTmWKwIryyzGFmMrVE 6HtIWBPPnIEQ== X-IronPort-AV: E=Sophos;i="5.77,356,1596524400"; d="scan'208";a="462316269" Received: from fyigit-mobl1.ger.corp.intel.com (HELO [10.252.18.7]) ([10.252.18.7]) by orsmga004-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Oct 2020 14:00:38 -0700 To: Asaf Penso , Kevin Traynor , NBU-Contact-Thomas Monjalon , "dev@dpdk.org" , "Min Hu (Connor)" , Yisen Zhuang , "Wei Hu (Xavier)" Cc: "arybchenko@solarflare.com" References: <20201007214848.249516-1-thomas@monjalon.net> From: Ferruh Yigit Message-ID: Date: Fri, 9 Oct 2020 22:00:35 +0100 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 8bit Subject: Re: [dpdk-dev] [PATCH] ethdev: fix xstat name of basic stats per queue X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On 10/8/2020 11:29 AM, Asaf Penso wrote: >> -----Original Message----- >> From: dev On Behalf Of Kevin Traynor >> Sent: Thursday, October 8, 2020 12:10 PM >> To: NBU-Contact-Thomas Monjalon ; 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 >> >> 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 >>> --- >> >> Acked-by: Kevin Traynor >> >> >>> 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. > Added hns3 maintainers. Xavier, Connor, Yisen, can you please check the above naming scheme for the xstats and apply it to the hns3 pmd specific ones? Thanks, ferruh