Re: [dpdk-dev] [PATCH v2 1/2] app/testpmd: add link speed check before port start
From: Andrew Rybchenko <hidden>
Date: 2021-06-08 10:49:53
On 4/28/21 11:36 AM, Min Hu (Connor) wrote:
quoted hunk ↗ jump to hunk
From: Huisong Li <lihuisong@huawei.com> 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 <lihuisong@huawei.com> Signed-off-by: Min Hu (Connor) <redacted> --- 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); }