From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id E64BBA0542; Thu, 13 Feb 2020 16:52:53 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9D2091C08C; Thu, 13 Feb 2020 16:52:38 +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: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgedugedrieekgdekvdcutefuodetggdotefrodftvf curfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdpuffrtefokffrpgfnqfghnecu uegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivghnthhsucdlqddutddtmdenuc fjughrpefhvffufffkofgjfhgggfestdekredtredttdenucfhrhhomhepvfhhohhmrghs ucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtqeenucfkph epjeejrddufeegrddvtdefrddukeegnecuvehluhhsthgvrhfuihiivgeptdenucfrrghr rghmpehmrghilhhfrhhomhepthhhohhmrghssehmohhnjhgrlhhonhdrnhgvth X-ME-Proxy: 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 To: dev@dpdk.org Cc: matan@mellanox.com, ferruh.yigit@intel.com, stable@dpdk.org, Wenzhuo Lu , Jingjing Wu , Bernard Iremonger 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-dev] [PATCH 2/2] app/testpmd: fix hot-unplug detaching X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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 --- 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