Re: [PATCH v5 3/3] app/procinfo: support descriptor dump

[Dongdong Liu <liudongdong3@huawei.com>]

Hi Reshma

Many thanks for your review.
I have sent out the below patchset. this patchset have fixed the comments.

[v8,7/8] app/procinfo: support descriptor dump
https://patches.dpdk.org/project/dpdk/list/?series=25047

Thanks,
Dongdong.
On 2022/10/7 22:43, Pattan, Reshma wrote:
>
>
>> -----Original Message-----
>> +/* Enable dump Rx/Tx descriptor. */
>> +#define DESC_PARAM_NUM 3
>> +
>> +struct desc_param {
>> +	uint16_t queue_id;  /* A queue identifier on this port. */
>> +	uint16_t offset;    /* The offset of the descriptor starting from tail. */
>> +	uint16_t num;       /* The number of the descriptors to dump. */
>> +	bool valid;
>
> You don't need to keep if the descriptor parameters are valid or not, as you are exiting the application when you see invalid parameters are entered by user.
>
>>
>> +static int
>> +parse_descriptor_param(char *list, struct desc_param *desc) {
>> +	int ret;
>> +
>> +	ret = sscanf(list, "%hu:%hu:%hu", &desc->queue_id, &desc->offset,
>> +		     &desc->num);
>> +	if (ret != DESC_PARAM_NUM) {
>> +		desc->valid = false;
>> +		return -EINVAL;
>
> On error return application is exiting , so no need to maintain desc->valid
>
>>  main(int argc, char **argv)
>>  {
>> @@ -1564,6 +1638,12 @@ main(int argc, char **argv)
>>  			metrics_display(i);
>>  #endif
>>
>> +		if (rx_desc_param.valid)
>
> So if rx_desc dump is requested in command line you can set some global variable like "enable-show-rx-desc-dump" and display below info only if that variable is set.
> So we no need to use valid here.
>
>
>
>> +			nic_descriptor_display(i, &rx_desc_param,
>> +					       rte_eth_rx_descriptor_dump);
>> +		if (tx_desc_param.valid)
>
> Same here as above comment.
>
> .
>