[v3] app/procinfo: display eventdev xstats for PMD data

Message ID 20230206230505.3029148-1-abdullah.sevincer@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [v3] app/procinfo: display eventdev xstats for PMD data |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/loongarch-compilation success Compilation OK
ci/loongarch-unit-testing success Unit Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/github-robot: build fail github build: failed
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-testing success Testing PASS
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Abdullah Sevincer Feb. 6, 2023, 11:05 p.m. UTC
  This commit extends proc-info application to
display xstats and PMD dump data for the eventdev
devices.

New command line arguments are introduced to
display stats for eventdev devices. The command
example is like:

For displaying a specific port stats (e.g. port 1):
dpdk-proc-info -- --edev-stats-enable --edev-port=1

edev-stats-enable: This parameters enables proc-info
to display xstats for eventdev devices. If the parameter
is enabled through proc-info command line, proc-info
will only dump event_dev data and exit.
Users should not enable this flag if they desire to
dump other proc-info data suc as Rx/Tx descriptor dump.
More information can be found in proc-info app doc.

Signed-off-by: Abdullah Sevincer <abdullah.sevincer@intel.com>
---
 app/proc-info/main.c           | 208 ++++++++++++++++++++++++++++++++-
 app/proc-info/meson.build      |   2 +-
 doc/guides/tools/proc_info.rst |  42 ++++++-
 3 files changed, 249 insertions(+), 3 deletions(-)
  

Comments

Stephen Hemminger Feb. 7, 2023, 12:22 a.m. UTC | #1
On Mon,  6 Feb 2023 17:05:05 -0600
Abdullah Sevincer <abdullah.sevincer@intel.com> wrote:

> +
> +	size = (unsigned int)ret; /* number of names */
> +
> +	/* Get memory to hold stat names, IDs, and values */
> +
> +	xstats_names = malloc(sizeof(struct rte_event_dev_xstats_name) * size);
> +	ids = malloc(sizeof(unsigned int) * size);
> +
> +
> +	if (!xstats_names || !ids)
> +		rte_panic("unable to alloc memory for stats retrieval\n");

You might want to use calloc() here.

Seems like lots of extra blank lines.
  
Abdullah Sevincer Feb. 12, 2023, 7:43 p.m. UTC | #2
Thanks Stephen,
I will remove extra line there.

Instead malloc using of calloc is required or just suggestion? 
I can see allocation is done in same way with malloc in lib\eventdev\rte_eventdev.c (reference to eventdev_build_telemetry_data function).
I will keep malloc as it is if there is no opposition.

Thanks,
Abdullah.

-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org> 
Sent: Monday, February 6, 2023 4:22 PM
To: Sevincer, Abdullah <abdullah.sevincer@intel.com>
Cc: dev@dpdk.org; jerinj@marvell.com
Subject: Re: [PATCH v3] app/procinfo: display eventdev xstats for PMD data

On Mon,  6 Feb 2023 17:05:05 -0600
Abdullah Sevincer <abdullah.sevincer@intel.com> wrote:

> +
> +	size = (unsigned int)ret; /* number of names */
> +
> +	/* Get memory to hold stat names, IDs, and values */
> +
> +	xstats_names = malloc(sizeof(struct rte_event_dev_xstats_name) * size);
> +	ids = malloc(sizeof(unsigned int) * size);
> +
> +
> +	if (!xstats_names || !ids)
> +		rte_panic("unable to alloc memory for stats retrieval\n");

You might want to use calloc() here.

Seems like lots of extra blank lines.
  
Abdullah Sevincer Feb. 17, 2023, 3:58 p.m. UTC | #3
Hi folks,

Any chance to look at this commit? Any other feedback before I push V6? (will remove extra line there below in the commit)


-----Original Message-----
From: Sevincer, Abdullah <abdullah.sevincer@intel.com> 
Sent: Sunday, February 12, 2023 11:44 AM
To: Stephen Hemminger <stephen@networkplumber.org>
Cc: dev@dpdk.org; jerinj@marvell.com
Subject: RE: [PATCH v3] app/procinfo: display eventdev xstats for PMD data

Thanks Stephen,
I will remove extra line there.

Instead malloc using of calloc is required or just suggestion? 
I can see allocation is done in same way with malloc in lib\eventdev\rte_eventdev.c (reference to eventdev_build_telemetry_data function).
I will keep malloc as it is if there is no opposition.

