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 E28E91BBC8 for ; Fri, 11 May 2018 01:58:53 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 9213C2251F; Thu, 10 May 2018 19:58:53 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Thu, 10 May 2018 19:58:53 -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=2J9FHquI6lwd4E diJ0R/jkTEFuFwbvShHj/DV+9UZ/k=; b=i8U3gbGgwnoMYolgFtiDPxZ8zLqXFF 1Qv7XERWWLz4er5cP6V5IDJSKbR/SJoGzAEB7+Uylottuia6nb3i1bIoG47XrSCz j7zMLObBvcSMxGna5tH8v7qXyxR9nud9Eekt/Gciu1uxZeNd73jVqgkRndSQRjTu BiV8wcJV3eY0M= 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=2J9FHquI6lwd4EdiJ0R/jkTEFuFwbvShHj/DV+9UZ/k=; b=I8NGQ6aF HUkbmaE/i/E/MGw1gO2bUlCKj/FC3wgPhltKX4EiAd3CbTGz9KAsT3xGYzDMkCCT /izs9dOnLcaCYC+0A1EzrDl7lC2O/YAWymjS3AmbPkRmos5e6lqccOvWuI7C4mTv AFT0VS9MzXOy+m6o/6d6pQWIN9oyWadCdRPtd5ZYHYC+/tmP6+MlahRedHULjnfi T+TR2JEJM0WSSTt9WLa4zWkMtHQswCtFymdp53Q20t+n3z3wYt1zdGoqjTYimrbJ nFT3fohmVfAPv4G91fi/5K8h5oyIr7X6OyD6jAI/SDxVmUYvsjaOWb2sdMbZcpN5 ML9DqfFdpRaUbg== 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 A0761E4488; Thu, 10 May 2018 19:58:52 -0400 (EDT) From: Thomas Monjalon To: dev@dpdk.org Cc: matan@mellanox.com, arybchenko@solarflare.com, stephen@networkplumber.org, ferruh.yigit@intel.com Date: Fri, 11 May 2018 01:58:35 +0200 Message-Id: <20180510235836.1099-11-thomas@monjalon.net> X-Mailer: git-send-email 2.16.2 In-Reply-To: <20180510235836.1099-1-thomas@monjalon.net> References: <20180509094337.26112-1-thomas@monjalon.net> <20180510235836.1099-1-thomas@monjalon.net> Subject: [dpdk-dev] [PATCH v3 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: Thu, 10 May 2018 23:58:54 -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 Reviewed-by: Stephen Hemminger --- drivers/net/failsafe/failsafe.c | 20 ++++++++++-- drivers/net/failsafe/failsafe_eal.c | 56 ++++++++++++++++++++++----------- drivers/net/failsafe/failsafe_ether.c | 23 ++++++++++++++ drivers/net/failsafe/failsafe_private.h | 3 ++ 4 files changed, 80 insertions(+), 22 deletions(-) diff --git a/drivers/net/failsafe/failsafe.c b/drivers/net/failsafe/failsafe.c index b35471dd1..eafbb75df 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) { @@ -263,6 +272,9 @@ fs_eth_dev_create(struct rte_vdev_device *vdev) return 0; cancel_alarm: failsafe_hotplug_alarm_cancel(dev); +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: @@ -282,6 +294,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..733e95df6 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]; + uint8_t i; + + FOREACH_SUBDEV_STATE(sdev, i, fs_dev, DEV_PARSED) { + if (sdev->state >= DEV_PROBED) + continue; + if (strcmp(sdev->devargs.name, dev->device->name) != 0) + continue; + rte_eth_dev_owner_set(port_id, &PRIV(fs_dev)->my_owner); + /* The actual owner will be checked after the port probing. */ + break; + } + return 0; +} diff --git a/drivers/net/failsafe/failsafe_private.h b/drivers/net/failsafe/failsafe_private.h index b54f8e336..7e6a3f82a 100644 --- a/drivers/net/failsafe/failsafe_private.h +++ b/drivers/net/failsafe/failsafe_private.h @@ -220,6 +220,9 @@ 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