[v8] app/procinfo: display eventdev xstats

Message ID 20230309185143.1006949-1-abdullah.sevincer@intel.com (mailing list archive)
State Changes Requested, archived
Delegated to: Jerin Jacob
Headers
Series [v8] app/procinfo: display eventdev xstats |

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/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/intel-Functional success Functional 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/iol-aarch64-unit-testing success Testing PASS
ci/iol-x86_64-compile-testing success Testing PASS
ci/iol-aarch64-compile-testing fail Testing issues
ci/iol-testing fail Testing issues
ci/iol-x86_64-unit-testing success Testing PASS
ci/iol-abi-testing success Testing PASS

Commit Message

Abdullah Sevincer March 9, 2023, 6:51 p.m. UTC
  This commit extends proc-info application to
display xstats for the eventdev devices.

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

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

If any xstats parameters for eventdev passed through
proc-info command line, proc-info will only display
requested eventdev data and exit.

Users should not pass any eventdev xstats parameters
if they desire to dump other proc-info data such 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           | 210 ++++++++++++++++++++++++++++++++-
 app/proc-info/meson.build      |   2 +-
 doc/guides/tools/proc_info.rst |  27 ++++-
 3 files changed, 236 insertions(+), 3 deletions(-)
  

Comments

Pattan, Reshma March 15, 2023, 11:56 a.m. UTC | #1
> -----Original Message-----
>  app/proc-info/main.c           | 210 ++++++++++++++++++++++++++++++++-

> +		{"edev-reset-xstats", 0, NULL, 0},
> +		{"show-edev-device-xstats", 0, NULL, 0},

We should support the eventdev id as argument to this command  and display stats for the given eventdev id.

> +	if (show_edev_xstats()) {
> +		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");

Why do we need to restrict to only eventdev 0? .I guess you should support displaying stats for any eventdev, not only the eventdev with index0.
 

> diff --git a/doc/guides/tools/proc_info.rst b/doc/guides/tools/proc_info.rst

> +   --show-edev-port-xstats=port_num | --edev-dump-xstats | --edev-reset-
> xstats |
> +   --show-edev-device-xstats]
> 

For "--show-evdev-device-xstats" why don't we have eventdev id as argument ?
There can be more than one eventdev can exists right on the system?


> +**--show-edev-queue-xstats**
> +The show-edev-queue-xstats parameter enables stats for specified queue or
> all queues.

Here and in below text replace "stats" to "xstats", because  stats and xstats are different.
  
Pattan, Reshma March 15, 2023, 2:24 p.m. UTC | #2
> -----Original Message-----
> From: Sevincer, Abdullah <abdullah.sevincer@intel.com>


> +static void
> +get_eventdev_xstats(uint8_t dev_id,


You can separate this function (basically the code inside this function) into 3 functions, one for reset and one for display stats values.
And move the common code(stats storage calculation logic and getting stats name  logic) to a another function, so you can call that function from reset and display functions.


> +
> +		if (enable_dump_eventdev_xstats) {
> +			ret = rte_event_dev_dump(evdev_id, stdout);
> +			if (ret)
> +				rte_panic("dump failed with err=%d\n", ret);
> +		}
> +
> +		process_eventdev_xstats(false);
> +
> +		if (enable_eventdev_reset_xstats)
> +			process_eventdev_xstats(true);

For easy code readability, I would say have a sperate function for reset stats, do not mix display and resets by just passing reset bool value. 

Thanks,
Reshma
  
Abdullah Sevincer March 15, 2023, 7:40 p.m. UTC | #3
From: Pattan, Reshma <reshma.pattan@intel.com> 
Sent: Wednesday, March 15, 2023 4:57 AM
To: Sevincer, Abdullah <abdullah.sevincer@intel.com>; dev@dpdk.org
Cc: jerinj@marvell.com
Subject: RE: [PATCH v8] app/procinfo: display eventdev xstats


->Why do we need to restrict to only eventdev 0? .I guess you should support displaying stats for any eventdev, not only the eventdev with index0.
The intent is only display one eventdev in use (id being zero), displaying multiple eventdevs was not in the scope.
  
Abdullah Sevincer March 18, 2023, 6:49 p.m. UTC | #4
> +static void
> +get_eventdev_xstats(uint8_t dev_id,


>+You can separate this function (basically the code inside this function) into 3 functions, one for reset and one for display stats values.
>+And move the common code(stats storage calculation logic and getting stats name  logic) to a another function, so you can call that function from reset and display functions.
    This can be done, but we will have 2 API calls for each now, if we have this flag we will be able to reset withing one API call. 


> +
> +		if (enable_dump_eventdev_xstats) {
> +			ret = rte_event_dev_dump(evdev_id, stdout);
> +			if (ret)
> +				rte_panic("dump failed with err=%d\n", ret);
> +		}
> +
> +		process_eventdev_xstats(false);
> +
> +		if (enable_eventdev_reset_xstats)
> +			process_eventdev_xstats(true);

>+For easy code readability, I would say have a sperate function for reset stats, do not mix display and resets by just passing reset bool value. 
    Same for this one as above, one API call to the xstats instead of two separate calls if we use this flag. 

Thanks,
Reshma
  

Patch

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index 53e852a07c..6ead43a439 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,18 @@  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_shw_all_eventdev_queues;
+static uint32_t enable_shw_all_eventdev_ports;
+static uint32_t enable_dump_eventdev_xstats;
+static uint32_t enable_eventdev_reset_xstats;
+static uint32_t enable_shw_eventdev_device_xstats;
+
+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 +190,12 @@  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"
+		"  --show-edev-queue-xstats=queue_num or * to get queue xstats for specified queue or all queues;\n"
+		"  --show-edev-port-xstats=port_num or * to get queue xstats for specified port or all ports;\n"
+		"  --edev-dump-xstats to dump all event_dev xstats;\n"
+		"  --edev-reset-xstats to reset event_dev stats after reading;\n"
+		"  --show-edev-device-xstats to get event_dev device stats;\n",
 		prgname);
 }
 
