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 05C26A034F; Tue, 8 Jun 2021 12:49:51 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 733E74067A; Tue, 8 Jun 2021 12:49:51 +0200 (CEST) Received: from shelob.oktetlabs.ru (shelob.oktetlabs.ru [91.220.146.113]) by mails.dpdk.org (Postfix) with ESMTP id D787F4013F for ; Tue, 8 Jun 2021 12:49:49 +0200 (CEST) Received: from [192.168.38.17] (aros.oktetlabs.ru [192.168.38.17]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by shelob.oktetlabs.ru (Postfix) with ESMTPSA id 6D7737F4FE; Tue, 8 Jun 2021 13:49:49 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 shelob.oktetlabs.ru 6D7737F4FE DKIM-Signature: v=1; a=rsa-sha256; c=simple/simple; d=oktetlabs.ru; s=default; t=1623149389; bh=1F40DSPn1YBw7PScvCuGtG7SVebZsiGFF7KBnRox5oE=; h=Subject:To:Cc:References:From:Date:In-Reply-To; b=fY2Gl5f2v2ZvXg9WiufDITi9gBimYKRBi0NoxuTOWQsONv8Lv2WuT6dO/rfgDLnTv 15wf6a+1folEDyQcn/TdTDVBdpS1U8mJpGz/MYW17w/HmWhn3u/LPwS7BOck8mxwzr I3agLiSzL6mqOdn70e/9FbC2zWfjU7tU2mTnYKeU= To: "Min Hu (Connor)" , dev@dpdk.org Cc: ferruh.yigit@intel.com, xiaoyun.li@intel.com 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> From: Andrew Rybchenko Organization: OKTET Labs Message-ID: <0d9233a9-1807-a425-7fc5-2efb4bfa647f@oktetlabs.ru> Date: Tue, 8 Jun 2021 13:49:49 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101 Thunderbird/78.10.0 MIME-Version: 1.0 In-Reply-To: <1619599019-46246-2-git-send-email-humin29@huawei.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit 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" 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. > + } > > + /* > + * If the cmd fails to execute, it is necessary to reconfigure device. > + */ > cmd_reconfig_device_queue(RTE_PORT_ALL, 1, 1); > } > >