DPDK patches and discussions
 help / color / mirror / Atom feed
From: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
To: dev@dpdk.org
Cc: thomas@monjalon.net,
	Darek Stojaczyk <dariusz.stojaczyk@intel.com>,
	gaetan.rivet@6wind.com, qi.z.zhang@intel.com
Subject: [dpdk-dev] [PATCH 3/3] eal: handle bus rescan failures during hotplug
Date: Mon,  5 Nov 2018 08:04:47 +0100	[thread overview]
Message-ID: <20181105070447.67700-3-dariusz.stojaczyk@intel.com> (raw)
In-Reply-To: <20181105070447.67700-1-dariusz.stojaczyk@intel.com>

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 <dariusz.stojaczyk@intel.com>
---
 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 <string.h>
 #include <inttypes.h>
 #include <sys/queue.h>
+#include <assert.h>
 
 #include <rte_compat.h>
 #include <rte_bus.h>
@@ -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);
-- 
2.17.1

  parent reply	other threads:[~2018-11-05  7:08 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-11-05  7:04 [dpdk-dev] [PATCH 1/3] bus/pci: update device devargs on each rescan Darek Stojaczyk
2018-11-05  7:04 ` [dpdk-dev] [PATCH 2/3] devargs: delay freeing previous devargs when overriding them Darek Stojaczyk
2018-11-05  7:30   ` Thomas Monjalon
2018-11-05  8:25     ` Stojaczyk, Dariusz
2018-11-05  9:46       ` Thomas Monjalon
2018-11-05 16:24         ` Gaëtan Rivet
2018-11-05  7:04 ` Darek Stojaczyk [this message]
2018-11-05 14:10 ` [dpdk-dev] [PATCH 1/3] bus/pci: update device devargs on each rescan Gaëtan Rivet
2018-11-05 14:52   ` Stojaczyk, Dariusz
2018-11-05 16:27     ` Gaëtan Rivet
2018-11-06  5:40 ` [dpdk-dev] [PATCH v2] " Dariusz Stojaczyk
2018-11-06 22:21   ` Zhang, Qi Z
2018-11-06 23:40     ` Gaëtan Rivet
2018-11-12  0:47       ` Thomas Monjalon

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181105070447.67700-3-dariusz.stojaczyk@intel.com \
    --to=dariusz.stojaczyk@intel.com \
    --cc=dev@dpdk.org \
    --cc=gaetan.rivet@6wind.com \
    --cc=qi.z.zhang@intel.com \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).