On 10/25/2021 7:39 AM, Min Hu (Connor) wrote:
From: Huisong Li <lihuis...@huawei.com>
Stopping a bond device also stops all active slaves under the bond device.
If this port is bond device, we need to modify the port status of all
slaves from RTE_PORT_STARTED to RTE_PORT_STOPPED.
Fixes: 0e545d3047fe ("app/testpmd: check stopping port is not in bonding")
Cc: sta...@dpdk.org
Signed-off-by: Huisong Li <lihuis...@huawei.com>
Signed-off-by: Min Hu (Connor) <humi...@huawei.com>
---
app/test-pmd/cmdline.c | 1 +
app/test-pmd/testpmd.c | 49 +++++++++++++++++++++++++++++++++++++++---
app/test-pmd/testpmd.h | 3 ++-
3 files changed, 49 insertions(+), 4 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 722f4fb9d9..5bfb4b509b 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -6639,6 +6639,7 @@ static void cmd_create_bonded_device_parsed(void
*parsed_result,
"Failed to enable promiscuous mode for port %u: %s -
ignore\n",
port_id, rte_strerror(-ret));
+ ports[port_id].bond_flag = 1;
ports[port_id].need_setup = 0;
ports[port_id].port_status = RTE_PORT_STOPPED;
}
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index af0e79fe6d..d6b9ebc4dd 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -65,6 +65,9 @@
#ifdef RTE_EXEC_ENV_WINDOWS
#include <process.h>
#endif
+#ifdef RTE_NET_BOND
+#include <rte_eth_bond.h>
+#endif
#include "testpmd.h"
@@ -2986,6 +2989,35 @@ start_port(portid_t pid)
return 0;
}
+#ifdef RTE_NET_BOND
+static void
+change_bonding_active_slave_port_status(portid_t bond_pid)
The function sets the status explicitly to PORT_STOPPED, but function
name is more generic, should we update the function name to reflect the
functionality?
+{
+ portid_t slave_pids[RTE_MAX_ETHPORTS];
+ struct rte_port *port;
+ int num_active_slaves;
+ portid_t slave_pid;
+ int i;
+
+ num_active_slaves = rte_eth_bond_active_slaves_get(bond_pid, slave_pids,
+ RTE_MAX_ETHPORTS);
+ if (num_active_slaves < 0) {
+ fprintf(stderr, "Failed to get slave list for port = %u\n",
+ bond_pid);
+ return;
+ }
+
+ for (i = 0; i < num_active_slaves; i++) {
+ slave_pid = slave_pids[i];
+ port = &ports[slave_pid];
+ if (rte_atomic16_cmpset(&(port->port_status),
+ RTE_PORT_STARTED, RTE_PORT_STOPPED) == 0)
+ fprintf(stderr, "Port %u can not be set into stopped\n",
+ slave_pid);
+ }
+}
+#endif
+
void
stop_port(portid_t pid)
{
@@ -3042,9 +3074,20 @@ stop_port(portid_t pid)
if (port->flow_list)
port_flow_flush(pi);
- if (eth_dev_stop_mp(pi) != 0)
- RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port
%u\n",
- pi);
Can you please remove the 'eth_dev_stop_mp()' function in this patch,
which is removed in patch 2/3.
+ if (is_proc_primary()) {
+#ifdef RTE_NET_BOND
+ /*
+ * Stopping a bond device also stops all active slaves
+ * under the bond device. If this port is bond device,
+ * we need to modify the port status of all slaves.
+ */
+ if (port->bond_flag == 1)
+ change_bonding_active_slave_port_status(pi);
+#endif
+ if (rte_eth_dev_stop(pi) != 0)
+ RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port
%u\n",
+ pi);
Should we roll back the slave port status if 'rte_eth_dev_stop(pi)' fails?
+ }
if (rte_atomic16_cmpset(&(port->port_status),
RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
index e3995d24ab..ad3b4f875c 100644
--- a/app/test-pmd/testpmd.h
+++ b/app/test-pmd/testpmd.h
@@ -237,7 +237,8 @@ struct rte_port {
struct rte_eth_txconf tx_conf[RTE_MAX_QUEUES_PER_PORT+1]; /**< per
queue tx configuration */
struct rte_ether_addr *mc_addr_pool; /**< pool of multicast addrs */
uint32_t mc_addr_nb; /**< nb. of addr. in mc_addr_pool */
- uint8_t slave_flag; /**< bonding slave port */
+ uint8_t slave_flag : 1, /**< bonding slave port */
+ bond_flag : 1; /**< port is bond device */
Can't we detect if the port is a bonding port without introducing a new
variable/state?
struct port_flow *flow_list; /**< Associated flows. */
struct port_indirect_action *actions_list;
/**< Associated indirect actions. */