@@ -302,6 +325,11 @@  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},
+		{"show-edev-queue-xstats", required_argument, NULL, 0},
+		{"show-edev-port-xstats", required_argument, NULL, 0},
+		{"edev-dump-xstats", 0, NULL, 0},
+		{"edev-reset-xstats", 0, NULL, 0},
+		{"show-edev-device-xstats", 0, NULL, 0},
 		{NULL, 0, 0, 0}
 	};
 
@@ -385,6 +413,32 @@  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-dump-xstats", MAX_LONG_OPT_SZ))
+				enable_dump_eventdev_xstats = 1;
+			else if (!strncmp(long_option[option_index].name,
+					"edev-reset-xstats", MAX_LONG_OPT_SZ))
+				enable_eventdev_reset_xstats = 1;
+			else if (!strncmp(long_option[option_index].name,
+					"show-edev-device-xstats", MAX_LONG_OPT_SZ))
+				enable_shw_eventdev_device_xstats = 1;
+			else if (!strncmp(long_option[option_index].name,
+					"show-edev-queue-xstats", MAX_LONG_OPT_SZ)) {
+				if (strcmp("*", optarg) == 0) {
+					enable_shw_all_eventdev_queues = 1;
+				} else {
+					queues[num_queues] = atoi(optarg);
+					num_queues++;
+				}
+			} else if (!strncmp(long_option[option_index].name,
+					"show-edev-port-xstats", MAX_LONG_OPT_SZ)) {
+				if (strcmp("*", optarg) == 0) {
+					enable_shw_all_eventdev_ports = 1;
+				} else {
+					ports[num_ports] = atoi(optarg);
+					num_ports++;
+				}
+			}
 			break;
 		case 1:
 			/* Print xstat single value given by name*/
@@ -1744,6 +1798,136 @@  nic_tx_descriptor_display(uint16_t port_id, struct desc_param *desc)
 			strerror(-ret));
 }
 
+static bool
+show_edev_xstats(void)
+{
+	/* Check if any event dev xstats requested from command line */
+	if (enable_shw_all_eventdev_queues || enable_shw_all_eventdev_ports
+		|| enable_dump_eventdev_xstats || enable_eventdev_reset_xstats ||
+		enable_shw_eventdev_device_xstats || num_ports > 0 || num_queues > 0)
+		return true;
+
+	return false;
+}
+
+static void
+get_eventdev_xstats(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_xstats(bool reset)
+{
+	int i;
+
+	if (enable_shw_eventdev_device_xstats) {
+		get_eventdev_xstats(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_xstats(evdev_id,
+				  RTE_EVENT_DEV_XSTATS_PORT,
+				  i,
+				  reset);
+		}
+	} else {
+		for (i = 0; i < num_ports; i++) {
+			get_eventdev_xstats(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_xstats(evdev_id,
+				  RTE_EVENT_DEV_XSTATS_QUEUE,
+				  i,
+				  reset);
+		}
+	} else {
+		for (i = 0; i < num_queues; i++) {
+			get_eventdev_xstats(evdev_id,
+				  RTE_EVENT_DEV_XSTATS_QUEUE,
+				  queues[i],
+				  reset);
+		}
+	}
+}
+
 int
 main(int argc, char **argv)
 {
@@ -1794,6 +1978,30 @@  main(int argc, char **argv)
 		return 0;
 	}
 
+	if (show_edev_xstats()) {
+		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_xstats) {
+			ret = rte_event_dev_dump(evdev_id, stdout);
+			if (ret)
+				rte_panic("dump failed with err=%d\n", ret);
+		}
+
+		process_eventdev_xstats(false);
+
+		if (enable_eventdev_reset_xstats)
+			process_eventdev_xstats(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..ad6d2e8aeb 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 | --show-edev-queue-xstats=queue_num |
+   --show-edev-port-xstats=port_num | --edev-dump-xstats | --edev-reset-xstats |
+   --show-edev-device-xstats]
 
 Parameters
 ~~~~~~~~~~
@@ -101,6 +103,29 @@  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.
 
+**--show-edev-queue-xstats**
+The show-edev-queue-xstats parameter enables stats for specified queue or all queues.
+queue_num: The queue number to get queue stats for this specified queue or * for all queues.
+
+**--show-edev-port-xstats**
+The show-edev-port-xstats parameter enables stats for specified port or all ports.
+queue_num: The port number to get port stats for this specified port or * for all ports.
+
+**--edev-dump-xstats**
+The edev-dump-xstats parameter dumps all eventdev stats.
+
+**--edev-reset-xstats**
+The edev-reset-xstats parameter resets eventdev stats after reading.
+
+**--show-edev-device-xstats**
+The show-edev-device-xstats parameter displays eventdev device stats.
+
+A typical command line usage for eventdev stats:
+
+    .. code-block:: console
+
+       ./dpdk-proc-info -- --show-edev-port-xstats=1
+
 Limitations
 -----------