From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out3-smtp.messagingengine.com (out3-smtp.messagingengine.com [66.111.4.27]) by dpdk.org (Postfix) with ESMTP id 90B41C254; Wed, 9 May 2018 11:43:52 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 3DBB9215FF; Wed, 9 May 2018 05:43:52 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 09 May 2018 05:43:52 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= cc:date:from:in-reply-to:message-id:references:subject:to :x-me-sender:x-me-sender:x-sasl-enc; s=mesmtp; bh=c1ARGzaKXQbR/8 vqBlUHEQRWQt5t5DAd9T1Bjpu45Ik=; b=RrgjT5LqTeUlx52SPjwVZu0DajfSkQ QPrg5rrSl4ZkB73G06n4nl/MsnjCBPTrkoWWm0b3W/IBhBfXL062PvXNQ+MLOf/A CBlCkGIMH/0dg1ghioahlK2/Z0vVIdi1UYtSHOxl+l3bzY4vH8l0llymuCWZtstR Tke45ZQU70lPY= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:date:from:in-reply-to:message-id :references:subject:to:x-me-sender:x-me-sender:x-sasl-enc; s= fm2; bh=c1ARGzaKXQbR/8vqBlUHEQRWQt5t5DAd9T1Bjpu45Ik=; b=eD9yVK/H fU27rg4oFwZIFxXI1uUAMkHXZ27JvwHCUaJeOYsSptNkikYyF2fWbkCTel4OB+MA XGW3mOYNGn8bwlAlkU5jVqcjz8PrVI7HUaq3j7ULnU5PbN8azIP+cKTECRvD7IsC 0lNdjT0wIsKytMjLqqqL3Sjmo86LESFF1S9dMZbbMQ8gnoXL0gIHH2HBRCovTJE0 PQZhomBKF3Q5o8B/T+IurSEAME0e8z8IUHDj9f8RyfCV6MxltHOGBts2CJHlnqwG lSqPNtWe3H2PmX+6gtetazsb8yFeK1QExSDYGQ6lAsiNlPaYERtQOnE9n8n/kH0m S5WdQcSJfqFVwA== X-ME-Sender: Received: from xps.monjalon.net (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 79388E4EEF; Wed, 9 May 2018 05:43:51 -0400 (EDT) From: Thomas Monjalon To: dev@dpdk.org Cc: Matan Azrad , stable@dpdk.org Date: Wed, 9 May 2018 11:43:36 +0200 Message-Id: <20180509094337.26112-11-thomas@monjalon.net> X-Mailer: git-send-email 2.16.2 In-Reply-To: <20180509094337.26112-1-thomas@monjalon.net> References: <20180509094337.26112-1-thomas@monjalon.net> Subject: [dpdk-stable] [PATCH 10/11] net/failsafe: fix sub-device ownership race X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 May 2018 09:43:53 -0000 From: Matan Azrad There is time between the sub-device port probing by the sub-device PMD to the sub-device port ownership taking by a fail-safe port. In this time, the port is available for the application usage. For example, the port will be exposed to the applications which use RTE_ETH_FOREACH_DEV iterator. Thus, ownership unaware applications may manage the port in this time what may cause a lot of problematic behaviors in the fail-safe sub-device initialization. Register to the ethdev NEW event to take the sub-device port ownership before it becomes exposed to the application. Fixes: a46f8d584eb8 ("net/failsafe: add fail-safe PMD") Cc: stable@dpdk.org Signed-off-by: Matan Azrad --- drivers/net/failsafe/failsafe.c | 22 ++++++++++--- drivers/net/failsafe/failsafe_eal.c | 58 +++++++++++++++++++++------------ drivers/net/failsafe/failsafe_ether.c | 23 +++++++++++++ drivers/net/failsafe/failsafe_private.h | 4 +++ 4 files changed, 83 insertions(+), 24 deletions(-) diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c index fc989c4f5..c9d128de3 100644 --- a/drivers/net/failsafe/failsafe.c +++ b/drivers/net/failsafe/failsafe.c @@ -204,16 +204,25 @@ fs_eth_dev_create(struct rte_vdev_device *vdev) } snprintf(priv->my_owner.name, sizeof(priv->my_owner.name), FAILSAFE_OWNER_NAME); + DEBUG("Failsafe port %u owner info: %s_%016"PRIX64, dev->data->port_id, + priv->my_owner.name, priv->my_owner.id); + ret = rte_eth_dev_callback_register(RTE_ETH_ALL, RTE_ETH_EVENT_NEW, + failsafe_eth_new_event_callback, + dev); + if (ret) { + ERROR("Failed to register NEW callback"); + goto free_args; + } ret = failsafe_eal_init(dev); if (ret) - goto free_args; + goto unregister_new_callback; ret = fs_mutex_init(priv); if (ret) - goto free_args; + goto unregister_new_callback; ret = failsafe_hotplug_alarm_install(dev); if (ret) { ERROR("Could not set up plug-in event detection"); - goto free_args; + goto unregister_new_callback; } mac = &dev->data->mac_addrs[0]; if (mac_from_arg) { @@ -226,7 +235,7 @@ fs_eth_dev_create(struct rte_vdev_device *vdev) mac); if (ret) { ERROR("Failed to set default MAC address"); - goto free_args; + goto unregister_new_callback; } } } else { @@ -261,6 +270,9 @@ fs_eth_dev_create(struct rte_vdev_device *vdev) }; rte_eth_dev_probing_finish(dev); return 0; +unregister_new_callback: + rte_eth_dev_callback_unregister(RTE_ETH_ALL, RTE_ETH_EVENT_NEW, + failsafe_eth_new_event_callback, dev); free_args: failsafe_args_free(dev); free_subs: @@ -280,6 +292,8 @@ fs_rte_eth_free(const char *name) dev = rte_eth_dev_allocated(name); if (dev == NULL) return -ENODEV; + rte_eth_dev_callback_unregister(RTE_ETH_ALL, RTE_ETH_EVENT_NEW, + failsafe_eth_new_event_callback, dev); ret = failsafe_eal_uninit(dev); if (ret) ERROR("Error while uninitializing sub-EAL"); diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c index ce767703f..8f1b9d845 100644 --- a/drivers/net/failsafe/failsafe_eal.c +++ b/drivers/net/failsafe/failsafe_eal.c @@ -10,7 +10,7 @@ static int fs_ethdev_portid_get(const char *name, uint16_t *port_id) { - uint16_t pid; + uint32_t pid; size_t len; if (name == NULL) { @@ -18,8 +18,9 @@ fs_ethdev_portid_get(const char *name, uint16_t *port_id) return -EINVAL; } len = strlen(name); - RTE_ETH_FOREACH_DEV(pid) { - if (!strncmp(name, rte_eth_devices[pid].device->name, len)) { + for (pid = 0; pid < RTE_MAX_ETHPORTS; pid++) { + if (rte_eth_dev_is_valid_port(pid) && + !strncmp(name, rte_eth_devices[pid].device->name, len)) { *port_id = pid; return 0; } @@ -41,6 +42,8 @@ fs_bus_init(struct rte_eth_dev *dev) continue; da = &sdev->devargs; if (fs_ethdev_portid_get(da->name, &pid) != 0) { + struct rte_eth_dev_owner pid_owner; + ret = rte_eal_hotplug_add(da->bus->name, da->name, da->args); @@ -55,12 +58,26 @@ fs_bus_init(struct rte_eth_dev *dev) ERROR("sub_device %d init went wrong", i); return -ENODEV; } + /* + * The NEW callback tried to take ownership, check + * whether it succeed or didn't. + */ + rte_eth_dev_owner_get(pid, &pid_owner); + if (pid_owner.id != PRIV(dev)->my_owner.id) { + INFO("sub_device %d owner(%s_%016"PRIX64") is not my," + " owner(%s_%016"PRIX64"), will try again later", + i, pid_owner.name, pid_owner.id, + PRIV(dev)->my_owner.name, + PRIV(dev)->my_owner.id); + continue; + } } else { + /* The sub-device port was found. */ char devstr[DEVARGS_MAXLEN] = ""; struct rte_devargs *probed_da = rte_eth_devices[pid].device->devargs; - /* Take control of device probed by EAL options. */ + /* Take control of probed device. */ free(da->args); memset(da, 0, sizeof(*da)); if (probed_da != NULL) @@ -77,22 +94,23 @@ fs_bus_init(struct rte_eth_dev *dev) } INFO("Taking control of a probed sub device" " %d named %s", i, da->name); - } - ret = rte_eth_dev_owner_set(pid, &PRIV(dev)->my_owner); - if (ret < 0) { - INFO("sub_device %d owner set failed (%s)," - " will try again later", i, strerror(-ret)); - continue; - } else if (strncmp(rte_eth_devices[pid].device->name, da->name, - strlen(da->name)) != 0) { - /* - * The device probably was removed and its port id was - * reallocated before ownership set. - */ - rte_eth_dev_owner_unset(pid, PRIV(dev)->my_owner.id); - INFO("sub_device %d was probably removed before taking" - " ownership, will try again later", i); - continue; + ret = rte_eth_dev_owner_set(pid, &PRIV(dev)->my_owner); + if (ret < 0) { + INFO("sub_device %d owner set failed (%s), " + "will try again later", i, strerror(-ret)); + continue; + } else if (strncmp(rte_eth_devices[pid].device->name, + da->name, strlen(da->name)) != 0) { + /* + * The device probably was removed and its port + * id was reallocated before ownership set. + */ + rte_eth_dev_owner_unset(pid, + PRIV(dev)->my_owner.id); + INFO("sub_device %d was removed before taking" + " ownership, will try again later", i); + continue; + } } ETH(sdev) = &rte_eth_devices[pid]; SUB_ID(sdev) = i; diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c index b414a7884..ebce87841 100644 --- a/drivers/net/failsafe/failsafe_ether.c +++ b/drivers/net/failsafe/failsafe_ether.c @@ -463,3 +463,26 @@ failsafe_eth_lsc_event_callback(uint16_t port_id __rte_unused, else return 0; } + +/* Take sub-device ownership before it becomes exposed to the application. */ +int +failsafe_eth_new_event_callback(uint16_t port_id, + enum rte_eth_event_type event __rte_unused, + void *cb_arg, void *out __rte_unused) +{ + struct rte_eth_dev *fs_dev = cb_arg; + struct sub_device *sdev; + struct rte_eth_dev *dev = &rte_eth_devices[port_id]; + size_t len = strlen(dev->device->name); + uint8_t i; + + FOREACH_SUBDEV_STATE(sdev, i, fs_dev, DEV_PARSED) { + if (sdev->state >= DEV_PROBED) + continue; + if (strncmp(sdev->devargs.name, dev->device->name, len) != 0) + continue; + rte_eth_dev_owner_set(port_id, &PRIV(fs_dev)->my_owner); + break; + } + return 0; +} diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h index b54f8e336..cd8e0b8c9 100644 --- a/drivers/net/failsafe/failsafe_private.h +++ b/drivers/net/failsafe/failsafe_private.h @@ -220,6 +220,10 @@ int failsafe_eth_rmv_event_callback(uint16_t port_id, int failsafe_eth_lsc_event_callback(uint16_t port_id, enum rte_eth_event_type event, void *cb_arg, void *out); +int +failsafe_eth_new_event_callback(uint16_t port_id, + enum rte_eth_event_type event, + void *cb_arg, void *out); /* GLOBALS */ -- 2.16.2