> -----Original Message-----
> From: Dongdong Liu <liudongdo...@huawei.com>
> Subject: [PATCH v4 3/3] app/procinfo: support descriptor dump
>
> From: "Min Hu (Connor)" <humi...@huawei.com>
>
> This patch support HW Rx/Tx descriptor dump
>
> The command is like:
> dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx -- -- --show-rx-descriptor
> queue_id:num
What is num here? You need to describe about this in commit message.
Is num means number of descriptors? Better use the descriptor word here in num
>
> dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx -- -- --show-tx-descriptor
> queue_id:num
Same here.
> +/* Enable dump buffer descriptor. */
> +#define MAX_NB_ITEM 2
> +static uint16_t rx_nb_item;
> +static uint16_t tx_nb_item;
> +static uint16_t rx_item_opt[MAX_NB_ITEM]; static uint16_t
Instead of using array to keep queid and number of descriptors info , better
have structure with 2 parameters one for queue and one for the number of
descriptors.
And You can use the same structure to declare tx_item.
> +tx_item_opt[MAX_NB_ITEM];
>
> + " --show-rx-descriptor queue_id:num: to display ports Rx
> buffer description by queue id and num\n"
> + " --show-tx-descriptor queue_id:num: to display ports Tx
> buffer description by queue id and num\n"
Here also use enough description what is num means or use the another name.
>+ if (ret < MAX_NB_ITEM) {
You can check ret < 0 no need to use MAX_NB_ITEM again.
>+ printf("Rx descriptor param parse
>error.\n");
>+ return -1;
>+ }
Instead of saying parse error. You might need to give clear reason of failure.
That is invalid queueid:num passed by user or user passed more than one pair.
>+ int ret = parse_descriptor_param(optarg,
>+ rx_item_opt,
>+ MAX_NB_ITEM);
Do we need to really pass the MAX_NB_ITEM> I don't think we need you can
directly use the macro in the called function.
>+ if (ret < MAX_NB_ITEM) {
You can check ret < 0 no need to use MAX_NB_ITEM again.
>+ printf("Tx descriptor param parse
>error.\n");
>+ return -1;
>+ }
Same here.
Thanks,
Reshma
