From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Jan Blunck <jblunck@infradead.org>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v2 13/15] devargs: pass busname argument when parsing
Date: Sat, 15 Jul 2017 16:48:38 +0200 [thread overview]
Message-ID: <20170715144837.GN11154@bidouze.vm.6wind.com> (raw)
In-Reply-To: <20170714211213.34436-14-jblunck@infradead.org>
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@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
next prev parent reply other threads:[~2017-07-15 14:48 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-07-11 23:24 [dpdk-dev] [PATCH 00/13] devargs fixes Jan Blunck
2017-07-11 23:25 ` [dpdk-dev] [PATCH 01/13] Revert "devargs: make device types generic" Jan Blunck
2017-07-11 23:25 ` [dpdk-dev] [PATCH 02/13] devargs: fix unittest Jan Blunck
2017-07-11 23:25 ` [dpdk-dev] [PATCH 03/13] devargs: deprecate enum rte_devtype based functions Jan Blunck
2017-08-07 23:02 ` Thomas Monjalon
2017-07-11 23:25 ` [dpdk-dev] [PATCH 04/13] pci: use scan_mode configuration Jan Blunck
2017-07-13 17:59 ` Gaëtan Rivet
2017-07-13 19:42 ` Jan Blunck
2017-07-13 20:48 ` Thomas Monjalon
2017-07-11 23:25 ` [dpdk-dev] [PATCH 05/13] bus: add configuration interface for buses Jan Blunck
2017-07-11 23:25 ` [dpdk-dev] [PATCH 06/13] devargs: use bus configuration interface to set scanning mode Jan Blunck
2017-07-11 23:25 ` [dpdk-dev] [PATCH 07/13] devargs: add busname string field Jan Blunck
2017-07-13 13:17 ` Gaëtan Rivet
2017-07-11 23:25 ` [dpdk-dev] [PATCH 08/13] devargs: use busname Jan Blunck
2017-07-11 23:25 ` [dpdk-dev] [PATCH 09/13] devargs: parse "bus=" argument Jan Blunck
2017-07-13 13:40 ` Gaëtan Rivet
2017-07-13 19:34 ` Jan Blunck
2017-07-11 23:25 ` [dpdk-dev] [PATCH 10/13] pci: use busname Jan Blunck
2017-07-11 23:25 ` [dpdk-dev] [PATCH 11/13] vdev: " Jan Blunck
2017-07-11 23:25 ` [dpdk-dev] [PATCH 12/13] devargs: remove type field Jan Blunck
2017-07-11 23:25 ` [dpdk-dev] [PATCH 13/13] devargs: remove bus field Jan Blunck
2017-07-12 7:29 ` [dpdk-dev] [PATCH 00/13] devargs fixes Thomas Monjalon
2017-07-12 8:09 ` Jan Blunck
2017-07-12 8:50 ` Thomas Monjalon
2017-07-12 9:25 ` Jan Blunck
2017-07-14 21:11 ` [dpdk-dev] [PATCH v2 00/15] " Jan Blunck
2017-07-14 21:11 ` [dpdk-dev] [PATCH v2 01/15] Revert "devargs: make device types generic" Jan Blunck
2017-09-04 16:05 ` Ferruh Yigit
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 02/15] devargs: fix unittest Jan Blunck
2017-09-04 16:05 ` Ferruh Yigit
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 03/15] devargs: extend unittest Jan Blunck
2017-09-04 16:05 ` Ferruh Yigit
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 04/15] devargs: deprecate enum rte_devtype based functions Jan Blunck
2017-09-04 16:06 ` Ferruh Yigit
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 05/15] pci: use scan_mode configuration Jan Blunck
2017-09-04 16:22 ` Ferruh Yigit
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 06/15] bus: add configuration interface for buses Jan Blunck
2017-09-04 16:23 ` Ferruh Yigit
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 07/15] devargs: use bus configuration interface to set scanning mode Jan Blunck
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 08/15] devargs: use existing functions in rte_eal_devargs_parse() Jan Blunck
2017-09-04 16:24 ` Ferruh Yigit
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 09/15] devargs: add busname string field Jan Blunck
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 10/15] devargs: use busname Jan Blunck
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 11/15] pci: " Jan Blunck
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 12/15] vdev: " Jan Blunck
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 13/15] devargs: pass busname argument when parsing Jan Blunck
2017-07-15 14:48 ` Gaëtan Rivet [this message]
2017-09-04 16:28 ` Ferruh Yigit
2017-09-04 16:28 ` Ferruh Yigit
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 14/15] devargs: remove type field Jan Blunck
2017-09-04 16:29 ` Ferruh Yigit
2017-07-14 21:12 ` [dpdk-dev] [PATCH v2 15/15] devargs: remove bus field Jan Blunck
2017-07-15 18:20 ` [dpdk-dev] [PATCH v2 00/15] devargs fixes Thomas Monjalon
2017-09-04 16:04 ` Ferruh Yigit
2017-09-05 8:20 ` Gaëtan Rivet
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=20170715144837.GN11154@bidouze.vm.6wind.com \
--to=gaetan.rivet@6wind.com \
--cc=dev@dpdk.org \
--cc=jblunck@infradead.org \
/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).