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 A2C5DA0A0F; Sat, 5 Jun 2021 05:15:15 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 134A34067B; Sat, 5 Jun 2021 05:15:15 +0200 (CEST) Received: from szxga01-in.huawei.com (szxga01-in.huawei.com [45.249.212.187]) by mails.dpdk.org (Postfix) with ESMTP id E391740147 for ; Sat, 5 Jun 2021 05:15:13 +0200 (CEST) Received: from dggeme765-chm.china.huawei.com (unknown [172.30.72.54]) by szxga01-in.huawei.com (SkyGuard) with ESMTP id 4Fxl6v5L8BzYqqM for ; Sat, 5 Jun 2021 11:12:23 +0800 (CST) Received: from [127.0.0.1] (10.69.27.114) by dggeme765-chm.china.huawei.com (10.3.19.111) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2; Sat, 5 Jun 2021 11:15:10 +0800 To: "Pattan, Reshma" , "Min Hu (Connor)" , "dev@dpdk.org" References: <1619355742-15429-1-git-send-email-humin29@huawei.com> CC: "Tahhan, Maryam" From: Chengchang Tang Message-ID: <0add2988-b4d1-0773-d075-8d51055d9cc2@huawei.com> Date: Sat, 5 Jun 2021 11:15:09 +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="UTF-8" Content-Transfer-Encoding: 8bit X-Originating-IP: [10.69.27.114] X-ClientProxiedBy: dggems706-chm.china.huawei.com (10.3.19.183) To dggeme765-chm.china.huawei.com (10.3.19.111) X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH] app/procinfo: add device registers dump 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 Sender: "dev" On 2021/6/4 23:04, Pattan, Reshma wrote: > > >> -----Original Message----- >> From: Min Hu (Connor) > > > >> + ret = rte_eth_dev_get_reg_info(i, ®_info); >> + if (ret) { >> + printf("Error getting device reg info: %d\n", ret); >> + continue; >> + } >> + >> + buf_size = reg_info.length * reg_info.width; > > > If it is to get the regs length, you can directly call "rte_ethtool_get_regs_len(uint16_t port_id)" API , instead of again writing the above logic. > And use the returned length in below malloc. This logic is indeed identical to the logic of the "rte_ethtool_get_regs_len" API of Ethtool, but the method of using the "rte_eth_dev_get_reg_info" API is the case. All users will have similar code logic when using this API. And use "rte_ethtool_get_regs_len" API here only reduces "buf_size = reg_info.length * reg_info.width;" this logic. But at the same time, it introduces the dependence of example/ethtool in procinfo. The code in example is not compiled by default, which seems not appropriate to import it in app/procinfo. So, I think it is fine to keep this. > > >> + fp_regs = fopen(file_name, "wb"); >> + if (fp_regs == NULL) { >> + printf("Error during opening '%s' for writing\n", >> + file_name); > > Better to print error string from fopen() errno on failure , to indicate the exact error. Agree， I will fix it in the next version. > >> + } else { >> + if ((int)fwrite(buf_data, 1, buf_size, fp_regs) != > > Better have "((int)fwrite(buf_data, 1, buf_size, fp_regs)" In separate line and use the returned value inside if check. Agree， I will fix it in the next version. > >> + buf_size) >> + printf("Error during writing %s\n", >> + file_prefix); > > Better to print error string from fwrite errno on failure , to indicate the exact error. > >> + else >> + printf("dump device (%s) regs successfully, " > > Reframe the sente to "Device regs dumped successfully" > Agree， I will fix it in the next version. > . >