DPDK patches and discussions
 help / color / mirror / Atom feed
* [PATCH v5 0/5] support dmadev/ethdev stats reset
@ 2022-12-19  9:07 Chengwen Feng
  2022-12-19  9:07 ` [PATCH v5 1/5] dmadev: support stats reset telemetry command Chengwen Feng
                   ` (10 more replies)
  0 siblings, 11 replies; 62+ messages in thread
From: Chengwen Feng @ 2022-12-19  9:07 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

This patchset contains dmadev/ethdev stats reset, and also support
hide zero for ethdev xstats.

Chengwen Feng (5):
  dmadev: support stats reset telemetry command
  telemetry: fix repeated display when callback don't set dict
  ethdev: support xstats reset telemetry command
  ethdev: telemetry xstats support hide zero
  ethdev: add newline to telemetry log string

 lib/dmadev/rte_dmadev.c   | 40 +++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c   | 57 ++++++++++++++++++++++++++++++++-------
 lib/telemetry/telemetry.c |  4 ++-
 3 files changed, 91 insertions(+), 10 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v5 1/5] dmadev: support stats reset telemetry command
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
@ 2022-12-19  9:07 ` Chengwen Feng
  2022-12-19  9:07 ` [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict Chengwen Feng
                   ` (9 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2022-12-19  9:07 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The stats reset is useful for debugging, so add it to the dmadev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/dmadev/rte_dmadev.c | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
index 4da653eec7..93e15d4972 100644
--- a/lib/dmadev/rte_dmadev.c
+++ b/lib/dmadev/rte_dmadev.c
@@ -994,6 +994,44 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+dmadev_handle_dev_stats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d __rte_unused)
+{
+	struct rte_dma_info dma_info;
+	int dev_id, ret, vchan_id;
+	const char *vchan_param;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -EINVAL;
+
+	dev_id = strtoul(params, &end_param, 0);
+
+	/* Function info_get validates dev_id so we don't need to. */
+	ret = rte_dma_info_get(dev_id, &dma_info);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* If the device has one vchan the user does not need to supply the
+	 * vchan id and only the device id is needed, no extra parameters.
+	 */
+	if (dma_info.nb_vchans == 1 && *end_param == '\0')
+		vchan_id = 0;
+	else {
+		vchan_param = strtok(end_param, ",");
+		if (!vchan_param || strlen(vchan_param) == 0 || !isdigit(*vchan_param))
+			return -EINVAL;
+
+		vchan_id = strtoul(vchan_param, &end_param, 0);
+	}
+	if (*end_param != '\0')
+		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
+
+	return rte_dma_stats_reset(dev_id, vchan_id);
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 dmadev_handle_dev_dump(const char *cmd __rte_unused,
@@ -1041,6 +1079,8 @@ RTE_INIT(dmadev_init_telemetry)
 			"Returns information for a dmadev. Parameters: int dev_id");
 	rte_telemetry_register_cmd("/dmadev/stats", dmadev_handle_dev_stats,
 			"Returns the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
+	rte_telemetry_register_cmd("/dmadev/stats_reset", dmadev_handle_dev_stats_reset,
+			"Reset the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/dmadev/dump", dmadev_handle_dev_dump,
 			"Returns dump information for a dmadev. Parameters: int dev_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
  2022-12-19  9:07 ` [PATCH v5 1/5] dmadev: support stats reset telemetry command Chengwen Feng
@ 2022-12-19  9:07 ` Chengwen Feng
  2022-12-19  9:33   ` Bruce Richardson
  2022-12-19  9:07 ` [PATCH v5 3/5] ethdev: support xstats reset telemetry command Chengwen Feng
                   ` (8 subsequent siblings)
  10 siblings, 1 reply; 62+ messages in thread
From: Chengwen Feng @ 2022-12-19  9:07 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

When telemetry callback didn't set dict and return a non-negative
number, the telemetry will repeat to display the last result.

Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/telemetry/telemetry.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index 8fbb4f3060..a6c84e3499 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -334,8 +334,10 @@ static void
 perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
 {
 	struct rte_tel_data data;
+	int ret;
 
-	int ret = fn(cmd, param, &data);
+	rte_tel_data_start_dict(&data);
+	ret = fn(cmd, param, &data);
 	if (ret < 0) {
 		char out_buf[MAX_CMD_LEN + 10];
 		int used = snprintf(out_buf, sizeof(out_buf), "{\"%.*s\":null}",
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v5 3/5] ethdev: support xstats reset telemetry command
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
  2022-12-19  9:07 ` [PATCH v5 1/5] dmadev: support stats reset telemetry command Chengwen Feng
  2022-12-19  9:07 ` [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict Chengwen Feng
@ 2022-12-19  9:07 ` Chengwen Feng
  2022-12-19  9:07 ` [PATCH v5 4/5] ethdev: telemetry xstats support hide zero Chengwen Feng
                   ` (7 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2022-12-19  9:07 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The xstats reset is useful for debugging, so add it to the ethdev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 5d5e18db1e..0774de81b0 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5915,6 +5915,27 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d __rte_unused)
+{
+	char *end_param;
+	int port_id;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -1;
+
+	port_id = strtoul(params, &end_param, 0);
+	if (*end_param != '\0')
+		RTE_ETHDEV_LOG(NOTICE,
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
+	if (!rte_eth_dev_is_valid_port(port_id))
+		return -1;
+
+	return rte_eth_xstats_reset(port_id);
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
@@ -6328,6 +6349,8 @@ RTE_INIT(ethdev_init_telemetry)
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
 			"Returns the extended stats for a port. Parameters: int port_id");
+	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
+			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
 			"Returns dump private information for a port. Parameters: int port_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v5 4/5] ethdev: telemetry xstats support hide zero
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
                   ` (2 preceding siblings ...)
  2022-12-19  9:07 ` [PATCH v5 3/5] ethdev: support xstats reset telemetry command Chengwen Feng
@ 2022-12-19  9:07 ` Chengwen Feng
  2022-12-19  9:07 ` [PATCH v5 5/5] ethdev: add newline to telemetry log string Chengwen Feng
                   ` (6 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2022-12-19  9:07 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The number of xstats may be large, after the hide zero option is added,
only non-zero values can be displayed.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 0774de81b0..2075086eeb 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5870,20 +5870,33 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 {
 	struct rte_eth_xstat *eth_xstats;
 	struct rte_eth_xstat_name *xstat_names;
+	char *end_param, *hide_param;
 	int port_id, num_xstats;
+	int hide_zero = 0;
 	int i, ret;
-	char *end_param;
 
 	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
 		return -1;
 
 	port_id = strtoul(params, &end_param, 0);
-	if (*end_param != '\0')
-		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
+	if (*end_param != '\0') {
+		hide_param = strtok(end_param, ",");
+		if (!hide_param || strlen(hide_param) == 0 || !isdigit(*hide_param))
+			return -EINVAL;
+		hide_zero = strtoul(hide_param, &end_param, 0);
+		if (*end_param != '\0')
+			RTE_ETHDEV_LOG(NOTICE,
+				"Extra parameters passed to ethdev telemetry command, ignoring\n");
+		if (hide_zero != 0 && hide_zero != 1) {
+			hide_zero = !!hide_zero;
+			RTE_ETHDEV_LOG(NOTICE,
+				"Hide zero parameter is non-boolean, cast to boolean\n");
+		}
+	}
+
 	num_xstats = rte_eth_xstats_get(port_id, NULL, 0);
 	if (num_xstats < 0)
 		return -1;
@@ -5908,9 +5921,12 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	}
 
 	rte_tel_data_start_dict(d);
-	for (i = 0; i < num_xstats; i++)
+	for (i = 0; i < num_xstats; i++) {
+		if (hide_zero && eth_xstats[i].value == 0)
+			continue;
 		rte_tel_data_add_dict_u64(d, xstat_names[i].name,
 				eth_xstats[i].value);
+	}
 	free(eth_xstats);
 	return 0;
 }
@@ -6348,7 +6364,7 @@ RTE_INIT(ethdev_init_telemetry)
 	rte_telemetry_register_cmd("/ethdev/stats", eth_dev_handle_port_stats,
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
-			"Returns the extended stats for a port. Parameters: int port_id");
+			"Returns the extended stats for a port. Parameters: int port_id, hide_zero (Optional, specify whether to hide zero values)");
 	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
 			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v5 5/5] ethdev: add newline to telemetry log string
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
                   ` (3 preceding siblings ...)
  2022-12-19  9:07 ` [PATCH v5 4/5] ethdev: telemetry xstats support hide zero Chengwen Feng
@ 2022-12-19  9:07 ` Chengwen Feng
  2022-12-26  4:55 ` [PATCH v5 0/5] support dmadev/ethdev stats reset fengchengwen
                   ` (5 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2022-12-19  9:07 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The telemetry related code may invoke RTE_ETHDEV_LOG to display
information, the newline character is not added automatically to the
RTE_ETHDEV_LOG, therefore, the newline character must be explicitly
added to the log string.

Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 2075086eeb..30f79b7d4c 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5968,7 +5968,7 @@ eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -EINVAL;
 
@@ -6010,7 +6010,7 @@ eth_dev_handle_port_link_status(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
@@ -6048,7 +6048,7 @@ eth_dev_handle_port_info(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -EINVAL;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict
  2022-12-19  9:07 ` [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict Chengwen Feng
@ 2022-12-19  9:33   ` Bruce Richardson
  2022-12-26  4:53     ` fengchengwen
  0 siblings, 1 reply; 62+ messages in thread
From: Bruce Richardson @ 2022-12-19  9:33 UTC (permalink / raw)
  To: Chengwen Feng
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On Mon, Dec 19, 2022 at 09:07:20AM +0000, Chengwen Feng wrote:
> When telemetry callback didn't set dict and return a non-negative
> number, the telemetry will repeat to display the last result.
> 
> Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---

Hi Chengwen,

I'm a little curious about this bug. Can you describe some steps to
reproduce it as I'm curious as to exactly what is happening. The fix seems
a little strange to me so I'd like to investigate a little more to see if
other approaches might work.

Thanks,
/Bruce

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict
  2022-12-19  9:33   ` Bruce Richardson
@ 2022-12-26  4:53     ` fengchengwen
  2023-01-06 16:07       ` Bruce Richardson
  0 siblings, 1 reply; 62+ messages in thread
From: fengchengwen @ 2022-12-26  4:53 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On 2022/12/19 17:33, Bruce Richardson wrote:
> On Mon, Dec 19, 2022 at 09:07:20AM +0000, Chengwen Feng wrote:
>> When telemetry callback didn't set dict and return a non-negative
>> number, the telemetry will repeat to display the last result.
>>
>> Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
> 
> Hi Chengwen,
> 
> I'm a little curious about this bug. Can you describe some steps to
> reproduce it as I'm curious as to exactly what is happening. The fix seems
> a little strange to me so I'd like to investigate a little more to see if
> other approaches might work.

Hi Bruce,

Sorry for late reply.

The steps:
  1. applay "[PATCH v5 1/5] dmadev: support stats reset telemetry command"
  2. compile
  3. start dpdk-dma: dpdk-dma -a DMA.BDF -a NIC.BDF -- -c hw
  4. start telemetry, and execute /dmadev/stats,0, and then /dmadev/stats_reset,0
     the output of /dmadev/stats_reset,0 will be the same of previous cmd "/dmadev/stats,0"
     e.g. my environment:
--> /dmadev/stats,0
{
  "/dmadev/stats": {
    "submitted": 23,
    "completed": 23,
    "errors": 0
  }
}
--> /dmadev/stats_reset,0
{
  "/dmadev/stats_reset": {
    "submitted": 23,
    "completed": 23,
    "errors": 0
  }
}

The rootcause is that the /dmadev/stats_reset don't set the outer parameter "struct rte_tel_data *info"
and return zero.


BTW: although the telemetry mainly used to query, but some reset counter maybe usefull, and it already
exist like: "/eventdev/rxa_stats_reset" and this patchset.


> 
> Thanks,
> /Bruce
> .
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 0/5] support dmadev/ethdev stats reset
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
                   ` (4 preceding siblings ...)
  2022-12-19  9:07 ` [PATCH v5 5/5] ethdev: add newline to telemetry log string Chengwen Feng
@ 2022-12-26  4:55 ` fengchengwen
  2023-01-11 12:02 ` [PATCH v2 " Chengwen Feng
                   ` (4 subsequent siblings)
  10 siblings, 0 replies; 62+ messages in thread
From: fengchengwen @ 2022-12-26  4:55 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

Sorry for the patchset version set wrong, it should be 'PATCH v1' not 'PATCH v5'

Will fix in V2

On 2022/12/19 17:07, Chengwen Feng wrote:
> This patchset contains dmadev/ethdev stats reset, and also support
> hide zero for ethdev xstats.
> 
> Chengwen Feng (5):
>   dmadev: support stats reset telemetry command
>   telemetry: fix repeated display when callback don't set dict
>   ethdev: support xstats reset telemetry command
>   ethdev: telemetry xstats support hide zero
>   ethdev: add newline to telemetry log string
> 
>  lib/dmadev/rte_dmadev.c   | 40 +++++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.c   | 57 ++++++++++++++++++++++++++++++++-------
>  lib/telemetry/telemetry.c |  4 ++-
>  3 files changed, 91 insertions(+), 10 deletions(-)
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict
  2022-12-26  4:53     ` fengchengwen
@ 2023-01-06 16:07       ` Bruce Richardson
  2023-01-06 17:33         ` Bruce Richardson
  0 siblings, 1 reply; 62+ messages in thread
From: Bruce Richardson @ 2023-01-06 16:07 UTC (permalink / raw)
  To: fengchengwen
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On Mon, Dec 26, 2022 at 12:53:57PM +0800, fengchengwen wrote:
> On 2022/12/19 17:33, Bruce Richardson wrote:
> > On Mon, Dec 19, 2022 at 09:07:20AM +0000, Chengwen Feng wrote:
> >> When telemetry callback didn't set dict and return a non-negative
> >> number, the telemetry will repeat to display the last result.
> >>
> >> Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
> >> Cc: stable@dpdk.org
> >>
> >> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> >> ---
> > 
> > Hi Chengwen,
> > 
> > I'm a little curious about this bug. Can you describe some steps to
> > reproduce it as I'm curious as to exactly what is happening. The fix seems
> > a little strange to me so I'd like to investigate a little more to see if
> > other approaches might work.
> 
> Hi Bruce,
> 
> Sorry for late reply.
> 
> The steps:
>   1. applay "[PATCH v5 1/5] dmadev: support stats reset telemetry command"
>   2. compile
>   3. start dpdk-dma: dpdk-dma -a DMA.BDF -a NIC.BDF -- -c hw
>   4. start telemetry, and execute /dmadev/stats,0, and then /dmadev/stats_reset,0
>      the output of /dmadev/stats_reset,0 will be the same of previous cmd "/dmadev/stats,0"
>      e.g. my environment:
> --> /dmadev/stats,0
> {
>   "/dmadev/stats": {
>     "submitted": 23,
>     "completed": 23,
>     "errors": 0
>   }
> }
> --> /dmadev/stats_reset,0
> {
>   "/dmadev/stats_reset": {
>     "submitted": 23,
>     "completed": 23,
>     "errors": 0
>   }
> }
> 
> The rootcause is that the /dmadev/stats_reset don't set the outer parameter "struct rte_tel_data *info"
> and return zero.
>
Thanks for the fuller explanation, I'll hopefully test it out myself.

However, in the meantime, looking at the telemetry library code, would the
following change work rather than explicitly always setting the telemetry
data to a dictionary by default? Zeroing the data by default sets it to a
null return which is what you probably want as default rather than an empty
dictionary. (And it's also a smaller diff)

/Bruce

diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index 8fbb4f3060..7b905355cd 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
 static void
 perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
 {
-       struct rte_tel_data data;
+       struct rte_tel_data data = {0};
 
        int ret = fn(cmd, param, &data);
        if (ret < 0) {
 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict
  2023-01-06 16:07       ` Bruce Richardson
@ 2023-01-06 17:33         ` Bruce Richardson
  2023-01-11 12:38           ` fengchengwen
  0 siblings, 1 reply; 62+ messages in thread
From: Bruce Richardson @ 2023-01-06 17:33 UTC (permalink / raw)
  To: fengchengwen
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On Fri, Jan 06, 2023 at 04:07:45PM +0000, Bruce Richardson wrote:
> On Mon, Dec 26, 2022 at 12:53:57PM +0800, fengchengwen wrote:
> > On 2022/12/19 17:33, Bruce Richardson wrote:
> > > On Mon, Dec 19, 2022 at 09:07:20AM +0000, Chengwen Feng wrote:
> > >> When telemetry callback didn't set dict and return a non-negative
> > >> number, the telemetry will repeat to display the last result.
> > >>
> > >> Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
> > >> Cc: stable@dpdk.org
> > >>
> > >> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > >> ---
> > > 
> > > Hi Chengwen,
> > > 
> > > I'm a little curious about this bug. Can you describe some steps to
> > > reproduce it as I'm curious as to exactly what is happening. The fix seems
> > > a little strange to me so I'd like to investigate a little more to see if
> > > other approaches might work.
> > 
> > Hi Bruce,
> > 
> > Sorry for late reply.
> > 
> > The steps:
> >   1. applay "[PATCH v5 1/5] dmadev: support stats reset telemetry command"
> >   2. compile
> >   3. start dpdk-dma: dpdk-dma -a DMA.BDF -a NIC.BDF -- -c hw
> >   4. start telemetry, and execute /dmadev/stats,0, and then /dmadev/stats_reset,0
> >      the output of /dmadev/stats_reset,0 will be the same of previous cmd "/dmadev/stats,0"
> >      e.g. my environment:
> > --> /dmadev/stats,0
> > {
> >   "/dmadev/stats": {
> >     "submitted": 23,
> >     "completed": 23,
> >     "errors": 0
> >   }
> > }
> > --> /dmadev/stats_reset,0
> > {
> >   "/dmadev/stats_reset": {
> >     "submitted": 23,
> >     "completed": 23,
> >     "errors": 0
> >   }
> > }
> > 
> > The rootcause is that the /dmadev/stats_reset don't set the outer parameter "struct rte_tel_data *info"
> > and return zero.
> >
> Thanks for the fuller explanation, I'll hopefully test it out myself.
> 
> However, in the meantime, looking at the telemetry library code, would the
> following change work rather than explicitly always setting the telemetry
> data to a dictionary by default? Zeroing the data by default sets it to a
> null return which is what you probably want as default rather than an empty
> dictionary. (And it's also a smaller diff)
> 
> /Bruce
> 
> diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
> index 8fbb4f3060..7b905355cd 100644
> --- a/lib/telemetry/telemetry.c
> +++ b/lib/telemetry/telemetry.c
> @@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
>  static void
>  perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
>  {
> -       struct rte_tel_data data;
> +       struct rte_tel_data data = {0};
>  
>         int ret = fn(cmd, param, &data);
>         if (ret < 0) {
>

I've handily reproduced the issue using the instructions you gave above,
thanks for those.

Based on that, and looking a little deeper:

* I think it is an error with the reset function not to initialize and
  complete the return value. I believe that for each telemetry callback we
  should require that the callback fill in a valid value on success. For
  reset, some options could be just a string "OK", or an array just
  containing "[0]", or similar.

* Given that point above, I do agree though that the "data" parameter
  should be properly initialized on entry to the callbacks. However, I feel
  that the correct init should be the empty/null value, as is given in the
  patch above.

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 0/5] support dmadev/ethdev stats reset
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
                   ` (5 preceding siblings ...)
  2022-12-26  4:55 ` [PATCH v5 0/5] support dmadev/ethdev stats reset fengchengwen
@ 2023-01-11 12:02 ` Chengwen Feng
  2023-01-11 12:02   ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
                     ` (4 more replies)
  2023-01-11 12:06 ` [PATCH v2 0/5] support dmadev/ethdev stats reset Chengwen Feng
                   ` (3 subsequent siblings)
  10 siblings, 5 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:02 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

This patchset contains dmadev/ethdev stats reset, and also support
hide zero for ethdev xstats and two telemetry related bugs.

Chengwen Feng (5):
  dmadev: support stats reset telemetry command
  telemetry: fix repeat display when callback don't set dict
  ethdev: add newline to telemetry log string
  ethdev: support xstats reset telemetry command
  ethdev: telemetry xstats support hide zero

 lib/dmadev/rte_dmadev.c   | 46 ++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c   | 63 +++++++++++++++++++++++++++++++++------
 lib/telemetry/telemetry.c |  2 +-
 3 files changed, 101 insertions(+), 10 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 1/5] dmadev: support stats reset telemetry command
  2023-01-11 12:02 ` [PATCH v2 " Chengwen Feng
@ 2023-01-11 12:02   ` Chengwen Feng
  2023-01-11 12:02   ` [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict Chengwen Feng
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:02 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The stats reset is useful for debugging, so add it to the dmadev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/dmadev/rte_dmadev.c | 46 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
index 4da653eec7..79593cdbe2 100644
--- a/lib/dmadev/rte_dmadev.c
+++ b/lib/dmadev/rte_dmadev.c
@@ -994,6 +994,50 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+dmadev_handle_dev_stats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d)
+{
+	struct rte_dma_info dma_info;
+	int dev_id, ret, vchan_id;
+	const char *vchan_param;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -EINVAL;
+
+	dev_id = strtoul(params, &end_param, 0);
+
+	/* Function info_get validates dev_id so we don't need to. */
+	ret = rte_dma_info_get(dev_id, &dma_info);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* If the device has one vchan the user does not need to supply the
+	 * vchan id and only the device id is needed, no extra parameters.
+	 */
+	if (dma_info.nb_vchans == 1 && *end_param == '\0')
+		vchan_id = 0;
+	else {
+		vchan_param = strtok(end_param, ",");
+		if (!vchan_param || strlen(vchan_param) == 0 || !isdigit(*vchan_param))
+			return -EINVAL;
+
+		vchan_id = strtoul(vchan_param, &end_param, 0);
+	}
+	if (*end_param != '\0')
+		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
+
+	ret = rte_dma_stats_reset(dev_id, vchan_id);
+	if (ret == 0) {
+		rte_tel_data_start_dict(d);
+		rte_tel_data_string(d, "success");
+	}
+
+	return ret;
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 dmadev_handle_dev_dump(const char *cmd __rte_unused,
@@ -1041,6 +1085,8 @@ RTE_INIT(dmadev_init_telemetry)
 			"Returns information for a dmadev. Parameters: int dev_id");
 	rte_telemetry_register_cmd("/dmadev/stats", dmadev_handle_dev_stats,
 			"Returns the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
+	rte_telemetry_register_cmd("/dmadev/stats_reset", dmadev_handle_dev_stats_reset,
+			"Reset the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/dmadev/dump", dmadev_handle_dev_dump,
 			"Returns dump information for a dmadev. Parameters: int dev_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict
  2023-01-11 12:02 ` [PATCH v2 " Chengwen Feng
  2023-01-11 12:02   ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
@ 2023-01-11 12:02   ` Chengwen Feng
  2023-01-11 12:02   ` [PATCH v2 3/5] ethdev: add newline to telemetry log string Chengwen Feng
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:02 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

When telemetry callback didn't set dict and return a non-negative
number, the telemetry will repeat to display the last result. This
patch zero the dict to avoid the problem.

Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/telemetry/telemetry.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index 8fbb4f3060..7b905355cd 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
 static void
 perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
 {
-	struct rte_tel_data data;
+	struct rte_tel_data data = {0};
 
 	int ret = fn(cmd, param, &data);
 	if (ret < 0) {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 3/5] ethdev: add newline to telemetry log string
  2023-01-11 12:02 ` [PATCH v2 " Chengwen Feng
  2023-01-11 12:02   ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
  2023-01-11 12:02   ` [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict Chengwen Feng
@ 2023-01-11 12:02   ` Chengwen Feng
  2023-01-11 12:02   ` [PATCH v2 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
  2023-01-11 12:02   ` [PATCH v2 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
  4 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:02 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The telemetry related code may invoke RTE_ETHDEV_LOG to display
information, the newline character is not added automatically to the
RTE_ETHDEV_LOG, therefore, the newline character must be explicitly
added to the log string.

Fixes: 5514319e7b43 ("telemetry: fix passing full params string to command")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 5d5e18db1e..9eeae61024 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5880,7 +5880,7 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
@@ -5931,7 +5931,7 @@ eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -EINVAL;
 
@@ -5973,7 +5973,7 @@ eth_dev_handle_port_link_status(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
@@ -6011,7 +6011,7 @@ eth_dev_handle_port_info(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -EINVAL;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 4/5] ethdev: support xstats reset telemetry command
  2023-01-11 12:02 ` [PATCH v2 " Chengwen Feng
                     ` (2 preceding siblings ...)
  2023-01-11 12:02   ` [PATCH v2 3/5] ethdev: add newline to telemetry log string Chengwen Feng
@ 2023-01-11 12:02   ` Chengwen Feng
  2023-01-11 12:02   ` [PATCH v2 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
  4 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:02 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The xstats reset is useful for debugging, so add it to the ethdev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9eeae61024..2fc593b865 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5915,6 +5915,33 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d)
+{
+	int port_id, ret;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -1;
+
+	port_id = strtoul(params, &end_param, 0);
+	if (*end_param != '\0')
+		RTE_ETHDEV_LOG(NOTICE,
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
+	if (!rte_eth_dev_is_valid_port(port_id))
+		return -1;
+
+	ret = rte_eth_xstats_reset(port_id);
+	if (ret == 0) {
+		rte_tel_data_start_dict(d);
+		rte_tel_data_string(d, "success");
+	}
+
+	return ret;
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
@@ -6328,6 +6355,8 @@ RTE_INIT(ethdev_init_telemetry)
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
 			"Returns the extended stats for a port. Parameters: int port_id");
+	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
+			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
 			"Returns dump private information for a port. Parameters: int port_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 5/5] ethdev: telemetry xstats support hide zero
  2023-01-11 12:02 ` [PATCH v2 " Chengwen Feng
                     ` (3 preceding siblings ...)
  2023-01-11 12:02   ` [PATCH v2 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
@ 2023-01-11 12:02   ` Chengwen Feng
  4 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:02 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The number of xstats may be large, after the hide zero option is added,
only non-zero values can be displayed.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 2fc593b865..77cacc0829 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5870,20 +5870,33 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 {
 	struct rte_eth_xstat *eth_xstats;
 	struct rte_eth_xstat_name *xstat_names;
+	char *end_param, *hide_param;
 	int port_id, num_xstats;
+	int hide_zero = 0;
 	int i, ret;
-	char *end_param;
 
 	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
 		return -1;
 
 	port_id = strtoul(params, &end_param, 0);
-	if (*end_param != '\0')
-		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
+	if (*end_param != '\0') {
+		hide_param = strtok(end_param, ",");
+		if (!hide_param || strlen(hide_param) == 0 || !isdigit(*hide_param))
+			return -EINVAL;
+		hide_zero = strtoul(hide_param, &end_param, 0);
+		if (*end_param != '\0')
+			RTE_ETHDEV_LOG(NOTICE,
+				"Extra parameters passed to ethdev telemetry command, ignoring\n");
+		if (hide_zero != 0 && hide_zero != 1) {
+			hide_zero = !!hide_zero;
+			RTE_ETHDEV_LOG(NOTICE,
+				"Hide zero parameter is non-boolean, cast to boolean\n");
+		}
+	}
+
 	num_xstats = rte_eth_xstats_get(port_id, NULL, 0);
 	if (num_xstats < 0)
 		return -1;
@@ -5908,9 +5921,12 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	}
 
 	rte_tel_data_start_dict(d);
-	for (i = 0; i < num_xstats; i++)
+	for (i = 0; i < num_xstats; i++) {
+		if (hide_zero && eth_xstats[i].value == 0)
+			continue;
 		rte_tel_data_add_dict_u64(d, xstat_names[i].name,
 				eth_xstats[i].value);
+	}
 	free(eth_xstats);
 	return 0;
 }
@@ -6354,7 +6370,7 @@ RTE_INIT(ethdev_init_telemetry)
 	rte_telemetry_register_cmd("/ethdev/stats", eth_dev_handle_port_stats,
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
-			"Returns the extended stats for a port. Parameters: int port_id");
+			"Returns the extended stats for a port. Parameters: int port_id, hide_zero (Optional, specify whether to hide zero values)");
 	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
 			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 0/5] support dmadev/ethdev stats reset
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
                   ` (6 preceding siblings ...)
  2023-01-11 12:02 ` [PATCH v2 " Chengwen Feng
@ 2023-01-11 12:06 ` Chengwen Feng
  2023-01-11 12:06   ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
                     ` (5 more replies)
  2023-01-20  3:25 ` [PATCH v3 " Chengwen Feng
                   ` (2 subsequent siblings)
  10 siblings, 6 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:06 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

This patchset contains dmadev/ethdev stats reset, and also support
hide zero for ethdev xstats and two telemetry related bugs.

Chengwen Feng (5):
  dmadev: support stats reset telemetry command
  telemetry: fix repeat display when callback don't set dict
  ethdev: add newline to telemetry log string
  ethdev: support xstats reset telemetry command
  ethdev: telemetry xstats support hide zero

---
v2: 
* address Bruce's comment on 2/5 patch.
* support output success when stats reset.

 lib/dmadev/rte_dmadev.c   | 46 ++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c   | 63 +++++++++++++++++++++++++++++++++------
 lib/telemetry/telemetry.c |  2 +-
 3 files changed, 101 insertions(+), 10 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 1/5] dmadev: support stats reset telemetry command
  2023-01-11 12:06 ` [PATCH v2 0/5] support dmadev/ethdev stats reset Chengwen Feng
@ 2023-01-11 12:06   ` Chengwen Feng
  2023-01-11 13:58     ` Bruce Richardson
  2023-01-11 12:06   ` [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict Chengwen Feng
                     ` (4 subsequent siblings)
  5 siblings, 1 reply; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:06 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The stats reset is useful for debugging, so add it to the dmadev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/dmadev/rte_dmadev.c | 46 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 46 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
index 4da653eec7..79593cdbe2 100644
--- a/lib/dmadev/rte_dmadev.c
+++ b/lib/dmadev/rte_dmadev.c
@@ -994,6 +994,50 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+dmadev_handle_dev_stats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d)
+{
+	struct rte_dma_info dma_info;
+	int dev_id, ret, vchan_id;
+	const char *vchan_param;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -EINVAL;
+
+	dev_id = strtoul(params, &end_param, 0);
+
+	/* Function info_get validates dev_id so we don't need to. */
+	ret = rte_dma_info_get(dev_id, &dma_info);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* If the device has one vchan the user does not need to supply the
+	 * vchan id and only the device id is needed, no extra parameters.
+	 */
+	if (dma_info.nb_vchans == 1 && *end_param == '\0')
+		vchan_id = 0;
+	else {
+		vchan_param = strtok(end_param, ",");
+		if (!vchan_param || strlen(vchan_param) == 0 || !isdigit(*vchan_param))
+			return -EINVAL;
+
+		vchan_id = strtoul(vchan_param, &end_param, 0);
+	}
+	if (*end_param != '\0')
+		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
+
+	ret = rte_dma_stats_reset(dev_id, vchan_id);
+	if (ret == 0) {
+		rte_tel_data_start_dict(d);
+		rte_tel_data_string(d, "success");
+	}
+
+	return ret;
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 dmadev_handle_dev_dump(const char *cmd __rte_unused,
@@ -1041,6 +1085,8 @@ RTE_INIT(dmadev_init_telemetry)
 			"Returns information for a dmadev. Parameters: int dev_id");
 	rte_telemetry_register_cmd("/dmadev/stats", dmadev_handle_dev_stats,
 			"Returns the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
+	rte_telemetry_register_cmd("/dmadev/stats_reset", dmadev_handle_dev_stats_reset,
+			"Reset the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/dmadev/dump", dmadev_handle_dev_dump,
 			"Returns dump information for a dmadev. Parameters: int dev_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict
  2023-01-11 12:06 ` [PATCH v2 0/5] support dmadev/ethdev stats reset Chengwen Feng
  2023-01-11 12:06   ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
@ 2023-01-11 12:06   ` Chengwen Feng
  2023-01-11 14:01     ` Bruce Richardson
  2023-01-11 12:06   ` [PATCH v2 3/5] ethdev: add newline to telemetry log string Chengwen Feng
                     ` (3 subsequent siblings)
  5 siblings, 1 reply; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:06 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

When telemetry callback didn't set dict and return a non-negative
number, the telemetry will repeat to display the last result. This
patch zero the dict to avoid the problem.

Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/telemetry/telemetry.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index 8fbb4f3060..7b905355cd 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
 static void
 perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
 {
-	struct rte_tel_data data;
+	struct rte_tel_data data = {0};
 
 	int ret = fn(cmd, param, &data);
 	if (ret < 0) {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 3/5] ethdev: add newline to telemetry log string
  2023-01-11 12:06 ` [PATCH v2 0/5] support dmadev/ethdev stats reset Chengwen Feng
  2023-01-11 12:06   ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
  2023-01-11 12:06   ` [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict Chengwen Feng
@ 2023-01-11 12:06   ` Chengwen Feng
  2023-01-11 12:06   ` [PATCH v2 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
                     ` (2 subsequent siblings)
  5 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:06 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The telemetry related code may invoke RTE_ETHDEV_LOG to display
information, the newline character is not added automatically to the
RTE_ETHDEV_LOG, therefore, the newline character must be explicitly
added to the log string.

Fixes: 5514319e7b43 ("telemetry: fix passing full params string to command")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 5d5e18db1e..9eeae61024 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5880,7 +5880,7 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
@@ -5931,7 +5931,7 @@ eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -EINVAL;
 
@@ -5973,7 +5973,7 @@ eth_dev_handle_port_link_status(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
@@ -6011,7 +6011,7 @@ eth_dev_handle_port_info(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -EINVAL;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 4/5] ethdev: support xstats reset telemetry command
  2023-01-11 12:06 ` [PATCH v2 0/5] support dmadev/ethdev stats reset Chengwen Feng
                     ` (2 preceding siblings ...)
  2023-01-11 12:06   ` [PATCH v2 3/5] ethdev: add newline to telemetry log string Chengwen Feng
@ 2023-01-11 12:06   ` Chengwen Feng
  2023-01-11 12:06   ` [PATCH v2 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
  2023-01-11 12:36   ` [PATCH v2 0/5] support dmadev/ethdev stats reset fengchengwen
  5 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:06 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The xstats reset is useful for debugging, so add it to the ethdev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 29 +++++++++++++++++++++++++++++
 1 file changed, 29 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9eeae61024..2fc593b865 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5915,6 +5915,33 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d)
+{
+	int port_id, ret;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -1;
+
+	port_id = strtoul(params, &end_param, 0);
+	if (*end_param != '\0')
+		RTE_ETHDEV_LOG(NOTICE,
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
+	if (!rte_eth_dev_is_valid_port(port_id))
+		return -1;
+
+	ret = rte_eth_xstats_reset(port_id);
+	if (ret == 0) {
+		rte_tel_data_start_dict(d);
+		rte_tel_data_string(d, "success");
+	}
+
+	return ret;
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
@@ -6328,6 +6355,8 @@ RTE_INIT(ethdev_init_telemetry)
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
 			"Returns the extended stats for a port. Parameters: int port_id");
+	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
+			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
 			"Returns dump private information for a port. Parameters: int port_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v2 5/5] ethdev: telemetry xstats support hide zero
  2023-01-11 12:06 ` [PATCH v2 0/5] support dmadev/ethdev stats reset Chengwen Feng
                     ` (3 preceding siblings ...)
  2023-01-11 12:06   ` [PATCH v2 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
@ 2023-01-11 12:06   ` Chengwen Feng
  2023-01-11 14:08     ` Bruce Richardson
  2023-01-11 12:36   ` [PATCH v2 0/5] support dmadev/ethdev stats reset fengchengwen
  5 siblings, 1 reply; 62+ messages in thread
From: Chengwen Feng @ 2023-01-11 12:06 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The number of xstats may be large, after the hide zero option is added,
only non-zero values can be displayed.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++------
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 2fc593b865..77cacc0829 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5870,20 +5870,33 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 {
 	struct rte_eth_xstat *eth_xstats;
 	struct rte_eth_xstat_name *xstat_names;
+	char *end_param, *hide_param;
 	int port_id, num_xstats;
+	int hide_zero = 0;
 	int i, ret;
-	char *end_param;
 
 	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
 		return -1;
 
 	port_id = strtoul(params, &end_param, 0);
-	if (*end_param != '\0')
-		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
+	if (*end_param != '\0') {
+		hide_param = strtok(end_param, ",");
+		if (!hide_param || strlen(hide_param) == 0 || !isdigit(*hide_param))
+			return -EINVAL;
+		hide_zero = strtoul(hide_param, &end_param, 0);
+		if (*end_param != '\0')
+			RTE_ETHDEV_LOG(NOTICE,
+				"Extra parameters passed to ethdev telemetry command, ignoring\n");
+		if (hide_zero != 0 && hide_zero != 1) {
+			hide_zero = !!hide_zero;
+			RTE_ETHDEV_LOG(NOTICE,
+				"Hide zero parameter is non-boolean, cast to boolean\n");
+		}
+	}
+
 	num_xstats = rte_eth_xstats_get(port_id, NULL, 0);
 	if (num_xstats < 0)
 		return -1;
@@ -5908,9 +5921,12 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	}
 
 	rte_tel_data_start_dict(d);
-	for (i = 0; i < num_xstats; i++)
+	for (i = 0; i < num_xstats; i++) {
+		if (hide_zero && eth_xstats[i].value == 0)
+			continue;
 		rte_tel_data_add_dict_u64(d, xstat_names[i].name,
 				eth_xstats[i].value);
+	}
 	free(eth_xstats);
 	return 0;
 }
@@ -6354,7 +6370,7 @@ RTE_INIT(ethdev_init_telemetry)
 	rte_telemetry_register_cmd("/ethdev/stats", eth_dev_handle_port_stats,
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
-			"Returns the extended stats for a port. Parameters: int port_id");
+			"Returns the extended stats for a port. Parameters: int port_id, hide_zero (Optional, specify whether to hide zero values)");
 	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
 			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 0/5] support dmadev/ethdev stats reset
  2023-01-11 12:06 ` [PATCH v2 0/5] support dmadev/ethdev stats reset Chengwen Feng
                     ` (4 preceding siblings ...)
  2023-01-11 12:06   ` [PATCH v2 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
@ 2023-01-11 12:36   ` fengchengwen
  5 siblings, 0 replies; 62+ messages in thread
From: fengchengwen @ 2023-01-11 12:36 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

Sorry to repeat send v2, this new version include change log.

On 2023/1/11 20:06, Chengwen Feng wrote:
> This patchset contains dmadev/ethdev stats reset, and also support
> hide zero for ethdev xstats and two telemetry related bugs.
> 
> Chengwen Feng (5):
>   dmadev: support stats reset telemetry command
>   telemetry: fix repeat display when callback don't set dict
>   ethdev: add newline to telemetry log string
>   ethdev: support xstats reset telemetry command
>   ethdev: telemetry xstats support hide zero
> 
> ---
> v2: 
> * address Bruce's comment on 2/5 patch.
> * support output success when stats reset.
> 
>  lib/dmadev/rte_dmadev.c   | 46 ++++++++++++++++++++++++++++
>  lib/ethdev/rte_ethdev.c   | 63 +++++++++++++++++++++++++++++++++------
>  lib/telemetry/telemetry.c |  2 +-
>  3 files changed, 101 insertions(+), 10 deletions(-)
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict
  2023-01-06 17:33         ` Bruce Richardson
@ 2023-01-11 12:38           ` fengchengwen
  0 siblings, 0 replies; 62+ messages in thread
From: fengchengwen @ 2023-01-11 12:38 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

Hi Bruce,

On 2023/1/7 1:33, Bruce Richardson wrote:
> On Fri, Jan 06, 2023 at 04:07:45PM +0000, Bruce Richardson wrote:
>> On Mon, Dec 26, 2022 at 12:53:57PM +0800, fengchengwen wrote:
>>> On 2022/12/19 17:33, Bruce Richardson wrote:
>>>> On Mon, Dec 19, 2022 at 09:07:20AM +0000, Chengwen Feng wrote:
>>>>> When telemetry callback didn't set dict and return a non-negative
>>>>> number, the telemetry will repeat to display the last result.
>>>>>
>>>>> Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>> ---
>>>>
>>>> Hi Chengwen,
>>>>
>>>> I'm a little curious about this bug. Can you describe some steps to
>>>> reproduce it as I'm curious as to exactly what is happening. The fix seems
>>>> a little strange to me so I'd like to investigate a little more to see if
>>>> other approaches might work.
>>>
>>> Hi Bruce,
>>>
>>> Sorry for late reply.
>>>
>>> The steps:
>>>   1. applay "[PATCH v5 1/5] dmadev: support stats reset telemetry command"
>>>   2. compile
>>>   3. start dpdk-dma: dpdk-dma -a DMA.BDF -a NIC.BDF -- -c hw
>>>   4. start telemetry, and execute /dmadev/stats,0, and then /dmadev/stats_reset,0
>>>      the output of /dmadev/stats_reset,0 will be the same of previous cmd "/dmadev/stats,0"
>>>      e.g. my environment:
>>> --> /dmadev/stats,0
>>> {
>>>   "/dmadev/stats": {
>>>     "submitted": 23,
>>>     "completed": 23,
>>>     "errors": 0
>>>   }
>>> }
>>> --> /dmadev/stats_reset,0
>>> {
>>>   "/dmadev/stats_reset": {
>>>     "submitted": 23,
>>>     "completed": 23,
>>>     "errors": 0
>>>   }
>>> }
>>>
>>> The rootcause is that the /dmadev/stats_reset don't set the outer parameter "struct rte_tel_data *info"
>>> and return zero.
>>>
>> Thanks for the fuller explanation, I'll hopefully test it out myself.
>>
>> However, in the meantime, looking at the telemetry library code, would the
>> following change work rather than explicitly always setting the telemetry
>> data to a dictionary by default? Zeroing the data by default sets it to a
>> null return which is what you probably want as default rather than an empty
>> dictionary. (And it's also a smaller diff)
>>
>> /Bruce
>>
>> diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
>> index 8fbb4f3060..7b905355cd 100644
>> --- a/lib/telemetry/telemetry.c
>> +++ b/lib/telemetry/telemetry.c
>> @@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
>>  static void
>>  perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
>>  {
>> -       struct rte_tel_data data;
>> +       struct rte_tel_data data = {0};
>>  
>>         int ret = fn(cmd, param, &data);
>>         if (ret < 0) {
>>
> 
> I've handily reproduced the issue using the instructions you gave above,
> thanks for those.
> 
> Based on that, and looking a little deeper:
> 
> * I think it is an error with the reset function not to initialize and
>   complete the return value. I believe that for each telemetry callback we
>   should require that the callback fill in a valid value on success. For
>   reset, some options could be just a string "OK", or an array just
>   containing "[0]", or similar.

good idea, the v2 follow it.

> 
> * Given that point above, I do agree though that the "data" parameter
>   should be properly initialized on entry to the callbacks. However, I feel
>   that the correct init should be the empty/null value, as is given in the
>   patch above.

already fix in v2

Thanks.

> 
> Regards,
> /Bruce
> .
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 1/5] dmadev: support stats reset telemetry command
  2023-01-11 12:06   ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
@ 2023-01-11 13:58     ` Bruce Richardson
  0 siblings, 0 replies; 62+ messages in thread
From: Bruce Richardson @ 2023-01-11 13:58 UTC (permalink / raw)
  To: Chengwen Feng
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On Wed, Jan 11, 2023 at 12:06:26PM +0000, Chengwen Feng wrote:
> The stats reset is useful for debugging, so add it to the dmadev
> telemetry command lists.
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>

Looks ok to me, just one small comment inline. With that fixed:

Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

> ---
>  lib/dmadev/rte_dmadev.c | 46 +++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 46 insertions(+)
> 
> diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
> index 4da653eec7..79593cdbe2 100644
> --- a/lib/dmadev/rte_dmadev.c
> +++ b/lib/dmadev/rte_dmadev.c
> @@ -994,6 +994,50 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
>  	return 0;
>  }
>  
> +static int
> +dmadev_handle_dev_stats_reset(const char *cmd __rte_unused,
> +		const char *params,
> +		struct rte_tel_data *d)
> +{
> +	struct rte_dma_info dma_info;
> +	int dev_id, ret, vchan_id;
> +	const char *vchan_param;
> +	char *end_param;
> +
> +	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> +		return -EINVAL;
> +
> +	dev_id = strtoul(params, &end_param, 0);
> +
> +	/* Function info_get validates dev_id so we don't need to. */
> +	ret = rte_dma_info_get(dev_id, &dma_info);
> +	if (ret < 0)
> +		return -EINVAL;
> +
> +	/* If the device has one vchan the user does not need to supply the
> +	 * vchan id and only the device id is needed, no extra parameters.
> +	 */
> +	if (dma_info.nb_vchans == 1 && *end_param == '\0')
> +		vchan_id = 0;
> +	else {
> +		vchan_param = strtok(end_param, ",");
> +		if (!vchan_param || strlen(vchan_param) == 0 || !isdigit(*vchan_param))
> +			return -EINVAL;
> +
> +		vchan_id = strtoul(vchan_param, &end_param, 0);
> +	}
> +	if (*end_param != '\0')
> +		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
> +
> +	ret = rte_dma_stats_reset(dev_id, vchan_id);
> +	if (ret == 0) {
> +		rte_tel_data_start_dict(d);
This line is unnecessary. "rte_tel_data_string" below sets the overall
datatype as "string", replacing the "dict" type set here. To set the string
inside the dict you have just created use "rte_tel_data_add_dict_string".
[Note: the data returned from a command is always put in a json dict
automatically - that is separate from any dict initialized here.]

> +		rte_tel_data_string(d, "success");
> +	}
> +
> +	return ret;
> +}
> +
>  #ifndef RTE_EXEC_ENV_WINDOWS
>  static int
>  dmadev_handle_dev_dump(const char *cmd __rte_unused,
> @@ -1041,6 +1085,8 @@ RTE_INIT(dmadev_init_telemetry)
>  			"Returns information for a dmadev. Parameters: int dev_id");
>  	rte_telemetry_register_cmd("/dmadev/stats", dmadev_handle_dev_stats,
>  			"Returns the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
> +	rte_telemetry_register_cmd("/dmadev/stats_reset", dmadev_handle_dev_stats_reset,
> +			"Reset the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
>  #ifndef RTE_EXEC_ENV_WINDOWS
>  	rte_telemetry_register_cmd("/dmadev/dump", dmadev_handle_dev_dump,
>  			"Returns dump information for a dmadev. Parameters: int dev_id");
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict
  2023-01-11 12:06   ` [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict Chengwen Feng
@ 2023-01-11 14:01     ` Bruce Richardson
  0 siblings, 0 replies; 62+ messages in thread
From: Bruce Richardson @ 2023-01-11 14:01 UTC (permalink / raw)
  To: Chengwen Feng
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On Wed, Jan 11, 2023 at 12:06:27PM +0000, Chengwen Feng wrote:
> When telemetry callback didn't set dict and return a non-negative

I'd suggest rewording the "didn't set dict", since it's not required that a
telemetry callback do anything with dictionaries. I'd suggest:

"When a telemetry callback doesn't initialize the telemetry data
structure and returns a non-negative number..."

> number, the telemetry will repeat to display the last result. This
> patch zero the dict to avoid the problem.

s/zero the dict/zero the data-structure/

> 
> Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---

Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>

>  lib/telemetry/telemetry.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
> index 8fbb4f3060..7b905355cd 100644
> --- a/lib/telemetry/telemetry.c
> +++ b/lib/telemetry/telemetry.c
> @@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
>  static void
>  perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
>  {
> -	struct rte_tel_data data;
> +	struct rte_tel_data data = {0};
>  
>  	int ret = fn(cmd, param, &data);
>  	if (ret < 0) {
> -- 
> 2.17.1
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 5/5] ethdev: telemetry xstats support hide zero
  2023-01-11 12:06   ` [PATCH v2 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
@ 2023-01-11 14:08     ` Bruce Richardson
  2023-01-20  3:51       ` fengchengwen
  0 siblings, 1 reply; 62+ messages in thread
From: Bruce Richardson @ 2023-01-11 14:08 UTC (permalink / raw)
  To: Chengwen Feng
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On Wed, Jan 11, 2023 at 12:06:30PM +0000, Chengwen Feng wrote:
> The number of xstats may be large, after the hide zero option is added,
> only non-zero values can be displayed.
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> ---
>  lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++------
>  1 file changed, 22 insertions(+), 6 deletions(-)
> 
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index 2fc593b865..77cacc0829 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -5870,20 +5870,33 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
>  {
>  	struct rte_eth_xstat *eth_xstats;
>  	struct rte_eth_xstat_name *xstat_names;
> +	char *end_param, *hide_param;
>  	int port_id, num_xstats;
> +	int hide_zero = 0;
>  	int i, ret;
> -	char *end_param;
>  
>  	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
>  		return -1;
>  
>  	port_id = strtoul(params, &end_param, 0);
> -	if (*end_param != '\0')
> -		RTE_ETHDEV_LOG(NOTICE,
> -			"Extra parameters passed to ethdev telemetry command, ignoring\n");
>  	if (!rte_eth_dev_is_valid_port(port_id))
>  		return -1;
>  
> +	if (*end_param != '\0') {
> +		hide_param = strtok(end_param, ",");
> +		if (!hide_param || strlen(hide_param) == 0 || !isdigit(*hide_param))
> +			return -EINVAL;
> +		hide_zero = strtoul(hide_param, &end_param, 0);
> +		if (*end_param != '\0')
> +			RTE_ETHDEV_LOG(NOTICE,
> +				"Extra parameters passed to ethdev telemetry command, ignoring\n");
> +		if (hide_zero != 0 && hide_zero != 1) {
> +			hide_zero = !!hide_zero;
> +			RTE_ETHDEV_LOG(NOTICE,
> +				"Hide zero parameter is non-boolean, cast to boolean\n");
> +		}
> +	}
> +

I'm not sure about this adding of an extra flag as a 0/1 value. That would
seem to make it confusing with a queue number or extra port number. For
such an optional parameter, I think a string value would be clearer. A
number of alternatives I'd suggest - in order of my preference (least
preferred to most preferred):

* have the extra parameter as a literal string "hide_zeros" which is
  matched against rather than looking for 0/1, or alternatively

* add a new command /ethdev/xstats_nonzero which does this, the same
  callback can be reusued, just checking the command given, or finally,

* if this is primarily for the benefit of users using the telemetry script
  to interactively view stats, then the functionality could be implemented
  in the script itself rather than in the backend. We could add some
  setting, or extra flag to the display code, to possibly filter out zeros
  in the display.

Thoughts?

/Bruce

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v3 0/5] support dmadev/ethdev stats reset
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
                   ` (7 preceding siblings ...)
  2023-01-11 12:06 ` [PATCH v2 0/5] support dmadev/ethdev stats reset Chengwen Feng
@ 2023-01-20  3:25 ` Chengwen Feng
  2023-01-20  3:25   ` [PATCH v3 1/5] dmadev: support stats reset telemetry command Chengwen Feng
                     ` (4 more replies)
  2023-01-20  3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
  2023-02-09  2:32 ` [PATCH v5 0/2] " Chengwen Feng
  10 siblings, 5 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:25 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

This patchset contains dmadev/ethdev stats reset, and also support
hide zero for ethdev xstats and two telemetry related bugs.

Chengwen Feng (5):
  dmadev: support stats reset telemetry command
  telemetry: fix repeat display when callback don't init dict
  ethdev: add newline to telemetry log string
  ethdev: support xstats reset telemetry command
  ethdev: telemetry xstats support hide zero

---
v3:
* address Bruce's comment on 1/5 and 2/5 patch, and also for 4/5 patch.
* reword the help info of 5/5 patch.
v2:
* address Bruce's comment on 2/5 patch.
* support output success when stats reset.

 lib/dmadev/rte_dmadev.c   | 43 ++++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c   | 56 ++++++++++++++++++++++++++++++++-------
 lib/telemetry/telemetry.c |  2 +-
 3 files changed, 91 insertions(+), 10 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v3 1/5] dmadev: support stats reset telemetry command
  2023-01-20  3:25 ` [PATCH v3 " Chengwen Feng
@ 2023-01-20  3:25   ` Chengwen Feng
  2023-01-20  3:25   ` [PATCH v3 2/5] telemetry: fix repeat display when callback don't init dict Chengwen Feng
                     ` (3 subsequent siblings)
  4 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:25 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The stats reset is useful for debugging, so add it to the dmadev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/dmadev/rte_dmadev.c | 43 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
index 4da653eec7..9960b917b7 100644
--- a/lib/dmadev/rte_dmadev.c
+++ b/lib/dmadev/rte_dmadev.c
@@ -994,6 +994,47 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+dmadev_handle_dev_stats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d)
+{
+	struct rte_dma_info dma_info;
+	int dev_id, ret, vchan_id;
+	const char *vchan_param;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -EINVAL;
+
+	dev_id = strtoul(params, &end_param, 0);
+
+	/* Function info_get validates dev_id so we don't need to. */
+	ret = rte_dma_info_get(dev_id, &dma_info);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* If the device has one vchan the user does not need to supply the
+	 * vchan id and only the device id is needed, no extra parameters.
+	 */
+	if (dma_info.nb_vchans == 1 && *end_param == '\0') {
+		vchan_id = 0;
+	} else {
+		vchan_param = strtok(end_param, ",");
+		if (vchan_param == NULL || strlen(vchan_param) == 0 || !isdigit(*vchan_param))
+			return -EINVAL;
+		vchan_id = strtoul(vchan_param, &end_param, 0);
+	}
+	if (*end_param != '\0')
+		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
+
+	ret = rte_dma_stats_reset(dev_id, vchan_id);
+	if (ret == 0)
+		rte_tel_data_string(d, "success");
+
+	return ret;
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 dmadev_handle_dev_dump(const char *cmd __rte_unused,
@@ -1041,6 +1082,8 @@ RTE_INIT(dmadev_init_telemetry)
 			"Returns information for a dmadev. Parameters: int dev_id");
 	rte_telemetry_register_cmd("/dmadev/stats", dmadev_handle_dev_stats,
 			"Returns the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
+	rte_telemetry_register_cmd("/dmadev/stats_reset", dmadev_handle_dev_stats_reset,
+			"Reset the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/dmadev/dump", dmadev_handle_dev_dump,
 			"Returns dump information for a dmadev. Parameters: int dev_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v3 2/5] telemetry: fix repeat display when callback don't init dict
  2023-01-20  3:25 ` [PATCH v3 " Chengwen Feng
  2023-01-20  3:25   ` [PATCH v3 1/5] dmadev: support stats reset telemetry command Chengwen Feng
@ 2023-01-20  3:25   ` Chengwen Feng
  2023-01-20  3:25   ` [PATCH v3 3/5] ethdev: add newline to telemetry log string Chengwen Feng
                     ` (2 subsequent siblings)
  4 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:25 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

When a telemetry callback doesn't initialize the telemetry data
structure and returns a non-negative number, the telemetry will repeat
to display the last result. This patch zero the data structure to avoid
the problem.

Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/telemetry/telemetry.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index 8fbb4f3060..7b905355cd 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
 static void
 perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
 {
-	struct rte_tel_data data;
+	struct rte_tel_data data = {0};
 
 	int ret = fn(cmd, param, &data);
 	if (ret < 0) {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v3 3/5] ethdev: add newline to telemetry log string
  2023-01-20  3:25 ` [PATCH v3 " Chengwen Feng
  2023-01-20  3:25   ` [PATCH v3 1/5] dmadev: support stats reset telemetry command Chengwen Feng
  2023-01-20  3:25   ` [PATCH v3 2/5] telemetry: fix repeat display when callback don't init dict Chengwen Feng
@ 2023-01-20  3:25   ` Chengwen Feng
  2023-01-20  3:25   ` [PATCH v3 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
  2023-01-20  3:25   ` [PATCH v3 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
  4 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:25 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The telemetry related code may invoke RTE_ETHDEV_LOG to display
information, the newline character is not added automatically to the
RTE_ETHDEV_LOG, therefore, the newline character must be explicitly
added to the log string.

Fixes: 5514319e7b43 ("telemetry: fix passing full params string to command")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 5d5e18db1e..9eeae61024 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5880,7 +5880,7 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
@@ -5931,7 +5931,7 @@ eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -EINVAL;
 
@@ -5973,7 +5973,7 @@ eth_dev_handle_port_link_status(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
@@ -6011,7 +6011,7 @@ eth_dev_handle_port_info(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -EINVAL;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v3 4/5] ethdev: support xstats reset telemetry command
  2023-01-20  3:25 ` [PATCH v3 " Chengwen Feng
                     ` (2 preceding siblings ...)
  2023-01-20  3:25   ` [PATCH v3 3/5] ethdev: add newline to telemetry log string Chengwen Feng
@ 2023-01-20  3:25   ` Chengwen Feng
  2023-01-20  3:25   ` [PATCH v3 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
  4 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:25 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The xstats reset is useful for debugging, so add it to the ethdev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9eeae61024..ebcb05b523 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5915,6 +5915,31 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d)
+{
+	int port_id, ret;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -1;
+
+	port_id = strtoul(params, &end_param, 0);
+	if (*end_param != '\0')
+		RTE_ETHDEV_LOG(NOTICE,
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
+	if (!rte_eth_dev_is_valid_port(port_id))
+		return -1;
+
+	ret = rte_eth_xstats_reset(port_id);
+	if (ret == 0)
+		rte_tel_data_string(d, "success");
+
+	return ret;
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
@@ -6328,6 +6353,8 @@ RTE_INIT(ethdev_init_telemetry)
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
 			"Returns the extended stats for a port. Parameters: int port_id");
+	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
+			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
 			"Returns dump private information for a port. Parameters: int port_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v3 5/5] ethdev: telemetry xstats support hide zero
  2023-01-20  3:25 ` [PATCH v3 " Chengwen Feng
                     ` (3 preceding siblings ...)
  2023-01-20  3:25   ` [PATCH v3 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
@ 2023-01-20  3:25   ` Chengwen Feng
  4 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:25 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The number of xstats may be large, after the hide zero option is added,
only non-zero values can be displayed.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index ebcb05b523..01bf78490d 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5870,20 +5870,28 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 {
 	struct rte_eth_xstat *eth_xstats;
 	struct rte_eth_xstat_name *xstat_names;
+	char *end_param, *hide_param;
 	int port_id, num_xstats;
+	int hide_zero = 0;
 	int i, ret;
-	char *end_param;
 
 	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
 		return -1;
 
 	port_id = strtoul(params, &end_param, 0);
-	if (*end_param != '\0')
-		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
+	if (*end_param != '\0') {
+		hide_param = strtok(end_param, ",");
+		if (hide_param == NULL || strlen(hide_param) == 0 || !isdigit(*hide_param))
+			return -EINVAL;
+		hide_zero = strtoul(hide_param, &end_param, 0);
+		if (*end_param != '\0')
+			RTE_ETHDEV_LOG(NOTICE,
+				"Extra parameters passed to ethdev telemetry command, ignoring\n");
+	}
+
 	num_xstats = rte_eth_xstats_get(port_id, NULL, 0);
 	if (num_xstats < 0)
 		return -1;
@@ -5908,9 +5916,12 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	}
 
 	rte_tel_data_start_dict(d);
-	for (i = 0; i < num_xstats; i++)
+	for (i = 0; i < num_xstats; i++) {
+		if (hide_zero != 0 && eth_xstats[i].value == 0)
+			continue;
 		rte_tel_data_add_dict_u64(d, xstat_names[i].name,
 				eth_xstats[i].value);
+	}
 	free(eth_xstats);
 	return 0;
 }
@@ -6352,7 +6363,7 @@ RTE_INIT(ethdev_init_telemetry)
 	rte_telemetry_register_cmd("/ethdev/stats", eth_dev_handle_port_stats,
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
-			"Returns the extended stats for a port. Parameters: int port_id");
+			"Returns the extended stats for a port. Parameters: int port_id, bool hide_zero (Optional, non-zero indicates hide zero xstats-value)");
 	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
 			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v4 0/5] support dmadev/ethdev stats reset
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
                   ` (8 preceding siblings ...)
  2023-01-20  3:25 ` [PATCH v3 " Chengwen Feng
@ 2023-01-20  3:34 ` Chengwen Feng
  2023-01-20  3:34   ` [PATCH v4 1/5] dmadev: support stats reset telemetry command Chengwen Feng
                     ` (6 more replies)
  2023-02-09  2:32 ` [PATCH v5 0/2] " Chengwen Feng
  10 siblings, 7 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:34 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

This patchset contains dmadev/ethdev stats reset, and also support
hide zero for ethdev xstats and two telemetry related bugs.

Chengwen Feng (5):
  dmadev: support stats reset telemetry command
  telemetry: fix repeat display when callback don't init dict
  ethdev: add newline to telemetry log string
  ethdev: support xstats reset telemetry command
  ethdev: telemetry xstats support hide zero

---
v4:
* solve the internal don't commit for 5/5 patch.
v3:
* address Bruce's comment on 1/5 and 2/5 patch, and also for 4/5 patch.
* reword the help info of 5/5 patch.
v2:
* address Bruce's comment on 2/5 patch.
* support output success when stats reset.

 lib/dmadev/rte_dmadev.c   | 43 ++++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c   | 56 ++++++++++++++++++++++++++++++++-------
 lib/telemetry/telemetry.c |  2 +-
 3 files changed, 91 insertions(+), 10 deletions(-)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v4 1/5] dmadev: support stats reset telemetry command
  2023-01-20  3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
@ 2023-01-20  3:34   ` Chengwen Feng
  2023-01-20  3:34   ` [PATCH v4 2/5] telemetry: fix repeat display when callback don't init dict Chengwen Feng
                     ` (5 subsequent siblings)
  6 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:34 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The stats reset is useful for debugging, so add it to the dmadev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/dmadev/rte_dmadev.c | 43 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 43 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
index 4da653eec7..9960b917b7 100644
--- a/lib/dmadev/rte_dmadev.c
+++ b/lib/dmadev/rte_dmadev.c
@@ -994,6 +994,47 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+dmadev_handle_dev_stats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d)
+{
+	struct rte_dma_info dma_info;
+	int dev_id, ret, vchan_id;
+	const char *vchan_param;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -EINVAL;
+
+	dev_id = strtoul(params, &end_param, 0);
+
+	/* Function info_get validates dev_id so we don't need to. */
+	ret = rte_dma_info_get(dev_id, &dma_info);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* If the device has one vchan the user does not need to supply the
+	 * vchan id and only the device id is needed, no extra parameters.
+	 */
+	if (dma_info.nb_vchans == 1 && *end_param == '\0') {
+		vchan_id = 0;
+	} else {
+		vchan_param = strtok(end_param, ",");
+		if (vchan_param == NULL || strlen(vchan_param) == 0 || !isdigit(*vchan_param))
+			return -EINVAL;
+		vchan_id = strtoul(vchan_param, &end_param, 0);
+	}
+	if (*end_param != '\0')
+		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
+
+	ret = rte_dma_stats_reset(dev_id, vchan_id);
+	if (ret == 0)
+		rte_tel_data_string(d, "success");
+
+	return ret;
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 dmadev_handle_dev_dump(const char *cmd __rte_unused,
@@ -1041,6 +1082,8 @@ RTE_INIT(dmadev_init_telemetry)
 			"Returns information for a dmadev. Parameters: int dev_id");
 	rte_telemetry_register_cmd("/dmadev/stats", dmadev_handle_dev_stats,
 			"Returns the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
+	rte_telemetry_register_cmd("/dmadev/stats_reset", dmadev_handle_dev_stats_reset,
+			"Reset the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/dmadev/dump", dmadev_handle_dev_dump,
 			"Returns dump information for a dmadev. Parameters: int dev_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v4 2/5] telemetry: fix repeat display when callback don't init dict
  2023-01-20  3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
  2023-01-20  3:34   ` [PATCH v4 1/5] dmadev: support stats reset telemetry command Chengwen Feng
@ 2023-01-20  3:34   ` Chengwen Feng
  2023-02-08 14:15     ` Bruce Richardson
  2023-01-20  3:34   ` [PATCH v4 3/5] ethdev: add newline to telemetry log string Chengwen Feng
                     ` (4 subsequent siblings)
  6 siblings, 1 reply; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:34 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

When a telemetry callback doesn't initialize the telemetry data
structure and returns a non-negative number, the telemetry will repeat
to display the last result. This patch zero the data structure to avoid
the problem.

Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/telemetry/telemetry.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
index 8fbb4f3060..7b905355cd 100644
--- a/lib/telemetry/telemetry.c
+++ b/lib/telemetry/telemetry.c
@@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
 static void
 perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
 {
-	struct rte_tel_data data;
+	struct rte_tel_data data = {0};
 
 	int ret = fn(cmd, param, &data);
 	if (ret < 0) {
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v4 3/5] ethdev: add newline to telemetry log string
  2023-01-20  3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
  2023-01-20  3:34   ` [PATCH v4 1/5] dmadev: support stats reset telemetry command Chengwen Feng
  2023-01-20  3:34   ` [PATCH v4 2/5] telemetry: fix repeat display when callback don't init dict Chengwen Feng
@ 2023-01-20  3:34   ` Chengwen Feng
  2023-01-20  3:34   ` [PATCH v4 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
                     ` (3 subsequent siblings)
  6 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:34 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The telemetry related code may invoke RTE_ETHDEV_LOG to display
information, the newline character is not added automatically to the
RTE_ETHDEV_LOG, therefore, the newline character must be explicitly
added to the log string.

Fixes: 5514319e7b43 ("telemetry: fix passing full params string to command")
Cc: stable@dpdk.org

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 5d5e18db1e..9eeae61024 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5880,7 +5880,7 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
@@ -5931,7 +5931,7 @@ eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -EINVAL;
 
@@ -5973,7 +5973,7 @@ eth_dev_handle_port_link_status(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
@@ -6011,7 +6011,7 @@ eth_dev_handle_port_info(const char *cmd __rte_unused,
 	port_id = strtoul(params, &end_param, 0);
 	if (*end_param != '\0')
 		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring");
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -EINVAL;
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v4 4/5] ethdev: support xstats reset telemetry command
  2023-01-20  3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
                     ` (2 preceding siblings ...)
  2023-01-20  3:34   ` [PATCH v4 3/5] ethdev: add newline to telemetry log string Chengwen Feng
@ 2023-01-20  3:34   ` Chengwen Feng
  2023-01-20  3:34   ` [PATCH v4 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
                     ` (2 subsequent siblings)
  6 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:34 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The xstats reset is useful for debugging, so add it to the ethdev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index 9eeae61024..ebcb05b523 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5915,6 +5915,31 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d)
+{
+	int port_id, ret;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -1;
+
+	port_id = strtoul(params, &end_param, 0);
+	if (*end_param != '\0')
+		RTE_ETHDEV_LOG(NOTICE,
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
+	if (!rte_eth_dev_is_valid_port(port_id))
+		return -1;
+
+	ret = rte_eth_xstats_reset(port_id);
+	if (ret == 0)
+		rte_tel_data_string(d, "success");
+
+	return ret;
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
@@ -6328,6 +6353,8 @@ RTE_INIT(ethdev_init_telemetry)
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
 			"Returns the extended stats for a port. Parameters: int port_id");
+	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
+			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
 			"Returns dump private information for a port. Parameters: int port_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v4 5/5] ethdev: telemetry xstats support hide zero
  2023-01-20  3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
                     ` (3 preceding siblings ...)
  2023-01-20  3:34   ` [PATCH v4 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
@ 2023-01-20  3:34   ` Chengwen Feng
  2023-02-06  8:39   ` [PATCH v4 0/5] support dmadev/ethdev stats reset Thomas Monjalon
  2023-02-08 14:17   ` Bruce Richardson
  6 siblings, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-01-20  3:34 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The number of xstats may be large, after the hide zero option is added,
only non-zero values can be displayed.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 23 +++++++++++++++++------
 1 file changed, 17 insertions(+), 6 deletions(-)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index ebcb05b523..a8ba3f2280 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5870,20 +5870,28 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 {
 	struct rte_eth_xstat *eth_xstats;
 	struct rte_eth_xstat_name *xstat_names;
+	char *end_param, *hide_param;
 	int port_id, num_xstats;
+	int hide_zero = 0;
 	int i, ret;
-	char *end_param;
 
 	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
 		return -1;
 
 	port_id = strtoul(params, &end_param, 0);
-	if (*end_param != '\0')
-		RTE_ETHDEV_LOG(NOTICE,
-			"Extra parameters passed to ethdev telemetry command, ignoring\n");
 	if (!rte_eth_dev_is_valid_port(port_id))
 		return -1;
 
+	if (*end_param != '\0') {
+		hide_param = strtok(end_param, ",");
+		if (hide_param == NULL || strlen(hide_param) == 0 || !isdigit(*hide_param))
+			return -EINVAL;
+		hide_zero = strtoul(hide_param, &end_param, 0);
+		if (*end_param != '\0')
+			RTE_ETHDEV_LOG(NOTICE,
+				"Extra parameters passed to ethdev telemetry command, ignoring\n");
+	}
+
 	num_xstats = rte_eth_xstats_get(port_id, NULL, 0);
 	if (num_xstats < 0)
 		return -1;
@@ -5908,9 +5916,12 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	}
 
 	rte_tel_data_start_dict(d);
-	for (i = 0; i < num_xstats; i++)
+	for (i = 0; i < num_xstats; i++) {
+		if (hide_zero != 0 && eth_xstats[i].value == 0)
+			continue;
 		rte_tel_data_add_dict_u64(d, xstat_names[i].name,
 				eth_xstats[i].value);
+	}
 	free(eth_xstats);
 	return 0;
 }
@@ -6352,7 +6363,7 @@ RTE_INIT(ethdev_init_telemetry)
 	rte_telemetry_register_cmd("/ethdev/stats", eth_dev_handle_port_stats,
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
-			"Returns the extended stats for a port. Parameters: int port_id");
+			"Returns the extended stats for a port. Parameters: int port_id, bool hide_zero (Optional, non-zero indicates hide zero xstats)");
 	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
 			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v2 5/5] ethdev: telemetry xstats support hide zero
  2023-01-11 14:08     ` Bruce Richardson
@ 2023-01-20  3:51       ` fengchengwen
  0 siblings, 0 replies; 62+ messages in thread
From: fengchengwen @ 2023-01-20  3:51 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

Hi Bruce,

On 2023/1/11 22:08, Bruce Richardson wrote:
> On Wed, Jan 11, 2023 at 12:06:30PM +0000, Chengwen Feng wrote:
>> The number of xstats may be large, after the hide zero option is added,
>> only non-zero values can be displayed.
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> ---
>>  lib/ethdev/rte_ethdev.c | 28 ++++++++++++++++++++++------
>>  1 file changed, 22 insertions(+), 6 deletions(-)
>>
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index 2fc593b865..77cacc0829 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -5870,20 +5870,33 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
>>  {
>>  	struct rte_eth_xstat *eth_xstats;
>>  	struct rte_eth_xstat_name *xstat_names;
>> +	char *end_param, *hide_param;
>>  	int port_id, num_xstats;
>> +	int hide_zero = 0;
>>  	int i, ret;
>> -	char *end_param;
>>  
>>  	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
>>  		return -1;
>>  
>>  	port_id = strtoul(params, &end_param, 0);
>> -	if (*end_param != '\0')
>> -		RTE_ETHDEV_LOG(NOTICE,
>> -			"Extra parameters passed to ethdev telemetry command, ignoring\n");
>>  	if (!rte_eth_dev_is_valid_port(port_id))
>>  		return -1;
>>  
>> +	if (*end_param != '\0') {
>> +		hide_param = strtok(end_param, ",");
>> +		if (!hide_param || strlen(hide_param) == 0 || !isdigit(*hide_param))
>> +			return -EINVAL;
>> +		hide_zero = strtoul(hide_param, &end_param, 0);
>> +		if (*end_param != '\0')
>> +			RTE_ETHDEV_LOG(NOTICE,
>> +				"Extra parameters passed to ethdev telemetry command, ignoring\n");
>> +		if (hide_zero != 0 && hide_zero != 1) {
>> +			hide_zero = !!hide_zero;
>> +			RTE_ETHDEV_LOG(NOTICE,
>> +				"Hide zero parameter is non-boolean, cast to boolean\n");
>> +		}
>> +	}
>> +
> 
> I'm not sure about this adding of an extra flag as a 0/1 value. That would
> seem to make it confusing with a queue number or extra port number. For
> such an optional parameter, I think a string value would be clearer. A
> number of alternatives I'd suggest - in order of my preference (least
> preferred to most preferred):
> 
> * have the extra parameter as a literal string "hide_zeros" which is
>   matched against rather than looking for 0/1, or alternatively

add string "hide_zeros" requires more characters, it may not user friendly.

> 
> * add a new command /ethdev/xstats_nonzero which does this, the same
>   callback can be reusued, just checking the command given, or finally,

The new command may cause confusion.

> 
> * if this is primarily for the benefit of users using the telemetry script
>   to interactively view stats, then the functionality could be implemented
>   in the script itself rather than in the backend. We could add some
>   setting, or extra flag to the display code, to possibly filter out zeros
>   in the display.

This may causes the display logic to be coupled with the command name.

> 
> Thoughts?

I prefer add optional parameter, and the optional parameter should be simpler, just a number here.

And for clearer expression, I reword the help string in V4:
Returns the extended stats for a port. Parameters: int port_id, bool hide_zero (Optional, non-zero indicates hide zero xstats)

Thanks.

> 
> /Bruce
> .
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v4 0/5] support dmadev/ethdev stats reset
  2023-01-20  3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
                     ` (4 preceding siblings ...)
  2023-01-20  3:34   ` [PATCH v4 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
@ 2023-02-06  8:39   ` Thomas Monjalon
  2023-02-08 14:17   ` Bruce Richardson
  6 siblings, 0 replies; 62+ messages in thread
From: Thomas Monjalon @ 2023-02-06  8:39 UTC (permalink / raw)
  To: dev
  Cc: ferruh.yigit, andrew.rybchenko, ciara.power, kevin.laatz,
	bruce.richardson, Chengwen Feng

20/01/2023 04:34, Chengwen Feng:
> This patchset contains dmadev/ethdev stats reset, and also support
> hide zero for ethdev xstats and two telemetry related bugs.
> 
> Chengwen Feng (5):
>   dmadev: support stats reset telemetry command
>   telemetry: fix repeat display when callback don't init dict
>   ethdev: add newline to telemetry log string
>   ethdev: support xstats reset telemetry command
>   ethdev: telemetry xstats support hide zero

Ping for review.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v4 2/5] telemetry: fix repeat display when callback don't init dict
  2023-01-20  3:34   ` [PATCH v4 2/5] telemetry: fix repeat display when callback don't init dict Chengwen Feng
@ 2023-02-08 14:15     ` Bruce Richardson
  2023-02-09  1:33       ` fengchengwen
  0 siblings, 1 reply; 62+ messages in thread
From: Bruce Richardson @ 2023-02-08 14:15 UTC (permalink / raw)
  To: Chengwen Feng
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On Fri, Jan 20, 2023 at 03:34:53AM +0000, Chengwen Feng wrote:
> When a telemetry callback doesn't initialize the telemetry data
> structure and returns a non-negative number, the telemetry will repeat
> to display the last result. This patch zero the data structure to avoid
> the problem.
> 
> Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
>  lib/telemetry/telemetry.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
> index 8fbb4f3060..7b905355cd 100644
> --- a/lib/telemetry/telemetry.c
> +++ b/lib/telemetry/telemetry.c
> @@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
>  static void
>  perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
>  {
> -	struct rte_tel_data data;
> +	struct rte_tel_data data = {0};
>  
>  	int ret = fn(cmd, param, &data);
>  	if (ret < 0) {
> -- 

Hi Chengwen,

this patch is not directly relevant to the rest of the patchset and is a
necessary fix. Can you perhaps submit this fix separately so it can be
merged, even when the rest of the patchset is looking for reviews?

Thanks,
/Bruce

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v4 0/5] support dmadev/ethdev stats reset
  2023-01-20  3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
                     ` (5 preceding siblings ...)
  2023-02-06  8:39   ` [PATCH v4 0/5] support dmadev/ethdev stats reset Thomas Monjalon
@ 2023-02-08 14:17   ` Bruce Richardson
  2023-02-09  1:45     ` fengchengwen
  6 siblings, 1 reply; 62+ messages in thread
From: Bruce Richardson @ 2023-02-08 14:17 UTC (permalink / raw)
  To: Chengwen Feng
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On Fri, Jan 20, 2023 at 03:34:51AM +0000, Chengwen Feng wrote:
> This patchset contains dmadev/ethdev stats reset, and also support
> hide zero for ethdev xstats and two telemetry related bugs.
> 
> Chengwen Feng (5):
>   dmadev: support stats reset telemetry command
>   telemetry: fix repeat display when callback don't init dict
>   ethdev: add newline to telemetry log string
>   ethdev: support xstats reset telemetry command
>   ethdev: telemetry xstats support hide zero
> 
On one level, I can understand how this may be useful, but on the other
side, we also support having multiple different clients connecting to
telemetry simultaneously. Would it not be problematic to have stats
suddenly being reset by one connection without the others being aware of
it?

/Bruce

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v4 2/5] telemetry: fix repeat display when callback don't init dict
  2023-02-08 14:15     ` Bruce Richardson
@ 2023-02-09  1:33       ` fengchengwen
  0 siblings, 0 replies; 62+ messages in thread
From: fengchengwen @ 2023-02-09  1:33 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On 2023/2/8 22:15, Bruce Richardson wrote:
> On Fri, Jan 20, 2023 at 03:34:53AM +0000, Chengwen Feng wrote:
>> When a telemetry callback doesn't initialize the telemetry data
>> structure and returns a non-negative number, the telemetry will repeat
>> to display the last result. This patch zero the data structure to avoid
>> the problem.
>>
>> Fixes: 6dd571fd07c3 ("telemetry: introduce new functionality")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
>> ---
>>  lib/telemetry/telemetry.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/lib/telemetry/telemetry.c b/lib/telemetry/telemetry.c
>> index 8fbb4f3060..7b905355cd 100644
>> --- a/lib/telemetry/telemetry.c
>> +++ b/lib/telemetry/telemetry.c
>> @@ -333,7 +333,7 @@ output_json(const char *cmd, const struct rte_tel_data *d, int s)
>>  static void
>>  perform_command(telemetry_cb fn, const char *cmd, const char *param, int s)
>>  {
>> -	struct rte_tel_data data;
>> +	struct rte_tel_data data = {0};
>>  
>>  	int ret = fn(cmd, param, &data);
>>  	if (ret < 0) {
>> -- 
> 
> Hi Chengwen,
> 
> this patch is not directly relevant to the rest of the patchset and is a
> necessary fix. Can you perhaps submit this fix separately so it can be
> merged, even when the rest of the patchset is looking for reviews?

done, thanks.

> 
> Thanks,
> /Bruce
> 
> .
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v4 0/5] support dmadev/ethdev stats reset
  2023-02-08 14:17   ` Bruce Richardson
@ 2023-02-09  1:45     ` fengchengwen
  0 siblings, 0 replies; 62+ messages in thread
From: fengchengwen @ 2023-02-09  1:45 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: thomas, ferruh.yigit, andrew.rybchenko, dev, ciara.power, kevin.laatz

On 2023/2/8 22:17, Bruce Richardson wrote:
> On Fri, Jan 20, 2023 at 03:34:51AM +0000, Chengwen Feng wrote:
>> This patchset contains dmadev/ethdev stats reset, and also support
>> hide zero for ethdev xstats and two telemetry related bugs.
>>
>> Chengwen Feng (5):
>>   dmadev: support stats reset telemetry command
>>   telemetry: fix repeat display when callback don't init dict
>>   ethdev: add newline to telemetry log string
>>   ethdev: support xstats reset telemetry command
>>   ethdev: telemetry xstats support hide zero
>>
> On one level, I can understand how this may be useful, but on the other
> side, we also support having multiple different clients connecting to
> telemetry simultaneously. Would it not be problematic to have stats
> suddenly being reset by one connection without the others being aware of
> it?

It's a problem. Anyone who can connect can execute the commands.
For this modify case, I suggest adding LOG for later refer. and will fix in V5.

Thanks.

> 
> /Bruce
> .
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v5 0/2] support dmadev/ethdev stats reset
  2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
                   ` (9 preceding siblings ...)
  2023-01-20  3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
@ 2023-02-09  2:32 ` Chengwen Feng
  2023-02-09  2:32   ` [PATCH v5 1/2] dmadev: support stats reset telemetry command Chengwen Feng
  2023-02-09  2:32   ` [PATCH v5 2/2] ethdev: support xstats " Chengwen Feng
  10 siblings, 2 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-02-09  2:32 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

This patchset contains dmadev/ethdev stats reset.

Chengwen Feng (2):
  dmadev: support stats reset telemetry command
  ethdev: support xstats reset telemetry command

---
v5:
* address Bruce's comments:
  support LOG with stats reset result.
  reassemble the patchset.
v4:
* solve the internal don't commit for 5/5 patch.
v3:
* address Bruce's comment on 1/5 and 2/5 patch, and also for 4/5 patch.
* reword the help info of 5/5 patch.
v2:
* address Bruce's comment on 2/5 patch.
* support output success when stats reset.

 lib/dmadev/rte_dmadev.c | 47 +++++++++++++++++++++++++++++++++++++++++
 lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v5 1/2] dmadev: support stats reset telemetry command
  2023-02-09  2:32 ` [PATCH v5 0/2] " Chengwen Feng
@ 2023-02-09  2:32   ` Chengwen Feng
  2023-02-09  2:32   ` [PATCH v5 2/2] ethdev: support xstats " Chengwen Feng
  1 sibling, 0 replies; 62+ messages in thread
From: Chengwen Feng @ 2023-02-09  2:32 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The stats reset is useful for debugging, so add it to the dmadev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
Reviewed-by: Bruce Richardson <bruce.richardson@intel.com>
---
 lib/dmadev/rte_dmadev.c | 47 +++++++++++++++++++++++++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/lib/dmadev/rte_dmadev.c b/lib/dmadev/rte_dmadev.c
index 8c095e1f35..acd4cc9750 100644
--- a/lib/dmadev/rte_dmadev.c
+++ b/lib/dmadev/rte_dmadev.c
@@ -994,6 +994,51 @@ dmadev_handle_dev_stats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+dmadev_handle_dev_stats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d)
+{
+	struct rte_dma_info dma_info;
+	int dev_id, ret, vchan_id;
+	const char *vchan_param;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -EINVAL;
+
+	dev_id = strtoul(params, &end_param, 0);
+
+	/* Function info_get validates dev_id so we don't need to. */
+	ret = rte_dma_info_get(dev_id, &dma_info);
+	if (ret < 0)
+		return -EINVAL;
+
+	/* If the device has one vchan the user does not need to supply the
+	 * vchan id and only the device id is needed, no extra parameters.
+	 */
+	if (dma_info.nb_vchans == 1 && *end_param == '\0') {
+		vchan_id = 0;
+	} else {
+		vchan_param = strtok(end_param, ",");
+		if (vchan_param == NULL || strlen(vchan_param) == 0 || !isdigit(*vchan_param))
+			return -EINVAL;
+		vchan_id = strtoul(vchan_param, &end_param, 0);
+	}
+	if (*end_param != '\0')
+		RTE_DMA_LOG(WARNING, "Extra parameters passed to dmadev telemetry command, ignoring");
+
+	ret = rte_dma_stats_reset(dev_id, vchan_id);
+	if (ret == 0) {
+		rte_tel_data_string(d, "success");
+		RTE_DMA_LOG(NOTICE, "Device %d reset stats success", dev_id);
+	} else {
+		RTE_DMA_LOG(ERR, "Device %d reset stats failed! ret: %d", dev_id, ret);
+	}
+
+	return ret;
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 dmadev_handle_dev_dump(const char *cmd __rte_unused,
@@ -1041,6 +1086,8 @@ RTE_INIT(dmadev_init_telemetry)
 			"Returns information for a dmadev. Parameters: int dev_id");
 	rte_telemetry_register_cmd("/dmadev/stats", dmadev_handle_dev_stats,
 			"Returns the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
+	rte_telemetry_register_cmd("/dmadev/stats_reset", dmadev_handle_dev_stats_reset,
+			"Reset the stats for a dmadev vchannel. Parameters: int dev_id, vchan_id (Optional if only one vchannel)");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/dmadev/dump", dmadev_handle_dev_dump,
 			"Returns dump information for a dmadev. Parameters: int dev_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-02-09  2:32 ` [PATCH v5 0/2] " Chengwen Feng
  2023-02-09  2:32   ` [PATCH v5 1/2] dmadev: support stats reset telemetry command Chengwen Feng
@ 2023-02-09  2:32   ` Chengwen Feng
  2023-02-15  3:19     ` Dongdong Liu
  1 sibling, 1 reply; 62+ messages in thread
From: Chengwen Feng @ 2023-02-09  2:32 UTC (permalink / raw)
  To: thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

The xstats reset is useful for debugging, so add it to the ethdev
telemetry command lists.

Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
---
 lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++++++++++++
 1 file changed, 31 insertions(+)

diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
index d25db35f7f..e85c98f307 100644
--- a/lib/ethdev/rte_ethdev.c
+++ b/lib/ethdev/rte_ethdev.c
@@ -5915,6 +5915,35 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
 	return 0;
 }
 
+static int
+eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
+		const char *params,
+		struct rte_tel_data *d)
+{
+	int port_id, ret;
+	char *end_param;
+
+	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
+		return -1;
+
+	port_id = strtoul(params, &end_param, 0);
+	if (*end_param != '\0')
+		RTE_ETHDEV_LOG(NOTICE,
+			"Extra parameters passed to ethdev telemetry command, ignoring\n");
+	if (!rte_eth_dev_is_valid_port(port_id))
+		return -1;
+
+	ret = rte_eth_xstats_reset(port_id);
+	if (ret == 0) {
+		rte_tel_data_string(d, "success");
+		RTE_ETHDEV_LOG(NOTICE, "Port %d reset xstats success\n", port_id);
+	} else {
+		RTE_ETHDEV_LOG(ERR, "Port %d reset xstats failed! ret: %d\n", port_id, ret);
+	}
+
+	return ret;
+}
+
 #ifndef RTE_EXEC_ENV_WINDOWS
 static int
 eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
@@ -6329,6 +6358,8 @@ RTE_INIT(ethdev_init_telemetry)
 			"Returns the common stats for a port. Parameters: int port_id");
 	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
 			"Returns the extended stats for a port. Parameters: int port_id");
+	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
+			"Reset the extended stats for a port. Parameters: int port_id");
 #ifndef RTE_EXEC_ENV_WINDOWS
 	rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
 			"Returns dump private information for a port. Parameters: int port_id");
-- 
2.17.1


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-02-09  2:32   ` [PATCH v5 2/2] ethdev: support xstats " Chengwen Feng
@ 2023-02-15  3:19     ` Dongdong Liu
  2023-02-16 11:53       ` fengchengwen
  0 siblings, 1 reply; 62+ messages in thread
From: Dongdong Liu @ 2023-02-15  3:19 UTC (permalink / raw)
  To: Chengwen Feng, thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

Hi Chengwen

On 2023/2/9 10:32, Chengwen Feng wrote:
> The xstats reset is useful for debugging, so add it to the ethdev
> telemetry command lists.
>
> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
This patch looks good, so
Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>

A minior question
Do we need to support stats reset ?

Thanks,
Dongdong
> ---
>  lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++++++++++++
>  1 file changed, 31 insertions(+)
>
> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
> index d25db35f7f..e85c98f307 100644
> --- a/lib/ethdev/rte_ethdev.c
> +++ b/lib/ethdev/rte_ethdev.c
> @@ -5915,6 +5915,35 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
>  	return 0;
>  }
>
> +static int
> +eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
> +		const char *params,
> +		struct rte_tel_data *d)
> +{
> +	int port_id, ret;
> +	char *end_param;
> +
> +	if (params == NULL || strlen(params) == 0 || !isdigit(*params))
> +		return -1;
> +
> +	port_id = strtoul(params, &end_param, 0);
> +	if (*end_param != '\0')
> +		RTE_ETHDEV_LOG(NOTICE,
> +			"Extra parameters passed to ethdev telemetry command, ignoring\n");
> +	if (!rte_eth_dev_is_valid_port(port_id))
> +		return -1;
> +
> +	ret = rte_eth_xstats_reset(port_id);
> +	if (ret == 0) {
> +		rte_tel_data_string(d, "success");
> +		RTE_ETHDEV_LOG(NOTICE, "Port %d reset xstats success\n", port_id);
> +	} else {
> +		RTE_ETHDEV_LOG(ERR, "Port %d reset xstats failed! ret: %d\n", port_id, ret);
> +	}
> +
> +	return ret;
> +}
> +
>  #ifndef RTE_EXEC_ENV_WINDOWS
>  static int
>  eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
> @@ -6329,6 +6358,8 @@ RTE_INIT(ethdev_init_telemetry)
>  			"Returns the common stats for a port. Parameters: int port_id");
>  	rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
>  			"Returns the extended stats for a port. Parameters: int port_id");
> +	rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
> +			"Reset the extended stats for a port. Parameters: int port_id");
>  #ifndef RTE_EXEC_ENV_WINDOWS
>  	rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
>  			"Returns dump private information for a port. Parameters: int port_id");
>

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-02-15  3:19     ` Dongdong Liu
@ 2023-02-16 11:53       ` fengchengwen
  2023-02-16 12:06         ` Ferruh Yigit
  0 siblings, 1 reply; 62+ messages in thread
From: fengchengwen @ 2023-02-16 11:53 UTC (permalink / raw)
  To: Dongdong Liu, thomas, ferruh.yigit, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

On 2023/2/15 11:19, Dongdong Liu wrote:
> Hi Chengwen
> 
> On 2023/2/9 10:32, Chengwen Feng wrote:
>> The xstats reset is useful for debugging, so add it to the ethdev
>> telemetry command lists.
>>
>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> This patch looks good, so
> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
> 
> A minior question
> Do we need to support stats reset ?

Stats is contained by xstats, and future direction I think is xstats.
So I think we don't need support stats reset.

Thanks.

> 
> Thanks,
> Dongdong
>> ---
>>  lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++++++++++++
>>  1 file changed, 31 insertions(+)
>>
>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>> index d25db35f7f..e85c98f307 100644
>> --- a/lib/ethdev/rte_ethdev.c
>> +++ b/lib/ethdev/rte_ethdev.c
>> @@ -5915,6 +5915,35 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
>>      return 0;
>>  }
>>
>> +static int
>> +eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
>> +        const char *params,
>> +        struct rte_tel_data *d)
>> +{
>> +    int port_id, ret;
>> +    char *end_param;
>> +
>> +    if (params == NULL || strlen(params) == 0 || !isdigit(*params))
>> +        return -1;
>> +
>> +    port_id = strtoul(params, &end_param, 0);
>> +    if (*end_param != '\0')
>> +        RTE_ETHDEV_LOG(NOTICE,
>> +            "Extra parameters passed to ethdev telemetry command, ignoring\n");
>> +    if (!rte_eth_dev_is_valid_port(port_id))
>> +        return -1;
>> +
>> +    ret = rte_eth_xstats_reset(port_id);
>> +    if (ret == 0) {
>> +        rte_tel_data_string(d, "success");
>> +        RTE_ETHDEV_LOG(NOTICE, "Port %d reset xstats success\n", port_id);
>> +    } else {
>> +        RTE_ETHDEV_LOG(ERR, "Port %d reset xstats failed! ret: %d\n", port_id, ret);
>> +    }
>> +
>> +    return ret;
>> +}
>> +
>>  #ifndef RTE_EXEC_ENV_WINDOWS
>>  static int
>>  eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
>> @@ -6329,6 +6358,8 @@ RTE_INIT(ethdev_init_telemetry)
>>              "Returns the common stats for a port. Parameters: int port_id");
>>      rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
>>              "Returns the extended stats for a port. Parameters: int port_id");
>> +    rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
>> +            "Reset the extended stats for a port. Parameters: int port_id");
>>  #ifndef RTE_EXEC_ENV_WINDOWS
>>      rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
>>              "Returns dump private information for a port. Parameters: int port_id");
>>
> .

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-02-16 11:53       ` fengchengwen
@ 2023-02-16 12:06         ` Ferruh Yigit
  2023-02-16 12:42           ` fengchengwen
  0 siblings, 1 reply; 62+ messages in thread
From: Ferruh Yigit @ 2023-02-16 12:06 UTC (permalink / raw)
  To: fengchengwen, Dongdong Liu, thomas, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

On 2/16/2023 11:53 AM, fengchengwen wrote:
> On 2023/2/15 11:19, Dongdong Liu wrote:
>> Hi Chengwen
>>
>> On 2023/2/9 10:32, Chengwen Feng wrote:
>>> The xstats reset is useful for debugging, so add it to the ethdev
>>> telemetry command lists.
>>>
>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>> This patch looks good, so
>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
>>
>> A minior question
>> Do we need to support stats reset ?
> 
> Stats is contained by xstats, and future direction I think is xstats.
> So I think we don't need support stats reset.
> 

I have similar question with Dongdong, readonly values are safe for
telemetry, but modifying data can be more tricky since we don't have
locking in ethdev APIs, this can cause concurrency issues.

Overall do we want telemetry go that way and become something that
alters ethdev data/config?


> Thanks.
> 
>>
>> Thanks,
>> Dongdong
>>> ---
>>>  lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++++++++++++
>>>  1 file changed, 31 insertions(+)
>>>
>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>> index d25db35f7f..e85c98f307 100644
>>> --- a/lib/ethdev/rte_ethdev.c
>>> +++ b/lib/ethdev/rte_ethdev.c
>>> @@ -5915,6 +5915,35 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
>>>      return 0;
>>>  }
>>>
>>> +static int
>>> +eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
>>> +        const char *params,
>>> +        struct rte_tel_data *d)
>>> +{
>>> +    int port_id, ret;
>>> +    char *end_param;
>>> +
>>> +    if (params == NULL || strlen(params) == 0 || !isdigit(*params))
>>> +        return -1;
>>> +
>>> +    port_id = strtoul(params, &end_param, 0);
>>> +    if (*end_param != '\0')
>>> +        RTE_ETHDEV_LOG(NOTICE,
>>> +            "Extra parameters passed to ethdev telemetry command, ignoring\n");
>>> +    if (!rte_eth_dev_is_valid_port(port_id))
>>> +        return -1;
>>> +
>>> +    ret = rte_eth_xstats_reset(port_id);
>>> +    if (ret == 0) {
>>> +        rte_tel_data_string(d, "success");
>>> +        RTE_ETHDEV_LOG(NOTICE, "Port %d reset xstats success\n", port_id);
>>> +    } else {
>>> +        RTE_ETHDEV_LOG(ERR, "Port %d reset xstats failed! ret: %d\n", port_id, ret);
>>> +    }
>>> +
>>> +    return ret;
>>> +}
>>> +
>>>  #ifndef RTE_EXEC_ENV_WINDOWS
>>>  static int
>>>  eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
>>> @@ -6329,6 +6358,8 @@ RTE_INIT(ethdev_init_telemetry)
>>>              "Returns the common stats for a port. Parameters: int port_id");
>>>      rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
>>>              "Returns the extended stats for a port. Parameters: int port_id");
>>> +    rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
>>> +            "Reset the extended stats for a port. Parameters: int port_id");
>>>  #ifndef RTE_EXEC_ENV_WINDOWS
>>>      rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
>>>              "Returns dump private information for a port. Parameters: int port_id");
>>>
>> .


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-02-16 12:06         ` Ferruh Yigit
@ 2023-02-16 12:42           ` fengchengwen
  2023-02-16 12:54             ` Bruce Richardson
  0 siblings, 1 reply; 62+ messages in thread
From: fengchengwen @ 2023-02-16 12:42 UTC (permalink / raw)
  To: Ferruh Yigit, Dongdong Liu, thomas, andrew.rybchenko
  Cc: dev, ciara.power, kevin.laatz, bruce.richardson

On 2023/2/16 20:06, Ferruh Yigit wrote:
> On 2/16/2023 11:53 AM, fengchengwen wrote:
>> On 2023/2/15 11:19, Dongdong Liu wrote:
>>> Hi Chengwen
>>>
>>> On 2023/2/9 10:32, Chengwen Feng wrote:
>>>> The xstats reset is useful for debugging, so add it to the ethdev
>>>> telemetry command lists.
>>>>
>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>> This patch looks good, so
>>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
>>>
>>> A minior question
>>> Do we need to support stats reset ?
>>
>> Stats is contained by xstats, and future direction I think is xstats.
>> So I think we don't need support stats reset.
>>
> 
> I have similar question with Dongdong, readonly values are safe for
> telemetry, but modifying data can be more tricky since we don't have
> locking in ethdev APIs, this can cause concurrency issues.

Yes, it indeed has concurrency issues.

> 
> Overall do we want telemetry go that way and become something that
> alters ethdev data/config?

There are at least two part of data: config and status.
For stats (which belong status data) could help for debugging, I think it's acceptable.

As for concurrency issues. People should know what to do and when to do, just like
the don't invoke config API (e.g. dev_configure/dev_start/...) concurrency.

> 
> 
>> Thanks.
>>
>>>
>>> Thanks,
>>> Dongdong
>>>> ---
>>>>  lib/ethdev/rte_ethdev.c | 31 +++++++++++++++++++++++++++++++
>>>>  1 file changed, 31 insertions(+)
>>>>
>>>> diff --git a/lib/ethdev/rte_ethdev.c b/lib/ethdev/rte_ethdev.c
>>>> index d25db35f7f..e85c98f307 100644
>>>> --- a/lib/ethdev/rte_ethdev.c
>>>> +++ b/lib/ethdev/rte_ethdev.c
>>>> @@ -5915,6 +5915,35 @@ eth_dev_handle_port_xstats(const char *cmd __rte_unused,
>>>>      return 0;
>>>>  }
>>>>
>>>> +static int
>>>> +eth_dev_handle_port_xstats_reset(const char *cmd __rte_unused,
>>>> +        const char *params,
>>>> +        struct rte_tel_data *d)
>>>> +{
>>>> +    int port_id, ret;
>>>> +    char *end_param;
>>>> +
>>>> +    if (params == NULL || strlen(params) == 0 || !isdigit(*params))
>>>> +        return -1;
>>>> +
>>>> +    port_id = strtoul(params, &end_param, 0);
>>>> +    if (*end_param != '\0')
>>>> +        RTE_ETHDEV_LOG(NOTICE,
>>>> +            "Extra parameters passed to ethdev telemetry command, ignoring\n");
>>>> +    if (!rte_eth_dev_is_valid_port(port_id))
>>>> +        return -1;
>>>> +
>>>> +    ret = rte_eth_xstats_reset(port_id);
>>>> +    if (ret == 0) {
>>>> +        rte_tel_data_string(d, "success");
>>>> +        RTE_ETHDEV_LOG(NOTICE, "Port %d reset xstats success\n", port_id);
>>>> +    } else {
>>>> +        RTE_ETHDEV_LOG(ERR, "Port %d reset xstats failed! ret: %d\n", port_id, ret);
>>>> +    }
>>>> +
>>>> +    return ret;
>>>> +}
>>>> +
>>>>  #ifndef RTE_EXEC_ENV_WINDOWS
>>>>  static int
>>>>  eth_dev_handle_port_dump_priv(const char *cmd __rte_unused,
>>>> @@ -6329,6 +6358,8 @@ RTE_INIT(ethdev_init_telemetry)
>>>>              "Returns the common stats for a port. Parameters: int port_id");
>>>>      rte_telemetry_register_cmd("/ethdev/xstats", eth_dev_handle_port_xstats,
>>>>              "Returns the extended stats for a port. Parameters: int port_id");
>>>> +    rte_telemetry_register_cmd("/ethdev/xstats_reset", eth_dev_handle_port_xstats_reset,
>>>> +            "Reset the extended stats for a port. Parameters: int port_id");
>>>>  #ifndef RTE_EXEC_ENV_WINDOWS
>>>>      rte_telemetry_register_cmd("/ethdev/dump_priv", eth_dev_handle_port_dump_priv,
>>>>              "Returns dump private information for a port. Parameters: int port_id");
>>>>
>>> .
> 
> .
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-02-16 12:42           ` fengchengwen
@ 2023-02-16 12:54             ` Bruce Richardson
  2023-02-16 12:55               ` Bruce Richardson
  2023-02-17  9:44               ` fengchengwen
  0 siblings, 2 replies; 62+ messages in thread
From: Bruce Richardson @ 2023-02-16 12:54 UTC (permalink / raw)
  To: fengchengwen
  Cc: Ferruh Yigit, Dongdong Liu, thomas, andrew.rybchenko, dev,
	ciara.power, kevin.laatz

On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote:
> On 2023/2/16 20:06, Ferruh Yigit wrote:
> > On 2/16/2023 11:53 AM, fengchengwen wrote:
> >> On 2023/2/15 11:19, Dongdong Liu wrote:
> >>> Hi Chengwen
> >>>
> >>> On 2023/2/9 10:32, Chengwen Feng wrote:
> >>>> The xstats reset is useful for debugging, so add it to the ethdev
> >>>> telemetry command lists.
> >>>>
> >>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> >>> This patch looks good, so
> >>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
> >>>
> >>> A minior question
> >>> Do we need to support stats reset ?
> >>
> >> Stats is contained by xstats, and future direction I think is xstats.
> >> So I think we don't need support stats reset.
> >>
> > 
> > I have similar question with Dongdong, readonly values are safe for
> > telemetry, but modifying data can be more tricky since we don't have
> > locking in ethdev APIs, this can cause concurrency issues.
> 
> Yes, it indeed has concurrency issues.
> 
> > 
> > Overall do we want telemetry go that way and become something that
> > alters ethdev data/config?
> 
> There are at least two part of data: config and status.
> For stats (which belong status data) could help for debugging, I think it's acceptable.
> 
> As for concurrency issues. People should know what to do and when to do, just like
> the don't invoke config API (e.g. dev_configure/dev_start/...) concurrency.
> 
While this is probably ok for now, I think in next release we should look
to add some sort of support for locking for destructive ops in a future
release. For example, we could:

1. Add support for marking a callback as "destructive" and only allow it to
be called if only one connection is present or

2. Make it possible for callbacks to query the number of connections so
that the callback itself is non-destructive in more than one connection is
open.

[Both of these will require locking support so that new connections aren't
openned when the callback is in-flight!]

Any other thoughts or suggestions?

/Bruce

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-02-16 12:54             ` Bruce Richardson
@ 2023-02-16 12:55               ` Bruce Richardson
  2023-02-17  9:44               ` fengchengwen
  1 sibling, 0 replies; 62+ messages in thread
From: Bruce Richardson @ 2023-02-16 12:55 UTC (permalink / raw)
  To: fengchengwen
  Cc: Ferruh Yigit, Dongdong Liu, thomas, andrew.rybchenko, dev,
	ciara.power, kevin.laatz

On Thu, Feb 16, 2023 at 12:54:20PM +0000, Bruce Richardson wrote:
> On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote:
> > On 2023/2/16 20:06, Ferruh Yigit wrote:
> > > On 2/16/2023 11:53 AM, fengchengwen wrote:
> > >> On 2023/2/15 11:19, Dongdong Liu wrote:
> > >>> Hi Chengwen
> > >>>
> > >>> On 2023/2/9 10:32, Chengwen Feng wrote:
> > >>>> The xstats reset is useful for debugging, so add it to the ethdev
> > >>>> telemetry command lists.
> > >>>>
> > >>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > >>> This patch looks good, so
> > >>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
> > >>>
> > >>> A minior question
> > >>> Do we need to support stats reset ?
> > >>
> > >> Stats is contained by xstats, and future direction I think is xstats.
> > >> So I think we don't need support stats reset.
> > >>
> > > 
> > > I have similar question with Dongdong, readonly values are safe for
> > > telemetry, but modifying data can be more tricky since we don't have
> > > locking in ethdev APIs, this can cause concurrency issues.
> > 
> > Yes, it indeed has concurrency issues.
> > 
> > > 
> > > Overall do we want telemetry go that way and become something that
> > > alters ethdev data/config?
> > 
> > There are at least two part of data: config and status.
> > For stats (which belong status data) could help for debugging, I think it's acceptable.
> > 
> > As for concurrency issues. People should know what to do and when to do, just like
> > the don't invoke config API (e.g. dev_configure/dev_start/...) concurrency.
> > 
> While this is probably ok for now, I think in next release we should look
> to add some sort of support for locking for destructive ops in a future
> release. For example, we could:
> 
> 1. Add support for marking a callback as "destructive" and only allow it to
> be called if only one connection is present or
> 
> 2. Make it possible for callbacks to query the number of connections so
> that the callback itself is non-destructive in more than one connection is
> open.
> 
> [Both of these will require locking support so that new connections aren't
> openned when the callback is in-flight!]
> 
> Any other thoughts or suggestions?
> 
Actually, another thought - we could also make the max-connections
configurable at runtime, so that the user can enforce only a single
connection.

/Bruce

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-02-16 12:54             ` Bruce Richardson
  2023-02-16 12:55               ` Bruce Richardson
@ 2023-02-17  9:44               ` fengchengwen
  2023-02-20 13:05                 ` Thomas Monjalon
  1 sibling, 1 reply; 62+ messages in thread
From: fengchengwen @ 2023-02-17  9:44 UTC (permalink / raw)
  To: Bruce Richardson
  Cc: Ferruh Yigit, Dongdong Liu, thomas, andrew.rybchenko, dev,
	ciara.power, kevin.laatz

On 2023/2/16 20:54, Bruce Richardson wrote:
> On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote:
>> On 2023/2/16 20:06, Ferruh Yigit wrote:
>>> On 2/16/2023 11:53 AM, fengchengwen wrote:
>>>> On 2023/2/15 11:19, Dongdong Liu wrote:
>>>>> Hi Chengwen
>>>>>
>>>>> On 2023/2/9 10:32, Chengwen Feng wrote:
>>>>>> The xstats reset is useful for debugging, so add it to the ethdev
>>>>>> telemetry command lists.
>>>>>>
>>>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>> This patch looks good, so
>>>>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
>>>>>
>>>>> A minior question
>>>>> Do we need to support stats reset ?
>>>>
>>>> Stats is contained by xstats, and future direction I think is xstats.
>>>> So I think we don't need support stats reset.
>>>>
>>>
>>> I have similar question with Dongdong, readonly values are safe for
>>> telemetry, but modifying data can be more tricky since we don't have
>>> locking in ethdev APIs, this can cause concurrency issues.
>>
>> Yes, it indeed has concurrency issues.
>>
>>>
>>> Overall do we want telemetry go that way and become something that
>>> alters ethdev data/config?
>>
>> There are at least two part of data: config and status.
>> For stats (which belong status data) could help for debugging, I think it's acceptable.
>>
>> As for concurrency issues. People should know what to do and when to do, just like
>> the don't invoke config API (e.g. dev_configure/dev_start/...) concurrency.
>>
> While this is probably ok for now, I think in next release we should look
> to add some sort of support for locking for destructive ops in a future
> release. For example, we could:
> 
> 1. Add support for marking a callback as "destructive" and only allow it to
> be called if only one connection is present or
> 
> 2. Make it possible for callbacks to query the number of connections so
> that the callback itself is non-destructive in more than one connection is
> open.
> 
> [Both of these will require locking support so that new connections aren't
> openned when the callback is in-flight!]

Except telemetry, the application may have other console could execute DPDK API.
So I think trying to keep it simple, it's up to the user to invoke.

> 
> Any other thoughts or suggestions?
> 
> /Bruce
> .
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-02-17  9:44               ` fengchengwen
@ 2023-02-20 13:05                 ` Thomas Monjalon
  2023-07-03  3:58                   ` fengchengwen
  0 siblings, 1 reply; 62+ messages in thread
From: Thomas Monjalon @ 2023-02-20 13:05 UTC (permalink / raw)
  To: Bruce Richardson, Dongdong Liu, fengchengwen
  Cc: dev, Ferruh Yigit, andrew.rybchenko, dev, ciara.power,
	kevin.laatz, david.marchand

17/02/2023 10:44, fengchengwen:
> On 2023/2/16 20:54, Bruce Richardson wrote:
> > On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote:
> >> On 2023/2/16 20:06, Ferruh Yigit wrote:
> >>> On 2/16/2023 11:53 AM, fengchengwen wrote:
> >>>> On 2023/2/15 11:19, Dongdong Liu wrote:
> >>>>> Hi Chengwen
> >>>>>
> >>>>> On 2023/2/9 10:32, Chengwen Feng wrote:
> >>>>>> The xstats reset is useful for debugging, so add it to the ethdev
> >>>>>> telemetry command lists.
> >>>>>>
> >>>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> >>>>> This patch looks good, so
> >>>>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
> >>>>>
> >>>>> A minior question
> >>>>> Do we need to support stats reset ?
> >>>>
> >>>> Stats is contained by xstats, and future direction I think is xstats.
> >>>> So I think we don't need support stats reset.
> >>>>
> >>>
> >>> I have similar question with Dongdong, readonly values are safe for
> >>> telemetry, but modifying data can be more tricky since we don't have
> >>> locking in ethdev APIs, this can cause concurrency issues.
> >>
> >> Yes, it indeed has concurrency issues.
> >>
> >>>
> >>> Overall do we want telemetry go that way and become something that
> >>> alters ethdev data/config?
> >>
> >> There are at least two part of data: config and status.
> >> For stats (which belong status data) could help for debugging, I think it's acceptable.
> >>
> >> As for concurrency issues. People should know what to do and when to do, just like
> >> the don't invoke config API (e.g. dev_configure/dev_start/...) concurrency.
> >>
> > While this is probably ok for now, I think in next release we should look
> > to add some sort of support for locking for destructive ops in a future
> > release. For example, we could:
> > 
> > 1. Add support for marking a callback as "destructive" and only allow it to
> > be called if only one connection is present or
> > 
> > 2. Make it possible for callbacks to query the number of connections so
> > that the callback itself is non-destructive in more than one connection is
> > open.
> > 
> > [Both of these will require locking support so that new connections aren't
> > openned when the callback is in-flight!]
> 
> Except telemetry, the application may have other console could execute DPDK API.
> So I think trying to keep it simple, it's up to the user to invoke.

No, the user should not be responsible for concurrency issues.
We can ask the app developper to take care,
but not to the user who has no control on what happens in the app.

On a more general note, I feel the expansion of telemetry is not controlled enough.
I would like to stop on adding more telemetry until we have a clear guideline
about what is telemetry for and how to use it.



^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-02-20 13:05                 ` Thomas Monjalon
@ 2023-07-03  3:58                   ` fengchengwen
  2023-07-03  7:20                     ` Thomas Monjalon
  0 siblings, 1 reply; 62+ messages in thread
From: fengchengwen @ 2023-07-03  3:58 UTC (permalink / raw)
  To: Thomas Monjalon, Bruce Richardson, Dongdong Liu
  Cc: dev, Ferruh Yigit, andrew.rybchenko, ciara.power, kevin.laatz,
	david.marchand



On 2023/2/20 21:05, Thomas Monjalon wrote:
> 17/02/2023 10:44, fengchengwen:
>> On 2023/2/16 20:54, Bruce Richardson wrote:
>>> On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote:
>>>> On 2023/2/16 20:06, Ferruh Yigit wrote:
>>>>> On 2/16/2023 11:53 AM, fengchengwen wrote:
>>>>>> On 2023/2/15 11:19, Dongdong Liu wrote:
>>>>>>> Hi Chengwen
>>>>>>>
>>>>>>> On 2023/2/9 10:32, Chengwen Feng wrote:
>>>>>>>> The xstats reset is useful for debugging, so add it to the ethdev
>>>>>>>> telemetry command lists.
>>>>>>>>
>>>>>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>>>> This patch looks good, so
>>>>>>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
>>>>>>>
>>>>>>> A minior question
>>>>>>> Do we need to support stats reset ?
>>>>>>
>>>>>> Stats is contained by xstats, and future direction I think is xstats.
>>>>>> So I think we don't need support stats reset.
>>>>>>
>>>>>
>>>>> I have similar question with Dongdong, readonly values are safe for
>>>>> telemetry, but modifying data can be more tricky since we don't have
>>>>> locking in ethdev APIs, this can cause concurrency issues.
>>>>
>>>> Yes, it indeed has concurrency issues.
>>>>
>>>>>
>>>>> Overall do we want telemetry go that way and become something that
>>>>> alters ethdev data/config?
>>>>
>>>> There are at least two part of data: config and status.
>>>> For stats (which belong status data) could help for debugging, I think it's acceptable.
>>>>
>>>> As for concurrency issues. People should know what to do and when to do, just like
>>>> the don't invoke config API (e.g. dev_configure/dev_start/...) concurrency.
>>>>
>>> While this is probably ok for now, I think in next release we should look
>>> to add some sort of support for locking for destructive ops in a future
>>> release. For example, we could:
>>>
>>> 1. Add support for marking a callback as "destructive" and only allow it to
>>> be called if only one connection is present or
>>>
>>> 2. Make it possible for callbacks to query the number of connections so
>>> that the callback itself is non-destructive in more than one connection is
>>> open.
>>>
>>> [Both of these will require locking support so that new connections aren't
>>> openned when the callback is in-flight!]
>>
>> Except telemetry, the application may have other console could execute DPDK API.
>> So I think trying to keep it simple, it's up to the user to invoke.
> 
> No, the user should not be responsible for concurrency issues.
> We can ask the app developper to take care,
> but not to the user who has no control on what happens in the app.
> 
> On a more general note, I feel the expansion of telemetry is not controlled enough.
> I would like to stop on adding more telemetry until we have a clear guideline
> about what is telemetry for and how to use it.

Hi Thomas,

Should this be discussed on TB?

Thanks.

> 
> 
> .
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-07-03  3:58                   ` fengchengwen
@ 2023-07-03  7:20                     ` Thomas Monjalon
  2023-07-03 13:44                       ` Morten Brørup
  0 siblings, 1 reply; 62+ messages in thread
From: Thomas Monjalon @ 2023-07-03  7:20 UTC (permalink / raw)
  To: Dongdong Liu, fengchengwen
  Cc: Bruce Richardson, dev, Ferruh Yigit, andrew.rybchenko,
	ciara.power, kevin.laatz, david.marchand

03/07/2023 05:58, fengchengwen:
> 
> On 2023/2/20 21:05, Thomas Monjalon wrote:
> > 17/02/2023 10:44, fengchengwen:
> >> On 2023/2/16 20:54, Bruce Richardson wrote:
> >>> On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote:
> >>>> On 2023/2/16 20:06, Ferruh Yigit wrote:
> >>>>> On 2/16/2023 11:53 AM, fengchengwen wrote:
> >>>>>> On 2023/2/15 11:19, Dongdong Liu wrote:
> >>>>>>> Hi Chengwen
> >>>>>>>
> >>>>>>> On 2023/2/9 10:32, Chengwen Feng wrote:
> >>>>>>>> The xstats reset is useful for debugging, so add it to the ethdev
> >>>>>>>> telemetry command lists.
> >>>>>>>>
> >>>>>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> >>>>>>> This patch looks good, so
> >>>>>>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
> >>>>>>>
> >>>>>>> A minior question
> >>>>>>> Do we need to support stats reset ?
> >>>>>>
> >>>>>> Stats is contained by xstats, and future direction I think is xstats.
> >>>>>> So I think we don't need support stats reset.
> >>>>>>
> >>>>>
> >>>>> I have similar question with Dongdong, readonly values are safe for
> >>>>> telemetry, but modifying data can be more tricky since we don't have
> >>>>> locking in ethdev APIs, this can cause concurrency issues.
> >>>>
> >>>> Yes, it indeed has concurrency issues.
> >>>>
> >>>>>
> >>>>> Overall do we want telemetry go that way and become something that
> >>>>> alters ethdev data/config?
> >>>>
> >>>> There are at least two part of data: config and status.
> >>>> For stats (which belong status data) could help for debugging, I think it's acceptable.
> >>>>
> >>>> As for concurrency issues. People should know what to do and when to do, just like
> >>>> the don't invoke config API (e.g. dev_configure/dev_start/...) concurrency.
> >>>>
> >>> While this is probably ok for now, I think in next release we should look
> >>> to add some sort of support for locking for destructive ops in a future
> >>> release. For example, we could:
> >>>
> >>> 1. Add support for marking a callback as "destructive" and only allow it to
> >>> be called if only one connection is present or
> >>>
> >>> 2. Make it possible for callbacks to query the number of connections so
> >>> that the callback itself is non-destructive in more than one connection is
> >>> open.
> >>>
> >>> [Both of these will require locking support so that new connections aren't
> >>> openned when the callback is in-flight!]
> >>
> >> Except telemetry, the application may have other console could execute DPDK API.
> >> So I think trying to keep it simple, it's up to the user to invoke.
> > 
> > No, the user should not be responsible for concurrency issues.
> > We can ask the app developper to take care,
> > but not to the user who has no control on what happens in the app.
> > 
> > On a more general note, I feel the expansion of telemetry is not controlled enough.
> > I would like to stop on adding more telemetry until we have a clear guideline
> > about what is telemetry for and how to use it.
> 
> Hi Thomas,
> 
> Should this be discussed on TB?

What would be your question exactly?





^ permalink raw reply	[flat|nested] 62+ messages in thread

* RE: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-07-03  7:20                     ` Thomas Monjalon
@ 2023-07-03 13:44                       ` Morten Brørup
  2023-07-04  6:41                         ` fengchengwen
  0 siblings, 1 reply; 62+ messages in thread
From: Morten Brørup @ 2023-07-03 13:44 UTC (permalink / raw)
  To: Thomas Monjalon, Dongdong Liu, fengchengwen
  Cc: Bruce Richardson, dev, Ferruh Yigit, andrew.rybchenko,
	ciara.power, kevin.laatz, david.marchand

> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Monday, 3 July 2023 09.20
> 
> 03/07/2023 05:58, fengchengwen:
> >
> > On 2023/2/20 21:05, Thomas Monjalon wrote:
> > > 17/02/2023 10:44, fengchengwen:
> > >> On 2023/2/16 20:54, Bruce Richardson wrote:
> > >>> On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote:
> > >>>> On 2023/2/16 20:06, Ferruh Yigit wrote:
> > >>>>> On 2/16/2023 11:53 AM, fengchengwen wrote:
> > >>>>>> On 2023/2/15 11:19, Dongdong Liu wrote:
> > >>>>>>> Hi Chengwen
> > >>>>>>>
> > >>>>>>> On 2023/2/9 10:32, Chengwen Feng wrote:
> > >>>>>>>> The xstats reset is useful for debugging, so add it to the
> ethdev
> > >>>>>>>> telemetry command lists.
> > >>>>>>>>
> > >>>>>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
> > >>>>>>> This patch looks good, so
> > >>>>>>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
> > >>>>>>>
> > >>>>>>> A minior question
> > >>>>>>> Do we need to support stats reset ?
> > >>>>>>
> > >>>>>> Stats is contained by xstats, and future direction I think is
> xstats.
> > >>>>>> So I think we don't need support stats reset.
> > >>>>>>
> > >>>>>
> > >>>>> I have similar question with Dongdong, readonly values are safe
> for
> > >>>>> telemetry, but modifying data can be more tricky since we don't
> have
> > >>>>> locking in ethdev APIs, this can cause concurrency issues.
> > >>>>
> > >>>> Yes, it indeed has concurrency issues.
> > >>>>
> > >>>>>
> > >>>>> Overall do we want telemetry go that way and become something
> that
> > >>>>> alters ethdev data/config?
> > >>>>
> > >>>> There are at least two part of data: config and status.
> > >>>> For stats (which belong status data) could help for debugging, I
> think it's acceptable.
> > >>>>
> > >>>> As for concurrency issues. People should know what to do and when
> to do, just like
> > >>>> the don't invoke config API (e.g. dev_configure/dev_start/...)
> concurrency.
> > >>>>
> > >>> While this is probably ok for now, I think in next release we
> should look
> > >>> to add some sort of support for locking for destructive ops in a
> future
> > >>> release. For example, we could:
> > >>>
> > >>> 1. Add support for marking a callback as "destructive" and only
> allow it to
> > >>> be called if only one connection is present or
> > >>>
> > >>> 2. Make it possible for callbacks to query the number of
> connections so
> > >>> that the callback itself is non-destructive in more than one
> connection is
> > >>> open.
> > >>>
> > >>> [Both of these will require locking support so that new
> connections aren't
> > >>> openned when the callback is in-flight!]
> > >>
> > >> Except telemetry, the application may have other console could
> execute DPDK API.
> > >> So I think trying to keep it simple, it's up to the user to invoke.
> > >
> > > No, the user should not be responsible for concurrency issues.
> > > We can ask the app developper to take care,
> > > but not to the user who has no control on what happens in the app.
> > >
> > > On a more general note, I feel the expansion of telemetry is not
> controlled enough.
> > > I would like to stop on adding more telemetry until we have a clear
> guideline
> > > about what is telemetry for and how to use it.
> >
> > Hi Thomas,
> >
> > Should this be discussed on TB?
> 
> What would be your question exactly?

A general comment about telemetry:

If an application exposes telemetry through an end user facing API, e.g. http(s) REST, it would be nice if non-read-only telemetry paths are easy to identify by following some DPDK standard convention, so the application does not need to manually maintain an allow-list of read-only paths.

Bruce's documentation about trace/log/telemetry/dump might also need to be updated regarding non-read-only telemetry actions.


^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-07-03 13:44                       ` Morten Brørup
@ 2023-07-04  6:41                         ` fengchengwen
  2024-10-03 18:51                           ` Stephen Hemminger
  0 siblings, 1 reply; 62+ messages in thread
From: fengchengwen @ 2023-07-04  6:41 UTC (permalink / raw)
  To: Morten Brørup, Thomas Monjalon, Dongdong Liu
  Cc: Bruce Richardson, dev, Ferruh Yigit, andrew.rybchenko,
	ciara.power, kevin.laatz, david.marchand

Hi Thomas and Morten,

On 2023/7/3 21:44, Morten Brørup wrote:
>> From: Thomas Monjalon [mailto:thomas@monjalon.net]
>> Sent: Monday, 3 July 2023 09.20
>>
>> 03/07/2023 05:58, fengchengwen:
>>>
>>> On 2023/2/20 21:05, Thomas Monjalon wrote:
>>>> 17/02/2023 10:44, fengchengwen:
>>>>> On 2023/2/16 20:54, Bruce Richardson wrote:
>>>>>> On Thu, Feb 16, 2023 at 08:42:34PM +0800, fengchengwen wrote:
>>>>>>> On 2023/2/16 20:06, Ferruh Yigit wrote:
>>>>>>>> On 2/16/2023 11:53 AM, fengchengwen wrote:
>>>>>>>>> On 2023/2/15 11:19, Dongdong Liu wrote:
>>>>>>>>>> Hi Chengwen
>>>>>>>>>>
>>>>>>>>>> On 2023/2/9 10:32, Chengwen Feng wrote:
>>>>>>>>>>> The xstats reset is useful for debugging, so add it to the
>> ethdev
>>>>>>>>>>> telemetry command lists.
>>>>>>>>>>>
>>>>>>>>>>> Signed-off-by: Chengwen Feng <fengchengwen@huawei.com>
>>>>>>>>>> This patch looks good, so
>>>>>>>>>> Reviewed-by: Dongdong Liu <liudongdong3@huawei.com>
>>>>>>>>>>
>>>>>>>>>> A minior question
>>>>>>>>>> Do we need to support stats reset ?
>>>>>>>>>
>>>>>>>>> Stats is contained by xstats, and future direction I think is
>> xstats.
>>>>>>>>> So I think we don't need support stats reset.
>>>>>>>>>
>>>>>>>>
>>>>>>>> I have similar question with Dongdong, readonly values are safe
>> for
>>>>>>>> telemetry, but modifying data can be more tricky since we don't
>> have
>>>>>>>> locking in ethdev APIs, this can cause concurrency issues.
>>>>>>>
>>>>>>> Yes, it indeed has concurrency issues.
>>>>>>>
>>>>>>>>
>>>>>>>> Overall do we want telemetry go that way and become something
>> that
>>>>>>>> alters ethdev data/config?
>>>>>>>
>>>>>>> There are at least two part of data: config and status.
>>>>>>> For stats (which belong status data) could help for debugging, I
>> think it's acceptable.
>>>>>>>
>>>>>>> As for concurrency issues. People should know what to do and when
>> to do, just like
>>>>>>> the don't invoke config API (e.g. dev_configure/dev_start/...)
>> concurrency.
>>>>>>>
>>>>>> While this is probably ok for now, I think in next release we
>> should look
>>>>>> to add some sort of support for locking for destructive ops in a
>> future
>>>>>> release. For example, we could:
>>>>>>
>>>>>> 1. Add support for marking a callback as "destructive" and only
>> allow it to
>>>>>> be called if only one connection is present or
>>>>>>
>>>>>> 2. Make it possible for callbacks to query the number of
>> connections so
>>>>>> that the callback itself is non-destructive in more than one
>> connection is
>>>>>> open.
>>>>>>
>>>>>> [Both of these will require locking support so that new
>> connections aren't
>>>>>> openned when the callback is in-flight!]
>>>>>
>>>>> Except telemetry, the application may have other console could
>> execute DPDK API.
>>>>> So I think trying to keep it simple, it's up to the user to invoke.
>>>>
>>>> No, the user should not be responsible for concurrency issues.
>>>> We can ask the app developper to take care,
>>>> but not to the user who has no control on what happens in the app.
>>>>
>>>> On a more general note, I feel the expansion of telemetry is not
>> controlled enough.
>>>> I would like to stop on adding more telemetry until we have a clear
>> guideline
>>>> about what is telemetry for and how to use it.
>>>
>>> Hi Thomas,
>>>
>>> Should this be discussed on TB?
>>
>> What would be your question exactly?
> 
> A general comment about telemetry:
> 
> If an application exposes telemetry through an end user facing API, e.g. http(s) REST, it would be nice if non-read-only telemetry paths are easy to identify by following some DPDK standard convention, so the application does not need to manually maintain an allow-list of read-only paths.

+1 for this point.

> 
> Bruce's documentation about trace/log/telemetry/dump might also need to be updated regarding non-read-only telemetry actions.

I just check Bruce's patch [1], and notice that the telemetry callback must be 'read-only': (Telemetry callbacks should not modify any program state, but be "read-only").

From internal product usage, we think xstats-reset is valid to identify problem, but this callback is not read-only.

We think telemetry callback should not limit to 'read-only'. Perhaps we could develop some strategy to better manage non-read-only callbacks (just like Morten's advise).

[1]: https://patchwork.dpdk.org/project/dpdk/patch/20230620170728.74117-3-bruce.richardson@intel.com/

> 
> 
> .
> 

^ permalink raw reply	[flat|nested] 62+ messages in thread

* Re: [PATCH v5 2/2] ethdev: support xstats reset telemetry command
  2023-07-04  6:41                         ` fengchengwen
@ 2024-10-03 18:51                           ` Stephen Hemminger
  0 siblings, 0 replies; 62+ messages in thread
From: Stephen Hemminger @ 2024-10-03 18:51 UTC (permalink / raw)
  To: fengchengwen
  Cc: Morten Brørup, Thomas Monjalon, Dongdong Liu,
	Bruce Richardson, dev, Ferruh Yigit, andrew.rybchenko,
	ciara.power, kevin.laatz, david.marchand

On Tue, 4 Jul 2023 14:41:27 +0800
fengchengwen <fengchengwen@huawei.com> wrote:

> > A general comment about telemetry:
> > 
> > If an application exposes telemetry through an end user facing API, e.g. http(s) REST, it would be nice if non-read-only telemetry paths are easy to identify by following some DPDK standard convention, so the application does not need to manually maintain an allow-list of read-only paths.  
> 
> +1 for this point.
> 
> > 
> > Bruce's documentation about trace/log/telemetry/dump might also need to be updated regarding non-read-only telemetry actions.  
> 
> I just check Bruce's patch [1], and notice that the telemetry callback must be 'read-only': (Telemetry callbacks should not modify any program state, but be "read-only").
> 
> From internal product usage, we think xstats-reset is valid to identify problem, but this callback is not read-only.
> 
> We think telemetry callback should not limit to 'read-only'. Perhaps we could develop some strategy to better manage non-read-only callbacks (just like Morten's advise).
> 
> [1]: https://patchwork.dpdk.org/project/dpdk/patch/20230620170728.74117-3-bruce.richardson@intel.com/

This patch set had functional test failures, and there never was a good conclusion about allowing telemetry to do reset.

If you want to keep going with it, go through the feedback and resubmit.

^ permalink raw reply	[flat|nested] 62+ messages in thread

end of thread, other threads:[~2024-10-03 18:52 UTC | newest]

Thread overview: 62+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-19  9:07 [PATCH v5 0/5] support dmadev/ethdev stats reset Chengwen Feng
2022-12-19  9:07 ` [PATCH v5 1/5] dmadev: support stats reset telemetry command Chengwen Feng
2022-12-19  9:07 ` [PATCH v5 2/5] telemetry: fix repeated display when callback don't set dict Chengwen Feng
2022-12-19  9:33   ` Bruce Richardson
2022-12-26  4:53     ` fengchengwen
2023-01-06 16:07       ` Bruce Richardson
2023-01-06 17:33         ` Bruce Richardson
2023-01-11 12:38           ` fengchengwen
2022-12-19  9:07 ` [PATCH v5 3/5] ethdev: support xstats reset telemetry command Chengwen Feng
2022-12-19  9:07 ` [PATCH v5 4/5] ethdev: telemetry xstats support hide zero Chengwen Feng
2022-12-19  9:07 ` [PATCH v5 5/5] ethdev: add newline to telemetry log string Chengwen Feng
2022-12-26  4:55 ` [PATCH v5 0/5] support dmadev/ethdev stats reset fengchengwen
2023-01-11 12:02 ` [PATCH v2 " Chengwen Feng
2023-01-11 12:02   ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
2023-01-11 12:02   ` [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict Chengwen Feng
2023-01-11 12:02   ` [PATCH v2 3/5] ethdev: add newline to telemetry log string Chengwen Feng
2023-01-11 12:02   ` [PATCH v2 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
2023-01-11 12:02   ` [PATCH v2 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
2023-01-11 12:06 ` [PATCH v2 0/5] support dmadev/ethdev stats reset Chengwen Feng
2023-01-11 12:06   ` [PATCH v2 1/5] dmadev: support stats reset telemetry command Chengwen Feng
2023-01-11 13:58     ` Bruce Richardson
2023-01-11 12:06   ` [PATCH v2 2/5] telemetry: fix repeat display when callback don't set dict Chengwen Feng
2023-01-11 14:01     ` Bruce Richardson
2023-01-11 12:06   ` [PATCH v2 3/5] ethdev: add newline to telemetry log string Chengwen Feng
2023-01-11 12:06   ` [PATCH v2 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
2023-01-11 12:06   ` [PATCH v2 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
2023-01-11 14:08     ` Bruce Richardson
2023-01-20  3:51       ` fengchengwen
2023-01-11 12:36   ` [PATCH v2 0/5] support dmadev/ethdev stats reset fengchengwen
2023-01-20  3:25 ` [PATCH v3 " Chengwen Feng
2023-01-20  3:25   ` [PATCH v3 1/5] dmadev: support stats reset telemetry command Chengwen Feng
2023-01-20  3:25   ` [PATCH v3 2/5] telemetry: fix repeat display when callback don't init dict Chengwen Feng
2023-01-20  3:25   ` [PATCH v3 3/5] ethdev: add newline to telemetry log string Chengwen Feng
2023-01-20  3:25   ` [PATCH v3 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
2023-01-20  3:25   ` [PATCH v3 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
2023-01-20  3:34 ` [PATCH v4 0/5] support dmadev/ethdev stats reset Chengwen Feng
2023-01-20  3:34   ` [PATCH v4 1/5] dmadev: support stats reset telemetry command Chengwen Feng
2023-01-20  3:34   ` [PATCH v4 2/5] telemetry: fix repeat display when callback don't init dict Chengwen Feng
2023-02-08 14:15     ` Bruce Richardson
2023-02-09  1:33       ` fengchengwen
2023-01-20  3:34   ` [PATCH v4 3/5] ethdev: add newline to telemetry log string Chengwen Feng
2023-01-20  3:34   ` [PATCH v4 4/5] ethdev: support xstats reset telemetry command Chengwen Feng
2023-01-20  3:34   ` [PATCH v4 5/5] ethdev: telemetry xstats support hide zero Chengwen Feng
2023-02-06  8:39   ` [PATCH v4 0/5] support dmadev/ethdev stats reset Thomas Monjalon
2023-02-08 14:17   ` Bruce Richardson
2023-02-09  1:45     ` fengchengwen
2023-02-09  2:32 ` [PATCH v5 0/2] " Chengwen Feng
2023-02-09  2:32   ` [PATCH v5 1/2] dmadev: support stats reset telemetry command Chengwen Feng
2023-02-09  2:32   ` [PATCH v5 2/2] ethdev: support xstats " Chengwen Feng
2023-02-15  3:19     ` Dongdong Liu
2023-02-16 11:53       ` fengchengwen
2023-02-16 12:06         ` Ferruh Yigit
2023-02-16 12:42           ` fengchengwen
2023-02-16 12:54             ` Bruce Richardson
2023-02-16 12:55               ` Bruce Richardson
2023-02-17  9:44               ` fengchengwen
2023-02-20 13:05                 ` Thomas Monjalon
2023-07-03  3:58                   ` fengchengwen
2023-07-03  7:20                     ` Thomas Monjalon
2023-07-03 13:44                       ` Morten Brørup
2023-07-04  6:41                         ` fengchengwen
2024-10-03 18:51                           ` Stephen Hemminger

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).