Thanks,
Abdullah.

-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org> 
Sent: Monday, February 6, 2023 4:22 PM
To: Sevincer, Abdullah <abdullah.sevincer@intel.com>
Cc: dev@dpdk.org; jerinj@marvell.com
Subject: Re: [PATCH v3] app/procinfo: display eventdev xstats for PMD data

On Mon,  6 Feb 2023 17:05:05 -0600
Abdullah Sevincer <abdullah.sevincer@intel.com> wrote:

> +
> +	size = (unsigned int)ret; /* number of names */
> +
> +	/* Get memory to hold stat names, IDs, and values */
> +
> +	xstats_names = malloc(sizeof(struct rte_event_dev_xstats_name) * size);
> +	ids = malloc(sizeof(unsigned int) * size);
> +
> +
> +	if (!xstats_names || !ids)
> +		rte_panic("unable to alloc memory for stats retrieval\n");

You might want to use calloc() here.

Seems like lots of extra blank lines.
  
Stephen Hemminger Feb. 17, 2023, 4:33 p.m. UTC | #4
On Fri, 17 Feb 2023 15:58:52 +0000
"Sevincer, Abdullah" <abdullah.sevincer@intel.com> wrote:

> From: Sevincer, Abdullah <abdullah.sevincer@intel.com> 
> Sent: Sunday, February 12, 2023 11:44 AM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: dev@dpdk.org; jerinj@marvell.com
> Subject: RE: [PATCH v3] app/procinfo: display eventdev xstats for PMD data
> 
> Thanks Stephen,
> I will remove extra line there.
> 
> Instead malloc using of calloc is required or just suggestion? 
> I can see allocation is done in same way with malloc in lib\eventdev\rte_eventdev.c (reference to eventdev_build_telemetry_data function).
> I will keep malloc as it is if there is no opposition.

It doesn't matter much if you use malloc vs calloc.
But there are some static analysis tools that might look at calloc as way to determine
number of elememts for later array checks.

Also, the kernel checkpatch warns when kmalloc is used but kcalloc or kmalloc_array
could be used instead. That is Linux kernel specific but same idea applies.
  
Abdullah Sevincer Feb. 22, 2023, 1:54 a.m. UTC | #5
Got it. Looking at forums and made some reading, like you said it does not matter much.
I will keep consistency with other call methods so keep malloc.

I will push another patch removing extra line.

Thanks.

-----Original Message-----
From: Stephen Hemminger <stephen@networkplumber.org> 
Sent: Friday, February 17, 2023 8:33 AM
To: Sevincer, Abdullah <abdullah.sevincer@intel.com>
Cc: dev@dpdk.org; jerinj@marvell.com
Subject: Re: [PATCH v3] app/procinfo: display eventdev xstats for PMD data

On Fri, 17 Feb 2023 15:58:52 +0000
"Sevincer, Abdullah" <abdullah.sevincer@intel.com> wrote:

> From: Sevincer, Abdullah <abdullah.sevincer@intel.com>
> Sent: Sunday, February 12, 2023 11:44 AM
> To: Stephen Hemminger <stephen@networkplumber.org>
> Cc: dev@dpdk.org; jerinj@marvell.com
> Subject: RE: [PATCH v3] app/procinfo: display eventdev xstats for PMD 
> data
> 
> Thanks Stephen,
> I will remove extra line there.
> 
> Instead malloc using of calloc is required or just suggestion? 
> I can see allocation is done in same way with malloc in lib\eventdev\rte_eventdev.c (reference to eventdev_build_telemetry_data function).
> I will keep malloc as it is if there is no opposition.

It doesn't matter much if you use malloc vs calloc.
But there are some static analysis tools that might look at calloc as way to determine number of elememts for later array checks.

Also, the kernel checkpatch warns when kmalloc is used but kcalloc or kmalloc_array could be used instead. That is Linux kernel specific but same idea applies.
  

Patch

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 53e852a07c..25b36205d0 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -40,11 +40,17 @@ 
 #include <rte_tm.h>
 #include <rte_hexdump.h>
 #include <rte_version.h>
+#include <rte_eventdev.h>
 
 /* Maximum long option length for option parsing. */
 #define MAX_LONG_OPT_SZ 64
 #define MAX_STRING_LEN 256
 
