[v2,22/25] app/testpmd: align behaviour of multi-port detach
diff mbox series

Message ID 20200927234249.3198780-23-thomas@monjalon.net
State Superseded
Delegated to: Thomas Monjalon
Headers show
Series
  • cleanup ethdev close operation
Related show

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Thomas Monjalon Sept. 27, 2020, 11:42 p.m. UTC
A port can be closed in multiple situations:
	- close command calling close_port() -> rte_eth_dev_close()
	- exit calling close_port() -> rte_eth_dev_close()
	- hotplug calling close_port() -> rte_eth_dev_close()
	- hotplug calling detach_device() -> rte_dev_remove()
	- port detach command, detach_device() -> rte_dev_remove()
	- device detach command, detach_devargs() -> rte_eal_hotplug_remove()

The flow rules are flushed before each close.
It was already done in close_port(), detach_devargs() and
detach_port_device() which calls detach_device(),
but not in detach_device(). As a consequence, it was missing for siblings
of port detach command and unplugged device.
The check before calling port_flow_flush() is moved inside the function.

The state of the port to close is checked to be stopped.
As above, this check was missing in detach_device(),
impacting the cases of a multi-port device unplugged or detached
with the port detach command.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 app/test-pmd/config.c  |  7 +++++--
 app/test-pmd/testpmd.c | 22 +++++++++++-----------
 2 files changed, 16 insertions(+), 13 deletions(-)

Patch
diff mbox series

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 2d9a456467..4691ae1f7b 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1590,9 +1590,12 @@  int
 port_flow_flush(portid_t port_id)
 {
 	struct rte_flow_error error;
-	struct rte_port *port;
+	struct rte_port *port = &ports[port_id];
 	int ret = 0;
 
+	if (port->flow_list == NULL)
+		return ret;
+
 	/* Poisoning to make sure PMDs update it in case of error. */
 	memset(&error, 0x44, sizeof(error));
 	if (rte_flow_flush(port_id, &error)) {
@@ -1601,7 +1604,7 @@  port_flow_flush(portid_t port_id)
 		    port_id == (portid_t)RTE_PORT_ALL)
 			return ret;
 	}
-	port = &ports[port_id];
+
 	while (port->flow_list) {
 		struct port_flow *pf = port->flow_list->next;
 
diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 1e10d2e2e4..ccba71c076 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2695,8 +2695,7 @@  close_port(portid_t pid)
 			continue;
 		}
 
-		if (port->flow_list)
-			port_flow_flush(pi);
+		port_flow_flush(pi);
 		rte_eth_dev_close(pi);
 	}
 
@@ -2826,15 +2825,20 @@  detach_device(struct rte_device *dev)
 
 	printf("Removing a device...\n");
 
-	if (rte_dev_remove(dev) < 0) {
-		TESTPMD_LOG(ERR, "Failed to detach device %s\n", dev->name);
-		return;
-	}
 	RTE_ETH_FOREACH_DEV_OF(sibling, dev) {
 		if (ports[sibling].port_status != RTE_PORT_CLOSED) {
+			if (ports[sibling].port_status != RTE_PORT_STOPPED) {
+				printf("Port %u not stopped\n", sibling);
+				return;
+			}
+			port_flow_flush(sibling);
 		}
 	}
 
+	if (rte_dev_remove(dev) < 0) {
+		TESTPMD_LOG(ERR, "Failed to detach device %s\n", dev->name);
+		return;
+	}
 	remove_invalid_ports();
 
 	printf("Device is detached\n");
@@ -2855,8 +2859,6 @@  detach_port_device(portid_t port_id)
 			return;
 		}
 		printf("Port was not closed\n");
-		if (ports[port_id].flow_list)
-			port_flow_flush(port_id);
 	}
 
 	detach_device(rte_eth_devices[port_id].device);
@@ -2886,9 +2888,7 @@  detach_devargs(char *identifier)
 				rte_eth_iterator_cleanup(&iterator);
 				return;
 			}
-
-			if (ports[port_id].flow_list)
-				port_flow_flush(port_id);
+			port_flow_flush(port_id);
 		}
 	}