[dpdk-stable] patch 'app/testpmd: fix hot-unplug detaching' has been queued to LTS release 18.11.7

[Kevin Traynor <ktraynor@redhat.com>]

Hi,

FYI, your patch has been queued to LTS release 18.11.7

Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
It will be pushed if I get no objections before 02/25/20. So please
shout if anyone has objections.

Also note that after the patch there's a diff of the upstream commit vs the
patch applied to the branch. This will indicate if there was any rebasing
needed to apply to the stable branch. If there were code changes for rebasing
(ie: not only metadata diffs), please double check that the rebase was
correctly done.

Queued patches are on a temporary branch at:
https://github.com/kevintraynor/dpdk-stable-queue

This queued commit can be viewed at:
https://github.com/kevintraynor/dpdk-stable-queue/commit/b0edd2452e95ae8558787895d04b07dd0a2e672c

Thanks.

Kevin.

---
From b0edd2452e95ae8558787895d04b07dd0a2e672c Mon Sep 17 00:00:00 2001
From: Thomas Monjalon <thomas@monjalon.net>
Date: Thu, 13 Feb 2020 16:52:26 +0100
Subject: [PATCH] app/testpmd: fix hot-unplug detaching

[ upstream commit 0654d4a8ad694821e3ce274e87b1ab89fd587f1c ]

There is a possible race condition in the hotplug path
in rmv_port_callback(). If a port is created between
close_port(port_id) and detach_port_device(port_id),
then the port_id will have been reallocated to a different
device which will be wrongly detached.

Since a check was added in detach_port_device() for
manual detach case, the hotplug path was even more broken.
It became impossible to run because the new check prevented
to run detach_port_device() after the port is closed.

The solution for both issues is to not rely on the port_id
for detaching the rte_device.
The function detach_port_device() is split to allow calling
detach_device() directly with the rte_device pointer, saved
before closing the port.

Fixes: cbb4c648c5df ("ethdev: use device handle to detach")

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
---
 app/test-pmd/testpmd.c | 49 +++++++++++++++++++++++++-----------------
 1 file changed, 29 insertions(+), 20 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 31d559494d..a910c06dc2 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2343,16 +2343,9 @@ setup_attached_port(portid_t pi)
 }
 
-void
-detach_port_device(portid_t port_id)
+static void
+detach_device(struct rte_device *dev)
 {
-	struct rte_device *dev;
 	portid_t sibling;
 
-	printf("Removing a device...\n");
-
-	if (port_id_is_invalid(port_id, ENABLED_WARN))
-		return;
-
-	dev = rte_eth_devices[port_id].device;
 	if (dev == NULL) {
 		printf("Device already removed\n");
@@ -2360,13 +2353,5 @@ detach_port_device(portid_t port_id)
 	}
 
-	if (ports[port_id].port_status != RTE_PORT_CLOSED) {
-		if (ports[port_id].port_status != RTE_PORT_STOPPED) {
-			printf("Port not stopped\n");
-			return;
-		}
-		printf("Port was not closed\n");
-		if (ports[port_id].flow_list)
-			port_flow_flush(port_id);
-	}
+	printf("Removing a device...\n");
 
 	if (rte_dev_remove(dev) < 0) {
@@ -2389,5 +2374,5 @@ detach_port_device(portid_t port_id)
 	remove_invalid_ports();
 
-	printf("Device of port %u is detached\n", port_id);
+	printf("Device is detached\n");
 	printf("Now total ports is %d\n", nb_ports);
 	printf("Done\n");
@@ -2395,4 +2380,23 @@ detach_port_device(portid_t port_id)
 }
 
+void
+detach_port_device(portid_t port_id)
+{
+	if (port_id_is_invalid(port_id, ENABLED_WARN))
+		return;
+
+	if (ports[port_id].port_status != RTE_PORT_CLOSED) {
+		if (ports[port_id].port_status != RTE_PORT_STOPPED) {
+			printf("Port not stopped\n");
+			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);
+}
+
 void
 pmd_test_exit(void)
@@ -2539,4 +2543,5 @@ rmv_port_callback(void *arg)
 	int org_no_link_check = no_link_check;
 	portid_t port_id = (intptr_t)arg;
+	struct rte_device *dev;
 
 	RTE_ETH_VALID_PORTID_OR_RET(port_id);
@@ -2549,6 +2554,10 @@ rmv_port_callback(void *arg)
 	stop_port(port_id);
 	no_link_check = org_no_link_check;
+
+	/* Save rte_device pointer before closing ethdev port */
+	dev = rte_eth_devices[port_id].device;
 	close_port(port_id);
-	detach_port_device(port_id);
+	detach_device(dev); /* might be already removed or have more ports */
+
 	if (need_to_start)
 		start_packet_forwarding(0);
-- 
2.21.1

---
  Diff of the applied patch vs upstream commit (please double-check if non-empty:
---
--- -	2020-02-19 15:43:50.666650093 +0000
+++ 0018-app-testpmd-fix-hot-unplug-detaching.patch	2020-02-19 15:43:49.761141384 +0000
@@ -1 +1 @@
-From 0654d4a8ad694821e3ce274e87b1ab89fd587f1c Mon Sep 17 00:00:00 2001
+From b0edd2452e95ae8558787895d04b07dd0a2e672c Mon Sep 17 00:00:00 2001
@@ -5,0 +6,2 @@
+[ upstream commit 0654d4a8ad694821e3ce274e87b1ab89fd587f1c ]
+
@@ -23,2 +25 @@
-Fixes: 43d0e304980a ("app/testpmd: fix invalid port detaching")
-Cc: stable@dpdk.org
+Fixes: cbb4c648c5df ("ethdev: use device handle to detach")
@@ -33 +34 @@
-index 031fe25f39..035836adfb 100644
+index 31d559494d..a910c06dc2 100644
@@ -36 +37 @@
-@@ -2634,16 +2634,9 @@ setup_attached_port(portid_t pi)
+@@ -2343,16 +2343,9 @@ setup_attached_port(portid_t pi)
@@ -55 +56 @@
-@@ -2651,13 +2644,5 @@ detach_port_device(portid_t port_id)
+@@ -2360,13 +2353,5 @@ detach_port_device(portid_t port_id)
@@ -70 +71 @@
-@@ -2677,5 +2662,5 @@ detach_port_device(portid_t port_id)
+@@ -2389,5 +2374,5 @@ detach_port_device(portid_t port_id)
@@ -77 +78 @@
-@@ -2683,4 +2668,23 @@ detach_port_device(portid_t port_id)
+@@ -2395,4 +2380,23 @@ detach_port_device(portid_t port_id)
@@ -100,2 +101,2 @@
- detach_devargs(char *identifier)
-@@ -2873,4 +2877,5 @@ rmv_port_callback(void *arg)
+ pmd_test_exit(void)
+@@ -2539,4 +2543,5 @@ rmv_port_callback(void *arg)
@@ -107 +108 @@
-@@ -2883,6 +2888,10 @@ rmv_port_callback(void *arg)
+@@ -2549,6 +2554,10 @@ rmv_port_callback(void *arg)