Re: [PATCH v16] app/procinfo: display eventdev xstats

[Stephen Hemminger <stephen@networkplumber.org>]

On Thu, 25 May 2023 11:41:27 -0500
Abdullah Sevincer <abdullah.sevincer@intel.com> wrote:

> 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:0
> 
> 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>

I was thinking of helper function and using const strings.
Don't need to panic on invalid eventdev.

Also the flags could be just bitfield to save some space.

Something like this. Untested since I have no hardware with
eventdevs.

diff --git a/app/proc-info/main.c b/app/proc-info/main.c
index f53fab2e3346..d330afd6815e 100644
--- a/app/proc-info/main.c
+++ b/app/proc-info/main.c
@@ -132,11 +132,11 @@ struct eventdev_params {
 	uint16_t queues[MAX_PORTS_QUEUES];
 	uint16_t num_queues;
 	uint16_t num_ports;
-	uint32_t shw_all_queues;
-	uint32_t shw_all_ports;
-	uint32_t dump_xstats;
-	uint32_t reset_xstats;
-	uint32_t shw_device_xstats;
+	uint8_t shw_all_queues:1,
+		shw_all_ports:1,
+		dump_xstats:1,
+		reset_xstats:1,
+		shw_device_xstats:1;
 };
 
 static struct eventdev_params eventdev_var[RTE_EVENT_MAX_DEVS];
@@ -261,71 +261,57 @@ parse_descriptor_param(char *list, struct desc_param *desc)
 	return 0;
 }
 
-static int
-parse_eventdev_dump_xstats_params(char *list)
+static uint16_t parse_eventdev(const char *str)
 {
 	unsigned long evdev_id;
 	char *endp;
 
-	evdev_id = strtoul(list, &endp, 0);
-
-	if (*list == '\0' || *endp != '\0' || evdev_id >= RTE_EVENT_MAX_DEVS) {
-		fprintf(stderr, "Invalid eventdev id: %s\n", list);
-		return -EINVAL;
+	evdev_id = strtoul(str, &endp, 0);
+	if (*str == '\0' || *endp != '\0' || evdev_id >= rte_event_dev_count()) {
+		fprintf(stderr, "Invalid eventdev id: %s\n", str);
+		return -1;
 	}
+	return evdev_id;
+}
 
-	eventdev_var[evdev_id].dump_xstats = 1;
+static int
+parse_eventdev_dump_xstats_params(const char *list)
+{
+	int evdev_id = parse_eventdev(list);
 
-	if (evdev_id >= rte_event_dev_count())
-		rte_panic("Invalid eventdev id: %s\n", list);
+	if (evdev_id < 0)
+		return -EINVAL;
 
+	eventdev_var[evdev_id].dump_xstats = 1;
 	return 0;
 }
 
 static int
-parse_eventdev_reset_xstats_params(char *list)
+parse_eventdev_reset_xstats_params(const char *list)
 {
-	unsigned long evdev_id;
-	char *endp;
-
-	evdev_id = strtoul(list, &endp, 0);
+	int evdev_id = parse_eventdev(list);
 
-	if (*list == '\0' || *endp != '\0' || evdev_id >= RTE_EVENT_MAX_DEVS) {
-		fprintf(stderr, "Invalid eventdev id: %s\n", list);
+	if (evdev_id < 0)
 		return -EINVAL;
-	}
 
 	eventdev_var[evdev_id].reset_xstats = 1;
-
-	if (evdev_id >= rte_event_dev_count())
-		rte_panic("Invalid eventdev id: %s\n", list);
-
 	return 0;
 }
 
 static int
-parse_eventdev_device_xstats_params(char *list)
+parse_eventdev_device_xstats_params(const char *list)
 {
-	unsigned long evdev_id;
-	char *endp;
-
-	evdev_id = strtoul(list, &endp, 0);
+	int evdev_id = parse_eventdev(list);
 
-	if (*list == '\0' || *endp != '\0' || evdev_id >= RTE_EVENT_MAX_DEVS) {
-		fprintf(stderr, "Invalid eventdev id: %s\n", list);
+	if (evdev_id < 0)
 		return -EINVAL;
-	}
 
 	eventdev_var[evdev_id].shw_device_xstats = 1;
-
-	if (evdev_id >= rte_event_dev_count())
-		rte_panic("Invalid eventdev id: %s\n", list);
-
 	return 0;
 }
 
 static int
-parse_eventdev_queue_xstats_params(char *list)
+parse_eventdev_queue_xstats_params(const char *list)
 {
 	uint16_t queue_id;
 	uint16_t evdev_id;
@@ -353,7 +339,7 @@ parse_eventdev_queue_xstats_params(char *list)
 		return -EINVAL;
 	}
 
-	if (evdev_id >= rte_event_dev_count())
+
 		rte_panic("invalid event device %hu\n", evdev_id);
 
 	return 0;