From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 4B2A0A0542; Sat, 24 Sep 2022 10:41:33 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 301DE400D5; Sat, 24 Sep 2022 10:41:33 +0200 (CEST) Received: from szxga08-in.huawei.com (szxga08-in.huawei.com [45.249.212.255]) by mails.dpdk.org (Postfix) with ESMTP id 77223400D4 for ; Sat, 24 Sep 2022 10:41:31 +0200 (CEST) Received: from kwepemi500017.china.huawei.com (unknown [172.30.72.56]) by szxga08-in.huawei.com (SkyGuard) with ESMTP id 4MZMp42N6Nz1P6kR; Sat, 24 Sep 2022 16:37:16 +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, 24 Sep 2022 16:41:26 +0800 Subject: Re: [PATCH v4 3/3] app/procinfo: support descriptor dump To: "Pattan, Reshma" , "dev@dpdk.org" , "thomas@monjalon.net" , "ferruh.yigit@xilinx.com" , "andrew.rybchenko@oktetlabs.ru" , "stephen@networkplumber.org" , "mdr@ashroe.eu" References: <20220527023351.40577-1-humin29@huawei.com> <20220923074316.25077-1-liudongdong3@huawei.com> <20220923074316.25077-4-liudongdong3@huawei.com> CC: "Min Hu (Connor)" , Maryam Tahhan From: Dongdong Liu Message-ID: <07c0f79d-6025-ad12-6a48-633dd313a2fc@huawei.com> Date: Sat, 24 Sep 2022 16:41:25 +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: Content-Type: text/plain; charset="windows-1252"; format=flowed Content-Transfer-Encoding: 7bit X-Originating-IP: [10.67.103.235] X-ClientProxiedBy: dggems704-chm.china.huawei.com (10.3.19.181) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Hi Reshma Many thanks for your view. On 2022/9/23 20:18, Pattan, Reshma wrote: > > >> -----Original Message----- >> From: Dongdong Liu >> Subject: [PATCH v4 3/3] app/procinfo: support descriptor dump >> >> From: "Min Hu (Connor)" >> >> 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 Yes, It means number of descriptors. How about rename it to desc_num. > >> >> dpdk-proc-info -a xxxx:xx:xx.x --file-prefix=xxx -- -- --show-tx-descriptor >> queue_id:num > > Same here. Will fix. > >> +/* 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. Good point, will do. > >> +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. Will do. > >> + 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. Will do. > >> + 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. Will fix. > >> + printf("Tx descriptor param parse error.\n"); >> + return -1; >> + } > > Same here. Will do. Thanks, Dongdong > > Thanks, > Reshma > > > . >