* [dpdk-dev] [PATCH 1/3] devargs: support path value for global device arguments
@ 2021-10-05 12:30 Xueming Li
2021-10-05 12:30 ` [dpdk-dev] [PATCH 2/3] devargs: make bus key parsing optional Xueming Li
` (5 more replies)
0 siblings, 6 replies; 43+ messages in thread
From: Xueming Li @ 2021-10-05 12:30 UTC (permalink / raw)
To: dev; +Cc: xuemingl, Thomas Monjalon, David Marchand
Slash is used to split global device arguments.
To support path value which contains slash, this patch parses devargs by
locating both slash and layer name key:
bus=a,name=/some/path/class=b,k1=v1/driver=c,k2=v2
"/class=" and "/driver" are valid start of a layer.
Signed-off-by: Xueming Li <xuemingl@nvidia.com>
---
lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------
1 file changed, 43 insertions(+), 74 deletions(-)
diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c
index 7ab9e71b2a3..e3786c6c02a 100644
--- a/lib/eal/common/eal_common_devargs.c
+++ b/lib/eal/common/eal_common_devargs.c
@@ -29,18 +29,6 @@ TAILQ_HEAD(rte_devargs_list, rte_devargs);
static struct rte_devargs_list devargs_list =
TAILQ_HEAD_INITIALIZER(devargs_list);
-static size_t
-devargs_layer_count(const char *s)
-{
- size_t i = s ? 1 : 0;
-
- while (s != NULL && s[0] != '\0') {
- i += s[0] == '/';
- s++;
- }
- return i;
-}
-
/* Resolve devargs name from bus arguments. */
static int
devargs_bus_parse_default(struct rte_devargs *devargs,
@@ -77,23 +65,13 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
{ RTE_DEVARGS_KEY_DRIVER "=", NULL, NULL, },
};
struct rte_kvargs_pair *kv = NULL;
- struct rte_class *cls = NULL;
- struct rte_bus *bus = NULL;
- const char *s = devstr;
- size_t nblayer;
- size_t i = 0;
+ struct rte_kvargs *bus_kvlist = NULL;
+ char *s;
+ size_t nblayer = 0;
+ size_t i;
int ret = 0;
bool allocated_data = false;
- /* Split each sub-lists. */
- nblayer = devargs_layer_count(devstr);
- if (nblayer > RTE_DIM(layers)) {
- RTE_LOG(ERR, EAL, "Invalid format: too many layers (%zu)\n",
- nblayer);
- ret = -E2BIG;
- goto get_out;
- }
-
/* If the devargs points the devstr
* as source data, then it should not allocate
* anything and keep referring only to it.
@@ -106,33 +84,41 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
goto get_out;
}
allocated_data = true;
- s = devargs->data;
}
+ s = devargs->data;
while (s != NULL) {
- if (i >= RTE_DIM(layers)) {
- RTE_LOG(ERR, EAL, "Unrecognized layer %s\n", s);
- ret = -EINVAL;
+ if (nblayer > RTE_DIM(layers)) {
+ ret = -E2BIG;
goto get_out;
}
- /*
- * The last layer is free-form.
- * The "driver" key is not required (but accepted).
- */
- if (strncmp(layers[i].key, s, strlen(layers[i].key)) &&
- i != RTE_DIM(layers) - 1)
- goto next_layer;
- layers[i].str = s;
- layers[i].kvlist = rte_kvargs_parse_delim(s, NULL, "/");
- if (layers[i].kvlist == NULL) {
+ layers[nblayer].str = s;
+
+ /* Locate next layer starts with valid layer key. */
+ while (s != NULL) {
+ s = strchr(s, '/');
+ if (s == NULL)
+ break;
+ for (i = 0; i < RTE_DIM(layers); i++) {
+ if (strncmp(s + 1, layers[i].key,
+ strlen(layers[i].key)) == 0) {
+ *s = '\0';
+ break;
+ }
+ }
+ s++;
+ if (i < RTE_DIM(layers))
+ break;
+ }
+
+ layers[nblayer].kvlist = rte_kvargs_parse
+ (layers[nblayer].str, NULL);
+ if (layers[nblayer].kvlist == NULL) {
ret = -EINVAL;
goto get_out;
}
- s = strchr(s, '/');
- if (s != NULL)
- s++;
-next_layer:
- i++;
+
+ nblayer++;
}
/* Parse each sub-list. */
@@ -143,52 +129,35 @@ rte_devargs_layers_parse(struct rte_devargs *devargs,
if (kv->key == NULL)
continue;
if (strcmp(kv->key, RTE_DEVARGS_KEY_BUS) == 0) {
- bus = rte_bus_find_by_name(kv->value);
- if (bus == NULL) {
+ bus_kvlist = layers[i].kvlist;
+ devargs->bus_str = layers[i].str;
+ devargs->bus = rte_bus_find_by_name(kv->value);
+ if (devargs->bus == NULL) {
RTE_LOG(ERR, EAL, "Could not find bus \"%s\"\n",
kv->value);
ret = -EFAULT;
goto get_out;
}
} else if (strcmp(kv->key, RTE_DEVARGS_KEY_CLASS) == 0) {
- cls = rte_class_find_by_name(kv->value);
- if (cls == NULL) {
+ devargs->cls_str = layers[i].str;
+ devargs->cls = rte_class_find_by_name(kv->value);
+ if (devargs->cls == NULL) {
RTE_LOG(ERR, EAL, "Could not find class \"%s\"\n",
kv->value);
ret = -EFAULT;
goto get_out;
}
} else if (strcmp(kv->key, RTE_DEVARGS_KEY_DRIVER) == 0) {
- /* Ignore */
+ devargs->drv_str = layers[i].str;
continue;
}
}
- /* Fill devargs fields. */
- devargs->bus_str = layers[0].str;
- devargs->cls_str = layers[1].str;
- devargs->drv_str = layers[2].str;
- devargs->bus = bus;
- devargs->cls = cls;
-
- /* If we own the data, clean up a bit
- * the several layers string, to ease
- * their parsing afterward.
- */
- if (devargs->data != devstr) {
- char *s = devargs->data;
-
- while ((s = strchr(s, '/'))) {
- *s = '\0';
- s++;
- }
- }
-
/* Resolve devargs name. */
- if (bus != NULL && bus->devargs_parse != NULL)
- ret = bus->devargs_parse(devargs);
- else if (layers[0].kvlist != NULL)
- ret = devargs_bus_parse_default(devargs, layers[0].kvlist);
+ if (devargs->bus != NULL && devargs->bus->devargs_parse != NULL)
+ ret = devargs->bus->devargs_parse(devargs);
+ else if (bus_kvlist != NULL)
+ ret = devargs_bus_parse_default(devargs, bus_kvlist);
get_out:
for (i = 0; i < RTE_DIM(layers); i++) {
--
2.33.0
^ permalink raw reply [flat|nested] 43+ messages in thread* [dpdk-dev] [PATCH 2/3] devargs: make bus key parsing optional 2021-10-05 12:30 [dpdk-dev] [PATCH 1/3] devargs: support path value for global device arguments Xueming Li @ 2021-10-05 12:30 ` Xueming Li 2021-10-05 12:30 ` [dpdk-dev] [PATCH 3/3] test/devargs: add devargs test cases Xueming Li ` (4 subsequent siblings) 5 siblings, 0 replies; 43+ messages in thread From: Xueming Li @ 2021-10-05 12:30 UTC (permalink / raw) To: dev; +Cc: xuemingl, Thomas Monjalon, David Marchand, stable, Gaetan Rivet Global devargs syntax is used as device iteration filter like "class=vdpa", a devargs without bus args is valid from parsing perspective. This patch makes bus args optional. Fixes: d2a66ad79480 ("bus: add device arguments name parsing") Cc: stable@dpdk.org Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/bus/pci/pci_params.c b/drivers/bus/pci/pci_params.c index 691b5ea0180..f6fa3a5d6ce 100644 --- a/drivers/bus/pci/pci_params.c +++ b/drivers/bus/pci/pci_params.c @@ -85,11 +85,10 @@ rte_pci_devargs_parse(struct rte_devargs *da) struct rte_kvargs *kvargs; const char *addr_str; struct rte_pci_addr addr; - int ret; + int ret = 0; - if (da == NULL) + if (da == NULL || da->bus_str == NULL) return 0; - RTE_ASSERT(da->bus_str != NULL); kvargs = rte_kvargs_parse(da->bus_str, NULL); if (kvargs == NULL) { @@ -101,9 +100,8 @@ rte_pci_devargs_parse(struct rte_devargs *da) addr_str = rte_kvargs_get(kvargs, pci_params_keys[RTE_PCI_PARAM_ADDR]); if (addr_str == NULL) { - RTE_LOG(ERR, EAL, "No PCI address specified using '%s=<id>' in: %s\n", + RTE_LOG(DEBUG, EAL, "No PCI address specified using '%s=<id>' in: %s\n", pci_params_keys[RTE_PCI_PARAM_ADDR], da->bus_str); - ret = -ENODEV; goto out; } diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index e3786c6c02a..616cf77f229 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -39,7 +39,7 @@ devargs_bus_parse_default(struct rte_devargs *devargs, /* Parse devargs name from bus key-value list. */ name = rte_kvargs_get(bus_args, "name"); if (name == NULL) { - RTE_LOG(INFO, EAL, "devargs name not found: %s\n", + RTE_LOG(DEBUG, EAL, "devargs name not found: %s\n", devargs->data); return 0; } -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH 3/3] test/devargs: add devargs test cases 2021-10-05 12:30 [dpdk-dev] [PATCH 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-05 12:30 ` [dpdk-dev] [PATCH 2/3] devargs: make bus key parsing optional Xueming Li @ 2021-10-05 12:30 ` Xueming Li 2021-10-05 14:01 ` David Marchand 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 0/3] devargs: support path in global syntax Xueming Li ` (3 subsequent siblings) 5 siblings, 1 reply; 43+ messages in thread From: Xueming Li @ 2021-10-05 12:30 UTC (permalink / raw) To: dev; +Cc: xuemingl, Thomas Monjalon, David Marchand Initial version to test Global devargs syntax. Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- app/test/meson.build | 1 + app/test/test_devargs.c | 147 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 app/test/test_devargs.c diff --git a/app/test/meson.build b/app/test/meson.build index f144d8b8ed6..c688ba2b70a 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -42,6 +42,7 @@ test_sources = files( 'test_cryptodev_security_pdcp.c', 'test_cycles.c', 'test_debug.c', + 'test_devargs.c', 'test_distributor.c', 'test_distributor_perf.c', 'test_eal_flags.c', diff --git a/app/test/test_devargs.c b/app/test/test_devargs.c new file mode 100644 index 00000000000..8a173368347 --- /dev/null +++ b/app/test/test_devargs.c @@ -0,0 +1,147 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright (c) 2021 NVIDIA Corporation & Affiliates + */ + +#include <stdlib.h> +#include <stdio.h> +#include <string.h> + +#include <rte_common.h> +#include <rte_devargs.h> +#include <rte_kvargs.h> + +#include "test.h" + +/* Check layer arguments. */ +static int +test_args(const char *devargs, const char *layer, const char *args, const int n) +{ + struct rte_kvargs *kvlist; + + if (n == 0) { + if (args != NULL && strlen(args) > 0) { + printf("rte_devargs_parse(%s) %s args parsed (not expected)\n", + devargs, layer); + return -1; + } else { + return 0; + } + } + if (args == NULL) { + printf("rte_devargs_parse(%s) %s args not parsed\n", + devargs, layer); + return -1; + } + kvlist = rte_kvargs_parse(args, NULL); + if (kvlist == NULL) { + printf("rte_devargs_parse(%s) %s_str: %s not parsed\n", + devargs, layer, args); + return -1; + } + if ((int)kvlist->count != n) { + printf("rte_devargs_parse(%s) %s_str: %s kv number %u, not %d\n", + devargs, layer, args, kvlist->count, n); + return -1; + } + return 0; +} + +/* Test several valid cases */ +static int +test_valid_devargs(void) +{ + static const struct { + const char *devargs; + int bus_kv; + int class_kv; + int driver_kv; + } list[] = { + /* Global devargs syntax: */ + { "bus=pci", 1, 0, 0 }, + { "class=eth", 0, 1, 0 }, + { "bus=pci,addr=1:2.3/class=eth/driver=abc,k0=v0", 2, 1, 2 }, + { "bus=vdev,name=/dev/file/name/class=eth", 2, 1, 0 }, + /* Legacy devargs syntax: */ + { "1:2.3", 0, 0, 0 }, + { "pci:1:2.3,k0=v0", 0, 0, 1 }, + { "net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1", + 0, 0, 3 }, + }; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + memset(&da, 0, sizeof(da)); + ret = rte_devargs_parse(&da, list[i].devargs); + if (ret < 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i].devargs, ret); + goto fail; + } + if (list[i].bus_kv > 0 && da.bus == NULL) { + printf("rte_devargs_parse(%s) bus not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "bus", da.bus_str, + list[i].bus_kv) != 0) + goto fail; + if (list[i].class_kv > 0 && da.cls == NULL) { + printf("rte_devargs_parse(%s) class not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "class", da.cls_str, + list[i].class_kv) != 0) + goto fail; + if (test_args(list[i].devargs, "driver", da.drv_str, + list[i].driver_kv) != 0) + goto fail; + goto cleanup; +fail: + fail = -1; +cleanup: + rte_devargs_reset(&da); + } + return fail; +} + +/* Test several invalid cases */ +static int +test_invalid_devargs(void) +{ + static const char * const list[] = { + "bus=wrong-bus", + "class=wrong-class"}; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + ret = rte_devargs_parse(&da, list[i]); + if (ret >= 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i], ret); + fail = ret; + } + rte_devargs_reset(&da); + } + return fail; +} + +static int +test_devargs(void) +{ + printf("== test valid case ==\n"); + if (test_valid_devargs() < 0) + return -1; + printf("== test invalid case ==\n"); + if (test_invalid_devargs() < 0) + return -1; + return 0; +} + +REGISTER_TEST_COMMAND(devargs_autotest, test_devargs); -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH 3/3] test/devargs: add devargs test cases 2021-10-05 12:30 ` [dpdk-dev] [PATCH 3/3] test/devargs: add devargs test cases Xueming Li @ 2021-10-05 14:01 ` David Marchand 2021-10-05 15:56 ` Xueming(Steven) Li 0 siblings, 1 reply; 43+ messages in thread From: David Marchand @ 2021-10-05 14:01 UTC (permalink / raw) To: Xueming Li; +Cc: dev, Thomas Monjalon On Tue, Oct 5, 2021 at 2:31 PM Xueming Li <xuemingl@nvidia.com> wrote: > diff --git a/app/test/meson.build b/app/test/meson.build > index f144d8b8ed6..c688ba2b70a 100644 > --- a/app/test/meson.build > +++ b/app/test/meson.build > @@ -42,6 +42,7 @@ test_sources = files( > 'test_cryptodev_security_pdcp.c', > 'test_cycles.c', > 'test_debug.c', > + 'test_devargs.c', > 'test_distributor.c', > 'test_distributor_perf.c', > 'test_eal_flags.c', You likely want to add devargs_autotest to fast_tests, else "meson test" (and the CI) won't execute this new test. As for tests referenced in autotest_data.py, I am not sure we really need this, but please update too for consistency. -- David Marchand ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH 3/3] test/devargs: add devargs test cases 2021-10-05 14:01 ` David Marchand @ 2021-10-05 15:56 ` Xueming(Steven) Li 0 siblings, 0 replies; 43+ messages in thread From: Xueming(Steven) Li @ 2021-10-05 15:56 UTC (permalink / raw) To: david.marchand; +Cc: NBU-Contact-Thomas Monjalon, dev On Tue, 2021-10-05 at 16:01 +0200, David Marchand wrote: > On Tue, Oct 5, 2021 at 2:31 PM Xueming Li <xuemingl@nvidia.com> wrote: > > diff --git a/app/test/meson.build b/app/test/meson.build > > index f144d8b8ed6..c688ba2b70a 100644 > > --- a/app/test/meson.build > > +++ b/app/test/meson.build > > @@ -42,6 +42,7 @@ test_sources = files( > > 'test_cryptodev_security_pdcp.c', > > 'test_cycles.c', > > 'test_debug.c', > > + 'test_devargs.c', > > 'test_distributor.c', > > 'test_distributor_perf.c', > > 'test_eal_flags.c', > > You likely want to add devargs_autotest to fast_tests, else "meson > test" (and the CI) won't execute this new test. > As for tests referenced in autotest_data.py, I am not sure we really > need this, but please update too for consistency. Good suggestion! v1 posted. > > ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v1 0/3] devargs: support path in global syntax 2021-10-05 12:30 [dpdk-dev] [PATCH 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-05 12:30 ` [dpdk-dev] [PATCH 2/3] devargs: make bus key parsing optional Xueming Li 2021-10-05 12:30 ` [dpdk-dev] [PATCH 3/3] test/devargs: add devargs test cases Xueming Li @ 2021-10-05 15:54 ` Xueming Li 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 1/3] devargs: support path value for global device arguments Xueming Li ` (3 more replies) 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 0/3] devargs: support path in global syntax Xueming Li ` (2 subsequent siblings) 5 siblings, 4 replies; 43+ messages in thread From: Xueming Li @ 2021-10-05 15:54 UTC (permalink / raw) To: dev; +Cc: xuemingl, Thomas Monjalon, David Marchand - Support path in global syntax. - Fix bus name resolving - Add devargs test cases v1: add test cases to test suite Xueming Li (3): devargs: support path value for global device arguments devargs: make bus key parsing optional test/devargs: add devargs test cases app/test/autotest_data.py | 6 ++ app/test/meson.build | 2 + app/test/test_devargs.c | 147 ++++++++++++++++++++++++++++ drivers/bus/pci/pci_params.c | 8 +- lib/eal/common/eal_common_devargs.c | 119 +++++++++------------- 5 files changed, 202 insertions(+), 80 deletions(-) create mode 100644 app/test/test_devargs.c -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v1 1/3] devargs: support path value for global device arguments 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 0/3] devargs: support path in global syntax Xueming Li @ 2021-10-05 15:54 ` Xueming Li 2021-10-19 14:57 ` Gaëtan Rivet 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 2/3] devargs: make bus key parsing optional Xueming Li ` (2 subsequent siblings) 3 siblings, 1 reply; 43+ messages in thread From: Xueming Li @ 2021-10-05 15:54 UTC (permalink / raw) To: dev; +Cc: xuemingl, Thomas Monjalon, David Marchand Slash is used to split global device arguments. To support path value which contains slash, this patch parses devargs by locating both slash and layer name key: bus=a,name=/some/path/class=b,k1=v1/driver=c,k2=v2 "/class=" and "/driver" are valid start of a layer. Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-) diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index 7ab9e71b2a3..e3786c6c02a 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -29,18 +29,6 @@ TAILQ_HEAD(rte_devargs_list, rte_devargs); static struct rte_devargs_list devargs_list = TAILQ_HEAD_INITIALIZER(devargs_list); -static size_t -devargs_layer_count(const char *s) -{ - size_t i = s ? 1 : 0; - - while (s != NULL && s[0] != '\0') { - i += s[0] == '/'; - s++; - } - return i; -} - /* Resolve devargs name from bus arguments. */ static int devargs_bus_parse_default(struct rte_devargs *devargs, @@ -77,23 +65,13 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, { RTE_DEVARGS_KEY_DRIVER "=", NULL, NULL, }, }; struct rte_kvargs_pair *kv = NULL; - struct rte_class *cls = NULL; - struct rte_bus *bus = NULL; - const char *s = devstr; - size_t nblayer; - size_t i = 0; + struct rte_kvargs *bus_kvlist = NULL; + char *s; + size_t nblayer = 0; + size_t i; int ret = 0; bool allocated_data = false; - /* Split each sub-lists. */ - nblayer = devargs_layer_count(devstr); - if (nblayer > RTE_DIM(layers)) { - RTE_LOG(ERR, EAL, "Invalid format: too many layers (%zu)\n", - nblayer); - ret = -E2BIG; - goto get_out; - } - /* If the devargs points the devstr * as source data, then it should not allocate * anything and keep referring only to it. @@ -106,33 +84,41 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, goto get_out; } allocated_data = true; - s = devargs->data; } + s = devargs->data; while (s != NULL) { - if (i >= RTE_DIM(layers)) { - RTE_LOG(ERR, EAL, "Unrecognized layer %s\n", s); - ret = -EINVAL; + if (nblayer > RTE_DIM(layers)) { + ret = -E2BIG; goto get_out; } - /* - * The last layer is free-form. - * The "driver" key is not required (but accepted). - */ - if (strncmp(layers[i].key, s, strlen(layers[i].key)) && - i != RTE_DIM(layers) - 1) - goto next_layer; - layers[i].str = s; - layers[i].kvlist = rte_kvargs_parse_delim(s, NULL, "/"); - if (layers[i].kvlist == NULL) { + layers[nblayer].str = s; + + /* Locate next layer starts with valid layer key. */ + while (s != NULL) { + s = strchr(s, '/'); + if (s == NULL) + break; + for (i = 0; i < RTE_DIM(layers); i++) { + if (strncmp(s + 1, layers[i].key, + strlen(layers[i].key)) == 0) { + *s = '\0'; + break; + } + } + s++; + if (i < RTE_DIM(layers)) + break; + } + + layers[nblayer].kvlist = rte_kvargs_parse + (layers[nblayer].str, NULL); + if (layers[nblayer].kvlist == NULL) { ret = -EINVAL; goto get_out; } - s = strchr(s, '/'); - if (s != NULL) - s++; -next_layer: - i++; + + nblayer++; } /* Parse each sub-list. */ @@ -143,52 +129,35 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, if (kv->key == NULL) continue; if (strcmp(kv->key, RTE_DEVARGS_KEY_BUS) == 0) { - bus = rte_bus_find_by_name(kv->value); - if (bus == NULL) { + bus_kvlist = layers[i].kvlist; + devargs->bus_str = layers[i].str; + devargs->bus = rte_bus_find_by_name(kv->value); + if (devargs->bus == NULL) { RTE_LOG(ERR, EAL, "Could not find bus \"%s\"\n", kv->value); ret = -EFAULT; goto get_out; } } else if (strcmp(kv->key, RTE_DEVARGS_KEY_CLASS) == 0) { - cls = rte_class_find_by_name(kv->value); - if (cls == NULL) { + devargs->cls_str = layers[i].str; + devargs->cls = rte_class_find_by_name(kv->value); + if (devargs->cls == NULL) { RTE_LOG(ERR, EAL, "Could not find class \"%s\"\n", kv->value); ret = -EFAULT; goto get_out; } } else if (strcmp(kv->key, RTE_DEVARGS_KEY_DRIVER) == 0) { - /* Ignore */ + devargs->drv_str = layers[i].str; continue; } } - /* Fill devargs fields. */ - devargs->bus_str = layers[0].str; - devargs->cls_str = layers[1].str; - devargs->drv_str = layers[2].str; - devargs->bus = bus; - devargs->cls = cls; - - /* If we own the data, clean up a bit - * the several layers string, to ease - * their parsing afterward. - */ - if (devargs->data != devstr) { - char *s = devargs->data; - - while ((s = strchr(s, '/'))) { - *s = '\0'; - s++; - } - } - /* Resolve devargs name. */ - if (bus != NULL && bus->devargs_parse != NULL) - ret = bus->devargs_parse(devargs); - else if (layers[0].kvlist != NULL) - ret = devargs_bus_parse_default(devargs, layers[0].kvlist); + if (devargs->bus != NULL && devargs->bus->devargs_parse != NULL) + ret = devargs->bus->devargs_parse(devargs); + else if (bus_kvlist != NULL) + ret = devargs_bus_parse_default(devargs, bus_kvlist); get_out: for (i = 0; i < RTE_DIM(layers); i++) { -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/3] devargs: support path value for global device arguments 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 1/3] devargs: support path value for global device arguments Xueming Li @ 2021-10-19 14:57 ` Gaëtan Rivet 0 siblings, 0 replies; 43+ messages in thread From: Gaëtan Rivet @ 2021-10-19 14:57 UTC (permalink / raw) To: Xueming(Steven) Li, dev; +Cc: Thomas Monjalon, David Marchand Hi Steven, On Tue, Oct 5, 2021, at 17:54, Xueming Li wrote: > Slash is used to split global device arguments. > > To support path value which contains slash, this patch parses devargs by > locating both slash and layer name key: > bus=a,name=/some/path/class=b,k1=v1/driver=c,k2=v2 > "/class=" and "/driver" are valid start of a layer. > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> The code looks good to me, and after executing with the test suite it seems to behave properly. Reviewed-by: Gaetan Rivet <grive@u256.net> -- Gaetan Rivet ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v1 2/3] devargs: make bus key parsing optional 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 0/3] devargs: support path in global syntax Xueming Li 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 1/3] devargs: support path value for global device arguments Xueming Li @ 2021-10-05 15:54 ` Xueming Li 2021-10-19 15:19 ` Gaëtan Rivet 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 3/3] test/devargs: add devargs test cases Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 0/3] devargs: support path in global syntax Xueming Li 3 siblings, 1 reply; 43+ messages in thread From: Xueming Li @ 2021-10-05 15:54 UTC (permalink / raw) To: dev; +Cc: xuemingl, Thomas Monjalon, David Marchand, stable, Gaetan Rivet Global devargs syntax is used as device iteration filter like "class=vdpa", a devargs without bus args is valid from parsing perspective. This patch makes bus args optional. Fixes: d2a66ad79480 ("bus: add device arguments name parsing") Cc: stable@dpdk.org Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/bus/pci/pci_params.c b/drivers/bus/pci/pci_params.c index 691b5ea0180..f6fa3a5d6ce 100644 --- a/drivers/bus/pci/pci_params.c +++ b/drivers/bus/pci/pci_params.c @@ -85,11 +85,10 @@ rte_pci_devargs_parse(struct rte_devargs *da) struct rte_kvargs *kvargs; const char *addr_str; struct rte_pci_addr addr; - int ret; + int ret = 0; - if (da == NULL) + if (da == NULL || da->bus_str == NULL) return 0; - RTE_ASSERT(da->bus_str != NULL); kvargs = rte_kvargs_parse(da->bus_str, NULL); if (kvargs == NULL) { @@ -101,9 +100,8 @@ rte_pci_devargs_parse(struct rte_devargs *da) addr_str = rte_kvargs_get(kvargs, pci_params_keys[RTE_PCI_PARAM_ADDR]); if (addr_str == NULL) { - RTE_LOG(ERR, EAL, "No PCI address specified using '%s=<id>' in: %s\n", + RTE_LOG(DEBUG, EAL, "No PCI address specified using '%s=<id>' in: %s\n", pci_params_keys[RTE_PCI_PARAM_ADDR], da->bus_str); - ret = -ENODEV; goto out; } diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index e3786c6c02a..616cf77f229 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -39,7 +39,7 @@ devargs_bus_parse_default(struct rte_devargs *devargs, /* Parse devargs name from bus key-value list. */ name = rte_kvargs_get(bus_args, "name"); if (name == NULL) { - RTE_LOG(INFO, EAL, "devargs name not found: %s\n", + RTE_LOG(DEBUG, EAL, "devargs name not found: %s\n", devargs->data); return 0; } -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v1 2/3] devargs: make bus key parsing optional 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 2/3] devargs: make bus key parsing optional Xueming Li @ 2021-10-19 15:19 ` Gaëtan Rivet 0 siblings, 0 replies; 43+ messages in thread From: Gaëtan Rivet @ 2021-10-19 15:19 UTC (permalink / raw) To: Xueming(Steven) Li, dev; +Cc: Thomas Monjalon, David Marchand, stable On Tue, Oct 5, 2021, at 17:54, Xueming Li wrote: > Global devargs syntax is used as device iteration filter like > "class=vdpa", a devargs without bus args is valid from parsing > perspective. > > This patch makes bus args optional. > > Fixes: d2a66ad79480 ("bus: add device arguments name parsing") > Cc: stable@dpdk.org I agree with the change, but I'm not sure it should go into stable. The question goes further than that: is there a spec describing the valid syntax to users, that would allow to say the current parsing is buggy? I haven't found it in the doc. If there is a change such as this one, it should also be notified in the release notes. This is user-facing. So, not saying you should do it and not part of this patch, but a doc might help in doc/guides/howto maybe. The general grammar of a devargs could be formally described (in BNF?). Each layers should document their supported keys as well. Not sure it would be better gathered in a single file or within each driver. Now that the global syntax has been enabled for users, it should be documented. > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> For this patch and beside the stable considerations, Reviewed-by: Gaetan Rivet <grive@u256.net> -- Gaetan Rivet ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v1 3/3] test/devargs: add devargs test cases 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 0/3] devargs: support path in global syntax Xueming Li 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 2/3] devargs: make bus key parsing optional Xueming Li @ 2021-10-05 15:54 ` Xueming Li 2021-10-19 15:07 ` Gaëtan Rivet 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 0/3] devargs: support path in global syntax Xueming Li 3 siblings, 1 reply; 43+ messages in thread From: Xueming Li @ 2021-10-05 15:54 UTC (permalink / raw) To: dev; +Cc: xuemingl, Thomas Monjalon, David Marchand Initial version to test Global devargs syntax. Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- app/test/autotest_data.py | 6 ++ app/test/meson.build | 2 + app/test/test_devargs.c | 147 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 155 insertions(+) create mode 100644 app/test/test_devargs.c diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py index 302d6374c16..3b841e71918 100644 --- a/app/test/autotest_data.py +++ b/app/test/autotest_data.py @@ -785,6 +785,12 @@ "Func": default_autotest, "Report": None, }, + { + "Name": "Devargs autotest", + "Command": "devargs_autotest", + "Func": default_autotest, + "Report": None, + }, # # Please always make sure that ring_perf is the last test! # diff --git a/app/test/meson.build b/app/test/meson.build index f144d8b8ed6..de8540f6119 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -42,6 +42,7 @@ test_sources = files( 'test_cryptodev_security_pdcp.c', 'test_cycles.c', 'test_debug.c', + 'test_devargs.c', 'test_distributor.c', 'test_distributor_perf.c', 'test_eal_flags.c', @@ -201,6 +202,7 @@ fast_tests = [ ['common_autotest', true], ['cpuflags_autotest', true], ['debug_autotest', true], + ['devargs_autotest', true], ['eal_flags_c_opt_autotest', false], ['eal_flags_main_opt_autotest', false], ['eal_flags_n_opt_autotest', false], diff --git a/app/test/test_devargs.c b/app/test/test_devargs.c new file mode 100644 index 00000000000..8a173368347 --- /dev/null +++ b/app/test/test_devargs.c @@ -0,0 +1,147 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright (c) 2021 NVIDIA Corporation & Affiliates + */ + +#include <stdlib.h> +#include <stdio.h> +#include <string.h> + +#include <rte_common.h> +#include <rte_devargs.h> +#include <rte_kvargs.h> + +#include "test.h" + +/* Check layer arguments. */ +static int +test_args(const char *devargs, const char *layer, const char *args, const int n) +{ + struct rte_kvargs *kvlist; + + if (n == 0) { + if (args != NULL && strlen(args) > 0) { + printf("rte_devargs_parse(%s) %s args parsed (not expected)\n", + devargs, layer); + return -1; + } else { + return 0; + } + } + if (args == NULL) { + printf("rte_devargs_parse(%s) %s args not parsed\n", + devargs, layer); + return -1; + } + kvlist = rte_kvargs_parse(args, NULL); + if (kvlist == NULL) { + printf("rte_devargs_parse(%s) %s_str: %s not parsed\n", + devargs, layer, args); + return -1; + } + if ((int)kvlist->count != n) { + printf("rte_devargs_parse(%s) %s_str: %s kv number %u, not %d\n", + devargs, layer, args, kvlist->count, n); + return -1; + } + return 0; +} + +/* Test several valid cases */ +static int +test_valid_devargs(void) +{ + static const struct { + const char *devargs; + int bus_kv; + int class_kv; + int driver_kv; + } list[] = { + /* Global devargs syntax: */ + { "bus=pci", 1, 0, 0 }, + { "class=eth", 0, 1, 0 }, + { "bus=pci,addr=1:2.3/class=eth/driver=abc,k0=v0", 2, 1, 2 }, + { "bus=vdev,name=/dev/file/name/class=eth", 2, 1, 0 }, + /* Legacy devargs syntax: */ + { "1:2.3", 0, 0, 0 }, + { "pci:1:2.3,k0=v0", 0, 0, 1 }, + { "net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1", + 0, 0, 3 }, + }; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + memset(&da, 0, sizeof(da)); + ret = rte_devargs_parse(&da, list[i].devargs); + if (ret < 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i].devargs, ret); + goto fail; + } + if (list[i].bus_kv > 0 && da.bus == NULL) { + printf("rte_devargs_parse(%s) bus not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "bus", da.bus_str, + list[i].bus_kv) != 0) + goto fail; + if (list[i].class_kv > 0 && da.cls == NULL) { + printf("rte_devargs_parse(%s) class not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "class", da.cls_str, + list[i].class_kv) != 0) + goto fail; + if (test_args(list[i].devargs, "driver", da.drv_str, + list[i].driver_kv) != 0) + goto fail; + goto cleanup; +fail: + fail = -1; +cleanup: + rte_devargs_reset(&da); + } + return fail; +} + +/* Test several invalid cases */ +static int +test_invalid_devargs(void) +{ + static const char * const list[] = { + "bus=wrong-bus", + "class=wrong-class"}; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + ret = rte_devargs_parse(&da, list[i]); + if (ret >= 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i], ret); + fail = ret; + } + rte_devargs_reset(&da); + } + return fail; +} + +static int +test_devargs(void) +{ + printf("== test valid case ==\n"); + if (test_valid_devargs() < 0) + return -1; + printf("== test invalid case ==\n"); + if (test_invalid_devargs() < 0) + return -1; + return 0; +} + +REGISTER_TEST_COMMAND(devargs_autotest, test_devargs); -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v1 3/3] test/devargs: add devargs test cases 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 3/3] test/devargs: add devargs test cases Xueming Li @ 2021-10-19 15:07 ` Gaëtan Rivet 2021-10-20 7:20 ` Xueming(Steven) Li 0 siblings, 1 reply; 43+ messages in thread From: Gaëtan Rivet @ 2021-10-19 15:07 UTC (permalink / raw) To: Xueming(Steven) Li, dev; +Cc: Thomas Monjalon, David Marchand On Tue, Oct 5, 2021, at 17:54, Xueming Li wrote: > Initial version to test Global devargs syntax. > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> Hi, The test is a very nice addition, absolutely required. I have however two remarks on the coverage and the implementation, below. > --- > app/test/autotest_data.py | 6 ++ > app/test/meson.build | 2 + > app/test/test_devargs.c | 147 ++++++++++++++++++++++++++++++++++++++ > 3 files changed, 155 insertions(+) > create mode 100644 app/test/test_devargs.c > > diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py > index 302d6374c16..3b841e71918 100644 > --- a/app/test/autotest_data.py > +++ b/app/test/autotest_data.py > @@ -785,6 +785,12 @@ > "Func": default_autotest, > "Report": None, > }, > + { > + "Name": "Devargs autotest", > + "Command": "devargs_autotest", > + "Func": default_autotest, > + "Report": None, > + }, > # > # Please always make sure that ring_perf is the last test! > # > diff --git a/app/test/meson.build b/app/test/meson.build > index f144d8b8ed6..de8540f6119 100644 > --- a/app/test/meson.build > +++ b/app/test/meson.build > @@ -42,6 +42,7 @@ test_sources = files( > 'test_cryptodev_security_pdcp.c', > 'test_cycles.c', > 'test_debug.c', > + 'test_devargs.c', > 'test_distributor.c', > 'test_distributor_perf.c', > 'test_eal_flags.c', > @@ -201,6 +202,7 @@ fast_tests = [ > ['common_autotest', true], > ['cpuflags_autotest', true], > ['debug_autotest', true], > + ['devargs_autotest', true], > ['eal_flags_c_opt_autotest', false], > ['eal_flags_main_opt_autotest', false], > ['eal_flags_n_opt_autotest', false], > diff --git a/app/test/test_devargs.c b/app/test/test_devargs.c > new file mode 100644 > index 00000000000..8a173368347 > --- /dev/null > +++ b/app/test/test_devargs.c > @@ -0,0 +1,147 @@ > +/* SPDX-License-Identifier: BSD-3-Clause > + * Copyright (c) 2021 NVIDIA Corporation & Affiliates > + */ > + > +#include <stdlib.h> > +#include <stdio.h> > +#include <string.h> > + > +#include <rte_common.h> > +#include <rte_devargs.h> > +#include <rte_kvargs.h> > + > +#include "test.h" > + > +/* Check layer arguments. */ > +static int > +test_args(const char *devargs, const char *layer, const char *args, > const int n) > +{ > + struct rte_kvargs *kvlist; > + > + if (n == 0) { > + if (args != NULL && strlen(args) > 0) { > + printf("rte_devargs_parse(%s) %s args parsed (not expected)\n", > + devargs, layer); > + return -1; > + } else { > + return 0; > + } > + } > + if (args == NULL) { > + printf("rte_devargs_parse(%s) %s args not parsed\n", > + devargs, layer); > + return -1; > + } > + kvlist = rte_kvargs_parse(args, NULL); > + if (kvlist == NULL) { > + printf("rte_devargs_parse(%s) %s_str: %s not parsed\n", > + devargs, layer, args); > + return -1; > + } > + if ((int)kvlist->count != n) { > + printf("rte_devargs_parse(%s) %s_str: %s kv number %u, not %d\n", > + devargs, layer, args, kvlist->count, n); > + return -1; > + } > + return 0; > +} > + > +/* Test several valid cases */ > +static int > +test_valid_devargs(void) > +{ > + static const struct { > + const char *devargs; > + int bus_kv; > + int class_kv; > + int driver_kv; > + } list[] = { > + /* Global devargs syntax: */ > + { "bus=pci", 1, 0, 0 }, > + { "class=eth", 0, 1, 0 }, > + { "bus=pci,addr=1:2.3/class=eth/driver=abc,k0=v0", 2, 1, 2 }, > + { "bus=vdev,name=/dev/file/name/class=eth", 2, 1, 0 }, > + /* Legacy devargs syntax: */ > + { "1:2.3", 0, 0, 0 }, > + { "pci:1:2.3,k0=v0", 0, 0, 1 }, > + { "net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1", > + 0, 0, 3 }, I would add here cases to verify that edge-case are properly parsed such as: + { "bus=vdev,name=/class/bus/path/class=eth", 2, 1, 0 }, [...] + { "net_virtio_user0,iface=test,path=/class/bus/,queues=1", + 0, 0, 3 }, To check the /class or /bus parts cannot throw off the parser (it does not currently). Additionally, paths with multiple slashes are correct. Maybe add: + { "bus=vdev,name=///dblslsh/class=eth", 2, 1, 0 }, as well. I tested all those cases without issue, it seems the parser is ok. A second point is that the test only verifies that the numbers of kv were found. I think this is not robust enough. Instead, I think the 'expect' part of the test should describe exactly what each layers should be within the devargs (which bus, class and driver are expected to be found, and the associated string). Right now the parser could mangle part of a key-value list within the string, recognize each layers and give incorrect strings to each layer parser. It might be too much? I don't know. But it seems it could help ensure no error is introduced at some point by future changes. -- Gaetan Rivet ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v1 3/3] test/devargs: add devargs test cases 2021-10-19 15:07 ` Gaëtan Rivet @ 2021-10-20 7:20 ` Xueming(Steven) Li 0 siblings, 0 replies; 43+ messages in thread From: Xueming(Steven) Li @ 2021-10-20 7:20 UTC (permalink / raw) To: dev, grive; +Cc: NBU-Contact-Thomas Monjalon, david.marchand On Tue, 2021-10-19 at 17:07 +0200, Gaëtan Rivet wrote: > On Tue, Oct 5, 2021, at 17:54, Xueming Li wrote: > > Initial version to test Global devargs syntax. > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > Hi, > > The test is a very nice addition, absolutely required. > I have however two remarks on the coverage and the implementation, below. > > > --- > > app/test/autotest_data.py | 6 ++ > > app/test/meson.build | 2 + > > app/test/test_devargs.c | 147 ++++++++++++++++++++++++++++++++++++++ > > 3 files changed, 155 insertions(+) > > create mode 100644 app/test/test_devargs.c > > > > diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py > > index 302d6374c16..3b841e71918 100644 > > --- a/app/test/autotest_data.py > > +++ b/app/test/autotest_data.py > > @@ -785,6 +785,12 @@ > > "Func": default_autotest, > > "Report": None, > > }, > > + { > > + "Name": "Devargs autotest", > > + "Command": "devargs_autotest", > > + "Func": default_autotest, > > + "Report": None, > > + }, > > # > > # Please always make sure that ring_perf is the last test! > > # > > diff --git a/app/test/meson.build b/app/test/meson.build > > index f144d8b8ed6..de8540f6119 100644 > > --- a/app/test/meson.build > > +++ b/app/test/meson.build > > @@ -42,6 +42,7 @@ test_sources = files( > > 'test_cryptodev_security_pdcp.c', > > 'test_cycles.c', > > 'test_debug.c', > > + 'test_devargs.c', > > 'test_distributor.c', > > 'test_distributor_perf.c', > > 'test_eal_flags.c', > > @@ -201,6 +202,7 @@ fast_tests = [ > > ['common_autotest', true], > > ['cpuflags_autotest', true], > > ['debug_autotest', true], > > + ['devargs_autotest', true], > > ['eal_flags_c_opt_autotest', false], > > ['eal_flags_main_opt_autotest', false], > > ['eal_flags_n_opt_autotest', false], > > diff --git a/app/test/test_devargs.c b/app/test/test_devargs.c > > new file mode 100644 > > index 00000000000..8a173368347 > > --- /dev/null > > +++ b/app/test/test_devargs.c > > @@ -0,0 +1,147 @@ > > +/* SPDX-License-Identifier: BSD-3-Clause > > + * Copyright (c) 2021 NVIDIA Corporation & Affiliates > > + */ > > + > > +#include <stdlib.h> > > +#include <stdio.h> > > +#include <string.h> > > + > > +#include <rte_common.h> > > +#include <rte_devargs.h> > > +#include <rte_kvargs.h> > > + > > +#include "test.h" > > + > > +/* Check layer arguments. */ > > +static int > > +test_args(const char *devargs, const char *layer, const char *args, > > const int n) > > +{ > > + struct rte_kvargs *kvlist; > > + > > + if (n == 0) { > > + if (args != NULL && strlen(args) > 0) { > > + printf("rte_devargs_parse(%s) %s args parsed (not expected)\n", > > + devargs, layer); > > + return -1; > > + } else { > > + return 0; > > + } > > + } > > + if (args == NULL) { > > + printf("rte_devargs_parse(%s) %s args not parsed\n", > > + devargs, layer); > > + return -1; > > + } > > + kvlist = rte_kvargs_parse(args, NULL); > > + if (kvlist == NULL) { > > + printf("rte_devargs_parse(%s) %s_str: %s not parsed\n", > > + devargs, layer, args); > > + return -1; > > + } > > + if ((int)kvlist->count != n) { > > + printf("rte_devargs_parse(%s) %s_str: %s kv number %u, not %d\n", > > + devargs, layer, args, kvlist->count, n); > > + return -1; > > + } > > + return 0; > > +} > > + > > +/* Test several valid cases */ > > +static int > > +test_valid_devargs(void) > > +{ > > + static const struct { > > + const char *devargs; > > + int bus_kv; > > + int class_kv; > > + int driver_kv; > > + } list[] = { > > + /* Global devargs syntax: */ > > + { "bus=pci", 1, 0, 0 }, > > + { "class=eth", 0, 1, 0 }, > > + { "bus=pci,addr=1:2.3/class=eth/driver=abc,k0=v0", 2, 1, 2 }, > > + { "bus=vdev,name=/dev/file/name/class=eth", 2, 1, 0 }, > > + /* Legacy devargs syntax: */ > > + { "1:2.3", 0, 0, 0 }, > > + { "pci:1:2.3,k0=v0", 0, 0, 1 }, > > + { "net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1", > > + 0, 0, 3 }, > > I would add here cases to verify that edge-case are properly parsed such as: > > + { "bus=vdev,name=/class/bus/path/class=eth", 2, 1, 0 }, > [...] > + { "net_virtio_user0,iface=test,path=/class/bus/,queues=1", > + 0, 0, 3 }, > > To check the /class or /bus parts cannot throw off the parser (it does not currently). > > Additionally, paths with multiple slashes are correct. Maybe add: > > + { "bus=vdev,name=///dblslsh/class=eth", 2, 1, 0 }, > > as well. > > I tested all those cases without issue, it seems the parser is ok. > > > A second point is that the test only verifies that the numbers of kv were found. > I think this is not robust enough. Instead, I think the 'expect' part of the test > should describe exactly what each layers should be within the devargs (which bus, > class and driver are expected to be found, and the associated string). > > Right now the parser could mangle part of a key-value list within the string, > recognize each layers and give incorrect strings to each layer parser. > > It might be too much? I don't know. But it seems it could help ensure no error > is introduced at some point by future changes. > Good suggestion, will added them in v2, thanks! ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v2 0/3] devargs: support path in global syntax 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 0/3] devargs: support path in global syntax Xueming Li ` (2 preceding siblings ...) 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 3/3] test/devargs: add devargs test cases Xueming Li @ 2021-10-20 7:31 ` Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 1/3] devargs: support path value for global device arguments Xueming Li ` (2 more replies) 3 siblings, 3 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 7:31 UTC (permalink / raw) To: dev, Gaetan Rivet; +Cc: xuemingl, Thomas Monjalon, Lior Margalit - Support path in global syntax. - Fix bus name resolving - Add devargs test cases v1: initial version v2: - add test cases to test suite - add more test cases, verify device name, bus name and class name Xueming Li (3): devargs: support path value for global device arguments devargs: make bus key parsing optional test/devargs: add devargs test cases app/test/autotest_data.py | 803 ++++++++++++++++++++++++++++ app/test/meson.build | 2 + app/test/test_devargs.c | 184 +++++++ drivers/bus/pci/pci_params.c | 8 +- lib/eal/common/eal_common_devargs.c | 119 ++--- 5 files changed, 1036 insertions(+), 80 deletions(-) create mode 100644 app/test/autotest_data.py create mode 100644 app/test/test_devargs.c -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v2 1/3] devargs: support path value for global device arguments 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 0/3] devargs: support path in global syntax Xueming Li @ 2021-10-20 7:31 ` Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 2/3] devargs: make bus key parsing optional Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 3/3] test/devargs: add devargs test cases Xueming Li 2 siblings, 0 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 7:31 UTC (permalink / raw) To: dev, Gaetan Rivet; +Cc: xuemingl, Thomas Monjalon, Lior Margalit Slash is used to split global device arguments. To support path value which contains slash, this patch parses devargs by locating both slash and layer name key: bus=a,name=/some/path/class=b,k1=v1/driver=c,k2=v2 "/class=" and "/driver" are valid start of a layer. Signed-off-by: Xueming Li <xuemingl@nvidia.com> Reviewed-by: Gaetan Rivet <grive@u256.net> --- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-) diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index 411dd6a75f6..d673598032d 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -29,18 +29,6 @@ TAILQ_HEAD(rte_devargs_list, rte_devargs); static struct rte_devargs_list devargs_list = TAILQ_HEAD_INITIALIZER(devargs_list); -static size_t -devargs_layer_count(const char *s) -{ - size_t i = s ? 1 : 0; - - while (s != NULL && s[0] != '\0') { - i += s[0] == '/'; - s++; - } - return i; -} - /* Resolve devargs name from bus arguments. */ static int devargs_bus_parse_default(struct rte_devargs *devargs, @@ -77,23 +65,13 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, { RTE_DEVARGS_KEY_DRIVER "=", NULL, NULL, }, }; struct rte_kvargs_pair *kv = NULL; - struct rte_class *cls = NULL; - struct rte_bus *bus = NULL; - const char *s = devstr; - size_t nblayer; - size_t i = 0; + struct rte_kvargs *bus_kvlist = NULL; + char *s; + size_t nblayer = 0; + size_t i; int ret = 0; bool allocated_data = false; - /* Split each sub-lists. */ - nblayer = devargs_layer_count(devstr); - if (nblayer > RTE_DIM(layers)) { - RTE_LOG(ERR, EAL, "Invalid format: too many layers (%zu)\n", - nblayer); - ret = -E2BIG; - goto get_out; - } - /* If the devargs points the devstr * as source data, then it should not allocate * anything and keep referring only to it. @@ -106,33 +84,41 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, goto get_out; } allocated_data = true; - s = devargs->data; } + s = devargs->data; while (s != NULL) { - if (i >= RTE_DIM(layers)) { - RTE_LOG(ERR, EAL, "Unrecognized layer %s\n", s); - ret = -EINVAL; + if (nblayer > RTE_DIM(layers)) { + ret = -E2BIG; goto get_out; } - /* - * The last layer is free-form. - * The "driver" key is not required (but accepted). - */ - if (strncmp(layers[i].key, s, strlen(layers[i].key)) && - i != RTE_DIM(layers) - 1) - goto next_layer; - layers[i].str = s; - layers[i].kvlist = rte_kvargs_parse_delim(s, NULL, "/"); - if (layers[i].kvlist == NULL) { + layers[nblayer].str = s; + + /* Locate next layer starts with valid layer key. */ + while (s != NULL) { + s = strchr(s, '/'); + if (s == NULL) + break; + for (i = 0; i < RTE_DIM(layers); i++) { + if (strncmp(s + 1, layers[i].key, + strlen(layers[i].key)) == 0) { + *s = '\0'; + break; + } + } + s++; + if (i < RTE_DIM(layers)) + break; + } + + layers[nblayer].kvlist = rte_kvargs_parse + (layers[nblayer].str, NULL); + if (layers[nblayer].kvlist == NULL) { ret = -EINVAL; goto get_out; } - s = strchr(s, '/'); - if (s != NULL) - s++; -next_layer: - i++; + + nblayer++; } /* Parse each sub-list. */ @@ -143,52 +129,35 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, if (kv->key == NULL) continue; if (strcmp(kv->key, RTE_DEVARGS_KEY_BUS) == 0) { - bus = rte_bus_find_by_name(kv->value); - if (bus == NULL) { + bus_kvlist = layers[i].kvlist; + devargs->bus_str = layers[i].str; + devargs->bus = rte_bus_find_by_name(kv->value); + if (devargs->bus == NULL) { RTE_LOG(ERR, EAL, "Could not find bus \"%s\"\n", kv->value); ret = -EFAULT; goto get_out; } } else if (strcmp(kv->key, RTE_DEVARGS_KEY_CLASS) == 0) { - cls = rte_class_find_by_name(kv->value); - if (cls == NULL) { + devargs->cls_str = layers[i].str; + devargs->cls = rte_class_find_by_name(kv->value); + if (devargs->cls == NULL) { RTE_LOG(ERR, EAL, "Could not find class \"%s\"\n", kv->value); ret = -EFAULT; goto get_out; } } else if (strcmp(kv->key, RTE_DEVARGS_KEY_DRIVER) == 0) { - /* Ignore */ + devargs->drv_str = layers[i].str; continue; } } - /* Fill devargs fields. */ - devargs->bus_str = layers[0].str; - devargs->cls_str = layers[1].str; - devargs->drv_str = layers[2].str; - devargs->bus = bus; - devargs->cls = cls; - - /* If we own the data, clean up a bit - * the several layers string, to ease - * their parsing afterward. - */ - if (devargs->data != devstr) { - char *s = devargs->data; - - while ((s = strchr(s, '/'))) { - *s = '\0'; - s++; - } - } - /* Resolve devargs name. */ - if (bus != NULL && bus->devargs_parse != NULL) - ret = bus->devargs_parse(devargs); - else if (layers[0].kvlist != NULL) - ret = devargs_bus_parse_default(devargs, layers[0].kvlist); + if (devargs->bus != NULL && devargs->bus->devargs_parse != NULL) + ret = devargs->bus->devargs_parse(devargs); + else if (bus_kvlist != NULL) + ret = devargs_bus_parse_default(devargs, bus_kvlist); get_out: for (i = 0; i < RTE_DIM(layers); i++) { -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v2 2/3] devargs: make bus key parsing optional 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 1/3] devargs: support path value for global device arguments Xueming Li @ 2021-10-20 7:31 ` Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 3/3] test/devargs: add devargs test cases Xueming Li 2 siblings, 0 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 7:31 UTC (permalink / raw) To: dev, Gaetan Rivet; +Cc: xuemingl, Thomas Monjalon, Lior Margalit Global devargs syntax is used as device iteration filter like "class=vdpa", a devargs without bus args is valid from parsing perspective. This patch makes bus args optional. Fixes: d2a66ad79480 ("bus: add device arguments name parsing") Signed-off-by: Xueming Li <xuemingl@nvidia.com> Reviewed-by: Gaetan Rivet <grive@u256.net> --- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/bus/pci/pci_params.c b/drivers/bus/pci/pci_params.c index 21c2e1d0368..60b424b8297 100644 --- a/drivers/bus/pci/pci_params.c +++ b/drivers/bus/pci/pci_params.c @@ -87,11 +87,10 @@ rte_pci_devargs_parse(struct rte_devargs *da) struct rte_kvargs *kvargs; const char *addr_str; struct rte_pci_addr addr; - int ret; + int ret = 0; - if (da == NULL) + if (da == NULL || da->bus_str == NULL) return 0; - RTE_ASSERT(da->bus_str != NULL); kvargs = rte_kvargs_parse(da->bus_str, NULL); if (kvargs == NULL) { @@ -103,9 +102,8 @@ rte_pci_devargs_parse(struct rte_devargs *da) addr_str = rte_kvargs_get(kvargs, pci_params_keys[RTE_PCI_PARAM_ADDR]); if (addr_str == NULL) { - RTE_LOG(ERR, EAL, "No PCI address specified using '%s=<id>' in: %s\n", + RTE_LOG(DEBUG, EAL, "No PCI address specified using '%s=<id>' in: %s\n", pci_params_keys[RTE_PCI_PARAM_ADDR], da->bus_str); - ret = -ENODEV; goto out; } diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index d673598032d..8c7650cf6c2 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -39,7 +39,7 @@ devargs_bus_parse_default(struct rte_devargs *devargs, /* Parse devargs name from bus key-value list. */ name = rte_kvargs_get(bus_args, "name"); if (name == NULL) { - RTE_LOG(INFO, EAL, "devargs name not found: %s\n", + RTE_LOG(DEBUG, EAL, "devargs name not found: %s\n", devargs->data); return 0; } -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v2 3/3] test/devargs: add devargs test cases 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 2/3] devargs: make bus key parsing optional Xueming Li @ 2021-10-20 7:31 ` Xueming Li 2021-10-20 7:38 ` David Marchand 2 siblings, 1 reply; 43+ messages in thread From: Xueming Li @ 2021-10-20 7:31 UTC (permalink / raw) To: dev, Gaetan Rivet; +Cc: xuemingl, Thomas Monjalon, Lior Margalit Initial version to test Global devargs syntax. Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- app/test/autotest_data.py | 803 ++++++++++++++++++++++++++++++++++++++ app/test/meson.build | 2 + app/test/test_devargs.c | 184 +++++++++ 3 files changed, 989 insertions(+) create mode 100644 app/test/autotest_data.py create mode 100644 app/test/test_devargs.c diff --git a/app/test/autotest_data.py b/app/test/autotest_data.py new file mode 100644 index 00000000000..3b841e71918 --- /dev/null +++ b/app/test/autotest_data.py @@ -0,0 +1,803 @@ +#!/usr/bin/env python3 +# SPDX-License-Identifier: BSD-3-Clause +# Copyright(c) 2010-2014 Intel Corporation + +# Test data for autotests + +from autotest_test_funcs import * + +# groups of tests that can be run in parallel +# the grouping has been found largely empirically +parallel_test_list = [ + { + "Name": "Timer autotest", + "Command": "timer_autotest", + "Func": timer_autotest, + "Report": None, + }, + { + "Name": "Debug autotest", + "Command": "debug_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Errno autotest", + "Command": "errno_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Meter autotest", + "Command": "meter_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Common autotest", + "Command": "common_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Resource autotest", + "Command": "resource_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Memory autotest", + "Command": "memory_autotest", + "Func": memory_autotest, + "Report": None, + }, + { + "Name": "Read/write lock autotest", + "Command": "rwlock_autotest", + "Func": rwlock_autotest, + "Report": None, + }, + { + "Name": "Lcores autotest", + "Command": "lcores_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Logs autotest", + "Command": "logs_autotest", + "Func": logs_autotest, + "Report": None, + }, + { + "Name": "CPU flags autotest", + "Command": "cpuflags_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Version autotest", + "Command": "version_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "EAL filesystem autotest", + "Command": "eal_fs_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "EAL flags autotest", + "Command": "eal_flags_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Hash autotest", + "Command": "hash_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "LPM autotest", + "Command": "lpm_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "LPM6 autotest", + "Command": "lpm6_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "RIB autotest", + "Command": "rib_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "RIB slow autotest", + "Command": "rib_slow_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "RIB6 autotest", + "Command": "rib6_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "RIB6 slow autotest", + "Command": "rib6_slow_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "FIB autotest", + "Command": "fib_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "FIB slow autotest", + "Command": "fib_slow_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "FIB6 autotest", + "Command": "fib6_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "FIB6 slow autotest", + "Command": "fib6_slow_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Memcpy autotest", + "Command": "memcpy_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Memzone autotest", + "Command": "memzone_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "String autotest", + "Command": "string_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Malloc autotest", + "Command": "malloc_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Multi-process autotest", + "Command": "multiprocess_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Mbuf autotest", + "Command": "mbuf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Per-lcore autotest", + "Command": "per_lcore_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Ring autotest", + "Command": "ring_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Spinlock autotest", + "Command": "spinlock_autotest", + "Func": spinlock_autotest, + "Report": None, + }, + { + "Name": "Ticketlock autotest", + "Command": "ticketlock_autotest", + "Func": ticketlock_autotest, + "Report": None, + }, + { + "Name": "MCSlock autotest", + "Command": "mcslock_autotest", + "Func": mcslock_autotest, + "Report": None, + }, + { + "Name": "Byte order autotest", + "Command": "byteorder_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "TAILQ autotest", + "Command": "tailq_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Command-line autotest", + "Command": "cmdline_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Interrupts autotest", + "Command": "interrupt_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Function reentrancy autotest", + "Command": "func_reentrancy_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Mempool autotest", + "Command": "mempool_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Atomics autotest", + "Command": "atomic_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Prefetch autotest", + "Command": "prefetch_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Red autotest", + "Command": "red_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "PMD ring autotest", + "Command": "ring_pmd_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Access list control autotest", + "Command": "acl_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Sched autotest", + "Command": "sched_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Eventdev selftest octeontx", + "Command": "eventdev_selftest_octeontx", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Event ring autotest", + "Command": "event_ring_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Table autotest", + "Command": "table_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Flow classify autotest", + "Command": "flow_classify_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Event eth rx adapter autotest", + "Command": "event_eth_rx_adapter_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "User delay", + "Command": "user_delay_us", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Rawdev autotest", + "Command": "rawdev_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Kvargs autotest", + "Command": "kvargs_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Link bonding autotest", + "Command": "link_bonding_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Link bonding mode4 autotest", + "Command": "link_bonding_mode4_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Link bonding rssconf autotest", + "Command": "link_bonding_rssconf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Crc autotest", + "Command": "crc_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Distributor autotest", + "Command": "distributor_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Reorder autotest", + "Command": "reorder_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Barrier autotest", + "Command": "barrier_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Bitmap test", + "Command": "bitmap_test", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Bitops test", + "Command": "bitops_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Hash multiwriter autotest", + "Command": "hash_multiwriter_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Service autotest", + "Command": "service_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Timer racecond autotest", + "Command": "timer_racecond_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Member autotest", + "Command": "member_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Efd_autotest", + "Command": "efd_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Thash autotest", + "Command": "thash_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Hash function autotest", + "Command": "hash_functions_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev sw mvsam autotest", + "Command": "cryptodev_sw_mvsam_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev dpaa2 sec autotest", + "Command": "cryptodev_dpaa2_sec_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev dpaa sec autotest", + "Command": "cryptodev_dpaa_sec_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev qat autotest", + "Command": "cryptodev_qat_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev aesni mb autotest", + "Command": "cryptodev_aesni_mb_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev openssl autotest", + "Command": "cryptodev_openssl_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev scheduler autotest", + "Command": "cryptodev_scheduler_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev aesni gcm autotest", + "Command": "cryptodev_aesni_gcm_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev null autotest", + "Command": "cryptodev_null_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev sw snow3g autotest", + "Command": "cryptodev_sw_snow3g_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev sw kasumi autotest", + "Command": "cryptodev_sw_kasumi_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Cryptodev_sw_zuc_autotest", + "Command": "cryptodev_sw_zuc_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Reciprocal division", + "Command": "reciprocal_division", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Red all", + "Command": "red_all", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Fbarray autotest", + "Command": "fbarray_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "External memory autotest", + "Command": "external_mem_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Metrics autotest", + "Command": "metrics_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Bitratestats autotest", + "Command": "bitratestats_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Latencystats autotest", + "Command": "latencystats_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Pdump autotest", + "Command": "pdump_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "IPsec_SAD", + "Command": "ipsec_sad_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Checksum autotest", + "Command": "cksum_autotest", + "Func": default_autotest, + "Report": None, + }, + # + #Please always keep all dump tests at the end and together! + # + { + "Name": "Dump physmem", + "Command": "dump_physmem", + "Func": dump_autotest, + "Report": None, + }, + { + "Name": "Dump memzone", + "Command": "dump_memzone", + "Func": dump_autotest, + "Report": None, + }, + { + "Name": "Dump struct sizes", + "Command": "dump_struct_sizes", + "Func": dump_autotest, + "Report": None, + }, + { + "Name": "Dump mempool", + "Command": "dump_mempool", + "Func": dump_autotest, + "Report": None, + }, + { + "Name": "Dump malloc stats", + "Command": "dump_malloc_stats", + "Func": dump_autotest, + "Report": None, + }, + { + "Name": "Dump devargs", + "Command": "dump_devargs", + "Func": dump_autotest, + "Report": None, + }, + { + "Name": "Dump log types", + "Command": "dump_log_types", + "Func": dump_autotest, + "Report": None, + }, + { + "Name": "Dump_ring", + "Command": "dump_ring", + "Func": dump_autotest, + "Report": None, + }, +] + +# tests that should not be run when any other tests are running +non_parallel_test_list = [ + { + "Name": "Eventdev common autotest", + "Command": "eventdev_common_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Eventdev selftest sw", + "Command": "eventdev_selftest_sw", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "KNI autotest", + "Command": "kni_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Mempool performance autotest", + "Command": "mempool_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Memcpy performance autotest", + "Command": "memcpy_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Hash performance autotest", + "Command": "hash_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Hash read-write concurrency functional autotest", + "Command": "hash_readwrite_func_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Hash read-write concurrency perf autotest", + "Command": "hash_readwrite_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Hash read-write lock-free concurrency perf autotest", + "Command": "hash_readwrite_lf_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Power autotest", + "Command": "power_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Power cpufreq autotest", + "Command": "power_cpufreq_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Power KVM VM autotest", + "Command": "power_kvm_vm_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Timer performance autotest", + "Command": "timer_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + + "Name": "Pmd perf autotest", + "Command": "pmd_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Ring pmd perf autotest", + "Command": "ring_pmd_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Distributor perf autotest", + "Command": "distributor_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Red_perf", + "Command": "red_perf", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Lpm6 perf autotest", + "Command": "lpm6_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Lpm perf autotest", + "Command": "lpm_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "FIB perf autotest", + "Command": "fib_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "FIB6 perf autotest", + "Command": "fib6_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Efd perf autotest", + "Command": "efd_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Member perf autotest", + "Command": "member_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Reciprocal division perf", + "Command": "reciprocal_division_perf", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "RCU QSBR autotest", + "Command": "rcu_qsbr_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "RCU QSBR performance autotest", + "Command": "rcu_qsbr_perf_autotest", + "Func": default_autotest, + "Report": None, + }, + { + "Name": "Devargs autotest", + "Command": "devargs_autotest", + "Func": default_autotest, + "Report": None, + }, + # + # Please always make sure that ring_perf is the last test! + # + { + "Name": "Ring performance autotest", + "Command": "ring_perf_autotest", + "Func": default_autotest, + "Report": None, + }, +] diff --git a/app/test/meson.build b/app/test/meson.build index a16374b7a10..842388b0d32 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -42,6 +42,7 @@ test_sources = files( 'test_cryptodev_security_pdcp.c', 'test_cycles.c', 'test_debug.c', + 'test_devargs.c', 'test_distributor.c', 'test_distributor_perf.c', 'test_dmadev.c', @@ -204,6 +205,7 @@ fast_tests = [ ['common_autotest', true], ['cpuflags_autotest', true], ['debug_autotest', true], + ['devargs_autotest', true], ['eal_flags_c_opt_autotest', false], ['eal_flags_main_opt_autotest', false], ['eal_flags_n_opt_autotest', false], diff --git a/app/test/test_devargs.c b/app/test/test_devargs.c new file mode 100644 index 00000000000..13e95f052b0 --- /dev/null +++ b/app/test/test_devargs.c @@ -0,0 +1,184 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright (c) 2021 NVIDIA Corporation & Affiliates + */ + +#include <stdlib.h> +#include <stdio.h> +#include <string.h> + +#include <rte_common.h> +#include <rte_devargs.h> +#include <rte_kvargs.h> +#include <rte_bus.h> +#include <rte_class.h> + +#include "test.h" + +/* Check layer arguments. */ +static int +test_args(const char *devargs, const char *layer, const char *args, const int n) +{ + struct rte_kvargs *kvlist; + + if (n == 0) { + if (args != NULL && strlen(args) > 0) { + printf("rte_devargs_parse(%s) %s args parsed (not expected)\n", + devargs, layer); + return -1; + } else { + return 0; + } + } + if (args == NULL) { + printf("rte_devargs_parse(%s) %s args not parsed\n", + devargs, layer); + return -1; + } + kvlist = rte_kvargs_parse(args, NULL); + if (kvlist == NULL) { + printf("rte_devargs_parse(%s) %s_str: %s not parsed\n", + devargs, layer, args); + return -1; + } + if ((int)kvlist->count != n) { + printf("rte_devargs_parse(%s) %s_str: %s kv number %u, not %d\n", + devargs, layer, args, kvlist->count, n); + return -1; + } + return 0; +} + +/* Test several valid cases */ +static int +test_valid_devargs(void) +{ + static const struct { + const char *devargs; + int bus_kv; + int class_kv; + int driver_kv; + const char *bus; + const char *name; + const char *class; + } list[] = { + /* Global devargs syntax: */ + { "bus=pci", + 1, 0, 0, "pci", NULL, NULL}, + { "class=eth", + 0, 1, 0, NULL, NULL, "eth" }, + { "bus=pci,addr=1:2.3/class=eth/driver=abc,k0=v0", + 2, 1, 2, "pci", "0000:01:02.3", "eth" }, + { "bus=vdev,name=/dev/file/name/class=eth", + 2, 1, 0, "vdev", "/dev/file/name", "eth" }, + { "bus=vdev,name=/class/bus/path/class=eth", + 2, 1, 0, "vdev", "/class/bus/path", "eth" }, + { "bus=vdev,name=///dblslsh/class=eth", + 2, 1, 0, "vdev", "///dblslsh", "eth" }, + /* Legacy devargs syntax: */ + { "1:2.3", 0, 0, 0, + "pci", "1:2.3", NULL }, + { "pci:1:2.3,k0=v0", + 0, 0, 1, "pci", "1:2.3", NULL }, + { "net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1", + 0, 0, 3, "vdev", "net_virtio_user0", NULL }, + { "net_virtio_user0,iface=test,path=/class/bus/,queues=1", + 0, 0, 3, "vdev", "net_virtio_user0", NULL }, + }; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + memset(&da, 0, sizeof(da)); + ret = rte_devargs_parse(&da, list[i].devargs); + if (ret < 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i].devargs, ret); + goto fail; + } + if ((list[i].bus_kv > 0 || list[i].bus != NULL) && + da.bus == NULL) { + printf("rte_devargs_parse(%s) bus not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "bus", da.bus_str, + list[i].bus_kv) != 0) + goto fail; + if (list[i].bus != NULL && + strcmp(da.bus->name, list[i].bus) != 0) { + printf("rte_devargs_parse(%s) bus name (%s) not expected (%s)\n", + list[i].devargs, da.bus->name, list[i].bus); + goto fail; + } + if ((list[i].class_kv > 0 || list[i].class != NULL) && + da.cls == NULL) { + printf("rte_devargs_parse(%s) class not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "class", da.cls_str, + list[i].class_kv) != 0) + goto fail; + if (list[i].class != NULL && + strcmp(da.cls->name, list[i].class) != 0) { + printf("rte_devargs_parse(%s) class name (%s) not expected (%s)\n", + list[i].devargs, da.cls->name, list[i].class); + goto fail; + } + if (test_args(list[i].devargs, "driver", da.drv_str, + list[i].driver_kv) != 0) + goto fail; + if (list[i].name != NULL && + strcmp(da.name, list[i].name) != 0) { + printf("rte_devargs_parse(%s) device name (%s) not expected (%s)\n", + list[i].devargs, da.name, list[i].name); + goto fail; + } + goto cleanup; +fail: + fail = -1; +cleanup: + rte_devargs_reset(&da); + } + return fail; +} + +/* Test several invalid cases */ +static int +test_invalid_devargs(void) +{ + static const char * const list[] = { + "bus=wrong-bus", + "class=wrong-class"}; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + ret = rte_devargs_parse(&da, list[i]); + if (ret >= 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i], ret); + fail = ret; + } + rte_devargs_reset(&da); + } + return fail; +} + +static int +test_devargs(void) +{ + printf("== test valid case ==\n"); + if (test_valid_devargs() < 0) + return -1; + printf("== test invalid case ==\n"); + if (test_invalid_devargs() < 0) + return -1; + return 0; +} + +REGISTER_TEST_COMMAND(devargs_autotest, test_devargs); -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/3] test/devargs: add devargs test cases 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 3/3] test/devargs: add devargs test cases Xueming Li @ 2021-10-20 7:38 ` David Marchand 2021-10-20 8:23 ` Xueming(Steven) Li 0 siblings, 1 reply; 43+ messages in thread From: David Marchand @ 2021-10-20 7:38 UTC (permalink / raw) To: Xueming Li; +Cc: dev, Gaetan Rivet, Thomas Monjalon, Lior Margalit On Wed, Oct 20, 2021 at 9:32 AM Xueming Li <xuemingl@nvidia.com> wrote: > > Initial version to test Global devargs syntax. > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > --- > app/test/autotest_data.py | 803 ++++++++++++++++++++++++++++++++++++++ This file is getting reintroduced by your patch. We dropped it recently: https://git.dpdk.org/dpdk/commit/?id=8c745bb62340e7b6a3ad61e5d79dfceebd4c28e4 > app/test/meson.build | 2 + > app/test/test_devargs.c | 184 +++++++++ > 3 files changed, 989 insertions(+) > create mode 100644 app/test/autotest_data.py > create mode 100644 app/test/test_devargs.c > -- David Marchand ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v2 3/3] test/devargs: add devargs test cases 2021-10-20 7:38 ` David Marchand @ 2021-10-20 8:23 ` Xueming(Steven) Li 0 siblings, 0 replies; 43+ messages in thread From: Xueming(Steven) Li @ 2021-10-20 8:23 UTC (permalink / raw) To: david.marchand; +Cc: Lior Margalit, NBU-Contact-Thomas Monjalon, dev, grive On Wed, 2021-10-20 at 09:38 +0200, David Marchand wrote: > On Wed, Oct 20, 2021 at 9:32 AM Xueming Li <xuemingl@nvidia.com> wrote: > > > > Initial version to test Global devargs syntax. > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > --- > > app/test/autotest_data.py | 803 ++++++++++++++++++++++++++++++++++++++ > > This file is getting reintroduced by your patch. > We dropped it recently: > https://git.dpdk.org/dpdk/commit/?id=8c745bb62340e7b6a3ad61e5d79dfceebd4c28e4 My bad, forgot to remove it :) new version posted. > > > > app/test/meson.build | 2 + > > app/test/test_devargs.c | 184 +++++++++ > > 3 files changed, 989 insertions(+) > > create mode 100644 app/test/autotest_data.py > > create mode 100644 app/test/test_devargs.c > > > > ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v3 0/3] devargs: support path in global syntax 2021-10-05 12:30 [dpdk-dev] [PATCH 1/3] devargs: support path value for global device arguments Xueming Li ` (2 preceding siblings ...) 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 0/3] devargs: support path in global syntax Xueming Li @ 2021-10-20 8:21 ` Xueming Li 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 1/3] devargs: support path value for global device arguments Xueming Li ` (2 more replies) 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax Xueming Li 5 siblings, 3 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 8:21 UTC (permalink / raw) To: dev, Gaetan Rivet; +Cc: xuemingl, Thomas Monjalon, Lior Margalit - Support path in global syntax. - Fix bus name resolving - Add devargs test cases v1: initial version v2: - add test cases to test suite - add more test cases, verify device name, bus name and class name v3: - remove autotest_data.py Xueming Li (3): devargs: support path value for global device arguments devargs: make bus key parsing optional test/devargs: add devargs test cases app/test/meson.build | 2 + app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++ drivers/bus/pci/pci_params.c | 8 +- lib/eal/common/eal_common_devargs.c | 119 +++++++----------- 4 files changed, 233 insertions(+), 80 deletions(-) create mode 100644 app/test/test_devargs.c -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v3 1/3] devargs: support path value for global device arguments 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 0/3] devargs: support path in global syntax Xueming Li @ 2021-10-20 8:21 ` Xueming Li 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 2/3] devargs: make bus key parsing optional Xueming Li 2021-10-20 8:22 ` [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases Xueming Li 2 siblings, 0 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 8:21 UTC (permalink / raw) To: dev, Gaetan Rivet; +Cc: xuemingl, Thomas Monjalon, Lior Margalit Slash is used to split global device arguments. To support path value which contains slash, this patch parses devargs by locating both slash and layer name key: bus=a,name=/some/path/class=b,k1=v1/driver=c,k2=v2 "/class=" and "/driver" are valid start of a layer. Signed-off-by: Xueming Li <xuemingl@nvidia.com> Reviewed-by: Gaetan Rivet <grive@u256.net> --- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-) diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index 411dd6a75f6..d673598032d 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -29,18 +29,6 @@ TAILQ_HEAD(rte_devargs_list, rte_devargs); static struct rte_devargs_list devargs_list = TAILQ_HEAD_INITIALIZER(devargs_list); -static size_t -devargs_layer_count(const char *s) -{ - size_t i = s ? 1 : 0; - - while (s != NULL && s[0] != '\0') { - i += s[0] == '/'; - s++; - } - return i; -} - /* Resolve devargs name from bus arguments. */ static int devargs_bus_parse_default(struct rte_devargs *devargs, @@ -77,23 +65,13 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, { RTE_DEVARGS_KEY_DRIVER "=", NULL, NULL, }, }; struct rte_kvargs_pair *kv = NULL; - struct rte_class *cls = NULL; - struct rte_bus *bus = NULL; - const char *s = devstr; - size_t nblayer; - size_t i = 0; + struct rte_kvargs *bus_kvlist = NULL; + char *s; + size_t nblayer = 0; + size_t i; int ret = 0; bool allocated_data = false; - /* Split each sub-lists. */ - nblayer = devargs_layer_count(devstr); - if (nblayer > RTE_DIM(layers)) { - RTE_LOG(ERR, EAL, "Invalid format: too many layers (%zu)\n", - nblayer); - ret = -E2BIG; - goto get_out; - } - /* If the devargs points the devstr * as source data, then it should not allocate * anything and keep referring only to it. @@ -106,33 +84,41 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, goto get_out; } allocated_data = true; - s = devargs->data; } + s = devargs->data; while (s != NULL) { - if (i >= RTE_DIM(layers)) { - RTE_LOG(ERR, EAL, "Unrecognized layer %s\n", s); - ret = -EINVAL; + if (nblayer > RTE_DIM(layers)) { + ret = -E2BIG; goto get_out; } - /* - * The last layer is free-form. - * The "driver" key is not required (but accepted). - */ - if (strncmp(layers[i].key, s, strlen(layers[i].key)) && - i != RTE_DIM(layers) - 1) - goto next_layer; - layers[i].str = s; - layers[i].kvlist = rte_kvargs_parse_delim(s, NULL, "/"); - if (layers[i].kvlist == NULL) { + layers[nblayer].str = s; + + /* Locate next layer starts with valid layer key. */ + while (s != NULL) { + s = strchr(s, '/'); + if (s == NULL) + break; + for (i = 0; i < RTE_DIM(layers); i++) { + if (strncmp(s + 1, layers[i].key, + strlen(layers[i].key)) == 0) { + *s = '\0'; + break; + } + } + s++; + if (i < RTE_DIM(layers)) + break; + } + + layers[nblayer].kvlist = rte_kvargs_parse + (layers[nblayer].str, NULL); + if (layers[nblayer].kvlist == NULL) { ret = -EINVAL; goto get_out; } - s = strchr(s, '/'); - if (s != NULL) - s++; -next_layer: - i++; + + nblayer++; } /* Parse each sub-list. */ @@ -143,52 +129,35 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, if (kv->key == NULL) continue; if (strcmp(kv->key, RTE_DEVARGS_KEY_BUS) == 0) { - bus = rte_bus_find_by_name(kv->value); - if (bus == NULL) { + bus_kvlist = layers[i].kvlist; + devargs->bus_str = layers[i].str; + devargs->bus = rte_bus_find_by_name(kv->value); + if (devargs->bus == NULL) { RTE_LOG(ERR, EAL, "Could not find bus \"%s\"\n", kv->value); ret = -EFAULT; goto get_out; } } else if (strcmp(kv->key, RTE_DEVARGS_KEY_CLASS) == 0) { - cls = rte_class_find_by_name(kv->value); - if (cls == NULL) { + devargs->cls_str = layers[i].str; + devargs->cls = rte_class_find_by_name(kv->value); + if (devargs->cls == NULL) { RTE_LOG(ERR, EAL, "Could not find class \"%s\"\n", kv->value); ret = -EFAULT; goto get_out; } } else if (strcmp(kv->key, RTE_DEVARGS_KEY_DRIVER) == 0) { - /* Ignore */ + devargs->drv_str = layers[i].str; continue; } } - /* Fill devargs fields. */ - devargs->bus_str = layers[0].str; - devargs->cls_str = layers[1].str; - devargs->drv_str = layers[2].str; - devargs->bus = bus; - devargs->cls = cls; - - /* If we own the data, clean up a bit - * the several layers string, to ease - * their parsing afterward. - */ - if (devargs->data != devstr) { - char *s = devargs->data; - - while ((s = strchr(s, '/'))) { - *s = '\0'; - s++; - } - } - /* Resolve devargs name. */ - if (bus != NULL && bus->devargs_parse != NULL) - ret = bus->devargs_parse(devargs); - else if (layers[0].kvlist != NULL) - ret = devargs_bus_parse_default(devargs, layers[0].kvlist); + if (devargs->bus != NULL && devargs->bus->devargs_parse != NULL) + ret = devargs->bus->devargs_parse(devargs); + else if (bus_kvlist != NULL) + ret = devargs_bus_parse_default(devargs, bus_kvlist); get_out: for (i = 0; i < RTE_DIM(layers); i++) { -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v3 2/3] devargs: make bus key parsing optional 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 1/3] devargs: support path value for global device arguments Xueming Li @ 2021-10-20 8:21 ` Xueming Li 2021-10-20 8:22 ` [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases Xueming Li 2 siblings, 0 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 8:21 UTC (permalink / raw) To: dev, Gaetan Rivet; +Cc: xuemingl, Thomas Monjalon, Lior Margalit Global devargs syntax is used as device iteration filter like "class=vdpa", a devargs without bus args is valid from parsing perspective. This patch makes bus args optional. Fixes: d2a66ad79480 ("bus: add device arguments name parsing") Signed-off-by: Xueming Li <xuemingl@nvidia.com> Reviewed-by: Gaetan Rivet <grive@u256.net> --- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/bus/pci/pci_params.c b/drivers/bus/pci/pci_params.c index 21c2e1d0368..60b424b8297 100644 --- a/drivers/bus/pci/pci_params.c +++ b/drivers/bus/pci/pci_params.c @@ -87,11 +87,10 @@ rte_pci_devargs_parse(struct rte_devargs *da) struct rte_kvargs *kvargs; const char *addr_str; struct rte_pci_addr addr; - int ret; + int ret = 0; - if (da == NULL) + if (da == NULL || da->bus_str == NULL) return 0; - RTE_ASSERT(da->bus_str != NULL); kvargs = rte_kvargs_parse(da->bus_str, NULL); if (kvargs == NULL) { @@ -103,9 +102,8 @@ rte_pci_devargs_parse(struct rte_devargs *da) addr_str = rte_kvargs_get(kvargs, pci_params_keys[RTE_PCI_PARAM_ADDR]); if (addr_str == NULL) { - RTE_LOG(ERR, EAL, "No PCI address specified using '%s=<id>' in: %s\n", + RTE_LOG(DEBUG, EAL, "No PCI address specified using '%s=<id>' in: %s\n", pci_params_keys[RTE_PCI_PARAM_ADDR], da->bus_str); - ret = -ENODEV; goto out; } diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index d673598032d..8c7650cf6c2 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -39,7 +39,7 @@ devargs_bus_parse_default(struct rte_devargs *devargs, /* Parse devargs name from bus key-value list. */ name = rte_kvargs_get(bus_args, "name"); if (name == NULL) { - RTE_LOG(INFO, EAL, "devargs name not found: %s\n", + RTE_LOG(DEBUG, EAL, "devargs name not found: %s\n", devargs->data); return 0; } -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 2/3] devargs: make bus key parsing optional Xueming Li @ 2021-10-20 8:22 ` Xueming Li 2021-10-20 9:08 ` David Marchand 2 siblings, 1 reply; 43+ messages in thread From: Xueming Li @ 2021-10-20 8:22 UTC (permalink / raw) To: dev, Gaetan Rivet; +Cc: xuemingl, Thomas Monjalon, Lior Margalit Initial version to test Global devargs syntax. Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- app/test/meson.build | 2 + app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 app/test/test_devargs.c diff --git a/app/test/meson.build b/app/test/meson.build index a16374b7a10..842388b0d32 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -42,6 +42,7 @@ test_sources = files( 'test_cryptodev_security_pdcp.c', 'test_cycles.c', 'test_debug.c', + 'test_devargs.c', 'test_distributor.c', 'test_distributor_perf.c', 'test_dmadev.c', @@ -204,6 +205,7 @@ fast_tests = [ ['common_autotest', true], ['cpuflags_autotest', true], ['debug_autotest', true], + ['devargs_autotest', true], ['eal_flags_c_opt_autotest', false], ['eal_flags_main_opt_autotest', false], ['eal_flags_n_opt_autotest', false], diff --git a/app/test/test_devargs.c b/app/test/test_devargs.c new file mode 100644 index 00000000000..13e95f052b0 --- /dev/null +++ b/app/test/test_devargs.c @@ -0,0 +1,184 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright (c) 2021 NVIDIA Corporation & Affiliates + */ + +#include <stdlib.h> +#include <stdio.h> +#include <string.h> + +#include <rte_common.h> +#include <rte_devargs.h> +#include <rte_kvargs.h> +#include <rte_bus.h> +#include <rte_class.h> + +#include "test.h" + +/* Check layer arguments. */ +static int +test_args(const char *devargs, const char *layer, const char *args, const int n) +{ + struct rte_kvargs *kvlist; + + if (n == 0) { + if (args != NULL && strlen(args) > 0) { + printf("rte_devargs_parse(%s) %s args parsed (not expected)\n", + devargs, layer); + return -1; + } else { + return 0; + } + } + if (args == NULL) { + printf("rte_devargs_parse(%s) %s args not parsed\n", + devargs, layer); + return -1; + } + kvlist = rte_kvargs_parse(args, NULL); + if (kvlist == NULL) { + printf("rte_devargs_parse(%s) %s_str: %s not parsed\n", + devargs, layer, args); + return -1; + } + if ((int)kvlist->count != n) { + printf("rte_devargs_parse(%s) %s_str: %s kv number %u, not %d\n", + devargs, layer, args, kvlist->count, n); + return -1; + } + return 0; +} + +/* Test several valid cases */ +static int +test_valid_devargs(void) +{ + static const struct { + const char *devargs; + int bus_kv; + int class_kv; + int driver_kv; + const char *bus; + const char *name; + const char *class; + } list[] = { + /* Global devargs syntax: */ + { "bus=pci", + 1, 0, 0, "pci", NULL, NULL}, + { "class=eth", + 0, 1, 0, NULL, NULL, "eth" }, + { "bus=pci,addr=1:2.3/class=eth/driver=abc,k0=v0", + 2, 1, 2, "pci", "0000:01:02.3", "eth" }, + { "bus=vdev,name=/dev/file/name/class=eth", + 2, 1, 0, "vdev", "/dev/file/name", "eth" }, + { "bus=vdev,name=/class/bus/path/class=eth", + 2, 1, 0, "vdev", "/class/bus/path", "eth" }, + { "bus=vdev,name=///dblslsh/class=eth", + 2, 1, 0, "vdev", "///dblslsh", "eth" }, + /* Legacy devargs syntax: */ + { "1:2.3", 0, 0, 0, + "pci", "1:2.3", NULL }, + { "pci:1:2.3,k0=v0", + 0, 0, 1, "pci", "1:2.3", NULL }, + { "net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1", + 0, 0, 3, "vdev", "net_virtio_user0", NULL }, + { "net_virtio_user0,iface=test,path=/class/bus/,queues=1", + 0, 0, 3, "vdev", "net_virtio_user0", NULL }, + }; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + memset(&da, 0, sizeof(da)); + ret = rte_devargs_parse(&da, list[i].devargs); + if (ret < 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i].devargs, ret); + goto fail; + } + if ((list[i].bus_kv > 0 || list[i].bus != NULL) && + da.bus == NULL) { + printf("rte_devargs_parse(%s) bus not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "bus", da.bus_str, + list[i].bus_kv) != 0) + goto fail; + if (list[i].bus != NULL && + strcmp(da.bus->name, list[i].bus) != 0) { + printf("rte_devargs_parse(%s) bus name (%s) not expected (%s)\n", + list[i].devargs, da.bus->name, list[i].bus); + goto fail; + } + if ((list[i].class_kv > 0 || list[i].class != NULL) && + da.cls == NULL) { + printf("rte_devargs_parse(%s) class not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "class", da.cls_str, + list[i].class_kv) != 0) + goto fail; + if (list[i].class != NULL && + strcmp(da.cls->name, list[i].class) != 0) { + printf("rte_devargs_parse(%s) class name (%s) not expected (%s)\n", + list[i].devargs, da.cls->name, list[i].class); + goto fail; + } + if (test_args(list[i].devargs, "driver", da.drv_str, + list[i].driver_kv) != 0) + goto fail; + if (list[i].name != NULL && + strcmp(da.name, list[i].name) != 0) { + printf("rte_devargs_parse(%s) device name (%s) not expected (%s)\n", + list[i].devargs, da.name, list[i].name); + goto fail; + } + goto cleanup; +fail: + fail = -1; +cleanup: + rte_devargs_reset(&da); + } + return fail; +} + +/* Test several invalid cases */ +static int +test_invalid_devargs(void) +{ + static const char * const list[] = { + "bus=wrong-bus", + "class=wrong-class"}; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + ret = rte_devargs_parse(&da, list[i]); + if (ret >= 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i], ret); + fail = ret; + } + rte_devargs_reset(&da); + } + return fail; +} + +static int +test_devargs(void) +{ + printf("== test valid case ==\n"); + if (test_valid_devargs() < 0) + return -1; + printf("== test invalid case ==\n"); + if (test_invalid_devargs() < 0) + return -1; + return 0; +} + +REGISTER_TEST_COMMAND(devargs_autotest, test_devargs); -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases 2021-10-20 8:22 ` [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases Xueming Li @ 2021-10-20 9:08 ` David Marchand 2021-10-20 9:40 ` Xueming(Steven) Li 0 siblings, 1 reply; 43+ messages in thread From: David Marchand @ 2021-10-20 9:08 UTC (permalink / raw) To: Xueming Li, Thomas Monjalon; +Cc: dev, Gaetan Rivet, Lior Margalit On Wed, Oct 20, 2021 at 10:22 AM Xueming Li <xuemingl@nvidia.com> wrote: > > Initial version to test Global devargs syntax. > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> 10/95 DPDK:fast-tests / devargs_autotest FAIL 0.17 s (exit status 255 or signal 127 SIGinvalid) --- command --- DPDK_TEST='devargs_autotest' /home/runner/work/dpdk/dpdk/build/app/test/dpdk-test -l 0-1 --file-prefix=devargs_autotest --- stdout --- RTE>>devargs_autotest == test valid case == rte_devargs_parse(net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1) returned -14 (but should not) rte_devargs_parse(net_virtio_user0,iface=test,path=/class/bus/,queues=1) returned -14 (but should not) Test Failed RTE>> --- stderr --- EAL: Detected CPU lcores: 2 EAL: Detected NUMA nodes: 1 EAL: Detected shared linkage of DPDK EAL: Multi-process socket /var/run/dpdk/devargs_autotest/mp_socket EAL: Selected IOVA mode 'PA' EAL: No available 1048576 kB hugepages reported EAL: VFIO support initialized APP: HPET is not enabled, using TSC as default timer EAL: failed to parse device "net_virtio_user0" EAL: failed to parse device "net_virtio_user0" ------- -- David Marchand ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases 2021-10-20 9:08 ` David Marchand @ 2021-10-20 9:40 ` Xueming(Steven) Li 2021-10-20 10:41 ` Bruce Richardson 0 siblings, 1 reply; 43+ messages in thread From: Xueming(Steven) Li @ 2021-10-20 9:40 UTC (permalink / raw) To: NBU-Contact-Thomas Monjalon, david.marchand; +Cc: Lior Margalit, dev, grive On Wed, 2021-10-20 at 11:08 +0200, David Marchand wrote: > On Wed, Oct 20, 2021 at 10:22 AM Xueming Li <xuemingl@nvidia.com> wrote: > > > > Initial version to test Global devargs syntax. > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > 10/95 DPDK:fast-tests / devargs_autotest FAIL 0.17 s (exit > status 255 or signal 127 SIGinvalid) > > --- command --- > DPDK_TEST='devargs_autotest' > /home/runner/work/dpdk/dpdk/build/app/test/dpdk-test -l 0-1 > --file-prefix=devargs_autotest > --- stdout --- > RTE>>devargs_autotest > == test valid case == > rte_devargs_parse(net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1) > returned -14 (but should not) > rte_devargs_parse(net_virtio_user0,iface=test,path=/class/bus/,queues=1) > returned -14 (but should not) > Test Failed > RTE>> > --- stderr --- > EAL: Detected CPU lcores: 2 > EAL: Detected NUMA nodes: 1 > EAL: Detected shared linkage of DPDK > EAL: Multi-process socket /var/run/dpdk/devargs_autotest/mp_socket > EAL: Selected IOVA mode 'PA' > EAL: No available 1048576 kB hugepages reported > EAL: VFIO support initialized > APP: HPET is not enabled, using TSC as default timer > EAL: failed to parse device "net_virtio_user0" > EAL: failed to parse device "net_virtio_user0" Yes, noticed that, seems virtio driver not enabled. Tried to add "net_virtio" to test_deps in meson file, but failed with: ../app/test/meson.build:444:4: ERROR: Tried to get unknown variable "static_rte_net_virtio". Seems meson scripts only lookup dependency on libs, I'm not good at meson, any suggestion? > ------- > > ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases 2021-10-20 9:40 ` Xueming(Steven) Li @ 2021-10-20 10:41 ` Bruce Richardson 0 siblings, 0 replies; 43+ messages in thread From: Bruce Richardson @ 2021-10-20 10:41 UTC (permalink / raw) To: Xueming(Steven) Li Cc: NBU-Contact-Thomas Monjalon, david.marchand, Lior Margalit, dev, grive On Wed, Oct 20, 2021 at 09:40:55AM +0000, Xueming(Steven) Li wrote: > On Wed, 2021-10-20 at 11:08 +0200, David Marchand wrote: > > On Wed, Oct 20, 2021 at 10:22 AM Xueming Li <xuemingl@nvidia.com> wrote: > > > > > > Initial version to test Global devargs syntax. > > > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > > > 10/95 DPDK:fast-tests / devargs_autotest FAIL 0.17 s (exit > > status 255 or signal 127 SIGinvalid) > > > > --- command --- > > DPDK_TEST='devargs_autotest' > > /home/runner/work/dpdk/dpdk/build/app/test/dpdk-test -l 0-1 > > --file-prefix=devargs_autotest > > --- stdout --- > > RTE>>devargs_autotest > > == test valid case == > > rte_devargs_parse(net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1) > > returned -14 (but should not) > > rte_devargs_parse(net_virtio_user0,iface=test,path=/class/bus/,queues=1) > > returned -14 (but should not) > > Test Failed > > RTE>> > > --- stderr --- > > EAL: Detected CPU lcores: 2 > > EAL: Detected NUMA nodes: 1 > > EAL: Detected shared linkage of DPDK > > EAL: Multi-process socket /var/run/dpdk/devargs_autotest/mp_socket > > EAL: Selected IOVA mode 'PA' > > EAL: No available 1048576 kB hugepages reported > > EAL: VFIO support initialized > > APP: HPET is not enabled, using TSC as default timer > > EAL: failed to parse device "net_virtio_user0" > > EAL: failed to parse device "net_virtio_user0" > > Yes, noticed that, seems virtio driver not enabled. Tried to add > "net_virtio" to test_deps in meson file, but failed with: > ../app/test/meson.build:444:4: ERROR: Tried to get unknown variable > "static_rte_net_virtio". > > Seems meson scripts only lookup dependency on libs, I'm not good at > meson, any suggestion? > "net_virtio" is correct for the virtio driver. However, you probably need to do like is done for the other network drivers linked into the autotest, and make it conditional on the support being present, since not every build enables the virtio driver. For example, as is currently in the meson.build for the autotests: # The following linkages of drivers are required because # they are used via a driver-specific API. if dpdk_conf.has('RTE_NET_BOND') test_deps += 'net_bond' test_sources += ['test_link_bonding.c', 'test_link_bonding_rssconf.c'] ... endif /Bruce ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v4 0/3] devargs: support path in global syntax 2021-10-05 12:30 [dpdk-dev] [PATCH 1/3] devargs: support path value for global device arguments Xueming Li ` (3 preceding siblings ...) 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 0/3] devargs: support path in global syntax Xueming Li @ 2021-10-20 11:12 ` Xueming Li 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 1/3] devargs: support path value for global device arguments Xueming Li ` (2 more replies) 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax Xueming Li 5 siblings, 3 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 11:12 UTC (permalink / raw) To: dev, Gaetan Rivet, David Marchand Cc: xuemingl, Thomas Monjalon, Lior Margalit - Support path in global syntax. - Fix bus name resolving - Add devargs test cases v1: initial version v2: - add test cases to test suite - add more test cases, verify device name, bus name and class name v3: - remove autotest_data.py v4: - make devargs test depends on virtio driver Email thread: 20211005123012.264727-1-xuemingl@nvidia.com Xueming Li (3): devargs: support path value for global device arguments devargs: make bus key parsing optional test/devargs: add devargs test cases app/test/meson.build | 5 + app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++ drivers/bus/pci/pci_params.c | 8 +- lib/eal/common/eal_common_devargs.c | 119 +++++++----------- 4 files changed, 236 insertions(+), 80 deletions(-) create mode 100644 app/test/test_devargs.c -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v4 1/3] devargs: support path value for global device arguments 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 0/3] devargs: support path in global syntax Xueming Li @ 2021-10-20 11:12 ` Xueming Li 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 2/3] devargs: make bus key parsing optional Xueming Li 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases Xueming Li 2 siblings, 0 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 11:12 UTC (permalink / raw) To: dev, Gaetan Rivet, David Marchand Cc: xuemingl, Thomas Monjalon, Lior Margalit Slash is used to split global device arguments. To support path value which contains slash, this patch parses devargs by locating both slash and layer name key: bus=a,name=/some/path/class=b,k1=v1/driver=c,k2=v2 "/class=" and "/driver" are valid start of a layer. Signed-off-by: Xueming Li <xuemingl@nvidia.com> Reviewed-by: Gaetan Rivet <grive@u256.net> --- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-) diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index 411dd6a75f6..d673598032d 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -29,18 +29,6 @@ TAILQ_HEAD(rte_devargs_list, rte_devargs); static struct rte_devargs_list devargs_list = TAILQ_HEAD_INITIALIZER(devargs_list); -static size_t -devargs_layer_count(const char *s) -{ - size_t i = s ? 1 : 0; - - while (s != NULL && s[0] != '\0') { - i += s[0] == '/'; - s++; - } - return i; -} - /* Resolve devargs name from bus arguments. */ static int devargs_bus_parse_default(struct rte_devargs *devargs, @@ -77,23 +65,13 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, { RTE_DEVARGS_KEY_DRIVER "=", NULL, NULL, }, }; struct rte_kvargs_pair *kv = NULL; - struct rte_class *cls = NULL; - struct rte_bus *bus = NULL; - const char *s = devstr; - size_t nblayer; - size_t i = 0; + struct rte_kvargs *bus_kvlist = NULL; + char *s; + size_t nblayer = 0; + size_t i; int ret = 0; bool allocated_data = false; - /* Split each sub-lists. */ - nblayer = devargs_layer_count(devstr); - if (nblayer > RTE_DIM(layers)) { - RTE_LOG(ERR, EAL, "Invalid format: too many layers (%zu)\n", - nblayer); - ret = -E2BIG; - goto get_out; - } - /* If the devargs points the devstr * as source data, then it should not allocate * anything and keep referring only to it. @@ -106,33 +84,41 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, goto get_out; } allocated_data = true; - s = devargs->data; } + s = devargs->data; while (s != NULL) { - if (i >= RTE_DIM(layers)) { - RTE_LOG(ERR, EAL, "Unrecognized layer %s\n", s); - ret = -EINVAL; + if (nblayer > RTE_DIM(layers)) { + ret = -E2BIG; goto get_out; } - /* - * The last layer is free-form. - * The "driver" key is not required (but accepted). - */ - if (strncmp(layers[i].key, s, strlen(layers[i].key)) && - i != RTE_DIM(layers) - 1) - goto next_layer; - layers[i].str = s; - layers[i].kvlist = rte_kvargs_parse_delim(s, NULL, "/"); - if (layers[i].kvlist == NULL) { + layers[nblayer].str = s; + + /* Locate next layer starts with valid layer key. */ + while (s != NULL) { + s = strchr(s, '/'); + if (s == NULL) + break; + for (i = 0; i < RTE_DIM(layers); i++) { + if (strncmp(s + 1, layers[i].key, + strlen(layers[i].key)) == 0) { + *s = '\0'; + break; + } + } + s++; + if (i < RTE_DIM(layers)) + break; + } + + layers[nblayer].kvlist = rte_kvargs_parse + (layers[nblayer].str, NULL); + if (layers[nblayer].kvlist == NULL) { ret = -EINVAL; goto get_out; } - s = strchr(s, '/'); - if (s != NULL) - s++; -next_layer: - i++; + + nblayer++; } /* Parse each sub-list. */ @@ -143,52 +129,35 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, if (kv->key == NULL) continue; if (strcmp(kv->key, RTE_DEVARGS_KEY_BUS) == 0) { - bus = rte_bus_find_by_name(kv->value); - if (bus == NULL) { + bus_kvlist = layers[i].kvlist; + devargs->bus_str = layers[i].str; + devargs->bus = rte_bus_find_by_name(kv->value); + if (devargs->bus == NULL) { RTE_LOG(ERR, EAL, "Could not find bus \"%s\"\n", kv->value); ret = -EFAULT; goto get_out; } } else if (strcmp(kv->key, RTE_DEVARGS_KEY_CLASS) == 0) { - cls = rte_class_find_by_name(kv->value); - if (cls == NULL) { + devargs->cls_str = layers[i].str; + devargs->cls = rte_class_find_by_name(kv->value); + if (devargs->cls == NULL) { RTE_LOG(ERR, EAL, "Could not find class \"%s\"\n", kv->value); ret = -EFAULT; goto get_out; } } else if (strcmp(kv->key, RTE_DEVARGS_KEY_DRIVER) == 0) { - /* Ignore */ + devargs->drv_str = layers[i].str; continue; } } - /* Fill devargs fields. */ - devargs->bus_str = layers[0].str; - devargs->cls_str = layers[1].str; - devargs->drv_str = layers[2].str; - devargs->bus = bus; - devargs->cls = cls; - - /* If we own the data, clean up a bit - * the several layers string, to ease - * their parsing afterward. - */ - if (devargs->data != devstr) { - char *s = devargs->data; - - while ((s = strchr(s, '/'))) { - *s = '\0'; - s++; - } - } - /* Resolve devargs name. */ - if (bus != NULL && bus->devargs_parse != NULL) - ret = bus->devargs_parse(devargs); - else if (layers[0].kvlist != NULL) - ret = devargs_bus_parse_default(devargs, layers[0].kvlist); + if (devargs->bus != NULL && devargs->bus->devargs_parse != NULL) + ret = devargs->bus->devargs_parse(devargs); + else if (bus_kvlist != NULL) + ret = devargs_bus_parse_default(devargs, bus_kvlist); get_out: for (i = 0; i < RTE_DIM(layers); i++) { -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v4 2/3] devargs: make bus key parsing optional 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 1/3] devargs: support path value for global device arguments Xueming Li @ 2021-10-20 11:12 ` Xueming Li 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases Xueming Li 2 siblings, 0 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 11:12 UTC (permalink / raw) To: dev, Gaetan Rivet, David Marchand Cc: xuemingl, Thomas Monjalon, Lior Margalit Global devargs syntax is used as device iteration filter like "class=vdpa", a devargs without bus args is valid from parsing perspective. This patch makes bus args optional. Fixes: d2a66ad79480 ("bus: add device arguments name parsing") Signed-off-by: Xueming Li <xuemingl@nvidia.com> Reviewed-by: Gaetan Rivet <grive@u256.net> --- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/bus/pci/pci_params.c b/drivers/bus/pci/pci_params.c index 21c2e1d0368..60b424b8297 100644 --- a/drivers/bus/pci/pci_params.c +++ b/drivers/bus/pci/pci_params.c @@ -87,11 +87,10 @@ rte_pci_devargs_parse(struct rte_devargs *da) struct rte_kvargs *kvargs; const char *addr_str; struct rte_pci_addr addr; - int ret; + int ret = 0; - if (da == NULL) + if (da == NULL || da->bus_str == NULL) return 0; - RTE_ASSERT(da->bus_str != NULL); kvargs = rte_kvargs_parse(da->bus_str, NULL); if (kvargs == NULL) { @@ -103,9 +102,8 @@ rte_pci_devargs_parse(struct rte_devargs *da) addr_str = rte_kvargs_get(kvargs, pci_params_keys[RTE_PCI_PARAM_ADDR]); if (addr_str == NULL) { - RTE_LOG(ERR, EAL, "No PCI address specified using '%s=<id>' in: %s\n", + RTE_LOG(DEBUG, EAL, "No PCI address specified using '%s=<id>' in: %s\n", pci_params_keys[RTE_PCI_PARAM_ADDR], da->bus_str); - ret = -ENODEV; goto out; } diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index d673598032d..8c7650cf6c2 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -39,7 +39,7 @@ devargs_bus_parse_default(struct rte_devargs *devargs, /* Parse devargs name from bus key-value list. */ name = rte_kvargs_get(bus_args, "name"); if (name == NULL) { - RTE_LOG(INFO, EAL, "devargs name not found: %s\n", + RTE_LOG(DEBUG, EAL, "devargs name not found: %s\n", devargs->data); return 0; } -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 2/3] devargs: make bus key parsing optional Xueming Li @ 2021-10-20 11:12 ` Xueming Li 2021-10-20 11:55 ` Gaëtan Rivet 2 siblings, 1 reply; 43+ messages in thread From: Xueming Li @ 2021-10-20 11:12 UTC (permalink / raw) To: dev, Gaetan Rivet, David Marchand Cc: xuemingl, Thomas Monjalon, Lior Margalit Initial version to test global devargs syntax. Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- app/test/meson.build | 5 ++ app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100644 app/test/test_devargs.c diff --git a/app/test/meson.build b/app/test/meson.build index a16374b7a10..c4b0241010d 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -399,6 +399,11 @@ if dpdk_conf.has('RTE_NET_RING') fast_tests += [['latencystats_autotest', true]] fast_tests += [['pdump_autotest', true]] endif +if dpdk_conf.has('RTE_NET_VIRTIO') + test_deps += 'net_virtio' + test_sources += 'test_devargs.c' + fast_tests += [['devargs_autotest', true]] +endif if dpdk_conf.has('RTE_LIB_POWER') test_deps += 'power' diff --git a/app/test/test_devargs.c b/app/test/test_devargs.c new file mode 100644 index 00000000000..13e95f052b0 --- /dev/null +++ b/app/test/test_devargs.c @@ -0,0 +1,184 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright (c) 2021 NVIDIA Corporation & Affiliates + */ + +#include <stdlib.h> +#include <stdio.h> +#include <string.h> + +#include <rte_common.h> +#include <rte_devargs.h> +#include <rte_kvargs.h> +#include <rte_bus.h> +#include <rte_class.h> + +#include "test.h" + +/* Check layer arguments. */ +static int +test_args(const char *devargs, const char *layer, const char *args, const int n) +{ + struct rte_kvargs *kvlist; + + if (n == 0) { + if (args != NULL && strlen(args) > 0) { + printf("rte_devargs_parse(%s) %s args parsed (not expected)\n", + devargs, layer); + return -1; + } else { + return 0; + } + } + if (args == NULL) { + printf("rte_devargs_parse(%s) %s args not parsed\n", + devargs, layer); + return -1; + } + kvlist = rte_kvargs_parse(args, NULL); + if (kvlist == NULL) { + printf("rte_devargs_parse(%s) %s_str: %s not parsed\n", + devargs, layer, args); + return -1; + } + if ((int)kvlist->count != n) { + printf("rte_devargs_parse(%s) %s_str: %s kv number %u, not %d\n", + devargs, layer, args, kvlist->count, n); + return -1; + } + return 0; +} + +/* Test several valid cases */ +static int +test_valid_devargs(void) +{ + static const struct { + const char *devargs; + int bus_kv; + int class_kv; + int driver_kv; + const char *bus; + const char *name; + const char *class; + } list[] = { + /* Global devargs syntax: */ + { "bus=pci", + 1, 0, 0, "pci", NULL, NULL}, + { "class=eth", + 0, 1, 0, NULL, NULL, "eth" }, + { "bus=pci,addr=1:2.3/class=eth/driver=abc,k0=v0", + 2, 1, 2, "pci", "0000:01:02.3", "eth" }, + { "bus=vdev,name=/dev/file/name/class=eth", + 2, 1, 0, "vdev", "/dev/file/name", "eth" }, + { "bus=vdev,name=/class/bus/path/class=eth", + 2, 1, 0, "vdev", "/class/bus/path", "eth" }, + { "bus=vdev,name=///dblslsh/class=eth", + 2, 1, 0, "vdev", "///dblslsh", "eth" }, + /* Legacy devargs syntax: */ + { "1:2.3", 0, 0, 0, + "pci", "1:2.3", NULL }, + { "pci:1:2.3,k0=v0", + 0, 0, 1, "pci", "1:2.3", NULL }, + { "net_virtio_user0,iface=test,path=/dev/vhost-net,queues=1", + 0, 0, 3, "vdev", "net_virtio_user0", NULL }, + { "net_virtio_user0,iface=test,path=/class/bus/,queues=1", + 0, 0, 3, "vdev", "net_virtio_user0", NULL }, + }; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + memset(&da, 0, sizeof(da)); + ret = rte_devargs_parse(&da, list[i].devargs); + if (ret < 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i].devargs, ret); + goto fail; + } + if ((list[i].bus_kv > 0 || list[i].bus != NULL) && + da.bus == NULL) { + printf("rte_devargs_parse(%s) bus not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "bus", da.bus_str, + list[i].bus_kv) != 0) + goto fail; + if (list[i].bus != NULL && + strcmp(da.bus->name, list[i].bus) != 0) { + printf("rte_devargs_parse(%s) bus name (%s) not expected (%s)\n", + list[i].devargs, da.bus->name, list[i].bus); + goto fail; + } + if ((list[i].class_kv > 0 || list[i].class != NULL) && + da.cls == NULL) { + printf("rte_devargs_parse(%s) class not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "class", da.cls_str, + list[i].class_kv) != 0) + goto fail; + if (list[i].class != NULL && + strcmp(da.cls->name, list[i].class) != 0) { + printf("rte_devargs_parse(%s) class name (%s) not expected (%s)\n", + list[i].devargs, da.cls->name, list[i].class); + goto fail; + } + if (test_args(list[i].devargs, "driver", da.drv_str, + list[i].driver_kv) != 0) + goto fail; + if (list[i].name != NULL && + strcmp(da.name, list[i].name) != 0) { + printf("rte_devargs_parse(%s) device name (%s) not expected (%s)\n", + list[i].devargs, da.name, list[i].name); + goto fail; + } + goto cleanup; +fail: + fail = -1; +cleanup: + rte_devargs_reset(&da); + } + return fail; +} + +/* Test several invalid cases */ +static int +test_invalid_devargs(void) +{ + static const char * const list[] = { + "bus=wrong-bus", + "class=wrong-class"}; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + ret = rte_devargs_parse(&da, list[i]); + if (ret >= 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i], ret); + fail = ret; + } + rte_devargs_reset(&da); + } + return fail; +} + +static int +test_devargs(void) +{ + printf("== test valid case ==\n"); + if (test_valid_devargs() < 0) + return -1; + printf("== test invalid case ==\n"); + if (test_invalid_devargs() < 0) + return -1; + return 0; +} + +REGISTER_TEST_COMMAND(devargs_autotest, test_devargs); -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases Xueming Li @ 2021-10-20 11:55 ` Gaëtan Rivet 2021-10-20 13:53 ` Xueming(Steven) Li 0 siblings, 1 reply; 43+ messages in thread From: Gaëtan Rivet @ 2021-10-20 11:55 UTC (permalink / raw) To: Xueming(Steven) Li, dev, David Marchand; +Cc: Thomas Monjalon, Lior Margalit On Wed, Oct 20, 2021, at 13:12, Xueming Li wrote: > Initial version to test global devargs syntax. > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > --- > app/test/meson.build | 5 ++ > app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ > 2 files changed, 189 insertions(+) > create mode 100644 app/test/test_devargs.c > > diff --git a/app/test/meson.build b/app/test/meson.build > index a16374b7a10..c4b0241010d 100644 > --- a/app/test/meson.build > +++ b/app/test/meson.build > @@ -399,6 +399,11 @@ if dpdk_conf.has('RTE_NET_RING') > fast_tests += [['latencystats_autotest', true]] > fast_tests += [['pdump_autotest', true]] > endif > +if dpdk_conf.has('RTE_NET_VIRTIO') > + test_deps += 'net_virtio' > + test_sources += 'test_devargs.c' > + fast_tests += [['devargs_autotest', true]] > +endif Hi Steven, Thanks for adding new use-cases and expanding the expect. The dep check for the test can be improved I think. When the build is shared, not all built drivers will be available, even if part of the meson config. It might generate false negative when running your test. Additionally, even if net_virtio is not built, a subset of your test remains valid. Finally, net_virtio might not be generic enough as a driver. A simpler one could be used, such as net_ring maybe. Instead of checking the dependencies in the meson file, you could detect support in preamble of the test: bool pci_bus_avail = (rte_bus_find_by_name("pci") != NULL); bool vdev_bus_avail = (rte_bus_find_by_name("vdev") != NULL); bool eth_class_avail = [...] Then during the test case, depending on the expect part of list[i], you can skip the case if its deps were not found. In that case, am INFO or DEBUG level message could be printed to say that the test was skipped. -- Gaetan Rivet ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases 2021-10-20 11:55 ` Gaëtan Rivet @ 2021-10-20 13:53 ` Xueming(Steven) Li 2021-10-20 14:22 ` Gaëtan Rivet 0 siblings, 1 reply; 43+ messages in thread From: Xueming(Steven) Li @ 2021-10-20 13:53 UTC (permalink / raw) To: dev, david.marchand, grive; +Cc: Lior Margalit, NBU-Contact-Thomas Monjalon On Wed, 2021-10-20 at 13:55 +0200, Gaëtan Rivet wrote: > On Wed, Oct 20, 2021, at 13:12, Xueming Li wrote: > > Initial version to test global devargs syntax. > > > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > > --- > > app/test/meson.build | 5 ++ > > app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ > > 2 files changed, 189 insertions(+) > > create mode 100644 app/test/test_devargs.c > > > > diff --git a/app/test/meson.build b/app/test/meson.build > > index a16374b7a10..c4b0241010d 100644 > > --- a/app/test/meson.build > > +++ b/app/test/meson.build > > @@ -399,6 +399,11 @@ if dpdk_conf.has('RTE_NET_RING') > > fast_tests += [['latencystats_autotest', true]] > > fast_tests += [['pdump_autotest', true]] > > endif > > +if dpdk_conf.has('RTE_NET_VIRTIO') > > + test_deps += 'net_virtio' > > + test_sources += 'test_devargs.c' > > + fast_tests += [['devargs_autotest', true]] > > +endif > > Hi Steven, > > Thanks for adding new use-cases and expanding the expect. > The dep check for the test can be improved I think. > > When the build is shared, not all built drivers will be available, even if part of the meson config. It might generate false negative when running your test. > > Additionally, even if net_virtio is not built, a subset of your test remains valid. > > Finally, net_virtio might not be generic enough as a driver. A simpler one could be used, such as net_ring maybe. > > Instead of checking the dependencies in the meson file, you could detect support in preamble > of the test: > > bool pci_bus_avail = (rte_bus_find_by_name("pci") != NULL); > bool vdev_bus_avail = (rte_bus_find_by_name("vdev") != NULL); > bool eth_class_avail = [...] > > Then during the test case, depending on the expect part of list[i], > you can skip the case if its deps were not found. In that case, am INFO or DEBUG level > message could be printed to say that the test was skipped. > For vdev supported drivers, currently no API to find it. Macro worked for me: #ifdef RTE_NET_VIRTIO { "net_virtio_user0,iface=test,path=/dev/vhost- net,queues=1", 0, 0, 3, "vdev", "net_virtio_user0", NULL }, { "net_virtio_user0,iface=test,path=/class/bus/,queues=1", 0, 0, 3, "vdev", "net_virtio_user0", NULL }, #endif PCI, vdev and eth is enabled by default, seems we don't need any flag. But macros can be used as well to be safe, how do you think? ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases 2021-10-20 13:53 ` Xueming(Steven) Li @ 2021-10-20 14:22 ` Gaëtan Rivet 2021-10-20 14:34 ` Bruce Richardson 0 siblings, 1 reply; 43+ messages in thread From: Gaëtan Rivet @ 2021-10-20 14:22 UTC (permalink / raw) To: Xueming(Steven) Li, dev, David Marchand; +Cc: Lior Margalit, Thomas Monjalon On Wed, Oct 20, 2021, at 15:53, Xueming(Steven) Li wrote: > On Wed, 2021-10-20 at 13:55 +0200, Gaëtan Rivet wrote: >> On Wed, Oct 20, 2021, at 13:12, Xueming Li wrote: >> > Initial version to test global devargs syntax. >> > >> > Signed-off-by: Xueming Li <xuemingl@nvidia.com> >> > --- >> > app/test/meson.build | 5 ++ >> > app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ >> > 2 files changed, 189 insertions(+) >> > create mode 100644 app/test/test_devargs.c >> > >> > diff --git a/app/test/meson.build b/app/test/meson.build >> > index a16374b7a10..c4b0241010d 100644 >> > --- a/app/test/meson.build >> > +++ b/app/test/meson.build >> > @@ -399,6 +399,11 @@ if dpdk_conf.has('RTE_NET_RING') >> > fast_tests += [['latencystats_autotest', true]] >> > fast_tests += [['pdump_autotest', true]] >> > endif >> > +if dpdk_conf.has('RTE_NET_VIRTIO') >> > + test_deps += 'net_virtio' >> > + test_sources += 'test_devargs.c' >> > + fast_tests += [['devargs_autotest', true]] >> > +endif >> >> Hi Steven, >> >> Thanks for adding new use-cases and expanding the expect. >> The dep check for the test can be improved I think. >> >> When the build is shared, not all built drivers will be available, even if part of the meson config. It might generate false negative when running your test. >> >> Additionally, even if net_virtio is not built, a subset of your test remains valid. >> >> Finally, net_virtio might not be generic enough as a driver. A simpler one could be used, such as net_ring maybe. >> >> Instead of checking the dependencies in the meson file, you could detect support in preamble >> of the test: >> >> bool pci_bus_avail = (rte_bus_find_by_name("pci") != NULL); >> bool vdev_bus_avail = (rte_bus_find_by_name("vdev") != NULL); >> bool eth_class_avail = [...] >> >> Then during the test case, depending on the expect part of list[i], >> you can skip the case if its deps were not found. In that case, am INFO or DEBUG level >> message could be printed to say that the test was skipped. >> > > For vdev supported drivers, currently no API to find it. Macro worked > for me: > > #ifdef RTE_NET_VIRTIO > { "net_virtio_user0,iface=test,path=/dev/vhost- > net,queues=1", > 0, 0, 3, "vdev", "net_virtio_user0", NULL }, > { > "net_virtio_user0,iface=test,path=/class/bus/,queues=1", > 0, 0, 3, "vdev", "net_virtio_user0", NULL }, > #endif > > PCI, vdev and eth is enabled by default, seems we don't need any flag. > But macros can be used as well to be safe, how do you think? With the macro you will go back to the same issue the meson-based dep check has: the macro will be set, but it's possible the relevant .so won't be loaded when executing the test. For Busses and classes, similarly is there a chance that even though they are built, they won't be loaded at runtime and make the test result incorrect? What do you think about switching from net_virtio to net_ring instead otherwise? It seems a lighter, utility driver that might be present even in minimal builds. It is used in EAL tests, I think it makes more sense than net_virtio for unit tests. The virtio-specific path-based parameters are irrelevant there, the actual driver probe won't be executed anyway so any parameter looking like a path will test properly. Checkout app/test/test_eal_flags.c for examples of net_ring dummy devargs. -- Gaetan Rivet ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases 2021-10-20 14:22 ` Gaëtan Rivet @ 2021-10-20 14:34 ` Bruce Richardson 0 siblings, 0 replies; 43+ messages in thread From: Bruce Richardson @ 2021-10-20 14:34 UTC (permalink / raw) To: Gaëtan Rivet Cc: Xueming(Steven) Li, dev, David Marchand, Lior Margalit, Thomas Monjalon On Wed, Oct 20, 2021 at 04:22:54PM +0200, Gaëtan Rivet wrote: > > > On Wed, Oct 20, 2021, at 15:53, Xueming(Steven) Li wrote: > > On Wed, 2021-10-20 at 13:55 +0200, Gaëtan Rivet wrote: > >> On Wed, Oct 20, 2021, at 13:12, Xueming Li wrote: > >> > Initial version to test global devargs syntax. > >> > > >> > Signed-off-by: Xueming Li <xuemingl@nvidia.com> > >> > --- > >> > app/test/meson.build | 5 ++ > >> > app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ > >> > 2 files changed, 189 insertions(+) > >> > create mode 100644 app/test/test_devargs.c > >> > > >> > diff --git a/app/test/meson.build b/app/test/meson.build > >> > index a16374b7a10..c4b0241010d 100644 > >> > --- a/app/test/meson.build > >> > +++ b/app/test/meson.build > >> > @@ -399,6 +399,11 @@ if dpdk_conf.has('RTE_NET_RING') > >> > fast_tests += [['latencystats_autotest', true]] > >> > fast_tests += [['pdump_autotest', true]] > >> > endif > >> > +if dpdk_conf.has('RTE_NET_VIRTIO') > >> > + test_deps += 'net_virtio' > >> > + test_sources += 'test_devargs.c' > >> > + fast_tests += [['devargs_autotest', true]] > >> > +endif > >> > >> Hi Steven, > >> > >> Thanks for adding new use-cases and expanding the expect. > >> The dep check for the test can be improved I think. > >> > >> When the build is shared, not all built drivers will be available, even if part of the meson config. It might generate false negative when running your test. > >> > >> Additionally, even if net_virtio is not built, a subset of your test remains valid. > >> > >> Finally, net_virtio might not be generic enough as a driver. A simpler one could be used, such as net_ring maybe. > >> > >> Instead of checking the dependencies in the meson file, you could detect support in preamble > >> of the test: > >> > >> bool pci_bus_avail = (rte_bus_find_by_name("pci") != NULL); > >> bool vdev_bus_avail = (rte_bus_find_by_name("vdev") != NULL); > >> bool eth_class_avail = [...] > >> > >> Then during the test case, depending on the expect part of list[i], > >> you can skip the case if its deps were not found. In that case, am INFO or DEBUG level > >> message could be printed to say that the test was skipped. > >> > > > > For vdev supported drivers, currently no API to find it. Macro worked > > for me: > > > > #ifdef RTE_NET_VIRTIO > > { "net_virtio_user0,iface=test,path=/dev/vhost- > > net,queues=1", > > 0, 0, 3, "vdev", "net_virtio_user0", NULL }, > > { > > "net_virtio_user0,iface=test,path=/class/bus/,queues=1", > > 0, 0, 3, "vdev", "net_virtio_user0", NULL }, > > #endif > > > > PCI, vdev and eth is enabled by default, seems we don't need any flag. > > But macros can be used as well to be safe, how do you think? > > With the macro you will go back to the same issue the meson-based dep check has: > the macro will be set, but it's possible the relevant .so won't be loaded when > executing the test. > > For Busses and classes, similarly is there a chance that even though they are built, > they won't be loaded at runtime and make the test result incorrect? > > What do you think about switching from net_virtio to net_ring instead otherwise? > It seems a lighter, utility driver that might be present even in minimal builds. > It is used in EAL tests, I think it makes more sense than net_virtio for unit tests. > > The virtio-specific path-based parameters are irrelevant there, the actual > driver probe won't be executed anyway so any parameter looking like a path will > test properly. > > Checkout app/test/test_eal_flags.c for examples of net_ring dummy devargs. > I'd also recommend making use of the "TEST_SKIPPED" return value rather than erroring out if a necessary dependency can't be found. ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax 2021-10-05 12:30 [dpdk-dev] [PATCH 1/3] devargs: support path value for global device arguments Xueming Li ` (4 preceding siblings ...) 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 0/3] devargs: support path in global syntax Xueming Li @ 2021-10-20 15:47 ` Xueming Li 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 1/3] devargs: support path value for global device arguments Xueming Li ` (3 more replies) 5 siblings, 4 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 15:47 UTC (permalink / raw) To: dev, Gaetan Rivet, David Marchand Cc: xuemingl, Thomas Monjalon, Lior Margalit - Support path in global syntax. - Fix bus name resolving - Add devargs test cases v1: initial version v2: - add test cases to test suite - add more test cases, verify device name, bus name and class name v3: - remove autotest_data.py v4: - make devargs test depends on virtio driver v5: - use api to filter out unsupported devarg test cases Email thread: 20211005123012.264727-1-xuemingl@nvidia.com Xueming Li (3): devargs: support path value for global device arguments devargs: make bus key parsing optional test/devargs: add devargs test cases app/test/meson.build | 2 + app/test/test_devargs.c | 214 ++++++++++++++++++++++++++++ drivers/bus/pci/pci_params.c | 8 +- lib/eal/common/eal_common_devargs.c | 119 ++++++---------- 4 files changed, 263 insertions(+), 80 deletions(-) create mode 100644 app/test/test_devargs.c -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v5 1/3] devargs: support path value for global device arguments 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax Xueming Li @ 2021-10-20 15:47 ` Xueming Li 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 2/3] devargs: make bus key parsing optional Xueming Li ` (2 subsequent siblings) 3 siblings, 0 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 15:47 UTC (permalink / raw) To: dev, Gaetan Rivet, David Marchand Cc: xuemingl, Thomas Monjalon, Lior Margalit Slash is used to split global device arguments. To support path value which contains slash, this patch parses devargs by locating both slash and layer name key: bus=a,name=/some/path/class=b,k1=v1/driver=c,k2=v2 "/class=" and "/driver" are valid start of a layer. Signed-off-by: Xueming Li <xuemingl@nvidia.com> Reviewed-by: Gaetan Rivet <grive@u256.net> --- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-) diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index 411dd6a75f6..d673598032d 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -29,18 +29,6 @@ TAILQ_HEAD(rte_devargs_list, rte_devargs); static struct rte_devargs_list devargs_list = TAILQ_HEAD_INITIALIZER(devargs_list); -static size_t -devargs_layer_count(const char *s) -{ - size_t i = s ? 1 : 0; - - while (s != NULL && s[0] != '\0') { - i += s[0] == '/'; - s++; - } - return i; -} - /* Resolve devargs name from bus arguments. */ static int devargs_bus_parse_default(struct rte_devargs *devargs, @@ -77,23 +65,13 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, { RTE_DEVARGS_KEY_DRIVER "=", NULL, NULL, }, }; struct rte_kvargs_pair *kv = NULL; - struct rte_class *cls = NULL; - struct rte_bus *bus = NULL; - const char *s = devstr; - size_t nblayer; - size_t i = 0; + struct rte_kvargs *bus_kvlist = NULL; + char *s; + size_t nblayer = 0; + size_t i; int ret = 0; bool allocated_data = false; - /* Split each sub-lists. */ - nblayer = devargs_layer_count(devstr); - if (nblayer > RTE_DIM(layers)) { - RTE_LOG(ERR, EAL, "Invalid format: too many layers (%zu)\n", - nblayer); - ret = -E2BIG; - goto get_out; - } - /* If the devargs points the devstr * as source data, then it should not allocate * anything and keep referring only to it. @@ -106,33 +84,41 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, goto get_out; } allocated_data = true; - s = devargs->data; } + s = devargs->data; while (s != NULL) { - if (i >= RTE_DIM(layers)) { - RTE_LOG(ERR, EAL, "Unrecognized layer %s\n", s); - ret = -EINVAL; + if (nblayer > RTE_DIM(layers)) { + ret = -E2BIG; goto get_out; } - /* - * The last layer is free-form. - * The "driver" key is not required (but accepted). - */ - if (strncmp(layers[i].key, s, strlen(layers[i].key)) && - i != RTE_DIM(layers) - 1) - goto next_layer; - layers[i].str = s; - layers[i].kvlist = rte_kvargs_parse_delim(s, NULL, "/"); - if (layers[i].kvlist == NULL) { + layers[nblayer].str = s; + + /* Locate next layer starts with valid layer key. */ + while (s != NULL) { + s = strchr(s, '/'); + if (s == NULL) + break; + for (i = 0; i < RTE_DIM(layers); i++) { + if (strncmp(s + 1, layers[i].key, + strlen(layers[i].key)) == 0) { + *s = '\0'; + break; + } + } + s++; + if (i < RTE_DIM(layers)) + break; + } + + layers[nblayer].kvlist = rte_kvargs_parse + (layers[nblayer].str, NULL); + if (layers[nblayer].kvlist == NULL) { ret = -EINVAL; goto get_out; } - s = strchr(s, '/'); - if (s != NULL) - s++; -next_layer: - i++; + + nblayer++; } /* Parse each sub-list. */ @@ -143,52 +129,35 @@ rte_devargs_layers_parse(struct rte_devargs *devargs, if (kv->key == NULL) continue; if (strcmp(kv->key, RTE_DEVARGS_KEY_BUS) == 0) { - bus = rte_bus_find_by_name(kv->value); - if (bus == NULL) { + bus_kvlist = layers[i].kvlist; + devargs->bus_str = layers[i].str; + devargs->bus = rte_bus_find_by_name(kv->value); + if (devargs->bus == NULL) { RTE_LOG(ERR, EAL, "Could not find bus \"%s\"\n", kv->value); ret = -EFAULT; goto get_out; } } else if (strcmp(kv->key, RTE_DEVARGS_KEY_CLASS) == 0) { - cls = rte_class_find_by_name(kv->value); - if (cls == NULL) { + devargs->cls_str = layers[i].str; + devargs->cls = rte_class_find_by_name(kv->value); + if (devargs->cls == NULL) { RTE_LOG(ERR, EAL, "Could not find class \"%s\"\n", kv->value); ret = -EFAULT; goto get_out; } } else if (strcmp(kv->key, RTE_DEVARGS_KEY_DRIVER) == 0) { - /* Ignore */ + devargs->drv_str = layers[i].str; continue; } } - /* Fill devargs fields. */ - devargs->bus_str = layers[0].str; - devargs->cls_str = layers[1].str; - devargs->drv_str = layers[2].str; - devargs->bus = bus; - devargs->cls = cls; - - /* If we own the data, clean up a bit - * the several layers string, to ease - * their parsing afterward. - */ - if (devargs->data != devstr) { - char *s = devargs->data; - - while ((s = strchr(s, '/'))) { - *s = '\0'; - s++; - } - } - /* Resolve devargs name. */ - if (bus != NULL && bus->devargs_parse != NULL) - ret = bus->devargs_parse(devargs); - else if (layers[0].kvlist != NULL) - ret = devargs_bus_parse_default(devargs, layers[0].kvlist); + if (devargs->bus != NULL && devargs->bus->devargs_parse != NULL) + ret = devargs->bus->devargs_parse(devargs); + else if (bus_kvlist != NULL) + ret = devargs_bus_parse_default(devargs, bus_kvlist); get_out: for (i = 0; i < RTE_DIM(layers); i++) { -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v5 2/3] devargs: make bus key parsing optional 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 1/3] devargs: support path value for global device arguments Xueming Li @ 2021-10-20 15:47 ` Xueming Li 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases Xueming Li 2021-10-21 9:22 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax David Marchand 3 siblings, 0 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 15:47 UTC (permalink / raw) To: dev, Gaetan Rivet, David Marchand Cc: xuemingl, Thomas Monjalon, Lior Margalit Global devargs syntax is used as device iteration filter like "class=vdpa", a devargs without bus args is valid from parsing perspective. This patch makes bus args optional. Fixes: d2a66ad79480 ("bus: add device arguments name parsing") Signed-off-by: Xueming Li <xuemingl@nvidia.com> Reviewed-by: Gaetan Rivet <grive@u256.net> --- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/drivers/bus/pci/pci_params.c b/drivers/bus/pci/pci_params.c index 21c2e1d0368..60b424b8297 100644 --- a/drivers/bus/pci/pci_params.c +++ b/drivers/bus/pci/pci_params.c @@ -87,11 +87,10 @@ rte_pci_devargs_parse(struct rte_devargs *da) struct rte_kvargs *kvargs; const char *addr_str; struct rte_pci_addr addr; - int ret; + int ret = 0; - if (da == NULL) + if (da == NULL || da->bus_str == NULL) return 0; - RTE_ASSERT(da->bus_str != NULL); kvargs = rte_kvargs_parse(da->bus_str, NULL); if (kvargs == NULL) { @@ -103,9 +102,8 @@ rte_pci_devargs_parse(struct rte_devargs *da) addr_str = rte_kvargs_get(kvargs, pci_params_keys[RTE_PCI_PARAM_ADDR]); if (addr_str == NULL) { - RTE_LOG(ERR, EAL, "No PCI address specified using '%s=<id>' in: %s\n", + RTE_LOG(DEBUG, EAL, "No PCI address specified using '%s=<id>' in: %s\n", pci_params_keys[RTE_PCI_PARAM_ADDR], da->bus_str); - ret = -ENODEV; goto out; } diff --git a/lib/eal/common/eal_common_devargs.c b/lib/eal/common/eal_common_devargs.c index d673598032d..8c7650cf6c2 100644 --- a/lib/eal/common/eal_common_devargs.c +++ b/lib/eal/common/eal_common_devargs.c @@ -39,7 +39,7 @@ devargs_bus_parse_default(struct rte_devargs *devargs, /* Parse devargs name from bus key-value list. */ name = rte_kvargs_get(bus_args, "name"); if (name == NULL) { - RTE_LOG(INFO, EAL, "devargs name not found: %s\n", + RTE_LOG(DEBUG, EAL, "devargs name not found: %s\n", devargs->data); return 0; } -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 2/3] devargs: make bus key parsing optional Xueming Li @ 2021-10-20 15:47 ` Xueming Li 2021-10-21 9:03 ` Gaëtan Rivet 2021-10-23 6:17 ` David Marchand 2021-10-21 9:22 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax David Marchand 3 siblings, 2 replies; 43+ messages in thread From: Xueming Li @ 2021-10-20 15:47 UTC (permalink / raw) To: dev, Gaetan Rivet, David Marchand Cc: xuemingl, Thomas Monjalon, Lior Margalit Initial version to test global devargs syntax. Signed-off-by: Xueming Li <xuemingl@nvidia.com> --- app/test/meson.build | 2 + app/test/test_devargs.c | 214 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+) create mode 100644 app/test/test_devargs.c diff --git a/app/test/meson.build b/app/test/meson.build index a16374b7a10..16f4ff14ee8 100644 --- a/app/test/meson.build +++ b/app/test/meson.build @@ -42,6 +42,7 @@ test_sources = files( 'test_cryptodev_security_pdcp.c', 'test_cycles.c', 'test_debug.c', + 'test_devargs.c', 'test_distributor.c', 'test_distributor_perf.c', 'test_dmadev.c', @@ -204,6 +205,7 @@ fast_tests = [ ['common_autotest', true], ['cpuflags_autotest', true], ['debug_autotest', true], + ['devargs_autotest', true], ['eal_flags_c_opt_autotest', false], ['eal_flags_main_opt_autotest', false], ['eal_flags_n_opt_autotest', false], diff --git a/app/test/test_devargs.c b/app/test/test_devargs.c new file mode 100644 index 00000000000..19036716bf7 --- /dev/null +++ b/app/test/test_devargs.c @@ -0,0 +1,214 @@ +/* SPDX-License-Identifier: BSD-3-Clause + * Copyright (c) 2021 NVIDIA Corporation & Affiliates + */ + +#include <stdlib.h> +#include <stdio.h> +#include <string.h> + +#include <rte_common.h> +#include <rte_devargs.h> +#include <rte_kvargs.h> +#include <rte_bus.h> +#include <rte_class.h> + +#include "test.h" + +/* Check layer arguments. */ +static int +test_args(const char *devargs, const char *layer, const char *args, const int n) +{ + struct rte_kvargs *kvlist; + + if (n == 0) { + if (args != NULL && strlen(args) > 0) { + printf("rte_devargs_parse(%s) %s args parsed (not expected)\n", + devargs, layer); + return -1; + } else { + return 0; + } + } + if (args == NULL) { + printf("rte_devargs_parse(%s) %s args not parsed\n", + devargs, layer); + return -1; + } + kvlist = rte_kvargs_parse(args, NULL); + if (kvlist == NULL) { + printf("rte_devargs_parse(%s) %s_str: %s not parsed\n", + devargs, layer, args); + return -1; + } + if ((int)kvlist->count != n) { + printf("rte_devargs_parse(%s) %s_str: %s kv number %u, not %d\n", + devargs, layer, args, kvlist->count, n); + return -1; + } + return 0; +} + +struct devargs_case { + const char *devargs; + int bus_kv; + int class_kv; + int driver_kv; + const char *bus; + const char *name; + const char *class; +}; + +static int +test_valid_devargs_cases(const struct devargs_case *list, size_t n) +{ + struct rte_devargs da; + uint32_t i; + int ret; + int fail = TEST_SUCCESS; + struct rte_bus *pci_bus = rte_bus_find_by_name("pci"); + struct rte_bus *vdev_bus = rte_bus_find_by_name("vdev"); + struct rte_class *eth_class = rte_class_find_by_name("eth"); + + for (i = 0; i < n; i++) { + if (pci_bus == NULL && list[i].bus != NULL && + strcmp(list[i].bus, "pci") == 0) + continue; + if (vdev_bus == NULL && list[i].bus != NULL && + strcmp(list[i].bus, "vdev") == 0) + continue; + if (eth_class == NULL && list[i].class != NULL && + strcmp(list[i].class, "eth") == 0) + continue; + memset(&da, 0, sizeof(da)); + ret = rte_devargs_parse(&da, list[i].devargs); + if (ret < 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i].devargs, ret); + goto fail; + } + if ((list[i].bus_kv > 0 || list[i].bus != NULL) && + da.bus == NULL) { + printf("rte_devargs_parse(%s) bus not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "bus", da.bus_str, + list[i].bus_kv) != 0) + goto fail; + if (list[i].bus != NULL && + strcmp(da.bus->name, list[i].bus) != 0) { + printf("rte_devargs_parse(%s) bus name (%s) not expected (%s)\n", + list[i].devargs, da.bus->name, list[i].bus); + goto fail; + } + if ((list[i].class_kv > 0 || list[i].class != NULL) && + da.cls == NULL) { + printf("rte_devargs_parse(%s) class not parsed\n", + list[i].devargs); + goto fail; + } + if (test_args(list[i].devargs, "class", da.cls_str, + list[i].class_kv) != 0) + goto fail; + if (list[i].class != NULL && + strcmp(da.cls->name, list[i].class) != 0) { + printf("rte_devargs_parse(%s) class name (%s) not expected (%s)\n", + list[i].devargs, da.cls->name, list[i].class); + goto fail; + } + if (test_args(list[i].devargs, "driver", da.drv_str, + list[i].driver_kv) != 0) + goto fail; + if (list[i].name != NULL && + strcmp(da.name, list[i].name) != 0) { + printf("rte_devargs_parse(%s) device name (%s) not expected (%s)\n", + list[i].devargs, da.name, list[i].name); + goto fail; + } + goto cleanup; +fail: + fail = TEST_FAILED; +cleanup: + rte_devargs_reset(&da); + } + return fail; +} + +/* Test several valid cases */ +static int +test_valid_devargs(void) +{ + static const struct devargs_case list[] = { + /* Global devargs syntax: */ + { "bus=pci", + 1, 0, 0, "pci", NULL, NULL}, + { "class=eth", + 0, 1, 0, NULL, NULL, "eth" }, + { "bus=pci,addr=1:2.3/class=eth/driver=abc,k0=v0", + 2, 1, 2, "pci", "0000:01:02.3", "eth" }, + { "bus=vdev,name=/dev/file/name/class=eth", + 2, 1, 0, "vdev", "/dev/file/name", "eth" }, + { "bus=vdev,name=/class/bus/path/class=eth", + 2, 1, 0, "vdev", "/class/bus/path", "eth" }, + { "bus=vdev,name=///dblslsh/class=eth", + 2, 1, 0, "vdev", "///dblslsh", "eth" }, + /* Legacy devargs syntax: */ + { "1:2.3", 0, 0, 0, + "pci", "1:2.3", NULL }, + { "pci:1:2.3,k0=v0", + 0, 0, 1, "pci", "1:2.3", NULL }, + }; + static const struct devargs_case legacy_ring_list[] = { + { "net_ring0", + 0, 0, 0, "vdev", "net_ring0", NULL }, + { "net_ring0,iface=test,path=/class/bus/,queues=1", + 0, 0, 3, "vdev", "net_ring0", NULL }, + }; + struct rte_bus *vdev_bus = rte_bus_find_by_name("vdev"); + int ret; + + ret = test_valid_devargs_cases(list, RTE_DIM(list)); + if (vdev_bus != NULL && vdev_bus->parse("net_ring0", NULL) == 0) + /* Ring vdev driver enabled. */ + ret |= test_valid_devargs_cases(legacy_ring_list, + RTE_DIM(legacy_ring_list)); + return ret; +} + +/* Test several invalid cases */ +static int +test_invalid_devargs(void) +{ + static const char * const list[] = { + "bus=wrong-bus", + "class=wrong-class"}; + struct rte_devargs da; + uint32_t i; + int ret; + int fail = 0; + + for (i = 0; i < RTE_DIM(list); i++) { + ret = rte_devargs_parse(&da, list[i]); + if (ret >= 0) { + printf("rte_devargs_parse(%s) returned %d (but should not)\n", + list[i], ret); + fail = ret; + } + rte_devargs_reset(&da); + } + return fail; +} + +static int +test_devargs(void) +{ + printf("== test valid case ==\n"); + if (test_valid_devargs() < 0) + return -1; + printf("== test invalid case ==\n"); + if (test_invalid_devargs() < 0) + return -1; + return 0; +} + +REGISTER_TEST_COMMAND(devargs_autotest, test_devargs); -- 2.33.0 ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases Xueming Li @ 2021-10-21 9:03 ` Gaëtan Rivet 2021-10-23 6:17 ` David Marchand 1 sibling, 0 replies; 43+ messages in thread From: Gaëtan Rivet @ 2021-10-21 9:03 UTC (permalink / raw) To: Xueming(Steven) Li, dev, David Marchand; +Cc: Thomas Monjalon, Lior Margalit On Wed, Oct 20, 2021, at 17:47, Xueming Li wrote: > Initial version to test global devargs syntax. > > Signed-off-by: Xueming Li <xuemingl@nvidia.com> Hi, Thanks for making the changes. I have the nagging impression that the runtime checks could have been written in a lighter way, but maybe that's just it. It seems to fit the function, so it will do. Thanks again for dealing with this, and it's very nice to have those unit tests. Reviewed-by: Gaetan Rivet <grive@u256.net> -- Gaetan Rivet ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases Xueming Li 2021-10-21 9:03 ` Gaëtan Rivet @ 2021-10-23 6:17 ` David Marchand 2021-10-23 12:20 ` Xueming(Steven) Li 1 sibling, 1 reply; 43+ messages in thread From: David Marchand @ 2021-10-23 6:17 UTC (permalink / raw) To: Xueming Li; +Cc: dev, Gaetan Rivet, Thomas Monjalon, Lior Margalit On Wed, Oct 20, 2021 at 5:48 PM Xueming Li <xuemingl@nvidia.com> wrote: > + kvlist = rte_kvargs_parse(args, NULL); > + if (kvlist == NULL) { > + printf("rte_devargs_parse(%s) %s_str: %s not parsed\n", > + devargs, layer, args); > + return -1; > + } > + if ((int)kvlist->count != n) { > + printf("rte_devargs_parse(%s) %s_str: %s kv number %u, not %d\n", > + devargs, layer, args, kvlist->count, n); > + return -1; > + } > + return 0; > +} Missing some rte_kvargs_free(). Can you send a fix? Thanks. -- David Marchand ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases 2021-10-23 6:17 ` David Marchand @ 2021-10-23 12:20 ` Xueming(Steven) Li 0 siblings, 0 replies; 43+ messages in thread From: Xueming(Steven) Li @ 2021-10-23 12:20 UTC (permalink / raw) To: david.marchand; +Cc: Lior Margalit, NBU-Contact-Thomas Monjalon, dev, grive On Sat, 2021-10-23 at 08:17 +0200, David Marchand wrote: > On Wed, Oct 20, 2021 at 5:48 PM Xueming Li <xuemingl@nvidia.com> wrote: > > + kvlist = rte_kvargs_parse(args, NULL); > > + if (kvlist == NULL) { > > + printf("rte_devargs_parse(%s) %s_str: %s not parsed\n", > > + devargs, layer, args); > > + return -1; > > + } > > + if ((int)kvlist->count != n) { > > + printf("rte_devargs_parse(%s) %s_str: %s kv number %u, not %d\n", > > + devargs, layer, args, kvlist->count, n); > > + return -1; > > + } > > + return 0; > > +} > > Missing some rte_kvargs_free(). > Can you send a fix? Thanks for checking this out, fix posted: https://patches.dpdk.org/project/dpdk/patch/20211023121755.169290-1-xuemingl@nvidia.com/ > > Thanks. > > ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax Xueming Li ` (2 preceding siblings ...) 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases Xueming Li @ 2021-10-21 9:22 ` David Marchand 2021-10-21 10:43 ` Xueming(Steven) Li 3 siblings, 1 reply; 43+ messages in thread From: David Marchand @ 2021-10-21 9:22 UTC (permalink / raw) To: Xueming Li; +Cc: dev, Gaetan Rivet, Thomas Monjalon, Lior Margalit On Wed, Oct 20, 2021 at 5:47 PM Xueming Li <xuemingl@nvidia.com> wrote: > > - Support path in global syntax. > - Fix bus name resolving > - Add devargs test cases > > v1: initial version > v2: > - add test cases to test suite > - add more test cases, verify device name, bus name and class name > v3: > - remove autotest_data.py > v4: > - make devargs test depends on virtio driver > v5: > - use api to filter out unsupported devarg test cases Fixed style issues in meson (this is new in v5, please pay attention to such details) and applied. Thanks. -- David Marchand ^ permalink raw reply [flat|nested] 43+ messages in thread
* Re: [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax 2021-10-21 9:22 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax David Marchand @ 2021-10-21 10:43 ` Xueming(Steven) Li 0 siblings, 0 replies; 43+ messages in thread From: Xueming(Steven) Li @ 2021-10-21 10:43 UTC (permalink / raw) To: david.marchand; +Cc: Lior Margalit, NBU-Contact-Thomas Monjalon, dev, grive On Thu, 2021-10-21 at 11:22 +0200, David Marchand wrote: > On Wed, Oct 20, 2021 at 5:47 PM Xueming Li <xuemingl@nvidia.com> wrote: > > > > - Support path in global syntax. > > - Fix bus name resolving > > - Add devargs test cases > > > > v1: initial version > > v2: > > - add test cases to test suite > > - add more test cases, verify device name, bus name and class name > > v3: > > - remove autotest_data.py > > v4: > > - make devargs test depends on virtio driver > > v5: > > - use api to filter out unsupported devarg test cases > > Fixed style issues in meson (this is new in v5, please pay attention > to such details) and applied. > Thanks. > Thanks David! > ^ permalink raw reply [flat|nested] 43+ messages in thread
end of thread, other threads:[~2021-10-23 12:20 UTC | newest] Thread overview: 43+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-10-05 12:30 [dpdk-dev] [PATCH 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-05 12:30 ` [dpdk-dev] [PATCH 2/3] devargs: make bus key parsing optional Xueming Li 2021-10-05 12:30 ` [dpdk-dev] [PATCH 3/3] test/devargs: add devargs test cases Xueming Li 2021-10-05 14:01 ` David Marchand 2021-10-05 15:56 ` Xueming(Steven) Li 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 0/3] devargs: support path in global syntax Xueming Li 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-19 14:57 ` Gaëtan Rivet 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 2/3] devargs: make bus key parsing optional Xueming Li 2021-10-19 15:19 ` Gaëtan Rivet 2021-10-05 15:54 ` [dpdk-dev] [PATCH v1 3/3] test/devargs: add devargs test cases Xueming Li 2021-10-19 15:07 ` Gaëtan Rivet 2021-10-20 7:20 ` Xueming(Steven) Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 2/3] devargs: make bus key parsing optional Xueming Li 2021-10-20 7:31 ` [dpdk-dev] [PATCH v2 3/3] test/devargs: add devargs test cases Xueming Li 2021-10-20 7:38 ` David Marchand 2021-10-20 8:23 ` Xueming(Steven) Li 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-20 8:21 ` [dpdk-dev] [PATCH v3 2/3] devargs: make bus key parsing optional Xueming Li 2021-10-20 8:22 ` [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases Xueming Li 2021-10-20 9:08 ` David Marchand 2021-10-20 9:40 ` Xueming(Steven) Li 2021-10-20 10:41 ` Bruce Richardson 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 2/3] devargs: make bus key parsing optional Xueming Li 2021-10-20 11:12 ` [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases Xueming Li 2021-10-20 11:55 ` Gaëtan Rivet 2021-10-20 13:53 ` Xueming(Steven) Li 2021-10-20 14:22 ` Gaëtan Rivet 2021-10-20 14:34 ` Bruce Richardson 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax Xueming Li 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 1/3] devargs: support path value for global device arguments Xueming Li 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 2/3] devargs: make bus key parsing optional Xueming Li 2021-10-20 15:47 ` [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases Xueming Li 2021-10-21 9:03 ` Gaëtan Rivet 2021-10-23 6:17 ` David Marchand 2021-10-23 12:20 ` Xueming(Steven) Li 2021-10-21 9:22 ` [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax David Marchand 2021-10-21 10:43 ` Xueming(Steven) Li
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).