From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
	by inbox.dpdk.org (Postfix) with ESMTP id 3F249A0542
	for <public@inbox.dpdk.org>; Thu, 13 Feb 2020 16:52:40 +0100 (CET)
Received: from [92.243.14.124] (localhost [127.0.0.1])
	by dpdk.org (Postfix) with ESMTP id 21BDB1C0AE;
	Thu, 13 Feb 2020 16:52:40 +0100 (CET)
Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com
 [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 603981C06D;
 Thu, 13 Feb 2020 16:52:36 +0100 (CET)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
 by mailout.nyi.internal (Postfix) with ESMTP id BBFAD21F76;
 Thu, 13 Feb 2020 10:52:35 -0500 (EST)
Received: from mailfrontend1 ([10.202.2.162])
 by compute1.internal (MEProxy); Thu, 13 Feb 2020 10:52:35 -0500
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h=
 from:to:cc:subject:date:message-id:in-reply-to:references
 :mime-version:content-transfer-encoding; s=mesmtp; bh=HYsnGajw6N
 w1FIahplUpZHRLskO6bPk7TXaFJ+xs7CU=; b=hiiFKeZyKrREpHetL4uO4u0lS2
 Byt0+nXSLTv8fLNdrAW37ddCXxr3p8jziCLe68EPPVuPx2b57Qi3Pi9P9GcrbuNq
 g2PKRV3q7+C/WyMpEOq9r2t3hiD+wZcy2RbSUJWD6CeM08FpW5ZePcxxAPW3iiRm
 B6/jEHDv4oHP/w23s=
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
 messagingengine.com; h=cc:content-transfer-encoding:date:from
 :in-reply-to:message-id:mime-version:references:subject:to
 :x-me-proxy:x-me-proxy:x-me-sender:x-me-sender:x-sasl-enc; s=
 fm2; bh=HYsnGajw6Nw1FIahplUpZHRLskO6bPk7TXaFJ+xs7CU=; b=BT8STHQV
 sQV6h7btx2Z/iAYK0glgQXK5QbsG+CItu4XW6ZikrtZGWnGAgDwR7giHwNrlskDT
 uTM1b0fEFMwARLmhX+1NrHckq9hK0jg5dqgJr83o8dm37VVanxy/+WuKkx/XXsvF
 RQoa04UzLYwcc+hMamnhvEnpZwUwRbOq859UpkaKiHTsQdePN7nagX8fXf9wDhxT
 dnR6BhKOeKq19voZMPQ21JyTVEWPLMq0KP3Z7iaXz8b2QLg3UXPrvslDHbbxLMxH
 G/xsGYQVW1BI5nZar0HTm2kDF33E6uxSQvm2YC5qXPyb29HJGMSTwSeZraY3KJ4L
 0O7ZwZW+g5d8Iw==
X-ME-Sender: <xms:w3BFXht9SEsmLHPT7b8kej8sZaMMlPX4fBBIKtTdmjontssc68f5cg>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedugedrieekgdekvdcutefuodetggdotefrodftvf
 curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu
 uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc
 fjughrpefhvffufffkofgjfhgggfestdekredtredttdenucfhrhhomhepvfhhohhmrghs
 ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkph
 epjeejrddufeegrddvtdefrddukeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghr
 rghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth
X-ME-Proxy: <xmx:w3BFXm1lO75rzQZ7-3OKpuWInvDvo3oMF0qWIPNJqBeLnO8vYTXUFA>
 <xmx:w3BFXnkK0KIN-pW53LZH8gyixspXdH3TIH_4FtMe1gGtSYHofxZKFA>
 <xmx:w3BFXnQhfzocBHph0x0m7OEEGJZvri5TYVhPsVA9v6F4NC4AgS4-sg>
 <xmx:w3BFXkKWJljwmC_Yv-MvJiZBP2xdOxnbYjhC37fbI1AZFuNC5VIzIQ>
Received: from xps.monjalon.net (184.203.134.77.rev.sfr.net [77.134.203.184])
 by mail.messagingengine.com (Postfix) with ESMTPA id C14BB328005E;
 Thu, 13 Feb 2020 10:52:34 -0500 (EST)
From: Thomas Monjalon <thomas@monjalon.net>
To: dev@dpdk.org
Cc: matan@mellanox.com, ferruh.yigit@intel.com, stable@dpdk.org,
 Wenzhuo Lu <wenzhuo.lu@intel.com>, Jingjing Wu <jingjing.wu@intel.com>,
 Bernard Iremonger <bernard.iremonger@intel.com>
Date: Thu, 13 Feb 2020 16:52:26 +0100
Message-Id: <20200213155226.1024939-3-thomas@monjalon.net>
X-Mailer: git-send-email 2.25.0
In-Reply-To: <20200213155226.1024939-1-thomas@monjalon.net>
References: <20200213155226.1024939-1-thomas@monjalon.net>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Subject: [dpdk-stable] [PATCH 2/2] app/testpmd: fix hot-unplug detaching
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
 <mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
 <mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
Sender: "stable" <stable-bounces@dpdk.org>

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: 43d0e304980a ("app/testpmd: fix invalid port detaching")
Cc: stable@dpdk.org

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 app/test-pmd/testpmd.c | 48 ++++++++++++++++++++++++------------------
 1 file changed, 28 insertions(+), 20 deletions(-)

diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
index 11203cb03d..035836adfb 100644
--- a/app/test-pmd/testpmd.c
+++ b/app/test-pmd/testpmd.c
@@ -2633,32 +2633,17 @@ setup_attached_port(portid_t pi)
 	printf("Done\n");
 }
 
-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");
 		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);
-	}
+	printf("Removing a device...\n");
 
 	if (rte_dev_remove(dev) < 0) {
 		TESTPMD_LOG(ERR, "Failed to detach device %s\n", dev->name);
@@ -2676,13 +2661,31 @@ 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");
 	return;
 }
 
 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
 detach_devargs(char *identifier)
 {
@@ -2873,6 +2876,7 @@ rmv_port_callback(void *arg)
 	int need_to_start = 0;
 	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);
 
@@ -2883,8 +2887,12 @@ rmv_port_callback(void *arg)
 	no_link_check = 1;
 	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.25.0