devargs: support path value for global device arguments
PATCH 1/3 devargs: support path value for global device arguments
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.
--- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-)
lib/eal/common/eal_common_devargs.c +43 -74
@@ -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++) {
Signature · 1 line
2.33.0
PATCH 2/3 devargs: make bus key parsing optional
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.
--- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-)
drivers/bus/pci/pci_params.c +3 -5
@@ -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; }
lib/eal/common/eal_common_devargs.c +1 -1
@@ -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 ; }
Signature · 1 line
2.33.0
PATCH 3/3 test/devargs: add devargs test cases
In reply to Xueming LiInitial version to test Global devargs syntax.
--- app/test/meson.build | 1 + app/test/test_devargs.c | 147 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 148 insertions(+) create mode 100644 app/test/test_devargs.c
app/test/meson.build +1 -0
@@ -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',
app/test/test_devargs.c new file +147 -0
147 lines not on this page · raw message
Signature · 1 line
2.33.0
Re: [dpdk-dev] [PATCH 3/3] test/devargs: add devargs test cases
app/test/meson.build test_sources = files( · line 48
Quoted 12 lines · Xueming Li
app/test/meson.build +1
-0@@ -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.
Signature · 1 line
David Marchand
Re: [dpdk-dev] [PATCH 3/3] test/devargs: add devargs test cases
Quoted 10 lines · David Marchand
Quoted 9 lines · Xueming Li
app/test/meson.build +1
-0@@ -42,6 +42,7 @@ test_sources = files( 'test_cryptodev_security_pdcp.c', 'test_cycles.c', 'test_debug.c', + 'test_devargs.c',
Quoted 8 lines
app/test/meson.build test_sources = files( · line 48
Quoted 3 lines
app/test/meson.build +0
-0'test_distributor.c', 'test_distributor_perf.c', 'test_eal_flags.c',You likely want to add
devargs_autotesttofast_tests, else "meson test" (and the CI) won't execute this new test.
As for tests referenced inautotest_data.py, I am not sure we really need this, but please update too for consistency.
Good suggestion! v1 posted.
Quoted 2 lines
cover devargs: support path in global syntax
In reply to Xueming Li- 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
Signature · 1 line
2.33.0
PATCH 1/3 devargs: support path value for global device arguments
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.
--- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-)
lib/eal/common/eal_common_devargs.c +43 -74
@@ -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++) {
Signature · 1 line
2.33.0
Hi Steven,
Quoted 8 lines · Xueming Li
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 <[email protected]>
The code looks good to me, and after executing with the test suite it seems to behave properly.
Signature · 1 line
Gaetan Rivet
PATCH 2/3 devargs: make bus key parsing optional
In reply to Xueming LiGlobal 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.
--- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-)
drivers/bus/pci/pci_params.c +3 -5
@@ -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; }
lib/eal/common/eal_common_devargs.c +1 -1
@@ -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 ; }
Signature · 1 line
2.33.0
Re: [dpdk-dev] [PATCH v1 2/3] devargs: make bus key parsing optional
Quoted 8 lines · Xueming Li
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")
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.
Quoted 2 lines
Signed-off-by Xueming Li <[email protected]>
For this patch and beside the stable considerations,
Signature · 1 line
Gaetan Rivet
PATCH 3/3 test/devargs: add devargs test cases
In reply to Xueming LiInitial version to test Global devargs syntax.
--- 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
app/test/autotest_data.py +6 -0
@@ -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! #
app/test/meson.build +2 -0
@@ -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],
app/test/test_devargs.c new file +147 -0
147 lines not on this page · raw message
Signature · 1 line
2.33.0
Re: [dpdk-dev] [PATCH v1 3/3] test/devargs: add devargs test cases
Quoted 3 lines · Xueming Li
Initial version to test Global devargs syntax.
Signed-off-by Xueming Li <[email protected]>
Hi,
The test is a very nice addition, absolutely required.
I have however two remarks on the coverage and the implementation, below.
Quoted 109 lines
--- 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.capp/test/autotest_data.py +6
-0@@ -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! #app/test/meson.build +2
-0@@ -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],app/test/test_devargs.c new file +17
-0@@ -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)
app/test/test_devargs.c new file +41
-0+{ +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[] = {
app/test/test_devargs.c line 68
Quoted 10 lines
app/test/test_devargs.c new file +10
-0+/* 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.
Signature · 1 line
Gaetan Rivet
Re: [dpdk-dev] [PATCH v1 3/3] test/devargs: add devargs test cases
Quoted 153 lines · Gaëtan Rivet
Quoted 3 lines · Xueming Li
Initial version to test Global devargs syntax.
Signed-off-by Xueming Li <[email protected]>Hi,
The test is a very nice addition, absolutely required.
I have however two remarks on the coverage and the implementation, below.Quoted 109 lines
--- 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.capp/test/autotest_data.py +6
-0@@ -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! #app/test/meson.build +2
-0@@ -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],app/test/test_devargs.c new file +17
-0@@ -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)
app/test/test_devargs.c new file +41
-0+{ +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[] = {app/test/test_devargs.c line 68
Quoted 10 lines
app/test/test_devargs.c new file +10
-0+/* 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:
(quoted diff) +3
-0+ { "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).
Quoted 6 lines
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!
v2 cover devargs: support path in global syntax
In reply to Xueming Li- 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
Signature · 1 line
2.33.0
PATCH v2 1/3 devargs: support path value for global device arguments
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.
--- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-)
lib/eal/common/eal_common_devargs.c +43 -74
@@ -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++) {
Signature · 1 line
2.33.0
PATCH v2 2/3 devargs: make bus key parsing optional
In reply to Xueming LiGlobal 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.
--- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-)
drivers/bus/pci/pci_params.c +3 -5
@@ -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; }
lib/eal/common/eal_common_devargs.c +1 -1
@@ -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 ; }
Signature · 1 line
2.33.0
PATCH v2 3/3 test/devargs: add devargs test cases
In reply to Xueming LiInitial version to test Global devargs syntax.
--- 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
app/test/autotest_data.py new file +803 -0
@@ -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, + }, +]
app/test/meson.build +2 -0
@@ -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],
app/test/test_devargs.c new file +184 -0
184 lines not on this page · raw message
Signature · 1 line
2.33.0
Re: [dpdk-dev] [PATCH v2 3/3] test/devargs: add devargs test cases
Quoted 6 lines · Xueming Li
Initial version to test Global devargs syntax.
Signed-off-by Xueming Li <[email protected]>--- 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
Quoted 6 lines
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
Signature · 1 line
David Marchand
Re: [dpdk-dev] [PATCH v2 3/3] test/devargs: add devargs test cases
Quoted 11 lines · David Marchand
Quoted 6 lines · Xueming Li
Initial version to test Global devargs syntax.
Signed-off-by Xueming Li <[email protected]>--- 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.
Quoted 10 lines
Quoted 8 lines
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
v3 cover devargs: support path in global syntax
In reply to Xueming Li- 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
Signature · 1 line
2.33.0
PATCH v3 1/3 devargs: support path value for global device arguments
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.
--- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-)
lib/eal/common/eal_common_devargs.c +43 -74
@@ -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++) {
Signature · 1 line
2.33.0
PATCH v3 2/3 devargs: make bus key parsing optional
In reply to Xueming LiGlobal 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.
--- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-)
drivers/bus/pci/pci_params.c +3 -5
@@ -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; }
lib/eal/common/eal_common_devargs.c +1 -1
@@ -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 ; }
Signature · 1 line
2.33.0
PATCH v3 3/3 test/devargs: add devargs test cases
In reply to Xueming LiInitial version to test Global devargs syntax.
--- app/test/meson.build | 2 + app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 186 insertions(+) create mode 100644 app/test/test_devargs.c
app/test/meson.build +2 -0
@@ -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],
app/test/test_devargs.c new file +184 -0
184 lines not on this page · raw message
Signature · 1 line
2.33.0
Re: [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases
Quoted 4 lines · Xueming Li
Initial version to test Global devargs syntax.
Signed-off-by Xueming Li <[email protected]>
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"
-------
Signature · 1 line
David Marchand
Re: [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases
Quoted 25 lines · David Marchand
Quoted 4 lines · Xueming Li
Initial version to test Global devargs syntax.
Signed-off-by Xueming Li <[email protected]>10/95 DPDK:fast-tests /
devargs_autotestFAIL 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
Quoted 8 lines
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?
Quoted 3 lines
-------
Re: [dpdk-dev] [PATCH v3 3/3] test/devargs: add devargs test cases
Quoted 35 lines · Xueming(Steven) Li
Quoted 25 lines · David Marchand
Quoted 4 lines · Xueming Li
Initial version to test Global devargs syntax.
Signed-off-by Xueming Li <[email protected]>10/95 DPDK:fast-tests /
devargs_autotestFAIL 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: 1Quoted 8 lines
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"
Quoted 8 lines
Yes, noticed that, seems virtio driver not enabled. Tried to add "
net_virtio" totest_depsin 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
v4 cover devargs: support path in global syntax
In reply to Xueming Li- 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: [email protected]
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
Signature · 1 line
2.33.0
PATCH v4 1/3 devargs: support path value for global device arguments
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.
--- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-)
lib/eal/common/eal_common_devargs.c +43 -74
@@ -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++) {
Signature · 1 line
2.33.0
PATCH v4 2/3 devargs: make bus key parsing optional
In reply to Xueming LiGlobal 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.
--- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-)
drivers/bus/pci/pci_params.c +3 -5
@@ -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; }
lib/eal/common/eal_common_devargs.c +1 -1
@@ -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 ; }
Signature · 1 line
2.33.0
PATCH v4 3/3 test/devargs: add devargs test cases
In reply to Xueming LiInitial version to test global devargs syntax.
--- app/test/meson.build | 5 ++ app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100644 app/test/test_devargs.c
app/test/meson.build +5 -0
@@ -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'
app/test/test_devargs.c new file +184 -0
184 lines not on this page · raw message
Signature · 1 line
2.33.0
Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases
Quoted 14 lines · Xueming Li
Initial version to test global devargs syntax.
Signed-off-by Xueming Li <[email protected]>--- app/test/meson.build | 5 ++ app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100644 app/test/test_devargs.capp/test/meson.build +0
-0
app/test/meson.build if dpdk_conf.has('RTE_NET_RING') · line 406
Quoted 8 lines
app/test/meson.build +5
-0@@ -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.
Signature · 1 line
Gaetan Rivet
Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases
Quoted 38 lines · Gaëtan Rivet
Quoted 14 lines · Xueming Li
Initial version to test global devargs syntax.
Signed-off-by Xueming Li <[email protected]>--- app/test/meson.build | 5 ++ app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100644 app/test/test_devargs.capp/test/meson.build +0
-0app/test/meson.build if dpdk_conf.has('RTE_NET_RING') · line 406
Quoted 8 lines
app/test/meson.build +5
-0@@ -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]] +endifHi 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_virtiois not built, a subset of your test remains valid.Finally,
net_virtiomight not be generic enough as a driver. A simpler one could be used, such asnet_ringmaybe.Instead of checking the dependencies in the meson file, you could detect support in preamble of the test:
Quoted 8 lines
bool
pci_bus_avail= (rte_bus_find_by_name("pci")!= NULL); boolvdev_bus_avail= (rte_bus_find_by_name("vdev")!= NULL); booleth_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?
Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases
Quoted 58 lines · Xueming(Steven) Li
Quoted 38 lines · Gaëtan Rivet
Quoted 14 lines · Xueming Li
Initial version to test global devargs syntax.
Signed-off-by Xueming Li <[email protected]>--- app/test/meson.build | 5 ++ app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100644 app/test/test_devargs.capp/test/meson.build +0
-0app/test/meson.build if dpdk_conf.has('RTE_NET_RING') · line 406
Quoted 8 lines
app/test/meson.build +5
-0@@ -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]] +endifHi 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_virtiois not built, a subset of your test remains valid.Finally,
net_virtiomight not be generic enough as a driver. A simpler one could be used, such asnet_ringmaybe.Instead of checking the dependencies in the meson file, you could detect support in preamble of the test:
Quoted 8 lines
bool
pci_bus_avail= (rte_bus_find_by_name("pci")!= NULL); boolvdev_bus_avail= (rte_bus_find_by_name("vdev")!= NULL); booleth_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 },
Quoted 4 lines
#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.
Signature · 1 line
Gaetan Rivet
Re: [dpdk-dev] [PATCH v4 3/3] test/devargs: add devargs test cases
Quoted 77 lines · Gaëtan Rivet
Quoted 58 lines · Xueming(Steven) Li
Quoted 38 lines · Gaëtan Rivet
Quoted 14 lines · Xueming Li
Initial version to test global devargs syntax.
Signed-off-by Xueming Li <[email protected]>--- app/test/meson.build | 5 ++ app/test/test_devargs.c | 184 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 189 insertions(+) create mode 100644 app/test/test_devargs.capp/test/meson.build +0
-0app/test/meson.build if dpdk_conf.has('RTE_NET_RING') · line 406
Quoted 8 lines
app/test/meson.build +5
-0@@ -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]] +endifHi 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_virtiois not built, a subset of your test remains valid.Finally,
net_virtiomight not be generic enough as a driver. A simpler one could be used, such asnet_ringmaybe.Instead of checking the dependencies in the meson file, you could detect support in preamble of the test:
Quoted 8 lines
bool
pci_bus_avail= (rte_bus_find_by_name("pci")!= NULL); boolvdev_bus_avail= (rte_bus_find_by_name("vdev")!= NULL); booleth_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 },Quoted 4 lines
#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_virtiotonet_ringinstead 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 thannet_virtiofor unit tests.
Quoted 6 lines
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.cfor examples ofnet_ringdummy 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.
v5 cover devargs: support path in global syntax
In reply to Xueming Li- 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: [email protected]
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
Signature · 1 line
2.33.0
PATCH v5 1/3 devargs: support path value for global device arguments
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.
--- lib/eal/common/eal_common_devargs.c | 117 ++++++++++------------------ 1 file changed, 43 insertions(+), 74 deletions(-)
lib/eal/common/eal_common_devargs.c +43 -74
@@ -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++) {
Signature · 1 line
2.33.0
PATCH v5 2/3 devargs: make bus key parsing optional
In reply to Xueming LiGlobal 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.
--- drivers/bus/pci/pci_params.c | 8 +++----- lib/eal/common/eal_common_devargs.c | 2 +- 2 files changed, 4 insertions(+), 6 deletions(-)
drivers/bus/pci/pci_params.c +3 -5
@@ -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; }
lib/eal/common/eal_common_devargs.c +1 -1
@@ -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 ; }
Signature · 1 line
2.33.0
PATCH v5 3/3 test/devargs: add devargs test cases
In reply to Xueming LiInitial version to test global devargs syntax.
--- app/test/meson.build | 2 + app/test/test_devargs.c | 214 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 216 insertions(+) create mode 100644 app/test/test_devargs.c
app/test/meson.build +2 -0
@@ -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],
app/test/test_devargs.c new file +214 -0
214 lines not on this page · raw message
Signature · 1 line
2.33.0
Re: [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases
Quoted 3 lines · Xueming Li
Initial version to test global devargs syntax.
Signed-off-by Xueming Li <[email protected]>
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.
Signature · 1 line
Gaetan Rivet
Re: [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases
In reply to Xueming Liapp/test/test_devargs.c line 49
Quoted 13 lines · Xueming Li
app/test/test_devargs.c new file +13
-0+ 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.
Signature · 1 line
David Marchand
Re: [dpdk-dev] [PATCH v5 3/3] test/devargs: add devargs test cases
Quoted 15 lines · David Marchand
app/test/test_devargs.c line 49
Quoted 13 lines · Xueming Li
app/test/test_devargs.c new file +13
-0+ 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 ; +}
Quoted 2 lines
Missing some
rte_kvargs_free().
Can you send a fix?
Thanks for checking this out, fix posted:
https://patches.dpdk.org/project/dpdk/patch/[email protected]/
Quoted 4 lines
Thanks.
Re: [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax
In reply to Xueming LiQuoted 5 lines · Xueming Li
- Support path in global syntax.
- Fix bus name resolving
- Add devargs test cases
Quoted 10 lines
v1: initial version
v2:
- add test cases to test suite
- add more test cases, verify device name, bus name and class name
v3:
- removeautotest_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.
Signature · 1 line
David Marchand
Re: [dpdk-dev] [PATCH v5 0/3] devargs: support path in global syntax
Quoted 17 lines · David Marchand
Quoted 5 lines · Xueming Li
- Support path in global syntax.
- Fix bus name resolving
- Add devargs test casesQuoted 10 lines
v1: initial version
v2:
- add test cases to test suite
- add more test cases, verify device name, bus name and class name
v3:
- removeautotest_data.py
v4:
- make devargs test depends on virtio driver
v5:
- use api to filter out unsupported devarg test cases
Quoted 4 lines
Fixed style issues in meson (this is new in v5, please pay attention to such details) and applied.
Thanks.
Thanks David!