+/* Note: Port_queue_id in xstats APIs is 8 bits, so we have a maximum of
+ * 256 ports and queues for event_Dev
+ */
+#define MAX_PORTS_QUEUES 256
+
 #define ETHDEV_FWVERS_LEN 32
 #define RTE_RETA_CONF_GROUP_NUM 32
 #define DIV_ROUND_UP(n, d) (((n) + (d) - 1) / (d))
@@ -121,6 +127,19 @@  static uint32_t enable_shw_module_eeprom;
 static uint32_t enable_shw_rx_desc_dump;
 static uint32_t enable_shw_tx_desc_dump;
 
+static uint32_t enable_eventdev_stats;
+static uint32_t enable_shw_all_eventdev_queues;
+static uint32_t enable_shw_all_eventdev_ports;
+static uint32_t enable_dump_eventdev;
+static uint32_t enable_eventdev_reset_stats;
+static uint32_t enable_shw_eventdev_device_stats;
+
+static uint8_t evdev_id;
+static uint8_t num_ports;
+static uint8_t ports[MAX_PORTS_QUEUES];
+static uint8_t num_queues;
+static uint8_t queues[MAX_PORTS_QUEUES];
+
 #define DESC_PARAM_NUM 3
 
 struct desc_param {
@@ -172,7 +191,15 @@  proc_info_usage(const char *prgname)
 			"offset: The offset of the descriptor starting from tail. "
 			"num: The number of the descriptors to dump.\n"
 		"  --iter-mempool=name: iterate mempool elements to display content\n"
-		"  --dump-regs=file-prefix: dump registers to file with the file-prefix\n",
+		"  --dump-regs=file-prefix: dump registers to file with the file-prefix\n"
+		"  --edev-stats-enable to enable stats for all event_dev queues, ports, device etc;\n"
+		"  --all-edev-queues to get stats for all event_dev queues;\n"
+		"  --edev-queue=queue_num to get queue stats for specified queue;\n"
+		"  --all-edev-ports to get stats for all event_dev ports;\n"
+		"  --edev-port=port_num to get queue stats for specified port;\n"
+		"  --edev-dump to dump all event_dev stats;\n"
+		"  --edev-reset to reset event_dev stats after reading;\n"
+		"  --edev-device-stats to get event_dev device stats;\n",
 		prgname);
 }
 
@@ -302,6 +329,14 @@  proc_info_parse_args(int argc, char **argv)
 		{"show-module-eeprom", 0, NULL, 0},
 		{"show-rx-descriptor", required_argument, NULL, 1},
 		{"show-tx-descriptor", required_argument, NULL, 1},
+		{"edev-stats-enable", 0, NULL, 0},
+		{"all-edev-queues", 0, NULL, 0},
+		{"edev-queue", required_argument, NULL, 0},
+		{"all-edev-ports", 0, NULL, 0},
+		{"edev-port", required_argument, NULL, 0},
+		{"edev-dump", 0, NULL, 0},
+		{"edev-reset", 0, NULL, 0},
+		{"edev-device-stats", 0, NULL, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -385,6 +420,33 @@  proc_info_parse_args(int argc, char **argv)
 			else if (!strncmp(long_option[option_index].name,
 					"show-module-eeprom", MAX_LONG_OPT_SZ))
 				enable_shw_module_eeprom = 1;
+			else if (!strncmp(long_option[option_index].name,
+					"edev-stats-enable", MAX_LONG_OPT_SZ)) {
+				enable_eventdev_stats = 1;
+			} else if (!strncmp(long_option[option_index].name,
+					"all-edev-queues", MAX_LONG_OPT_SZ)) {
+				enable_shw_all_eventdev_queues = 1;
+			} else if (!strncmp(long_option[option_index].name,
+					"all-edev-ports", MAX_LONG_OPT_SZ)) {
+				enable_shw_all_eventdev_ports = 1;
+			} else if (!strncmp(long_option[option_index].name,
+					"edev-dump", MAX_LONG_OPT_SZ)) {
+				enable_dump_eventdev = 1;
+			} else if (!strncmp(long_option[option_index].name,
+					"edev-reset", MAX_LONG_OPT_SZ)) {
+				enable_eventdev_reset_stats = 1;
+			} else if (!strncmp(long_option[option_index].name,
+					"edev-device-stats", MAX_LONG_OPT_SZ)) {
+				enable_shw_eventdev_device_stats = 1;
+			} else if (!strncmp(long_option[option_index].name,
+					"edev-queue", MAX_LONG_OPT_SZ)) {
+				queues[num_queues] = atoi(optarg);
+				num_queues++;
+			} else if (!strncmp(long_option[option_index].name,
+					"edev-port", MAX_LONG_OPT_SZ)) {
+				ports[num_ports] = atoi(optarg);
+				num_ports++;
+			}
 			break;
 		case 1:
 			/* Print xstat single value given by name*/
@@ -1744,6 +1806,126 @@  nic_tx_descriptor_display(uint16_t port_id, struct desc_param *desc)
 			strerror(-ret));
 }
 
