From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 398EF1B7BE for ; Thu, 10 May 2018 00:43:29 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 702502299B; Wed, 9 May 2018 18:43:28 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Wed, 09 May 2018 18:43:28 -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=y/PWM3kDGYguIz mwyr103gFXhbc381Hfyu55ec/9hJo=; b=Bdu7fuglYf4QDBCGtYjnNozIwCBqgY HGUAq4bNHnV5U6YIGs44RyUyebV57Uut8TBaXVF4gDVz4hi+DqWuhKqEEyC5hsaQ W6x3JKI19vwS690keQMugytysxelB0jWGFxidq+37XNHnvEF4EKrITwwKDIaI7pt Nn2lZNIReD6yE= 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=y/PWM3kDGYguIzmwyr103gFXhbc381Hfyu55ec/9hJo=; b=cp6wjyLF J3LGgv710fyBDyrLoNz2jdGbXsf+a29BQt65nP88OeedFA/YHCvVcvWXWMoFghXQ CBGpL33oX9mpjY8F93VQvfhgHdHjtbU7STWwmcLrrFL/Fvj5faPMg6lZug3X4i8g UyeXqxvP5KmuSV+gFqwfV/2rOzmzbNIg0w5a5J24ghNzdhNP/1mp9QP/OwE34tr5 xo9ImUDUnhLavjYgwuiY959Yq/lBX35ybTUI8XY5J8AjPX5ng5O8fzd4ROZN6U/A 2ZDcUzNKBUYI9k9D6Y3Ir5cXV++Vh0qXfCXntb/R7+WXhpDVvGRXT9OvkmnLSzyg PtKcdNu8Hngrqw== 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 CDF64E5094; Wed, 9 May 2018 18:43:27 -0400 (EDT) From: Thomas Monjalon To: dev@dpdk.org Cc: Matan Azrad Date: Thu, 10 May 2018 00:43:12 +0200 Message-Id: <20180509224313.27289-11-thomas@monjalon.net> X-Mailer: git-send-email 2.16.2 In-Reply-To: <20180509224313.27289-1-thomas@monjalon.net> References: <20180509094337.26112-1-thomas@monjalon.net> <20180509224313.27289-1-thomas@monjalon.net> Subject: [dpdk-dev] [PATCH v2 10/11] net/failsafe: fix sub-device ownership race 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: , X-List-Received-Date: Wed, 09 May 2018 22:43:29 -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 Acked-by: Gaetan Rivet --- drivers/net/failsafe/failsafe.c | 22 ++++++++++--- drivers/net/failsafe/failsafe_eal.c | 56 ++++++++++++++++++++++----------- drivers/net/failsafe/failsafe_ether.c | 23 ++++++++++++++ drivers/net/failsafe/failsafe_private.h | 4 +++ 4 files changed, 82 insertions(+), 23 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..5672f3961 100644 --- a/drivers/net/failsafe/failsafe_eal.c +++ b/drivers/net/failsafe/failsafe_eal.c @@ -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