[dpdk-dev] [PATCH v2 13/15] devargs: pass busname argument when parsing

[Jan Blunck]

Let the rte_eal_devargs_parse() function explicitly take a "busname"
argument that is validated.

Now that the busname is known and validated at parse time the validity of
the device name is checked for all device types when they get probed.

Signed-off-by: Jan Blunck <jblunck at infradead.org>
---
 lib/librte_eal/common/eal_common_devargs.c  | 72 +++++++++++------------------
 lib/librte_eal/common/include/rte_devargs.h | 17 ++++---
 test/test/test_devargs.c                    | 21 +++------
 3 files changed, 45 insertions(+), 65 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 38ba64567..44a8d8562 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -77,30 +77,21 @@ rte_eal_parse_devargs_str(const char *devargs_str,
 	return 0;
 }
 
-static int
-bus_name_cmp(const struct rte_bus *bus, const void *name)
-{
-	return strncmp(bus->name, name, strlen(bus->name));
-}
-
 int
-rte_eal_devargs_parse(const char *dev, struct rte_devargs *da)
+rte_eal_devargs_parse(const char *busname, const char *dev,
+	struct rte_devargs *da)
 {
-	struct rte_bus *bus = NULL;
 	char *devname = NULL, *drvargs = NULL;
 	int ret;
 
-	if (dev == NULL || da == NULL)
+	if (busname == NULL || dev == NULL || da == NULL)
 		return -EINVAL;
 	/* Retrieve eventual bus info */
-	do {
-		bus = rte_bus_find(bus, bus_name_cmp, dev);
-		if (bus == NULL)
-			break;
-		dev += strlen(bus->name) + 1;
-		if (rte_bus_find_by_device_name(dev) == bus)
-			break;
-	} while (1);
+	da->bus = rte_bus_find_by_name(busname);
+	if (da->bus == NULL) {
+		RTE_LOG(ERR, EAL, "Bus not found: \"%s\"\n", busname);
+		return -EFAULT;
+	}
 
 	ret = rte_eal_parse_devargs_str(dev, &devname, &drvargs);
 	if (ret != 0)
@@ -118,21 +109,10 @@ rte_eal_devargs_parse(const char *dev, struct rte_devargs *da)
 	da->args = drvargs;
 	drvargs = NULL;
 
-	if (bus == NULL) {
-		bus = rte_bus_find_by_device_name(da->name);
-		if (bus == NULL) {
-			RTE_LOG(ERR, EAL, "Failed to parse device: \"%s\"\n",
-				da->name);
-			ret = -EFAULT;
-			goto fail;
-		}
-	}
-	da->bus = bus;
-
 	/* Store bus name */
-	ret = snprintf(da->busname, sizeof(da->busname), "%s", bus->name);
+	ret = snprintf(da->busname, sizeof(da->busname), "%s", busname);
 	if (ret < 0 || ret >= (int)sizeof(da->busname)) {
-		RTE_LOG(ERR, EAL, "Invalid bus name: \"%s\"\n", bus->name);
+		RTE_LOG(ERR, EAL, "Invalid bus name: \"%s\"\n", busname);
 		ret = -EINVAL;
 		goto fail;
 	}
@@ -158,8 +138,7 @@ int
 rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 {
 	struct rte_devargs *devargs = NULL;
-	struct rte_bus *bus = NULL;
-	const char *dev = devargs_str;
+	const char *busname = NULL;
 	int ret;
 
 	/* use calloc instead of rte_zmalloc as it's called early at init */
@@ -167,31 +146,36 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 	if (devargs == NULL)
 		goto fail;
 
-	if (rte_eal_devargs_parse(dev, devargs))
-		goto fail;
-	devargs->type = devtype;
-	bus = devargs->bus;
-	if (devtype != RTE_DEVTYPE_VIRTUAL) {
-		ret = rte_bus_configure(bus,
+	switch (devtype) {
+	case RTE_DEVTYPE_WHITELISTED_PCI:
+	case RTE_DEVTYPE_BLACKLISTED_PCI:
+		busname = "pci";
+		ret = rte_bus_configure(rte_bus_find_by_name(busname),
 			devtype == RTE_DEVTYPE_WHITELISTED_PCI ?
 			&BUS_CONF_WHITELIST : &BUS_CONF_BLACKLIST);
 		if (ret != 0) {
 			RTE_LOG(ERR, EAL,
 				"Bus [%s] scan_mode conflicts with devtype: "
-				"%s\n", bus->name, devargs_str);
+				"%s\n", busname, devargs_str);
 			goto fail;
 		}
+		break;
+	case RTE_DEVTYPE_VIRTUAL:
+		busname = "vdev";
+		break;
 	}
 
+	if (rte_eal_devargs_parse(busname, devargs_str, devargs))
+		goto fail;
+
+	RTE_LOG(DEBUG, EAL, "Adding devargs for device [%s] on bus [%s]: %s\n",
+		devargs->name, busname, devargs->args);
+
 	TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
 	return 0;
 
 fail:
-	if (devargs) {
-		free(devargs->args);
-		free(devargs);
-	}
-
+	free(devargs);
 	return -1;
 }
 
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 8dafb167e..10953327f 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -124,10 +124,12 @@ int rte_eal_parse_devargs_str(const char *devargs_str,
 /**
  * Parse a device string.
  *
- * Verify that a bus is capable of handling the device passed
- * in argument. Store which bus will handle the device, its name
- * and the eventual device parameters.
+ * The function parses the arguments string to get driver name and driver
+ * arguments and stores together with the busname that will handle the device,
+ * its name and the eventual device parameters.
  *
+ * @param busname
+ *   The bus name the device declaration string applies to.
  * @param dev
  *   The device declaration string.
  * @param da
@@ -138,7 +140,7 @@ int rte_eal_parse_devargs_str(const char *devargs_str,
  *   - Negative errno on error.
  */
 int
-rte_eal_devargs_parse(const char *dev,
+rte_eal_devargs_parse(const char *busname, const char *dev,
 		      struct rte_devargs *da);
 
 /**
@@ -151,9 +153,10 @@ rte_eal_devargs_parse(const char *dev,
  *
  * For virtual devices, the format of arguments string is "DRIVER_NAME*"
  * or "DRIVER_NAME*,key=val,key2=val2,...". Examples: "net_ring",
- * "net_ring0", "net_pmdAnything,arg=0:arg2=1". The validity of the
- * driver name is not checked by this function, it is done when probing
- * the drivers.
+ * "net_ring0", "net_pmdAnything,arg=0:arg2=1".
+ *
+ * The validity of the device name is not checked by this function, it is done
+ * when probing the drivers.
  *
  * @param devtype
  *   The type of the device.
diff --git a/test/test/test_devargs.c b/test/test/test_devargs.c
index 9e0ff5995..84803cf5a 100644
--- a/test/test/test_devargs.c
+++ b/test/test/test_devargs.c
@@ -110,24 +110,17 @@ test_devargs(void)
 		goto fail;
 	free_devargs_list();
 
-	/* test error case: bad PCI address */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "08:1") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "00.1") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "foo") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, ",") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "000f:0:0") == 0)
-		goto fail;
-
-	if (rte_eal_devargs_parse("", &devargs_stack) == 0) {
+	if (rte_eal_devargs_parse("", "", &devargs_stack) == 0) {
 		printf("Error in rte_eal_devargs_parse()\n");
 		goto fail;
 	}
 
-	if (rte_eal_devargs_parse("08:00.1,foo=bar", &devargs_stack) != 0) {
+	if (rte_eal_devargs_parse("vdev", "not_found", &devargs_stack) != 0) {
+		printf("Error in rte_eal_devargs_parse(vdev:not_found)\n");
+		goto fail;
+	}
+	if (rte_eal_devargs_parse("pci", "08:00.1,foo=bar",
+		&devargs_stack) != 0) {
 		printf("Error in rte_eal_devargs_parse(08:00.1,foo=bar)\n");
 		goto fail;
 	}
-- 
2.13.2