+static void
+get_eventdev_stats(uint8_t dev_id,
+	  enum rte_event_dev_xstats_mode mode,
+	  uint8_t queue_port_id,
+	  bool reset)
+{
+	int ret;
+	struct rte_event_dev_xstats_name *xstats_names;
+	uint64_t *ids;
+	unsigned int size;
+	int i;
+
+
+	/* Get amount of storage required */
+	ret = rte_event_dev_xstats_names_get(dev_id,
+					     mode,
+					     queue_port_id,
+					     NULL, /* names */
+					     NULL, /* ids */
+					     0);   /* num */
+
+	if (ret < 0)
+		rte_panic("rte_event_dev_xstats_names_get err %d\n", ret);
+
+	if (ret == 0) {
+		printf(
+		"No stats available for this item, mode=%d, queue_port_id=%d\n",
+			mode, queue_port_id);
+		return;
+	}
+
+	size = (unsigned int)ret; /* number of names */
+
+	/* Get memory to hold stat names, IDs, and values */
+
+	xstats_names = malloc(sizeof(struct rte_event_dev_xstats_name) * size);
+	ids = malloc(sizeof(unsigned int) * size);
+
+
+	if (!xstats_names || !ids)
+		rte_panic("unable to alloc memory for stats retrieval\n");
+
+	ret = rte_event_dev_xstats_names_get(dev_id, mode, queue_port_id,
+					     xstats_names, ids,
+					     size);
+	if (ret != (int)size)
+		rte_panic("rte_event_dev_xstats_names_get err %d\n", ret);
+
+	if (!reset) {
+		uint64_t *values;
+
+		values = malloc(sizeof(uint64_t) * size);
+		if (!values)
+			rte_panic("unable to alloc memory for stats retrieval\n");
+
+		ret = rte_event_dev_xstats_get(dev_id, mode, queue_port_id,
+					       ids, values, size);
+
+		if (ret != (int)size)
+			rte_panic("rte_event_dev_xstats_get err %d\n", ret);
+
+		for (i = 0; i < (int)size; i++) {
+			printf("id %"PRIu64"  %s = %"PRIu64"\n",
+				ids[i], &xstats_names[i].name[0], values[i]);
+		}
+
+		free(values);
+	} else
+		rte_event_dev_xstats_reset(dev_id, mode, queue_port_id,
+					   ids, size);
+
+	free(xstats_names);
+	free(ids);
+}
+
+static void
+process_eventdev_stats(bool reset)
+{
+	int i;
+
+	if (enable_shw_eventdev_device_stats) {
+		get_eventdev_stats(evdev_id,
+			  RTE_EVENT_DEV_XSTATS_DEVICE,
+			  0,
+			  reset);
+	}
+
+	if (enable_shw_all_eventdev_ports) {
+		for (i = 0; i < MAX_PORTS_QUEUES; i++) {
+			get_eventdev_stats(evdev_id,
+				  RTE_EVENT_DEV_XSTATS_PORT,
+				  i,
+				  reset);
+		}
+	} else {
+		for (i = 0; i < num_ports; i++) {
+			get_eventdev_stats(evdev_id,
+				  RTE_EVENT_DEV_XSTATS_PORT,
+				  ports[i],
+				  reset);
+		}
+	}
+
+	if (enable_shw_all_eventdev_queues) {
+		for (i = 0; i < MAX_PORTS_QUEUES; i++) {
+			get_eventdev_stats(evdev_id,
+				  RTE_EVENT_DEV_XSTATS_QUEUE,
+				  i,
+				  reset);
+		}
+	} else {
+		for (i = 0; i < num_queues; i++) {
+			get_eventdev_stats(evdev_id,
+				  RTE_EVENT_DEV_XSTATS_QUEUE,
+				  queues[i],
+				  reset);
+		}
+	}
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1794,6 +1976,30 @@  main(int argc, char **argv)
 		return 0;
 	}
 
