From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 0CFBCA0542;
	Sat,  8 Oct 2022 11:46:29 +0200 (CEST)
Received: from [217.70.189.124] (localhost [127.0.0.1])
	by mails.dpdk.org (Postfix) with ESMTP id A659440146;
	Sat,  8 Oct 2022 11:46:28 +0200 (CEST)
Received: from szxga02-in.huawei.com (szxga02-in.huawei.com [45.249.212.188])
 by mails.dpdk.org (Postfix) with ESMTP id 41C9540042
 for <dev@dpdk.org>; Sat,  8 Oct 2022 11:46:26 +0200 (CEST)
Received: from kwepemi500017.china.huawei.com (unknown [172.30.72.56])
 by szxga02-in.huawei.com (SkyGuard) with ESMTP id 4Ml0ZM3mHnzVhpg;
 Sat,  8 Oct 2022 17:42:03 +0800 (CST)
Received: from [10.67.103.235] (10.67.103.235) by
 kwepemi500017.china.huawei.com (7.221.188.110) with Microsoft SMTP Server
 (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id
 15.1.2375.31; Sat, 8 Oct 2022 17:46:23 +0800
Subject: Re: [PATCH v5 3/3] app/procinfo: support descriptor dump
To: "Pattan, Reshma" <reshma.pattan@intel.com>, "dev@dpdk.org" <dev@dpdk.org>, 
 "thomas@monjalon.net" <thomas@monjalon.net>, "ferruh.yigit@amd.com"
 <ferruh.yigit@amd.com>, "andrew.rybchenko@oktetlabs.ru"
 <andrew.rybchenko@oktetlabs.ru>, "stephen@networkplumber.org"
 <stephen@networkplumber.org>, "mdr@ashroe.eu" <mdr@ashroe.eu>
References: <20220527023351.40577-1-humin29@huawei.com>
 <20221006120514.28830-1-liudongdong3@huawei.com>
 <20221006120514.28830-4-liudongdong3@huawei.com>
 <BYAPR11MB33664A16646C126FDC34C9DFFF5F9@BYAPR11MB3366.namprd11.prod.outlook.com>
CC: Min Hu <humin29@huawei.com>, Maryam Tahhan <maryam.tahhan@intel.com>
From: Dongdong Liu <liudongdong3@huawei.com>
Message-ID: <8ea1b901-0cf3-a5c8-f417-2865a791d2ab@huawei.com>
Date: Sat, 8 Oct 2022 17:46:23 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:45.0) Gecko/20100101
 Thunderbird/45.7.1
MIME-Version: 1.0
In-Reply-To: <BYAPR11MB33664A16646C126FDC34C9DFFF5F9@BYAPR11MB3366.namprd11.prod.outlook.com>
Content-Type: text/plain; charset="windows-1252"; format=flowed
Content-Transfer-Encoding: 7bit
X-Originating-IP: [10.67.103.235]
X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To
 kwepemi500017.china.huawei.com (7.221.188.110)
X-CFilter-Loop: Reflected
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: DPDK patches and discussions <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org

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.
>
> .
>