From patchwork Mon Nov 5 07:04:45 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Stojaczyk, Dariusz" X-Patchwork-Id: 47799 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 6CBD54CE4; Mon, 5 Nov 2018 08:08:15 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 69AD64CA2 for ; Mon, 5 Nov 2018 08:08:13 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Nov 2018 23:08:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,467,1534834800"; d="scan'208";a="105308426" Received: from violet.igk.intel.com ([10.102.54.137]) by fmsmga001.fm.intel.com with ESMTP; 04 Nov 2018 23:08:08 -0800 From: Darek Stojaczyk To: dev@dpdk.org Cc: thomas@monjalon.net, Darek Stojaczyk , qi.z.zhang@intel.com Date: Mon, 5 Nov 2018 08:04:45 +0100 Message-Id: <20181105070447.67700-1-dariusz.stojaczyk@intel.com> X-Mailer: git-send-email 2.17.1 Subject: [dpdk-dev] [PATCH 1/3] bus/pci: update device devargs on each rescan 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" Bus rescan is done e.g. during the device hotplug, where devargs are re-allocated. By not updating the rte_device->devargs pointer we potentially make it a dangling one, as previous devargs could have been (or will be soon) freed. Fixes: 55e411b301c3 ("bus/pci: fix resource mapping override") Cc: qi.z.zhang@intel.com Signed-off-by: Darek Stojaczyk --- drivers/bus/pci/linux/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c index f87533c5c..f3172960e 100644 --- a/drivers/bus/pci/linux/pci.c +++ b/drivers/bus/pci/linux/pci.c @@ -349,10 +349,10 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr) if (ret < 0) { rte_pci_insert_device(dev2, dev); } else { /* already registered */ + pci_name_set(dev2); if (!rte_dev_is_probed(&dev2->device)) { dev2->kdrv = dev->kdrv; dev2->max_vfs = dev->max_vfs; - pci_name_set(dev2); memmove(dev2->mem_resource, dev->mem_resource, sizeof(dev->mem_resource)); From patchwork Mon Nov 5 07:04:46 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Stojaczyk, Dariusz" X-Patchwork-Id: 47800 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 240904F9A; Mon, 5 Nov 2018 08:08:17 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id 0C3254CA2 for ; Mon, 5 Nov 2018 08:08:13 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Nov 2018 23:08:12 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,467,1534834800"; d="scan'208";a="105308432" Received: from violet.igk.intel.com ([10.102.54.137]) by fmsmga001.fm.intel.com with ESMTP; 04 Nov 2018 23:08:11 -0800 From: Darek Stojaczyk To: dev@dpdk.org Cc: thomas@monjalon.net, Darek Stojaczyk Date: Mon, 5 Nov 2018 08:04:46 +0100 Message-Id: <20181105070447.67700-2-dariusz.stojaczyk@intel.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181105070447.67700-1-dariusz.stojaczyk@intel.com> References: <20181105070447.67700-1-dariusz.stojaczyk@intel.com> Subject: [dpdk-dev] [PATCH 2/3] devargs: delay freeing previous devargs when overriding them 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" In eal hotplug path, the previous devargs may be still referenced by device structs at the time the rte_devargs_insert() is called. Those references are updated almost immediately afterwards, but in cases something goes wrong and they cannot be updated, we might want to still keep the old devargs around. This patch modifies rte_devargs_insert() so that it returns a pointer to previous devargs that are being overridden. In case something in the EAL hotplug path goes wrong, we can now remove the newly inserted devargs and re-insert the old ones. Note: Functional changes will come in a subsequent patch. This one only extends the API. Signed-off-by: Darek Stojaczyk --- drivers/bus/vdev/vdev.c | 12 +++++++++--- lib/librte_eal/common/eal_common_dev.c | 11 +++++++---- lib/librte_eal/common/eal_common_devargs.c | 21 ++++++++++++++------- lib/librte_eal/common/include/rte_devargs.h | 8 ++------ 4 files changed, 32 insertions(+), 20 deletions(-) diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c index 9c66bdc78..bbdae2314 100644 --- a/drivers/bus/vdev/vdev.c +++ b/drivers/bus/vdev/vdev.c @@ -9,6 +9,7 @@ #include #include #include +#include #include #include @@ -207,7 +208,7 @@ insert_vdev(const char *name, const char *args, bool init) { struct rte_vdev_device *dev; - struct rte_devargs *devargs; + struct rte_devargs *devargs, *prev_devargs; int ret; if (name == NULL) @@ -239,8 +240,13 @@ insert_vdev(const char *name, const char *args, } TAILQ_INSERT_TAIL(&vdev_device_list, dev, next); - if (init) - rte_devargs_insert(devargs); + if (init) { + rte_devargs_insert(devargs, &prev_devargs); + /* any previous devargs should have been caught by the above + * find_vdev(name) check + */ + assert(prev_devargs == NULL); + } if (p_dev) *p_dev = dev; diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c index 62e9ed477..4cb424df1 100644 --- a/lib/librte_eal/common/eal_common_dev.c +++ b/lib/librte_eal/common/eal_common_dev.c @@ -131,7 +131,7 @@ int local_dev_probe(const char *devargs, struct rte_device **new_dev) { struct rte_device *dev; - struct rte_devargs *da; + struct rte_devargs *da, *prev_da; int ret; *new_dev = NULL; @@ -150,9 +150,12 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev) goto err_devarg; } - ret = rte_devargs_insert(da); - if (ret) - goto err_devarg; + rte_devargs_insert(da, &prev_da); + + if (prev_da != NULL) { + free(prev_da->args); + free(prev_da); + } ret = da->bus->scan(); if (ret) diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c index b7b9cb69e..c46365b69 100644 --- a/lib/librte_eal/common/eal_common_devargs.c +++ b/lib/librte_eal/common/eal_common_devargs.c @@ -262,16 +262,23 @@ rte_devargs_parsef(struct rte_devargs *da, const char *format, ...) return ret; } -int __rte_experimental -rte_devargs_insert(struct rte_devargs *da) +void __rte_experimental +rte_devargs_insert(struct rte_devargs *da, struct rte_devargs **prev_da) { - int ret; + struct rte_devargs *d; + void *tmp; + + *prev_da = NULL; + TAILQ_FOREACH_SAFE(d, &devargs_list, next, tmp) { + if (strcmp(d->bus->name, da->bus->name) == 0 && + strcmp(d->name, da->name) == 0) { + TAILQ_REMOVE(&devargs_list, d, next); + *prev_da = d; + break; + } + } - ret = rte_devargs_remove(da); - if (ret < 0) - return ret; TAILQ_INSERT_TAIL(&devargs_list, da, next); - return 0; } /* store a whitelist parameter for later parsing */ diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h index b1f121f83..753cf7f54 100644 --- a/lib/librte_eal/common/include/rte_devargs.h +++ b/lib/librte_eal/common/include/rte_devargs.h @@ -146,14 +146,10 @@ __attribute__((format(printf, 2, 0))); * * @param da * The devargs structure to insert. - * - * @return - * - 0 on success - * - Negative on error. */ __rte_experimental -int -rte_devargs_insert(struct rte_devargs *da); +void +rte_devargs_insert(struct rte_devargs *da, struct rte_devargs **prev_da); /** * Add a device to the user device list From patchwork Mon Nov 5 07:04:47 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: "Stojaczyk, Dariusz" X-Patchwork-Id: 47801 X-Patchwork-Delegate: thomas@monjalon.net Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id B14FE5424; Mon, 5 Nov 2018 08:08:18 +0100 (CET) Received: from mga12.intel.com (mga12.intel.com [192.55.52.136]) by dpdk.org (Postfix) with ESMTP id C7D394CA2 for ; Mon, 5 Nov 2018 08:08:14 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga106.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 04 Nov 2018 23:08:13 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,467,1534834800"; d="scan'208";a="105308435" Received: from violet.igk.intel.com ([10.102.54.137]) by fmsmga001.fm.intel.com with ESMTP; 04 Nov 2018 23:08:12 -0800 From: Darek Stojaczyk To: dev@dpdk.org Cc: thomas@monjalon.net, Darek Stojaczyk , gaetan.rivet@6wind.com, qi.z.zhang@intel.com Date: Mon, 5 Nov 2018 08:04:47 +0100 Message-Id: <20181105070447.67700-3-dariusz.stojaczyk@intel.com> X-Mailer: git-send-email 2.17.1 In-Reply-To: <20181105070447.67700-1-dariusz.stojaczyk@intel.com> References: <20181105070447.67700-1-dariusz.stojaczyk@intel.com> Subject: [dpdk-dev] [PATCH 3/3] eal: handle bus rescan failures during hotplug 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" Fixes for devargs dangling pointers and various segfaults caused by failed hotplug requests. If rescan failed, we have one the following scenarios to cover: a) the device was never scanned at all; the devargs that were just allocated are not referenced anywhere and can be removed straight away. Previous devargs (if any) need to be re-inserted as they are still in use. b) the device was scanned before, but the rescan failed before it reached our device, meaning that its devargs were not overridden and we can remove our newly allocated ones. Previous devargs need to be re-inserted. c) we managed to rescan our device and only failed on something after that, which means we have to free the memory of previous devargs and stick with the new ones, despite the fact that the hotplug still needs to be failed. Fixes: 7e8b26650146 ("eal: fix hotplug add / remove") Cc: gaetan.rivet@6wind.com Cc: qi.z.zhang@intel.com Signed-off-by: Darek Stojaczyk --- lib/librte_eal/common/eal_common_dev.c | 30 ++++++++++++++++++-------- 1 file changed, 21 insertions(+), 9 deletions(-) diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c index 4cb424df1..b090295b9 100644 --- a/lib/librte_eal/common/eal_common_dev.c +++ b/lib/librte_eal/common/eal_common_dev.c @@ -7,6 +7,7 @@ #include #include #include +#include #include #include @@ -141,32 +142,33 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev) ret = rte_devargs_parse(da, devargs); if (ret) - goto err_devarg; + goto err_remove_devargs; if (da->bus->plug == NULL) { RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n", da->bus->name); ret = -ENOTSUP; - goto err_devarg; + goto err_remove_devargs; } rte_devargs_insert(da, &prev_da); + ret = da->bus->scan(); + if (ret) + goto err_rollback_devargs; + if (prev_da != NULL) { free(prev_da->args); free(prev_da); + prev_da = NULL; } - ret = da->bus->scan(); - if (ret) - goto err_devarg; - dev = da->bus->find_device(NULL, cmp_dev_name, da->name); if (dev == NULL) { RTE_LOG(ERR, EAL, "Cannot find device (%s)\n", da->name); ret = -ENODEV; - goto err_devarg; + goto err_remove_devargs; } ret = dev->bus->plug(dev); @@ -175,13 +177,23 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev) return ret; /* no rollback */ RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", dev->name); - goto err_devarg; + goto err_remove_devargs; } *new_dev = dev; return 0; -err_devarg: +err_rollback_devargs: + dev = da->bus->find_device(NULL, cmp_dev_name, da->name); + if (prev_da != NULL && (dev == NULL || dev->devargs != da)) { + /* either the device wasn't scanned, or we didn't + * manage yet to update its devargs. + */ + rte_devargs_insert(prev_da, &da); + assert(da != NULL); + } + +err_remove_devargs: if (rte_devargs_remove(da) != 0) { free(da->args); free(da);