+	if (enable_eventdev_stats) {
+		const uint8_t ndevs = rte_event_dev_count();
+
+		if (ndevs == 0)
+			rte_panic("No event devs found. Do you need"
+			  " to pass in a --vdev flag?\n");
+
+		if (ndevs > 1)
+			printf("Warning: More than one event dev, but using idx 0\n");
+
+		if (enable_dump_eventdev) {
+			ret = rte_event_dev_dump(evdev_id, stdout);
+			if (ret)
+				rte_panic("dump failed with err=%d\n", ret);
+		}
+
+		process_eventdev_stats(false);
+
+		if (enable_eventdev_reset_stats)
+			process_eventdev_stats(true);
+
+		return 0;
+	}
+
 	nb_ports = rte_eth_dev_count_avail();
 	if (nb_ports == 0)
 		rte_exit(EXIT_FAILURE, "No Ethernet ports - bye\n");
diff --git a/app/proc-info/meson.build b/app/proc-info/meson.build
index 1563ce656a..4f83f29a64 100644
--- a/app/proc-info/meson.build
+++ b/app/proc-info/meson.build
@@ -8,7 +8,7 @@  if is_windows
 endif
 
 sources = files('main.c')
-deps += ['ethdev', 'security']
+deps += ['ethdev', 'security', 'eventdev']
 if dpdk_conf.has('RTE_LIB_METRICS')
     deps += 'metrics'
 endif
diff --git a/doc/guides/tools/proc_info.rst b/doc/guides/tools/proc_info.rst
index cf3502a8cb..59d8ff2490 100644
--- a/doc/guides/tools/proc_info.rst
+++ b/doc/guides/tools/proc_info.rst
@@ -22,7 +22,9 @@  The application has a number of command line options:
    --show-ring[=name] | --show-mempool[=name] | --iter-mempool=name |
    --show-port-private | --version | --firmware-version | --show-rss-reta |
    --show-module-eeprom | --show-rx-descriptor queue_id:offset:num |
-   --show-tx-descriptor queue_id:offset:num ]
+   --show-tx-descriptor queue_id:offset:num | --edev-stats-enable |
+   --all-edev-queues | --edev-queue=queue_num | --all-edev-ports |
+   --edev-port=port_num | --edev-dump | --edev-reset | --edev-device-stats]
 
 Parameters
 ~~~~~~~~~~
@@ -101,6 +103,44 @@  queue_id: A Tx queue identifier on this port.
 offset: The offset of the descriptor starting from tail.
 num: The number of the descriptors to dump.
 
+**--edev-stats-enable**
+The edev-stats-enable parameter enables proc-info application
+to display stats for eventdev devices. If the parameter is entered
+through proc-info application command line, proc-info application will
+only dump eventdev data and exit from the application. Hence,
+this parameter is required and a must  with other eventdev parameters
+explained below. Users should not enable this flag if they desire to dump
+other proc-info application stats such as Rx/Tx descriptor dump.
+
+**--all-edev-queues**
+The all-edev-queues parameter enables stats for all eventdev queues.
+
+**--edev-queue**
+The edev-queue parameter enables stats for specified queue.
+queue_num: The queue number to get queue stats for this specified queue.
+
+**--all-edev-ports**
+The all-edev-ports parameter enables stats for all eventdev ports.
+
+**--edev-port**
+The edev-port parameter enables stats for specified port.
+queue_num: The port number to get port stats for this specified port.
+
+**--edev-dump**
+The edev-dump parameter dumps all eventdev stats.
+
+**--edev-reset**
+The edev-reset parameter resets eventdev stats after reading.
+
+**--edev-device-stats**
+The edev-device-stats parameter displays eventdev device stats.
+
+A typical command line usage for eventdev stats:
+
+    .. code-block:: console
+
+       ./dpdk-proc-info -- --edev-stats-enable --edev-port=1
+
 Limitations
 -----------