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 8F715A0C51; Thu, 10 Jun 2021 08:13:55 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0D5DB4067C; Thu, 10 Jun 2021 08:13:55 +0200 (CEST) Received: from szxga03-in.huawei.com (szxga03-in.huawei.com [45.249.212.189]) by mails.dpdk.org (Postfix) with ESMTP id B8C804003C for ; Thu, 10 Jun 2021 08:13:52 +0200 (CEST) Received: from dggemv704-chm.china.huawei.com (unknown [172.30.72.54]) by szxga03-in.huawei.com (SkyGuard) with ESMTP id 4G0tqR53nxz6w6M for ; Thu, 10 Jun 2021 14:09:55 +0800 (CST) Received: from dggema767-chm.china.huawei.com (10.1.198.209) by dggemv704-chm.china.huawei.com (10.3.19.47) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.2176.2; Thu, 10 Jun 2021 14:13:45 +0800 Received: from [10.66.74.184] (10.66.74.184) by dggema767-chm.china.huawei.com (10.1.198.209) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256_P256) id 15.1.2176.2; Thu, 10 Jun 2021 14:13:44 +0800 To: Andrew Rybchenko , "Min Hu (Connor)" , CC: , References: <1618813303-32945-1-git-send-email-humin29@huawei.com> <1619599019-46246-1-git-send-email-humin29@huawei.com> <1619599019-46246-2-git-send-email-humin29@huawei.com> <0d9233a9-1807-a425-7fc5-2efb4bfa647f@oktetlabs.ru> From: Huisong Li Message-ID: <20f800bb-cb3a-0580-d97d-0e1726ff6c34@huawei.com> Date: Thu, 10 Jun 2021 14:13:39 +0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.8.0 MIME-Version: 1.0 In-Reply-To: <0d9233a9-1807-a425-7fc5-2efb4bfa647f@oktetlabs.ru> Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 8bit X-Originating-IP: [10.66.74.184] X-ClientProxiedBy: dggems705-chm.china.huawei.com (10.3.19.182) To dggema767-chm.china.huawei.com (10.1.198.209) X-CFilter-Loop: Reflected Subject: Re: [dpdk-dev] [PATCH v2 1/2] app/testpmd: add link speed check before port start 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" 在 2021/6/8 18:49, Andrew Rybchenko 写道: > On 4/28/21 11:36 AM, Min Hu (Connor) wrote: >> From: Huisong Li >> >> Currently, to check whether the configured link_speeds is valid, we >> have to run "port start". In addition, if the configuration fails, >> "port->dev_conf.link_speeds" maintained in testpmd cannot be >> restored. >> >> This patch adds the link_speeds check before port start by calling >> dev_configure, and resolves these problems. >> >> Signed-off-by: Huisong Li >> Signed-off-by: Min Hu (Connor) >> --- >> app/test-pmd/cmdline.c | 42 ++++++++++++++++++++++++++++++++++++++++-- >> 1 file changed, 40 insertions(+), 2 deletions(-) >> >> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c >> index 5fdcc1c..ddbc629 100644 >> --- a/app/test-pmd/cmdline.c >> +++ b/app/test-pmd/cmdline.c >> @@ -1549,8 +1549,12 @@ cmd_config_speed_all_parsed(void *parsed_result, >> __rte_unused void *data) >> { >> struct cmd_config_speed_all *res = parsed_result; >> + uint32_t old_link_speeds[RTE_MAX_ETHPORTS]; >> + struct rte_port *port; >> uint32_t link_speed; >> portid_t pid; >> + portid_t i; >> + int ret; >> >> if (!all_ports_stopped()) { >> printf("Please stop all ports first\n"); >> @@ -1562,7 +1566,26 @@ cmd_config_speed_all_parsed(void *parsed_result, >> return; >> >> RTE_ETH_FOREACH_DEV(pid) { >> - ports[pid].dev_conf.link_speeds = link_speed; >> + port = &ports[pid]; >> + old_link_speeds[pid] = port->dev_conf.link_speeds; >> + port->dev_conf.link_speeds = link_speed; >> + ret = rte_eth_dev_configure(pid, nb_rxq, nb_txq, >> + &port->dev_conf); >> + if (ret < 0) { >> + printf("Failed to check link speeds for port %d, ret = %d.\n", >> + pid, ret); >> + goto roolback; >> + } >> + } >> + >> + cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1); >> + >> + return; >> + >> +roolback: >> + for (i = 0; i <= pid; i++) { >> + port = &ports[i]; >> + port->dev_conf.link_speeds = old_link_speeds[i]; >> } >> >> cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1); >> @@ -1621,7 +1644,10 @@ cmd_config_speed_specific_parsed(void *parsed_result, >> __rte_unused void *data) >> { >> struct cmd_config_speed_specific *res = parsed_result; >> + uint32_t old_link_speeds; >> + struct rte_port *port; >> uint32_t link_speed; >> + int ret; >> >> if (!all_ports_stopped()) { >> printf("Please stop all ports first\n"); >> @@ -1635,8 +1661,20 @@ cmd_config_speed_specific_parsed(void *parsed_result, >> &link_speed) < 0) >> return; >> >> - ports[res->id].dev_conf.link_speeds = link_speed; >> + port = &ports[res->id]; >> + old_link_speeds = port->dev_conf.link_speeds; >> + port->dev_conf.link_speeds = link_speed; >> + ret = rte_eth_dev_configure(res->id, nb_rxq, nb_txq, >> + &port->dev_conf); >> + if (ret < 0) { >> + printf("Failed to check link speeds for port %d, ret = %d.\n", >> + res->id, ret); >> + port->dev_conf.link_speeds = old_link_speeds; > In fact, you don't really know here if link speed is a problem > or something else configured while device is stopped. > So, I think there is no point to over-complicate it > with addition of possibly misleading diagnostics. Thanks for your reivew. You're right. There are many configurations in tespmd that need to recall dev_configure and are not verified during configuration. Only one of them is verified, which does not indicate that the failure is caused by the current configuration. So let's keep it the way it was. >> + } >> >> + /* >> + * If the cmd fails to execute, it is necessary to reconfigure device. >> + */ >> cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1); >> } >> >> > .