Device querying
Subsystems (3)
Re: [PATCH v10 02/27] kvargs: remove error logs
In reply to Gaetan Rivet05/07/2018 13:48, Gaetan Rivet:
Quoted 6 lines
Error logs in kvargs parsing should be better handled in components calling the library.
This library must be as lean as possible.
Signed-off-by Gaetan Rivet <[email protected]>
PATCH v10 3/27 kvargs: build before EAL
In reply to Gaetan Rivet--- lib/Makefile | 3 +-- lib/librte_eal/meson.build | 1 + lib/librte_kvargs/Makefile | 2 +- lib/librte_kvargs/meson.build | 3 +++ lib/meson.build | 8 ++++++-- 5 files changed, 12 insertions(+), 5 deletions(-)
lib/Makefile +1 -2
@@ -4,6 +4,7 @@ include $(RTE_SDK)/mk/rte.vars.mk DIRS-y += librte_compat +DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci DEPDIRS-librte_pci := librte_eal
@@ -76,8 +77,6 @@ DEPDIRS-librte_flow_classify := librte_net librte_table librte_acl DIRS-$(CONFIG_RTE_LIBRTE_SCHED) += librte_sched DEPDIRS-librte_sched := librte_eal librte_mempool librte_mbuf librte_net DEPDIRS-librte_sched += librte_timer -DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs -DEPDIRS-librte_kvargs := librte_eal DIRS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += librte_distributor DEPDIRS-librte_distributor := librte_eal librte_mbuf librte_ethdev DIRS-$(CONFIG_RTE_LIBRTE_PORT) += librte_port
lib/librte_eal/meson.build +1 -0
@@ -24,6 +24,7 @@ endif version = 7 # the version of the EAL API allow_experimental_apis = true deps += 'compat' +deps += 'kvargs' cflags += '-D_GNU_SOURCE' sources = common_sources + env_sources objs = common_objs + env_objs
lib/librte_kvargs/Makefile +1 -1
@@ -7,7 +7,7 @@ include $(RTE_SDK)/mk/rte.vars.mk LIB = librte_kvargs.a CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -LDLIBS += -lrte_eal +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include EXPORT_MAP := rte_kvargs_version.map
lib/librte_kvargs/meson.build +3 -0
@@ -1,6 +1,9 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2017 Intel Corporation +includes = [global_inc] +includes += include_directories('../../../lib/librte_eal/common/include') + version = 1 sources = files('rte_kvargs.c') headers = files('rte_kvargs.h')
lib/meson.build +6 -2
@@ -9,7 +9,8 @@ # given as a dep, no need to mention ring. This is especially true for the # core libs which are widely reused, so their deps are kept to a minimum. libraries = [ 'compat', # just a header, used for versioning - 'eal', 'ring', 'mempool', 'mbuf', 'net', 'kvargs', 'ethdev', 'pci', # core + 'kvargs', + 'eal', 'ring', 'mempool', 'mbuf', 'net', 'ethdev', 'pci', # core 'metrics', # bitrate/latency stats depends on this 'hash', # efd depends on this 'timer', # eventdev depends on this
@@ -41,9 +42,12 @@ foreach l:libraries # external package/library requirements ext_deps = [] deps = ['eal'] # eal is standard dependency except for itself - if l == 'eal' + if l == 'kvargs' deps = [] endif + if l == 'eal' + deps = ['kvargs'] + endif dir_name = 'librte_' + l subdir(dir_name)
Signature · 1 line
2.18.0
Re: [PATCH v10 03/27] kvargs: build before EAL
05/07/2018 13:48, Gaetan Rivet:
Quoted 1 line
Signed-off-by Gaetan Rivet <[email protected]>
You should add a commit log to explain the intent:
kvargs will be used in EAL for devargs parsing.
PATCH v10 4/27 kvargs: introduce a more flexible parsing function
In reply to Gaetan RivetThis function permits defining additional terminating characters, ending the parsing to arbitrary delimiters.
--- lib/Makefile | 1 + lib/librte_kvargs/meson.build | 2 ++ lib/librte_kvargs/rte_kvargs.c | 25 ++++++++++++++++++++ lib/librte_kvargs/rte_kvargs.h | 30 ++++++++++++++++++++++++ lib/librte_kvargs/rte_kvargs_version.map | 7 ++++++ 5 files changed, 65 insertions(+)
lib/Makefile +1 -0
@@ -5,6 +5,7 @@ include $(RTE_SDK)/mk/rte.vars.mk DIRS-y += librte_compat DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs +DEPDIRS-librte_kvargs := librte_compat DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci DEPDIRS-librte_pci := librte_eal
lib/librte_kvargs/meson.build +2 -0
@@ -7,3 +7,5 @@ includes += include_directories('../../../lib/librte_eal/common/include') version = 1 sources = files('rte_kvargs.c') headers = files('rte_kvargs.h') + +deps += 'compat'
lib/librte_kvargs/rte_kvargs.c +25 -0
@@ -168,3 +168,28 @@ rte_kvargs_parse(const char *args, const char * const valid_keys[])return kvlist; } + +__rte_experimental +struct rte_kvargs * +rte_kvargs_parse2 (const char *args,const char *const valid_keys[], +const char *valid_ends) +{ +struct rte_kvargs *kvlist =NULL ; +char *copy; +size_t len; + +if (valid_ends ==NULL ) +return rte_kvargs_parse (args, valid_keys); + + copy =strdup (args); +if (copy ==NULL ) +return NULL ; + + len =strcspn (copy, valid_ends); + copy[len] ='\0' ; + + kvlist =rte_kvargs_parse (copy, valid_keys); + +free (copy); +return kvlist; +}
lib/librte_kvargs/rte_kvargs.h +30 -0
@@ -71,6 +71,36 @@ struct rte_kvargs {struct rte_kvargs *rte_kvargs_parse (const char *args,const char *const valid_keys[]); +/** +* Allocate a rte_kvargs and store key/value associations from a string. +* This version will consider any byte from valid_ends as a possible +* terminating character, and will not parse beyond any of their occurrence. +* +* The function allocates and fills an rte_kvargs structure from a given +* string whose format is key1=value1,key2=value2,... +* +* The structure can be freed with rte_kvargs_free(). +* +* @param args +* The input string containing the key/value associations +* +* @param valid_keys +* A list of valid keys (table of const char *, the last must be NULL). +* This argument is ignored if NULL +* +* @param valid_ends +* Acceptable terminating characters. +* If NULL, the behavior is the same as ``rte_kvargs_parse``. +* +* @return +* - A pointer to an allocated rte_kvargs structure on success +* - NULL on error +*/ +__rte_experimental +struct rte_kvargs *rte_kvargs_parse2(const char *args, +const char *const valid_keys[], +const char *valid_ends); +/** * Free a rte_kvargs structure *
lib/librte_kvargs/rte_kvargs_version.map +7 -0
@@ -8,3 +8,10 @@ DPDK_2.0 { local: *; }; + +EXPERIMENTAL { + global: + + rte_kvargs_parse2; + +} DPDK_2.0;
Signature · 1 line
2.18.0
Re: [PATCH v10 04/27] kvargs: introduce a more flexible parsing function
05/07/2018 13:48, Gaetan Rivet:
Quoted 2 lines
This function permits defining additional terminating characters, ending the parsing to arbitrary delimiters.
[...]
lib/librte_kvargs/rte_kvargs.h struct rte_kvargs { · line 102
Quoted 4 lines
lib/librte_kvargs/rte_kvargs.h +4
-0+__rte_experimental +struct rte_kvargs *rte_kvargs_parse2(const char *args, +const char *const valid_keys[], +const char *valid_ends);
The name is not ideal but I don't have a better idea.
Shreyansh Jain
Acked-byRe: [PATCH v10 04/27] kvargs: introduce a more flexible parsing function
Quoted 11 lines · Thomas Monjalon
05/07/2018 13:48, Gaetan Rivet:
Quoted 2 lines
This function permits defining additional terminating characters, ending the parsing to arbitrary delimiters.
[...]
lib/librte_kvargs/rte_kvargs.h struct rte_kvargs { · line 102
Quoted 4 lines
lib/librte_kvargs/rte_kvargs.h +4
-0+__rte_experimental +struct rte_kvargs *rte_kvargs_parse2(const char *args, +const char *const valid_keys[], +const char *valid_ends);The name is not ideal but I don't have a better idea.
Does 'rte_kvargs_parse_delim()' seem any better? Or, a more obvious one 'rte_kvargs_parse_with_delim()'.
[...]
As I am already late in commenting on this, I am OK changing this later as well (using the 'experimental' blanket) when a better suggestion comes along. So, from me as well:
PATCH v10 5/27 eal: introduce dtor macros
In reply to Gaetan Rivet--- lib/librte_eal/common/include/rte_common.h | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+)
lib/librte_eal/common/include/rte_common.h +23 -0
@@ -111,6 +111,29 @@ static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)#define RTE_INIT (func) \ RTE_INIT_PRIO(func, LAST) +/** +* Run after main() with low priority. +* +* @param func +* Destructor function name. +* @param prio +* Priority number must be above 100. +* Lowest number is the last to run. +*/ +#define RTE_FINI_PRIO (func, prio) \ +static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void) + +/** +* Run after main() with high priority. +* +* The destructor will be run *before* prioritized destructors. +* +* @param func +* Destructor function name. +*/ +#define RTE_FINI (func) \ + RTE_FINI_PRIO(func, LAST) +/** * Force a function to be inlined */
Signature · 1 line
2.18.0
Re: [PATCH v10 05/27] eal: introduce dtor macros
lib/librte_eal/common/include/rte_common.h static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void) · line 113
Quoted 13 lines · Gaetan Rivet
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/include/rte_common.h | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+)lib/librte_eal/common/include/rte_common.h +0
-0@@ -111,6 +111,29 @@ static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)#define RTE_INIT (func) \ RTE_INIT_PRIO(func, LAST)
Re: [PATCH v10 05/27] eal: introduce dtor macros
In reply to Gaetan Rivet05/07/2018 13:48, Gaetan Rivet:
Quoted 1 line
Signed-off-by Gaetan Rivet <[email protected]>
Please justify why you need destructors, by providing a commit log.
Quoted 7 lines
lib/librte_eal/common/include/rte_common.h +1
-0@@ -111,6 +111,29 @@ static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)#define RTE_INIT (func) \ RTE_INIT_PRIO(func, LAST) +/**
lib/librte_eal/common/include/rte_common.h static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void) · line 124
Quoted 10 lines
lib/librte_eal/common/include/rte_common.h +10
-0+* Run after main() with low priority. +* +* @param func +* Destructor function name. +* @param prio +* Priority number must be above 100. +* Lowest number is the last to run. +*/ +#define RTE_FINI_PRIO (func, prio) \ +static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)
I don't like the name of this macro.
What about RTE_CLEAN_PRIO?
Re: [PATCH v10 05/27] eal: introduce dtor macros
Hi Thomas,
Quoted 11 lines · Thomas Monjalon
05/07/2018 13:48, Gaetan Rivet:
Quoted 1 line
Signed-off-by Gaetan Rivet <[email protected]>Please justify why you need destructors, by providing a commit log.
Quoted 6 lines
lib/librte_eal/common/include/rte_common.h +0
-0@@ -111,6 +111,29 @@ static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)#define RTE_INIT (func) \ RTE_INIT_PRIO(func, LAST)
Quoted 16 lines
lib/librte_eal/common/include/rte_common.h static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void) · line 124
Quoted 11 lines
lib/librte_eal/common/include/rte_common.h +11
-0+/** +* Run after main() with low priority. +* +* @param func +* Destructor function name. +* @param prio +* Priority number must be above 100. +* Lowest number is the last to run. +*/ +#define RTE_FINI_PRIO (func, prio) \ +static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)I don't like the name of this macro.
What aboutRTE_CLEAN_PRIO?
FINI is symmetrical to INIT in referencing the related ELF section.
RTE_CLEAN presumes that the intended purpose of the function will be to cleanup resources. As far as we are concerned, this code could send a signal, dump config info or format / (which would be a pretty advanced cleanup, granted).
Sometimes, it could be used to release resources, presumably.
I'm not a fan of FINI either, but I appreciate the symmetry. It's pretty neutral about what it does, as its meaning is literally "The following function will be part of the .fini section".
Alternatives:
FINALIZE
UNINIT
But they have the same issue as RTE_CLEAN, IMO.
Signature · 2 lines
Gaëtan Rivet 6WIND
Thomas Monjalon
Acked-byRe: [PATCH v10 05/27] eal: introduce dtor macros
10/07/2018 14:56, Gaëtan Rivet:
Quoted 44 lines
Hi Thomas,
Quoted 11 lines · Thomas Monjalon
05/07/2018 13:48, Gaetan Rivet:
Quoted 1 line
Signed-off-by Gaetan Rivet <[email protected]>Please justify why you need destructors, by providing a commit log.
Quoted 6 lines
lib/librte_eal/common/include/rte_common.h +0
-0@@ -111,6 +111,29 @@ static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)#define RTE_INIT (func) \ RTE_INIT_PRIO(func, LAST)Quoted 16 lines
lib/librte_eal/common/include/rte_common.h static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void) · line 124
Quoted 11 lines
lib/librte_eal/common/include/rte_common.h +11
-0+/** +* Run after main() with low priority. +* +* @param func +* Destructor function name. +* @param prio +* Priority number must be above 100. +* Lowest number is the last to run. +*/ +#define RTE_FINI_PRIO (func, prio) \ +static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void)I don't like the name of this macro.
What aboutRTE_CLEAN_PRIO?FINI is symmetrical to INIT in referencing the related ELF section.
RTE_CLEANpresumes that the intended purpose of the function will be to cleanup resources. As far as we are concerned, this code could send a signal, dump config info or format / (which would be a pretty advanced cleanup, granted).Sometimes, it could be used to release resources, presumably.
I'm not a fan of FINI either, but I appreciate the symmetry. It's pretty neutral about what it does, as its meaning is literally "The following function will be part of the .fini section".
Quoted 6 lines
Alternatives:
FINALIZE
UNINITBut they have the same issue as
RTE_CLEAN, IMO.
OK, you convinced me. And I remember now that you already convinced me earlier with the same argument :)
PATCH v10 6/27 eal: introduce device class abstraction
In reply to Gaetan Rivet--- lib/librte_eal/bsdapp/eal/Makefile | 1 + lib/librte_eal/common/Makefile | 2 +- lib/librte_eal/common/eal_common_class.c | 62 +++++++++++ lib/librte_eal/common/include/rte_class.h | 121 +++++++++++++++++++++ lib/librte_eal/common/include/rte_common.h | 1 + lib/librte_eal/common/meson.build | 2 + lib/librte_eal/linuxapp/eal/Makefile | 1 + lib/librte_eal/rte_eal_version.map | 2 + 8 files changed, 191 insertions(+), 1 deletion(-) create mode 100644 lib/librte_eal/common/eal_common_class.c create mode 100644 lib/librte_eal/common/include/rte_class.h
lib/librte_eal/bsdapp/eal/Makefile +1 -0
@@ -52,6 +52,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_hypervisor.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_string_fns.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_hexdump.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_devargs.c +SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_class.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_bus.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_dev.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_options.c
lib/librte_eal/common/Makefile +1 -1
@@ -11,7 +11,7 @@ INC += rte_per_lcore.h rte_random.h INC += rte_tailq.h rte_interrupts.h rte_alarm.h INC += rte_string_fns.h rte_version.h INC += rte_eal_memconfig.h rte_malloc_heap.h -INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h +INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h rte_class.h INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h INC += rte_malloc.h rte_keepalive.h rte_time.h INC += rte_service.h rte_service_component.h
lib/librte_eal/common/eal_common_class.c new file +62 -0
62 lines not on this page · raw message
lib/librte_eal/common/include/rte_class.h new file +121 -0
121 lines not on this page · raw message
lib/librte_eal/common/include/rte_common.h +1 -0
@@ -83,6 +83,7 @@ typedef uint16_t unaligned_uint16_t;#define RTE_PRIORITY_LOG 101#define RTE_PRIORITY_BUS 110 +#define RTE_PRIORITY_CLASS 120#define RTE_PRIORITY_LAST 65535#define RTE_PRIO (prio) \
lib/librte_eal/common/meson.build +2 -0
@@ -8,6 +8,7 @@ common_objs = [] common_sources = files( 'eal_common_bus.c', 'eal_common_cpuflags.c', + 'eal_common_class.c', 'eal_common_devargs.c', 'eal_common_dev.c', 'eal_common_errno.c',
@@ -46,6 +47,7 @@ common_headers = files( 'include/rte_branch_prediction.h', 'include/rte_bus.h', 'include/rte_bitmap.h', + 'include/rte_class.h', 'include/rte_common.h', 'include/rte_debug.h', 'include/rte_devargs.h',
lib/librte_eal/linuxapp/eal/Makefile +1 -0
@@ -60,6 +60,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_hypervisor.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_string_fns.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_hexdump.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_devargs.c +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_class.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_bus.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_dev.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_options.c
lib/librte_eal/rte_eal_version.map +2 -0
@@ -244,6 +244,8 @@ DPDK_18.05 { EXPERIMENTAL { global: + rte_class_register; + rte_class_unregister; rte_ctrl_thread_create; rte_dev_event_callback_register; rte_dev_event_callback_unregister;
Signature · 1 line
2.18.0
Re: [PATCH v10 06/27] eal: introduce device class abstraction
05/07/2018 13:48, Gaetan Rivet:
Quoted 6 lines
+/**
+ * @file
+ *
+ * DPDK device class interface.
+ *
+ * This file exposes API and interfaces of device classes.
What is the difference between "API" and "interfaces"? I think you need to explain what is a device class. You can mention examples ethdev and cryptodev.
lib/librte_eal/common/include/rte_class.h line 110
Quoted 2 lines
lib/librte_eal/common/include/rte_class.h new file +2
-0+#define RTE_REGISTER_CLASS (nm, cls) \ +RTE_INIT_PRIO(classinitfn_ ##nm, CLASS); \
If you remove the ; you can avoid next line.
See this patch which removes such useless line:
https://patches.dpdk.org/patch/41228/
lib/librte_eal/common/include/rte_class.h line 113
Quoted 3 lines
lib/librte_eal/common/include/rte_class.h new file +3
-0+static void classinitfn_ ##nm(void) \ +{\ + (cls).name =RTE_STR (nm);\
Better to insert a space before \
Quoted 2 lines
(quoted diff) +2
-0+ rte_class_register(&cls); \ +}
PATCH v10 7/27 eal/class: register destructor
In reply to Gaetan Rivet--- lib/librte_eal/common/include/rte_class.h | 5 +++++ 1 file changed, 5 insertions(+)
lib/librte_eal/common/include/rte_class.h +5 -0
@@ -112,6 +112,11 @@ static void classinitfn_ ##nm(void) \ {\ (cls).name =RTE_STR (nm);\rte_class_register (&cls); \ +} \ +RTE_FINI_PRIO(classfinifn_ ##nm, CLASS); \ +static void classfinifn_ ##nm(void) \ +{ \ + rte_class_unregister(&cls); \ }#ifdef __cplusplus
Signature · 1 line
2.18.0
Re: [PATCH v10 07/27] eal/class: register destructor
05/07/2018 13:48, Gaetan Rivet:
Quoted 1 line
Signed-off-by Gaetan Rivet <[email protected]>
If there is no specific comment, this patch can be squashed.
PATCH v10 8/27 devargs: add function to parse device layers
In reply to Gaetan RivetThis function is private to the EAL.
It is used to parse each layers in a device description string, and store the result in an rte_devargs structure.
--- lib/librte_eal/common/eal_common_devargs.c | 144 ++++++++++++++++++++ lib/librte_eal/common/eal_private.h | 27 ++++ lib/librte_eal/common/include/rte_devargs.h | 13 +- 3 files changed, 181 insertions(+), 3 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +144 -0
@@ -13,9 +13,13 @@#include <string.h> #include <stdarg.h> +#include <rte_bus.h> +#include <rte_class.h> #include <rte_compat.h> #include <rte_dev.h> #include <rte_devargs.h> +#include <rte_kvargs.h> +#include <rte_errno.h> #include <rte_tailq.h> #include "eal_private.h"
@@ -56,6 +60,146 @@ rte_eal_parse_devargs_str(const char *devargs_str,return 0 ; } +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; +} + +int +rte_devargs_layers_parse (struct rte_devargs *da, +const char *devstr) +{ +struct { +const char *key; +const char *str; +struct rte_kvargs *kvlist; + } layers[] = { + {"bus=" ,NULL ,NULL , }, + {"class=" ,NULL ,NULL , }, + {"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 ; +int ret =0 ; + +/* 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. +*/ +if (da->data != devstr) { + da->data =strdup (devstr); +if (da->data ==NULL ) { +RTE_LOG (ERR ,EAL ,"OOM\n" ); + ret = -ENOMEM ; + goto get_out; + } + s = da->data; + } + +while (s !=NULL ) { +if (strncmp (layers[i].key, s, +strlen (layers[i].key)) && +/* The last layer is free-form. +* The "driver" key is not required (but accepted). +*/ + i !=RTE_DIM (layers) -1 ) + goto next_layer; + layers[i].str = s; + layers[i].kvlist =rte_kvargs_parse2 (s,NULL ,"/" ); +if (layers[i].kvlist ==NULL ) { +RTE_LOG (ERR ,EAL ,"Could not parse %s\n" , s); + ret = -EINVAL ; + goto get_out; + } + s =strchr (s,'/' ); +if (s !=NULL ) + s++; +next_layer: +if (i >=RTE_DIM (layers)) { +RTE_LOG (ERR ,EAL ,"Unrecognized layer %s\n" , s); + ret = -EINVAL ; + goto get_out; + } + i++; + } + +/* Parse each sub-list. */ +for (i =0 ; i <RTE_DIM (layers); i++) { +if (layers[i].kvlist ==NULL ) +continue ; + kv = &layers[i].kvlist->pairs[0 ]; +if (strcmp (kv->key,"bus" ) ==0 ) { + bus =rte_bus_find_by_name (kv->value); +if (bus ==NULL ) { +RTE_LOG (ERR ,EAL ,"Could not find bus \"%s\"\n" , + kv->value); + ret = -EFAULT ; + goto get_out; + } + }else if (strcmp (kv->key,"class" ) ==0 ) { + cls =rte_class_find_by_name (kv->value); +if (cls ==NULL ) { +RTE_LOG (ERR ,EAL ,"Could not find class \"%s\"\n" , + kv->value); + ret = -EFAULT ; + goto get_out; + } + }else if (strcmp (kv->key,"driver" ) ==0 ) { +/* Ignore */ +continue ; + } + } + +/* Fill devargs fields. */ + da->busstr = layers[0 ].str; + da->clsstr = layers[1 ].str; + da->drvstr = layers[2 ].str; + da->bus = bus; + da->cls = cls; + +/* If we own the data, clean up a bit +* the several layers string, to ease +* their parsing afterward. +*/ +if (da->data != devstr) { +char *s = (void *)(intptr_t )(da->data); + +while ((s =strchr (s,'/' ))) { + *s ='\0' ; + s++; + } + } + +get_out: +for (i =0 ; i <RTE_DIM (layers); i++) { +if (layers[i].kvlist) +rte_kvargs_free (layers[i].kvlist); + } +if (ret !=0 ) + rte_errno = -ret; +return ret; +} +static int bus_name_cmp (const struct rte_bus *bus,const void *name) {
lib/librte_eal/common/eal_private.h +27 -0
@@ -258,4 +258,31 @@ int rte_mp_channel_init(void);*/ void dev_callback_process (char *device_name,enum rte_dev_event_type event); +/** +* @internal +* Parse a device string and store its information in an +* rte_devargs structure. +* +* Note: if the "data" field of da points to devstr, +* then no dynamic allocation is performed and the rte_devargs +* can be safely discarded. +* +* Otherwise ``data`` will hold a workable copy of devstr, that will be +* used by layers descriptors within rte_devargs. In this case, +* any rte_devargs should be cleaned-up before being freed. +* +* @param da +* rte_devargs structure to fill. +* +* @param devstr +* Device string. +* +* @return +* 0 on success. +* Negative errno values on error (rte_errno is set). +*/ +int +rte_devargs_layers_parse (struct rte_devargs *da, +const char *devstr); +#endif /* _EAL_PRIVATE_H_ */
lib/librte_eal/common/include/rte_devargs.h +10 -3
@@ -51,12 +51,19 @@ struct rte_devargs {enum rte_devtype type;/** Device policy. */ enum rte_dev_policy policy; -/** Bus handle for the device. */ -struct rte_bus *bus;/** Name of the device. */ char name[RTE_DEV_NAME_MAX_LEN ]; +RTE_STD_C11 + union {/** Arguments string as given by user or "" for no argument. */ -char *args; +char *args; +const char *drvstr; + }; +struct rte_bus *bus;/**< bus handle. */ +struct rte_class *cls;/**< class handle. */ +const char *busstr;/**< bus-related part of device string. */ +const char *clsstr;/**< bus-related part of device string. */ +const char *data;/**< Device string storage. */ };/**
Signature · 1 line
2.18.0
Re: [PATCH v10 08/27] devargs: add function to parse device layers
05/07/2018 13:48, Gaetan Rivet:
Quoted 4 lines
+/**
+ * @internal
+ * Parse a device string and store its information in an
+ *rte_devargsstructure.
Please, explain what is a layer.
Quoted 2 lines
+ *
+ * Note: if the "data" field of da points to devstr,
Better to use "devargs" as variable name, instead of "da".
Quoted 33 lines
+ * then no dynamic allocation is performed and the
rte_devargs
+ * can be safely discarded.
+ *
+ * Otherwise ``data`` will hold a workable copy of devstr, that will be
+ * used by layers descriptors withinrte_devargs. In this case,
+ * anyrte_devargsshould be cleaned-up before being freed.
+ *
+ * @param da
+ *rte_devargsstructure to fill.
+ *
+ * @param devstr
+ * Device string.
+ *
+ * @return
+ * 0 on success.
+ * Negative errno values on error (rte_errnois set).
+ */lib/librte_eal/common/eal_private.h +4
-0+int +rte_devargs_layers_parse (struct rte_devargs *da, +const char *devstr); +#endif /* _EAL_PRIVATE_H_ */ lib/librte_eal/common/include/rte_devargs.h +0
-2@@ -51,12 +51,19 @@ struct rte_devargs {enum rte_devtype type;/** Device policy. */ enum rte_dev_policy policy; -/** Bus handle for the device. */ -struct rte_bus *bus;/** Name of the device. */
lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 63
Quoted 10 lines
lib/librte_eal/common/include/rte_devargs.h +7
-1char name[RTE_DEV_NAME_MAX_LEN ]; +RTE_STD_C11 + union {/** Arguments string as given by user or "" for no argument. */ -char *args; +char *args; +const char *drvstr; + }; +struct rte_bus *bus;/**< bus handle. */ +struct rte_class *cls;/**< class handle. */
"class" is more readable than "cls"
lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 64
Quoted 1 line
lib/librte_eal/common/include/rte_devargs.h +1
-0+const char *busstr;/**< bus-related part of device string. */
bus_str ?
lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 65
Quoted 1 line
lib/librte_eal/common/include/rte_devargs.h +1
-0+const char *clsstr;/**< bus-related part of device string. */
class_str ?
+ there is a typo in the comment (copy/pasted "bus")
Quoted 2 lines
lib/librte_eal/common/include/rte_devargs.h +1
-0+const char *data;/**< Device string storage. */ };
Re: [PATCH v10 08/27] devargs: add function to parse device layers
Hi Thomas,
Quoted 50 lines · Thomas Monjalon
05/07/2018 13:48, Gaetan Rivet:
Quoted 4 lines
+/**
+ * @internal
+ * Parse a device string and store its information in an
+ *rte_devargsstructure.Please, explain what is a layer.
Quoted 2 lines
+ *
+ * Note: if the "data" field of da points to devstr,Better to use "devargs" as variable name, instead of "da".
Quoted 37 lines
+ * then no dynamic allocation is performed and the
rte_devargs
+ * can be safely discarded.
+ *
+ * Otherwise ``data`` will hold a workable copy of devstr, that will be
+ * used by layers descriptors withinrte_devargs. In this case,
+ * anyrte_devargsshould be cleaned-up before being freed.
+ *
+ * @param da
+ *rte_devargsstructure to fill.
+ *
+ * @param devstr
+ * Device string.
+ *
+ * @return
+ * 0 on success.
+ * Negative errno values on error (rte_errnois set).
+ */lib/librte_eal/common/eal_private.h +4
-0+int +rte_devargs_layers_parse (struct rte_devargs *da, +const char *devstr); +#endif /* _EAL_PRIVATE_H_ */ lib/librte_eal/common/include/rte_devargs.h +2
-2@@ -51,12 +51,19 @@ struct rte_devargs {enum rte_devtype type;/** Device policy. */ enum rte_dev_policy policy; -/** Bus handle for the device. */ -struct rte_bus *bus;/** Name of the device. */ char name[RTE_DEV_NAME_MAX_LEN ]; +RTE_STD_C11 + union {/** Arguments string as given by user or "" for no argument. */
Quoted 9 lines
lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 63
Quoted 6 lines
lib/librte_eal/common/include/rte_devargs.h +5
-1-char *args; +char *args; +const char *drvstr; + }; +struct rte_bus *bus;/**< bus handle. */ +struct rte_class *cls;/**< class handle. */ "class" is more readable than "cls"
I was thinking that maybe this could be a problem when trying to build with C++. The EAL headers in DPDK are meant to be included in C++ apps, I think ``class`` would be an issue then.
If I'm mistaken, then sure, class is a better name.
Quoted 4 lines
lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 64
Quoted 1 line
lib/librte_eal/common/include/rte_devargs.h +1
-0+const char *busstr;/**< bus-related part of device string. */
bus_str?
Quoted 10 lines
lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 65
Quoted 1 line
lib/librte_eal/common/include/rte_devargs.h +1
-0+const char *clsstr;/**< bus-related part of device string. */
class_str?
+ there is a typo in the comment (copy/pasted "bus")lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 67
Quoted 2 lines
lib/librte_eal/common/include/rte_devargs.h +1
-0+const char *data;/**< Device string storage. */ };
Otherwise, ok with the rest of the comments, will fix (as well as the previous emails). Thanks for reading.
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v10 08/27] devargs: add function to parse device layers
11/07/2018 10:41, Gaëtan Rivet:
Quoted 9 lines
Quoted 5 lines
lib/librte_eal/common/include/rte_devargs.h struct rte_devargs { · line 63
Quoted 2 lines
lib/librte_eal/common/include/rte_devargs.h +2
-0+struct rte_bus *bus;/**< bus handle. */ +struct rte_class *cls;/**< class handle. */ "class" is more readable than "cls"
I was thinking that maybe this could be a problem when trying to build with C++. The EAL headers in DPDK are meant to be included in C++ apps, I think ``class`` would be an issue then.
Yes, right.
So only options are "cls" or "devclass".
Up to you.
Quoted 1 line
If I'm mistaken, then sure, class is a better name.
PATCH v10 9/27 eal/dev: add device iterator interface
In reply to Gaetan RivetA device iterator allows iterating over a set of devices. This set is defined by the two descriptions offered,
* rte_bus
* rte_class
Only one description can be provided, or both. It is not allowed to provide no description at all.
Each layer of abstraction then performs a filter based on the description provided. This filtering allows iterating on their internal set of devices, stopping when a match is valid and returning the current iteration context.
This context allows starting the next iteration from the same point and going forward.
--- lib/librte_eal/common/include/rte_dev.h | 47 +++++++++++++++++++++++++ 1 file changed, 47 insertions(+)
lib/librte_eal/common/include/rte_dev.h +47 -0
@@ -285,6 +285,53 @@ __attribute__((used)) = str static const char DRV_EXP_TAG(name, kmod_dep_export)[] \ __attribute__((used)) = str +/** +* Iteration context. +* +* This context carries over the current iteration state. +*/ +struct rte_dev_iterator { +const char *devstr;/**< device string. */ +const char *busstr;/**< bus-related part of device string. */ +const char *clsstr;/**< class-related part of device string. */ +struct rte_bus *bus;/**< bus handle. */ +struct rte_class *cls;/**< class handle. */ +struct rte_device *device;/**< current position. */ +void *class_device;/**< additional specialized context. */ +}; + +/** +* Device iteration function. +* +* Find the next device matching properties passed in parameters. +* The function takes an additional ``start`` parameter, that is +* used as starting context when relevant. +* +* The function returns the current element in the iteration. +* This return value will potentially be used as a start parameter +* in subsequent calls to the function. +* +* The additional iterator parameter is only there if a specific +* implementation needs additional context. It must not be modified by +* the iteration function itself. +* +* @param start +* Starting iteration context. +* +* @param devstr +* Device description string. +* +* @param it +* Device iterator. +* +* @return +* The address of the current element matching the device description +* string. +*/ +typedef void *(*rte_dev_iterate_t )(const void *start, +const char *devstr, +const struct rte_dev_iterator *it); + #ifdef __cplusplus }#endif
Signature · 1 line
2.18.0
PATCH v10 10/27 eal/class: add device iteration
In reply to Gaetan Rivet--- lib/librte_eal/common/include/rte_class.h | 1 + 1 file changed, 1 insertion(+)
lib/librte_eal/common/include/rte_class.h +1 -0
@@ -30,6 +30,7 @@ TAILQ_HEAD(rte_class_list, rte_class);struct rte_class {TAILQ_ENTRY (rte_class) next;/**< Next device class in linked list */ const char *name;/**< Name of the class */ +rte_dev_iterate_t dev_iterate;/**< Device iterator. */ };/**
Signature · 1 line
2.18.0
Re: [PATCH v10 10/27] eal/class: add device iteration
05/07/2018 13:48, Gaetan Rivet:
Quoted 10 lines
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/include/rte_class.h | 1 + 1 file changed, 1 insertion(+)lib/librte_eal/common/include/rte_class.h +0
-0
lib/librte_eal/common/include/rte_class.h TAILQ_HEAD(rte_class_list, rte_class); · line 34
Quoted 5 lines
lib/librte_eal/common/include/rte_class.h +1
-0@@ -30,6 +30,7 @@ TAILQ_HEAD(rte_class_list, rte_class);struct rte_class {TAILQ_ENTRY (rte_class) next;/**< Next device class in linked list */ const char *name;/**< Name of the class */ +rte_dev_iterate_t dev_iterate;/**< Device iterator. */ };
Patches 10 and 11 can be squashed in 9.
PATCH v10 11/27 eal/bus: add device iteration
In reply to Gaetan Rivet--- lib/librte_eal/common/include/rte_bus.h | 1 + 1 file changed, 1 insertion(+)
lib/librte_eal/common/include/rte_bus.h +1 -0
@@ -211,6 +211,7 @@ struct rte_bus {rte_bus_parse_t parse;/**< Parse a device name */ struct rte_bus_conf conf;/**< Bus configuration */ rte_bus_get_iommu_class_t get_iommu_class;/**< Get iommu class */ +rte_dev_iterate_t dev_iterate;/**< Device iterator. */ };/**
Signature · 1 line
2.18.0
PATCH v10 12/27 eal/dev: implement device iteration initialization
In reply to Gaetan RivetParse a device description.
Split this description in their relevant part for each layers. No dynamic allocation is performed.
--- lib/Makefile | 1 + lib/librte_eal/bsdapp/eal/Makefile | 1 + lib/librte_eal/common/eal_common_dev.c | 55 +++++++++++++++++++++++++ lib/librte_eal/common/include/rte_dev.h | 24 +++++++++++ lib/librte_eal/linuxapp/eal/Makefile | 1 + lib/librte_eal/rte_eal_version.map | 1 + 6 files changed, 83 insertions(+)
lib/Makefile +1 -0
@@ -7,6 +7,7 @@ DIRS-y += librte_compat DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs DEPDIRS-librte_kvargs := librte_compat DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal +DEPDIRS-librte_eal := librte_kvargs DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci DEPDIRS-librte_pci := librte_eal DIRS-$(CONFIG_RTE_LIBRTE_RING) += librte_ring
lib/librte_eal/bsdapp/eal/Makefile +1 -0
@@ -18,6 +18,7 @@ CFLAGS += $(WERROR_FLAGS) -O3 LDLIBS += -lexecinfo LDLIBS += -lpthread LDLIBS += -lgcc_s +LDLIBS += -lrte_kvargs EXPORT_MAP := ../../rte_eal_version.map
lib/librte_eal/common/eal_common_dev.c +55 -0
67 lines not on this page · raw message
lib/librte_eal/common/include/rte_dev.h +24 -0
@@ -332,6 +332,30 @@ typedef void *(*rte_dev_iterate_t)(const void *start,const char *devstr, const struct rte_dev_iterator *it); +/** +* Initializes a device iterator. +* +* This iterator allows accessing a list of devices matching a criteria. +* The device matching is made among all buses and classes currently registered, +* filtered by the device description given as parameter. +* +* This function will not allocate any memory. It is safe to stop the +* iteration at any moment and let the iterator go out of context. +* +* @param it +* Device iterator handle. +* +* @param str +* Device description string. +* +* @return +* 0 on successful initialization. +* <0 on error. +*/ +__rte_experimental +int +rte_dev_iterator_init (struct rte_dev_iterator *it,const char *str); + #ifdef __cplusplus }#endif
lib/librte_eal/linuxapp/eal/Makefile +1 -0
@@ -27,6 +27,7 @@ LDLIBS += -lrt ifeq ($(CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES),y) LDLIBS += -lnuma endif +LDLIBS += -lrte_kvargs # specific to linuxapp exec-env SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) := eal.c
lib/librte_eal/rte_eal_version.map +1 -0
@@ -251,6 +251,7 @@ EXPERIMENTAL { rte_dev_event_callback_unregister; rte_dev_event_monitor_start; rte_dev_event_monitor_stop; + rte_dev_iterator_init; rte_devargs_add; rte_devargs_dump; rte_devargs_insert;
Signature · 1 line
2.18.0
PATCH v10 13/27 eal/dev: implement device iteration
In reply to Gaetan RivetUse the iteration hooks in the abstraction layers to perform the requested filtering on the internal device lists.
--- lib/librte_eal/common/eal_common_dev.c | 168 ++++++++++++++++++++++++ lib/librte_eal/common/include/rte_dev.h | 26 ++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 195 insertions(+)
lib/librte_eal/common/eal_common_dev.c +168 -0
@@ -45,6 +45,28 @@ static struct dev_event_cb_list dev_event_cbs;/* spinlock for device callbacks */ static rte_spinlock_t dev_event_lock =RTE_SPINLOCK_INITIALIZER ; +struct dev_next_ctx { +struct rte_dev_iterator *it; +const char *busstr; +const char *clsstr; +}; + +#define CTX (it, busstr, clsstr) \ + (&(const struct dev_next_ctx){ \ + .it = it, \ + .busstr = busstr, \ + .clsstr = clsstr, \ + }) + +#define ITCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->it) + +#define BUSCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->busstr) + +#define CLSCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->clsstr) +static int cmp_detached_dev_name (const struct rte_device *dev,const void *_name) {
@@ -398,3 +420,149 @@ rte_dev_iterator_init(struct rte_dev_iterator *it, get_out:return -rte_errno; } + +static char * +dev_str_sane_copy (const char *str) +{ +size_t end; +char *copy; + + end =strcspn (str,",/" ); +if (str[end] ==',' ) { + copy =strdup (&str[end +1 ]); + }else { +/* '/' or '\0' */ + copy =strdup ("" ); + } +if (copy ==NULL ) { + rte_errno =ENOMEM ; + }else { +char *slash; + + slash =strchr (copy,'/' ); +if (slash !=NULL ) + slash[0 ] ='\0' ; + } +return copy; +} + +static int +class_next_dev_cmp (const struct rte_class *cls, +const void *ctx) +{ +struct rte_dev_iterator *it; +const char *clsstr =NULL ; +void *dev; + +if (cls->dev_iterate ==NULL ) +return 1 ; + it =ITCTX (ctx); + clsstr =CLSCTX (ctx); + dev = it->class_device; +/* it->clsstr != NULL means a class +* was specified in the devstr. +*/ +if (it->clsstr !=NULL && cls != it->cls) +return 1 ; +/* If an error occurred previously, +* no need to test further. +*/ +if (rte_errno !=0 ) +return -1 ; + dev = cls->dev_iterate (dev, clsstr, it); + it->class_device = dev; +return dev ==NULL ; +} + +static int +bus_next_dev_cmp (const struct rte_bus *bus, +const void *ctx) +{ +struct rte_device *dev =NULL ; +struct rte_class *cls =NULL ; +struct rte_dev_iterator *it; +const char *busstr =NULL ; + +if (bus->dev_iterate ==NULL ) +return 1 ; + it =ITCTX (ctx); + busstr =BUSCTX (ctx); + dev = it->device; +/* it->busstr != NULL means a bus +* was specified in the devstr. +*/ +if (it->busstr !=NULL && bus != it->bus) +return 1 ; +/* If an error occurred previously, +* no need to test further. +*/ +if (rte_errno !=0 ) +return -1 ; +if (it->clsstr ==NULL ) { + dev = bus->dev_iterate (dev, busstr, it); + goto end; + } +/* clsstr != NULL */ +if (dev ==NULL ) { +next_dev_on_bus: + dev = bus->dev_iterate (dev, busstr, it); + it->device = dev; + } +if (dev ==NULL ) +return 1 ; +if (it->cls !=NULL ) + cls =TAILQ_PREV (it->cls, rte_class_list, next); + cls =rte_class_find (cls, class_next_dev_cmp, ctx); +if (cls !=NULL ) { + it->cls = cls; + goto end; + } + goto next_dev_on_bus; +end: + it->device = dev; +return dev ==NULL ; +} +__rte_experimental +struct rte_device * +rte_dev_iterator_next (struct rte_dev_iterator *it) +{ +struct rte_bus *bus =NULL ; +int old_errno = rte_errno; +char *busstr =NULL ; +char *clsstr =NULL ; + + rte_errno =0 ; +if (it->busstr ==NULL && it->clsstr ==NULL ) { +/* Invalid iterator. */ + rte_errno =EINVAL ; +return NULL ; + } +if (it->bus !=NULL ) + bus =TAILQ_PREV (it->bus, rte_bus_list, next); +if (it->busstr !=NULL ) { + busstr =dev_str_sane_copy (it->busstr); +if (busstr ==NULL ) + goto out; + } +if (it->clsstr !=NULL ) { + clsstr =dev_str_sane_copy (it->clsstr); +if (clsstr ==NULL ) + goto out; + } +while ((bus =rte_bus_find (bus, bus_next_dev_cmp, +CTX (it, busstr, clsstr)))) { +if (it->device !=NULL ) { + it->bus = bus; + goto out; + } +if (it->busstr !=NULL || + rte_errno !=0 ) +break ; + } +if (rte_errno ==0 ) + rte_errno = old_errno; +out: +free (busstr); +free (clsstr); +return it->device; +}
lib/librte_eal/common/include/rte_dev.h +26 -0
@@ -356,6 +356,32 @@ __rte_experimentalint rte_dev_iterator_init (struct rte_dev_iterator *it,const char *str); +/** +* Iterates on a device iterator. +* +* Generates a new rte_device handle corresponding to the next element +* in the list described in comprehension by the iterator. +* +* The next object is returned, and the iterator is updated. +* +* @param it +* Device iterator handle. +* +* @return +* An rte_device handle if found. +* NULL if an error occurred (rte_errno is set). +* NULL if no device could be found (rte_errno is not set). +*/ +__rte_experimental +struct rte_device * +rte_dev_iterator_next (struct rte_dev_iterator *it); + +#define RTE_DEV_FOREACH (dev, devstr, it) \ + for (rte_dev_iterator_init(it, devstr), \ + dev = rte_dev_iterator_next(it); \ + dev != NULL; \ + dev = rte_dev_iterator_next(it)) + #ifdef __cplusplus }#endif
lib/librte_eal/rte_eal_version.map +1 -0
@@ -252,6 +252,7 @@ EXPERIMENTAL { rte_dev_event_monitor_start; rte_dev_event_monitor_stop; rte_dev_iterator_init; + rte_dev_iterator_next; rte_devargs_add; rte_devargs_dump; rte_devargs_insert;
Signature · 1 line
2.18.0
PATCH v10 14/27 kvargs: add generic string matching callback
In reply to Gaetan RivetThis function can be used as a callback torte_kvargs_process.
This should reduce code duplication.
--- lib/librte_kvargs/rte_kvargs.c | 10 +++++++++ lib/librte_kvargs/rte_kvargs.h | 28 ++++++++++++++++++++++++ lib/librte_kvargs/rte_kvargs_version.map | 1 + 3 files changed, 39 insertions(+)
lib/librte_kvargs/rte_kvargs.c +10 -0
@@ -193,3 +193,13 @@ rte_kvargs_parse2(const char *args, const char * const valid_keys[],free (copy);return kvlist; } + +__rte_experimental +int +rte_kvargs_strcmp(const char *key __rte_unused, +const char *value,void *opaque) +{ +const char *str = opaque; + +return -abs (strcmp (str, value)); +}
lib/librte_kvargs/rte_kvargs.h +28 -0
@@ -25,6 +25,8 @@extern "C" {#endif +#include <rte_compat.h> +/** Maximum number of key/value associations */ #define RTE_KVARGS_MAX 32
@@ -151,6 +153,32 @@ int rte_kvargs_process(const struct rte_kvargs *kvlist,unsigned rte_kvargs_count (const struct rte_kvargs *kvlist,const char *key_match); +/** +* Generic kvarg handler for string comparison. +* +* This function can be used for a generic string comparison processing +* on a list of kvargs. +* +* @param key +* kvarg pair key. +* +* @param value +* kvarg pair value. +* +* @param opaque +* Opaque pointer to a string. +* +* @return +* 0 if the strings match. +* !0 otherwise or on error. +* +* Unless strcmp, comparison ordering is not kept. +* In order for rte_kvargs_process to stop processing on match error, +* a negative value is returned even if strcmp had returned a positive one. +*/ +__rte_experimental +int rte_kvargs_strcmp(const char *key, const char *value, void *opaque); + #ifdef __cplusplus }#endif
lib/librte_kvargs/rte_kvargs_version.map +1 -0
@@ -13,5 +13,6 @@ EXPERIMENTAL { global: rte_kvargs_parse2; + rte_kvargs_strcmp; } DPDK_2.0;
Signature · 1 line
2.18.0
PATCH v10 15/27 bus/pci: implement device iteration and comparison
In reply to Gaetan Rivet--- drivers/bus/pci/Makefile | 3 +- drivers/bus/pci/meson.build | 6 +++- drivers/bus/pci/pci_common.c | 3 +- drivers/bus/pci/pci_params.c | 53 ++++++++++++++++++++++++++++++++++++ drivers/bus/pci/private.h | 25 +++++++++++++++++ 5 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 drivers/bus/pci/pci_params.c
drivers/bus/pci/Makefile +2 -1
@@ -26,10 +26,11 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/$(SYSTEM)app/eal CFLAGS += -DALLOW_EXPERIMENTAL_API LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring -LDLIBS += -lrte_ethdev -lrte_pci +LDLIBS += -lrte_ethdev -lrte_pci -lrte_kvargs include $(RTE_SDK)/drivers/bus/pci/$(SYSTEM)/Makefile SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) := $(addprefix $(SYSTEM)/,$(SRCS)) +SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_params.c SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_common.c SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_common_uio.c
drivers/bus/pci/meson.build +5 -1
@@ -3,7 +3,9 @@ deps += ['pci'] install_headers('rte_bus_pci.h') -sources = files('pci_common.c', 'pci_common_uio.c') +sources = files('pci_common.c', + 'pci_common_uio.c', + 'pci_params.c') if host_machine.system() == 'linux' sources += files('linux/pci.c', 'linux/pci_uio.c',
@@ -17,3 +19,5 @@ endif # memseg walk is not part of stable API yet allow_experimental_apis = true + +deps += ['kvargs']
drivers/bus/pci/pci_common.c +1 -2
@@ -29,8 +29,6 @@static void rte_pci_remove_device (struct rte_pci_device *pci_device); -extern struct rte_pci_bus rte_pci_bus; -#define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"const char *rte_pci_get_sysfs_path (void )
@@ -437,6 +435,7 @@ struct rte_pci_bus rte_pci_bus = { .unplug = pci_unplug, .parse = pci_parse, .get_iommu_class = rte_pci_get_iommu_class, + .dev_iterate = rte_pci_dev_iterate, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
drivers/bus/pci/pci_params.c new file +53 -0
53 lines not on this page · raw message
drivers/bus/pci/private.h +25 -0
@@ -10,6 +10,8 @@#include <rte_pci.h> #include <rte_bus_pci.h> +extern struct rte_pci_bus rte_pci_bus; +struct rte_pci_driver ;struct rte_pci_device ;
@@ -166,4 +168,27 @@ rte_pci_match(const struct rte_pci_driver *pci_drv,enum rte_iova_mode rte_pci_get_iommu_class (void ); +/* +* Iterate over internal devices, +* matching any device against the provided +* string. +* +* @param start +* Iteration starting point. +* +* @param str +* Device string to match against. +* +* @param it +* (unused) iterator structure. +* +* @return +* A pointer to the next matching device if any. +* NULL otherwise. +*/ +void * +rte_pci_dev_iterate (const void *start, +const char *str, +const struct rte_dev_iterator *it); +#endif /* _PCI_PRIVATE_H_ */
Signature · 1 line
2.18.0
PATCH v10 16/27 bus/pci: add device matching field id
In reply to Gaetan RivetThe PCI bus can now parse a matching field "id" as follows:
"bus=pci,id=0000:00:00.0"
or
"bus=pci,id=00:00.0"
--- drivers/bus/pci/pci_params.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)
drivers/bus/pci/pci_params.c +27 -2
@@ -3,6 +3,7 @@*/ #include <rte_bus.h> +#include <rte_bus_pci.h> #include <rte_dev.h> #include <rte_errno.h> #include <rte_kvargs.h>
@@ -11,21 +12,45 @@#include "private.h" enum pci_params { +RTE_PCI_PARAMS_ID ,RTE_PCI_PARAMS_MAX , };static const char *const pci_params_keys[] = { + [RTE_PCI_PARAMS_ID ] ="id" , [RTE_PCI_PARAMS_MAX ] =NULL , }; +static int +pci_addr_kv_cmp(const char *key __rte_unused, +const char *value, +void *_addr2) +{ +struct rte_pci_addr _addr1; +struct rte_pci_addr *addr1 = &_addr1; +struct rte_pci_addr *addr2 = _addr2; + +if (rte_pci_addr_parse (value, addr1)) +return -1 ; +return -abs (rte_pci_addr_cmp (addr1, addr2)); +} +static int pci_dev_match (const struct rte_device *dev,const void *_kvlist) {const struct rte_kvargs *kvlist = _kvlist; +const struct rte_pci_device *pdev; - (void ) dev; - (void ) kvlist; +if (kvlist ==NULL ) +/* Empty string matches everything. */ +return 0 ; + pdev =RTE_DEV_TO_PCI_CONST (dev); +/* if any field does not match. */ +if (rte_kvargs_process (kvlist,"id" , + &pci_addr_kv_cmp, + (void *)(intptr_t )&pdev->addr)) +return 1 ;return 0 ; }
Signature · 1 line
2.18.0
PATCH v10 17/27 bus/vdev: implement device iteration
In reply to Gaetan Rivet--- drivers/bus/vdev/Makefile | 3 +- drivers/bus/vdev/meson.build | 5 +++- drivers/bus/vdev/vdev.c | 10 ++++--- drivers/bus/vdev/vdev_params.c | 51 +++++++++++++++++++++++++++++++++ drivers/bus/vdev/vdev_private.h | 26 +++++++++++++++++ 5 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 drivers/bus/vdev/vdev_params.c create mode 100644 drivers/bus/vdev/vdev_private.h
drivers/bus/vdev/Makefile +2 -1
@@ -19,8 +19,9 @@ EXPORT_MAP := rte_bus_vdev_version.map LIBABIVER := 1 SRCS-y += vdev.c +SRCS-y += vdev_params.c -LDLIBS += -lrte_eal +LDLIBS += -lrte_eal -lrte_kvargs # # Export include files
drivers/bus/vdev/meson.build +4 -1
@@ -1,7 +1,10 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2017 Intel Corporation -sources = files('vdev.c') +sources = files('vdev.c', + 'vdev_params.c') install_headers('rte_bus_vdev.h') allow_experimental_apis = true + +deps += ['kvargs']
drivers/bus/vdev/vdev.c +6 -4
@@ -23,6 +23,7 @@#include "rte_bus_vdev.h" #include "vdev_logs.h" +#include "vdev_private.h" #define VDEV_MP_KEY "bus_vdev_mp"
@@ -493,9 +494,9 @@ vdev_probe(void)return ret; } -static struct rte_device * -vdev_find_device (const struct rte_device *start,rte_dev_cmp_t cmp, -const void *data) +struct rte_device * +rte_vdev_find_device (const struct rte_device *start,rte_dev_cmp_t cmp, +const void *data) {const struct rte_vdev_device *vstart;struct rte_vdev_device *dev;
@@ -532,10 +533,11 @@ vdev_unplug(struct rte_device *dev)static struct rte_bus rte_vdev_bus = { .scan = vdev_scan, .probe = vdev_probe, - .find_device = vdev_find_device, + .find_device = rte_vdev_find_device, .plug = vdev_plug, .unplug = vdev_unplug, .parse = vdev_parse, + .dev_iterate = rte_vdev_dev_iterate, };RTE_REGISTER_BUS (vdev, rte_vdev_bus);
drivers/bus/vdev/vdev_params.c new file +51 -0
51 lines not on this page · raw message
drivers/bus/vdev/vdev_private.h new file +26 -0
@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: BSD-3-Clause +* Copyright 2018 Gaëtan Rivet +*/ + +#ifndef _VDEV_PRIVATE_H_ +#define _VDEV_PRIVATE_H_ + +#ifdef __cplusplus +extern "C" { +#endif + +struct rte_device * +rte_vdev_find_device (const struct rte_device *start, +rte_dev_cmp_t cmp, +const void *data); + +void * +rte_vdev_dev_iterate (const void *start, +const char *str, +const struct rte_dev_iterator *it); + +#ifdef __cplusplus +} +#endif + +#endif /* _VDEV_PRIVATE_H_ */
Signature · 1 line
2.18.0
PATCH v10 18/27 bus/vdev: add device matching field driver
In reply to Gaetan RivetThe vdev bus parses a field "driver", matching
a vdev driver name with one passed as follows:
"bus=vdev,driver=xxxx"
--- drivers/bus/vdev/vdev_params.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
drivers/bus/vdev/vdev_params.c +13 -2
@@ -4,6 +4,7 @@#include <rte_dev.h> #include <rte_bus.h> +#include <rte_bus_vdev.h> #include <rte_kvargs.h> #include <rte_errno.h>
@@ -11,10 +12,12 @@#include "vdev_private.h" enum vdev_params { +RTE_VDEV_PARAMS_DRIVER ,RTE_VDEV_PARAMS_MAX , };static const char *const vdev_params_keys[] = { + [RTE_VDEV_PARAMS_DRIVER ] ="driver" , [RTE_VDEV_PARAMS_MAX ] =NULL , };
@@ -23,9 +26,17 @@ vdev_dev_match(const struct rte_device *dev, const void *_kvlist) {const struct rte_kvargs *kvlist = _kvlist; +const struct rte_vdev_device *vdev; - (void ) kvlist; - (void ) dev; +if (kvlist ==NULL ) +/* Empty string matches everything. */ +return 0 ; + vdev =RTE_DEV_TO_VDEV_CONST (dev); +/* if any field does not match. */ +if (rte_kvargs_process (kvlist,"driver" , + rte_kvargs_strcmp, + (void *)(intptr_t )vdev->device.driver->name)) +return -1 ;return 0 ; }
Signature · 1 line
2.18.0
PATCH v10 19/27 ethdev: add private generic device iterator
In reply to Gaetan RivetThis iterator can be customized with a comparison function that will trigger a stopping condition.
It can be leveraged to write several different iterators that have similar but non-identical purposes.
It is private to librte_ethdev.
--- lib/librte_ethdev/Makefile | 1 + lib/librte_ethdev/eth_private.c | 31 +++++++++++++++++++++++++++++++ lib/librte_ethdev/eth_private.h | 26 ++++++++++++++++++++++++++ lib/librte_ethdev/meson.build | 3 ++- 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 lib/librte_ethdev/eth_private.c create mode 100644 lib/librte_ethdev/eth_private.h
lib/librte_ethdev/Makefile +1 -0
@@ -18,6 +18,7 @@ EXPORT_MAP := rte_ethdev_version.map LIBABIVER := 9 +SRCS-y += eth_private.c SRCS-y += rte_ethdev.c SRCS-y += rte_flow.c SRCS-y += rte_tm.c
lib/librte_ethdev/eth_private.c new file +31 -0
@@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: BSD-3-Clause +* Copyright(c) 2018 Gaëtan Rivet +*/ + +#include "rte_ethdev.h" +#include "eth_private.h" + +struct rte_eth_dev * +eth_find_device (const struct rte_eth_dev *start,rte_eth_cmp_t cmp, +const void *data) +{ +struct rte_eth_dev *edev; +ptrdiff_t idx; + +/* Avoid Undefined Behaviour */ +if (start !=NULL && + (start < &rte_eth_devices[0 ] || + start > &rte_eth_devices[RTE_MAX_ETHPORTS ])) +return NULL ; +if (start !=NULL ) + idx = start - &rte_eth_devices[0 ] +1 ; +else + idx =0 ; +for (; idx <RTE_MAX_ETHPORTS ; idx++) { + edev = &rte_eth_devices[idx]; +if (cmp (edev, data) ==0 ) +return edev; + } +return NULL ; +} +
lib/librte_ethdev/eth_private.h new file +26 -0
@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: BSD-3-Clause +* Copyright(c) 2018 Gaëtan Rivet +*/ + +#ifndef _RTE_ETH_PRIVATE_H_ +#define _RTE_ETH_PRIVATE_H_ + +#include "rte_ethdev.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/* Generic rte_eth_dev comparison function. */ +typedef int (*rte_eth_cmp_t )(const struct rte_eth_dev *,const void *); + +/* Generic rte_eth_dev iterator. */ +struct rte_eth_dev * +eth_find_device (const struct rte_eth_dev *_start,rte_eth_cmp_t cmp, +const void *data); + +#ifdef __cplusplus +} +#endif + +#endif /* _RTE_ETH_PRIVATE_H_ */
lib/librte_ethdev/meson.build +2 -1
@@ -4,7 +4,8 @@ name = 'ethdev' version = 9 allow_experimental_apis = true -sources = files('ethdev_profile.c', +sources = files('eth_private.c', + 'ethdev_profile.c', 'rte_ethdev.c', 'rte_flow.c', 'rte_mtr.c',
Signature · 1 line
2.18.0
PATCH v10 20/27 ethdev: register ether layer as a class
In reply to Gaetan Rivet--- lib/librte_ethdev/Makefile | 3 +- lib/librte_ethdev/meson.build | 1 + lib/librte_ethdev/rte_class_eth.c | 79 +++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 lib/librte_ethdev/rte_class_eth.c
lib/librte_ethdev/Makefile +2 -1
@@ -12,7 +12,7 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API CFLAGS += -O3 CFLAGS += $(WERROR_FLAGS) LDLIBS += -lrte_net -lrte_eal -lrte_mempool -lrte_ring -LDLIBS += -lrte_mbuf +LDLIBS += -lrte_mbuf -lrte_kvargs EXPORT_MAP := rte_ethdev_version.map
@@ -20,6 +20,7 @@ LIBABIVER := 9 SRCS-y += eth_private.c SRCS-y += rte_ethdev.c +SRCS-y += rte_class_eth.c SRCS-y += rte_flow.c SRCS-y += rte_tm.c SRCS-y += rte_mtr.c
lib/librte_ethdev/meson.build +1 -0
@@ -6,6 +6,7 @@ version = 9 allow_experimental_apis = true sources = files('eth_private.c', 'ethdev_profile.c', + 'rte_class_eth.c', 'rte_ethdev.c', 'rte_flow.c', 'rte_mtr.c',
lib/librte_ethdev/rte_class_eth.c new file +79 -0
79 lines not on this page · raw message
Signature · 1 line
2.18.0
PATCH v10 21/27 ethdev: add device matching field name
In reply to Gaetan RivetThe eth device class can now parse a field name,
matching the eth_dev name with one passed as
"class=eth,name=xxxxxx"
--- lib/librte_ethdev/rte_class_eth.c | 7 +++++++ 1 file changed, 7 insertions(+)
lib/librte_ethdev/rte_class_eth.c +7 -0
@@ -15,10 +15,12 @@#include "eth_private.h" enum eth_params { +RTE_ETH_PARAMS_NAME ,RTE_ETH_PARAMS_MAX , };static const char *const eth_params_keys[] = { + [RTE_ETH_PARAMS_NAME ] ="name" , [RTE_ETH_PARAMS_MAX ] =NULL , };
@@ -39,6 +41,7 @@ eth_dev_match(const struct rte_eth_dev *edev, {const struct eth_dev_match_arg *arg = _arg;const struct rte_kvargs *kvlist = arg->kvlist; +struct rte_eth_dev_data *data;if (edev->state ==RTE_ETH_DEV_UNUSED )return -1 ;
@@ -47,6 +50,10 @@ eth_dev_match(const struct rte_eth_dev *edev,if (kvlist ==NULL )/* Empty string matches everything. */ return 0 ; + data = edev->data; +if (rte_kvargs_process (kvlist,"name" , + rte_kvargs_strcmp, data->name)) +return -1 ;return 0 ; }
Signature · 1 line
2.18.0
PATCH v10 22/27 app/testpmd: add show device command
In reply to Gaetan RivetA new interactive command is offered:
show device <device description>
This commands lists all rte_device element matching the device description. e.g.:
show device bus=pci
show device bus=vdev
show device bus=vdev/class=eth
show device bus=vdev,driver=net_ring/class=eth
show device bus=vdev/class=eth,name=net_ring0
These devices may not be otherwise useful, some buses will spawn devices to keep track of their assets without having a driver to use them.
--- app/test-pmd/cmdline.c | 54 +++++++++++++++++++++ doc/guides/testpmd_app_ug/testpmd_funcs.rst | 24 +++++++++ 2 files changed, 78 insertions(+)
app/test-pmd/cmdline.c +54 -0
@@ -228,6 +228,9 @@ static void cmd_help_long_parsed(void *parsed_result,"show port tm node stats (port_id) (node_id) (clear)\n" " Display the port TM node stats.\n\n" +"show device (device_string)\n" +" Display devices matching the device string.\n\n" + ); }
@@ -7083,6 +7086,56 @@ cmdline_parse_inst_t cmd_showportall = { }, }; +/* *** SHOW DEVICE INFO *** */ +struct cmd_showdevice_result { +cmdline_fixed_string_t show; +cmdline_fixed_string_t device; +cmdline_fixed_string_t filter; +}; + +static void +cmd_showdevice_dump_device (const struct rte_device *dev) +{ +const struct rte_driver *drv = dev->driver; + +printf ("0x%p: %s:%s\n" , (const void *)dev, dev->name, + drv ? drv->name :"<nil>" ); +} + +static void cmd_showdevice_parsed (void *parsed_result, + __attribute__((unused))struct cmdline *cl, + __attribute__((unused))void *data) +{ +struct cmd_showdevice_result *res = parsed_result; +struct rte_dev_iterator it; +const struct rte_device *dev; + + RTE_DEV_FOREACH(dev, res->filter, &it) +cmd_showdevice_dump_device (dev); +} + +cmdline_parse_token_string_t cmd_showdevice_show = + TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, + show,"show" ); +cmdline_parse_token_string_t cmd_showdevice_device = + TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, + device,"device" ); +cmdline_parse_token_string_t cmd_showdevice_filter = + TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, + filter,NULL ); + +cmdline_parse_inst_t cmd_showdevice = { + .f = cmd_showdevice_parsed, + .data =NULL , + .help_str ="show device <device string>" , + .tokens = { + (void *)&cmd_showdevice_show, + (void *)&cmd_showdevice_device, + (void *)&cmd_showdevice_filter, +NULL , + }, +}; +/* *** SHOW PORT INFO *** */ struct cmd_showport_result {cmdline_fixed_string_t show;
@@ -17262,6 +17315,7 @@ cmdline_parse_ctx_t main_ctx[] = { (cmdline_parse_inst_t *)&cmd_help_long, (cmdline_parse_inst_t *)&cmd_quit, (cmdline_parse_inst_t *)&cmd_load_from_file, + (cmdline_parse_inst_t *)&cmd_showdevice, (cmdline_parse_inst_t *)&cmd_showport, (cmdline_parse_inst_t *)&cmd_showqueue, (cmdline_parse_inst_t *)&cmd_showportall,
doc/guides/testpmd_app_ug/testpmd_funcs.rst +24 -0
@@ -2628,6 +2628,30 @@ set the traffic management default hierarchy on the port:: testpmd> set port tm hierarchy default (port_id) +Device functions +---------------- + +Show devices +~~~~~~~~~~~~ + +Display any registered devices:: + + testpmd> show device <device_string> + +where: + +* ``device_string``: Device description string, of the format + + layer[/layer[/layer]] + + where one layer is in the form + + layer_key=layer_name[,key1=value1[,...]] + + Valid layer keys are ``bus`` and ``class``. + Their respective values is defined by registered ``bus`` and ``class`` + drivers. + Filter Functions ----------------
Signature · 1 line
2.18.0
Re: [PATCH v10 22/27] app/testpmd: add show device command
Quoted 21 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Gaetan Rivet
Sent: Thursday, July 5, 2018 12:48 PM
To: [email protected]
Cc: Gaetan Rivet <[email protected]>
Subject: [dpdk-dev] [PATCH v10 22/27] app/testpmd: add show device commandA new interactive command is offered:
show device <device description>
This commands lists all
rte_deviceelement matching the device description. e.g.:show device bus=pci
show device bus=vdev
show device bus=vdev/class=eth
show device bus=vdev,driver=net_ring/class=eth
show device bus=vdev/class=eth,name=net_ring0
Quoted 4 lines
These devices may not be otherwise useful, some buses will spawn devices to keep track of their assets without having a driver to use them.
Signed-off-by Gaetan Rivet <[email protected]>
PATCH v10 23/27 bus/pci: pre-process declarative PCI devargs
In reply to Gaetan RivetThe new devargs format does not recognize a particular device name. Each bus uses its specific format.
Instead of introducing a new bus API, process those devargs privately for the moment. Prepare them for matching during scan against the bus devices.
--- drivers/bus/pci/bsd/pci.c | 5 ++++ drivers/bus/pci/linux/pci.c | 5 ++++ drivers/bus/pci/pci_params.c | 51 ++++++++++++++++++++++++++++++++++++ drivers/bus/pci/private.h | 16 +++++++++++ 4 files changed, 77 insertions(+)
drivers/bus/pci/bsd/pci.c +5 -0
@@ -327,6 +327,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)int rte_pci_scan (void ) { +struct rte_devargs *devargs;int fd;unsigned dev_count =0 ;struct pci_conf matches[16 ];
@@ -342,6 +343,10 @@ rte_pci_scan(void)if (!rte_eal_has_pci ())return 0 ; + RTE_EAL_DEVARGS_FOREACH("pci", devargs) +if (rte_pci_devargs_prepare (devargs)) +continue ; + fd =open ("/dev/pci" ,O_RDONLY );if (fd <0 ) {RTE_LOG (ERR ,EAL ,"%s(): error opening /dev/pci\n" , __func__);
drivers/bus/pci/linux/pci.c +5 -0
@@ -430,6 +430,7 @@ parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)int rte_pci_scan (void ) { +struct rte_devargs *devargs;struct dirent *e;DIR *dir;char dirname[PATH_MAX ];
@@ -439,6 +440,10 @@ rte_pci_scan(void)if (!rte_eal_has_pci ())return 0 ; + RTE_EAL_DEVARGS_FOREACH("pci", devargs) +if (rte_pci_devargs_prepare (devargs)) +continue ; +#ifdef VFIO_PRESENT if (!pci_vfio_is_enabled ())RTE_LOG (DEBUG ,EAL ,"VFIO PCI modules not loaded\n" );
drivers/bus/pci/pci_params.c +51 -0
63 lines not on this page · raw message
drivers/bus/pci/private.h +16 -0
@@ -191,4 +191,20 @@ rte_pci_dev_iterate(const void *start,const char *str, const struct rte_dev_iterator *it); +/* +* Prepare a devargs meant for this bus. +* This function is only used for a transitory period, +* to translate the new devargs format in one +* compatible with the old form. +* +* @param da +* Devargs to process. +* +* @return +* 0 on success. +* <0 on error. +*/ +int +rte_pci_devargs_prepare (struct rte_devargs *da); +#endif /* _PCI_PRIVATE_H_ */
Signature · 1 line
2.18.0
PATCH v10 24/27 bus/vdev: pre-process declarative vdev devargs
In reply to Gaetan RivetThe new devargs format does not recognize a particular device name. Each bus uses its specific format.
Process each devargs privately prior to attempting a bus scan. Prepare them if they are using the new devargs format.
--- drivers/bus/vdev/vdev.c | 10 ++++--- drivers/bus/vdev/vdev_params.c | 50 +++++++++++++++++++++++++++++++++ drivers/bus/vdev/vdev_private.h | 6 ++++ 3 files changed, 62 insertions(+), 4 deletions(-)
drivers/bus/vdev/vdev.c +6 -4
@@ -115,8 +115,8 @@ rte_vdev_remove_custom_scan(rte_vdev_scan_callback callback, void *user_arg)return 0 ; } -static int -vdev_parse (const char *name,void *addr) +int +rte_vdev_parse (const char *name,void *addr) {struct rte_vdev_driver **out = addr;struct rte_vdev_driver *driver =NULL ;
@@ -148,7 +148,7 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)VDEV_LOG (DEBUG ,"Search driver %s to probe device %s" , name,rte_vdev_device_name (dev)); -if (vdev_parse (name, &driver)) +if (rte_vdev_parse (name, &driver))return -1 ; dev->device.driver = &driver->driver; ret = driver->probe (dev);
@@ -443,6 +443,8 @@ vdev_scan(void)/* for virtual devices we scan the devargs_list populated via cmdline */ RTE_EAL_DEVARGS_FOREACH("vdev", devargs) { +if (rte_vdev_devargs_prepare (devargs)) +continue ; dev =calloc (1 ,sizeof (*dev));if (!dev)
@@ -536,7 +538,7 @@ static struct rte_bus rte_vdev_bus = { .find_device = rte_vdev_find_device, .plug = vdev_plug, .unplug = vdev_unplug, - .parse = vdev_parse, + .parse = rte_vdev_parse, .dev_iterate = rte_vdev_dev_iterate, };
drivers/bus/vdev/vdev_params.c +50 -0
64 lines not on this page · raw message
drivers/bus/vdev/vdev_private.h +6 -0
@@ -19,6 +19,12 @@ rte_vdev_dev_iterate(const void *start,const char *str, const struct rte_dev_iterator *it); +int +rte_vdev_parse (const char *name,void *addr); + +int +rte_vdev_devargs_prepare (struct rte_devargs *da); + #ifdef __cplusplus }#endif
Signature · 1 line
2.18.0
PATCH v10 25/27 bus/pci: process declarative PCI devargs
In reply to Gaetan RivetIntroduce the facility to process future PCI parameters.
Once the matching between PCI devices and devargs has been done, it is possible to process each devargs. New parameters would have the PCI device handle to work with when parsing the device (bus specific) parameters.
--- drivers/bus/pci/pci_common.c | 3 +++ drivers/bus/pci/pci_params.c | 10 ++++++++++ drivers/bus/pci/private.h | 13 +++++++++++++ 3 files changed, 26 insertions(+)
drivers/bus/pci/pci_common.c +3 -0
@@ -249,6 +249,9 @@ pci_probe_all_drivers(struct rte_pci_device *dev)if (dev->driver !=NULL )return 0 ; +if (rte_pci_devargs_process (dev) <0 ) +return -1 ; +FOREACH_DRIVER_ON_PCIBUS (dr) { rc =rte_pci_probe_one_driver (dr, dev);if (rc <0 )
drivers/bus/pci/pci_params.c +10 -0
@@ -127,3 +127,13 @@ rte_pci_devargs_prepare(struct rte_devargs *da)rte_kvargs_free (kvargs);return ret; } + +int +rte_pci_devargs_process (struct rte_pci_device *pdev) +{ +/* For the moment, no PCI param +* needs to be processed. +*/ + (void ) pdev; +return 0 ; +}
drivers/bus/pci/private.h +13 -0
@@ -207,4 +207,17 @@ rte_pci_dev_iterate(const void *start,int rte_pci_devargs_prepare (struct rte_devargs *da); +/* +* Process the device devargs, if any. +* +* @param pdev +* PCI device +* +* @return +* 0 on success. +* <0 on error. +*/ +int +rte_pci_devargs_process (struct rte_pci_device *pdev); +#endif /* _PCI_PRIVATE_H_ */
Signature · 1 line
2.18.0
PATCH v10 26/27 ethdev: process declarative eth devargs
In reply to Gaetan RivetProcess the class-specific arguments in a devargs.
This processing takes the form of setting the proper eth_dev fields when relevant.
--- lib/librte_ethdev/eth_private.h | 5 +++ lib/librte_ethdev/rte_class_eth.c | 61 +++++++++++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev.c | 7 ++++ 3 files changed, 73 insertions(+)
lib/librte_ethdev/eth_private.h +5 -0
@@ -19,6 +19,11 @@ struct rte_eth_dev * eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp, const void *data); +/* Generic rte_eth_dev parameters processor. */ +int +rte_eth_dev_args_parse (struct rte_eth_dev *eth_dev, +struct rte_devargs *da); + #ifdef __cplusplus }#endif
lib/librte_ethdev/rte_class_eth.c +61 -0
73 lines not on this page · raw message
lib/librte_ethdev/rte_ethdev.c +7 -0
@@ -41,6 +41,7 @@#include "rte_ethdev.h" #include "rte_ethdev_driver.h" #include "ethdev_profile.h" +#include "eth_private.h" static int ethdev_logtype;
@@ -3594,6 +3595,12 @@ rte_eth_dev_create(struct rte_device *device, const char *name, } } + retval =rte_eth_dev_args_parse (ethdev, device->devargs); +if (retval) { +RTE_LOG (ERR ,EAL ,"ethdev parsing failed" ); + goto probe_failed; + } + retval =ethdev_init (ethdev, init_params);if (retval) {RTE_LOG (ERR ,EAL ,"ethdev initialisation failed" );
Signature · 1 line
2.18.0
PATCH v10 27/27 eal: add generic dev parameter
In reply to Gaetan RivetAdd the --dev parameter to the EAL.
This new parameter takes a generic device declaration as argument.
It uses the new devargs parsing API.
--- lib/librte_eal/common/eal_common_devargs.c | 4 +++ lib/librte_eal/common/eal_common_options.c | 36 +++++++++++++++++++--- lib/librte_eal/common/eal_options.h | 2 ++ 3 files changed, 37 insertions(+), 5 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +4 -0
@@ -218,6 +218,10 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev)if (da ==NULL )return -EINVAL ; +if (strncmp (dev,"bus=" ,4 ) ==0 || +strncmp (dev,"class=" ,6 ) ==0 ) +return rte_devargs_layers_parse (da, dev); +/* Retrieve eventual bus info */ do { devname = dev;
lib/librte_eal/common/eal_common_options.c +31 -5
@@ -54,6 +54,7 @@ const struct option eal_long_options[] = { {OPT_BASE_VIRTADDR, 1, NULL, OPT_BASE_VIRTADDR_NUM }, {OPT_CREATE_UIO_DEV, 0, NULL, OPT_CREATE_UIO_DEV_NUM }, + {OPT_DEV, 1, NULL, OPT_DEV_NUM }, {OPT_FILE_PREFIX, 1, NULL, OPT_FILE_PREFIX_NUM }, {OPT_HELP, 0, NULL, OPT_HELP_NUM }, {OPT_HUGE_DIR, 1, NULL, OPT_HUGE_DIR_NUM },
@@ -109,6 +110,7 @@ TAILQ_HEAD(device_option_list, device_option);struct device_option {TAILQ_ENTRY (device_option) next; +int new;enum rte_devtype type;char arg[]; };
@@ -121,7 +123,8 @@ static int mem_parsed;static int core_parsed;static int -eal_option_device_add (enum rte_devtype type,const char *optarg) +eal_option_device_add (enum rte_devtype type,const char *optarg, +int new) {struct device_option *devopt;size_t optlen;
@@ -135,6 +138,7 @@ eal_option_device_add(enum rte_devtype type, const char *optarg) } devopt->type = type; + devopt->new = new; ret =snprintf (devopt->arg, optlen,"%s" , optarg);if (ret <0 ) {RTE_LOG (ERR ,EAL ,"Unable to copy device option\n" );
@@ -154,7 +158,22 @@ eal_option_device_parse(void) TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {if (ret ==0 ) { - ret =rte_devargs_add (devopt->type, devopt->arg); +if (devopt->new) { +struct rte_devargs *da; + + da =calloc (1 ,sizeof (*da)); + ret =rte_devargs_parse (da, devopt->arg); +if (ret) { +free (da); + }else { + ret =rte_devargs_insert (da); +if (ret) +free (da); + } + }else { + ret =rte_devargs_add (devopt->type, + devopt->arg); + }if (ret)RTE_LOG (ERR ,EAL ,"Unable to parse device '%s'\n" , devopt->arg);
@@ -1038,7 +1057,7 @@ eal_parse_common_option(int opt, const char *optarg,if (w_used) goto bw_used;if (eal_option_device_add (RTE_DEVTYPE_BLACKLISTED_PCI , - optarg) <0 ) { + optarg,0 ) <0 ) {return -1 ; } b_used =1 ;
@@ -1048,7 +1067,7 @@ eal_parse_common_option(int opt, const char *optarg,if (b_used) goto bw_used;if (eal_option_device_add (RTE_DEVTYPE_WHITELISTED_PCI , - optarg) <0 ) { + optarg,0 ) <0 ) {return -1 ; } w_used =1 ;
@@ -1177,9 +1196,16 @@ eal_parse_common_option(int opt, const char *optarg, }break ; +case OPT_DEV_NUM : +if (eal_option_device_add (RTE_DEVTYPE_VIRTUAL , + optarg,1 ) <0 ) { +return -1 ; + } +break ; +case OPT_VDEV_NUM :if (eal_option_device_add (RTE_DEVTYPE_VIRTUAL , - optarg) <0 ) { + optarg,0 ) <0 ) {return -1 ; }break ;
lib/librte_eal/common/eal_options.h +2 -0
@@ -21,6 +21,8 @@ enum {OPT_BASE_VIRTADDR_NUM , #define OPT_CREATE_UIO_DEV "create-uio-dev"OPT_CREATE_UIO_DEV_NUM , +#define OPT_DEV "dev" +OPT_DEV_NUM , #define OPT_FILE_PREFIX "file-prefix"OPT_FILE_PREFIX_NUM , #define OPT_HUGE_DIR "huge-dir"
Signature · 1 line
2.18.0
v11 cover Device querying
In reply to Gaetan RivetThis patchset introduces a new EAL API for querying devices, filtered by arbitrary properties.
The following elements are introduced to this end:
* A new object, "rte_class", is used to describe the device class abstraction layer (eth, crypto, ...).
* Both rte_bus and rte_class now offer a way to
list their devices and filter the result
using locally defined properties.
* The rte_dev API now has an rte_dev_iterator, which is the way for the user to define the device filter and iterate upon the resulting set.
As an example, the "eth" device class is implemented.
Additionally, the device filters for
+ rte_bus_pci
+ rte_bus_vdev
+ rte_class_eth
are implemented and can be used with some
properties each, to show how to extend those.
Some example of filters:
"bus=pci/class=eth"
"bus=pci"
"class=eth"
"class=eth,name=net_ring0"
"bus=pci,id=00:00.0"
"bus=vdev,driver=net_ring"
---
v2:
* Reworked the dev_iterate callback to simplify
its implementation.
Now dev_iterate implementation do not need to learn about the intricacies of the rte_dev_iterator. The rte_dev_iterator is managed purely by the rte_dev_iterator_next function. Buses and classes then do not have to care about settings things right.
Additionally, dev_iterate implementations do not have to sanitize their dev string anymore, they are prepared by the rte_dev layer prior, which also reduces the number of dynamic allocations.
v3:
* Introduced central constructor priority list.
* Removed lightweight kvarg parsing utility,
using librte_kvargs instead.
* Reversed dependencies of librte_kvargs and
librte_eal.
* Fixed a few bugs.
* @Bruce: I have noted the request for meson support.
I will install it and attempt it once the bulk of the work is done.
v4:
* Fixed a few bugs, added relevant acks,
fixed some typos.
* Made each matching functions actually check for a proper list of accepted properties.
* rte_kvargs now includes rte_eal directly and keeps rte_log.
* added generic string comparison function to rte_kvargs, as some kind of comparison should probably be shared by many layers.
v5:
* Rebased on master
* Use strcspn instead of custom function.
* Introduce private generic rte_eth_dev iterator. This could be generalized to other iterators (port_next, owner_id-aware, etc).
* Attempted to support meson.build.
Got lost in the implicit variables declared when inversing dependencies between kvargs and EAL. Removed anything related to meson.
* Postponed genericization of work from device query to device declaration.
Much bigger than anticipated, will let this part get in first and iterate over it.
v6:
* Rebased on master
* Introduce RTE_PRIORITY_LAST, to explicitly set the lowest constructor priority.
* Fix copyright notice for eth_privage.* files.
v7:
* Rebased on master
* Fix rte_kvargs_strcmp return value.
* Fix layer parsing error
devstr "bus=pci/onemorelayer" now tells
that the additional layer is not recognized.
v8:
* Rebased on master
* Cleaned kvargs use: introduced
a new parser function, that simplifies using the library for DPDK devargs.
* Refactored devargs parsing in a single function within rte_devargs.
This function is useful both for rte_dev parsing its iterator, and for rte_devargs parsing --dev parameters (not yet implemented).
* A few small bugfixes.
v9:
* Rebased on master
* Fixed cyclic dependency kvargs <-> eal
* Fixed a few checkpatch issues
* Added dynamic help to testpmd for new "show device" command
* Added devargs processing stubs to
primary buses / classes
* Added new --dev generic device declaration option
v10:
* Support meson build
v11:
* Some cleanup, rewriting of comments
and commit logs.
* New fields in devargs and dev_it renamed.
Gaetan Rivet (25):
devargs: use rte-log functions
devargs: add non-variadic parsing function
kvargs: remove error logs
kvargs: build before EAL
kvargs: introduce a more flexible parsing function
eal: introduce dtor macros
eal: introduce device class abstraction
devargs: add function to parse device layers eal/dev: add device iterator interface eal/dev: implement device iteration initialization eal/dev: implement device iteration
kvargs: add generic string matching callback bus/pci: implement device iteration and comparison bus/pci: add device matching field id bus/vdev: implement device iteration bus/vdev: add device matching field driver
ethdev: add private generic device iterator
ethdev: register ether layer as a class
ethdev: add device matching field name app/testpmd: add show device command bus/pci: pre-process declarative PCI devargs bus/vdev: pre-process declarative vdev devargs bus/pci: process declarative PCI devargs
ethdev: process declarative eth devargs
eal: add generic dev parameter
app/test-pmd/cmdline.c | 54 +++++ doc/guides/testpmd_app_ug/testpmd_funcs.rst | 24 +++ drivers/bus/pci/Makefile | 3 +- drivers/bus/pci/bsd/pci.c | 5 + drivers/bus/pci/linux/pci.c | 5 + drivers/bus/pci/meson.build | 6 +- drivers/bus/pci/pci_common.c | 6 +- drivers/bus/pci/pci_params.c | 139 ++++++++++++ drivers/bus/pci/private.h | 54 +++++ drivers/bus/vdev/Makefile | 3 +- drivers/bus/vdev/meson.build | 5 +- drivers/bus/vdev/vdev.c | 20 +- drivers/bus/vdev/vdev_params.c | 112 ++++++++++ drivers/bus/vdev/vdev_private.h | 32 +++ drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- lib/Makefile | 5 +- lib/librte_eal/bsdapp/eal/Makefile | 2 + lib/librte_eal/common/Makefile | 2 +- lib/librte_eal/common/eal_common_class.c | 62 ++++++ lib/librte_eal/common/eal_common_dev.c | 227 +++++++++++++++++++- lib/librte_eal/common/eal_common_devargs.c | 197 +++++++++++++++-- lib/librte_eal/common/eal_common_options.c | 36 +++- lib/librte_eal/common/eal_options.h | 2 + lib/librte_eal/common/eal_private.h | 34 +++ lib/librte_eal/common/include/rte_bus.h | 1 + lib/librte_eal/common/include/rte_class.h | 132 ++++++++++++ lib/librte_eal/common/include/rte_common.h | 24 +++ lib/librte_eal/common/include/rte_dev.h | 97 +++++++++ lib/librte_eal/common/include/rte_devargs.h | 53 ++++- lib/librte_eal/common/meson.build | 2 + lib/librte_eal/linuxapp/eal/Makefile | 2 + lib/librte_eal/meson.build | 1 + lib/librte_eal/rte_eal_version.map | 5 + lib/librte_ethdev/Makefile | 4 +- lib/librte_ethdev/eth_private.c | 31 +++ lib/librte_ethdev/eth_private.h | 31 +++ lib/librte_ethdev/meson.build | 4 +- lib/librte_ethdev/rte_class_eth.c | 148 +++++++++++++ lib/librte_ethdev/rte_ethdev.c | 9 +- lib/librte_kvargs/Makefile | 2 +- lib/librte_kvargs/meson.build | 5 + lib/librte_kvargs/rte_kvargs.c | 57 +++-- lib/librte_kvargs/rte_kvargs.h | 58 +++++ lib/librte_kvargs/rte_kvargs_version.map | 8 + lib/meson.build | 8 +- 46 files changed, 1654 insertions(+), 67 deletions(-) create mode 100644 drivers/bus/pci/pci_params.c create mode 100644 drivers/bus/vdev/vdev_params.c create mode 100644 drivers/bus/vdev/vdev_private.h create mode 100644 lib/librte_eal/common/eal_common_class.c create mode 100644 lib/librte_eal/common/include/rte_class.h create mode 100644 lib/librte_ethdev/eth_private.c create mode 100644 lib/librte_ethdev/eth_private.h create mode 100644 lib/librte_ethdev/rte_class_eth.c
Signature · 1 line
2.18.0
PATCH v11 1/25 devargs: use rte-log functions
Use the standard EAL logging functions in rte_devargs.
--- lib/librte_eal/common/eal_common_devargs.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +4 -3
@@ -16,6 +16,7 @@#include <rte_compat.h> #include <rte_dev.h> #include <rte_devargs.h> +#include <rte_log.h> #include <rte_tailq.h> #include "eal_private.h"
@@ -96,7 +97,7 @@ rte_devargs_parse(struct rte_devargs *da, const char *format, ...) da->name[i] = devname[i]; i++;if (i == maxlen) { -fprintf (stderr,"WARNING: Parsing \"%s\": device name should be shorter than %zu\n" , +RTE_LOG (WARNING ,EAL ,"Parsing \"%s\": device name should be shorter than %zu\n" , dev, maxlen); da->name[i -1 ] ='\0' ;return -EINVAL ;
@@ -106,7 +107,7 @@ rte_devargs_parse(struct rte_devargs *da, const char *format, ...)if (bus ==NULL ) { bus =rte_bus_find_by_device_name (da->name);if (bus ==NULL ) { -fprintf (stderr,"ERROR: failed to parse device \"%s\"\n" , +RTE_LOG (ERR ,EAL ,"failed to parse device \"%s\"\n" , da->name);return -EFAULT ; }
@@ -118,7 +119,7 @@ rte_devargs_parse(struct rte_devargs *da, const char *format, ...)else da->args =strdup ("" );if (da->args ==NULL ) { -fprintf (stderr,"ERROR: not enough memory to parse arguments\n" ); +RTE_LOG (ERR ,EAL ,"not enough memory to parse arguments\n" );return -ENOMEM ; }return 0 ;
Signature · 1 line
2.18.0
PATCH v11 2/25 devargs: add non-variadic parsing function
In reply to Gaetan Rivetrte_devargs_parse becomes non-variadic,rte_devargs_parsef becomes the variadic version, to be used to compose device strings.
--- drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 4 +- lib/librte_eal/common/eal_common_devargs.c | 42 ++++++++++++++++----- lib/librte_eal/common/include/rte_devargs.h | 40 +++++++++++++++++++- lib/librte_eal/rte_eal_version.map | 1 + lib/librte_ethdev/rte_ethdev.c | 2 +- 7 files changed, 76 insertions(+), 17 deletions(-)
drivers/net/failsafe/failsafe_args.c +1 -1
@@ -63,7 +63,7 @@ fs_parse_device(struct sub_device *sdev, char *args) d = &sdev->devargs;DEBUG ("%s" , args); - ret =rte_devargs_parse (d,"%s" , args); + ret =rte_devargs_parse (d, args);if (ret) {DEBUG ("devargs parsing failed with code %d" , ret);return ret;
drivers/net/failsafe/failsafe_eal.c +1 -1
@@ -86,7 +86,7 @@ fs_bus_init(struct rte_eth_dev *dev)else snprintf (devstr,sizeof (devstr),"%s" , rte_eth_devices[pid].device->name); - ret =rte_devargs_parse (da,"%s" , devstr); + ret =rte_devargs_parse (da, devstr);if (ret) {ERROR ("Probed devargs parsing failed with code" " %d" , ret);
lib/librte_eal/common/eal_common_dev.c +2 -2
@@ -138,8 +138,8 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnif (da ==NULL )return -ENOMEM ; - ret =rte_devargs_parse (da,"%s:%s,%s" , - busname, devname, devargs); + ret =rte_devargs_parsef (da,"%s:%s,%s" , + busname, devname, devargs);if (ret) goto err_devarg;
lib/librte_eal/common/eal_common_devargs.c +32 -10
69 lines not on this page · raw message
lib/librte_eal/common/include/rte_devargs.h +38 -2
@@ -89,6 +89,42 @@ __rte_deprecatedint rte_eal_parse_devargs_str (const char *devargs_str,char **drvname,char **drvargs); +/** +* Parse a device string. +* +* Verify that a bus is capable of handling the device passed +* in argument. Store which bus will handle the device, its name +* and the eventual device parameters. +* +* The syntax is: +* +* bus:device_identifier,arg1=val1,arg2=val2 +* +* where "bus:" is the bus name followed by any character separator. +* The bus name is optional. If no bus name is specified, each bus +* will attempt to recognize the device identifier. The first one +* to succeed will be used. +* +* Examples: +* +* pci:0000:05.00.0,arg=val +* 05.00.0,arg=val +* vdev:net_ring0 +* +* @param da +* The devargs structure holding the device information. +* +* @param dev +* String describing a device. +* +* @return +* - 0 on success. +* - Negative errno on error. +*/ +__rte_experimental +int +rte_devargs_parse (struct rte_devargs *da,const char *dev); +/** * Parse a device string. *
@@ -124,8 +160,8 @@ int rte_eal_parse_devargs_str(const char *devargs_str,*/ __rte_experimental int -rte_devargs_parse (struct rte_devargs *da, -const char *format, ...) +rte_devargs_parsef (struct rte_devargs *da, +const char *format, ...) __attribute__((format (printf,2 ,0 )));/**
lib/librte_eal/rte_eal_version.map +1 -0
@@ -254,6 +254,7 @@ EXPERIMENTAL { rte_devargs_insert; rte_devargs_next; rte_devargs_parse; + rte_devargs_parsef; rte_devargs_remove; rte_devargs_type_count; rte_eal_cleanup;
lib/librte_ethdev/rte_ethdev.c +1 -1
@@ -658,7 +658,7 @@ rte_eth_dev_attach(const char *devargs, uint16_t *port_id) }/* parse devargs */ -if (rte_devargs_parse (&da,"%s" , devargs)) +if (rte_devargs_parse (&da, devargs)) goto err; ret =rte_eal_hotplug_add (da.bus->name, da.name, da.args);
Signature · 1 line
2.18.0
PATCH v11 3/25 kvargs: remove error logs
In reply to Gaetan RivetError logs in kvargs parsing should be better handled in components calling the library.
This library must be as lean as possible.
--- lib/librte_kvargs/rte_kvargs.c | 22 +++++----------------- 1 file changed, 5 insertions(+), 17 deletions(-)
lib/librte_kvargs/rte_kvargs.c +5 -17
@@ -6,7 +6,6 @@#include <string.h> #include <stdlib.h> -#include <rte_log.h> #include <rte_string_fns.h> #include "rte_kvargs.h"
@@ -28,29 +27,22 @@ rte_kvargs_tokenize(struct rte_kvargs *kvlist, const char *params)* to pass to rte_strsplit */ kvlist->str =strdup (params); -if (kvlist->str ==NULL ) { -RTE_LOG (ERR ,PMD ,"Cannot parse arguments: not enough memory\n" ); +if (kvlist->str ==NULL )return -1 ; - }/* browse each key/value pair and add it in kvlist */ str = kvlist->str;while ((str =strtok_r (str,RTE_KVARGS_PAIRS_DELIM , &ctx1)) !=NULL ) { i = kvlist->count; -if (i >=RTE_KVARGS_MAX ) { -RTE_LOG (ERR ,PMD ,"Cannot parse arguments: list full\n" ); +if (i >=RTE_KVARGS_MAX )return -1 ; - } kvlist->pairs[i].key =strtok_r (str,RTE_KVARGS_KV_DELIM , &ctx2); kvlist->pairs[i].value =strtok_r (NULL ,RTE_KVARGS_KV_DELIM , &ctx2); -if (kvlist->pairs[i].key ==NULL || kvlist->pairs[i].value ==NULL ) { -RTE_LOG (ERR ,PMD , -"Cannot parse arguments: wrong key or value\n" -"params=<%s>\n" , params); +if (kvlist->pairs[i].key ==NULL || + kvlist->pairs[i].value ==NULL )return -1 ; - } kvlist->count++; str =NULL ;
@@ -89,12 +81,8 @@ check_for_valid_keys(struct rte_kvargs *kvlist,for (i =0 ; i < kvlist->count; i++) { pair = &kvlist->pairs[i]; ret =is_valid_key (valid, pair->key); -if (!ret) { -RTE_LOG (ERR ,PMD , -"Error parsing device, invalid key <%s>\n" , - pair->key); +if (!ret)return -1 ; - } }return 0 ; }
Signature · 1 line
2.18.0
PATCH v11 4/25 kvargs: build before EAL
In reply to Gaetan RivetThis library will be used by the EAL to parse parameters.
--- lib/Makefile | 3 +-- lib/librte_eal/meson.build | 1 + lib/librte_kvargs/Makefile | 2 +- lib/librte_kvargs/meson.build | 3 +++ lib/meson.build | 8 ++++++-- 5 files changed, 12 insertions(+), 5 deletions(-)
lib/Makefile +1 -2
@@ -4,6 +4,7 @@ include $(RTE_SDK)/mk/rte.vars.mk DIRS-y += librte_compat +DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci DEPDIRS-librte_pci := librte_eal
@@ -76,8 +77,6 @@ DEPDIRS-librte_flow_classify := librte_net librte_table librte_acl DIRS-$(CONFIG_RTE_LIBRTE_SCHED) += librte_sched DEPDIRS-librte_sched := librte_eal librte_mempool librte_mbuf librte_net DEPDIRS-librte_sched += librte_timer -DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs -DEPDIRS-librte_kvargs := librte_eal DIRS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += librte_distributor DEPDIRS-librte_distributor := librte_eal librte_mbuf librte_ethdev DIRS-$(CONFIG_RTE_LIBRTE_PORT) += librte_port
lib/librte_eal/meson.build +1 -0
@@ -24,6 +24,7 @@ endif version = 7 # the version of the EAL API allow_experimental_apis = true deps += 'compat' +deps += 'kvargs' cflags += '-D_GNU_SOURCE' sources = common_sources + env_sources objs = common_objs + env_objs
lib/librte_kvargs/Makefile +1 -1
@@ -7,7 +7,7 @@ include $(RTE_SDK)/mk/rte.vars.mk LIB = librte_kvargs.a CFLAGS += $(WERROR_FLAGS) -I$(SRCDIR) -O3 -LDLIBS += -lrte_eal +CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include EXPORT_MAP := rte_kvargs_version.map
lib/librte_kvargs/meson.build +3 -0
@@ -1,6 +1,9 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2017 Intel Corporation +includes = [global_inc] +includes += include_directories('../../../lib/librte_eal/common/include') + version = 1 sources = files('rte_kvargs.c') headers = files('rte_kvargs.h')
lib/meson.build +6 -2
@@ -9,7 +9,8 @@ # given as a dep, no need to mention ring. This is especially true for the # core libs which are widely reused, so their deps are kept to a minimum. libraries = [ 'compat', # just a header, used for versioning - 'eal', 'ring', 'mempool', 'mbuf', 'net', 'kvargs', 'ethdev', 'pci', # core + 'kvargs', + 'eal', 'ring', 'mempool', 'mbuf', 'net', 'ethdev', 'pci', # core 'metrics', # bitrate/latency stats depends on this 'hash', # efd depends on this 'timer', # eventdev depends on this
@@ -41,9 +42,12 @@ foreach l:libraries # external package/library requirements ext_deps = [] deps = ['eal'] # eal is standard dependency except for itself - if l == 'eal' + if l == 'kvargs' deps = [] endif + if l == 'eal' + deps = ['kvargs'] + endif dir_name = 'librte_' + l subdir(dir_name)
Signature · 1 line
2.18.0
PATCH v11 5/25 kvargs: introduce a more flexible parsing function
In reply to Gaetan RivetThis function permits defining additional terminating characters, ending the parsing to arbitrary delimiters.
--- lib/Makefile | 1 + lib/librte_kvargs/meson.build | 2 ++ lib/librte_kvargs/rte_kvargs.c | 25 ++++++++++++++++++ lib/librte_kvargs/rte_kvargs.h | 32 ++++++++++++++++++++++++ lib/librte_kvargs/rte_kvargs_version.map | 7 ++++++ 5 files changed, 67 insertions(+)
lib/Makefile +1 -0
@@ -5,6 +5,7 @@ include $(RTE_SDK)/mk/rte.vars.mk DIRS-y += librte_compat DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs +DEPDIRS-librte_kvargs := librte_compat DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci DEPDIRS-librte_pci := librte_eal
lib/librte_kvargs/meson.build +2 -0
@@ -7,3 +7,5 @@ includes += include_directories('../../../lib/librte_eal/common/include') version = 1 sources = files('rte_kvargs.c') headers = files('rte_kvargs.h') + +deps += 'compat'
lib/librte_kvargs/rte_kvargs.c +25 -0
@@ -168,3 +168,28 @@ rte_kvargs_parse(const char *args, const char * const valid_keys[])return kvlist; } + +__rte_experimental +struct rte_kvargs * +rte_kvargs_parse_delim (const char *args,const char *const valid_keys[], +const char *valid_ends) +{ +struct rte_kvargs *kvlist =NULL ; +char *copy; +size_t len; + +if (valid_ends ==NULL ) +return rte_kvargs_parse (args, valid_keys); + + copy =strdup (args); +if (copy ==NULL ) +return NULL ; + + len =strcspn (copy, valid_ends); + copy[len] ='\0' ; + + kvlist =rte_kvargs_parse (copy, valid_keys); + +free (copy); +return kvlist; +}
lib/librte_kvargs/rte_kvargs.h +32 -0
@@ -25,6 +25,8 @@extern "C" {#endif +#include <rte_compat.h> +/** Maximum number of key/value associations */ #define RTE_KVARGS_MAX 32
@@ -71,6 +73,36 @@ struct rte_kvargs {struct rte_kvargs *rte_kvargs_parse (const char *args,const char *const valid_keys[]); +/** +* Allocate a rte_kvargs and store key/value associations from a string. +* This version will consider any byte from valid_ends as a possible +* terminating character, and will not parse beyond any of their occurrence. +* +* The function allocates and fills an rte_kvargs structure from a given +* string whose format is key1=value1,key2=value2,... +* +* The structure can be freed with rte_kvargs_free(). +* +* @param args +* The input string containing the key/value associations +* +* @param valid_keys +* A list of valid keys (table of const char *, the last must be NULL). +* This argument is ignored if NULL +* +* @param valid_ends +* Acceptable terminating characters. +* If NULL, the behavior is the same as ``rte_kvargs_parse``. +* +* @return +* - A pointer to an allocated rte_kvargs structure on success +* - NULL on error +*/ +__rte_experimental +struct rte_kvargs *rte_kvargs_parse_delim(const char *args, +const char *const valid_keys[], +const char *valid_ends); +/** * Free a rte_kvargs structure *
lib/librte_kvargs/rte_kvargs_version.map +7 -0
@@ -8,3 +8,10 @@ DPDK_2.0 { local: *; }; + +EXPERIMENTAL { + global: + + rte_kvargs_parse_delim; + +} DPDK_2.0;
Signature · 1 line
2.18.0
PATCH v11 6/25 eal: introduce dtor macros
In reply to Gaetan RivetThis macro adds symbols to the .fini section using the global RTE priorities, to ensure consistency.
--- lib/librte_eal/common/include/rte_common.h | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+)
lib/librte_eal/common/include/rte_common.h +23 -0
@@ -111,6 +111,29 @@ static void __attribute__((constructor(RTE_PRIO(prio)), used)) func(void)#define RTE_INIT (func) \ RTE_INIT_PRIO(func, LAST) +/** +* Run after main() with low priority. +* +* @param func +* Destructor function name. +* @param prio +* Priority number must be above 100. +* Lowest number is the last to run. +*/ +#define RTE_FINI_PRIO (func, prio) \ +static void __attribute__((destructor(RTE_PRIO(prio)), used)) func(void) + +/** +* Run after main() with high priority. +* +* The destructor will be run *before* prioritized destructors. +* +* @param func +* Destructor function name. +*/ +#define RTE_FINI (func) \ + RTE_FINI_PRIO(func, LAST) +/** * Force a function to be inlined */
Signature · 1 line
2.18.0
PATCH v11 7/25 eal: introduce device class abstraction
In reply to Gaetan RivetThis abstraction exists since the infancy of DPDK. It needs to be fleshed out however, to allow a generic description of devices properties and capabilities.
A device class is the northbound interface of the device, intended for applications to know what it can be used for.
It is conceptually just above buses.
--- lib/librte_eal/bsdapp/eal/Makefile | 1 + lib/librte_eal/common/Makefile | 2 +- lib/librte_eal/common/eal_common_class.c | 62 ++++++++++ lib/librte_eal/common/include/rte_class.h | 131 +++++++++++++++++++++ lib/librte_eal/common/include/rte_common.h | 1 + lib/librte_eal/common/meson.build | 2 + lib/librte_eal/linuxapp/eal/Makefile | 1 + lib/librte_eal/rte_eal_version.map | 2 + 8 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 lib/librte_eal/common/eal_common_class.c create mode 100644 lib/librte_eal/common/include/rte_class.h
lib/librte_eal/bsdapp/eal/Makefile +1 -0
@@ -52,6 +52,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_hypervisor.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_string_fns.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_hexdump.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_devargs.c +SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_class.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_bus.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_dev.c SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_options.c
lib/librte_eal/common/Makefile +1 -1
@@ -11,7 +11,7 @@ INC += rte_per_lcore.h rte_random.h INC += rte_tailq.h rte_interrupts.h rte_alarm.h INC += rte_string_fns.h rte_version.h INC += rte_eal_memconfig.h rte_malloc_heap.h -INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h +INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h rte_class.h INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h INC += rte_malloc.h rte_keepalive.h rte_time.h INC += rte_service.h rte_service_component.h
lib/librte_eal/common/eal_common_class.c new file +62 -0
62 lines not on this page · raw message
lib/librte_eal/common/include/rte_class.h new file +131 -0
131 lines not on this page · raw message
lib/librte_eal/common/include/rte_common.h +1 -0
@@ -83,6 +83,7 @@ typedef uint16_t unaligned_uint16_t;#define RTE_PRIORITY_LOG 101#define RTE_PRIORITY_BUS 110 +#define RTE_PRIORITY_CLASS 120#define RTE_PRIORITY_LAST 65535#define RTE_PRIO (prio) \
lib/librte_eal/common/meson.build +2 -0
@@ -8,6 +8,7 @@ common_objs = [] common_sources = files( 'eal_common_bus.c', 'eal_common_cpuflags.c', + 'eal_common_class.c', 'eal_common_devargs.c', 'eal_common_dev.c', 'eal_common_errno.c',
@@ -46,6 +47,7 @@ common_headers = files( 'include/rte_branch_prediction.h', 'include/rte_bus.h', 'include/rte_bitmap.h', + 'include/rte_class.h', 'include/rte_common.h', 'include/rte_debug.h', 'include/rte_devargs.h',
lib/librte_eal/linuxapp/eal/Makefile +1 -0
@@ -60,6 +60,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_hypervisor.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_string_fns.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_hexdump.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_devargs.c +SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_class.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_bus.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_dev.c SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_options.c
lib/librte_eal/rte_eal_version.map +2 -0
@@ -244,6 +244,8 @@ DPDK_18.05 { EXPERIMENTAL { global: + rte_class_register; + rte_class_unregister; rte_ctrl_thread_create; rte_dev_event_callback_register; rte_dev_event_callback_unregister;
Signature · 1 line
2.18.0
Re: [PATCH v11 07/25] eal: introduce device class abstraction
Quoted 11 lines · Gaetan Rivet
This abstraction exists since the infancy of DPDK. It needs to be fleshed out however, to allow a generic description of devices properties and capabilities.
A device class is the northbound interface of the device, intended for applications to know what it can be used for.
It is conceptually just above buses.
Signed-off-by Gaetan Rivet <[email protected]>---
[...]
lib/librte_eal/rte_eal_version.map DPDK_18.05 { · line 251
Quoted 12 lines
lib/librte_eal/rte_eal_version.map +2
-0@@ -244,6 +244,8 @@ DPDK_18.05 { EXPERIMENTAL { global: + rte_class_register; + rte_class_unregister; rte_ctrl_thread_create; rte_dev_event_callback_register; rte_dev_event_callback_unregister;
Any reason you don't want the rte_class_find and rte_class_find_by_name as exposed APIs? There is no experimental tag on these APIs either.
Re: [PATCH v11 07/25] eal: introduce device class abstraction
Quoted 15 lines · Shreyansh Jain
Quoted 11 lines · Gaetan Rivet
This abstraction exists since the infancy of DPDK. It needs to be fleshed out however, to allow a generic description of devices properties and capabilities.
A device class is the northbound interface of the device, intended for applications to know what it can be used for.
It is conceptually just above buses.
Signed-off-by Gaetan Rivet <[email protected]>---
[...]
Quoted 15 lines
lib/librte_eal/rte_eal_version.map DPDK_18.05 { · line 251
Quoted 11 lines
lib/librte_eal/rte_eal_version.map +2
-0@@ -244,6 +244,8 @@ DPDK_18.05 { EXPERIMENTAL { global: + rte_class_register; + rte_class_unregister; rte_ctrl_thread_create; rte_dev_event_callback_register; rte_dev_event_callback_unregister;Any reason you don't want the
rte_class_findandrte_class_find_by_nameas exposed APIs? There is no experimental tag on these APIs either.
No actually I just overlooked that part! Thanks for catching this, I think it should be exposed and tagged experimental.
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v11 07/25] eal: introduce device class abstraction
12/07/2018 09:41, Gaëtan Rivet:
Quoted 6 lines · Shreyansh Jain
Quoted 2 lines
Any reason you don't want the
rte_class_findandrte_class_find_by_nameas exposed APIs? There is no experimental tag on these APIs either.No actually I just overlooked that part! Thanks for catching this, I think it should be exposed and tagged experimental.
Fixup below:
lib/librte_eal/common/eal_common_class.c +2 -0
@@ -29,6 +29,7 @@ rte_class_unregister(struct rte_class *class)RTE_LOG (DEBUG ,EAL ,"Unregistered [%s] device class.\n" , class->name); } +__rte_experimental struct rte_class *rte_class_find (const struct rte_class *start,rte_class_cmp_t cmp,const void *data)
@@ -55,6 +56,7 @@ cmp_class_name(const struct rte_class *class, const void *_name)return strcmp (class->name, name); } +__rte_experimental struct rte_class *rte_class_find_by_name (const char *name) {
lib/librte_eal/common/include/rte_class.h +2 -0
@@ -76,6 +76,7 @@ typedef int (*rte_class_cmp_t)(const struct rte_class *cls, const void *data);* @return * A pointer to a rte_class structure or NULL in case no class matches */ +__rte_experimental struct rte_class *rte_class_find (const struct rte_class *start,rte_class_cmp_t cmp,const void *data);
@@ -83,6 +84,7 @@ rte_class_find(const struct rte_class *start, rte_class_cmp_t cmp,/** * Find the registered class for a given name. */ +__rte_experimental struct rte_class *rte_class_find_by_name (const char *name);
lib/librte_eal/rte_eal_version.map +2 -0
@@ -254,6 +254,8 @@ DPDK_18.08 { EXPERIMENTAL { global: + rte_class_find; + rte_class_find_by_name; rte_class_register; rte_class_unregister; rte_ctrl_thread_create;
Re: [PATCH v11 07/25] eal: introduce device class abstraction
In reply to Gaetan RivetHi,
Some doxygen parameters must be fixed.
See below:
11/07/2018 23:44, Gaetan Rivet:
Quoted 19 lines
lib/librte_eal/common/include/rte_class.h new file +19
-0+/** +* Register a Class handle. +* +* @param +* A pointer to a rte_class structure describing the class +* to be registered. +*/ +__rte_experimental +void rte_class_register(struct rte_class *cls); + +/** +* Unregister a Class handle. +* +* @param class +* A pointer to a rte_class structure describing the class +* to be unregistered. +*/ +__rte_experimental +void rte_class_unregister(struct rte_class *cls);
PATCH v11 8/25 devargs: add function to parse device layers
In reply to Gaetan RivetThis function is private to the EAL.
It is used to parse each layers in a device description string, and store the result in an rte_devargs structure.
--- lib/librte_eal/common/eal_common_devargs.c | 144 ++++++++++++++++++++ lib/librte_eal/common/eal_private.h | 34 +++++ lib/librte_eal/common/include/rte_devargs.h | 13 +- 3 files changed, 188 insertions(+), 3 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +144 -0
@@ -13,9 +13,13 @@#include <string.h> #include <stdarg.h> +#include <rte_bus.h> +#include <rte_class.h> #include <rte_compat.h> #include <rte_dev.h> #include <rte_devargs.h> +#include <rte_errno.h> +#include <rte_kvargs.h> #include <rte_log.h> #include <rte_tailq.h> #include "eal_private.h"
@@ -57,6 +61,146 @@ rte_eal_parse_devargs_str(const char *devargs_str,return 0 ; } +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; +} + +int +rte_devargs_layers_parse (struct rte_devargs *devargs, +const char *devstr) +{ +struct { +const char *key; +const char *str; +struct rte_kvargs *kvlist; + } layers[] = { + {"bus=" ,NULL ,NULL , }, + {"class=" ,NULL ,NULL , }, + {"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 ; +int ret =0 ; + +/* 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. +*/ +if (devargs->data != devstr) { + devargs->data =strdup (devstr); +if (devargs->data ==NULL ) { +RTE_LOG (ERR ,EAL ,"OOM\n" ); + ret = -ENOMEM ; + goto get_out; + } + s = devargs->data; + } + +while (s !=NULL ) { +if (strncmp (layers[i].key, s, +strlen (layers[i].key)) && +/* The last layer is free-form. +* The "driver" key is not required (but accepted). +*/ + 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 ) { +RTE_LOG (ERR ,EAL ,"Could not parse %s\n" , s); + ret = -EINVAL ; + goto get_out; + } + s =strchr (s,'/' ); +if (s !=NULL ) + s++; +next_layer: +if (i >=RTE_DIM (layers)) { +RTE_LOG (ERR ,EAL ,"Unrecognized layer %s\n" , s); + ret = -EINVAL ; + goto get_out; + } + i++; + } + +/* Parse each sub-list. */ +for (i =0 ; i <RTE_DIM (layers); i++) { +if (layers[i].kvlist ==NULL ) +continue ; + kv = &layers[i].kvlist->pairs[0 ]; +if (strcmp (kv->key,"bus" ) ==0 ) { + bus =rte_bus_find_by_name (kv->value); +if (bus ==NULL ) { +RTE_LOG (ERR ,EAL ,"Could not find bus \"%s\"\n" , + kv->value); + ret = -EFAULT ; + goto get_out; + } + }else if (strcmp (kv->key,"class" ) ==0 ) { + cls =rte_class_find_by_name (kv->value); +if (cls ==NULL ) { +RTE_LOG (ERR ,EAL ,"Could not find class \"%s\"\n" , + kv->value); + ret = -EFAULT ; + goto get_out; + } + }else if (strcmp (kv->key,"driver" ) ==0 ) { +/* Ignore */ +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 = (void *)(intptr_t )(devargs->data); + +while ((s =strchr (s,'/' ))) { + *s ='\0' ; + s++; + } + } + +get_out: +for (i =0 ; i <RTE_DIM (layers); i++) { +if (layers[i].kvlist) +rte_kvargs_free (layers[i].kvlist); + } +if (ret !=0 ) + rte_errno = -ret; +return ret; +} +static int bus_name_cmp (const struct rte_bus *bus,const void *name) {
lib/librte_eal/common/eal_private.h +34 -0
@@ -258,4 +258,38 @@ int rte_mp_channel_init(void);*/ void dev_callback_process (char *device_name,enum rte_dev_event_type event); +/** +* @internal +* Parse a device string and store its information in an +* rte_devargs structure. +* +* A device description is split by layers of abstraction of the device: +* bus, class and driver. Each layer will offer a set of properties that +* can be applied either to configure or recognize a device. +* +* This function will parse those properties and prepare the rte_devargs +* to be given to each layers for processing. +* +* Note: if the "data" field of the devargs points to devstr, +* then no dynamic allocation is performed and the rte_devargs +* can be safely discarded. +* +* Otherwise ``data`` will hold a workable copy of devstr, that will be +* used by layers descriptors within rte_devargs. In this case, +* any rte_devargs should be cleaned-up before being freed. +* +* @param da +* rte_devargs structure to fill. +* +* @param devstr +* Device string. +* +* @return +* 0 on success. +* Negative errno values on error (rte_errno is set). +*/ +int +rte_devargs_layers_parse (struct rte_devargs *devargs, +const char *devstr); +#endif /* _EAL_PRIVATE_H_ */
lib/librte_eal/common/include/rte_devargs.h +10 -3
@@ -51,12 +51,19 @@ struct rte_devargs {enum rte_devtype type;/** Device policy. */ enum rte_dev_policy policy; -/** Bus handle for the device. */ -struct rte_bus *bus;/** Name of the device. */ char name[RTE_DEV_NAME_MAX_LEN ]; +RTE_STD_C11 + union {/** Arguments string as given by user or "" for no argument. */ -char *args; +char *args; +const char *drv_str; + }; +struct rte_bus *bus;/**< bus handle. */ +struct rte_class *cls;/**< class handle. */ +const char *bus_str;/**< bus-related part of device string. */ +const char *cls_str;/**< class-related part of device string. */ +const char *data;/**< Device string storage. */ };/**
Signature · 1 line
2.18.0
Re: [PATCH v11 08/25] devargs: add function to parse device layers
Quoted 11 lines · Gaetan Rivet
This function is private to the EAL.
It is used to parse each layers in a device description string, and store the result in anrte_devargsstructure.Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_devargs.c | 144 ++++++++++++++++++++ lib/librte_eal/common/eal_private.h | 34 +++++ lib/librte_eal/common/include/rte_devargs.h | 13 +- 3 files changed, 188 insertions(+), 3 deletions(-)
[...]
Re: [PATCH v11 08/25] devargs: add function to parse device layers
In reply to Gaetan Rivet11/07/2018 23:44, Gaetan Rivet:
Quoted 5 lines
This function is private to the EAL.
It is used to parse each layers in a device description string, and store the result in anrte_devargsstructure.Signed-off-by Gaetan Rivet <[email protected]>
There is a compilation issue.
It makes EAL depends on kvargs.
This change is required (to be squashed):
lib/librte_eal/bsdapp/eal/Makefile +1 -0
@@ -18,6 +18,7 @@ CFLAGS += $(WERROR_FLAGS) -O3 LDLIBS += -lexecinfo LDLIBS += -lpthread LDLIBS += -lgcc_s +LDLIBS += -lrte_kvargs EXPORT_MAP := ../../rte_eal_version.map
lib/librte_eal/bsdapp/eal/meson.build +2 -0
@@ -16,3 +16,5 @@ env_sources = files('eal_alarm.c', 'eal_memory.c', 'eal_dev.c' ) + +deps += ['kvargs']
lib/librte_eal/linuxapp/eal/Makefile +1 -0
@@ -24,6 +24,7 @@ LDLIBS += -ldl LDLIBS += -lpthread LDLIBS += -lgcc_s LDLIBS += -lrt +LDLIBS += -lrte_kvargs ifeq ($(CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES),y) LDLIBS += -lnuma endif
lib/librte_eal/linuxapp/eal/meson.build +1 -0
@@ -23,6 +23,7 @@ env_sources = files('eal_alarm.c', 'eal_dev.c', ) +deps += ['kvargs'] if has_libnuma == 1 dpdk_conf.set10('RTE_EAL_NUMA_AWARE_HUGEPAGES', true) endif
PATCH v11 9/25 eal/dev: add device iterator interface
In reply to Gaetan RivetA device iterator allows iterating over a set of devices. This set is defined by the two descriptions offered,
* rte_bus
* rte_class
Only one description can be provided, or both. It is not allowed to provide no description at all.
Each layer of abstraction then performs a filter based on the description provided. This filtering allows iterating on their internal set of devices, stopping when a match is valid and returning the current iteration context.
This context allows starting the next iteration from the same point and going forward.
--- lib/librte_eal/common/include/rte_bus.h | 1 + lib/librte_eal/common/include/rte_class.h | 1 + lib/librte_eal/common/include/rte_dev.h | 47 +++++++++++++++++++++++ 3 files changed, 49 insertions(+)
lib/librte_eal/common/include/rte_bus.h +1 -0
@@ -211,6 +211,7 @@ struct rte_bus {rte_bus_parse_t parse;/**< Parse a device name */ struct rte_bus_conf conf;/**< Bus configuration */ rte_bus_get_iommu_class_t get_iommu_class;/**< Get iommu class */ +rte_dev_iterate_t dev_iterate;/**< Device iterator. */ };/**
lib/librte_eal/common/include/rte_class.h +1 -0
@@ -35,6 +35,7 @@ TAILQ_HEAD(rte_class_list, rte_class);struct rte_class {TAILQ_ENTRY (rte_class) next;/**< Next device class in linked list */ const char *name;/**< Name of the class */ +rte_dev_iterate_t dev_iterate;/**< Device iterator. */ };/**
lib/librte_eal/common/include/rte_dev.h +47 -0
53 lines not on this page · raw message
Signature · 1 line
2.18.0
PATCH v11 10/25 eal/dev: implement device iteration initialization
In reply to Gaetan RivetParse a device description.
Split this description in their relevant part for each layers. No dynamic allocation is performed.
--- lib/Makefile | 1 + lib/librte_eal/bsdapp/eal/Makefile | 1 + lib/librte_eal/common/eal_common_dev.c | 55 +++++++++++++++++++++++++ lib/librte_eal/common/include/rte_dev.h | 24 +++++++++++ lib/librte_eal/linuxapp/eal/Makefile | 1 + lib/librte_eal/rte_eal_version.map | 1 + 6 files changed, 83 insertions(+)
lib/Makefile +1 -0
@@ -7,6 +7,7 @@ DIRS-y += librte_compat DIRS-$(CONFIG_RTE_LIBRTE_KVARGS) += librte_kvargs DEPDIRS-librte_kvargs := librte_compat DIRS-$(CONFIG_RTE_LIBRTE_EAL) += librte_eal +DEPDIRS-librte_eal := librte_kvargs DIRS-$(CONFIG_RTE_LIBRTE_PCI) += librte_pci DEPDIRS-librte_pci := librte_eal DIRS-$(CONFIG_RTE_LIBRTE_RING) += librte_ring
lib/librte_eal/bsdapp/eal/Makefile +1 -0
@@ -18,6 +18,7 @@ CFLAGS += $(WERROR_FLAGS) -O3 LDLIBS += -lexecinfo LDLIBS += -lpthread LDLIBS += -lgcc_s +LDLIBS += -lrte_kvargs EXPORT_MAP := ../../rte_eal_version.map
lib/librte_eal/common/eal_common_dev.c +55 -0
67 lines not on this page · raw message
lib/librte_eal/common/include/rte_dev.h +24 -0
@@ -331,6 +331,30 @@ typedef void *(*rte_dev_iterate_t)(const void *start,const char *devstr, const struct rte_dev_iterator *it); +/** +* Initializes a device iterator. +* +* This iterator allows accessing a list of devices matching a criteria. +* The device matching is made among all buses and classes currently registered, +* filtered by the device description given as parameter. +* +* This function will not allocate any memory. It is safe to stop the +* iteration at any moment and let the iterator go out of context. +* +* @param it +* Device iterator handle. +* +* @param str +* Device description string. +* +* @return +* 0 on successful initialization. +* <0 on error. +*/ +__rte_experimental +int +rte_dev_iterator_init (struct rte_dev_iterator *it,const char *str); + #ifdef __cplusplus }#endif
lib/librte_eal/linuxapp/eal/Makefile +1 -0
@@ -27,6 +27,7 @@ LDLIBS += -lrt ifeq ($(CONFIG_RTE_EAL_NUMA_AWARE_HUGEPAGES),y) LDLIBS += -lnuma endif +LDLIBS += -lrte_kvargs # specific to linuxapp exec-env SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) := eal.c
lib/librte_eal/rte_eal_version.map +1 -0
@@ -251,6 +251,7 @@ EXPERIMENTAL { rte_dev_event_callback_unregister; rte_dev_event_monitor_start; rte_dev_event_monitor_stop; + rte_dev_iterator_init; rte_devargs_add; rte_devargs_dump; rte_devargs_insert;
Signature · 1 line
2.18.0
PATCH v11 11/25 eal/dev: implement device iteration
In reply to Gaetan RivetUse the iteration hooks in the abstraction layers to perform the requested filtering on the internal device lists.
--- lib/librte_eal/common/eal_common_dev.c | 168 ++++++++++++++++++++++++ lib/librte_eal/common/include/rte_dev.h | 26 ++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 195 insertions(+)
lib/librte_eal/common/eal_common_dev.c +168 -0
@@ -45,6 +45,28 @@ static struct dev_event_cb_list dev_event_cbs;/* spinlock for device callbacks */ static rte_spinlock_t dev_event_lock =RTE_SPINLOCK_INITIALIZER ; +struct dev_next_ctx { +struct rte_dev_iterator *it; +const char *bus_str; +const char *cls_str; +}; + +#define CTX (it, bus_str, cls_str) \ + (&(const struct dev_next_ctx){ \ + .it = it, \ + .bus_str = bus_str, \ + .cls_str = cls_str, \ + }) + +#define ITCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->it) + +#define BUSCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->bus_str) + +#define CLSCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->cls_str) +static int cmp_detached_dev_name (const struct rte_device *dev,const void *_name) {
@@ -398,3 +420,149 @@ rte_dev_iterator_init(struct rte_dev_iterator *it, get_out:return -rte_errno; } + +static char * +dev_str_sane_copy (const char *str) +{ +size_t end; +char *copy; + + end =strcspn (str,",/" ); +if (str[end] ==',' ) { + copy =strdup (&str[end +1 ]); + }else { +/* '/' or '\0' */ + copy =strdup ("" ); + } +if (copy ==NULL ) { + rte_errno =ENOMEM ; + }else { +char *slash; + + slash =strchr (copy,'/' ); +if (slash !=NULL ) + slash[0 ] ='\0' ; + } +return copy; +} + +static int +class_next_dev_cmp (const struct rte_class *cls, +const void *ctx) +{ +struct rte_dev_iterator *it; +const char *cls_str =NULL ; +void *dev; + +if (cls->dev_iterate ==NULL ) +return 1 ; + it =ITCTX (ctx); + cls_str =CLSCTX (ctx); + dev = it->class_device; +/* it->cls_str != NULL means a class +* was specified in the devstr. +*/ +if (it->cls_str !=NULL && cls != it->cls) +return 1 ; +/* If an error occurred previously, +* no need to test further. +*/ +if (rte_errno !=0 ) +return -1 ; + dev = cls->dev_iterate (dev, cls_str, it); + it->class_device = dev; +return dev ==NULL ; +} + +static int +bus_next_dev_cmp (const struct rte_bus *bus, +const void *ctx) +{ +struct rte_device *dev =NULL ; +struct rte_class *cls =NULL ; +struct rte_dev_iterator *it; +const char *bus_str =NULL ; + +if (bus->dev_iterate ==NULL ) +return 1 ; + it =ITCTX (ctx); + bus_str =BUSCTX (ctx); + dev = it->device; +/* it->bus_str != NULL means a bus +* was specified in the devstr. +*/ +if (it->bus_str !=NULL && bus != it->bus) +return 1 ; +/* If an error occurred previously, +* no need to test further. +*/ +if (rte_errno !=0 ) +return -1 ; +if (it->cls_str ==NULL ) { + dev = bus->dev_iterate (dev, bus_str, it); + goto end; + } +/* cls_str != NULL */ +if (dev ==NULL ) { +next_dev_on_bus: + dev = bus->dev_iterate (dev, bus_str, it); + it->device = dev; + } +if (dev ==NULL ) +return 1 ; +if (it->cls !=NULL ) + cls =TAILQ_PREV (it->cls, rte_class_list, next); + cls =rte_class_find (cls, class_next_dev_cmp, ctx); +if (cls !=NULL ) { + it->cls = cls; + goto end; + } + goto next_dev_on_bus; +end: + it->device = dev; +return dev ==NULL ; +} +__rte_experimental +struct rte_device * +rte_dev_iterator_next (struct rte_dev_iterator *it) +{ +struct rte_bus *bus =NULL ; +int old_errno = rte_errno; +char *bus_str =NULL ; +char *cls_str =NULL ; + + rte_errno =0 ; +if (it->bus_str ==NULL && it->cls_str ==NULL ) { +/* Invalid iterator. */ + rte_errno =EINVAL ; +return NULL ; + } +if (it->bus !=NULL ) + bus =TAILQ_PREV (it->bus, rte_bus_list, next); +if (it->bus_str !=NULL ) { + bus_str =dev_str_sane_copy (it->bus_str); +if (bus_str ==NULL ) + goto out; + } +if (it->cls_str !=NULL ) { + cls_str =dev_str_sane_copy (it->cls_str); +if (cls_str ==NULL ) + goto out; + } +while ((bus =rte_bus_find (bus, bus_next_dev_cmp, +CTX (it, bus_str, cls_str)))) { +if (it->device !=NULL ) { + it->bus = bus; + goto out; + } +if (it->bus_str !=NULL || + rte_errno !=0 ) +break ; + } +if (rte_errno ==0 ) + rte_errno = old_errno; +out: +free (bus_str); +free (cls_str); +return it->device; +}
lib/librte_eal/common/include/rte_dev.h +26 -0
@@ -355,6 +355,32 @@ __rte_experimentalint rte_dev_iterator_init (struct rte_dev_iterator *it,const char *str); +/** +* Iterates on a device iterator. +* +* Generates a new rte_device handle corresponding to the next element +* in the list described in comprehension by the iterator. +* +* The next object is returned, and the iterator is updated. +* +* @param it +* Device iterator handle. +* +* @return +* An rte_device handle if found. +* NULL if an error occurred (rte_errno is set). +* NULL if no device could be found (rte_errno is not set). +*/ +__rte_experimental +struct rte_device * +rte_dev_iterator_next (struct rte_dev_iterator *it); + +#define RTE_DEV_FOREACH (dev, devstr, it) \ + for (rte_dev_iterator_init(it, devstr), \ + dev = rte_dev_iterator_next(it); \ + dev != NULL; \ + dev = rte_dev_iterator_next(it)) + #ifdef __cplusplus }#endif
lib/librte_eal/rte_eal_version.map +1 -0
@@ -252,6 +252,7 @@ EXPERIMENTAL { rte_dev_event_monitor_start; rte_dev_event_monitor_stop; rte_dev_iterator_init; + rte_dev_iterator_next; rte_devargs_add; rte_devargs_dump; rte_devargs_insert;
Signature · 1 line
2.18.0
Re: [PATCH v11 11/25] eal/dev: implement device iteration
Quoted 51 lines · Gaetan Rivet
Use the iteration hooks in the abstraction layers to perform the requested filtering on the internal device lists.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_dev.c | 168 ++++++++++++++++++++++++ lib/librte_eal/common/include/rte_dev.h | 26 ++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 195 insertions(+)lib/librte_eal/common/eal_common_dev.c +26
-0@@ -45,6 +45,28 @@ static struct dev_event_cb_list dev_event_cbs;/* spinlock for device callbacks */ static rte_spinlock_t dev_event_lock =RTE_SPINLOCK_INITIALIZER ; +struct dev_next_ctx { +struct rte_dev_iterator *it; +const char *bus_str; +const char *cls_str; +}; + +#define CTX (it, bus_str, cls_str) \ + (&(const struct dev_next_ctx){ \ + .it = it, \ + .bus_str = bus_str, \ + .cls_str = cls_str, \ + }) + +#define ITCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->it) + +#define BUSCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->bus_str) + +#define CLSCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->cls_str) +static int cmp_detached_dev_name (const struct rte_device *dev,const void *_name) {@@ -398,3 +420,149 @@ rte_dev_iterator_init(struct rte_dev_iterator *it, get_out:return -rte_errno; } + +static char * +dev_str_sane_copy (const char *str) +{
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 436
Quoted 10 lines
lib/librte_eal/common/eal_common_dev.c +10
-0+size_t end; +char *copy; + + end =strcspn (str,",/" ); +if (str[end] ==',' ) { + copy =strdup (&str[end +1 ]); + }else { +/* '/' or '\0' */ + copy =strdup ("" ); + }
Though it doesn't change anything functionally, if you can separate blocks of if-else with new lines, it really makes it easier to read. Like here...
Quoted 25 lines
lib/librte_eal/common/eal_common_dev.c +25
-0+if (copy ==NULL ) { + rte_errno =ENOMEM ; + }else { +char *slash; + + slash =strchr (copy,'/' ); +if (slash !=NULL ) + slash[0 ] ='\0' ; + } +return copy; +} + +static int +class_next_dev_cmp (const struct rte_class *cls, +const void *ctx) +{ +struct rte_dev_iterator *it; +const char *cls_str =NULL ; +void *dev; + +if (cls->dev_iterate ==NULL ) +return 1 ; + it =ITCTX (ctx); + cls_str =CLSCTX (ctx); + dev = it->class_device;
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 471
Quoted 10 lines
lib/librte_eal/common/eal_common_dev.c +10
-0+/* it->cls_str != NULL means a class +* was specified in the devstr. +*/ +if (it->cls_str !=NULL && cls != it->cls) +return 1 ; +/* If an error occurred previously, +* no need to test further. +*/ +if (rte_errno !=0 ) +return -1 ;
I am guessing here by '..error occurred previously..' you mean sane_copy. If so, why wait until this point to return? Anyway the caller (rte_bus_find, probably) would only look for '0' or non-zero.
Quoted 22 lines
lib/librte_eal/common/eal_common_dev.c +22
-0+ dev = cls->dev_iterate (dev, cls_str, it); + it->class_device = dev; +return dev ==NULL ; +} + +static int +bus_next_dev_cmp (const struct rte_bus *bus, +const void *ctx) +{ +struct rte_device *dev =NULL ; +struct rte_class *cls =NULL ; +struct rte_dev_iterator *it; +const char *bus_str =NULL ; + +if (bus->dev_iterate ==NULL ) +return 1 ; + it =ITCTX (ctx); + bus_str =BUSCTX (ctx); + dev = it->device; +/* it->bus_str != NULL means a bus +* was specified in the devstr. +*/
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 503
Quoted 10 lines
lib/librte_eal/common/eal_common_dev.c +10
-0+if (it->bus_str !=NULL && bus != it->bus) +return 1 ; +/* If an error occurred previously, +* no need to test further. +*/ +if (rte_errno !=0 ) +return -1 ; +if (it->cls_str ==NULL ) { + dev = bus->dev_iterate (dev, bus_str, it); + goto end;
This is slightly confusing. If it->cls_str == NULL, you do bus->dev_iterate... but
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 508
Quoted 5 lines
lib/librte_eal/common/eal_common_dev.c +5
-0+ } +/* cls_str != NULL */ +if (dev ==NULL ) { +next_dev_on_bus: + dev = bus->dev_iterate (dev, bus_str, it);
When cls_str!=NULL, you still do bus->dev_iterate...
So, maybe they are OR case resulting in check of dev==NULL and return (as being done right now by jumping to out)...?
And, how can bus iterate over a 'null' device?
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 512
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c +4
-0+ it->device = dev; + } +if (dev ==NULL ) +return 1 ;
Maybe, this check should move in the if(dev==NULL) above - that way, it can in path of 'next_dev_on_bus' yet do the same as function as its current location.
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 513
Quoted 1 line
lib/librte_eal/common/eal_common_dev.c +1
-0+if (it->cls !=NULL )
In what case would (it->cls_str == NULL) but (it->cls != NULL)?
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 520
Quoted 7 lines
lib/librte_eal/common/eal_common_dev.c +7
-0+ cls =TAILQ_PREV (it->cls, rte_class_list, next); + cls =rte_class_find (cls, class_next_dev_cmp, ctx); +if (cls !=NULL ) { + it->cls = cls; + goto end; + } + goto next_dev_on_bus;
Maybe I have completely mixed your intention of this function. So, if you still find the above comments naive - maybe you can tell me what you are attempting here? Is it: find next bus and stop if no class specified. find next class as well, iff that too was specified?
Reason I am confused is that bus_next_dev_cmp attempts to compare both - bus and class, yet class_next_dev_cmp simply stops by comparing class only.
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 524
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c +4
-0+end: + it->device = dev; +return dev ==NULL ; +}
A new line should be added here - start of a new function.
Quoted 94 lines
lib/librte_eal/common/eal_common_dev.c +44
-0+__rte_experimental +struct rte_device * +rte_dev_iterator_next (struct rte_dev_iterator *it) +{ +struct rte_bus *bus =NULL ; +int old_errno = rte_errno; +char *bus_str =NULL ; +char *cls_str =NULL ; + + rte_errno =0 ; +if (it->bus_str ==NULL && it->cls_str ==NULL ) { +/* Invalid iterator. */ + rte_errno =EINVAL ; +return NULL ; + } +if (it->bus !=NULL ) + bus =TAILQ_PREV (it->bus, rte_bus_list, next); +if (it->bus_str !=NULL ) { + bus_str =dev_str_sane_copy (it->bus_str); +if (bus_str ==NULL ) + goto out; + } +if (it->cls_str !=NULL ) { + cls_str =dev_str_sane_copy (it->cls_str); +if (cls_str ==NULL ) + goto out; + } +while ((bus =rte_bus_find (bus, bus_next_dev_cmp, +CTX (it, bus_str, cls_str)))) { +if (it->device !=NULL ) { + it->bus = bus; + goto out; + } +if (it->bus_str !=NULL || + rte_errno !=0 ) +break ; + } +if (rte_errno ==0 ) + rte_errno = old_errno; +out: +free (bus_str); +free (cls_str); +return it->device; +}lib/librte_eal/common/include/rte_dev.h +26
-0@@ -355,6 +355,32 @@ __rte_experimentalint rte_dev_iterator_init (struct rte_dev_iterator *it,const char *str); +/** +* Iterates on a device iterator. +* +* Generates a new rte_device handle corresponding to the next element +* in the list described in comprehension by the iterator. +* +* The next object is returned, and the iterator is updated. +* +* @param it +* Device iterator handle. +* +* @return +* An rte_device handle if found. +* NULL if an error occurred (rte_errno is set). +* NULL if no device could be found (rte_errno is not set). +*/ +__rte_experimental +struct rte_device * +rte_dev_iterator_next (struct rte_dev_iterator *it); + +#define RTE_DEV_FOREACH (dev, devstr, it) \ + for (rte_dev_iterator_init(it, devstr), \ + dev = rte_dev_iterator_next(it); \ + dev != NULL; \ + dev = rte_dev_iterator_next(it)) + #ifdef __cplusplus }#endif lib/librte_eal/rte_eal_version.map +1
-0@@ -252,6 +252,7 @@ EXPERIMENTAL { rte_dev_event_monitor_start; rte_dev_event_monitor_stop; rte_dev_iterator_init; + rte_dev_iterator_next; rte_devargs_add; rte_devargs_dump; rte_devargs_insert;
Re: [PATCH v11 11/25] eal/dev: implement device iteration
Hi Shreyansh,
Quoted 57 lines · Shreyansh Jain
Quoted 56 lines · Gaetan Rivet
Use the iteration hooks in the abstraction layers to perform the requested filtering on the internal device lists.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_dev.c | 168 ++++++++++++++++++++++++ lib/librte_eal/common/include/rte_dev.h | 26 ++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 195 insertions(+)lib/librte_eal/common/eal_common_dev.c +32
-0@@ -45,6 +45,28 @@ static struct dev_event_cb_list dev_event_cbs;/* spinlock for device callbacks */ static rte_spinlock_t dev_event_lock =RTE_SPINLOCK_INITIALIZER ; +struct dev_next_ctx { +struct rte_dev_iterator *it; +const char *bus_str; +const char *cls_str; +}; + +#define CTX (it, bus_str, cls_str) \ + (&(const struct dev_next_ctx){ \ + .it = it, \ + .bus_str = bus_str, \ + .cls_str = cls_str, \ + }) + +#define ITCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->it) + +#define BUSCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->bus_str) + +#define CLSCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->cls_str) +static int cmp_detached_dev_name (const struct rte_device *dev,const void *_name) {@@ -398,3 +420,149 @@ rte_dev_iterator_init(struct rte_dev_iterator *it, get_out:return -rte_errno; } + +static char * +dev_str_sane_copy (const char *str) +{ +size_t end; +char *copy; + + end =strcspn (str,",/" ); +if (str[end] ==',' ) { + copy =strdup (&str[end +1 ]);
Quoted 9 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 436
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c +4
-0+ }else { +/* '/' or '\0' */ + copy =strdup ("" ); + }Though it doesn't change anything functionally, if you can separate blocks of if-else with new lines, it really makes it easier to read. Like here...
sure,
Quoted 31 lines
Quoted 31 lines
lib/librte_eal/common/eal_common_dev.c +31
-0+if (copy ==NULL ) { + rte_errno =ENOMEM ; + }else { +char *slash; + + slash =strchr (copy,'/' ); +if (slash !=NULL ) + slash[0 ] ='\0' ; + } +return copy; +} + +static int +class_next_dev_cmp (const struct rte_class *cls, +const void *ctx) +{ +struct rte_dev_iterator *it; +const char *cls_str =NULL ; +void *dev; + +if (cls->dev_iterate ==NULL ) +return 1 ; + it =ITCTX (ctx); + cls_str =CLSCTX (ctx); + dev = it->class_device; +/* it->cls_str != NULL means a class +* was specified in the devstr. +*/ +if (it->cls_str !=NULL && cls != it->cls) +return 1 ; +/* If an error occurred previously,
Quoted 9 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 471
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c +4
-0+* no need to test further. +*/ +if (rte_errno !=0 ) +return -1 ;I am guessing here by '..error occurred previously..' you mean
sane_copy. If so, why wait until this point to return? Anyway the caller (rte_bus_find, probably) would only look for '0' or non-zero.
No, rte_errno could be set by a bus / class implementation, for any error occurring during a call to dev_iterate: maybe a device was lost (hotplugged), etc. The return value of dev_iterate() cannot transmit an error as not matching a filter is not an error. The only error channel is rte_errno.
sane_copy was already checked before and should be cleared at this point.
Quoted 36 lines
Quoted 22 lines
lib/librte_eal/common/eal_common_dev.c +22
-0+ dev = cls->dev_iterate (dev, cls_str, it); + it->class_device = dev; +return dev ==NULL ; +} + +static int +bus_next_dev_cmp (const struct rte_bus *bus, +const void *ctx) +{ +struct rte_device *dev =NULL ; +struct rte_class *cls =NULL ; +struct rte_dev_iterator *it; +const char *bus_str =NULL ; + +if (bus->dev_iterate ==NULL ) +return 1 ; + it =ITCTX (ctx); + bus_str =BUSCTX (ctx); + dev = it->device; +/* it->bus_str != NULL means a bus +* was specified in the devstr. +*/ lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 503
Quoted 10 lines
lib/librte_eal/common/eal_common_dev.c +10
-0+if (it->bus_str !=NULL && bus != it->bus) +return 1 ; +/* If an error occurred previously, +* no need to test further. +*/ +if (rte_errno !=0 ) +return -1 ; +if (it->cls_str ==NULL ) { + dev = bus->dev_iterate (dev, bus_str, it); + goto end;This is slightly confusing. If
it->cls_str== NULL, you dobus->dev_iterate... but
Quoted 10 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 508
Quoted 5 lines
lib/librte_eal/common/eal_common_dev.c +5
-0+ } +/* cls_str != NULL */ +if (dev ==NULL ) { +next_dev_on_bus: + dev = bus->dev_iterate (dev, bus_str, it);When cls_str!=NULL, you still do
bus->dev_iterate...
So, maybe they are OR case resulting in check of dev==NULL and return (as being done right now by jumping to out)...?
Yes, this iteration is pretty complex.
The best way to walk through it is to define the possible cases:
1. Iterating on one bus:
if (bus_str != NULL && cls_str == NULL)
Simplest case. You got one bus, no classes.
Just call the current bus dev_iterate() callback, then report the result (whether NULL or not).
2. Iterating on one bus and one class:
if (bus_str != NULL && cls_str != NULL)
Possible states are:
a. We are starting the iteration: dev is NULL.
Iterate on the current bus.
If device is not NULL anymore, iterate on the class. To ensure we stay on the same class, set cls to the previous class and call rte_class_find();
If device is still NULL, return 1 to iterate on the next bus.
b. We are in the middle of an iteration: dev is not NULL.
We start iterating on the class to find a possible second instance of the same device in the class (e.g. mlx devices with multiple eth ports per PCI devices). If none is found, we
come back to bus->dev_iterate() (bypassing the dev == NULL check), restarting this (b) cycle as many times as necessary. If the result is NULL, the iteration is finished.
3. Iterating on one class:
if (bus_str == NULL && cls_str != NULL)
The most complex case. Same as (2), however we start by the first bus, and if a device is NULL we will continue onto the next bus within the loop line 554 (within rte_dev_iterator_next).
The core of the complexity here lies in the next_dev_on_bus cycle described in 2.b.
Quoted 2 lines
And, how can bus iterate over a 'null' device?
A NULL device means that we are starting the iteration: a bus will give its first device.
Quoted 9 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 512
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c +4
-0+ it->device = dev; + } +if (dev ==NULL ) +return 1 ;Maybe, this check should move in the if(dev==NULL) above - that way, it can in path of '
next_dev_on_bus' yet do the same as function as its current location.
Yes
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 513
Quoted 1 line
lib/librte_eal/common/eal_common_dev.c +1
-0+if (it->cls !=NULL )In what case would (
it->cls_str== NULL) but (it->cls!= NULL)?
When one rte_device was used to spawn several class_devices: multiple adapters per PCI addresses for example.
However, in this case, given that the matching would be useless, we skip the class matching process and only return the rte_device. This single rte_device is not returned multiple times.
However, if someone was giving:
bus=pci,id=00:02.0/class=eth
(str_sane_copy would set cls_str to "" here)
Then, if 00:02.0 had spawned several eth ports, it would be returned once for each instance.
This is a pretty ambiguous case. I'm not sure of the best way to deal with it. My decision here was to simplify the iteration if possible, as I considered that someone that did not care for the class properties would not care about counting the instances of the bus device. maybe I'm wrong.
Quoted 14 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 520
Quoted 7 lines
lib/librte_eal/common/eal_common_dev.c +7
-0+ cls =TAILQ_PREV (it->cls, rte_class_list, next); + cls =rte_class_find (cls, class_next_dev_cmp, ctx); +if (cls !=NULL ) { + it->cls = cls; + goto end; + } + goto next_dev_on_bus;Maybe I have completely mixed your intention of this function. So, if you still find the above comments naive - maybe you can tell me what you are attempting here?
Is it: find next bus and stop if no class specified. find next class as well, iff that too was specified?
Quoted 3 lines
Reason I am confused is that
bus_next_dev_cmpattempts to compare both - bus and class, yetclass_next_dev_cmpsimply stops by comparing class only.
You are right: bus comparator will produce an iteration on the bus devices, then on the class devices iff class is specified.
Thus the class comparator only has to iterate on class, because we called it from the bus iterator.
Quoted 7 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 524
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c +4
-0+end: + it->device = dev; +return dev ==NULL ; +}A new line should be added here - start of a new function.
yes
I'm pretty sorry about this code, to be honest. This is a nasty piece with a lot of states to care for.
At least it works. I'd like to have the properties integrated so that developpers can add their own quickly. In the meantime I could rework this function. But simple is not easy.
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v11 11/25] eal/dev: implement device iteration
(I am not reducing the thread as it contains quite interesting discussion - so, you might have to fish for inline comments...)
Quoted 204 lines · Gaëtan Rivet
Hi Shreyansh,
Quoted 57 lines · Shreyansh Jain
Quoted 56 lines · Gaetan Rivet
Use the iteration hooks in the abstraction layers to perform the requested filtering on the internal device lists.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_dev.c | 168 ++++++++++++++++++++++++ lib/librte_eal/common/include/rte_dev.h | 26 ++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 195 insertions(+)lib/librte_eal/common/eal_common_dev.c +32
-0@@ -45,6 +45,28 @@ static struct dev_event_cb_list dev_event_cbs;/* spinlock for device callbacks */ static rte_spinlock_t dev_event_lock =RTE_SPINLOCK_INITIALIZER ; +struct dev_next_ctx { +struct rte_dev_iterator *it; +const char *bus_str; +const char *cls_str; +}; + +#define CTX (it, bus_str, cls_str) \ + (&(const struct dev_next_ctx){ \ + .it = it, \ + .bus_str = bus_str, \ + .cls_str = cls_str, \ + }) + +#define ITCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->it) + +#define BUSCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->bus_str) + +#define CLSCTX (ptr) \ + (((struct dev_next_ctx *)(intptr_t)ptr)->cls_str) +static int cmp_detached_dev_name (const struct rte_device *dev,const void *_name) {@@ -398,3 +420,149 @@ rte_dev_iterator_init(struct rte_dev_iterator *it, get_out:return -rte_errno; } + +static char * +dev_str_sane_copy (const char *str) +{ +size_t end; +char *copy; + + end =strcspn (str,",/" ); +if (str[end] ==',' ) { + copy =strdup (&str[end +1 ]);Quoted 9 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 436
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c +4
-0+ }else { +/* '/' or '\0' */ + copy =strdup ("" ); + }Though it doesn't change anything functionally, if you can separate blocks of if-else with new lines, it really makes it easier to read. Like here...
sure,
Quoted 31 lines
Quoted 31 lines
lib/librte_eal/common/eal_common_dev.c +31
-0+if (copy ==NULL ) { + rte_errno =ENOMEM ; + }else { +char *slash; + + slash =strchr (copy,'/' ); +if (slash !=NULL ) + slash[0 ] ='\0' ; + } +return copy; +} + +static int +class_next_dev_cmp (const struct rte_class *cls, +const void *ctx) +{ +struct rte_dev_iterator *it; +const char *cls_str =NULL ; +void *dev; + +if (cls->dev_iterate ==NULL ) +return 1 ; + it =ITCTX (ctx); + cls_str =CLSCTX (ctx); + dev = it->class_device; +/* it->cls_str != NULL means a class +* was specified in the devstr. +*/ +if (it->cls_str !=NULL && cls != it->cls) +return 1 ; +/* If an error occurred previously, Quoted 9 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 471
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c +4
-0+* no need to test further. +*/ +if (rte_errno !=0 ) +return -1 ;I am guessing here by '..error occurred previously..' you mean
sane_copy. If so, why wait until this point to return? Anyway the caller (rte_bus_find, probably) would only look for '0' or non-zero.No,
rte_errnocould be set by a bus / class implementation, for any error occurring during a call todev_iterate: maybe a device was lost (hotplugged), etc. The return value ofdev_iterate()cannot transmit an error as not matching a filter is not an error. The only error channel isrte_errno.
sane_copywas already checked before and should be cleared at this point.Quoted 36 lines
Quoted 22 lines
lib/librte_eal/common/eal_common_dev.c +22
-0+ dev = cls->dev_iterate (dev, cls_str, it); + it->class_device = dev; +return dev ==NULL ; +} + +static int +bus_next_dev_cmp (const struct rte_bus *bus, +const void *ctx) +{ +struct rte_device *dev =NULL ; +struct rte_class *cls =NULL ; +struct rte_dev_iterator *it; +const char *bus_str =NULL ; + +if (bus->dev_iterate ==NULL ) +return 1 ; + it =ITCTX (ctx); + bus_str =BUSCTX (ctx); + dev = it->device; +/* it->bus_str != NULL means a bus +* was specified in the devstr. +*/ lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 503
Quoted 10 lines
lib/librte_eal/common/eal_common_dev.c +10
-0+if (it->bus_str !=NULL && bus != it->bus) +return 1 ; +/* If an error occurred previously, +* no need to test further. +*/ +if (rte_errno !=0 ) +return -1 ; +if (it->cls_str ==NULL ) { + dev = bus->dev_iterate (dev, bus_str, it); + goto end;This is slightly confusing. If
it->cls_str== NULL, you dobus->dev_iterate... butQuoted 10 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 508
Quoted 5 lines
lib/librte_eal/common/eal_common_dev.c +5
-0+ } +/* cls_str != NULL */ +if (dev ==NULL ) { +next_dev_on_bus: + dev = bus->dev_iterate (dev, bus_str, it);When cls_str!=NULL, you still do
bus->dev_iterate...
So, maybe they are OR case resulting in check of dev==NULL and return (as being done right now by jumping to out)...?Yes, this iteration is pretty complex.
The best way to walk through it is to define the possible cases:
1. Iterating on one bus:
if (
bus_str!= NULL &&cls_str== NULL)Simplest case. You got one bus, no classes.
Just call the current busdev_iterate()callback, then report the result (whether NULL or not).2. Iterating on one bus and one class:
if (
bus_str!= NULL &&cls_str!= NULL)Possible states are:
a. We are starting the iteration: dev is NULL.
Iterate on the current bus.If device is not NULL anymore, iterate on the class. To ensure we stay on the same class, set cls to the previous class and call
rte_class_find();If device is still NULL, return 1 to iterate on the next bus.
b. We are in the middle of an iteration: dev is not NULL.
We start iterating on the class to find a possible second instance of the same device in the class (e.g. mlx devices with multiple eth ports per PCI devices). If none is found, we
come back tobus->dev_iterate()(bypassing the dev == NULL check), restarting this (b) cycle as many times as necessary. If the result is NULL, the iteration is finished.
Quoted 8 lines
3. Iterating on one class:
if (
bus_str== NULL &&cls_str!= NULL)The most complex case. Same as (2), however we start by the first bus, and if a device is NULL we will continue onto the next bus within the loop line 554 (within
rte_dev_iterator_next).
This, above, is most useful. I had missed out the case (b) above entirely. I will re-review with this in mind. Thanks for writing this.
Quoted 39 lines
The core of the complexity here lies in the
next_dev_on_buscycle described in 2.b.Quoted 2 lines
And, how can bus iterate over a 'null' device?
A NULL device means that we are starting the iteration: a bus will give its first device.
Quoted 9 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 512
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c +4
-0+ it->device = dev; + } +if (dev ==NULL ) +return 1 ;Maybe, this check should move in the if(dev==NULL) above - that way, it can in path of '
next_dev_on_bus' yet do the same as function as its current location.Yes
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 513
Quoted 1 line
lib/librte_eal/common/eal_common_dev.c +1
-0+if (it->cls !=NULL )In what case would (
it->cls_str== NULL) but (it->cls!= NULL)?When one
rte_devicewas used to spawn severalclass_devices: multiple adapters per PCI addresses for example.However, in this case, given that the matching would be useless, we skip the class matching process and only return the
rte_device. This singlerte_deviceis not returned multiple times.However, if someone was giving:
bus=pci,id=00:02.0/class=eth
(str_sane_copywould setcls_strto "" here)
Quoted 8 lines
Then, if 00:02.0 had spawned several eth ports, it would be returned once for each instance.
This is a pretty ambiguous case. I'm not sure of the best way to deal with it. My decision here was to simplify the iteration if possible, as I considered that someone that did not care for the class properties would not care about counting the instances of the bus device. maybe I'm wrong.
Frankly, I have nothing extra to add as a use-case. For your approach of 'simple is better': +1
Quoted 33 lines
Quoted 15 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 520
Quoted 8 lines
lib/librte_eal/common/eal_common_dev.c +7
-0+ cls =TAILQ_PREV (it->cls, rte_class_list, next); + cls =rte_class_find (cls, class_next_dev_cmp, ctx); +if (cls !=NULL ) { + it->cls = cls; + goto end; + } + goto next_dev_on_bus;Maybe I have completely mixed your intention of this function. So, if you still find the above comments naive - maybe you can tell me what you are attempting here?
Is it: find next bus and stop if no class specified. find next class as well, iff that too was specified?Quoted 3 lines
Reason I am confused is that
bus_next_dev_cmpattempts to compare both - bus and class, yetclass_next_dev_cmpsimply stops by comparing class only.You are right: bus comparator will produce an iteration on the bus devices, then on the class devices iff class is specified.
Thus the class comparator only has to iterate on class, because we called it from the bus iterator.
Quoted 7 lines
lib/librte_eal/common/eal_common_dev.c rte_dev_iterator_init(struct rte_dev_iterator *it, · line 524
Quoted 4 lines
lib/librte_eal/common/eal_common_dev.c +4
-0+end: + it->device = dev; +return dev ==NULL ; +}A new line should be added here - start of a new function.
Quoted 5 lines
yes
I'm pretty sorry about this code, to be honest. This is a nasty piece with a lot of states to care for.
Actually, the cases that you have mentioned above indeed requires a complex logic. I am not sure I have any suggestion to make it simpler. I will read again based on what you have explained.
Quoted 5 lines
At least it works. I'd like to have the properties integrated so that developpers can add their own quickly. In the meantime I could rework this function. But simple is not easy.
Problem is that the 18.08 window is almost closed - I though I could review it within the window but now I am not confident. Maybe someone else too can look through the PCI/VDEV part after this patch..
PATCH v11 12/25 kvargs: add generic string matching callback
In reply to Gaetan RivetThis function can be used as a callback torte_kvargs_process.
This should reduce code duplication.
--- lib/librte_kvargs/rte_kvargs.c | 10 +++++++++ lib/librte_kvargs/rte_kvargs.h | 26 ++++++++++++++++++++++++ lib/librte_kvargs/rte_kvargs_version.map | 1 + 3 files changed, 37 insertions(+)
lib/librte_kvargs/rte_kvargs.c +10 -0
@@ -193,3 +193,13 @@ rte_kvargs_parse_delim(const char *args, const char * const valid_keys[],free (copy);return kvlist; } + +__rte_experimental +int +rte_kvargs_strcmp(const char *key __rte_unused, +const char *value,void *opaque) +{ +const char *str = opaque; + +return -abs (strcmp (str, value)); +}
lib/librte_kvargs/rte_kvargs.h +26 -0
@@ -153,6 +153,32 @@ int rte_kvargs_process(const struct rte_kvargs *kvlist,unsigned rte_kvargs_count (const struct rte_kvargs *kvlist,const char *key_match); +/** +* Generic kvarg handler for string comparison. +* +* This function can be used for a generic string comparison processing +* on a list of kvargs. +* +* @param key +* kvarg pair key. +* +* @param value +* kvarg pair value. +* +* @param opaque +* Opaque pointer to a string. +* +* @return +* 0 if the strings match. +* !0 otherwise or on error. +* +* Unless strcmp, comparison ordering is not kept. +* In order for rte_kvargs_process to stop processing on match error, +* a negative value is returned even if strcmp had returned a positive one. +*/ +__rte_experimental +int rte_kvargs_strcmp(const char *key, const char *value, void *opaque); + #ifdef __cplusplus }#endif
lib/librte_kvargs/rte_kvargs_version.map +1 -0
@@ -13,5 +13,6 @@ EXPERIMENTAL { global: rte_kvargs_parse_delim; + rte_kvargs_strcmp; } DPDK_2.0;
Signature · 1 line
2.18.0
PATCH v11 13/25 bus/pci: implement device iteration and comparison
In reply to Gaetan Rivet--- drivers/bus/pci/Makefile | 3 +- drivers/bus/pci/meson.build | 6 +++- drivers/bus/pci/pci_common.c | 3 +- drivers/bus/pci/pci_params.c | 53 ++++++++++++++++++++++++++++++++++++ drivers/bus/pci/private.h | 25 +++++++++++++++++ 5 files changed, 86 insertions(+), 4 deletions(-) create mode 100644 drivers/bus/pci/pci_params.c
drivers/bus/pci/Makefile +2 -1
@@ -26,10 +26,11 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/$(SYSTEM)app/eal CFLAGS += -DALLOW_EXPERIMENTAL_API LDLIBS += -lrte_eal -lrte_mbuf -lrte_mempool -lrte_ring -LDLIBS += -lrte_ethdev -lrte_pci +LDLIBS += -lrte_ethdev -lrte_pci -lrte_kvargs include $(RTE_SDK)/drivers/bus/pci/$(SYSTEM)/Makefile SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) := $(addprefix $(SYSTEM)/,$(SRCS)) +SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_params.c SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_common.c SRCS-$(CONFIG_RTE_LIBRTE_PCI_BUS) += pci_common_uio.c
drivers/bus/pci/meson.build +5 -1
@@ -3,7 +3,9 @@ deps += ['pci'] install_headers('rte_bus_pci.h') -sources = files('pci_common.c', 'pci_common_uio.c') +sources = files('pci_common.c', + 'pci_common_uio.c', + 'pci_params.c') if host_machine.system() == 'linux' sources += files('linux/pci.c', 'linux/pci_uio.c',
@@ -17,3 +19,5 @@ endif # memseg walk is not part of stable API yet allow_experimental_apis = true + +deps += ['kvargs']
drivers/bus/pci/pci_common.c +1 -2
@@ -29,8 +29,6 @@static void rte_pci_remove_device (struct rte_pci_device *pci_device); -extern struct rte_pci_bus rte_pci_bus; -#define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"const char *rte_pci_get_sysfs_path (void )
@@ -437,6 +435,7 @@ struct rte_pci_bus rte_pci_bus = { .unplug = pci_unplug, .parse = pci_parse, .get_iommu_class = rte_pci_get_iommu_class, + .dev_iterate = rte_pci_dev_iterate, }, .device_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.device_list), .driver_list = TAILQ_HEAD_INITIALIZER(rte_pci_bus.driver_list),
drivers/bus/pci/pci_params.c new file +53 -0
53 lines not on this page · raw message
drivers/bus/pci/private.h +25 -0
@@ -10,6 +10,8 @@#include <rte_pci.h> #include <rte_bus_pci.h> +extern struct rte_pci_bus rte_pci_bus; +struct rte_pci_driver ;struct rte_pci_device ;
@@ -166,4 +168,27 @@ rte_pci_match(const struct rte_pci_driver *pci_drv,enum rte_iova_mode rte_pci_get_iommu_class (void ); +/* +* Iterate over internal devices, +* matching any device against the provided +* string. +* +* @param start +* Iteration starting point. +* +* @param str +* Device string to match against. +* +* @param it +* (unused) iterator structure. +* +* @return +* A pointer to the next matching device if any. +* NULL otherwise. +*/ +void * +rte_pci_dev_iterate (const void *start, +const char *str, +const struct rte_dev_iterator *it); +#endif /* _PCI_PRIVATE_H_ */
Signature · 1 line
2.18.0
PATCH v11 14/25 bus/pci: add device matching field id
In reply to Gaetan RivetThe PCI bus can now parse a matching field "id" as follows:
"bus=pci,id=0000:00:00.0"
or
"bus=pci,id=00:00.0"
--- drivers/bus/pci/pci_params.c | 29 +++++++++++++++++++++++++++-- 1 file changed, 27 insertions(+), 2 deletions(-)
drivers/bus/pci/pci_params.c +27 -2
@@ -3,6 +3,7 @@*/ #include <rte_bus.h> +#include <rte_bus_pci.h> #include <rte_dev.h> #include <rte_errno.h> #include <rte_kvargs.h>
@@ -11,21 +12,45 @@#include "private.h" enum pci_params { +RTE_PCI_PARAMS_ID ,RTE_PCI_PARAMS_MAX , };static const char *const pci_params_keys[] = { + [RTE_PCI_PARAMS_ID ] ="id" , [RTE_PCI_PARAMS_MAX ] =NULL , }; +static int +pci_addr_kv_cmp(const char *key __rte_unused, +const char *value, +void *_addr2) +{ +struct rte_pci_addr _addr1; +struct rte_pci_addr *addr1 = &_addr1; +struct rte_pci_addr *addr2 = _addr2; + +if (rte_pci_addr_parse (value, addr1)) +return -1 ; +return -abs (rte_pci_addr_cmp (addr1, addr2)); +} +static int pci_dev_match (const struct rte_device *dev,const void *_kvlist) {const struct rte_kvargs *kvlist = _kvlist; +const struct rte_pci_device *pdev; - (void ) dev; - (void ) kvlist; +if (kvlist ==NULL ) +/* Empty string matches everything. */ +return 0 ; + pdev =RTE_DEV_TO_PCI_CONST (dev); +/* if any field does not match. */ +if (rte_kvargs_process (kvlist,"id" , + &pci_addr_kv_cmp, + (void *)(intptr_t )&pdev->addr)) +return 1 ;return 0 ; }
Signature · 1 line
2.18.0
PATCH v11 15/25 bus/vdev: implement device iteration
In reply to Gaetan Rivet--- drivers/bus/vdev/Makefile | 3 +- drivers/bus/vdev/meson.build | 5 +++- drivers/bus/vdev/vdev.c | 10 ++++--- drivers/bus/vdev/vdev_params.c | 51 +++++++++++++++++++++++++++++++++ drivers/bus/vdev/vdev_private.h | 26 +++++++++++++++++ 5 files changed, 89 insertions(+), 6 deletions(-) create mode 100644 drivers/bus/vdev/vdev_params.c create mode 100644 drivers/bus/vdev/vdev_private.h
drivers/bus/vdev/Makefile +2 -1
@@ -19,8 +19,9 @@ EXPORT_MAP := rte_bus_vdev_version.map LIBABIVER := 1 SRCS-y += vdev.c +SRCS-y += vdev_params.c -LDLIBS += -lrte_eal +LDLIBS += -lrte_eal -lrte_kvargs # # Export include files
drivers/bus/vdev/meson.build +4 -1
@@ -1,7 +1,10 @@ # SPDX-License-Identifier: BSD-3-Clause # Copyright(c) 2017 Intel Corporation -sources = files('vdev.c') +sources = files('vdev.c', + 'vdev_params.c') install_headers('rte_bus_vdev.h') allow_experimental_apis = true + +deps += ['kvargs']
drivers/bus/vdev/vdev.c +6 -4
@@ -23,6 +23,7 @@#include "rte_bus_vdev.h" #include "vdev_logs.h" +#include "vdev_private.h" #define VDEV_MP_KEY "bus_vdev_mp"
@@ -493,9 +494,9 @@ vdev_probe(void)return ret; } -static struct rte_device * -vdev_find_device (const struct rte_device *start,rte_dev_cmp_t cmp, -const void *data) +struct rte_device * +rte_vdev_find_device (const struct rte_device *start,rte_dev_cmp_t cmp, +const void *data) {const struct rte_vdev_device *vstart;struct rte_vdev_device *dev;
@@ -532,10 +533,11 @@ vdev_unplug(struct rte_device *dev)static struct rte_bus rte_vdev_bus = { .scan = vdev_scan, .probe = vdev_probe, - .find_device = vdev_find_device, + .find_device = rte_vdev_find_device, .plug = vdev_plug, .unplug = vdev_unplug, .parse = vdev_parse, + .dev_iterate = rte_vdev_dev_iterate, };RTE_REGISTER_BUS (vdev, rte_vdev_bus);
drivers/bus/vdev/vdev_params.c new file +51 -0
51 lines not on this page · raw message
drivers/bus/vdev/vdev_private.h new file +26 -0
@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: BSD-3-Clause +* Copyright 2018 Gaëtan Rivet +*/ + +#ifndef _VDEV_PRIVATE_H_ +#define _VDEV_PRIVATE_H_ + +#ifdef __cplusplus +extern "C" { +#endif + +struct rte_device * +rte_vdev_find_device (const struct rte_device *start, +rte_dev_cmp_t cmp, +const void *data); + +void * +rte_vdev_dev_iterate (const void *start, +const char *str, +const struct rte_dev_iterator *it); + +#ifdef __cplusplus +} +#endif + +#endif /* _VDEV_PRIVATE_H_ */
Signature · 1 line
2.18.0
PATCH v11 16/25 bus/vdev: add device matching field driver
In reply to Gaetan RivetThe vdev bus parses a field "driver", matching
a vdev driver name with one passed as follows:
"bus=vdev,driver=xxxx"
--- drivers/bus/vdev/vdev_params.c | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-)
drivers/bus/vdev/vdev_params.c +13 -2
@@ -4,6 +4,7 @@#include <rte_dev.h> #include <rte_bus.h> +#include <rte_bus_vdev.h> #include <rte_kvargs.h> #include <rte_errno.h>
@@ -11,10 +12,12 @@#include "vdev_private.h" enum vdev_params { +RTE_VDEV_PARAMS_DRIVER ,RTE_VDEV_PARAMS_MAX , };static const char *const vdev_params_keys[] = { + [RTE_VDEV_PARAMS_DRIVER ] ="driver" , [RTE_VDEV_PARAMS_MAX ] =NULL , };
@@ -23,9 +26,17 @@ vdev_dev_match(const struct rte_device *dev, const void *_kvlist) {const struct rte_kvargs *kvlist = _kvlist; +const struct rte_vdev_device *vdev; - (void ) kvlist; - (void ) dev; +if (kvlist ==NULL ) +/* Empty string matches everything. */ +return 0 ; + vdev =RTE_DEV_TO_VDEV_CONST (dev); +/* if any field does not match. */ +if (rte_kvargs_process (kvlist,"driver" , + rte_kvargs_strcmp, + (void *)(intptr_t )vdev->device.driver->name)) +return -1 ;return 0 ; }
Signature · 1 line
2.18.0
PATCH v11 17/25 ethdev: add private generic device iterator
In reply to Gaetan RivetThis iterator can be customized with a comparison function that will trigger a stopping condition.
It can be leveraged to write several different iterators that have similar but non-identical purposes.
It is private to librte_ethdev.
--- lib/librte_ethdev/Makefile | 1 + lib/librte_ethdev/eth_private.c | 31 +++++++++++++++++++++++++++++++ lib/librte_ethdev/eth_private.h | 26 ++++++++++++++++++++++++++ lib/librte_ethdev/meson.build | 3 ++- 4 files changed, 60 insertions(+), 1 deletion(-) create mode 100644 lib/librte_ethdev/eth_private.c create mode 100644 lib/librte_ethdev/eth_private.h
lib/librte_ethdev/Makefile +1 -0
@@ -18,6 +18,7 @@ EXPORT_MAP := rte_ethdev_version.map LIBABIVER := 9 +SRCS-y += eth_private.c SRCS-y += rte_ethdev.c SRCS-y += rte_flow.c SRCS-y += rte_tm.c
lib/librte_ethdev/eth_private.c new file +31 -0
@@ -0,0 +1,31 @@ +/* SPDX-License-Identifier: BSD-3-Clause +* Copyright(c) 2018 Gaëtan Rivet +*/ + +#include "rte_ethdev.h" +#include "eth_private.h" + +struct rte_eth_dev * +eth_find_device (const struct rte_eth_dev *start,rte_eth_cmp_t cmp, +const void *data) +{ +struct rte_eth_dev *edev; +ptrdiff_t idx; + +/* Avoid Undefined Behaviour */ +if (start !=NULL && + (start < &rte_eth_devices[0 ] || + start > &rte_eth_devices[RTE_MAX_ETHPORTS ])) +return NULL ; +if (start !=NULL ) + idx = start - &rte_eth_devices[0 ] +1 ; +else + idx =0 ; +for (; idx <RTE_MAX_ETHPORTS ; idx++) { + edev = &rte_eth_devices[idx]; +if (cmp (edev, data) ==0 ) +return edev; + } +return NULL ; +} +
lib/librte_ethdev/eth_private.h new file +26 -0
@@ -0,0 +1,26 @@ +/* SPDX-License-Identifier: BSD-3-Clause +* Copyright(c) 2018 Gaëtan Rivet +*/ + +#ifndef _RTE_ETH_PRIVATE_H_ +#define _RTE_ETH_PRIVATE_H_ + +#include "rte_ethdev.h" + +#ifdef __cplusplus +extern "C" { +#endif + +/* Generic rte_eth_dev comparison function. */ +typedef int (*rte_eth_cmp_t )(const struct rte_eth_dev *,const void *); + +/* Generic rte_eth_dev iterator. */ +struct rte_eth_dev * +eth_find_device (const struct rte_eth_dev *_start,rte_eth_cmp_t cmp, +const void *data); + +#ifdef __cplusplus +} +#endif + +#endif /* _RTE_ETH_PRIVATE_H_ */
lib/librte_ethdev/meson.build +2 -1
@@ -4,7 +4,8 @@ name = 'ethdev' version = 9 allow_experimental_apis = true -sources = files('ethdev_profile.c', +sources = files('eth_private.c', + 'ethdev_profile.c', 'rte_ethdev.c', 'rte_flow.c', 'rte_mtr.c',
Signature · 1 line
2.18.0
PATCH v11 18/25 ethdev: register ether layer as a class
In reply to Gaetan Rivet--- lib/librte_ethdev/Makefile | 3 +- lib/librte_ethdev/meson.build | 1 + lib/librte_ethdev/rte_class_eth.c | 79 +++++++++++++++++++++++++++++++ 3 files changed, 82 insertions(+), 1 deletion(-) create mode 100644 lib/librte_ethdev/rte_class_eth.c
lib/librte_ethdev/Makefile +2 -1
@@ -12,7 +12,7 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API CFLAGS += -O3 CFLAGS += $(WERROR_FLAGS) LDLIBS += -lrte_net -lrte_eal -lrte_mempool -lrte_ring -LDLIBS += -lrte_mbuf +LDLIBS += -lrte_mbuf -lrte_kvargs EXPORT_MAP := rte_ethdev_version.map
@@ -20,6 +20,7 @@ LIBABIVER := 9 SRCS-y += eth_private.c SRCS-y += rte_ethdev.c +SRCS-y += rte_class_eth.c SRCS-y += rte_flow.c SRCS-y += rte_tm.c SRCS-y += rte_mtr.c
lib/librte_ethdev/meson.build +1 -0
@@ -6,6 +6,7 @@ version = 9 allow_experimental_apis = true sources = files('eth_private.c', 'ethdev_profile.c', + 'rte_class_eth.c', 'rte_ethdev.c', 'rte_flow.c', 'rte_mtr.c',
lib/librte_ethdev/rte_class_eth.c new file +79 -0
79 lines not on this page · raw message
Signature · 1 line
2.18.0
PATCH v11 19/25 ethdev: add device matching field name
In reply to Gaetan RivetThe eth device class can now parse a field name,
matching the eth_dev name with one passed as
"class=eth,name=xxxxxx"
--- lib/librte_ethdev/rte_class_eth.c | 7 +++++++ 1 file changed, 7 insertions(+)
lib/librte_ethdev/rte_class_eth.c +7 -0
@@ -15,10 +15,12 @@#include "eth_private.h" enum eth_params { +RTE_ETH_PARAMS_NAME ,RTE_ETH_PARAMS_MAX , };static const char *const eth_params_keys[] = { + [RTE_ETH_PARAMS_NAME ] ="name" , [RTE_ETH_PARAMS_MAX ] =NULL , };
@@ -39,6 +41,7 @@ eth_dev_match(const struct rte_eth_dev *edev, {const struct eth_dev_match_arg *arg = _arg;const struct rte_kvargs *kvlist = arg->kvlist; +struct rte_eth_dev_data *data;if (edev->state ==RTE_ETH_DEV_UNUSED )return -1 ;
@@ -47,6 +50,10 @@ eth_dev_match(const struct rte_eth_dev *edev,if (kvlist ==NULL )/* Empty string matches everything. */ return 0 ; + data = edev->data; +if (rte_kvargs_process (kvlist,"name" , + rte_kvargs_strcmp, data->name)) +return -1 ;return 0 ; }
Signature · 1 line
2.18.0
PATCH v11 20/25 app/testpmd: add show device command
In reply to Gaetan RivetA new interactive command is offered:
show device <device description>
This commands lists all rte_device element matching the device description. e.g.:
show device bus=pci
show device bus=vdev
show device bus=vdev/class=eth
show device bus=vdev,driver=net_ring/class=eth
show device bus=vdev/class=eth,name=net_ring0
These devices may not be otherwise useful, some buses will spawn devices to keep track of their assets without having a driver to use them.
--- app/test-pmd/cmdline.c | 54 +++++++++++++++++++++ doc/guides/testpmd_app_ug/testpmd_funcs.rst | 24 +++++++++ 2 files changed, 78 insertions(+)
app/test-pmd/cmdline.c +54 -0
@@ -228,6 +228,9 @@ static void cmd_help_long_parsed(void *parsed_result,"show port tm node stats (port_id) (node_id) (clear)\n" " Display the port TM node stats.\n\n" +"show device (device_string)\n" +" Display devices matching the device string.\n\n" + ); }
@@ -7093,6 +7096,56 @@ cmdline_parse_inst_t cmd_showportall = { }, }; +/* *** SHOW DEVICE INFO *** */ +struct cmd_showdevice_result { +cmdline_fixed_string_t show; +cmdline_fixed_string_t device; +cmdline_fixed_string_t filter; +}; + +static void +cmd_showdevice_dump_device (const struct rte_device *dev) +{ +const struct rte_driver *drv = dev->driver; + +printf ("0x%p: %s:%s\n" , (const void *)dev, dev->name, + drv ? drv->name :"<nil>" ); +} + +static void cmd_showdevice_parsed (void *parsed_result, + __attribute__((unused))struct cmdline *cl, + __attribute__((unused))void *data) +{ +struct cmd_showdevice_result *res = parsed_result; +struct rte_dev_iterator it; +const struct rte_device *dev; + + RTE_DEV_FOREACH(dev, res->filter, &it) +cmd_showdevice_dump_device (dev); +} + +cmdline_parse_token_string_t cmd_showdevice_show = + TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, + show,"show" ); +cmdline_parse_token_string_t cmd_showdevice_device = + TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, + device,"device" ); +cmdline_parse_token_string_t cmd_showdevice_filter = + TOKEN_STRING_INITIALIZER(struct cmd_showdevice_result, + filter,NULL ); + +cmdline_parse_inst_t cmd_showdevice = { + .f = cmd_showdevice_parsed, + .data =NULL , + .help_str ="show device <device string>" , + .tokens = { + (void *)&cmd_showdevice_show, + (void *)&cmd_showdevice_device, + (void *)&cmd_showdevice_filter, +NULL , + }, +}; +/* *** SHOW PORT INFO *** */ struct cmd_showport_result {cmdline_fixed_string_t show;
@@ -17272,6 +17325,7 @@ cmdline_parse_ctx_t main_ctx[] = { (cmdline_parse_inst_t *)&cmd_help_long, (cmdline_parse_inst_t *)&cmd_quit, (cmdline_parse_inst_t *)&cmd_load_from_file, + (cmdline_parse_inst_t *)&cmd_showdevice, (cmdline_parse_inst_t *)&cmd_showport, (cmdline_parse_inst_t *)&cmd_showqueue, (cmdline_parse_inst_t *)&cmd_showportall,
doc/guides/testpmd_app_ug/testpmd_funcs.rst +24 -0
@@ -2628,6 +2628,30 @@ set the traffic management default hierarchy on the port:: testpmd> set port tm hierarchy default (port_id) +Device functions +---------------- + +Show devices +~~~~~~~~~~~~ + +Display any registered devices:: + + testpmd> show device <device_string> + +where: + +* ``device_string``: Device description string, of the format + + layer[/layer[/layer]] + + where one layer is in the form + + layer_key=layer_name[,key1=value1[,...]] + + Valid layer keys are ``bus`` and ``class``. + Their respective values is defined by registered ``bus`` and ``class`` + drivers. + Filter Functions ----------------
Signature · 1 line
2.18.0
PATCH v11 21/25 bus/pci: pre-process declarative PCI devargs
In reply to Gaetan RivetThe new devargs format does not recognize a particular device name. Each bus uses its specific format.
Instead of introducing a new bus API, process those devargs privately for the moment. Prepare them for matching during scan against the bus devices.
--- drivers/bus/pci/bsd/pci.c | 5 ++++ drivers/bus/pci/linux/pci.c | 5 ++++ drivers/bus/pci/pci_params.c | 51 ++++++++++++++++++++++++++++++++++++ drivers/bus/pci/private.h | 16 +++++++++++ 4 files changed, 77 insertions(+)
drivers/bus/pci/bsd/pci.c +5 -0
@@ -327,6 +327,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)int rte_pci_scan (void ) { +struct rte_devargs *devargs;int fd;unsigned dev_count =0 ;struct pci_conf matches[16 ];
@@ -342,6 +343,10 @@ rte_pci_scan(void)if (!rte_eal_has_pci ())return 0 ; + RTE_EAL_DEVARGS_FOREACH("pci", devargs) +if (rte_pci_devargs_prepare (devargs)) +continue ; + fd =open ("/dev/pci" ,O_RDONLY );if (fd <0 ) {RTE_LOG (ERR ,EAL ,"%s(): error opening /dev/pci\n" , __func__);
drivers/bus/pci/linux/pci.c +5 -0
@@ -430,6 +430,7 @@ parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)int rte_pci_scan (void ) { +struct rte_devargs *devargs;struct dirent *e;DIR *dir;char dirname[PATH_MAX ];
@@ -439,6 +440,10 @@ rte_pci_scan(void)if (!rte_eal_has_pci ())return 0 ; + RTE_EAL_DEVARGS_FOREACH("pci", devargs) +if (rte_pci_devargs_prepare (devargs)) +continue ; +#ifdef VFIO_PRESENT if (!pci_vfio_is_enabled ())RTE_LOG (DEBUG ,EAL ,"VFIO PCI modules not loaded\n" );
drivers/bus/pci/pci_params.c +51 -0
63 lines not on this page · raw message
drivers/bus/pci/private.h +16 -0
@@ -191,4 +191,20 @@ rte_pci_dev_iterate(const void *start,const char *str, const struct rte_dev_iterator *it); +/* +* Prepare a devargs meant for this bus. +* This function is only used for a transitory period, +* to translate the new devargs format in one +* compatible with the old form. +* +* @param da +* Devargs to process. +* +* @return +* 0 on success. +* <0 on error. +*/ +int +rte_pci_devargs_prepare (struct rte_devargs *da); +#endif /* _PCI_PRIVATE_H_ */
Signature · 1 line
2.18.0
PATCH v11 22/25 bus/vdev: pre-process declarative vdev devargs
In reply to Gaetan RivetThe new devargs format does not recognize a particular device name. Each bus uses its specific format.
Process each devargs privately prior to attempting a bus scan. Prepare them if they are using the new devargs format.
--- drivers/bus/vdev/vdev.c | 10 ++++--- drivers/bus/vdev/vdev_params.c | 50 +++++++++++++++++++++++++++++++++ drivers/bus/vdev/vdev_private.h | 6 ++++ 3 files changed, 62 insertions(+), 4 deletions(-)
drivers/bus/vdev/vdev.c +6 -4
@@ -115,8 +115,8 @@ rte_vdev_remove_custom_scan(rte_vdev_scan_callback callback, void *user_arg)return 0 ; } -static int -vdev_parse (const char *name,void *addr) +int +rte_vdev_parse (const char *name,void *addr) {struct rte_vdev_driver **out = addr;struct rte_vdev_driver *driver =NULL ;
@@ -148,7 +148,7 @@ vdev_probe_all_drivers(struct rte_vdev_device *dev)VDEV_LOG (DEBUG ,"Search driver %s to probe device %s" , name,rte_vdev_device_name (dev)); -if (vdev_parse (name, &driver)) +if (rte_vdev_parse (name, &driver))return -1 ; dev->device.driver = &driver->driver; ret = driver->probe (dev);
@@ -443,6 +443,8 @@ vdev_scan(void)/* for virtual devices we scan the devargs_list populated via cmdline */ RTE_EAL_DEVARGS_FOREACH("vdev", devargs) { +if (rte_vdev_devargs_prepare (devargs)) +continue ; dev =calloc (1 ,sizeof (*dev));if (!dev)
@@ -536,7 +538,7 @@ static struct rte_bus rte_vdev_bus = { .find_device = rte_vdev_find_device, .plug = vdev_plug, .unplug = vdev_unplug, - .parse = vdev_parse, + .parse = rte_vdev_parse, .dev_iterate = rte_vdev_dev_iterate, };
drivers/bus/vdev/vdev_params.c +50 -0
64 lines not on this page · raw message
drivers/bus/vdev/vdev_private.h +6 -0
@@ -19,6 +19,12 @@ rte_vdev_dev_iterate(const void *start,const char *str, const struct rte_dev_iterator *it); +int +rte_vdev_parse (const char *name,void *addr); + +int +rte_vdev_devargs_prepare (struct rte_devargs *da); + #ifdef __cplusplus }#endif
Signature · 1 line
2.18.0
PATCH v11 23/25 bus/pci: process declarative PCI devargs
In reply to Gaetan RivetIntroduce the facility to process future PCI parameters.
Once the matching between PCI devices and devargs has been done, it is possible to process each devargs. New parameters would have the PCI device handle to work with when parsing the device (bus specific) parameters.
--- drivers/bus/pci/pci_common.c | 3 +++ drivers/bus/pci/pci_params.c | 10 ++++++++++ drivers/bus/pci/private.h | 13 +++++++++++++ 3 files changed, 26 insertions(+)
drivers/bus/pci/pci_common.c +3 -0
@@ -249,6 +249,9 @@ pci_probe_all_drivers(struct rte_pci_device *dev)if (dev->driver !=NULL )return 0 ; +if (rte_pci_devargs_process (dev) <0 ) +return -1 ; +FOREACH_DRIVER_ON_PCIBUS (dr) { rc =rte_pci_probe_one_driver (dr, dev);if (rc <0 )
drivers/bus/pci/pci_params.c +10 -0
@@ -127,3 +127,13 @@ rte_pci_devargs_prepare(struct rte_devargs *devargs)rte_kvargs_free (kvargs);return ret; } + +int +rte_pci_devargs_process (struct rte_pci_device *pdev) +{ +/* For the moment, no PCI param +* needs to be processed. +*/ + (void ) pdev; +return 0 ; +}
drivers/bus/pci/private.h +13 -0
@@ -207,4 +207,17 @@ rte_pci_dev_iterate(const void *start,int rte_pci_devargs_prepare (struct rte_devargs *da); +/* +* Process the device devargs, if any. +* +* @param pdev +* PCI device +* +* @return +* 0 on success. +* <0 on error. +*/ +int +rte_pci_devargs_process (struct rte_pci_device *pdev); +#endif /* _PCI_PRIVATE_H_ */
Signature · 1 line
2.18.0
PATCH v11 24/25 ethdev: process declarative eth devargs
In reply to Gaetan RivetProcess the class-specific arguments in a devargs.
This processing takes the form of setting the proper eth_dev fields when relevant.
--- lib/librte_ethdev/eth_private.h | 5 +++ lib/librte_ethdev/rte_class_eth.c | 62 +++++++++++++++++++++++++++++++ lib/librte_ethdev/rte_ethdev.c | 7 ++++ 3 files changed, 74 insertions(+)
lib/librte_ethdev/eth_private.h +5 -0
@@ -19,6 +19,11 @@ struct rte_eth_dev * eth_find_device(const struct rte_eth_dev *_start, rte_eth_cmp_t cmp, const void *data); +/* Generic rte_eth_dev parameters processor. */ +int +rte_eth_dev_args_parse (struct rte_eth_dev *eth_dev, +struct rte_devargs *da); + #ifdef __cplusplus }#endif
lib/librte_ethdev/rte_class_eth.c +62 -0
80 lines not on this page · raw message
lib/librte_ethdev/rte_ethdev.c +7 -0
@@ -41,6 +41,7 @@#include "rte_ethdev.h" #include "rte_ethdev_driver.h" #include "ethdev_profile.h" +#include "eth_private.h" int rte_eth_dev_logtype;
@@ -3486,6 +3487,12 @@ rte_eth_dev_create(struct rte_device *device, const char *name, } } + retval =rte_eth_dev_args_parse (ethdev, device->devargs); +if (retval) { +RTE_LOG (ERR ,EAL ,"ethdev parsing failed" ); + goto probe_failed; + } + retval =ethdev_init (ethdev, init_params);if (retval) {RTE_LOG (ERR ,EAL ,"ethdev initialisation failed" );
Signature · 1 line
2.18.0
PATCH v11 25/25 eal: add generic dev parameter
In reply to Gaetan RivetAdd the --dev parameter to the EAL.
This new parameter takes a generic device declaration as argument.
It uses the new devargs parsing API.
--- lib/librte_eal/common/eal_common_devargs.c | 4 +++ lib/librte_eal/common/eal_common_options.c | 36 +++++++++++++++++++--- lib/librte_eal/common/eal_options.h | 2 ++ 3 files changed, 37 insertions(+), 5 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +4 -0
@@ -219,6 +219,10 @@ rte_devargs_parse(struct rte_devargs *da, const char *dev)if (da ==NULL )return -EINVAL ; +if (strncmp (dev,"bus=" ,4 ) ==0 || +strncmp (dev,"class=" ,6 ) ==0 ) +return rte_devargs_layers_parse (da, dev); +/* Retrieve eventual bus info */ do { devname = dev;
lib/librte_eal/common/eal_common_options.c +31 -5
@@ -54,6 +54,7 @@ const struct option eal_long_options[] = { {OPT_BASE_VIRTADDR, 1, NULL, OPT_BASE_VIRTADDR_NUM }, {OPT_CREATE_UIO_DEV, 0, NULL, OPT_CREATE_UIO_DEV_NUM }, + {OPT_DEV, 1, NULL, OPT_DEV_NUM }, {OPT_FILE_PREFIX, 1, NULL, OPT_FILE_PREFIX_NUM }, {OPT_HELP, 0, NULL, OPT_HELP_NUM }, {OPT_HUGE_DIR, 1, NULL, OPT_HUGE_DIR_NUM },
@@ -109,6 +110,7 @@ TAILQ_HEAD(device_option_list, device_option);struct device_option {TAILQ_ENTRY (device_option) next; +int new;enum rte_devtype type;char arg[]; };
@@ -121,7 +123,8 @@ static int mem_parsed;static int core_parsed;static int -eal_option_device_add (enum rte_devtype type,const char *optarg) +eal_option_device_add (enum rte_devtype type,const char *optarg, +int new) {struct device_option *devopt;size_t optlen;
@@ -135,6 +138,7 @@ eal_option_device_add(enum rte_devtype type, const char *optarg) } devopt->type = type; + devopt->new = new; ret =snprintf (devopt->arg, optlen,"%s" , optarg);if (ret <0 ) {RTE_LOG (ERR ,EAL ,"Unable to copy device option\n" );
@@ -154,7 +158,22 @@ eal_option_device_parse(void) TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {if (ret ==0 ) { - ret =rte_devargs_add (devopt->type, devopt->arg); +if (devopt->new) { +struct rte_devargs *da; + + da =calloc (1 ,sizeof (*da)); + ret =rte_devargs_parse (da, devopt->arg); +if (ret) { +free (da); + }else { + ret =rte_devargs_insert (da); +if (ret) +free (da); + } + }else { + ret =rte_devargs_add (devopt->type, + devopt->arg); + }if (ret)RTE_LOG (ERR ,EAL ,"Unable to parse device '%s'\n" , devopt->arg);
@@ -1038,7 +1057,7 @@ eal_parse_common_option(int opt, const char *optarg,if (w_used) goto bw_used;if (eal_option_device_add (RTE_DEVTYPE_BLACKLISTED_PCI , - optarg) <0 ) { + optarg,0 ) <0 ) {return -1 ; } b_used =1 ;
@@ -1048,7 +1067,7 @@ eal_parse_common_option(int opt, const char *optarg,if (b_used) goto bw_used;if (eal_option_device_add (RTE_DEVTYPE_WHITELISTED_PCI , - optarg) <0 ) { + optarg,0 ) <0 ) {return -1 ; } w_used =1 ;
@@ -1177,9 +1196,16 @@ eal_parse_common_option(int opt, const char *optarg, }break ; +case OPT_DEV_NUM : +if (eal_option_device_add (RTE_DEVTYPE_VIRTUAL , + optarg,1 ) <0 ) { +return -1 ; + } +break ; +case OPT_VDEV_NUM :if (eal_option_device_add (RTE_DEVTYPE_VIRTUAL , - optarg) <0 ) { + optarg,0 ) <0 ) {return -1 ; }break ;
lib/librte_eal/common/eal_options.h +2 -0
@@ -21,6 +21,8 @@ enum {OPT_BASE_VIRTADDR_NUM , #define OPT_CREATE_UIO_DEV "create-uio-dev"OPT_CREATE_UIO_DEV_NUM , +#define OPT_DEV "dev" +OPT_DEV_NUM , #define OPT_FILE_PREFIX "file-prefix"OPT_FILE_PREFIX_NUM , #define OPT_HUGE_DIR "huge-dir"
Signature · 1 line
2.18.0
11/07/2018 23:44, Gaetan Rivet:
Quoted 18 lines
Gaetan Rivet (25):
devargs: use rte-log functions
devargs: add non-variadic parsing function
kvargs: remove error logs
kvargs: build before EAL
kvargs: introduce a more flexible parsing function
eal: introduce dtor macros
eal: introduce device class abstraction
devargs: add function to parse device layers eal/dev: add device iterator interface eal/dev: implement device iteration initialization eal/dev: implement device iteration
kvargs: add generic string matching callback bus/pci: implement device iteration and comparison bus/pci: add device matching field id bus/vdev: implement device iteration bus/vdev: add device matching field driver
ethdev: add private generic device iterator
Quoted 8 lines
ethdev: register ether layer as a class
ethdev: add device matching field name
app/testpmd: add show device command
bus/pci: pre-process declarative PCI devargs
bus/vdev: pre-process declarative vdev devargs
bus/pci: process declarative PCI devargs
ethdev: process declarative eth devargs
eal: add generic dev parameter
This series did not get a lot of reviews, especially for the last half of patches.
As it allows to make some progress to get a generic devargs syntax, I decide to apply the first 12 patches, which brings a base for a generic devargs syntax parsing.
The last half of patches are implementing some parsing/iteration for PCI, vdev and ethdev. They need to be discussed and agreed with more reviews. And more importantly, we need to add more properties in the devargs syntax, and we need to address more buses and device classes.
Getting the parsing base in 18.08 should help to close the full syntax in 18.11.
Thank you Gaetan for the big work.