[dpdk-dev,v2,4/8] eal: fix hotplug add / remove

Message ID 5f9ae5a1d9230c96cf7edeff89cc316523732747.1499728330.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Gaëtan Rivet July 10, 2017, 11:19 p.m. UTC
  New device should be represented by an rte_devargs prior to being
plugged.

Device parameters are available to rte_devices via their devargs field.
This field should be set up as soon as possible, as central information
are stored within, such as the device name which is used to search
the newly scanned device before plugging it in.

When a device is introduced using the hotplug API, it is implicitly
whitelisted. As such, it can conflict with existing bus configuration.
The new rte_devargs uses the new rte_eal_devargs_insert function that
supersedes previous rte_devargs, allowing to force the insertion of new
devices.

Upon device removal, the newly allocated rte_devargs are freed.

Fixes: a3ee360f4440 ("eal: add hotplug add/remove device")

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_dev.c | 57 ++++++++++++++++++++++++++++++++--
 1 file changed, 54 insertions(+), 3 deletions(-)
  

Comments

Thomas Monjalon July 11, 2017, 10:18 p.m. UTC | #1
11/07/2017 01:19, Gaetan Rivet:
> New device should be represented by an rte_devargs prior to being
> plugged.

Why this assumption?
Please start by stated the issue to solve.

> Device parameters are available to rte_devices via their devargs field.
> This field should be set up as soon as possible, as central information
> are stored within, such as the device name which is used to search
> the newly scanned device before plugging it in.
> 
> When a device is introduced using the hotplug API, it is implicitly
> whitelisted. As such, it can conflict with existing bus configuration.
> The new rte_devargs uses the new rte_eal_devargs_insert function that
> supersedes previous rte_devargs, allowing to force the insertion of new
> devices.

I cannot parse this sentence. I probably need to sleep and read it again ;)
  

Patch

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 32e12b5..f5566a6 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -118,11 +118,32 @@  int rte_eal_dev_detach(struct rte_device *dev)
 	return ret;
 }
 
+static char *
+full_dev_name(const char *bus, const char *dev, const char *args)
+{
+	char *name;
+	size_t len;
+
+	len = strlen(bus) + 1 +
+	      strlen(dev) + 1 +
+	      strlen(args) + 1;
+	name = calloc(1, len);
+	if (name == NULL) {
+		RTE_LOG(ERR, EAL, "Could not allocate full device name\n");
+		return NULL;
+	}
+	snprintf(name, len, "%s:%s,%s", bus, dev,
+		 args ? args : "");
+	return name;
+}
+
 int rte_eal_hotplug_add(const char *busname, const char *devname,
 			const char *devargs)
 {
 	struct rte_bus *bus;
 	struct rte_device *dev;
+	struct rte_devargs *da;
+	char *name;
 	int ret;
 
 	bus = rte_bus_find_by_name(busname);
@@ -137,21 +158,49 @@  int rte_eal_hotplug_add(const char *busname, const char *devname,
 		return -ENOTSUP;
 	}
 
+	name = full_dev_name(busname, devname, devargs);
+	if (name == NULL)
+		return -ENOMEM;
+
+	da = calloc(1, sizeof(*da));
+	if (da == NULL) {
+		ret = -ENOMEM;
+		goto err_name;
+	}
+
+	ret = rte_eal_devargs_parse(name, da);
+	if (ret)
+		goto err_devarg;
+
+	ret = rte_eal_devargs_insert(da);
+	if (ret)
+		goto err_devarg;
+
 	ret = bus->scan();
 	if (ret)
-		return ret;
+		goto err_devarg;
 
 	dev = bus->find_device(NULL, cmp_detached_dev_name, devname);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find unplugged device (%s)\n",
 			devname);
-		return -EINVAL;
+		ret = -ENODEV;
+		goto err_devarg;
 	}
 
 	ret = bus->plug(dev, devargs);
-	if (ret)
+	if (ret) {
 		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
 			dev->name);
+		goto err_devarg;
+	}
+	free(name);
+	return 0;
+
+err_devarg:
+	rte_eal_devargs_remove(busname, devname);
+err_name:
+	free(name);
 	return ret;
 }
 
@@ -179,6 +228,8 @@  int rte_eal_hotplug_remove(const char *busname, const char *devname)
 		return -EINVAL;
 	}
 
+	rte_eal_devargs_remove(busname, devname);
+	dev->devargs = NULL;
 	ret = bus->unplug(dev);
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",