diff mbox series

[v5,5/5] devargs: parse global device syntax

Message ID 1618283653-16510-6-git-send-email-xuemingl@nvidia.com (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers show
Series eal: enable global device syntax by default | expand

Checks

Context Check Description
ci/iol-testing success Testing PASS
ci/intel-Testing success Testing PASS
ci/iol-abi-testing success Testing PASS
ci/Intel-compilation success Compilation OK
ci/github-robot fail github build: failed
ci/travis-robot fail travis build: failed
ci/checkpatch success coding style OK

Commit Message

Xueming Li April 13, 2021, 3:14 a.m. UTC 
When parsing a devargs, try to parse using the global device syntax
first. Fallback on legacy syntax on error.

Example of new global device syntax:
 -a bus=pci,addr=82:00.0/class=eth/driver=mlx5,dv_flow_en=1

Signed-off-by: Xueming Li <xuemingl@nvidia.com>
Reviewed-by: Gaetan Rivet <grive@u256.net>
---
 doc/guides/rel_notes/release_21_05.rst     |  7 +++++++
 lib/librte_eal/common/eal_common_devargs.c | 16 ++++++++++++----
 lib/librte_eal/include/rte_devargs.h       |  4 ++++
 lib/librte_ethdev/rte_ethdev.c             |  1 -
 4 files changed, 23 insertions(+), 5 deletions(-)

Comments

David Marchand Sept. 28, 2021, 8:29 a.m. UTC | #1 
On Tue, Apr 13, 2021 at 5:15 AM Xueming Li <xuemingl@nvidia.com> wrote:
>
> When parsing a devargs, try to parse using the global device syntax
> first. Fallback on legacy syntax on error.
>
> Example of new global device syntax:
>  -a bus=pci,addr=82:00.0/class=eth/driver=mlx5,dv_flow_en=1
>
> Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> Reviewed-by: Gaetan Rivet <grive@u256.net>

Starting with a virtio user port, I get a warning:

# dpdk-testpmd --vdev
net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1
--log-level=pmd.net.virtio.*:debug -- -i
...
EAL: Unrecognized layer dev/vhost-net,queues=1
...

Am I doing something wrong?
If not, could you have a look?


Thanks.
Thomas Monjalon Sept. 28, 2021, 9:04 a.m. UTC | #2 
28/09/2021 10:29, David Marchand:
> On Tue, Apr 13, 2021 at 5:15 AM Xueming Li <xuemingl@nvidia.com> wrote:
> >
> > When parsing a devargs, try to parse using the global device syntax
> > first. Fallback on legacy syntax on error.
> >
> > Example of new global device syntax:
> >  -a bus=pci,addr=82:00.0/class=eth/driver=mlx5,dv_flow_en=1
> >
> > Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> > Reviewed-by: Gaetan Rivet <grive@u256.net>
> 
> Starting with a virtio user port, I get a warning:
> 
> # dpdk-testpmd --vdev
> net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1
> --log-level=pmd.net.virtio.*:debug -- -i
> ...
> EAL: Unrecognized layer dev/vhost-net,queues=1
> ...
> 
> Am I doing something wrong?
> If not, could you have a look?

The new global syntax is using the slash / as separator.
We should detect legit use of slash in a path.
Here, the value starts with a slash so it should be easy to ignore.
Another way is to consider slash only if followed by "class=" or "driver="
Xueming Li Oct. 3, 2021, 10:51 a.m. UTC | #3 
On Tue, 2021-09-28 at 11:04 +0200, Thomas Monjalon wrote:
> 28/09/2021 10:29, David Marchand:
> > On Tue, Apr 13, 2021 at 5:15 AM Xueming Li <xuemingl@nvidia.com> wrote:
> > > 
> > > When parsing a devargs, try to parse using the global device syntax
> > > first. Fallback on legacy syntax on error.
> > > 
> > > Example of new global device syntax:
> > >  -a bus=pci,addr=82:00.0/class=eth/driver=mlx5,dv_flow_en=1
> > > 
> > > Signed-off-by: Xueming Li <xuemingl@nvidia.com>
> > > Reviewed-by: Gaetan Rivet <grive@u256.net>
> > 
> > Starting with a virtio user port, I get a warning:
> > 
> > # dpdk-testpmd --vdev
> > net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1
> > --log-level=pmd.net.virtio.*:debug -- -i
> > ...
> > EAL: Unrecognized layer dev/vhost-net,queues=1
> > ...
> > 
> > Am I doing something wrong?
> > If not, could you have a look?
> 
> The new global syntax is using the slash / as separator.
> We should detect legit use of slash in a path.
> Here, the value starts with a slash so it should be easy to ignore.
> Another way is to consider slash only if followed by "class=" or "driver="

There is an assumption in the function rte_devargs_layers_parse():
		 * The last layer is free-form.
		 * The "driver" key is not required (but accepted).
For some case like "bus=x,k1=v1/class=y,k2=/some/path/k3=v3", "k3"
belongs to driver layer, this will confuse parser.
diff mbox series

Patch

diff --git a/doc/guides/rel_notes/release_21_05.rst b/doc/guides/rel_notes/release_21_05.rst
index 113b37cddc..ff29c88749 100644
--- a/doc/guides/rel_notes/release_21_05.rst
+++ b/doc/guides/rel_notes/release_21_05.rst
@@ -55,6 +55,13 @@  New Features
      Also, make sure to start the actual text at the margin.
      =======================================================
 
+* **Enabled new devargs parser.**
+
+  * Enabled devargs syntax
+    ``bus=X,paramX=x/class=Y,paramY=y/driver=Z,paramZ=z``
+  * Added bus-level parsing of the devargs syntax.
+  * Kept compatibility with the legacy syntax as parsing fallback.
+
 * **Added support for Marvell CN10K SoC drivers.**
 
   Added Marvell CN10K SoC support. Marvell CN10K SoC are based on Octeon 10
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 2d87e63d2a..a81ce973fe 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -125,7 +125,6 @@  rte_devargs_layers_parse(struct rte_devargs *devargs,
 		layers[i].str = s;
 		layers[i].kvlist = rte_kvargs_parse_delim(s, NULL, "/");
 		if (layers[i].kvlist == NULL) {
-			RTE_LOG(ERR, EAL, "Could not parse %s\n", s);
 			ret = -EINVAL;
 			goto get_out;
 		}
@@ -143,7 +142,7 @@  rte_devargs_layers_parse(struct rte_devargs *devargs,
 		kv = &layers[i].kvlist->pairs[0];
 		if (kv->key == NULL)
 			continue;
-		if (strcmp(kv->key, "bus") == 0) {
+		if (strcmp(kv->key, RTE_DEVARGS_KEY_BUS) == 0) {
 			bus = rte_bus_find_by_name(kv->value);
 			if (bus == NULL) {
 				RTE_LOG(ERR, EAL, "Could not find bus \"%s\"\n",
@@ -151,7 +150,7 @@  rte_devargs_layers_parse(struct rte_devargs *devargs,
 				ret = -EFAULT;
 				goto get_out;
 			}
-		} else if (strcmp(kv->key, "class") == 0) {
+		} else if (strcmp(kv->key, RTE_DEVARGS_KEY_CLASS) == 0) {
 			cls = rte_class_find_by_name(kv->value);
 			if (cls == NULL) {
 				RTE_LOG(ERR, EAL, "Could not find class \"%s\"\n",
@@ -159,7 +158,7 @@  rte_devargs_layers_parse(struct rte_devargs *devargs,
 				ret = -EFAULT;
 				goto get_out;
 			}
-		} else if (strcmp(kv->key, "driver") == 0) {
+		} else if (strcmp(kv->key, RTE_DEVARGS_KEY_DRIVER) == 0) {
 			/* Ignore */
 			continue;
 		}
@@ -224,6 +223,15 @@  rte_devargs_parse(struct rte_devargs *da, const char *dev)
 	if (da == NULL)
 		return -EINVAL;
 
+	/* First parse according global device syntax. */
+	if (rte_devargs_layers_parse(da, dev) == 0) {
+		if (da->bus != NULL || da->cls != NULL)
+			return 0;
+		rte_devargs_reset(da);
+	}
+
+	/* Otherwise fallback to legacy syntax: */
+
 	/* Retrieve eventual bus info */
 	do {
 		devname = dev;
diff --git a/lib/librte_eal/include/rte_devargs.h b/lib/librte_eal/include/rte_devargs.h
index 134b44a887..39e34ea02e 100644
--- a/lib/librte_eal/include/rte_devargs.h
+++ b/lib/librte_eal/include/rte_devargs.h
@@ -25,6 +25,10 @@  extern "C" {
 #include <rte_compat.h>
 #include <rte_bus.h>
 
+#define RTE_DEVARGS_KEY_BUS "bus"
+#define RTE_DEVARGS_KEY_CLASS "class"
+#define RTE_DEVARGS_KEY_DRIVER "driver"
+
 /**
  * Type of generic device
  */
diff --git a/lib/librte_ethdev/rte_ethdev.c b/lib/librte_ethdev/rte_ethdev.c
index 0419500fc3..c417599f79 100644
--- a/lib/librte_ethdev/rte_ethdev.c
+++ b/lib/librte_ethdev/rte_ethdev.c
@@ -207,7 +207,6 @@  rte_eth_iterator_init(struct rte_dev_iterator *iter, const char *devargs_str)
 	 *   - 0000:08:00.0,representor=[1-3]
 	 *   - pci:0000:06:00.0,representor=[0,5]
 	 *   - class=eth,mac=00:11:22:33:44:55
-	 * A new syntax is in development (not yet supported):
 	 *   - bus=X,paramX=x/class=Y,paramY=y/driver=Z,paramZ=z
 	 */