From 0654d4a8ad694821e3ce274e87b1ab89fd587f1c Mon Sep 17 00:00:00 2001 From: Thomas Monjalon Date: Thu, 13 Feb 2020 16:52:26 +0100 Subject: [PATCH] app/testpmd: fix hot-unplug detaching 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 Reviewed-by: Ferruh Yigit --- 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 031fe25f39..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,12 +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) { @@ -2872,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); @@ -2882,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.20.1