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

Gaëtan Rivet gaetan.rivet at 6wind.com
Sat Jul 15 16:48:38 CEST 2017


On Fri, Jul 14, 2017 at 05:12:11PM -0400, Jan Blunck wrote:
> 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.
> 

What use is there for a parsing API if the fields have already been
recognized? Why would someone need to parse an rte_devargs if they already
know the busname from the devname? This function becomes useless.

You are applying here the same logic you used with the rte_dev API,
where you divided the fields because it made no sense to have them
merged.

However, the rte_dev API is not in contact with users / applications, it
is not the interface with the outside world where it must take in external
inputs and format them for subsequent systems to use.

> 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
> 

-- 
Gaëtan Rivet
6WIND


More information about the dev mailing list