diff mbox series

[20/20] app/testpmd: align behaviour of multi-port detach

Message ID 20200913220711.3768597-21-thomas@monjalon.net (mailing list archive)
State Changes Requested, archived
Delegated to: Ferruh Yigit
Headers
Series cleanup ethdev close operation |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-broadcom-Performance success Performance Testing PASS
ci/iol-testing success Testing PASS
ci/iol-broadcom-Functional success Functional Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot success Travis build: passed
ci/Intel-compilation success Compilation OK

Commit Message

Thomas Monjalon Sept. 13, 2020, 10:07 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(-)
diff mbox series

Patch

diff --git a/app/test-pmd/config.c b/app/test-pmd/config.c
index 30bee33248..21513a2978 100644
--- a/app/test-pmd/config.c
+++ b/app/test-pmd/config.c
@@ -1588,9 +1588,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)) {
@@ -1599,7 +1602,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 31dc97239b..a76e9877f6 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2694,8 +2694,7 @@  close_port(portid_t pid)
 			continue;
 		}
 
-		if (port->flow_list)
-			port_flow_flush(pi);
+		port_flow_flush(pi);
 		rte_eth_dev_close(pi);
 	}
 
@@ -2825,15 +2824,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");
@@ -2854,8 +2858,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);
@@ -2885,9 +2887,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);
 		}
 	}