RE: [PATCH v12] app/procinfo: display eventdev xstats

["Pattan, Reshma" <reshma.pattan@intel.com>]

> -----Original Message-----
> From: Sevincer, Abdullah <abdullah.sevincer@intel.com>

> +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];
> +static uint32_t enable_shw_all_eventdev_queues; static uint32_t

<snip>

> +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;

How about keeping  a struct to maintain  all the data together that way you know on which all ports and queues of eventdev we have requested display. 
You can refer the existing code "struct desc_param" to see how this is done
Then you can declare global variables of type struct eventdev_params eventdev_var[MAX_EVENTDEV_DEV] to handle data of all evendev ids as an array, instead of keeping many global variables.
.

Ex: 
struct eventdev_params {
           	unit8_t eventdev_id;
           	unit8_t ports[MAX_PORTS_QUEUES]
          	 unit8_t queues[MAX_PORTS_QUEUES]
          	 static uint8_t num_queues
          	 static uint8_t num_ports

	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;
}

> +	if (show_edev_xstats()) {

I don't think we really need the show_edev_xstats() function.  You can directly call process_eventdev_xstats() here.


> +		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");
> +
> +		/* Verify the command line options */
> +		if (evdev_id >= rte_event_dev_count())
> +			rte_panic("invalid event device %hhu\n", evdev_id);

Also, You can move above eventdev id validation code to  a small separate function().  
That new function can be called from parse_eventdev_queue_params() and parse_eventdev_port_params(), to validate the event dev id.
So you no need to do this eventdev validation here.

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


Also I guess you can move the below piece of code to be part of the process_eventdev_xstats()

> diff --git a/doc/guides/tools/proc_info.rst b/doc/guides/tools/proc_info.rst
> index cf3502a8cb..f0a7d37e41 100644

> -   --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]
> 
Need to edit these eventdev commands  to show the new  format port:eventdevid, queue:eventdevid and eventdev id.

Thanks,
Reshma