From: "Gaëtan Rivet" <gaetan.rivet@6wind.com>
To: Shreyansh Jain <shreyansh.jain@nxp.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v7 6/6] devargs: parse bus info
Date: Thu, 6 Jul 2017 12:00:21 +0200 [thread overview]
Message-ID: <20170706100021.GJ11154@bidouze.vm.6wind.com> (raw)
In-Reply-To: <39810ff7-d5fd-921b-4fad-81a8013bf1c4@nxp.com>
On Thu, Jul 06, 2017 at 02:37:16PM +0530, Shreyansh Jain wrote:
> Hello Gaetan,
>
> On Wednesday 05 July 2017 05:25 AM, Gaetan Rivet wrote:
> >Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> >---
> > lib/librte_eal/common/eal_common_devargs.c | 42 ++++++++++++++++++++++++-----
> > lib/librte_eal/common/eal_common_vdev.c | 6 +++--
> > lib/librte_eal/common/include/rte_devargs.h | 3 +++
> > 3 files changed, 42 insertions(+), 9 deletions(-)
> >
> >diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> >index ffa8ad9..102bd8d 100644
> >--- a/lib/librte_eal/common/eal_common_devargs.c
> >+++ b/lib/librte_eal/common/eal_common_devargs.c
> >@@ -78,12 +78,23 @@ rte_eal_parse_devargs_str(const char *devargs_str,
> > return 0;
> > }
> >+static int
> >+bus_name_cmp(const struct rte_bus *bus, const void *_name)
> >+{
> >+ const char *name = _name;
> >+
> >+ return strncmp(bus->name, name,
> >+ strlen(bus->name));
>
> Trivial: Any specific reason why this is split across multiple lines even
> though it is less than 80 chars in total?
>
Not really, it's only a matter of taste.
If is mostly to hightlight that we are limiting the comparison to
bus->name length, by putting it on its own line.
> >+}
> >+
> > /* store a whitelist parameter for later parsing */
> > int
> > rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
> > {
> > struct rte_devargs *devargs = NULL;
> >- char *buf = NULL;
> >+ struct rte_bus *bus = NULL;
> >+ char *dev = NULL;
> >+ const char *devname;
> > int ret;
> > /* use malloc instead of rte_malloc as it's called early at init */
> >@@ -94,34 +105,51 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
> > memset(devargs, 0, sizeof(*devargs));
> > devargs->type = devtype;
> >- if (rte_eal_parse_devargs_str(devargs_str, &buf, &devargs->args))
> >+ if (rte_eal_parse_devargs_str(devargs_str, &dev, &devargs->args))
>
> A very basic (and possibly incorrect) question:
>
> here, for example "'net_pcap0,rx_pcap=input.pcap,tx_pcap=output.pcap'" was
> passed to application, which would mean;
>
> dev = "net_pcap0" (name of the device)
> devargs = "rx_pcap=input.pcap,tx_pcap=output.pcap"
>
> > goto fail;
> >+ devname = dev;
> >+ do {
> >+ bus = rte_bus_find(bus, bus_name_cmp, dev);
> >+ if (bus == NULL)
> >+ break;
> >+ devname = dev + strlen(bus->name) + 1;
>
> Assuming "vdev" bus:
> "net_pcap0"
> |
> devname points here --------' (4+1) chars from start of "net_pcap0".
> Is that the expectation?
>
Yes :)
Well, actually, almost. The lines
> >+ bus = rte_bus_find(bus, bus_name_cmp, dev);
> >+ if (bus == NULL)
> >+ break;
Mean that at this point, we would already have broken out of the loop.
But to continue with your example, assuming that we have a bus that
is named "net_p":
> Probably I am missing something here (maybe the input already has a bus
> name.)
>
> Or, if this is not the case, then we will have to change the test
> application (test_devargs.c) which passes such strings to
> "rte_eal_devargs_add".
>
> >+ if (rte_bus_find_by_device_name(devname) == bus)
>
> Obviously, if my assumption is correct, this fails. Or, maybe I am
> completely off the mark.
>
It should not be necessary to update the tests (yet).
This bit of code tries to recognize bus names at the start of the
declaration. However, some device names might be ambiguous and start
like bus names for any reason. Device names have no specification beside
not having commas within, bus names have no specification at all.
Thus, we first try here to recognize a bus name within dev, but we do
not stop here. We also verify afterward that the resulting device would
be correct, and that its bus handler is actually the same as the bus we
first recognized.
In your example, the line
> >+ if (rte_bus_find_by_device_name(devname) == bus)
Fails, as the device is incorrect and rte_bus_find_by_device_name
returns NULL as no bus is able to handle a device named
> "cap0,rx_pcap=input.pcap,tx_pcap=output.pcap"
Considering this, we should break from this loop with no recognized bus.
As such, we enter afterward in the branch:
> >+ break;
> >+ devname = dev;
Note here that devname is set back to the start of dev.
> >+ } while (1);
> >+ if (bus == NULL) {
> >+ bus = rte_bus_find_by_device_name(devname);
Which will try to recognize a bus from the device name ("net_pcap0"),
which should succeed in recognizing vdev.
It is a little contrived, but I wanted this parsing to both be flexible
and perform as many checks as possible.
I am developing a new bus that have a high probability of name conflicts
with other device names, so I am extra careful on this side.
> >+ if (bus == NULL) {
> >+ fprintf(stderr, "ERROR: failed to parse bus info from device declaration\n");
> >+ goto fail;
> >+ }
> >+ }
> >+ devargs->bus = bus;
> > switch (devargs->type) {
> > case RTE_DEVTYPE_WHITELISTED_PCI:
> > case RTE_DEVTYPE_BLACKLISTED_PCI:
> > /* try to parse pci identifier */
> >- if (eal_parse_pci_BDF(buf, &devargs->pci.addr) != 0 &&
> >- eal_parse_pci_DomBDF(buf, &devargs->pci.addr) != 0)
> >+ if (bus->parse(devname, &devargs->pci.addr) != 0)
> > goto fail;
> > break;
> > case RTE_DEVTYPE_VIRTUAL:
> > /* save driver name */
> > ret = snprintf(devargs->virt.drv_name,
> >- sizeof(devargs->virt.drv_name), "%s", buf);
> >+ sizeof(devargs->virt.drv_name), "%s", devname);
> > if (ret < 0 || ret >= (int)sizeof(devargs->virt.drv_name))
> > goto fail;
> > break;
> > }
>
> I think all this will change in the devargs series.
>
Indeed :)
> >- free(buf);
> >+ free(dev);
> > TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
> > return 0;
> > fail:
> >- free(buf);
> >+ free(dev);
> > if (devargs) {
> > free(devargs->args);
> > free(devargs);
> >diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
> >index 6ecd1b5..8fd1ef7 100644
> >--- a/lib/librte_eal/common/eal_common_vdev.c
> >+++ b/lib/librte_eal/common/eal_common_vdev.c
> >@@ -177,6 +177,7 @@ alloc_devargs(const char *name, const char *args)
> > return NULL;
> > devargs->type = RTE_DEVTYPE_VIRTUAL;
> >+ devargs->bus = rte_bus_find_by_name("vdev");
> > if (args)
> > devargs->args = strdup(args);
> >@@ -289,12 +290,13 @@ vdev_scan(void)
> > {
> > struct rte_vdev_device *dev;
> > struct rte_devargs *devargs;
> >+ struct rte_bus *vbus;
> > /* for virtual devices we scan the devargs_list populated via cmdline */
> >-
> >+ vbus = rte_bus_find_by_name("vdev");
> > TAILQ_FOREACH(devargs, &devargs_list, next) {
> >- if (devargs->type != RTE_DEVTYPE_VIRTUAL)
> >+ if (devargs->bus != vbus)
> > continue;
> > dev = find_vdev(devargs->virt.drv_name);
> >diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> >index 88120a1..1f50a24 100644
> >--- a/lib/librte_eal/common/include/rte_devargs.h
> >+++ b/lib/librte_eal/common/include/rte_devargs.h
> >@@ -51,6 +51,7 @@ extern "C" {
> > #include <stdio.h>
> > #include <sys/queue.h>
> > #include <rte_pci.h>
> >+#include <rte_bus.h>
> > /**
> > * Type of generic device
> >@@ -89,6 +90,8 @@ struct rte_devargs {
> > char drv_name[32];
> > } virt;
> > };
> >+ /** Bus handle for the device. */
> >+ struct rte_bus *bus;
> > /** Arguments string as given by user or "" for no argument. */
> > char *args;
> > };
> >
> -
> Shreyansh
--
Gaëtan Rivet
6WIND
next prev parent reply other threads:[~2017-07-06 10:00 UTC|newest]
Thread overview: 123+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-24 15:12 [dpdk-dev] [PATCH 0/9] rte_bus parse API Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 1/9] bus: fix bus name registration Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 2/9] bus: verify bus name on registration Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 3/9] bus: introduce parsing functionality Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 4/9] vdev: implement parse bus operation Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 5/9] pci: " Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 6/9] bus: add helper to find bus from a name Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 7/9] bus: add helper to find a bus from a device name Gaetan Rivet
2017-06-07 17:28 ` Jan Blunck
2017-06-07 20:03 ` Gaëtan Rivet
2017-06-08 10:45 ` Jan Blunck
2017-06-08 11:36 ` Gaëtan Rivet
2017-06-08 11:40 ` Jan Blunck
2017-06-08 12:51 ` Gaëtan Rivet
2017-06-10 8:50 ` Jan Blunck
2017-06-12 14:21 ` Gaëtan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 8/9] vdev: expose bus name Gaetan Rivet
2017-05-24 15:12 ` [dpdk-dev] [PATCH 9/9] devargs: parse bus info Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 0/9] rte_bus parse API Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 1/9] bus: fix bus name registration Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 2/9] bus: verify bus name on registration Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 3/9] bus: introduce parsing functionality Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 4/9] vdev: implement parse bus operation Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 5/9] pci: " Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 6/9] bus: add helper to find bus from a name Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 7/9] bus: add helper to find a bus from a device name Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 8/9] vdev: expose bus name Gaetan Rivet
2017-05-24 16:15 ` [dpdk-dev] [PATCH v2 9/9] devargs: parse bus info Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 0/9] rte_bus parse API Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 1/9] bus: fix bus name registration Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 2/9] bus: verify bus name on registration Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 3/9] bus: introduce parsing functionality Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 4/9] vdev: implement parse bus operation Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 5/9] pci: " Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 6/9] bus: add helper to find bus from a name Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 7/9] bus: add helper to find a bus from a device name Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 8/9] vdev: expose bus name Gaetan Rivet
2017-06-01 10:08 ` [dpdk-dev] [PATCH v3 9/9] devargs: parse bus info Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 0/9] rte_bus parse API Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 1/9] bus: fix bus name registration Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 2/9] bus: verify bus name on registration Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 3/9] bus: introduce parsing functionality Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 4/9] vdev: implement parse bus operation Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 5/9] pci: " Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 6/9] bus: add helper to find a bus from a bus name Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 7/9] bus: add helper to find a bus from a device name Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 8/9] vdev: expose bus name Gaetan Rivet
2017-06-07 23:56 ` [dpdk-dev] [PATCH v4 9/9] devargs: parse bus info Gaetan Rivet
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 0/7] rte_bus parse API Gaetan Rivet
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 1/7] bus: fix bus name registration Gaetan Rivet
2017-06-27 15:53 ` Bruce Richardson
2017-06-27 19:19 ` Jan Blunck
2017-07-04 1:05 ` Gaëtan Rivet
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 2/7] bus: introduce parsing functionality Gaetan Rivet
2017-06-27 15:54 ` Bruce Richardson
2017-06-27 19:26 ` Jan Blunck
2017-07-04 1:35 ` Gaëtan Rivet
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 3/7] vdev: implement parse bus operation Gaetan Rivet
2017-06-27 15:59 ` Bruce Richardson
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 4/7] pci: " Gaetan Rivet
2017-06-27 16:18 ` Bruce Richardson
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 5/7] bus: add helper to find a bus from a device name Gaetan Rivet
2017-06-27 16:26 ` Bruce Richardson
2017-06-27 18:55 ` Jan Blunck
2017-06-28 17:03 ` Thomas Monjalon
2017-06-29 7:56 ` Jan Blunck
2017-06-29 8:21 ` Thomas Monjalon
2017-06-29 10:23 ` Gaëtan Rivet
2017-06-29 10:30 ` Richardson, Bruce
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 6/7] vdev: expose bus name Gaetan Rivet
2017-06-20 23:30 ` [dpdk-dev] [PATCH v5 7/7] devargs: parse bus info Gaetan Rivet
2017-06-27 16:01 ` [dpdk-dev] [PATCH v5 0/7] rte_bus parse API Bruce Richardson
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 0/6] " Gaetan Rivet
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 1/6] bus: fix bus name registration Gaetan Rivet
2017-07-04 21:43 ` [dpdk-dev] [PATCH] bus: fix driver registration Thomas Monjalon
2017-07-05 5:47 ` Shreyansh Jain
2017-07-05 6:01 ` Shreyansh Jain
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 2/6] bus: introduce parsing functionality Gaetan Rivet
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 3/6] vdev: implement parse bus operation Gaetan Rivet
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 4/6] pci: " Gaetan Rivet
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 5/6] bus: add helper to find a bus from a device name Gaetan Rivet
2017-07-04 12:28 ` Gaëtan Rivet
2017-07-04 0:58 ` [dpdk-dev] [PATCH v6 6/6] devargs: parse bus info Gaetan Rivet
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 0/6] rte_bus parse API Gaetan Rivet
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 1/6] bus: fix driver registration Gaetan Rivet
2017-07-05 13:03 ` Shreyansh Jain
2017-07-06 6:05 ` santosh
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 2/6] bus: introduce parsing functionality Gaetan Rivet
2017-07-05 13:04 ` Shreyansh Jain
2017-07-06 9:19 ` santosh
2017-07-06 12:30 ` Gaëtan Rivet
2017-07-06 13:12 ` santosh
2017-07-06 13:30 ` Gaëtan Rivet
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 3/6] vdev: implement parse bus operation Gaetan Rivet
2017-07-05 13:16 ` Shreyansh Jain
2017-07-06 9:35 ` santosh
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 4/6] pci: " Gaetan Rivet
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 5/6] bus: add helper to find a bus from a device name Gaetan Rivet
2017-07-05 13:35 ` Shreyansh Jain
2017-07-05 13:45 ` Gaëtan Rivet
2017-07-05 14:12 ` Shreyansh Jain
2017-07-06 10:10 ` Thomas Monjalon
2017-07-06 11:37 ` Gaëtan Rivet
2017-07-04 23:55 ` [dpdk-dev] [PATCH v7 6/6] devargs: parse bus info Gaetan Rivet
2017-07-05 18:03 ` Stephen Hemminger
2017-07-06 9:07 ` Shreyansh Jain
2017-07-06 10:00 ` Gaëtan Rivet [this message]
2017-07-06 13:17 ` Shreyansh Jain
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 0/6] rte_bus parse API Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 1/6] bus: fix driver registration Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 2/6] bus: introduce parsing functionality Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 3/6] vdev: implement parse bus operation Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 4/6] pci: " Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 5/6] bus: add helper to find a bus from a device name Gaetan Rivet
2017-07-07 0:03 ` [dpdk-dev] [PATCH v8 6/6] devargs: parse bus info Gaetan Rivet
2017-07-07 0:45 ` Stephen Hemminger
2017-07-07 8:31 ` Gaëtan Rivet
2017-07-08 20:31 ` Thomas Monjalon
2017-07-08 20:30 ` [dpdk-dev] [PATCH v8 0/6] rte_bus parse API Thomas Monjalon
2017-06-07 17:22 ` [dpdk-dev] [PATCH 0/9] " Jan Blunck
2017-06-07 19:55 ` Gaëtan Rivet
2017-06-08 11:38 ` Jan Blunck
2017-06-08 13:04 ` 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=20170706100021.GJ11154@bidouze.vm.6wind.com \
--to=gaetan.rivet@6wind.com \
--cc=dev@dpdk.org \
--cc=shreyansh.jain@nxp.com \
/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).