[PATCH v12] app/procinfo: display eventdev xstats

Sevincer, Abdullah abdullah.sevincer at intel.com
Mon Mar 20 19:01:51 CET 2023


+>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.
This is very good idea, there are other global variables as well which can be put in the structure. I think many global variables declared in this file can follow this approach. However, I haven't scoped this within this release timeline as we have reached almost end of it.


> +	if (show_edev_xstats()) {

>+I don't think we really need the show_edev_xstats() function.  You can directly call process_eventdev_xstats() here.
I think we need this here because we want to exit from the app after displaying xstats for eventdev. We don't want to mix rte_eventdev data displayed (if we don't exit rest of the program continues to display other stats for rte, e.g. check show mempool command line)


> +		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.
I think consolidating this call(checking the validation after evdevid is passed) in here is better, it seems more consolidated compared to have call at each parameter read in the parse function. 

> +
> +		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()
Okay, I will move this inside  process_eventdev_xstats().

>+Need to edit these eventdev commands  to show the new  format port:eventdevid, queue:eventdevid and eventdev id.
Will do.

Thanks,
Abdullah


More information about the dev mailing list