devargs cleanup
cover devargs cleanup
The use of rte_devargs is inconsistent in the light of new functionalities such as device hotplug.
Most of its API is still experimental and needs stabilization. Older functions were deprecated and need to be rewritten or removed. The rte_devtype is meant to disappear. A replacement needs to be discussed and agreed upon in the coming weeks.
This patchset initiates this work.
TODO:
- Restrict device parameter parsing to the proposed new format.
- Remove devtype enum.
- Rewrite and deprecate relevant functions.
- Rewrite unit tests for new format and new API.
This patchset depends on:
Move PCI away from the EAL
http://dpdk.org/ml/archives/dev/2017-August/073512.html
Gaetan Rivet (6):
devargs: introduce iterator
devargs: introduce foreach macro
vdev: do not reference devargs_list
bus/pci: do not reference devargs_list
test: remove rte_devargs unit tests
devargs: make devargs_list private
MAINTAINERS | 1 - drivers/bus/pci/rte_pci_common.c | 6 +- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 2 +- lib/librte_eal/common/eal_common_devargs.c | 22 ++++ lib/librte_eal/common/eal_common_vdev.c | 11 +- lib/librte_eal/common/include/rte_devargs.h | 33 ++++-- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 2 +- test/test/Makefile | 1 - test/test/test_devargs.c | 131 ------------------------ 9 files changed, 55 insertions(+), 154 deletions(-) delete mode 100644 test/test/test_devargs.c
Signature · 1 line
2.1.4
PATCH 1/6 devargs: introduce iterator
In preparation to making devargs_list private.
Bus drivers generally need to access rte_devargs pertaining to their operations. This match is a common operation for bus drivers.
Add a new accessor for the rte_devargs list.
--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 + lib/librte_eal/common/eal_common_devargs.c | 19 +++++++++++++++++++ lib/librte_eal/common/include/rte_devargs.h | 18 ++++++++++++++++++ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 + 4 files changed, 39 insertions(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +1 -0
@@ -208,6 +208,7 @@ EXPERIMENTAL { global: rte_eal_devargs_insert; + rte_eal_devargs_next; rte_eal_devargs_parse; rte_eal_devargs_remove; rte_eal_hotplug_add;
lib/librte_eal/common/eal_common_devargs.c +19 -0
@@ -234,3 +234,22 @@ rte_eal_devargs_dump(FILE *f) devargs->name, devargs->args); } } + +/* bus-aware rte_devargs iterator. */ +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +{ +struct rte_devargs *da; + +if (start !=NULL ) + da =TAILQ_NEXT (start, next); +else + da =TAILQ_FIRST (&devargs_list); +while (da !=NULL ) { +if (busname ==NULL || + (strcmp (busname, da->bus->name) ==0 )) +return da; + da =TAILQ_NEXT (da, next); + } +return NULL ; +}
lib/librte_eal/common/include/rte_devargs.h +18 -0
@@ -215,6 +215,24 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);*/ void rte_eal_devargs_dump (FILE *f); +/** +* Find next rte_devargs matching the provided bus name. +* +* @param busname +* Limit the iteration to bus matching this name. +* Will return any next rte_devargs if NULL. +* +* @param start +* Starting iteration point. The iteration will start at +* the first rte_devargs if NULL. +* +* @return +* Next rte_devargs entry matching the requested bus, +* NULL if there is none. +*/ +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); + #ifdef __cplusplus }#endif
lib/librte_eal/linuxapp/eal/rte_eal_version.map +1 -0
@@ -214,6 +214,7 @@ EXPERIMENTAL { global: rte_eal_devargs_insert; + rte_eal_devargs_next; rte_eal_devargs_parse; rte_eal_devargs_remove; rte_eal_hotplug_add;
Signature · 1 line
2.1.4
PATCH 2/6 devargs: introduce foreach macro
In reply to Gaetan RivetIntroduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining to a bus.
--- lib/librte_eal/common/include/rte_devargs.h | 9 +++++++++ 1 file changed, 9 insertions(+)
lib/librte_eal/common/include/rte_devargs.h +9 -0
@@ -233,6 +233,15 @@ void rte_eal_devargs_dump(FILE *f);struct rte_devargs *rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); +/** +* Iterate over all rte_devargs for a specific bus. +*/ +#define RTE_EAL_DEVARGS_FOREACH (busname, da) \ + for (da = rte_eal_devargs_next(busname, NULL); \ + da != NULL; \ + da = rte_eal_devargs_next(busname, da)) \ + + #ifdef __cplusplus }#endif
Signature · 1 line
2.1.4
PATCH 3/6 vdev: do not reference devargs_list
In reply to Gaetan RivetThis list should not be operated upon by drivers. Use the public API to achieve the same functionalities.
--- lib/librte_eal/common/eal_common_vdev.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
lib/librte_eal/common/eal_common_vdev.c +3 -8
@@ -192,7 +192,7 @@ rte_vdev_init(const char *name, const char *args) goto fail; } -TAILQ_INSERT_TAIL (&devargs_list, devargs, next); +rte_eal_devargs_insert (devargs);TAILQ_INSERT_TAIL (&vdev_device_list, dev, next);return 0 ;
@@ -242,10 +242,8 @@ rte_vdev_uninit(const char *name)TAILQ_REMOVE (&vdev_device_list, dev, next); -TAILQ_REMOVE (&devargs_list, devargs, next); +rte_eal_devargs_remove (devargs->bus->name, devargs->name); -free (devargs->args); -free (devargs);free (dev);return 0 ; }
@@ -257,10 +255,7 @@ vdev_scan(void)struct rte_devargs *devargs;/* for virtual devices we scan the devargs_list populated via cmdline */ - TAILQ_FOREACH(devargs, &devargs_list, next) { - -if (devargs->bus != &rte_vdev_bus) -continue ; + RTE_EAL_DEVARGS_FOREACH("vdev", devargs) { dev =find_vdev (devargs->name);if (dev)
Signature · 1 line
2.1.4
PATCH 4/6 bus/pci: do not reference devargs_list
In reply to Gaetan RivetThis list should not be used by drivers.
Use the public API instead.
--- drivers/bus/pci/rte_pci_common.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/bus/pci/rte_pci_common.c +1 -5
@@ -75,12 +75,8 @@ static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev) {struct rte_devargs *devargs;struct rte_pci_addr addr; -struct rte_bus *pbus; - pbus =rte_bus_find_by_name ("pci" ); - TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->bus != pbus) -continue ; + RTE_EAL_DEVARGS_FOREACH("pci", devargs) { devargs->bus->parse (devargs->name, &addr);if (!rte_eal_compare_pci_addr (&dev->addr, &addr))return devargs;
Signature · 1 line
2.1.4
PATCH 5/6 test: remove rte_devargs unit tests
In reply to Gaetan RivetThe current test will not be compatible anymore with a private devargs_list.
Moreover, the new functions should have new tests, while the existing API will be removed.
The current unit tests are thus obsolete and hereby removed.
--- MAINTAINERS | 1 - test/test/Makefile | 1 - test/test/test_devargs.c | 131 ----------------------------------------------- 3 files changed, 133 deletions(-) delete mode 100644 test/test/test_devargs.c
MAINTAINERS +0 -1
@@ -93,7 +93,6 @@ F: test/test/test_common.c F: test/test/test_cpuflags.c F: test/test/test_cycles.c F: test/test/test_debug.c -F: test/test/test_devargs.c F: test/test/test_eal* F: test/test/test_errno.c F: test/test/test_interrupts.c
test/test/Makefile +0 -1
@@ -180,7 +180,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += test_distributor_perf.c SRCS-$(CONFIG_RTE_LIBRTE_REORDER) += test_reorder.c -SRCS-y += test_devargs.c SRCS-y += virtual_pmd.c SRCS-y += packet_burst_generator.c SRCS-$(CONFIG_RTE_LIBRTE_ACL) += test_acl.c
test/test/test_devargs.c deleted +0 -131
131 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH 6/6 devargs: make devargs_list private
In reply to Gaetan RivetInitially, rte_devargs was meant to be populated once and sometimes accessed, then never emptied.
With the new hotplug functionality having better standing, new usage appeared with repeated addition of devices and their subsequent removal.
Exposing devargs_list pushed bus drivers and libraries to be careless and inconsistent in their memory management. Making it private will allow to rationalize this part of the EAL and ensure that fewer memory leaks occur during operations.
--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 - lib/librte_eal/common/eal_common_devargs.c | 3 +++ lib/librte_eal/common/include/rte_devargs.h | 6 ------ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 - 4 files changed, 3 insertions(+), 8 deletions(-)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +0 -1
@@ -2,7 +2,6 @@ DPDK_2.0 { global: __rte_panic; - devargs_list; eal_parse_sysfs_value; eal_timer_source; lcore_config;
lib/librte_eal/common/eal_common_devargs.c +3 -0
@@ -45,6 +45,9 @@#include <rte_tailq.h> #include "eal_private.h" +/** user device double-linked queue type definition */ +TAILQ_HEAD (rte_devargs_list, rte_devargs); +/** Global list of user devices */ struct rte_devargs_list devargs_list =TAILQ_HEAD_INITIALIZER (devargs_list);
lib/librte_eal/common/include/rte_devargs.h +0 -6
@@ -86,12 +86,6 @@ struct rte_devargs {char *args; }; -/** user device double-linked queue type definition */ -TAILQ_HEAD (rte_devargs_list, rte_devargs); - -/** Global list of user devices */ -extern struct rte_devargs_list devargs_list; -/** * Parse a devargs string. *
lib/librte_eal/linuxapp/eal/rte_eal_version.map +0 -1
@@ -2,7 +2,6 @@ DPDK_2.0 { global: __rte_panic; - devargs_list; eal_parse_sysfs_value; eal_timer_source; lcore_config;
Signature · 1 line
2.1.4
v2 cover devargs cleanup
In reply to Gaetan RivetThe use of rte_devargs is inconsistent in the light of new functionalities such as device hotplug.
Most of its API is still experimental and needs stabilization. Older functions were deprecated and need to be rewritten or removed. The rte_devtype is meant to disappear.
v2:
Big rework.
* Enact requiring bus name prepended in rte_devargs parsing functions.
* Remove rte_devtype. Use new probe mode setter along with generic bus reference within rte_devargs.
* Rework devargs parsing API.
The function is now variadic, does not enforce bus rules on the devargs being inserted as the bus has been configured previously. Old parsing function is removed.
* Expose bus guessing from device name.
This uses the "parse" bus operator, which may be meant to disappear. This is optional, but nice to have in a transition period.
* Introduce new --dev generic device declaration parameter.
This patchset depends on:
Move PCI away from the EAL
http://dpdk.org/ml/archives/dev/2017-August/073512.html
Bus control framework
http://dpdk.org/ml/archives/dev/2017-October/078752.html
Gaetan Rivet (18):
eal: prepend busname on legacy device declaration
eal: remove generic devtype
devargs: introduce iterator
devargs: introduce foreach macro
vdev: do not reference devargs list bus/pci: do not reference devargs list
test: remove devargs unit tests
devargs: make devargs list private
devargs: make parsing variadic
devargs: require bus name prefix
devargs: simplify implementation
eal: add generic device declaration parameter
bus: make device recognition function public net/failsafe: keep legacy sub-device declaration
ether: use new devargs parsing function
devargs: remove old devargs parsing function
devargs: use proper prefix
doc: remove devargs deprecation notices
MAINTAINERS | 1 - app/test-pmd/cmdline.c | 2 +- doc/guides/rel_notes/deprecation.rst | 13 --- drivers/bus/pci/pci_common.c | 22 +--- drivers/net/failsafe/failsafe_args.c | 11 +- examples/bond/main.c | 2 +- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 15 ++- lib/librte_eal/common/eal_common_dev.c | 39 ++----- lib/librte_eal/common/eal_common_devargs.c | 129 +++++++++-------------- lib/librte_eal/common/eal_common_options.c | 47 ++++++--- lib/librte_eal/common/eal_common_vdev.c | 11 +- lib/librte_eal/common/eal_options.h | 2 + lib/librte_eal/common/include/rte_bus.h | 12 +++ lib/librte_eal/common/include/rte_dev.h | 8 -- lib/librte_eal/common/include/rte_devargs.h | 120 ++++++++-------------- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 15 ++- lib/librte_ether/rte_ethdev.c | 11 +- test/test/Makefile | 1 - test/test/commands.c | 2 +- test/test/test_devargs.c | 131 ------------------------ 20 files changed, 186 insertions(+), 408 deletions(-) delete mode 100644 test/test/test_devargs.c
Signature · 1 line
2.1.4
PATCH v2 1/18 eal: prepend busname on legacy device declaration
Legacy device options (-b, -w, --vdev) need to prepend their bus name to user parameters for backward compatibility.
--- lib/librte_eal/common/eal_common_options.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)
lib/librte_eal/common/eal_common_options.c +12 -5
@@ -143,13 +143,16 @@ 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 *busname,const char *optarg) {struct device_option *devopt;size_t optlen;int ret; optlen =strlen (optarg) +1 ; +if (busname !=NULL ) + optlen +=strlen (optarg) +1 ; devopt =calloc (1 ,sizeof (*devopt) + optlen);if (devopt ==NULL ) {RTE_LOG (ERR ,EAL ,"Unable to allocate device option\n" );
@@ -157,7 +160,11 @@ eal_option_device_add(enum rte_devtype type, const char *optarg) } devopt->type = type; - ret =snprintf (devopt->arg, optlen,"%s" , optarg); +if (busname !=NULL ) + ret =snprintf (devopt->arg, optlen,"%s:%s" , + busname, optarg); +else + ret =snprintf (devopt->arg, optlen,"%s" , optarg);if (ret <0 ) {RTE_LOG (ERR ,EAL ,"Unable to copy device option\n" );free (devopt);
@@ -1003,7 +1010,7 @@ eal_parse_common_option(int opt, const char *optarg,if (rte_bus_probe_mode_set ("pci" ,RTE_BUS_PROBE_BLACKLIST ) <0 )return -1 ;if (eal_option_device_add (RTE_DEVTYPE_BLACKLISTED_PCI , - optarg) <0 ) { +"pci" , optarg) <0 ) {return -1 ; }break ;
@@ -1012,7 +1019,7 @@ eal_parse_common_option(int opt, const char *optarg,if (rte_bus_probe_mode_set ("pci" ,RTE_BUS_PROBE_WHITELIST ) <0 )return -1 ;if (eal_option_device_add (RTE_DEVTYPE_WHITELISTED_PCI , - optarg) <0 ) { +"pci" , optarg) <0 ) {return -1 ; }break ;
@@ -1122,7 +1129,7 @@ eal_parse_common_option(int opt, const char *optarg,case OPT_VDEV_NUM :if (eal_option_device_add (RTE_DEVTYPE_VIRTUAL , - optarg) <0 ) { +"vdev" , optarg) <0 ) {return -1 ; }break ;
Signature · 1 line
2.1.4
PATCH v2 2/18 eal: remove generic devtype
In reply to Gaetan RivetThe devtype is now entirely defined by the device bus. As such, it is already characterized by the bus identifier within an rte_devargs.
The rte_devtype enum can disappear, along with crutches added during this transition.
rte_eal_devargs_type_count becomes useless and is removed.
--- drivers/bus/pci/pci_common.c | 16 ++------------ lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 - lib/librte_eal/common/eal_common_devargs.c | 20 +---------------- lib/librte_eal/common/eal_common_options.c | 19 +++++----------- lib/librte_eal/common/include/rte_dev.h | 8 ------- lib/librte_eal/common/include/rte_devargs.h | 29 +------------------------ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 - 7 files changed, 9 insertions(+), 85 deletions(-)
drivers/bus/pci/pci_common.c +2 -14
@@ -172,15 +172,6 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr, loc->domain, loc->bus, loc->devid, loc->function, dev->device.numa_node); -/* no initialization when blacklisted, return without error */ -if (dev->device.devargs !=NULL && - dev->device.devargs->policy == -RTE_DEV_BLACKLISTED ) { -RTE_LOG (INFO ,EAL ," Device is blacklisted, not" -" initializing\n" ); -return 1 ; - } -if (dev->device.numa_node <0 ) {RTE_LOG (WARNING ,EAL ," Invalid NUMA socket, default to 0\n" ); dev->device.numa_node =0 ;
@@ -380,11 +371,8 @@ rte_pci_probe(void) probed++; devargs = dev->device.devargs; -/* probe all or only whitelisted devices */ -if (probe_all) - ret =pci_probe_all_drivers (dev); -else if (devargs !=NULL && - devargs->policy ==RTE_DEV_WHITELISTED ) +/* probe all or only declared devices */ +if (probe_all ^ (devargs !=NULL )) ret =pci_probe_all_drivers (dev);if (ret <0 ) { RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMT
lib/librte_eal/bsdapp/eal/rte_eal_version.map +0 -1
@@ -22,7 +22,6 @@ DPDK_2.0 { rte_eal_alarm_set; rte_eal_devargs_add; rte_eal_devargs_dump; - rte_eal_devargs_type_count; rte_eal_get_configuration; rte_eal_get_lcore_state; rte_eal_get_physmem_layout;
lib/librte_eal/common/eal_common_devargs.c +1 -19
@@ -153,7 +153,7 @@ rte_eal_devargs_insert(struct rte_devargs *da)/* store a whitelist parameter for later parsing */ int -rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) +rte_eal_devargs_add (const char *devargs_str) {struct rte_devargs *devargs =NULL ;const char *dev = devargs_str;
@@ -165,9 +165,6 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)if (rte_eal_devargs_parse (dev, devargs)) goto fail; - devargs->type = devtype; -if (devargs->type ==RTE_DEVTYPE_BLACKLISTED_PCI ) - devargs->policy =RTE_DEV_BLACKLISTED ;TAILQ_INSERT_TAIL (&devargs_list, devargs, next);return 0 ;
@@ -198,21 +195,6 @@ rte_eal_devargs_remove(const char *busname, const char *devname)return 1 ; } -/* count the number of devices of a specified type */ -unsigned int -rte_eal_devargs_type_count (enum rte_devtype devtype) -{ -struct rte_devargs *devargs; -unsigned int count =0 ; - - TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type != devtype) -continue ; - count++; - } -return count; -} -/* dump the user devices on the console */ void rte_eal_devargs_dump (FILE *f)
lib/librte_eal/common/eal_common_options.c +5 -14
@@ -131,7 +131,6 @@ TAILQ_HEAD(device_option_list, device_option);struct device_option {TAILQ_ENTRY (device_option) next; -enum rte_devtype type;char arg[]; };
@@ -143,8 +142,7 @@ static int mem_parsed;static int core_parsed;static int -eal_option_device_add (enum rte_devtype type, -const char *busname,const char *optarg) +eal_option_device_add (const char *busname,const char *optarg) {struct device_option *devopt;size_t optlen;
@@ -159,7 +157,6 @@ eal_option_device_add(enum rte_devtype type,return -ENOMEM ; } - devopt->type = type;if (busname !=NULL ) ret =snprintf (devopt->arg, optlen,"%s:%s" , busname, optarg);
@@ -183,7 +180,7 @@ eal_option_device_parse(void) TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {if (ret ==0 ) { - ret =rte_eal_devargs_add (devopt->type, devopt->arg); + ret =rte_eal_devargs_add (devopt->arg);if (ret)RTE_LOG (ERR ,EAL ,"Unable to parse device '%s'\n" , devopt->arg);
@@ -1009,19 +1006,15 @@ eal_parse_common_option(int opt, const char *optarg,case 'b' :if (rte_bus_probe_mode_set ("pci" ,RTE_BUS_PROBE_BLACKLIST ) <0 )return -1 ; -if (eal_option_device_add (RTE_DEVTYPE_BLACKLISTED_PCI , -"pci" , optarg) <0 ) { +if (eal_option_device_add ("pci" , optarg) <0 )return -1 ; - }break ;/* whitelist */ case 'w' :if (rte_bus_probe_mode_set ("pci" ,RTE_BUS_PROBE_WHITELIST ) <0 )return -1 ; -if (eal_option_device_add (RTE_DEVTYPE_WHITELISTED_PCI , -"pci" , optarg) <0 ) { +if (eal_option_device_add ("pci" , optarg) <0 )return -1 ; - }break ;/* coremask */ case 'c' :
@@ -1128,10 +1121,8 @@ eal_parse_common_option(int opt, const char *optarg,break ;case OPT_VDEV_NUM : -if (eal_option_device_add (RTE_DEVTYPE_VIRTUAL , -"vdev" , optarg) <0 ) { +if (eal_option_device_add ("vdev" , optarg) <0 )return -1 ; - }break ;case OPT_SYSLOG_NUM :
lib/librte_eal/common/include/rte_dev.h +0 -8
@@ -127,14 +127,6 @@ enum rte_kernel_driver { };/** -* Device policies. -*/ -enum rte_dev_policy { -RTE_DEV_WHITELISTED , -RTE_DEV_BLACKLISTED , -}; - -/** * A generic memory resource representation. */ struct rte_mem_resource {
lib/librte_eal/common/include/rte_devargs.h +1 -28
@@ -53,15 +53,6 @@ extern "C" {#include <rte_bus.h> /** -* Type of generic device -*/ -enum rte_devtype { -RTE_DEVTYPE_WHITELISTED_PCI , -RTE_DEVTYPE_BLACKLISTED_PCI , -RTE_DEVTYPE_VIRTUAL , -}; - -/** * Structure that stores a device given by the user with its arguments * * A user device is a physical or a virtual device given by the user to
@@ -74,10 +65,6 @@ enum rte_devtype {struct rte_devargs {/** Next in list. */ TAILQ_ENTRY (rte_devargs) next; -/** Type of device. */ -enum rte_devtype type; -/** Device policy. */ -enum rte_dev_policy policy;/** Bus handle for the device. */ struct rte_bus *bus;/** Name of the device. */
@@ -166,8 +153,6 @@ rte_eal_devargs_insert(struct rte_devargs *da);* driver name is not checked by this function, it is done when probing * the drivers. * -* @param devtype -* The type of the device. * @param devargs_str * The arguments as given by the user. *
@@ -175,7 +160,7 @@ rte_eal_devargs_insert(struct rte_devargs *da);* - 0 on success * - A negative value on error */ -int rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str); +int rte_eal_devargs_add (const char *devargs_str);/** * Remove a device from the user device list.
@@ -196,18 +181,6 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str);int rte_eal_devargs_remove (const char *busname,const char *devname);/** -* Count the number of user devices of a specified type -* -* @param devtype -* The type of the devices to counted. -* -* @return -* The number of devices. -*/ -unsigned int -rte_eal_devargs_type_count (enum rte_devtype devtype); - -/** * This function dumps the list of user device and their arguments. * * @param f
lib/librte_eal/linuxapp/eal/rte_eal_version.map +0 -1
@@ -22,7 +22,6 @@ DPDK_2.0 { rte_eal_alarm_set; rte_eal_devargs_add; rte_eal_devargs_dump; - rte_eal_devargs_type_count; rte_eal_get_configuration; rte_eal_get_lcore_state; rte_eal_get_physmem_layout;
Signature · 1 line
2.1.4
PATCH v2 3/18 devargs: introduce iterator
In reply to Gaetan RivetIn preparation to making devargs_list private.
Bus drivers generally need to access rte_devargs pertaining to their operations. This match is a common operation for bus drivers.
Add a new accessor for the rte_devargs list.
--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 + lib/librte_eal/common/eal_common_devargs.c | 19 +++++++++++++++++++ lib/librte_eal/common/include/rte_devargs.h | 18 ++++++++++++++++++ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 + 4 files changed, 39 insertions(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +1 -0
@@ -189,6 +189,7 @@ EXPERIMENTAL { global: rte_eal_devargs_insert; + rte_eal_devargs_next; rte_eal_devargs_parse; rte_eal_devargs_remove; rte_eal_hotplug_add;
lib/librte_eal/common/eal_common_devargs.c +19 -0
@@ -208,3 +208,22 @@ rte_eal_devargs_dump(FILE *f) devargs->name, devargs->args); } } + +/* bus-aware rte_devargs iterator. */ +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +{ +struct rte_devargs *da; + +if (start !=NULL ) + da =TAILQ_NEXT (start, next); +else + da =TAILQ_FIRST (&devargs_list); +while (da !=NULL ) { +if (busname ==NULL || + (strcmp (busname, da->bus->name) ==0 )) +return da; + da =TAILQ_NEXT (da, next); + } +return NULL ; +}
lib/librte_eal/common/include/rte_devargs.h +18 -0
@@ -188,6 +188,24 @@ int rte_eal_devargs_remove(const char *busname, const char *devname);*/ void rte_eal_devargs_dump (FILE *f); +/** +* Find next rte_devargs matching the provided bus name. +* +* @param busname +* Limit the iteration to bus matching this name. +* Will return any next rte_devargs if NULL. +* +* @param start +* Starting iteration point. The iteration will start at +* the first rte_devargs if NULL. +* +* @return +* Next rte_devargs entry matching the requested bus, +* NULL if there is none. +*/ +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); + #ifdef __cplusplus }#endif
lib/librte_eal/linuxapp/eal/rte_eal_version.map +1 -0
@@ -193,6 +193,7 @@ EXPERIMENTAL { global: rte_eal_devargs_insert; + rte_eal_devargs_next; rte_eal_devargs_parse; rte_eal_devargs_remove; rte_eal_hotplug_add;
Signature · 1 line
2.1.4
PATCH v2 4/18 devargs: introduce foreach macro
In reply to Gaetan RivetIntroduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining to a bus.
--- lib/librte_eal/common/include/rte_devargs.h | 8 ++++++++ 1 file changed, 8 insertions(+)
lib/librte_eal/common/include/rte_devargs.h +8 -0
@@ -206,6 +206,14 @@ void rte_eal_devargs_dump(FILE *f);struct rte_devargs *rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); +/** +* Iterate over all rte_devargs for a specific bus. +*/ +#define RTE_EAL_DEVARGS_FOREACH (busname, da) \ + for (da = rte_eal_devargs_next(busname, NULL); \ + da != NULL; \ + da = rte_eal_devargs_next(busname, da)) \ + #ifdef __cplusplus }#endif
Signature · 1 line
2.1.4
PATCH v2 5/18 vdev: do not reference devargs list
In reply to Gaetan RivetThis list should not be operated upon by drivers. Use the public API to achieve the same functionalities.
--- lib/librte_eal/common/eal_common_vdev.c | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-)
lib/librte_eal/common/eal_common_vdev.c +3 -8
@@ -192,7 +192,7 @@ rte_vdev_init(const char *name, const char *args) goto fail; } -TAILQ_INSERT_TAIL (&devargs_list, devargs, next); +rte_eal_devargs_insert (devargs);TAILQ_INSERT_TAIL (&vdev_device_list, dev, next);return 0 ;
@@ -242,10 +242,8 @@ rte_vdev_uninit(const char *name)TAILQ_REMOVE (&vdev_device_list, dev, next); -TAILQ_REMOVE (&devargs_list, devargs, next); +rte_eal_devargs_remove (devargs->bus->name, devargs->name); -free (devargs->args); -free (devargs);free (dev);return 0 ; }
@@ -257,10 +255,7 @@ vdev_scan(void)struct rte_devargs *devargs;/* for virtual devices we scan the devargs_list populated via cmdline */ - TAILQ_FOREACH(devargs, &devargs_list, next) { - -if (devargs->bus != &rte_vdev_bus) -continue ; + RTE_EAL_DEVARGS_FOREACH("vdev", devargs) { dev =find_vdev (devargs->name);if (dev)
Signature · 1 line
2.1.4
PATCH v2 6/18 bus/pci: do not reference devargs list
In reply to Gaetan RivetThis list should not be used by drivers.
Use the public API instead.
--- drivers/bus/pci/pci_common.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-)
drivers/bus/pci/pci_common.c +1 -5
@@ -75,12 +75,8 @@ static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev) {struct rte_devargs *devargs;struct rte_pci_addr addr; -struct rte_bus *pbus; - pbus =rte_bus_find_by_name ("pci" ); - TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->bus != pbus) -continue ; + RTE_EAL_DEVARGS_FOREACH("pci", devargs) { devargs->bus->parse (devargs->name, &addr);if (!rte_eal_compare_pci_addr (&dev->addr, &addr))return devargs;
Signature · 1 line
2.1.4
PATCH v2 7/18 test: remove devargs unit tests
In reply to Gaetan RivetThe current test will not be compatible anymore with a private devargs list.
Moreover, the new functions should have new tests, while the existing API will be removed.
The current unit tests are thus obsolete and hereby removed.
--- MAINTAINERS | 1 - test/test/Makefile | 1 - test/test/test_devargs.c | 131 ----------------------------------------------- 3 files changed, 133 deletions(-) delete mode 100644 test/test/test_devargs.c
MAINTAINERS +0 -1
@@ -96,7 +96,6 @@ F: test/test/test_common.c F: test/test/test_cpuflags.c F: test/test/test_cycles.c F: test/test/test_debug.c -F: test/test/test_devargs.c F: test/test/test_eal* F: test/test/test_errno.c F: test/test/test_interrupts.c
test/test/Makefile +0 -1
@@ -184,7 +184,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += test_distributor_perf.c SRCS-$(CONFIG_RTE_LIBRTE_REORDER) += test_reorder.c -SRCS-y += test_devargs.c SRCS-y += virtual_pmd.c SRCS-y += packet_burst_generator.c SRCS-$(CONFIG_RTE_LIBRTE_ACL) += test_acl.c
test/test/test_devargs.c deleted +0 -131
131 lines not on this page · raw message
Signature · 1 line
2.1.4
PATCH v2 8/18 devargs: make devargs list private
In reply to Gaetan RivetInitially, rte_devargs was meant to be populated once and sometimes accessed, then never emptied.
With the new hotplug functionality having better standing, new usage appeared with repeated addition of devices and their subsequent removal.
Exposing devargs_list pushed bus drivers and libraries to be careless and inconsistent in their memory management. Making it private will allow to rationalize this part of the EAL and ensure that fewer memory leaks occur during operations.
--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 - lib/librte_eal/common/eal_common_devargs.c | 3 +++ lib/librte_eal/common/include/rte_devargs.h | 6 ------ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 - 4 files changed, 3 insertions(+), 8 deletions(-)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +0 -1
@@ -2,7 +2,6 @@ DPDK_2.0 { global: __rte_panic; - devargs_list; eal_parse_sysfs_value; eal_timer_source; lcore_config;
lib/librte_eal/common/eal_common_devargs.c +3 -0
@@ -45,6 +45,9 @@#include <rte_tailq.h> #include "eal_private.h" +/** user device double-linked queue type definition */ +TAILQ_HEAD (rte_devargs_list, rte_devargs); +/** Global list of user devices */ struct rte_devargs_list devargs_list =TAILQ_HEAD_INITIALIZER (devargs_list);
lib/librte_eal/common/include/rte_devargs.h +0 -6
@@ -73,12 +73,6 @@ struct rte_devargs {char *args; }; -/** user device double-linked queue type definition */ -TAILQ_HEAD (rte_devargs_list, rte_devargs); - -/** Global list of user devices */ -extern struct rte_devargs_list devargs_list; -/** * Parse a devargs string. *
lib/librte_eal/linuxapp/eal/rte_eal_version.map +0 -1
@@ -2,7 +2,6 @@ DPDK_2.0 { global: __rte_panic; - devargs_list; eal_parse_sysfs_value; eal_timer_source; lcore_config;
Signature · 1 line
2.1.4
PATCH v2 9/18 devargs: make parsing variadic
In reply to Gaetan Rivetrte_eal_devargs_parse can be used by EAL subsystems, drivers, applications alike.
Device parameters may be presented with different structure each time; as a single declaration string or several strings each describing different parts of the declaration.
To simplify the use of this parsing facility, its parameters are made variadic.
--- drivers/net/failsafe/failsafe_args.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 33 ++++------------------------- lib/librte_eal/common/eal_common_devargs.c | 15 ++++++++++--- lib/librte_eal/common/include/rte_devargs.h | 25 +++++++++++----------- 4 files changed, 30 insertions(+), 45 deletions(-)
drivers/net/failsafe/failsafe_args.c +1 -1
@@ -88,7 +88,7 @@ fs_parse_device(struct sub_device *sdev, char *args) d = &sdev->devargs;DEBUG ("%s" , args); - ret =rte_eal_devargs_parse (args, d); + ret =rte_eal_devargs_parse (d,"%s" , args);if (ret) {DEBUG ("devargs parsing failed with code %d" , ret);return ret;
lib/librte_eal/common/eal_common_dev.c +4 -29
@@ -127,29 +127,12 @@ int rte_eal_dev_detach(struct rte_device *dev)return ret; } -static char * -full_dev_name (const char *bus,const char *dev,const char *args) -{ -char *name; -size_t len; - - len =snprintf (NULL ,0 ,"%s:%s,%s" , bus, dev, args) +1 ; - name =calloc (1 , len); -if (name ==NULL ) { -RTE_LOG (ERR ,EAL ,"Could not allocate full device name\n" ); -return NULL ; - } -snprintf (name, len,"%s:%s,%s" , bus, dev, args); -return name; -} -int rte_eal_hotplug_add (const char *busname,const char *devname,const char *devargs) {struct rte_bus *bus;struct rte_device *dev;struct rte_devargs *da; -char *name;int ret; bus =rte_bus_find_by_name (busname);
@@ -164,17 +147,12 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,return -ENOTSUP ; } - name =full_dev_name (busname, devname, devargs); -if (name ==NULL ) + da =calloc (1 ,sizeof (*da)); +if (da ==NULL )return -ENOMEM ; - da =calloc (1 ,sizeof (*da)); -if (da ==NULL ) { - ret = -ENOMEM ; - goto err_name; - } - - ret =rte_eal_devargs_parse (name, da); + ret =rte_eal_devargs_parse (da,"%s:%s,%s" , + busname, devname, devargs);if (ret) goto err_devarg;
@@ -200,7 +178,6 @@ int rte_eal_hotplug_add(const char *busname, const char *devname, dev->name); goto err_devarg; } -free (name);return 0 ; err_devarg:
@@ -208,8 +185,6 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,free (da->args);free (da); } -err_name: -free (name);return ret; }
lib/librte_eal/common/eal_common_devargs.c +12 -3
@@ -39,6 +39,7 @@#include <stdio.h> #include <string.h> +#include <stdarg.h> #include <rte_dev.h> #include <rte_devargs.h>
@@ -89,15 +90,23 @@ bus_name_cmp(const struct rte_bus *bus, const void *name) }int -rte_eal_devargs_parse (const char *dev,struct rte_devargs *da) +rte_eal_devargs_parse (struct rte_devargs *da,const char *format, ...) {struct rte_bus *bus =NULL ; +va_list ap; +va_start (ap, format); +char dev[vsnprintf (NULL ,0 , format, ap) +1 ];const char *devname;const size_t maxlen =sizeof (da->name);size_t i; -if (dev ==NULL || da ==NULL ) +va_end (ap); +if (da ==NULL )return -EINVAL ; + +va_start (ap, format); +vsnprintf (dev,sizeof (dev), format, ap); +va_end (ap);/* Retrieve eventual bus info */ do { devname = dev;
@@ -166,7 +175,7 @@ rte_eal_devargs_add(const char *devargs_str)if (devargs ==NULL ) goto fail; -if (rte_eal_devargs_parse (dev, devargs)) +if (rte_eal_devargs_parse (devargs,"%s" , dev)) goto fail;TAILQ_INSERT_TAIL (&devargs_list, devargs, next);return 0 ;
lib/librte_eal/common/include/rte_devargs.h +13 -12
@@ -108,18 +108,20 @@ int rte_eal_parse_devargs_str(const char *devargs_str,* in argument. Store which bus will handle the device, its name * and the eventual device parameters. * -* @param dev -* The device declaration string. +* The device string is built with a printf-like syntax. +* * @param da * The devargs structure holding the device information. +* @param format +* Format string describing a device. * * @return * - 0 on success. * - Negative errno on error. */ int -rte_eal_devargs_parse (const char *dev, -struct rte_devargs *da); +rte_eal_devargs_parse (struct rte_devargs *da, +const char *format, ...);/** * Insert an rte_devargs in the global list.
@@ -137,15 +139,14 @@ rte_eal_devargs_insert(struct rte_devargs *da);/** * Add a device to the user device list * -* For PCI devices, the format of arguments string is "PCI_ADDR" or -* "PCI_ADDR,key=val,key2=val2,...". Examples: "08:00.1", "0000:5:00.0", -* "04:00.0,arg=val". +* The format is * -* For virtual devices, the format of arguments string is "DRIVER_NAME*" -* or "DRIVER_NAME*,key=val,key2=val2,...". Examples: "net_ring", -* "net_ring0", "net_pmdAnything,arg=0:arg2=1". The validity of the -* driver name is not checked by this function, it is done when probing -* the drivers. +* bus:device_identifier,arg1=val1,arg2=val2 +* +* Examples: +* +* pci:0000:05.00.0,arg=val +* vdev:net_ring0 * * @param devargs_str * The arguments as given by the user.
Signature · 1 line
2.1.4
PATCH v2 10/18 devargs: require bus name prefix
In reply to Gaetan RivetThe EAL now requires the bus to be prepended to the device declaration string.
--- lib/librte_eal/common/eal_common_devargs.c | 28 +++++++++------------------- 1 file changed, 9 insertions(+), 19 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +9 -19
@@ -107,18 +107,17 @@ rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...)va_start (ap, format);vsnprintf (dev,sizeof (dev), format, ap);va_end (ap); -/* Retrieve eventual bus info */ -do { - devname = dev; - bus =rte_bus_find (bus, bus_name_cmp, dev); -if (bus ==NULL ) -break ; - devname = dev +strlen (bus->name) +1 ; -if (rte_bus_find_by_device_name (devname) == bus) -break ; - }while (1 ); +/* Retrieve bus info */ + bus =rte_bus_find (bus, bus_name_cmp, dev); +if (bus ==NULL ) { +fprintf (stderr,"ERROR: failed to parse bus from \"%s\"\n" , + dev); +return -EFAULT ; + } + da->bus = bus;/* Store device name */ i =0 ; + devname = dev +strlen (bus->name) +1 ;while (devname[i] !='\0' && devname[i] !=',' ) { da->name[i] = devname[i]; i++;
@@ -130,15 +129,6 @@ rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...) } } da->name[i] ='\0' ; -if (bus ==NULL ) { - bus =rte_bus_find_by_device_name (da->name); -if (bus ==NULL ) { -fprintf (stderr,"ERROR: failed to parse device \"%s\"\n" , - da->name); -return -EFAULT ; - } - } - da->bus = bus;/* Parse eventual device arguments */ if (devname[i] ==',' ) da->args =strdup (&devname[i +1 ]);
Signature · 1 line
2.1.4
PATCH v2 11/18 devargs: simplify implementation
In reply to Gaetan RivetRe-use existing code, remove incorrect comments.
--- lib/librte_eal/common/eal_common_devargs.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +3 -5
@@ -153,21 +153,19 @@ rte_eal_devargs_insert(struct rte_devargs *da)return 0 ; } -/* store a whitelist parameter for later parsing */ int -rte_eal_devargs_add (const char *devargs_str) +rte_eal_devargs_add (const char *dev) {struct rte_devargs *devargs =NULL ; -const char *dev = devargs_str; -/* use calloc instead of rte_zmalloc as it's called early at init */ devargs =calloc (1 ,sizeof (*devargs));if (devargs ==NULL ) goto fail;if (rte_eal_devargs_parse (devargs,"%s" , dev)) goto fail; -TAILQ_INSERT_TAIL (&devargs_list, devargs, next); +if (rte_eal_devargs_insert (devargs)) + goto fail;return 0 ; fail:
Signature · 1 line
2.1.4
PATCH v2 12/18 eal: add generic device declaration parameter
In reply to Gaetan RivetAdd a new generic device declaration parameter:
--dev=<device_declaration>
That allows to declare device from any bus. The format is as follows:
device_declaration := <bus><c><device>[,arg_list]
bus := bus name c := arbitrary character separator device := device name (PCI location, virtual PMD name, ...)
arg_list := key value list: key1=val1[,key2=val2[,...]]
The bus name is mandatory. The character separator can be anything. The device name is mandatory. The argument list is optional.
Examples:
--dev=pci:0000:05:00.0,port=1
--dev=vdev_net_ring0--- lib/librte_eal/common/eal_common_options.c | 19 +++++++++++++++++++ lib/librte_eal/common/eal_options.h | 2 ++ 2 files changed, 21 insertions(+)
lib/librte_eal/common/eal_common_options.c +19 -0
@@ -95,6 +95,7 @@ eal_long_options[] = { {OPT_PROC_TYPE, 1, NULL, OPT_PROC_TYPE_NUM }, {OPT_SOCKET_MEM, 1, NULL, OPT_SOCKET_MEM_NUM }, {OPT_SYSLOG, 1, NULL, OPT_SYSLOG_NUM }, + {OPT_DEV, 1, NULL, OPT_DEV_NUM }, {OPT_VDEV, 1, NULL, OPT_VDEV_NUM }, {OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM }, {OPT_VMWARE_TSC_MAP, 0, NULL, OPT_VMWARE_TSC_MAP_NUM },
@@ -1120,6 +1121,21 @@ eal_parse_common_option(int opt, const char *optarg, }break ; +case OPT_DEV_NUM : { +struct rte_devargs da; +int ret; + +if (rte_eal_devargs_parse (&da, optarg) <0 ) +return -1 ; + ret =rte_bus_probe_mode_set (da.bus->name, +RTE_BUS_PROBE_WHITELIST ); +if (ret <0 && ret != -ENOTSUP ) +return -1 ; +if (eal_option_device_add (NULL , optarg) <0 ) +return -1 ; + } +break ; +case OPT_VDEV_NUM :if (eal_option_device_add ("vdev" , optarg) <0 )return -1 ;
@@ -1271,6 +1287,9 @@ eal_common_usage(void)" -n CHANNELS Number of memory channels\n" " -m MB Memory to allocate (see also --" OPT_SOCKET_MEM ")\n" " -r RANKS Force number of memory ranks (don't detect)\n" +" --" OPT_DEV " Declare a device.\n" +" The argument format is <bus><c><device>[,key=val,...]\n" +" ex: pci:00:00.0,key=val\n" " -b, --" OPT_PCI_BLACKLIST " Add a PCI device in black list.\n" " Prevent EAL from using this PCI device. The argument\n" " format is <domain:bus:devid.func>.\n"
lib/librte_eal/common/eal_options.h +2 -0
@@ -77,6 +77,8 @@ enum {OPT_SOCKET_MEM_NUM , #define OPT_SYSLOG "syslog"OPT_SYSLOG_NUM , +#define OPT_DEV "dev" +OPT_DEV_NUM , #define OPT_VDEV "vdev"OPT_VDEV_NUM , #define OPT_VFIO_INTR "vfio-intr"
Signature · 1 line
2.1.4
PATCH v2 13/18 bus: make device recognition function public
In reply to Gaetan RivetAs other EAL facilities now requires the bus to be explicitly mentioned, the function rte_bus_find_by_device_name can be used by third parties to ease the transition to a more formal device definition scheme.
--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 + lib/librte_eal/common/include/rte_bus.h | 12 ++++++++++++ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 + 3 files changed, 14 insertions(+)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +1 -0
@@ -223,6 +223,7 @@ EXPERIMENTAL { DPDK_17.11 { global: + rte_bus_find_by_device_name; rte_bus_get_iommu_class; rte_eal_iova_mode; rte_eal_mbuf_default_mempool_ops;
lib/librte_eal/common/include/rte_bus.h +12 -0
@@ -364,6 +364,18 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);struct rte_bus *rte_bus_find_by_name (const char *busname);/** +* Find a bus capable of identifying a device. +* +* @param str +* A device identifier (PCI address, virtual PMD name, ...). +* +* @return +* A valid bus handle if found. +* NULL if no bus is able to parse this device. +*/ +struct rte_bus *rte_bus_find_by_device_name (const char *str); + +/** * Helper for Bus registration. * The constructor has higher priority than PMD constructors. */
lib/librte_eal/linuxapp/eal/rte_eal_version.map +1 -0
@@ -227,6 +227,7 @@ EXPERIMENTAL { DPDK_17.11 { global: + rte_bus_find_by_device_name; rte_bus_get_iommu_class; rte_eal_iova_mode; rte_eal_mbuf_default_mempool_ops;
Signature · 1 line
2.1.4
PATCH v2 14/18 net/failsafe: keep legacy sub-device declaration
In reply to Gaetan RivetAllow users to use the same sub-device declaration format.
The rte_devargs library now requires the bus name to be prepended to device declarations. While it is possible to use this new format, the transition to the new one can be made smoother.
--- drivers/net/failsafe/failsafe_args.c | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-)
drivers/net/failsafe/failsafe_args.c +10 -1
@@ -34,6 +34,7 @@#include <string.h> #include <errno.h> +#include <rte_bus.h> #include <rte_debug.h> #include <rte_devargs.h> #include <rte_malloc.h>
@@ -83,12 +84,20 @@ closing_paren(const char *text)static int fs_parse_device (struct sub_device *sdev,char *args) { +struct rte_bus *bus;struct rte_devargs *d;int ret; d = &sdev->devargs;DEBUG ("%s" , args); - ret =rte_eal_devargs_parse (d,"%s" , args); + bus =rte_bus_find_by_device_name (args); +if (bus ==NULL ) +/* args may contain the bus name */ + ret =rte_eal_devargs_parse (d,"%s" , args); +else +/* args is a device name */ + ret =rte_eal_devargs_parse (d,"%s:%s" , + bus->name, args);if (ret) {DEBUG ("devargs parsing failed with code %d" , ret);return ret;
Signature · 1 line
2.1.4
PATCH v2 15/18 ether: use new devargs parsing function
In reply to Gaetan RivetThe previous one is being deprecated.
--- lib/librte_ether/rte_ethdev.c | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-)
lib/librte_ether/rte_ethdev.c +7 -4
@@ -372,21 +372,24 @@ rte_eth_dev_is_detachable(uint16_t port_id)/* attach the new device, then store port_id of the device */ int -rte_eth_dev_attach (const char *devargs,uint16_t *port_id) +rte_eth_dev_attach (const char *devstr,uint16_t *port_id) {int ret =-1 ;int current =rte_eth_dev_count (); +struct rte_devargs devargs;char *name =NULL ;char *args =NULL ; -if ((devargs ==NULL ) || (port_id ==NULL )) { +if ((devstr ==NULL ) || (port_id ==NULL )) { ret = -EINVAL ; goto err; } -/* parse devargs, then retrieve device name and args */ -if (rte_eal_parse_devargs_str (devargs, &name, &args)) +/* parse device, then retrieve device name and args */ +if (rte_eal_devargs_parse (&devargs,"%s" , devstr)) goto err; + name = devargs.name; + args = devargs.args; ret =rte_eal_dev_attach (name, args);if (ret <0 )
Signature · 1 line
2.1.4
PATCH v2 16/18 devargs: remove old devargs parsing function
In reply to Gaetan RivetThis functionality is already covered by rte_eal_devargs_parse.
--- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 - lib/librte_eal/common/eal_common_devargs.c | 30 ------------------------- lib/librte_eal/common/include/rte_devargs.h | 28 ----------------------- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 - 4 files changed, 60 deletions(-)
lib/librte_eal/bsdapp/eal/rte_eal_version.map +0 -1
@@ -32,7 +32,6 @@ DPDK_2.0 { rte_eal_lcore_role; rte_eal_mp_remote_launch; rte_eal_mp_wait_lcore; - rte_eal_parse_devargs_str; rte_eal_process_type; rte_eal_remote_launch; rte_eal_tailq_lookup;
lib/librte_eal/common/eal_common_devargs.c +0 -30
@@ -53,36 +53,6 @@ TAILQ_HEAD(rte_devargs_list, rte_devargs);struct rte_devargs_list devargs_list =TAILQ_HEAD_INITIALIZER (devargs_list); -int -rte_eal_parse_devargs_str (const char *devargs_str, -char **drvname,char **drvargs) -{ -char *sep; - -if ((devargs_str) ==NULL || (drvname) ==NULL || (drvargs ==NULL )) -return -1 ; - - *drvname =strdup (devargs_str); -if (*drvname ==NULL ) -return -1 ; - -/* set the first ',' to '\0' to split name and arguments */ - sep =strchr (*drvname,',' ); -if (sep !=NULL ) { - sep[0 ] ='\0' ; - *drvargs =strdup (sep +1 ); - }else { - *drvargs =strdup ("" ); - } - -if (*drvargs ==NULL ) { -free (*drvname); - *drvname =NULL ; -return -1 ; - } -return 0 ; -} -static int bus_name_cmp (const struct rte_bus *bus,const void *name) {
lib/librte_eal/common/include/rte_devargs.h +0 -28
@@ -74,34 +74,6 @@ struct rte_devargs { };/** -* Parse a devargs string. -* -* For PCI devices, the format of arguments string is "PCI_ADDR" or -* "PCI_ADDR,key=val,key2=val2,...". Examples: "08:00.1", "0000:5:00.0", -* "04:00.0,arg=val". -* -* For virtual devices, the format of arguments string is "DRIVER_NAME*" -* or "DRIVER_NAME*,key=val,key2=val2,...". Examples: "net_ring", -* "net_ring0", "net_pmdAnything,arg=0:arg2=1". -* -* The function parses the arguments string to get driver name and driver -* arguments. -* -* @param devargs_str -* The arguments as given by the user. -* @param drvname -* The pointer to the string to store parsed driver name. -* @param drvargs -* The pointer to the string to store parsed driver arguments. -* -* @return -* - 0 on success -* - A negative value on error -*/ -int 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
lib/librte_eal/linuxapp/eal/rte_eal_version.map +0 -1
@@ -32,7 +32,6 @@ DPDK_2.0 { rte_eal_lcore_role; rte_eal_mp_remote_launch; rte_eal_mp_wait_lcore; - rte_eal_parse_devargs_str; rte_eal_process_type; rte_eal_remote_launch; rte_eal_tailq_lookup;
Signature · 1 line
2.1.4
PATCH v2 17/18 devargs: use proper prefix
In reply to Gaetan Rivetrte_eal_devargs is redundant.
Make it concise.
--- app/test-pmd/cmdline.c | 2 +- drivers/net/failsafe/failsafe_args.c | 4 ++-- examples/bond/main.c | 2 +- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 12 ++++++------ lib/librte_eal/common/eal_common_dev.c | 8 ++++---- lib/librte_eal/common/eal_common_devargs.c | 18 +++++++++--------- lib/librte_eal/common/eal_common_options.c | 4 ++-- lib/librte_eal/common/eal_common_vdev.c | 4 ++-- lib/librte_eal/common/include/rte_devargs.h | 18 +++++++++--------- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 12 ++++++------ lib/librte_ether/rte_ethdev.c | 2 +- test/test/commands.c | 2 +- 12 files changed, 44 insertions(+), 44 deletions(-)
app/test-pmd/cmdline.c +1 -1
@@ -8322,7 +8322,7 @@ static void cmd_dump_parsed(void *parsed_result,else if (!strcmp (res->dump,"dump_mempool" ))rte_mempool_list_dump (stdout);else if (!strcmp (res->dump,"dump_devargs" )) -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);else if (!strcmp (res->dump,"dump_log_types" ))rte_log_dump (stdout); }
drivers/net/failsafe/failsafe_args.c +2 -2
@@ -93,10 +93,10 @@ fs_parse_device(struct sub_device *sdev, char *args) bus =rte_bus_find_by_device_name (args);if (bus ==NULL )/* args may contain the bus name */ - ret =rte_eal_devargs_parse (d,"%s" , args); + ret =rte_devargs_parse (d,"%s" , args);else /* args is a device name */ - ret =rte_eal_devargs_parse (d,"%s:%s" , + ret =rte_devargs_parse (d,"%s:%s" , bus->name, args);if (ret) {DEBUG ("devargs parsing failed with code %d" , ret);
examples/bond/main.c +1 -1
@@ -744,7 +744,7 @@ main(int argc, char *argv[])/* init EAL */ ret =rte_eal_init (argc, argv); -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);if (ret <0 )rte_exit (EXIT_FAILURE ,"Error with EAL initialization\n" ); argc -= ret;
lib/librte_eal/bsdapp/eal/rte_eal_version.map +6 -6
@@ -19,8 +19,6 @@ DPDK_2.0 { rte_dump_tailq; rte_eal_alarm_cancel; rte_eal_alarm_set; - rte_eal_devargs_add; - rte_eal_devargs_dump; rte_eal_get_configuration; rte_eal_get_lcore_state; rte_eal_get_physmem_layout;
@@ -186,10 +184,12 @@ DPDK_17.08 { EXPERIMENTAL { global: - rte_eal_devargs_insert; - rte_eal_devargs_next; - rte_eal_devargs_parse; - rte_eal_devargs_remove; + rte_devargs_add; + rte_devargs_dump; + rte_devargs_insert; + rte_devargs_next; + rte_devargs_parse; + rte_devargs_remove; rte_eal_hotplug_add; rte_eal_hotplug_remove; rte_service_component_register;
lib/librte_eal/common/eal_common_dev.c +4 -4
@@ -151,12 +151,12 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,if (da ==NULL )return -ENOMEM ; - ret =rte_eal_devargs_parse (da,"%s:%s,%s" , + ret =rte_devargs_parse (da,"%s:%s,%s" , busname, devname, devargs);if (ret) goto err_devarg; - ret =rte_eal_devargs_insert (da); + ret =rte_devargs_insert (da);if (ret) goto err_devarg;
@@ -181,7 +181,7 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,return 0 ; err_devarg: -if (rte_eal_devargs_remove (busname, devname)) { +if (rte_devargs_remove (busname, devname)) {free (da->args);free (da); }
@@ -216,6 +216,6 @@ int rte_eal_hotplug_remove(const char *busname, const char *devname)if (ret)RTE_LOG (ERR ,EAL ,"Driver cannot detach the device (%s)\n" , dev->name); -rte_eal_devargs_remove (busname, devname); +rte_devargs_remove (busname, devname);return ret; }
lib/librte_eal/common/eal_common_devargs.c +9 -9
@@ -60,7 +60,7 @@ bus_name_cmp(const struct rte_bus *bus, const void *name) }int -rte_eal_devargs_parse (struct rte_devargs *da,const char *format, ...) +rte_devargs_parse (struct rte_devargs *da,const char *format, ...) {struct rte_bus *bus =NULL ;va_list ap;
@@ -112,11 +112,11 @@ rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...) }int -rte_eal_devargs_insert (struct rte_devargs *da) +rte_devargs_insert (struct rte_devargs *da) {int ret; - ret =rte_eal_devargs_remove (da->bus->name, da->name); + ret =rte_devargs_remove (da->bus->name, da->name);if (ret <0 )return ret;TAILQ_INSERT_TAIL (&devargs_list, da, next);
@@ -124,7 +124,7 @@ rte_eal_devargs_insert(struct rte_devargs *da) }int -rte_eal_devargs_add (const char *dev) +rte_devargs_add (const char *dev) {struct rte_devargs *devargs =NULL ;
@@ -132,9 +132,9 @@ rte_eal_devargs_add(const char *dev)if (devargs ==NULL ) goto fail; -if (rte_eal_devargs_parse (devargs,"%s" , dev)) +if (rte_devargs_parse (devargs,"%s" , dev)) goto fail; -if (rte_eal_devargs_insert (devargs)) +if (rte_devargs_insert (devargs)) goto fail;return 0 ;
@@ -148,7 +148,7 @@ rte_eal_devargs_add(const char *dev) }int -rte_eal_devargs_remove (const char *busname,const char *devname) +rte_devargs_remove (const char *busname,const char *devname) {struct rte_devargs *d;void *tmp;
@@ -167,7 +167,7 @@ rte_eal_devargs_remove(const char *busname, const char *devname)/* dump the user devices on the console */ void -rte_eal_devargs_dump (FILE *f) +rte_devargs_dump (FILE *f) {struct rte_devargs *devargs;
@@ -181,7 +181,7 @@ rte_eal_devargs_dump(FILE *f)/* bus-aware rte_devargs iterator. */ struct rte_devargs * -rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +rte_devargs_next (const char *busname,const struct rte_devargs *start) {struct rte_devargs *da;
lib/librte_eal/common/eal_common_options.c +2 -2
@@ -181,7 +181,7 @@ eal_option_device_parse(void) TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {if (ret ==0 ) { - ret =rte_eal_devargs_add (devopt->arg); + ret =rte_devargs_add (devopt->arg);if (ret)RTE_LOG (ERR ,EAL ,"Unable to parse device '%s'\n" , devopt->arg);
@@ -1125,7 +1125,7 @@ eal_parse_common_option(int opt, const char *optarg,struct rte_devargs da;int ret; -if (rte_eal_devargs_parse (&da, optarg) <0 ) +if (rte_devargs_parse (&da, optarg) <0 )return -1 ; ret =rte_bus_probe_mode_set (da.bus->name,RTE_BUS_PROBE_WHITELIST );
lib/librte_eal/common/eal_common_vdev.c +2 -2
@@ -192,7 +192,7 @@ rte_vdev_init(const char *name, const char *args) goto fail; } -rte_eal_devargs_insert (devargs); +rte_devargs_insert (devargs);TAILQ_INSERT_TAIL (&vdev_device_list, dev, next);return 0 ;
@@ -242,7 +242,7 @@ rte_vdev_uninit(const char *name)TAILQ_REMOVE (&vdev_device_list, dev, next); -rte_eal_devargs_remove (devargs->bus->name, devargs->name); +rte_devargs_remove (devargs->bus->name, devargs->name);free (dev);return 0 ;
lib/librte_eal/common/include/rte_devargs.h +9 -9
@@ -92,8 +92,8 @@ struct rte_devargs {* - Negative errno on error. */ int -rte_eal_devargs_parse (struct rte_devargs *da, -const char *format, ...); +rte_devargs_parse (struct rte_devargs *da, +const char *format, ...);/** * Insert an rte_devargs in the global list.
@@ -106,7 +106,7 @@ rte_eal_devargs_parse(struct rte_devargs *da,* - Negative on error. */ int -rte_eal_devargs_insert (struct rte_devargs *da); +rte_devargs_insert (struct rte_devargs *da);/** * Add a device to the user device list
@@ -127,7 +127,7 @@ rte_eal_devargs_insert(struct rte_devargs *da);* - 0 on success * - A negative value on error */ -int rte_eal_devargs_add (const char *devargs_str); +int rte_devargs_add (const char *devargs_str);/** * Remove a device from the user device list.
@@ -145,7 +145,7 @@ int rte_eal_devargs_add(const char *devargs_str);* <0 on error. * >0 if the devargs was not within the user device list. */ -int rte_eal_devargs_remove (const char *busname,const char *devname); +int rte_devargs_remove (const char *busname,const char *devname);/** * This function dumps the list of user device and their arguments.
@@ -153,7 +153,7 @@ int rte_eal_devargs_remove(const char *busname, const char *devname);* @param f * A pointer to a file for output */ -void rte_eal_devargs_dump (FILE *f); +void rte_devargs_dump (FILE *f);/** * Find next rte_devargs matching the provided bus name.
@@ -171,15 +171,15 @@ void rte_eal_devargs_dump(FILE *f);* NULL if there is none. */ struct rte_devargs * -rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); +rte_devargs_next (const char *busname,const struct rte_devargs *start);/** * Iterate over all rte_devargs for a specific bus. */ #define RTE_EAL_DEVARGS_FOREACH (busname, da) \ - for (da = rte_eal_devargs_next(busname, NULL); \ + for (da = rte_devargs_next(busname, NULL); \ da != NULL; \ - da = rte_eal_devargs_next(busname, da)) \ + da = rte_devargs_next(busname, da)) \ #ifdef __cplusplus }
lib/librte_eal/linuxapp/eal/rte_eal_version.map +6 -6
@@ -19,8 +19,6 @@ DPDK_2.0 { rte_dump_tailq; rte_eal_alarm_cancel; rte_eal_alarm_set; - rte_eal_devargs_add; - rte_eal_devargs_dump; rte_eal_get_configuration; rte_eal_get_lcore_state; rte_eal_get_physmem_layout;
@@ -190,10 +188,12 @@ DPDK_17.08 { EXPERIMENTAL { global: - rte_eal_devargs_insert; - rte_eal_devargs_next; - rte_eal_devargs_parse; - rte_eal_devargs_remove; + rte_devargs_add; + rte_devargs_dump; + rte_devargs_insert; + rte_devargs_next; + rte_devargs_parse; + rte_devargs_remove; rte_eal_hotplug_add; rte_eal_hotplug_remove; rte_service_component_register;
lib/librte_ether/rte_ethdev.c +1 -1
@@ -386,7 +386,7 @@ rte_eth_dev_attach(const char *devstr, uint16_t *port_id) }/* parse device, then retrieve device name and args */ -if (rte_eal_devargs_parse (&devargs,"%s" , devstr)) +if (rte_devargs_parse (&devargs,"%s" , devstr)) goto err; name = devargs.name; args = devargs.args;
test/test/commands.c +1 -1
@@ -157,7 +157,7 @@ static void cmd_dump_parsed(void *parsed_result,else if (!strcmp (res->dump,"dump_mempool" ))rte_mempool_list_dump (stdout);else if (!strcmp (res->dump,"dump_devargs" )) -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);else if (!strcmp (res->dump,"dump_log_types" ))rte_log_dump (stdout); }
Signature · 1 line
2.1.4
PATCH v2 18/18 doc: remove devargs deprecation notices
In reply to Gaetan RivetThese actions have been enacted.
--- doc/guides/rel_notes/deprecation.rst | 13 ------------- 1 file changed, 13 deletions(-)
doc/guides/rel_notes/deprecation.rst +0 -13
@@ -16,19 +16,6 @@ Deprecation Notices - ``rte_set_log_type``, replaced by ``rte_log_set_level`` - ``rte_get_log_type``, replaced by ``rte_log_get_level`` -* eal: several API and ABI changes are planned for ``rte_devargs`` in v17.11. - The format of device command line parameters will change. The bus will need - to be explicitly stated in the device declaration. The enum ``rte_devtype`` - was used to identify a bus and will disappear. - The structure ``rte_devargs`` will change. - The ``rte_devargs_list`` will be made private. - The following functions are deprecated starting from 17.08 and will either be - modified or removed in 17.11: - - - ``rte_eal_devargs_add`` - - ``rte_eal_devargs_type_count`` - - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse`` - * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA address translation scheme. Reference to phys address in EAL data-structure or functions may change to
Signature · 1 line
2.1.4
Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
Quoted 10 lines
-----Original Message-----
From: dev [mailto:[email protected]] On Behalf Of Gaetan Rivet
Sent: Thursday, October 12, 2017 9:21 AM
To: [email protected]
Cc: Gaetan Rivet <[email protected]>
Subject: [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices
Quoted 3 lines
These actions have been enacted.
Signed-off-by Gaetan Rivet <[email protected]>
Re: [PATCH v2 11/18] devargs: simplify implementation
In reply to Gaetan RivetHello Gaetan,
Quoted 12 lines · Gaetan Rivet
Re-use existing code, remove incorrect comments.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_devargs.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-)lib/librte_eal/common/eal_common_devargs.c +0
-0
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_insert(struct rte_devargs *da) · line 155
Quoted 3 lines
lib/librte_eal/common/eal_common_devargs.c +0
-0@@ -153,21 +153,19 @@ rte_eal_devargs_insert(struct rte_devargs *da)return 0 ; }
While trying to work on this patch, I noticed that the complete series (including "Move PCI away from EAL") is not cleanly applicable on current master (17.11 RC1). I thought it would be some tiny issues.
But there are some issues which I couldn't pass, Like...
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_insert(struct rte_devargs *da) · line 156
Quoted 2 lines
lib/librte_eal/common/eal_common_devargs.c +0
-1-/* store a whitelist parameter for later parsing */ int
In the this function
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_insert(struct rte_devargs *da) · line 166
Quoted 13 lines
lib/librte_eal/common/eal_common_devargs.c +1
-3-rte_eal_devargs_add(const char *devargs_str) +rte_eal_devargs_add(const char *dev) {struct rte_devargs *devargs =NULL ; -const char *dev = devargs_str; -/* use calloc instead of rte_zmalloc as it's called early at init */ devargs =calloc (1 ,sizeof (*devargs));if (devargs ==NULL ) goto fail;if (rte_eal_devargs_parse (devargs,"%s" , dev)) goto fail;
These lines don't exist in your patch
---59c2ba6c 172) if (bus->conf.probe_mode == RTE_BUS_PROBE_UNDEFINED) {b631f3b0 173) if (devargs->policy == RTE_DEV_WHITELISTED)59c2ba6c 174) bus->conf.probe_mode = RTE_BUS_PROBE_WHITELIST;b631f3b0 175) else if (devargs->policy == RTE_DEV_BLACKLISTED)59c2ba6c 176) bus->conf.probe_mode = RTE_BUS_PROBE_BLACKLIST;
02823c1d 177) } bf6dea0e 178) TAILQ_INSERT_TAIL(&devargs_list, devargs, next); bf6dea0e 179) return 0;
0215a4c6 180)
---
(Some introduced by the move PCI series, but others like b631f3b0 are very old ~17.08)
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_insert(struct rte_devargs *da) · line 168
Quoted 3 lines
lib/librte_eal/common/eal_common_devargs.c +2
-1-TAILQ_INSERT_TAIL (&devargs_list, devargs, next); +if (rte_eal_devargs_insert (devargs)) + goto fail;
And hence, I don't know whether you intend to insert the above line after or before checking PROBE.
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_insert(struct rte_devargs *da) · line 171
Quoted 4 lines
lib/librte_eal/common/eal_common_devargs.c +0
-0return 0 ; fail:
Maybe I am doing something wrong here - any ideas? Can you send an updated/rebased version on current master HEAD?
-
Shreyansh
Re: [PATCH v2 11/18] devargs: simplify implementation
Hello Gaetan,
Please ignore this email (reason inline)
Quoted 59 lines · Shreyansh Jain
Hello Gaetan,
Quoted 12 lines · Gaetan Rivet
Re-use existing code, remove incorrect comments.
Signed-off-by Gaetan Rivet <[email protected]>---
lib/librte_eal/common/eal_common_devargs.c| 8 +++-----1 file changed, 3 insertions(+), 5 deletions(-)lib/librte_eal/common/eal_common_devargs.c +0
-0lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_insert(struct rte_devargs *da) · line 154
Quoted 2 lines
lib/librte_eal/common/eal_common_devargs.c +0
-0@@ -153,21 +153,19 @@ rte_eal_devargs_insert(struct rte_devargs *da) return 0; }While trying to work on this patch, I noticed that the complete series (including "Move PCI away from EAL") is not cleanly applicable on current master (17.11 RC1). I thought it would be some tiny issues.
But there are some issues which I couldn't pass, Like...
Quoted 2 lines
lib/librte_eal/common/eal_common_devargs.c +0
-1-/* store a whitelist parameter for later parsing */ int
In the this function
Quoted 11 lines
lib/librte_eal/common/eal_common_devargs.c +1
-1-rte_eal_devargs_add(const char *devargs_str) +rte_eal_devargs_add(const char *dev){
structrte_devargs*devargs = NULL;
- const char *dev =devargs_str;
- /* use calloc instead ofrte_zmallocas it's called early at init */ devargs =calloc(1, sizeof(*devargs));
if (devargs == NULL)
goto fail;
if (rte_eal_devargs_parse(devargs, "%s", dev))
goto fail;These lines don't exist in your patch
---
59c2ba6c172) if (bus->conf.probe_mode==RTE_BUS_PROBE_UNDEFINED) {b631f3b0173) if (devargs->policy==RTE_DEV_WHITELISTED)59c2ba6c174)bus->conf.probe_mode=RTE_BUS_PROBE_WHITELIST;b631f3b0175) else if (devargs->policy==RTE_DEV_BLACKLISTED)59c2ba6c176)bus->conf.probe_mode=RTE_BUS_PROBE_BLACKLIST;02823c1d177) }bf6dea0e178)TAILQ_INSERT_TAIL(&devargs_list, devargs, next);bf6dea0e179) return 0;0215a4c6180)
---
(Some introduced by the move PCI series, but others likeb631f3b0are very old ~17.08)Quoted 3 lines
-
TAILQ_INSERT_TAIL(&devargs_list, devargs, next); + if (rte_eal_devargs_insert(devargs))
+ goto fail;
Quoted 7 lines
And hence, I don't know whether you intend to insert the above line after or before checking PROBE.
Quoted 3 lines
return 0;
fail:Maybe I am doing something wrong here - any ideas? Can you send an updated/rebased version on current master HEAD?
Just after sending this, I noticed that I had not applied the "Bus control framework" patch set which the "devargs..." cover letter talks about. I will try with that and confirm if there is still any issue.
Quoted 4 lines
-
Shreyansh
Re: [PATCH v2 11/18] devargs: simplify implementation
Quoted 4 lines · Shreyansh Jain
Hello Gaetan,
Please ignore this email (reason inline)
Hello Shreyansh,
Thanks for reading this patchset, and sorry about the confusion (the previous cover-letter only listed the PCI bus move, the bus control was only added in this version).
I'd be very happy to see these series integrated for this release or the next, and as such your opinion would matter a great deal.
However, I have found important issues that I am still working on with the PCI bus move. I am trying to fix this in time, but I'm not sure yet to succeed soon enough.
It would block the rest. I could still redo both series without the PCI bus move but the bus control scheme is necessary for the devargs cleanup.
I don't know if this bus control scheme is interesting enough though. I need additional opinions about it.
Anyway, thanks,
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v2 11/18] devargs: simplify implementation
Hello Gaetan,
Quoted 11 lines · Gaëtan Rivet
Quoted 4 lines · Shreyansh Jain
Hello Gaetan,
Please ignore this email (reason inline)
Hello Shreyansh,
Thanks for reading this patchset, and sorry about the confusion (the previous cover-letter only listed the PCI bus move, the bus control was only added in this version).
That's OK. I mixed up the cover-letters anyway.
Probably the complete series needs a rebase as some patches have changes exactly same locations in EAL where this patch impacts (at least for PCI movement patches, I found patches had fuzz factor)
Quoted 3 lines
I'd be very happy to see these series integrated for this release or the next, and as such your opinion would matter a great deal.
I do understand the importance of this patch series - but I have not been able to devote enough time to this lately. Even now, I am not sure if I would be able to completely review all within 17.11 timeframe as it is quite big. (And I have some personal time-off coming up soon!)
Quoted 4 lines
However, I have found important issues that I am still working on with the PCI bus move. I am trying to fix this in time, but I'm not sure yet to succeed soon enough.
I understand.
It is a complex series - specially the build break in PCI movement on per patch basis - that would be difficult to solve. Probably that requires adding dummy functions/variables to allow compilation and then moving them out. It is indeed tough choice.
Quoted 4 lines
It would block the rest. I could still redo both series without the PCI bus move but the bus control scheme is necessary for the devargs cleanup.
Probably it is best to postpone for 18.02. It indeed is difficult to let a proposal fall through a planned release - but this work is really critical as it impacts a lot of people (PCI, Buses, args etc). Just my personal opinion.
Quoted 3 lines
I don't know if this bus control scheme is interesting enough though. I need additional opinions about it.
I haven't started reading the Bus control yet - I am still focusing on PCI movement and then I had planned the devargs. But, I will start looking into this as well. I would try to add whatever opinion I can.
Quoted 3 lines
Anyway, thanks,
Always welcome! But, that's what the community is for. :D
-
Shreyansh
Re: [PATCH v2 02/18] eal: remove generic devtype
In reply to Gaetan RivetQuoted 40 lines · Gaetan Rivet
The devtype is now entirely defined by the device bus. As such, it is already characterized by the bus identifier within an
rte_devargs.The
rte_devtypeenum can disappear, along with crutches added during this transition.
rte_eal_devargs_type_countbecomes useless and is removed.Signed-off-by Gaetan Rivet <[email protected]>--- drivers/bus/pci/pci_common.c | 16 ++------------ lib/librte_eal/bsdapp/eal/rte_eal_version.map | 1 - lib/librte_eal/common/eal_common_devargs.c | 20 +---------------- lib/librte_eal/common/eal_common_options.c | 19 +++++----------- lib/librte_eal/common/include/rte_dev.h | 8 ------- lib/librte_eal/common/include/rte_devargs.h | 29 +------------------------ lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 - 7 files changed, 9 insertions(+), 85 deletions(-)drivers/bus/pci/pci_common.c +0
-9@@ -172,15 +172,6 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr, loc->domain, loc->bus, loc->devid, loc->function, dev->device.numa_node); -/* no initialization when blacklisted, return without error */ -if (dev->device.devargs !=NULL && - dev->device.devargs->policy == -RTE_DEV_BLACKLISTED ) { -RTE_LOG (INFO ,EAL ," Device is blacklisted, not" -" initializing\n" ); -return 1 ; - } -if (dev->device.numa_node <0 ) {RTE_LOG (WARNING ,EAL ," Invalid NUMA socket, default to 0\n" ); dev->device.numa_node =0 ;
drivers/bus/pci/pci_common.c rte_pci_probe(void) · line 375
Quoted 10 lines
drivers/bus/pci/pci_common.c +2
-5@@ -380,11 +371,8 @@ rte_pci_probe(void) probed++; devargs = dev->device.devargs; -/* probe all or only whitelisted devices */ -if (probe_all) - ret =pci_probe_all_drivers (dev); -else if (devargs !=NULL && - devargs->policy ==RTE_DEV_WHITELISTED ) +/* probe all or only declared devices */ +if (probe_all ^ (devargs !=NULL ))
What is the intent of this? If probe_all is true, and devargs != null, I think this branch isn't taken.
Shouldn't this be ||? Maybe I missed something?
Quoted 232 lines
drivers/bus/pci/pci_common.c +0
-0ret =pci_probe_all_drivers (dev);if (ret <0 ) { RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMTlib/librte_eal/bsdapp/eal/rte_eal_version.map +0
-1@@ -22,7 +22,6 @@ DPDK_2.0 { rte_eal_alarm_set; rte_eal_devargs_add; rte_eal_devargs_dump; - rte_eal_devargs_type_count; rte_eal_get_configuration; rte_eal_get_lcore_state; rte_eal_get_physmem_layout;lib/librte_eal/common/eal_common_devargs.c +1
-19@@ -153,7 +153,7 @@ rte_eal_devargs_insert(struct rte_devargs *da)/* store a whitelist parameter for later parsing */ int -rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) +rte_eal_devargs_add (const char *devargs_str) {struct rte_devargs *devargs =NULL ;const char *dev = devargs_str;@@ -165,9 +165,6 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)if (rte_eal_devargs_parse (dev, devargs)) goto fail; - devargs->type = devtype; -if (devargs->type ==RTE_DEVTYPE_BLACKLISTED_PCI ) - devargs->policy =RTE_DEV_BLACKLISTED ;TAILQ_INSERT_TAIL (&devargs_list, devargs, next);return 0 ;@@ -198,21 +195,6 @@ rte_eal_devargs_remove(const char *busname, const char *devname)return 1 ; } -/* count the number of devices of a specified type */ -unsigned int -rte_eal_devargs_type_count (enum rte_devtype devtype) -{ -struct rte_devargs *devargs; -unsigned int count =0 ; - - TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->type != devtype) -continue ; - count++; - } -return count; -} -/* dump the user devices on the console */ void rte_eal_devargs_dump (FILE *f)lib/librte_eal/common/eal_common_options.c +5
-14@@ -131,7 +131,6 @@ TAILQ_HEAD(device_option_list, device_option);struct device_option {TAILQ_ENTRY (device_option) next; -enum rte_devtype type;char arg[]; };@@ -143,8 +142,7 @@ static int mem_parsed;static int core_parsed;static int -eal_option_device_add (enum rte_devtype type, -const char *busname,const char *optarg) +eal_option_device_add (const char *busname,const char *optarg) {struct device_option *devopt;size_t optlen;@@ -159,7 +157,6 @@ eal_option_device_add(enum rte_devtype type,return -ENOMEM ; } - devopt->type = type;if (busname !=NULL ) ret =snprintf (devopt->arg, optlen,"%s:%s" , busname, optarg);@@ -183,7 +180,7 @@ eal_option_device_parse(void) TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {if (ret ==0 ) { - ret =rte_eal_devargs_add (devopt->type, devopt->arg); + ret =rte_eal_devargs_add (devopt->arg);if (ret)RTE_LOG (ERR ,EAL ,"Unable to parse device '%s'\n" , devopt->arg);@@ -1009,19 +1006,15 @@ eal_parse_common_option(int opt, const char *optarg,case 'b' :if (rte_bus_probe_mode_set ("pci" ,RTE_BUS_PROBE_BLACKLIST ) <0 )return -1 ; -if (eal_option_device_add (RTE_DEVTYPE_BLACKLISTED_PCI , -"pci" , optarg) <0 ) { +if (eal_option_device_add ("pci" , optarg) <0 )return -1 ; - }break ;/* whitelist */ case 'w' :if (rte_bus_probe_mode_set ("pci" ,RTE_BUS_PROBE_WHITELIST ) <0 )return -1 ; -if (eal_option_device_add (RTE_DEVTYPE_WHITELISTED_PCI , -"pci" , optarg) <0 ) { +if (eal_option_device_add ("pci" , optarg) <0 )return -1 ; - }break ;/* coremask */ case 'c' :@@ -1128,10 +1121,8 @@ eal_parse_common_option(int opt, const char *optarg,break ;case OPT_VDEV_NUM : -if (eal_option_device_add (RTE_DEVTYPE_VIRTUAL , -"vdev" , optarg) <0 ) { +if (eal_option_device_add ("vdev" , optarg) <0 )return -1 ; - }break ;case OPT_SYSLOG_NUM :lib/librte_eal/common/include/rte_dev.h +0
-8@@ -127,14 +127,6 @@ enum rte_kernel_driver { };/** -* Device policies. -*/ -enum rte_dev_policy { -RTE_DEV_WHITELISTED , -RTE_DEV_BLACKLISTED , -}; - -/** * A generic memory resource representation. */ struct rte_mem_resource {lib/librte_eal/common/include/rte_devargs.h +1
-28@@ -53,15 +53,6 @@ extern "C" {#include <rte_bus.h> /** -* Type of generic device -*/ -enum rte_devtype { -RTE_DEVTYPE_WHITELISTED_PCI , -RTE_DEVTYPE_BLACKLISTED_PCI , -RTE_DEVTYPE_VIRTUAL , -}; - -/** * Structure that stores a device given by the user with its arguments * * A user device is a physical or a virtual device given by the user to @@ -74,10 +65,6 @@ enum rte_devtype {struct rte_devargs {/** Next in list. */ TAILQ_ENTRY (rte_devargs) next; -/** Type of device. */ -enum rte_devtype type; -/** Device policy. */ -enum rte_dev_policy policy;/** Bus handle for the device. */ struct rte_bus *bus;/** Name of the device. */ @@ -166,8 +153,6 @@ rte_eal_devargs_insert(struct rte_devargs *da);* driver name is not checked by this function, it is done when probing * the drivers. * -* @param devtype -* The type of the device. * @param devargs_str * The arguments as given by the user. * @@ -175,7 +160,7 @@ rte_eal_devargs_insert(struct rte_devargs *da);* - 0 on success * - A negative value on error */ -int rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str); +int rte_eal_devargs_add (const char *devargs_str);/** * Remove a device from the user device list. @@ -196,18 +181,6 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str);int rte_eal_devargs_remove (const char *busname,const char *devname);/** -* Count the number of user devices of a specified type -* -* @param devtype -* The type of the devices to counted. -* -* @return -* The number of devices. -*/ -unsigned int -rte_eal_devargs_type_count (enum rte_devtype devtype); - -/** * This function dumps the list of user device and their arguments. * * @param f lib/librte_eal/linuxapp/eal/rte_eal_version.map +0
-1@@ -22,7 +22,6 @@ DPDK_2.0 { rte_eal_alarm_set; rte_eal_devargs_add; rte_eal_devargs_dump; - rte_eal_devargs_type_count; rte_eal_get_configuration; rte_eal_get_lcore_state; rte_eal_get_physmem_layout;
Quoted 23 lines · Gaetan Rivet
The use of
rte_devargsis inconsistent in the light of new functionalities such as device hotplug.Most of its API is still experimental and needs stabilization. Older functions were deprecated and need to be rewritten or removed. The
rte_devtypeis meant to disappear.v2:
Big rework.
* Enact requiring bus name prepended in
rte_devargsparsing functions.
* Removerte_devtype. Use new probe mode setter along with generic bus reference withinrte_devargs.
* Rework devargs parsing API.
The function is now variadic, does not enforce bus rules on the devargs being inserted as the bus has been configured previously. Old parsing function is removed.
* Expose bus guessing from device name.
This uses the "parse" bus operator, which may be meant to disappear. This is optional, but nice to have in a transition period.
* Introduce new --dev generic device declaration parameter.
Quoted 1 line
This patchset depends on:
It is weird to me that you introduce patch sets, and then introduce cleanups later? Shouldn't we revise the existing patchsets?
Maybe I missed a discussion somewhere?
Quoted 48 lines
Move PCI away from the EAL
http://dpdk.org/ml/archives/dev/2017-August/073512.htmlBus control framework
http://dpdk.org/ml/archives/dev/2017-October/078752.htmlGaetan Rivet (18):
eal: prepend busname on legacy device declaration
eal: remove generic devtype
devargs: introduce iterator
devargs: introduce foreach macro
vdev: do not reference devargs list bus/pci: do not reference devargs list
test: remove devargs unit tests
devargs: make devargs list private
devargs: make parsing variadic
devargs: require bus name prefix
devargs: simplify implementation
eal: add generic device declaration parameter
bus: make device recognition function public net/failsafe: keep legacy sub-device declaration
ether: use new devargs parsing function
devargs: remove old devargs parsing function
devargs: use proper prefix
doc: remove devargs deprecation noticesMAINTAINERS | 1 - app/test-pmd/cmdline.c | 2 +- doc/guides/rel_notes/deprecation.rst | 13 --- drivers/bus/pci/pci_common.c | 22 +--- drivers/net/failsafe/failsafe_args.c | 11 +- examples/bond/main.c | 2 +- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 15 ++- lib/librte_eal/common/eal_common_dev.c | 39 ++----- lib/librte_eal/common/eal_common_devargs.c | 129 +++++++++-------------- lib/librte_eal/common/eal_common_options.c | 47 ++++++--- lib/librte_eal/common/eal_common_vdev.c | 11 +- lib/librte_eal/common/eal_options.h | 2 + lib/librte_eal/common/include/rte_bus.h | 12 +++ lib/librte_eal/common/include/rte_dev.h | 8 -- lib/librte_eal/common/include/rte_devargs.h | 120 ++++++++-------------- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 15 ++- lib/librte_ether/rte_ethdev.c | 11 +- test/test/Makefile | 1 - test/test/commands.c | 2 +- test/test/test_devargs.c | 131 ------------------------ 20 files changed, 186 insertions(+), 408 deletions(-) delete mode 100644 test/test/test_devargs.c
Re: [PATCH v2 02/18] eal: remove generic devtype
In reply to Aaron ConoleHello Aaron,
Quoted 12 lines · Aaron Conole
Quoted 10 lines · Gaetan Rivet
The devtype is now entirely defined by the device bus. As such, it is already characterized by the bus identifier within an
rte_devargs.The
rte_devtypeenum can disappear, along with crutches added during this transition.
rte_eal_devargs_type_countbecomes useless and is removed.Signed-off-by Gaetan Rivet <[email protected]>---
< ... >
Quoted 15 lines
drivers/bus/pci/pci_common.c rte_pci_probe(void) · line 375
Quoted 11 lines
drivers/bus/pci/pci_common.c +2
-5@@ -380,11 +371,8 @@ rte_pci_probe(void) probed++; devargs = dev->device.devargs; -/* probe all or only whitelisted devices */ -if (probe_all) - ret =pci_probe_all_drivers (dev); -else if (devargs !=NULL && - devargs->policy ==RTE_DEV_WHITELISTED ) +/* probe all or only declared devices */ +if (probe_all ^ (devargs !=NULL ))What is the intent of this? If
probe_allis true, and devargs != null, I think this branch isn't taken.
Quoted 2 lines
Shouldn't this be ||? Maybe I missed something?
Here are the possible choices:
probe_all \ devargs !NULL NULL
--------------------------+----------------------+----------------------------+
true |blacklist and the |blacklist mode and no |
|devargs describes a |devargs, meaning the device |
|blacklisted device |is not blacklisted |
|--> do not probe |--> do probe |
--------------------------+----------------------+----------------------------+
false |whitelist mode and |whitelist mode and no |
|whitelisted device |devargs, device has been |
|using a devargs |scanned but not whitelisted |
|--> do probe |--> do not probe |--------------------------+----------------------+----------------------------+
A XOR is thus the logical expression of this decision.
What I could be doubtful about here is that using a xor anywhere can be confusing for some people and it could all be expressed in two simpler ifs.
Also, probe_all could be renamed as "blacklist_mode" for example to be more descriptive.
But unless I'm mistaken, the logic should be correct.
Quoted 3 lines
Quoted 3 lines
drivers/bus/pci/pci_common.c +0
-0ret =pci_probe_all_drivers (dev);if (ret <0 ) { RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMT
< ... >
Signature · 2 lines
Gaëtan Rivet 6WIND
Quoted 25 lines · Aaron Conole
Quoted 23 lines · Gaetan Rivet
The use of
rte_devargsis inconsistent in the light of new functionalities such as device hotplug.Most of its API is still experimental and needs stabilization. Older functions were deprecated and need to be rewritten or removed. The
rte_devtypeis meant to disappear.v2:
Big rework.
* Enact requiring bus name prepended in
rte_devargsparsing functions.
* Removerte_devtype. Use new probe mode setter along with generic bus reference withinrte_devargs.
* Rework devargs parsing API.
The function is now variadic, does not enforce bus rules on the devargs being inserted as the bus has been configured previously. Old parsing function is removed.
* Expose bus guessing from device name.
This uses the "parse" bus operator, which may be meant to disappear. This is optional, but nice to have in a transition period.
* Introduce new --dev generic device declaration parameter.
Quoted 7 lines
Quoted 1 line
This patchset depends on:
It is weird to me that you introduce patch sets, and then introduce cleanups later? Shouldn't we revise the existing patchsets?
Maybe I missed a discussion somewhere?
No discussion missed :)
The way it happened here was that I started cleaning up the mess I did last release in the rte_devargs to streamline somewhat the subsystem.
Several things needed to disappear:
1. List of devargs manipulated by buses. With hotplug feature they had to be properly managed (i.e. freed on device unplug).
2. rte_devtype, which was meant to disappear last release but was kept to smooth the transition to the new API.
3. Different usages of devargs_add among applications / PMDs There had been some different last release about the exposed API, thus I made a compromise that could potentially benefit everyone.
Points 1 and 3 did not require additional work on the buses. Point 2 however, required a way for buses to be configured in whitelist / blacklist mode. To clean up the rte_devtype, a configuration facility is necessary.
While working on this facility, I found that I was once again breaking the ABI of rte_bus, along with the IOVA configuration. I thought it could be streamlined in a more stable way, thus I wrote the separate patchset for a cleaner integration of these changes in this release.
All in all, the PCI move is not necessary but was sent a few weeks ago already and it could be skipped with a small rework.
The bus control could be done in a more hackish way as well, without having a whole separate patchset to introduce the control facility. I wanted to propose a cleaner approach that could possibly keep the rte_bus ABI stable for some time.
Quoted 48 lines
Quoted 48 lines
Move PCI away from the EAL
http://dpdk.org/ml/archives/dev/2017-August/073512.htmlBus control framework
http://dpdk.org/ml/archives/dev/2017-October/078752.htmlGaetan Rivet (18):
eal: prepend busname on legacy device declaration
eal: remove generic devtype
devargs: introduce iterator
devargs: introduce foreach macro
vdev: do not reference devargs list bus/pci: do not reference devargs list
test: remove devargs unit tests
devargs: make devargs list private
devargs: make parsing variadic
devargs: require bus name prefix
devargs: simplify implementation
eal: add generic device declaration parameter
bus: make device recognition function public net/failsafe: keep legacy sub-device declaration
ether: use new devargs parsing function
devargs: remove old devargs parsing function
devargs: use proper prefix
doc: remove devargs deprecation noticesMAINTAINERS | 1 - app/test-pmd/cmdline.c | 2 +- doc/guides/rel_notes/deprecation.rst | 13 --- drivers/bus/pci/pci_common.c | 22 +--- drivers/net/failsafe/failsafe_args.c | 11 +- examples/bond/main.c | 2 +- lib/librte_eal/bsdapp/eal/rte_eal_version.map | 15 ++- lib/librte_eal/common/eal_common_dev.c | 39 ++----- lib/librte_eal/common/eal_common_devargs.c | 129 +++++++++-------------- lib/librte_eal/common/eal_common_options.c | 47 ++++++--- lib/librte_eal/common/eal_common_vdev.c | 11 +- lib/librte_eal/common/eal_options.h | 2 + lib/librte_eal/common/include/rte_bus.h | 12 +++ lib/librte_eal/common/include/rte_dev.h | 8 -- lib/librte_eal/common/include/rte_devargs.h | 120 ++++++++-------------- lib/librte_eal/linuxapp/eal/rte_eal_version.map | 15 ++- lib/librte_ether/rte_ethdev.c | 11 +- test/test/Makefile | 1 - test/test/commands.c | 2 +- test/test/test_devargs.c | 131 ------------------------ 20 files changed, 186 insertions(+), 408 deletions(-) delete mode 100644 test/test/test_devargs.c
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v2 01/18] eal: prepend busname on legacy device declaration
In reply to Gaetan RivetOne very quick comment:
Quoted 17 lines · Gaetan Rivet
Legacy device options (-b, -w, --vdev) need to prepend their bus name to user parameters for backward compatibility.
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_options.c | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-)lib/librte_eal/common/eal_common_options.c +0
-1@@ -143,13 +143,16 @@ static int mem_parsed;static int core_parsed; static int -eal_option_device_add (enum rte_devtype type,const char *optarg)
lib/librte_eal/common/eal_common_options.c static int mem_parsed; · line 155
Quoted 10 lines
lib/librte_eal/common/eal_common_options.c +4
-0+eal_option_device_add(enum rte_devtype type, + const char *busname, const char *optarg) { struct device_option *devopt;size_t optlen;int ret; optlen =strlen (optarg) +1 ; +if (busname !=NULL ) + optlen +=strlen (optarg) +1 ;
I think you want "optlen += strlen(busname) + 1";
Quoted 3 lines
lib/librte_eal/common/eal_common_options.c +0
-0devopt =calloc (1 ,sizeof (*devopt) + optlen);if (devopt ==NULL ) {RTE_LOG (ERR ,EAL ,"Unable to allocate device option\n" );
[...]
Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
In reply to Gaetan RivetHello Gaetan,
Quoted 22 lines · Gaetan Rivet
These actions have been enacted.
Signed-off-by Gaetan Rivet <[email protected]>--- doc/guides/rel_notes/deprecation.rst | 13 ------------- 1 file changed, 13 deletions(-)doc/guides/rel_notes/deprecation.rst +0
-6@@ -16,19 +16,6 @@ Deprecation Notices - ``rte_set_log_type``, replaced by ``rte_log_set_level`` - ``rte_get_log_type``, replaced by ``rte_log_get_level`` -* eal: several API and ABI changes are planned for ``rte_devargs`` in v17.11. - The format of device command line parameters will change. The bus will need - to be explicitly stated in the device declaration. The enum ``rte_devtype`` - was used to identify a bus and will disappear. - The structure ``rte_devargs`` will change. - The ``rte_devargs_list`` will be made private.
doc/guides/rel_notes/deprecation.rst Deprecation Notices · line 21
Quoted 10 lines
doc/guides/rel_notes/deprecation.rst +0
-7- The following functions are deprecated starting from 17.08 and will either be - modified or removed in 17.11: - - - ``rte_eal_devargs_add`` - - ``rte_eal_devargs_type_count`` - - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse`` - * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA address translation scheme. Reference to phys address in EAL data-structure or functions may change to
Once this patch is formalized, the documentation reference for rte_devargs.h also needs to be changed as it still refers to RTE devargs as:
"...These devices can be PCI devices or virtual devices....".
Similarly, the rte_devargs_parse too has PCI traces.
Next step would be to remove the "pci"/"vdev" reference from rte_eal_dev_attach.
Former can be part of this series, but the later needs to be a separate patch, I think. Let me know if you want me to work on these (or later).
Other than that, I think I am OK with overall patch. If you can push the final series (I am not sure it would be with or without bus control), I can give it a spin (to vaildate if non-PCI like FSLMC bus can work fine).
Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
Hello Shreyansh,
Quoted 38 lines · Shreyansh Jain
Hello Gaetan,
Quoted 21 lines · Gaetan Rivet
These actions have been enacted.
Signed-off-by Gaetan Rivet <[email protected]>--- doc/guides/rel_notes/deprecation.rst | 13 ------------- 1 file changed, 13 deletions(-)doc/guides/rel_notes/deprecation.rst +0
-7@@ -16,19 +16,6 @@ Deprecation Notices - ``rte_set_log_type``, replaced by ``rte_log_set_level`` - ``rte_get_log_type``, replaced by ``rte_log_get_level`` -* eal: several API and ABI changes are planned for ``rte_devargs`` in v17.11. - The format of device command line parameters will change. The bus will need - to be explicitly stated in the device declaration. The enum ``rte_devtype`` - was used to identify a bus and will disappear. - The structure ``rte_devargs`` will change. - The ``rte_devargs_list`` will be made private. - The following functions are deprecated starting from 17.08 and will either bedoc/guides/rel_notes/deprecation.rst Deprecation Notices · line 21
Quoted 10 lines
doc/guides/rel_notes/deprecation.rst +0
-6- modified or removed in 17.11: - - - ``rte_eal_devargs_add`` - - ``rte_eal_devargs_type_count`` - - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse`` - * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA address translation scheme. Reference to phys address in EAL data-structure or functions may change toOnce this patch is formalized, the documentation reference for
rte_devargs.halso needs to be changed as it still refers to RTE devargs as:
Quoted 7 lines
"...These devices can be PCI devices or virtual devices....".
Similarly, the
rte_devargs_parsetoo has PCI traces.Next step would be to remove the "pci"/"vdev" reference from
rte_eal_dev_attach.
Noted, thanks.
Quoted 6 lines
Former can be part of this series, but the later needs to be a separate patch, I think. Let me know if you want me to work on these (or later).
Other than that, I think I am OK with overall patch. If you can push the final series (I am not sure it would be with or without bus control), I can give it a spin (to vaildate if non-PCI like FSLMC bus can work fine).
Indeed, I also think everything should be settled first. I have mostly finished working on this series yesterday, I will integrate your above remarks which will be short.
(Well, by finished I mean I finished the first 90%. The other 90% is still in progress...)
I removed the rte_devargs unit test and am not too happy about it. There are parsing functions there, which are extremely error-prone and would like to have at least the basis for some tests, that we could populate as we go. If I have the courage I will try to write it and send it with this series.
I would certainly appreciate if you are able to fix the pci / vdev limitation in rte_eal_dev_attach, as I am starting to be overwhelmed with work (trying to finish a lot of things before the holidays).
Thanks for the review!
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
Hello Gaetan,
Quoted 77 lines
-----Original Message-----
From: Gaëtan Rivet [mailto:[email protected]]
Sent: Wednesday, December 13, 2017 3:56 PM
To: Shreyansh Jain <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH v2 18/18] doc: remove devargs deprecation noticesHello Shreyansh,
Quoted 11 lines · Shreyansh Jain
Hello Gaetan,
Review
Quoted 8 lines · Gaetan Rivet
These actions have been enacted.
Signed-off-by Gaetan Rivet <[email protected]>--- doc/guides/rel_notes/deprecation.rst | 13 ------------- 1 file changed, 13 deletions(-)(quoted diff) +0
-0
b/doc/guides/rel_notes/deprecation.rstQuoted 7 lines
doc/guides/rel_notes/deprecation.rst Deprecation Notices · line 18
Quoted 7 lines
index ef2264f..23faa19 100644
doc/guides/rel_notes/deprecation.rst +0
-1@@ -16,19 +16,6 @@ Deprecation Notices - ``rte_set_log_type``, replaced by ``rte_log_set_level`` - ``rte_get_log_type``, replaced by ``rte_log_get_level`` -* eal: several API and ABI changes are planned for ``rte_devargs`` inv17.11.
Quoted 1 line
doc/guides/rel_notes/deprecation.rst Deprecation Notices · line 19
Quoted 1 line
doc/guides/rel_notes/deprecation.rst +0
-1- The format of device command line parameters will change. The bus willneed
Quoted 1 line
doc/guides/rel_notes/deprecation.rst Deprecation Notices · line 20
Quoted 1 line
doc/guides/rel_notes/deprecation.rst +0
-1- to be explicitly stated in the device declaration. The enum``rte_devtype``
Quoted 4 lines
doc/guides/rel_notes/deprecation.rst Deprecation Notices
Quoted 4 lines
doc/guides/rel_notes/deprecation.rst +0
-4- was used to identify a bus and will disappear. - The structure ``rte_devargs`` will change. - The ``rte_devargs_list`` will be made private. - The following functions are deprecated starting from 17.08 and willeither be
Quoted 7 lines
doc/guides/rel_notes/deprecation.rst Deprecation Notices
Quoted 7 lines
doc/guides/rel_notes/deprecation.rst +0
-6- modified or removed in 17.11: - - - ``rte_eal_devargs_add`` - - ``rte_eal_devargs_type_count`` - - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse`` - * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVAaddress
Quoted 2 lines
doc/guides/rel_notes/deprecation.rst Deprecation Notices · line 20
Quoted 2 lines
doc/guides/rel_notes/deprecation.rst +0
-0translation scheme. Reference to phys address in EAL data-structure or functions maychange to
Quoted 3 lines
Quoted 1 line
Once this patch is formalized, the documentation reference for
rte_devargs.hQuoted 9 lines
also needs to be changed as it still refers to RTE devargs as:
"...These devices can be PCI devices or virtual devices....".
Similarly, the
rte_devargs_parsetoo has PCI traces.Next step would be to remove the "pci"/"vdev" reference from
rte_eal_dev_attach.Noted, thanks.
Quoted 6 lines
Former can be part of this series, but the later needs to be a separate patch, I think. Let me know if you want me to work on these (or later).
Other than that, I think I am OK with overall patch. If you can push the final series (I am not sure it would be with or without bus control), I can give it a spin (to vaildate if non-PCI like FSLMC bus can work fine).
Indeed, I also think everything should be settled first. I have mostly finished working on this series yesterday, I will integrate your above remarks which will be short.
Quoted 8 lines
(Well, by finished I mean I finished the first 90%. The other 90% is still in progress...)
I removed the
rte_devargsunit test and am not too happy about it. There are parsing functions there, which are extremely error-prone and would like to have at least the basis for some tests, that we could populate as we go. If I have the courage I will try to write it and send it with this series.
While reading through the code, I also had the same feeling - there can be corner cases in the parsing functions which I can't imagine. Anyways, those need to be runtime-verified - static reviews may not suffice.
Quoted 4 lines
I would certainly appreciate if you are able to fix the pci / vdev limitation in
rte_eal_dev_attach, as I am starting to be overwhelmed with work (trying to finish a lot of things before the holidays).
OK.
Once you give the devargs a push, I will start work on the PCI removal from rte_eal_dev_attach. Before that, I just want to be sure of devargs with non-PCI bus (non hotplug case).
And, thanks for tons of work you are handling. I saw the patches and really appreciate how you have split things up in sequential manner per-patch. It is difficult.
Quoted 6 lines
Thanks for the review!
--
Gaëtan Rivet
6WIND
Re: [PATCH v2 12/18] eal: add generic device declaration parameter
In reply to Gaetan RivetQuoted 4 lines · Gaetan Rivet
Add a new generic device declaration parameter:
--dev=<device_declaration>
[...]
Quoted 20 lines
lib/librte_eal/common/eal_common_options.c +4
-0@@ -95,6 +95,7 @@ eal_long_options[] = { {OPT_PROC_TYPE, 1, NULL, OPT_PROC_TYPE_NUM }, {OPT_SOCKET_MEM, 1, NULL, OPT_SOCKET_MEM_NUM }, {OPT_SYSLOG, 1, NULL, OPT_SYSLOG_NUM }, + {OPT_DEV, 1, NULL, OPT_DEV_NUM }, {OPT_VDEV, 1, NULL, OPT_VDEV_NUM }, {OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM }, {OPT_VMWARE_TSC_MAP, 0, NULL, OPT_VMWARE_TSC_MAP_NUM },@@ -1120,6 +1121,21 @@ eal_parse_common_option(int opt, const char *optarg, }break ; +case OPT_DEV_NUM : { +struct rte_devargs da; +int ret;
lib/librte_eal/common/eal_common_options.c eal_parse_common_option(int opt, const char *optarg, · line 1136
Quoted 10 lines
lib/librte_eal/common/eal_common_options.c +10
-0+ +if (rte_eal_devargs_parse (&da, optarg) <0 ) +return -1 ; + ret =rte_bus_probe_mode_set (da.bus->name, +RTE_BUS_PROBE_WHITELIST ); +if (ret <0 && ret != -ENOTSUP ) +return -1 ; +if (eal_option_device_add (NULL , optarg) <0 ) +return -1 ; + }
Might be a naive question: Any specific reason why we don't add the devices directly into devargs_list here (eal_parse_args -> eal_parse_common_option -> OPT_DEV ->) rather than wait for eal to call eal_option_device_parse again?
Is it to allow eal_plugins_init() to finish?
Quoted 5 lines
lib/librte_eal/common/eal_common_options.c +2
-0+break ; +case OPT_VDEV_NUM :if (eal_option_device_add ("vdev" , optarg) <0 )return -1 ;
[...]
Re: [PATCH v2 12/18] eal: add generic device declaration parameter
Quoted 43 lines · Shreyansh Jain
Quoted 4 lines · Gaetan Rivet
Add a new generic device declaration parameter:
--dev=<device_declaration>
[...]
Quoted 19 lines
lib/librte_eal/common/eal_common_options.c +4
-0@@ -95,6 +95,7 @@ eal_long_options[] = { {OPT_PROC_TYPE, 1, NULL, OPT_PROC_TYPE_NUM }, {OPT_SOCKET_MEM, 1, NULL, OPT_SOCKET_MEM_NUM }, {OPT_SYSLOG, 1, NULL, OPT_SYSLOG_NUM }, + {OPT_DEV, 1, NULL, OPT_DEV_NUM }, {OPT_VDEV, 1, NULL, OPT_VDEV_NUM }, {OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM }, {OPT_VMWARE_TSC_MAP, 0, NULL, OPT_VMWARE_TSC_MAP_NUM },@@ -1120,6 +1121,21 @@ eal_parse_common_option(int opt, const char *optarg, }break ; +case OPT_DEV_NUM : { +struct rte_devargs da; +int ret;lib/librte_eal/common/eal_common_options.c eal_parse_common_option(int opt, const char *optarg, · line 1135
Quoted 10 lines
lib/librte_eal/common/eal_common_options.c +10
-0+ +if (rte_eal_devargs_parse (&da, optarg) <0 ) +return -1 ; + ret =rte_bus_probe_mode_set (da.bus->name, +RTE_BUS_PROBE_WHITELIST ); +if (ret <0 && ret != -ENOTSUP ) +return -1 ; +if (eal_option_device_add (NULL , optarg) <0 ) +return -1 ; + }Might be a naive question: Any specific reason why we don't add the devices directly into
devargs_listhere (eal_parse_args->eal_parse_common_option->OPT_DEV->) rather than wait for eal to calleal_option_device_parseagain?
Quoted 2 lines
Is it to allow
eal_plugins_init()to finish?
Yes. And actually this makes me aware of an issue with this implementation.
Calling rte_eal_devargs_parse here is premature, and rte_bus_probe_mode_set as well.
eal_plugins_init() must be executed before calling rte_devargs to allow for buses introduced as plugins to be able to recognize their devices.
I will reorder a few things in eal_options, thanks for catching this.
Quoted 8 lines
Quoted 5 lines
lib/librte_eal/common/eal_common_options.c +2
-0+break ; +case OPT_VDEV_NUM :if (eal_option_device_add ("vdev" , optarg) <0 )return -1 ;[...]
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v2 12/18] eal: add generic device declaration parameter
Quoted 47 lines · Gaëtan Rivet
Quoted 43 lines · Shreyansh Jain
Quoted 4 lines · Gaetan Rivet
Add a new generic device declaration parameter:
--dev=<device_declaration>
[...]
Quoted 19 lines
lib/librte_eal/common/eal_common_options.c +4
-0@@ -95,6 +95,7 @@ eal_long_options[] = { {OPT_PROC_TYPE, 1, NULL, OPT_PROC_TYPE_NUM }, {OPT_SOCKET_MEM, 1, NULL, OPT_SOCKET_MEM_NUM }, {OPT_SYSLOG, 1, NULL, OPT_SYSLOG_NUM }, + {OPT_DEV, 1, NULL, OPT_DEV_NUM }, {OPT_VDEV, 1, NULL, OPT_VDEV_NUM }, {OPT_VFIO_INTR, 1, NULL, OPT_VFIO_INTR_NUM }, {OPT_VMWARE_TSC_MAP, 0, NULL, OPT_VMWARE_TSC_MAP_NUM },@@ -1120,6 +1121,21 @@ eal_parse_common_option(int opt, const char *optarg, }break ; +case OPT_DEV_NUM : { +struct rte_devargs da; +int ret;lib/librte_eal/common/eal_common_options.c eal_parse_common_option(int opt, const char *optarg, · line 1135
Quoted 10 lines
lib/librte_eal/common/eal_common_options.c +10
-0+ +if (rte_eal_devargs_parse (&da, optarg) <0 ) +return -1 ; + ret =rte_bus_probe_mode_set (da.bus->name, +RTE_BUS_PROBE_WHITELIST ); +if (ret <0 && ret != -ENOTSUP ) +return -1 ; +if (eal_option_device_add (NULL , optarg) <0 ) +return -1 ; + }Might be a naive question: Any specific reason why we don't add the devices directly into
devargs_listhere (eal_parse_args->eal_parse_common_option->OPT_DEV->) rather than wait for eal to calleal_option_device_parseagain?Quoted 2 lines
Is it to allow
eal_plugins_init()to finish?
Quoted 8 lines
Yes. And actually this makes me aware of an issue with this implementation.
Calling
rte_eal_devargs_parsehere is premature, andrte_bus_probe_mode_setas well.
eal_plugins_init()must be executed before callingrte_devargsto allow for buses introduced as plugins to be able to recognize their devices.
There might be one more catch. Maybe eal_parse_args also finds all the plugins to load (-d ...).
Quoted 12 lines
I will reorder a few things in
eal_options, thanks for catching this.Quoted 8 lines
Quoted 5 lines
lib/librte_eal/common/eal_common_options.c +2
-0+break ; +case OPT_VDEV_NUM :if (eal_option_device_add ("vdev" , optarg) <0 )return -1 ;[...]
Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
In reply to Shreyansh JainHello Gaetan,
Quoted 9 lines · Shreyansh Jain
Hello Gaetan,
Quoted 7 lines
-----Original Message-----
From: Gaëtan Rivet [mailto:[email protected]]
Sent: Wednesday, December 13, 2017 3:56 PM
To: Shreyansh Jain <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
[...]
Quoted 4 lines
While reading through the code, I also had the same feeling - there can be corner cases in the parsing functions which I can't imagine. Anyways, those need to be runtime-verified - static reviews may not suffice.
Quoted 1 line
Quoted 8 lines
Quoted 3 lines
I would certainly appreciate if you are able to fix the pci / vdev limitation in
rte_eal_dev_attach, as I am starting to be overwhelmed with work (trying to finish a lot of things before the holidays).OK.
Once you give the devargs a push, I will start work on the PCI removal fromrte_eal_dev_attach. Before that, I just want to be sure of devargs with non-PCI bus (non hotplug case).And, thanks for tons of work you are handling. I saw the patches and really appreciate how you have split things up in sequential manner per-patch. It is difficult.
Have you pushed the new version of the devargs patches? Just wanted to check in case I have missed it.
-
Shreyansh
Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
Hi Shreyansh,
Quoted 21 lines · Shreyansh Jain
Hello Gaetan,
Quoted 9 lines · Shreyansh Jain
Hello Gaetan,
Quoted 7 lines
-----Original Message-----
From: Gaëtan Rivet [mailto:[email protected]]
Sent: Wednesday, December 13, 2017 3:56 PM
To: Shreyansh Jain <[email protected]>
Cc: [email protected]
Subject: Re: [PATCH v2 18/18] doc: remove devargs deprecation notices[...]
Quoted 6 lines
While reading through the code, I also had the same feeling - there can be corner cases in the parsing functions which I can't imagine. Anyways, those need to be runtime-verified - static reviews may not suffice.
Quoted 4 lines
I would certainly appreciate if you are able to fix the pci / vdev limitation in
rte_eal_dev_attach, as I am starting to be overwhelmed with work (trying to finish a lot of things before the holidays).
Quoted 8 lines
Quoted 4 lines
OK.
Once you give the devargs a push, I will start work on the PCI removal fromrte_eal_dev_attach. Before that, I just want to be sure of devargs with non-PCI bus (non hotplug case).And, thanks for tons of work you are handling. I saw the patches and really appreciate how you have split things up in sequential manner per-patch. It is difficult.
Have you pushed the new version of the devargs patches? Just wanted to check in case I have missed it.
No sorry,
I am removing the dependency on the bus control framework from the devargs patchset.
I finished the patchset otherwise (redid the unit test), but after some thinking I found that the bus control was maybe not ideal.
I will send the devargs patchset soon, once it is made independent, and will expose the issue with the bus control.
Best,
Signature · 2 lines
Gaëtan Rivet 6WIND
v3 cover devargs cleanup
In reply to Gaetan RivetCleanup the rte_devargs API.
This is a continuous process.
The previous version of this patchset was dependent on changes on the PCI bus and the rte_bus library. Not all these changes were integrated. This patchset has thus been limited to elements that are available right now, without dependencies.
The devargs list is made private. All devargs users are meant to use them using the proper API. Devargs will allocate dynamic memory. With device hotplug, this memory is usually mishandled and lost by libraries not following the evolutions of rte_devargs.
Gaetan Rivet (10):
devargs: introduce iterator
devargs: introduce foreach macro
bus/vdev: do not reference devargs list
bus/pci: do not reference devargs list
net/vdev_netvsc: do not reference devargs list
test: remove devargs unit tests
devargs: make devargs list private
devargs: make parsing variadic
devargs: use proper namespace prefix
devargs: rename legacy API
MAINTAINERS | 1 - app/test-pmd/cmdline.c | 2 +- drivers/bus/pci/Makefile | 1 + drivers/bus/pci/pci_common.c | 6 +- drivers/bus/vdev/Makefile | 1 + drivers/bus/vdev/vdev.c | 13 +-- drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- drivers/net/vdev_netvsc/vdev_netvsc.c | 4 +- examples/bond/Makefile | 1 + examples/bond/main.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 39 ++------- lib/librte_eal/common/eal_common_devargs.c | 53 ++++++++++-- lib/librte_eal/common/eal_common_options.c | 2 +- lib/librte_eal/common/include/rte_devargs.h | 127 +++++++++++++++++++++++----- lib/librte_eal/rte_eal_version.map | 11 ++- lib/librte_ether/rte_ethdev.c | 44 +++++----- test/test/Makefile | 1 - test/test/commands.c | 2 +- test/test/test_devargs.c | 103 ---------------------- 20 files changed, 198 insertions(+), 219 deletions(-) delete mode 100644 test/test/test_devargs.c
Signature · 1 line
2.11.0
PATCH v3 1/10 devargs: introduce iterator
In preparation to making devargs_list private.
Bus drivers generally need to access rte_devargs pertaining to their operations. This match is a common operation for bus drivers.
Add a new accessor for the rte_devargs list.
--- lib/librte_eal/common/eal_common_devargs.c | 20 ++++++++++++++++++++ lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 41 insertions(+)
lib/librte_eal/common/eal_common_devargs.c +20 -0
@@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f) devargs->name, devargs->args); } } + +/* bus-aware rte_devargs iterator. */ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +{ +struct rte_devargs *da; + +if (start !=NULL ) + da =TAILQ_NEXT (start, next); +else + da =TAILQ_FIRST (&devargs_list); +while (da !=NULL ) { +if (busname ==NULL || + (strcmp (busname, da->bus->name) ==0 )) +return da; + da =TAILQ_NEXT (da, next); + } +return NULL ; +}
lib/librte_eal/common/include/rte_devargs.h +20 -0
@@ -189,6 +189,26 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);*/ void rte_eal_devargs_dump (FILE *f); +/** +* Find next rte_devargs matching the provided bus name. +* +* @param busname +* Limit the iteration to devargs related to buses +* matching this name. +* Will return any next rte_devargs if NULL. +* +* @param start +* Starting iteration point. The iteration will start at +* the first rte_devargs if NULL. +* +* @return +* Next rte_devargs entry matching the requested bus, +* NULL if there is none. +*/ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); + #ifdef __cplusplus }#endif
lib/librte_eal/rte_eal_version.map +1 -0
@@ -216,6 +216,7 @@ EXPERIMENTAL { rte_eal_cleanup; rte_eal_devargs_insert; + rte_eal_devargs_next; rte_eal_devargs_parse; rte_eal_devargs_remove; rte_eal_hotplug_add;
Signature · 1 line
2.11.0
PATCH v3 2/10 devargs: introduce foreach macro
In reply to Gaetan RivetIntroduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining to a bus.
--- lib/librte_eal/common/include/rte_devargs.h | 8 ++++++++ 1 file changed, 8 insertions(+)
lib/librte_eal/common/include/rte_devargs.h +8 -0
@@ -209,6 +209,14 @@ __rte_experimentalstruct rte_devargs *rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); +/** +* Iterate over all rte_devargs for a specific bus. +*/ +#define RTE_EAL_DEVARGS_FOREACH (busname, da) \ + for (da = rte_eal_devargs_next(busname, NULL); \ + da != NULL; \ + da = rte_eal_devargs_next(busname, da)) \ + #ifdef __cplusplus }#endif
Signature · 1 line
2.11.0
PATCH v3 3/10 bus/vdev: do not reference devargs list
In reply to Gaetan RivetThis list should not be operated upon by drivers. Use the public API to achieve the same functionalities.
--- drivers/bus/vdev/Makefile | 1 + drivers/bus/vdev/vdev.c | 11 +++-------- 2 files changed, 4 insertions(+), 8 deletions(-)
drivers/bus/vdev/Makefile +1 -0
@@ -10,6 +10,7 @@ LIB = librte_bus_vdev.a CFLAGS += -O3 CFLAGS += $(WERROR_FLAGS) +CFLAGS += -DALLOW_EXPERIMENTAL_API # versioning export map EXPORT_MAP := rte_bus_vdev_version.map
drivers/bus/vdev/vdev.c +3 -8
@@ -256,7 +256,7 @@ rte_vdev_init(const char *name, const char *args) goto fail; } -TAILQ_INSERT_TAIL (&devargs_list, devargs, next); +rte_eal_devargs_insert (devargs);TAILQ_INSERT_TAIL (&vdev_device_list, dev, next);return 0 ;
@@ -306,10 +306,8 @@ rte_vdev_uninit(const char *name)TAILQ_REMOVE (&vdev_device_list, dev, next); -TAILQ_REMOVE (&devargs_list, devargs, next); +rte_eal_devargs_remove (devargs->bus->name, devargs->name); -free (devargs->args); -free (devargs);free (dev);return 0 ; }
@@ -337,10 +335,7 @@ vdev_scan(void)rte_spinlock_unlock (&vdev_custom_scan_lock);/* for virtual devices we scan the devargs_list populated via cmdline */ - TAILQ_FOREACH(devargs, &devargs_list, next) { - -if (devargs->bus != &rte_vdev_bus) -continue ; + RTE_EAL_DEVARGS_FOREACH("vdev", devargs) { dev =find_vdev (devargs->name);if (dev)
Signature · 1 line
2.11.0
PATCH v3 4/10 bus/pci: do not reference devargs list
In reply to Gaetan RivetThis list should not be used by drivers.
Use the public API instead.
--- drivers/bus/pci/Makefile | 1 + drivers/bus/pci/pci_common.c | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-)
drivers/bus/pci/Makefile +1 -0
@@ -37,6 +37,7 @@ EXPORT_MAP := rte_bus_pci_version.map CFLAGS := -I$(SRCDIR) $(CFLAGS) CFLAGS += -O3 $(WERROR_FLAGS) +CFLAGS += -DALLOW_EXPERIMENTAL_API ifneq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),) SYSTEM := linux
drivers/bus/pci/pci_common.c +1 -5
@@ -45,12 +45,8 @@ static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev) {struct rte_devargs *devargs;struct rte_pci_addr addr; -struct rte_bus *pbus; - pbus =rte_bus_find_by_name ("pci" ); - TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->bus != pbus) -continue ; + RTE_EAL_DEVARGS_FOREACH("pci", devargs) { devargs->bus->parse (devargs->name, &addr);if (!rte_pci_addr_cmp (&dev->addr, &addr))return devargs;
Signature · 1 line
2.11.0
PATCH v3 5/10 net/vdev_netvsc: do not reference devargs list
In reply to Gaetan RivetThis list should not be used by drivers.
Use the public API instead.
--- drivers/net/vdev_netvsc/vdev_netvsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/vdev_netvsc/vdev_netvsc.c +1 -1
@@ -733,7 +733,7 @@ vdev_netvsc_scan_callback(__rte_unused void *arg)struct rte_devargs *devargs;struct rte_bus *vbus =rte_bus_find_by_name ("vdev" ); - TAILQ_FOREACH(devargs, &devargs_list, next) + RTE_EAL_DEVARGS_FOREACH("vdev", devargs)if (!strcmp (devargs->name,VDEV_NETVSC_DRIVER_NAME ))return ; dev = (struct rte_vdev_device *)vbus->find_device(NULL,
Signature · 1 line
2.11.0
PATCH v3 6/10 test: remove devargs unit tests
In reply to Gaetan RivetThe current test will not be compatible anymore with a private devargs list.
Moreover, the new functions should have new tests, while the existing API will be removed.
The current unit tests are thus obsolete and hereby removed.
--- MAINTAINERS | 1 - test/test/Makefile | 1 - test/test/test_devargs.c | 103 ----------------------------------------------- 3 files changed, 105 deletions(-) delete mode 100644 test/test/test_devargs.c
MAINTAINERS +0 -1
@@ -113,7 +113,6 @@ F: test/test/test_common.c F: test/test/test_cpuflags.c F: test/test/test_cycles.c F: test/test/test_debug.c -F: test/test/test_devargs.c F: test/test/test_eal* F: test/test/test_errno.c F: test/test/test_interrupts.c
test/test/Makefile +0 -1
@@ -161,7 +161,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += test_distributor_perf.c SRCS-$(CONFIG_RTE_LIBRTE_REORDER) += test_reorder.c -SRCS-y += test_devargs.c SRCS-y += virtual_pmd.c SRCS-y += packet_burst_generator.c SRCS-$(CONFIG_RTE_LIBRTE_ACL) += test_acl.c
test/test/test_devargs.c deleted +0 -103
103 lines not on this page · raw message
Signature · 1 line
2.11.0
PATCH v3 7/10 devargs: make devargs list private
In reply to Gaetan RivetInitially, rte_devargs was meant to be populated once and sometimes accessed, then never emptied.
With the new hotplug functionality having better standing, new usage appeared with repeated addition of devices and their subsequent removal.
Exposing devargs_list pushed bus drivers and libraries to be careless and inconsistent in their memory management. Making it private will allow to rationalize this part of the EAL and ensure that fewer memory leaks occur during operations.
--- lib/librte_eal/common/eal_common_devargs.c | 3 +++ lib/librte_eal/common/include/rte_devargs.h | 6 ------ lib/librte_eal/rte_eal_version.map | 1 - 3 files changed, 3 insertions(+), 7 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +3 -0
@@ -18,6 +18,9 @@#include <rte_tailq.h> #include "eal_private.h" +/** user device double-linked queue type definition */ +TAILQ_HEAD (rte_devargs_list, rte_devargs); +/** Global list of user devices */ struct rte_devargs_list devargs_list =TAILQ_HEAD_INITIALIZER (devargs_list);
lib/librte_eal/common/include/rte_devargs.h +0 -6
@@ -59,12 +59,6 @@ struct rte_devargs {char *args; }; -/** user device double-linked queue type definition */ -TAILQ_HEAD (rte_devargs_list, rte_devargs); - -/** Global list of user devices */ -extern struct rte_devargs_list devargs_list; -/** * Parse a devargs string. *
lib/librte_eal/rte_eal_version.map +0 -1
@@ -2,7 +2,6 @@ DPDK_2.0 { global: __rte_panic; - devargs_list; eal_parse_sysfs_value; eal_timer_source; lcore_config;
Signature · 1 line
2.11.0
PATCH v3 8/10 devargs: make parsing variadic
In reply to Gaetan Rivetrte_eal_devargs_parse can be used by EAL subsystems, drivers, applications alike.
Device parameters may be presented with different structure each time; as a single declaration string or several strings each describing different parts of the declaration.
To simplify the use of this parsing facility, its parameters are made variadic.
--- drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 33 ++++------------------------- lib/librte_eal/common/eal_common_devargs.c | 15 ++++++++++--- lib/librte_eal/common/include/rte_devargs.h | 26 ++++++++++++----------- 5 files changed, 32 insertions(+), 46 deletions(-)
drivers/net/failsafe/failsafe_args.c +1 -1
@@ -62,7 +62,7 @@ fs_parse_device(struct sub_device *sdev, char *args) d = &sdev->devargs;DEBUG ("%s" , args); - ret =rte_eal_devargs_parse (args, d); + ret =rte_eal_devargs_parse (d,"%s" , args);if (ret) {DEBUG ("devargs parsing failed with code %d" , ret);return ret;
drivers/net/failsafe/failsafe_eal.c +1 -1
@@ -69,7 +69,7 @@ fs_bus_init(struct rte_eth_dev *dev)else snprintf (devstr,sizeof (devstr),"%s" , rte_eth_devices[pid].device->name); - ret =rte_eal_devargs_parse (devstr, da); + ret =rte_eal_devargs_parse (da,"%s" , devstr);if (ret) {ERROR ("Probed devargs parsing failed with code" " %d" , ret);
lib/librte_eal/common/eal_common_dev.c +4 -29
@@ -89,29 +89,12 @@ int rte_eal_dev_detach(struct rte_device *dev)return ret; } -static char * -full_dev_name (const char *bus,const char *dev,const char *args) -{ -char *name; -size_t len; - - len =snprintf (NULL ,0 ,"%s:%s,%s" , bus, dev, args) +1 ; - name =calloc (1 , len); -if (name ==NULL ) { -RTE_LOG (ERR ,EAL ,"Could not allocate full device name\n" ); -return NULL ; - } -snprintf (name, len,"%s:%s,%s" , bus, dev, args); -return name; -} - int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,const char *devargs) {struct rte_bus *bus;struct rte_device *dev;struct rte_devargs *da; -char *name;int ret; bus =rte_bus_find_by_name (busname);
@@ -126,17 +109,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnreturn -ENOTSUP ; } - name =full_dev_name (busname, devname, devargs); -if (name ==NULL ) + da =calloc (1 ,sizeof (*da)); +if (da ==NULL )return -ENOMEM ; - da =calloc (1 ,sizeof (*da)); -if (da ==NULL ) { - ret = -ENOMEM ; - goto err_name; - } - - ret =rte_eal_devargs_parse (name, da); + ret =rte_eal_devargs_parse (da,"%s:%s,%s" , + busname, devname, devargs);if (ret) goto err_devarg;
@@ -162,7 +140,6 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn dev->name); goto err_devarg; } -free (name);return 0 ; err_devarg:
@@ -170,8 +147,6 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnfree (da->args);free (da); } -err_name: -free (name);return ret; }
lib/librte_eal/common/eal_common_devargs.c +12 -3
@@ -11,6 +11,7 @@#include <stdio.h> #include <string.h> +#include <stdarg.h> #include <rte_compat.h> #include <rte_dev.h>
@@ -62,15 +63,23 @@ bus_name_cmp(const struct rte_bus *bus, const void *name) } int __rte_experimental -rte_eal_devargs_parse (const char *dev,struct rte_devargs *da) +rte_eal_devargs_parse (struct rte_devargs *da,const char *format, ...) {struct rte_bus *bus =NULL ; +va_list ap; +va_start (ap, format); +char dev[vsnprintf (NULL ,0 , format, ap) +1 ];const char *devname;const size_t maxlen =sizeof (da->name);size_t i; -if (dev ==NULL || da ==NULL ) +va_end (ap); +if (da ==NULL )return -EINVAL ; + +va_start (ap, format); +vsnprintf (dev,sizeof (dev), format, ap); +va_end (ap);/* Retrieve eventual bus info */ do { devname = dev;
@@ -140,7 +149,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)if (devargs ==NULL ) goto fail; -if (rte_eal_devargs_parse (dev, devargs)) +if (rte_eal_devargs_parse (devargs,"%s" , dev)) goto fail; devargs->type = devtype; bus = devargs->bus;
lib/librte_eal/common/include/rte_devargs.h +14 -12
@@ -94,18 +94,21 @@ int rte_eal_parse_devargs_str(const char *devargs_str,* in argument. Store which bus will handle the device, its name * and the eventual device parameters. * -* @param dev -* The device declaration string. +* The device string is built with a printf-like syntax. +* * @param da * The devargs structure holding the device information. +* @param format +* Format string describing a device. * * @return * - 0 on success. * - Negative errno on error. */ int __rte_experimental -rte_eal_devargs_parse (const char *dev, -struct rte_devargs *da); +rte_eal_devargs_parse (struct rte_devargs *da, +const char *format, ...) +__attribute__((format (printf,2 ,0 )));/** * Insert an rte_devargs in the global list.
@@ -123,15 +126,14 @@ rte_eal_devargs_insert(struct rte_devargs *da);/** * Add a device to the user device list * -* For PCI devices, the format of arguments string is "PCI_ADDR" or -* "PCI_ADDR,key=val,key2=val2,...". Examples: "08:00.1", "0000:5:00.0", -* "04:00.0,arg=val". +* The format is * -* For virtual devices, the format of arguments string is "DRIVER_NAME*" -* or "DRIVER_NAME*,key=val,key2=val2,...". Examples: "net_ring", -* "net_ring0", "net_pmdAnything,arg=0:arg2=1". The validity of the -* driver name is not checked by this function, it is done when probing -* the drivers. +* bus:device_identifier,arg1=val1,arg2=val2 +* +* Examples: +* +* pci:0000:05.00.0,arg=val +* vdev:net_ring0 * * @param devtype * The type of the device.
Signature · 1 line
2.11.0
PATCH v3 9/10 devargs: use proper namespace prefix
In reply to Gaetan Rivetrte_eal_devargs is useless, rte_devargs is sufficient.
Only experimental functions are changed for now.
--- drivers/bus/vdev/vdev.c | 6 +++--- drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 8 ++++---- lib/librte_eal/common/eal_common_devargs.c | 12 ++++++------ lib/librte_eal/common/include/rte_devargs.h | 23 +++++++++++++---------- lib/librte_eal/rte_eal_version.map | 8 ++++---- 7 files changed, 32 insertions(+), 29 deletions(-)
drivers/bus/vdev/vdev.c +3 -3
@@ -256,7 +256,7 @@ rte_vdev_init(const char *name, const char *args) goto fail; } -rte_eal_devargs_insert (devargs); +rte_devargs_insert (devargs);TAILQ_INSERT_TAIL (&vdev_device_list, dev, next);return 0 ;
@@ -306,7 +306,7 @@ rte_vdev_uninit(const char *name)TAILQ_REMOVE (&vdev_device_list, dev, next); -rte_eal_devargs_remove (devargs->bus->name, devargs->name); +rte_devargs_remove (devargs->bus->name, devargs->name);free (dev);return 0 ;
@@ -325,7 +325,7 @@ vdev_scan(void)if (custom_scan->callback !=NULL )/* * the callback should update devargs list -* by calling rte_eal_devargs_insert() with +* by calling rte_devargs_insert() with * devargs.bus = rte_bus_find_by_name("vdev"); * devargs.type = RTE_DEVTYPE_VIRTUAL; * devargs.policy = RTE_DEV_WHITELISTED;
drivers/net/failsafe/failsafe_args.c +1 -1
@@ -62,7 +62,7 @@ fs_parse_device(struct sub_device *sdev, char *args) d = &sdev->devargs;DEBUG ("%s" , args); - ret =rte_eal_devargs_parse (d,"%s" , args); + ret =rte_devargs_parse (d,"%s" , args);if (ret) {DEBUG ("devargs parsing failed with code %d" , ret);return ret;
drivers/net/failsafe/failsafe_eal.c +1 -1
@@ -69,7 +69,7 @@ fs_bus_init(struct rte_eth_dev *dev)else snprintf (devstr,sizeof (devstr),"%s" , rte_eth_devices[pid].device->name); - ret =rte_eal_devargs_parse (da,"%s" , devstr); + ret =rte_devargs_parse (da,"%s" , devstr);if (ret) {ERROR ("Probed devargs parsing failed with code" " %d" , ret);
lib/librte_eal/common/eal_common_dev.c +4 -4
@@ -113,12 +113,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnif (da ==NULL )return -ENOMEM ; - ret =rte_eal_devargs_parse (da,"%s:%s,%s" , + ret =rte_devargs_parse (da,"%s:%s,%s" , busname, devname, devargs);if (ret) goto err_devarg; - ret =rte_eal_devargs_insert (da); + ret =rte_devargs_insert (da);if (ret) goto err_devarg;
@@ -143,7 +143,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnreturn 0 ; err_devarg: -if (rte_eal_devargs_remove (busname, devname)) { +if (rte_devargs_remove (busname, devname)) {free (da->args);free (da); }
@@ -179,6 +179,6 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)if (ret)RTE_LOG (ERR ,EAL ,"Driver cannot detach the device (%s)\n" , dev->name); -rte_eal_devargs_remove (busname, devname); +rte_devargs_remove (busname, devname);return ret; }
lib/librte_eal/common/eal_common_devargs.c +6 -6
@@ -63,7 +63,7 @@ bus_name_cmp(const struct rte_bus *bus, const void *name) } int __rte_experimental -rte_eal_devargs_parse (struct rte_devargs *da,const char *format, ...) +rte_devargs_parse (struct rte_devargs *da,const char *format, ...) {struct rte_bus *bus =NULL ;va_list ap;
@@ -125,11 +125,11 @@ rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...) } int __rte_experimental -rte_eal_devargs_insert (struct rte_devargs *da) +rte_devargs_insert (struct rte_devargs *da) {int ret; - ret =rte_eal_devargs_remove (da->bus->name, da->name); + ret =rte_devargs_remove (da->bus->name, da->name);if (ret <0 )return ret;TAILQ_INSERT_TAIL (&devargs_list, da, next);
@@ -149,7 +149,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)if (devargs ==NULL ) goto fail; -if (rte_eal_devargs_parse (devargs,"%s" , dev)) +if (rte_devargs_parse (devargs,"%s" , dev)) goto fail; devargs->type = devtype; bus = devargs->bus;
@@ -174,7 +174,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) } int __rte_experimental -rte_eal_devargs_remove (const char *busname,const char *devname) +rte_devargs_remove (const char *busname,const char *devname) {struct rte_devargs *d;void *tmp;
@@ -223,7 +223,7 @@ rte_eal_devargs_dump(FILE *f)/* bus-aware rte_devargs iterator. */ __rte_experimental struct rte_devargs * -rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +rte_devargs_next (const char *busname,const struct rte_devargs *start) {struct rte_devargs *da;
lib/librte_eal/common/include/rte_devargs.h +13 -10
@@ -105,9 +105,10 @@ int rte_eal_parse_devargs_str(const char *devargs_str,* - 0 on success. * - Negative errno on error. */ -int __rte_experimental -rte_eal_devargs_parse (struct rte_devargs *da, -const char *format, ...) +__rte_experimental +int +rte_devargs_parse (struct rte_devargs *da, +const char *format, ...) __attribute__((format (printf,2 ,0 )));/**
@@ -120,8 +121,9 @@ __attribute__((format(printf, 2, 0)));* - 0 on success * - Negative on error. */ -int __rte_experimental -rte_eal_devargs_insert (struct rte_devargs *da); +__rte_experimental +int +rte_devargs_insert (struct rte_devargs *da);/** * Add a device to the user device list
@@ -162,8 +164,9 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str);* <0 on error. * >0 if the devargs was not within the user device list. */ -int __rte_experimental rte_eal_devargs_remove(const char *busname, -const char *devname); +__rte_experimental +int rte_devargs_remove(const char *busname, +const char *devname);/** * Count the number of user devices of a specified type
@@ -203,15 +206,15 @@ void rte_eal_devargs_dump(FILE *f);*/ __rte_experimental struct rte_devargs * -rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); +rte_devargs_next (const char *busname,const struct rte_devargs *start);/** * Iterate over all rte_devargs for a specific bus. */ #define RTE_EAL_DEVARGS_FOREACH (busname, da) \ - for (da = rte_eal_devargs_next(busname, NULL); \ + for (da = rte_devargs_next(busname, NULL); \ da != NULL; \ - da = rte_eal_devargs_next(busname, da)) \ + da = rte_devargs_next(busname, da)) \ #ifdef __cplusplus }
lib/librte_eal/rte_eal_version.map +4 -4
@@ -214,10 +214,10 @@ EXPERIMENTAL { global: rte_eal_cleanup; - rte_eal_devargs_insert; - rte_eal_devargs_next; - rte_eal_devargs_parse; - rte_eal_devargs_remove; + rte_devargs_insert; + rte_devargs_next; + rte_devargs_parse; + rte_devargs_remove; rte_eal_hotplug_add; rte_eal_hotplug_remove; rte_eal_mbuf_user_pool_ops;
Signature · 1 line
2.11.0
PATCH v3 10/10 devargs: rename legacy API
In reply to Gaetan RivetThis previous symbols were deprecated for two releases. They are now marked as such and cannot be used anymore.
They are replaced by ones respecting the new namespace that are marked experimental.
As a result, eth_dev attach and detach is slightly reworked to follow the changes.
--- app/test-pmd/cmdline.c | 2 +- drivers/net/vdev_netvsc/vdev_netvsc.c | 2 +- examples/bond/Makefile | 1 + examples/bond/main.c | 2 +- lib/librte_eal/common/eal_common_devargs.c | 9 +++-- lib/librte_eal/common/eal_common_options.c | 2 +- lib/librte_eal/common/include/rte_devargs.h | 54 +++++++++++++++++++++++++++++ lib/librte_eal/rte_eal_version.map | 3 ++ lib/librte_ether/rte_ethdev.c | 44 +++++++++++------------ test/test/commands.c | 2 +- 10 files changed, 89 insertions(+), 32 deletions(-)
app/test-pmd/cmdline.c +1 -1
@@ -8621,7 +8621,7 @@ static void cmd_dump_parsed(void *parsed_result,else if (!strcmp (res->dump,"dump_mempool" ))rte_mempool_list_dump (stdout);else if (!strcmp (res->dump,"dump_devargs" )) -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);else if (!strcmp (res->dump,"dump_log_types" ))rte_log_dump (stdout); }
drivers/net/vdev_netvsc/vdev_netvsc.c +1 -1
@@ -740,7 +740,7 @@ vdev_netvsc_scan_callback(__rte_unused void *arg) vdev_netvsc_cmp_rte_device, VDEV_NETVSC_DRIVER_NAME);if (dev)return ; -if (rte_eal_devargs_add (RTE_DEVTYPE_VIRTUAL ,VDEV_NETVSC_DRIVER_NAME )) +if (rte_devargs_add (RTE_DEVTYPE_VIRTUAL ,VDEV_NETVSC_DRIVER_NAME ))DRV_LOG (ERR ,"unable to add netvsc devargs." ); }
examples/bond/Makefile +1 -0
@@ -59,6 +59,7 @@ CFLAGS_main.o += -Wno-return-type endif CFLAGS += -O3 +CFLAGS += -DALLOW_EXPERIMENTAL_API ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y) LDLIBS += -lrte_pmd_bond
examples/bond/main.c +1 -1
@@ -742,7 +742,7 @@ main(int argc, char *argv[])/* init EAL */ ret =rte_eal_init (argc, argv); -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);if (ret <0 )rte_exit (EXIT_FAILURE ,"Error with EAL initialization\n" ); argc -= ret;
lib/librte_eal/common/eal_common_devargs.c +6 -3
@@ -137,8 +137,9 @@ rte_devargs_insert(struct rte_devargs *da) }/* store a whitelist parameter for later parsing */ +__rte_experimental int -rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) +rte_devargs_add (enum rte_devtype devtype,const char *devargs_str) {struct rte_devargs *devargs =NULL ;struct rte_bus *bus =NULL ;
@@ -192,8 +193,9 @@ rte_devargs_remove(const char *busname, const char *devname) }/* count the number of devices of a specified type */ +__rte_experimental unsigned int -rte_eal_devargs_type_count (enum rte_devtype devtype) +rte_devargs_type_count (enum rte_devtype devtype) {struct rte_devargs *devargs;unsigned int count =0 ;
@@ -207,8 +209,9 @@ rte_eal_devargs_type_count(enum rte_devtype devtype) }/* dump the user devices on the console */ +__rte_experimental void -rte_eal_devargs_dump (FILE *f) +rte_devargs_dump (FILE *f) {struct rte_devargs *devargs;
lib/librte_eal/common/eal_common_options.c +1 -1
@@ -151,7 +151,7 @@ eal_option_device_parse(void) TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {if (ret ==0 ) { - ret =rte_eal_devargs_add (devopt->type, devopt->arg); + ret =rte_devargs_add (devopt->type, devopt->arg);if (ret)RTE_LOG (ERR ,EAL ,"Unable to parse device '%s'\n" , devopt->arg);
lib/librte_eal/common/include/rte_devargs.h +54 -0
84 lines not on this page · raw message
lib/librte_eal/rte_eal_version.map +3 -0
@@ -214,10 +214,13 @@ EXPERIMENTAL { global: rte_eal_cleanup; + rte_devargs_add; + rte_devargs_dump; rte_devargs_insert; rte_devargs_next; rte_devargs_parse; rte_devargs_remove; + rte_devargs_type_count; rte_eal_hotplug_add; rte_eal_hotplug_remove; rte_eal_mbuf_user_pool_ops;
lib/librte_ether/rte_ethdev.c +20 -24
92 lines not on this page · raw message
test/test/commands.c +1 -1
@@ -132,7 +132,7 @@ static void cmd_dump_parsed(void *parsed_result,else if (!strcmp (res->dump,"dump_mempool" ))rte_mempool_list_dump (stdout);else if (!strcmp (res->dump,"dump_devargs" )) -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);else if (!strcmp (res->dump,"dump_log_types" ))rte_log_dump (stdout);else if (!strcmp (res->dump,"dump_malloc_stats" ))
Signature · 1 line
2.11.0
Re: [PATCH v3 03/10] bus/vdev: do not reference devargs list
In reply to Gaetan RivetQuoted 4 lines · Gaetan Rivet
This list should not be operated upon by drivers. Use the public API to achieve the same functionalities.
Signed-off-by Gaetan Rivet <[email protected]>
Thanks,
Jianfeng
Quoted 54 lines
--- drivers/bus/vdev/Makefile | 1 + drivers/bus/vdev/vdev.c | 11 +++-------- 2 files changed, 4 insertions(+), 8 deletions(-)drivers/bus/vdev/Makefile +1
-0@@ -10,6 +10,7 @@ LIB = librte_bus_vdev.a CFLAGS += -O3 CFLAGS += $(WERROR_FLAGS) +CFLAGS += -DALLOW_EXPERIMENTAL_API # versioning export map EXPORT_MAP := rte_bus_vdev_version.mapdrivers/bus/vdev/vdev.c +3
-8@@ -256,7 +256,7 @@ rte_vdev_init(const char *name, const char *args) goto fail; } -TAILQ_INSERT_TAIL (&devargs_list, devargs, next); +rte_eal_devargs_insert (devargs);TAILQ_INSERT_TAIL (&vdev_device_list, dev, next);return 0 ;@@ -306,10 +306,8 @@ rte_vdev_uninit(const char *name)TAILQ_REMOVE (&vdev_device_list, dev, next); -TAILQ_REMOVE (&devargs_list, devargs, next); +rte_eal_devargs_remove (devargs->bus->name, devargs->name); -free (devargs->args); -free (devargs);free (dev);return 0 ; }@@ -337,10 +335,7 @@ vdev_scan(void)rte_spinlock_unlock (&vdev_custom_scan_lock);/* for virtual devices we scan the devargs_list populated via cmdline */ - TAILQ_FOREACH(devargs, &devargs_list, next) { - -if (devargs->bus != &rte_vdev_bus) -continue ; + RTE_EAL_DEVARGS_FOREACH("vdev", devargs) { dev =find_vdev (devargs->name);if (dev)
Re: [PATCH v3 01/10] devargs: introduce iterator
In reply to Gaetan RivetHi Gaetan,
Quoted 78 lines · Gaetan Rivet
In preparation to making
devargs_listprivate.Bus drivers generally need to access
rte_devargspertaining to their operations. This match is a common operation for bus drivers.Add a new accessor for the
rte_devargslist.Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_devargs.c | 20 ++++++++++++++++++++ lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 41 insertions(+)lib/librte_eal/common/eal_common_devargs.c +20
-0@@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f) devargs->name, devargs->args); } } + +/* bus-aware rte_devargs iterator. */ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +{ +struct rte_devargs *da; + +if (start !=NULL ) + da =TAILQ_NEXT (start, next); +else + da =TAILQ_FIRST (&devargs_list); +while (da !=NULL ) { +if (busname ==NULL || + (strcmp (busname, da->bus->name) ==0 )) +return da; + da =TAILQ_NEXT (da, next); + } +return NULL ; +}lib/librte_eal/common/include/rte_devargs.h +20
-0@@ -189,6 +189,26 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);*/ void rte_eal_devargs_dump (FILE *f); +/** +* Find next rte_devargs matching the provided bus name. +* +* @param busname +* Limit the iteration to devargs related to buses +* matching this name. +* Will return any next rte_devargs if NULL. +* +* @param start +* Starting iteration point. The iteration will start at +* the first rte_devargs if NULL. +* +* @return +* Next rte_devargs entry matching the requested bus, +* NULL if there is none. +*/ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); + #ifdef __cplusplus }#endif lib/librte_eal/rte_eal_version.map +0
-0
lib/librte_eal/rte_eal_version.map EXPERIMENTAL { · line 222
Quoted 7 lines
lib/librte_eal/rte_eal_version.map +1
-0@@ -216,6 +216,7 @@ EXPERIMENTAL { rte_eal_cleanup; rte_eal_devargs_insert; + rte_eal_devargs_next; rte_eal_devargs_parse; rte_eal_devargs_remove; rte_eal_hotplug_add;
Shall we change these APIs to be thread-safe?
Thanks,
Jianfeng
Re: [PATCH v3 01/10] devargs: introduce iterator
Hello Jianfeng,
Quoted 81 lines · Jianfeng
Hi Gaetan,
Quoted 77 lines · Gaetan Rivet
In preparation to making
devargs_listprivate.Bus drivers generally need to access
rte_devargspertaining to their operations. This match is a common operation for bus drivers.Add a new accessor for the
rte_devargslist.Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_devargs.c | 20 ++++++++++++++++++++ lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 41 insertions(+)lib/librte_eal/common/eal_common_devargs.c +20
-0@@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f) devargs->name, devargs->args); } } + +/* bus-aware rte_devargs iterator. */ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +{ +struct rte_devargs *da; + +if (start !=NULL ) + da =TAILQ_NEXT (start, next); +else + da =TAILQ_FIRST (&devargs_list); +while (da !=NULL ) { +if (busname ==NULL || + (strcmp (busname, da->bus->name) ==0 )) +return da; + da =TAILQ_NEXT (da, next); + } +return NULL ; +}lib/librte_eal/common/include/rte_devargs.h +20
-0@@ -189,6 +189,26 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);*/ void rte_eal_devargs_dump (FILE *f); +/** +* Find next rte_devargs matching the provided bus name. +* +* @param busname +* Limit the iteration to devargs related to buses +* matching this name. +* Will return any next rte_devargs if NULL. +* +* @param start +* Starting iteration point. The iteration will start at +* the first rte_devargs if NULL. +* +* @return +* Next rte_devargs entry matching the requested bus, +* NULL if there is none. +*/ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); + #ifdef __cplusplus }#endif lib/librte_eal/rte_eal_version.map +0
-0@@ -216,6 +216,7 @@ EXPERIMENTAL {
Quoted 8 lines
lib/librte_eal/rte_eal_version.map EXPERIMENTAL { · line 222
Quoted 6 lines
lib/librte_eal/rte_eal_version.map +1
-0rte_eal_cleanup; rte_eal_devargs_insert; + rte_eal_devargs_next; rte_eal_devargs_parse; rte_eal_devargs_remove; rte_eal_hotplug_add;Shall we change these APIs to be thread-safe?
It was not planned, but I think you raise an interesting point.
At the moment, most devargs operations are happening when a single thread is started (eal init).
However, hotplug means having insertion and deletion possibly happening in additional threads. This API exists and could help making these operations thread-safe.
So this is low-priority I think for now (unless shown otherwise), but definitely something to look into.
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v3 02/10] devargs: introduce foreach macro
In reply to Gaetan Rivet21/03/2018 00:20, Gaetan Rivet:
Quoted 2 lines
Introduce new
rte_devargsaccessor allowing to iterate over allrte_devargspertaining to a bus.
This patch can be merged with previous one.
Re: [PATCH v3 07/10] devargs: make devargs list private
In reply to Gaetan Rivet21/03/2018 00:20, Gaetan Rivet:
Quoted 11 lines
Initially,
rte_devargswas meant to be populated once and sometimes accessed, then never emptied.With the new hotplug functionality having better standing, new usage appeared with repeated addition of devices and their subsequent removal.
Exposing
devargs_listpushed bus drivers and libraries to be careless and inconsistent in their memory management. Making it private will allow to rationalize this part of the EAL and ensure that fewer memory leaks occur during operations.
Quoted 6 lines
Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_devargs.c | 3 +++ lib/librte_eal/common/include/rte_devargs.h | 6 ------ lib/librte_eal/rte_eal_version.map | 1 - 3 files changed, 3 insertions(+), 7 deletions(-)
You must remove the related line in deprecation notice.
Re: [PATCH v3 08/10] devargs: make parsing variadic
In reply to Gaetan Rivet21/03/2018 00:20, Gaetan Rivet:
Quoted 11 lines
/**
* Add a device to the user device list
*
- * For PCI devices, the format of arguments string is "PCI_ADDR" or
- * "PCI_ADDR,key=val,key2=val2,...". Examples: "08:00.1", "0000:5:00.0",
- * "04:00.0,arg=val".
+ * The format is
*
- * For virtual devices, the format of arguments string is "DRIVER_NAME*"
- * or "DRIVER_NAME*,key=val,key2=val2,...". Examples: "net_ring",
- * "net_ring0", "net_pmdAnything,arg=0:arg2=1". The validity of the
Quoted 8 lines
- * driver name is not checked by this function, it is done when probing
- * the drivers.
+ * bus:device_identifier,arg1=val1,arg2=val2
+ *
+ * Examples:
+ *
+ * pci:0000:05.00.0,arg=val
+ * vdev:net_ring0
I think this part should not be in the patch.
Or I miss something?
21/03/2018 00:20, Gaetan Rivet:
Quoted 13 lines
Cleanup the
rte_devargsAPI.This is a continuous process.
The previous version of this patchset was dependent on changes on the PCI bus and therte_buslibrary. Not all these changes were integrated. This patchset has thus been limited to elements that are available right now, without dependencies.The devargs list is made private. All devargs users are meant to use them using the proper API. Devargs will allocate dynamic memory. With device hotplug, this memory is usually mishandled and lost by libraries not following the evolutions of
rte_devargs.
Quoted 11 lines
Gaetan Rivet (10):
devargs: introduce iterator
devargs: introduce foreach macro
bus/vdev: do not reference devargs list
bus/pci: do not reference devargs list
net/vdev_netvsc: do not reference devargs list
test: remove devargs unit tests
devargs: make devargs list private
devargs: make parsing variadic
devargs: use proper namespace prefix
devargs: rename legacy API
Sorry, I missed this series sent one month ago.
I did few small comments.
When then they are addressed, you can rebase and add my
Thanks
v4 cover devargs cleanup
In reply to Gaetan RivetCleanup the rte_devargs API.
This is a continuous process.
The previous version of this patchset was dependent on changes on the PCI bus and the rte_bus library. Not all these changes were integrated. This patchset has thus been limited to elements that are available right now, without dependencies.
The devargs list is made private. All devargs users are meant to use them using the proper API. Devargs will allocate dynamic memory. With device hotplug, this memory is usually mishandled and lost by libraries not following the evolutions of rte_devargs.
v4:
* Rebased on master.
Gaetan Rivet (10):
devargs: introduce iterator
devargs: introduce foreach macro
bus/vdev: do not reference devargs list
bus/pci: do not reference devargs list
net/vdev_netvsc: do not reference devargs list
test: remove devargs unit tests
devargs: make devargs list private
devargs: make parsing variadic
devargs: use proper namespace prefix
devargs: rename legacy API
MAINTAINERS | 1 - app/test-pmd/cmdline.c | 2 +- drivers/bus/pci/Makefile | 1 + drivers/bus/pci/pci_common.c | 6 +- drivers/bus/vdev/Makefile | 1 + drivers/bus/vdev/vdev.c | 13 +-- drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- drivers/net/vdev_netvsc/vdev_netvsc.c | 4 +- examples/bond/Makefile | 1 + examples/bond/main.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 39 ++------- lib/librte_eal/common/eal_common_devargs.c | 53 ++++++++++-- lib/librte_eal/common/eal_common_options.c | 2 +- lib/librte_eal/common/include/rte_devargs.h | 127 +++++++++++++++++++++++----- lib/librte_eal/rte_eal_version.map | 11 ++- lib/librte_ether/rte_ethdev.c | 44 +++++----- test/test/Makefile | 1 - test/test/commands.c | 2 +- test/test/test_devargs.c | 103 ---------------------- 20 files changed, 198 insertions(+), 219 deletions(-) delete mode 100644 test/test/test_devargs.c
Signature · 1 line
2.11.0
PATCH v4 1/10 devargs: introduce iterator
In preparation to making devargs_list private.
Bus drivers generally need to access rte_devargs pertaining to their operations. This match is a common operation for bus drivers.
Add a new accessor for the rte_devargs list.
--- lib/librte_eal/common/eal_common_devargs.c | 20 ++++++++++++++++++++ lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 41 insertions(+)
lib/librte_eal/common/eal_common_devargs.c +20 -0
@@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f) devargs->name, devargs->args); } } + +/* bus-aware rte_devargs iterator. */ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +{ +struct rte_devargs *da; + +if (start !=NULL ) + da =TAILQ_NEXT (start, next); +else + da =TAILQ_FIRST (&devargs_list); +while (da !=NULL ) { +if (busname ==NULL || + (strcmp (busname, da->bus->name) ==0 )) +return da; + da =TAILQ_NEXT (da, next); + } +return NULL ; +}
lib/librte_eal/common/include/rte_devargs.h +20 -0
@@ -189,6 +189,26 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);*/ void rte_eal_devargs_dump (FILE *f); +/** +* Find next rte_devargs matching the provided bus name. +* +* @param busname +* Limit the iteration to devargs related to buses +* matching this name. +* Will return any next rte_devargs if NULL. +* +* @param start +* Starting iteration point. The iteration will start at +* the first rte_devargs if NULL. +* +* @return +* Next rte_devargs entry matching the requested bus, +* NULL if there is none. +*/ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); + #ifdef __cplusplus }#endif
lib/librte_eal/rte_eal_version.map +1 -0
@@ -219,6 +219,7 @@ EXPERIMENTAL { rte_dev_event_monitor_stop; rte_eal_cleanup; rte_eal_devargs_insert; + rte_eal_devargs_next; rte_eal_devargs_parse; rte_eal_devargs_remove; rte_eal_hotplug_add;
Signature · 1 line
2.11.0
PATCH v4 2/10 devargs: introduce foreach macro
In reply to Gaetan RivetIntroduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining to a bus.
--- lib/librte_eal/common/include/rte_devargs.h | 8 ++++++++ 1 file changed, 8 insertions(+)
lib/librte_eal/common/include/rte_devargs.h +8 -0
@@ -209,6 +209,14 @@ __rte_experimentalstruct rte_devargs *rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); +/** +* Iterate over all rte_devargs for a specific bus. +*/ +#define RTE_EAL_DEVARGS_FOREACH (busname, da) \ + for (da = rte_eal_devargs_next(busname, NULL); \ + da != NULL; \ + da = rte_eal_devargs_next(busname, da)) \ + #ifdef __cplusplus }#endif
Signature · 1 line
2.11.0
PATCH v4 3/10 bus/vdev: do not reference devargs list
In reply to Gaetan RivetThis list should not be operated upon by drivers. Use the public API to achieve the same functionalities.
--- drivers/bus/vdev/Makefile | 1 + drivers/bus/vdev/vdev.c | 11 +++-------- 2 files changed, 4 insertions(+), 8 deletions(-)
drivers/bus/vdev/Makefile +1 -0
@@ -10,6 +10,7 @@ LIB = librte_bus_vdev.a CFLAGS += -O3 CFLAGS += $(WERROR_FLAGS) +CFLAGS += -DALLOW_EXPERIMENTAL_API # versioning export map EXPORT_MAP := rte_bus_vdev_version.map
drivers/bus/vdev/vdev.c +3 -8
@@ -228,7 +228,7 @@ rte_vdev_init(const char *name, const char *args) goto fail; } -TAILQ_INSERT_TAIL (&devargs_list, devargs, next); +rte_eal_devargs_insert (devargs);TAILQ_INSERT_TAIL (&vdev_device_list, dev, next);return 0 ;
@@ -278,10 +278,8 @@ rte_vdev_uninit(const char *name)TAILQ_REMOVE (&vdev_device_list, dev, next); -TAILQ_REMOVE (&devargs_list, devargs, next); +rte_eal_devargs_remove (devargs->bus->name, devargs->name); -free (devargs->args); -free (devargs);free (dev);return 0 ; }
@@ -309,10 +307,7 @@ vdev_scan(void)rte_spinlock_unlock (&vdev_custom_scan_lock);/* for virtual devices we scan the devargs_list populated via cmdline */ - TAILQ_FOREACH(devargs, &devargs_list, next) { - -if (devargs->bus != &rte_vdev_bus) -continue ; + RTE_EAL_DEVARGS_FOREACH("vdev", devargs) { dev =find_vdev (devargs->name);if (dev)
Signature · 1 line
2.11.0
PATCH v4 4/10 bus/pci: do not reference devargs list
In reply to Gaetan RivetThis list should not be used by drivers.
Use the public API instead.
--- drivers/bus/pci/Makefile | 1 + drivers/bus/pci/pci_common.c | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-)
drivers/bus/pci/Makefile +1 -0
@@ -37,6 +37,7 @@ EXPORT_MAP := rte_bus_pci_version.map CFLAGS := -I$(SRCDIR) $(CFLAGS) CFLAGS += -O3 $(WERROR_FLAGS) +CFLAGS += -DALLOW_EXPERIMENTAL_API ifneq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),) SYSTEM := linux
drivers/bus/pci/pci_common.c +1 -5
@@ -45,12 +45,8 @@ static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev) {struct rte_devargs *devargs;struct rte_pci_addr addr; -struct rte_bus *pbus; - pbus =rte_bus_find_by_name ("pci" ); - TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->bus != pbus) -continue ; + RTE_EAL_DEVARGS_FOREACH("pci", devargs) { devargs->bus->parse (devargs->name, &addr);if (!rte_pci_addr_cmp (&dev->addr, &addr))return devargs;
Signature · 1 line
2.11.0
PATCH v4 5/10 net/vdev_netvsc: do not reference devargs list
In reply to Gaetan RivetThis list should not be used by drivers.
Use the public API instead.
--- drivers/net/vdev_netvsc/vdev_netvsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/vdev_netvsc/vdev_netvsc.c +1 -1
@@ -796,7 +796,7 @@ vdev_netvsc_scan_callback(__rte_unused void *arg)struct rte_devargs *devargs;struct rte_bus *vbus =rte_bus_find_by_name ("vdev" ); - TAILQ_FOREACH(devargs, &devargs_list, next) + RTE_EAL_DEVARGS_FOREACH("vdev", devargs)if (!strcmp (devargs->name,VDEV_NETVSC_DRIVER_NAME ))return ; dev = (struct rte_vdev_device *)vbus->find_device(NULL,
Signature · 1 line
2.11.0
PATCH v4 6/10 test: remove devargs unit tests
In reply to Gaetan RivetThe current test will not be compatible anymore with a private devargs list.
Moreover, the new functions should have new tests, while the existing API will be removed.
The current unit tests are thus obsolete and hereby removed.
--- MAINTAINERS | 1 - test/test/Makefile | 1 - test/test/test_devargs.c | 103 ----------------------------------------------- 3 files changed, 105 deletions(-) delete mode 100644 test/test/test_devargs.c
MAINTAINERS +0 -1
@@ -145,7 +145,6 @@ F: test/test/test_common.c F: test/test/test_cpuflags.c F: test/test/test_cycles.c F: test/test/test_debug.c -F: test/test/test_devargs.c F: test/test/test_eal* F: test/test/test_errno.c F: test/test/test_interrupts.c
test/test/Makefile +0 -1
@@ -161,7 +161,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += test_distributor_perf.c SRCS-$(CONFIG_RTE_LIBRTE_REORDER) += test_reorder.c -SRCS-y += test_devargs.c SRCS-y += virtual_pmd.c SRCS-y += packet_burst_generator.c SRCS-$(CONFIG_RTE_LIBRTE_ACL) += test_acl.c
test/test/test_devargs.c deleted +0 -103
103 lines not on this page · raw message
Signature · 1 line
2.11.0
PATCH v4 7/10 devargs: make devargs list private
In reply to Gaetan RivetInitially, rte_devargs was meant to be populated once and sometimes accessed, then never emptied.
With the new hotplug functionality having better standing, new usage appeared with repeated addition of devices and their subsequent removal.
Exposing devargs_list pushed bus drivers and libraries to be careless and inconsistent in their memory management. Making it private will allow to rationalize this part of the EAL and ensure that fewer memory leaks occur during operations.
--- lib/librte_eal/common/eal_common_devargs.c | 3 +++ lib/librte_eal/common/include/rte_devargs.h | 6 ------ lib/librte_eal/rte_eal_version.map | 1 - 3 files changed, 3 insertions(+), 7 deletions(-)
lib/librte_eal/common/eal_common_devargs.c +3 -0
@@ -18,6 +18,9 @@#include <rte_tailq.h> #include "eal_private.h" +/** user device double-linked queue type definition */ +TAILQ_HEAD (rte_devargs_list, rte_devargs); +/** Global list of user devices */ struct rte_devargs_list devargs_list =TAILQ_HEAD_INITIALIZER (devargs_list);
lib/librte_eal/common/include/rte_devargs.h +0 -6
@@ -59,12 +59,6 @@ struct rte_devargs {char *args; }; -/** user device double-linked queue type definition */ -TAILQ_HEAD (rte_devargs_list, rte_devargs); - -/** Global list of user devices */ -extern struct rte_devargs_list devargs_list; -/** * Parse a devargs string. *
lib/librte_eal/rte_eal_version.map +0 -1
@@ -2,7 +2,6 @@ DPDK_2.0 { global: __rte_panic; - devargs_list; eal_parse_sysfs_value; eal_timer_source; lcore_config;
Signature · 1 line
2.11.0
PATCH v4 8/10 devargs: make parsing variadic
In reply to Gaetan Rivetrte_eal_devargs_parse can be used by EAL subsystems, drivers, applications alike.
Device parameters may be presented with different structure each time; as a single declaration string or several strings each describing different parts of the declaration.
To simplify the use of this parsing facility, its parameters are made variadic.
--- drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 33 ++++------------------------- lib/librte_eal/common/eal_common_devargs.c | 15 ++++++++++--- lib/librte_eal/common/include/rte_devargs.h | 26 ++++++++++++----------- 5 files changed, 32 insertions(+), 46 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_eal_devargs_parse (args, d); + ret =rte_eal_devargs_parse (d,"%s" , args);if (ret) {DEBUG ("devargs parsing failed with code %d" , ret);return ret;
drivers/net/failsafe/failsafe_eal.c +1 -1
@@ -69,7 +69,7 @@ fs_bus_init(struct rte_eth_dev *dev)else snprintf (devstr,sizeof (devstr),"%s" , rte_eth_devices[pid].device->name); - ret =rte_eal_devargs_parse (devstr, da); + ret =rte_eal_devargs_parse (da,"%s" , devstr);if (ret) {ERROR ("Probed devargs parsing failed with code" " %d" , ret);
lib/librte_eal/common/eal_common_dev.c +4 -29
@@ -114,29 +114,12 @@ int rte_eal_dev_detach(struct rte_device *dev)return ret; } -static char * -full_dev_name (const char *bus,const char *dev,const char *args) -{ -char *name; -size_t len; - - len =snprintf (NULL ,0 ,"%s:%s,%s" , bus, dev, args) +1 ; - name =calloc (1 , len); -if (name ==NULL ) { -RTE_LOG (ERR ,EAL ,"Could not allocate full device name\n" ); -return NULL ; - } -snprintf (name, len,"%s:%s,%s" , bus, dev, args); -return name; -} - int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,const char *devargs) {struct rte_bus *bus;struct rte_device *dev;struct rte_devargs *da; -char *name;int ret; bus =rte_bus_find_by_name (busname);
@@ -151,17 +134,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnreturn -ENOTSUP ; } - name =full_dev_name (busname, devname, devargs); -if (name ==NULL ) + da =calloc (1 ,sizeof (*da)); +if (da ==NULL )return -ENOMEM ; - da =calloc (1 ,sizeof (*da)); -if (da ==NULL ) { - ret = -ENOMEM ; - goto err_name; - } - - ret =rte_eal_devargs_parse (name, da); + ret =rte_eal_devargs_parse (da,"%s:%s,%s" , + busname, devname, devargs);if (ret) goto err_devarg;
@@ -187,7 +165,6 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn dev->name); goto err_devarg; } -free (name);return 0 ; err_devarg:
@@ -195,8 +172,6 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnfree (da->args);free (da); } -err_name: -free (name);return ret; }
lib/librte_eal/common/eal_common_devargs.c +12 -3
@@ -11,6 +11,7 @@#include <stdio.h> #include <string.h> +#include <stdarg.h> #include <rte_compat.h> #include <rte_dev.h>
@@ -62,15 +63,23 @@ bus_name_cmp(const struct rte_bus *bus, const void *name) } int __rte_experimental -rte_eal_devargs_parse (const char *dev,struct rte_devargs *da) +rte_eal_devargs_parse (struct rte_devargs *da,const char *format, ...) {struct rte_bus *bus =NULL ; +va_list ap; +va_start (ap, format); +char dev[vsnprintf (NULL ,0 , format, ap) +1 ];const char *devname;const size_t maxlen =sizeof (da->name);size_t i; -if (dev ==NULL || da ==NULL ) +va_end (ap); +if (da ==NULL )return -EINVAL ; + +va_start (ap, format); +vsnprintf (dev,sizeof (dev), format, ap); +va_end (ap);/* Retrieve eventual bus info */ do { devname = dev;
@@ -140,7 +149,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)if (devargs ==NULL ) goto fail; -if (rte_eal_devargs_parse (dev, devargs)) +if (rte_eal_devargs_parse (devargs,"%s" , dev)) goto fail; devargs->type = devtype; bus = devargs->bus;
lib/librte_eal/common/include/rte_devargs.h +14 -12
@@ -94,18 +94,21 @@ int rte_eal_parse_devargs_str(const char *devargs_str,* in argument. Store which bus will handle the device, its name * and the eventual device parameters. * -* @param dev -* The device declaration string. +* The device string is built with a printf-like syntax. +* * @param da * The devargs structure holding the device information. +* @param format +* Format string describing a device. * * @return * - 0 on success. * - Negative errno on error. */ int __rte_experimental -rte_eal_devargs_parse (const char *dev, -struct rte_devargs *da); +rte_eal_devargs_parse (struct rte_devargs *da, +const char *format, ...) +__attribute__((format (printf,2 ,0 )));/** * Insert an rte_devargs in the global list.
@@ -123,15 +126,14 @@ rte_eal_devargs_insert(struct rte_devargs *da);/** * Add a device to the user device list * -* For PCI devices, the format of arguments string is "PCI_ADDR" or -* "PCI_ADDR,key=val,key2=val2,...". Examples: "08:00.1", "0000:5:00.0", -* "04:00.0,arg=val". +* The format is * -* For virtual devices, the format of arguments string is "DRIVER_NAME*" -* or "DRIVER_NAME*,key=val,key2=val2,...". Examples: "net_ring", -* "net_ring0", "net_pmdAnything,arg=0:arg2=1". The validity of the -* driver name is not checked by this function, it is done when probing -* the drivers. +* bus:device_identifier,arg1=val1,arg2=val2 +* +* Examples: +* +* pci:0000:05.00.0,arg=val +* vdev:net_ring0 * * @param devtype * The type of the device.
Signature · 1 line
2.11.0
PATCH v4 9/10 devargs: use proper namespace prefix
In reply to Gaetan Rivetrte_eal_devargs is useless, rte_devargs is sufficient.
Only experimental functions are changed for now.
--- drivers/bus/vdev/vdev.c | 6 +++--- drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 8 ++++---- lib/librte_eal/common/eal_common_devargs.c | 12 ++++++------ lib/librte_eal/common/include/rte_devargs.h | 23 +++++++++++++---------- lib/librte_eal/rte_eal_version.map | 8 ++++---- 7 files changed, 32 insertions(+), 29 deletions(-)
drivers/bus/vdev/vdev.c +3 -3
@@ -228,7 +228,7 @@ rte_vdev_init(const char *name, const char *args) goto fail; } -rte_eal_devargs_insert (devargs); +rte_devargs_insert (devargs);TAILQ_INSERT_TAIL (&vdev_device_list, dev, next);return 0 ;
@@ -278,7 +278,7 @@ rte_vdev_uninit(const char *name)TAILQ_REMOVE (&vdev_device_list, dev, next); -rte_eal_devargs_remove (devargs->bus->name, devargs->name); +rte_devargs_remove (devargs->bus->name, devargs->name);free (dev);return 0 ;
@@ -297,7 +297,7 @@ vdev_scan(void)if (custom_scan->callback !=NULL )/* * the callback should update devargs list -* by calling rte_eal_devargs_insert() with +* by calling rte_devargs_insert() with * devargs.bus = rte_bus_find_by_name("vdev"); * devargs.type = RTE_DEVTYPE_VIRTUAL; * devargs.policy = RTE_DEV_WHITELISTED;
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_eal_devargs_parse (d,"%s" , args); + ret =rte_devargs_parse (d,"%s" , args);if (ret) {DEBUG ("devargs parsing failed with code %d" , ret);return ret;
drivers/net/failsafe/failsafe_eal.c +1 -1
@@ -69,7 +69,7 @@ fs_bus_init(struct rte_eth_dev *dev)else snprintf (devstr,sizeof (devstr),"%s" , rte_eth_devices[pid].device->name); - ret =rte_eal_devargs_parse (da,"%s" , devstr); + ret =rte_devargs_parse (da,"%s" , devstr);if (ret) {ERROR ("Probed devargs parsing failed with code" " %d" , ret);
lib/librte_eal/common/eal_common_dev.c +4 -4
@@ -138,12 +138,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnif (da ==NULL )return -ENOMEM ; - ret =rte_eal_devargs_parse (da,"%s:%s,%s" , + ret =rte_devargs_parse (da,"%s:%s,%s" , busname, devname, devargs);if (ret) goto err_devarg; - ret =rte_eal_devargs_insert (da); + ret =rte_devargs_insert (da);if (ret) goto err_devarg;
@@ -168,7 +168,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnreturn 0 ; err_devarg: -if (rte_eal_devargs_remove (busname, devname)) { +if (rte_devargs_remove (busname, devname)) {free (da->args);free (da); }
@@ -204,7 +204,7 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)if (ret)RTE_LOG (ERR ,EAL ,"Driver cannot detach the device (%s)\n" , dev->name); -rte_eal_devargs_remove (busname, devname); +rte_devargs_remove (busname, devname);return ret; }
lib/librte_eal/common/eal_common_devargs.c +6 -6
@@ -63,7 +63,7 @@ bus_name_cmp(const struct rte_bus *bus, const void *name) } int __rte_experimental -rte_eal_devargs_parse (struct rte_devargs *da,const char *format, ...) +rte_devargs_parse (struct rte_devargs *da,const char *format, ...) {struct rte_bus *bus =NULL ;va_list ap;
@@ -125,11 +125,11 @@ rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...) } int __rte_experimental -rte_eal_devargs_insert (struct rte_devargs *da) +rte_devargs_insert (struct rte_devargs *da) {int ret; - ret =rte_eal_devargs_remove (da->bus->name, da->name); + ret =rte_devargs_remove (da->bus->name, da->name);if (ret <0 )return ret;TAILQ_INSERT_TAIL (&devargs_list, da, next);
@@ -149,7 +149,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)if (devargs ==NULL ) goto fail; -if (rte_eal_devargs_parse (devargs,"%s" , dev)) +if (rte_devargs_parse (devargs,"%s" , dev)) goto fail; devargs->type = devtype; bus = devargs->bus;
@@ -174,7 +174,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) } int __rte_experimental -rte_eal_devargs_remove (const char *busname,const char *devname) +rte_devargs_remove (const char *busname,const char *devname) {struct rte_devargs *d;void *tmp;
@@ -223,7 +223,7 @@ rte_eal_devargs_dump(FILE *f)/* bus-aware rte_devargs iterator. */ __rte_experimental struct rte_devargs * -rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +rte_devargs_next (const char *busname,const struct rte_devargs *start) {struct rte_devargs *da;
lib/librte_eal/common/include/rte_devargs.h +13 -10
@@ -105,9 +105,10 @@ int rte_eal_parse_devargs_str(const char *devargs_str,* - 0 on success. * - Negative errno on error. */ -int __rte_experimental -rte_eal_devargs_parse (struct rte_devargs *da, -const char *format, ...) +__rte_experimental +int +rte_devargs_parse (struct rte_devargs *da, +const char *format, ...) __attribute__((format (printf,2 ,0 )));/**
@@ -120,8 +121,9 @@ __attribute__((format(printf, 2, 0)));* - 0 on success * - Negative on error. */ -int __rte_experimental -rte_eal_devargs_insert (struct rte_devargs *da); +__rte_experimental +int +rte_devargs_insert (struct rte_devargs *da);/** * Add a device to the user device list
@@ -162,8 +164,9 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str);* <0 on error. * >0 if the devargs was not within the user device list. */ -int __rte_experimental rte_eal_devargs_remove(const char *busname, -const char *devname); +__rte_experimental +int rte_devargs_remove(const char *busname, +const char *devname);/** * Count the number of user devices of a specified type
@@ -203,15 +206,15 @@ void rte_eal_devargs_dump(FILE *f);*/ __rte_experimental struct rte_devargs * -rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); +rte_devargs_next (const char *busname,const struct rte_devargs *start);/** * Iterate over all rte_devargs for a specific bus. */ #define RTE_EAL_DEVARGS_FOREACH (busname, da) \ - for (da = rte_eal_devargs_next(busname, NULL); \ + for (da = rte_devargs_next(busname, NULL); \ da != NULL; \ - da = rte_eal_devargs_next(busname, da)) \ + da = rte_devargs_next(busname, da)) \ #ifdef __cplusplus }
lib/librte_eal/rte_eal_version.map +4 -4
@@ -217,10 +217,10 @@ EXPERIMENTAL { rte_dev_event_monitor_start; rte_dev_event_monitor_stop; rte_eal_cleanup; - rte_eal_devargs_insert; - rte_eal_devargs_next; - rte_eal_devargs_parse; - rte_eal_devargs_remove; + rte_devargs_insert; + rte_devargs_next; + rte_devargs_parse; + rte_devargs_remove; rte_eal_hotplug_add; rte_eal_hotplug_remove; rte_eal_mbuf_user_pool_ops;
Signature · 1 line
2.11.0
PATCH v4 10/10 devargs: rename legacy API
In reply to Gaetan RivetThis previous symbols were deprecated for two releases. They are now marked as such and cannot be used anymore.
They are replaced by ones respecting the new namespace that are marked experimental.
As a result, eth_dev attach and detach is slightly reworked to follow the changes.
--- app/test-pmd/cmdline.c | 2 +- drivers/net/vdev_netvsc/vdev_netvsc.c | 2 +- examples/bond/Makefile | 1 + examples/bond/main.c | 2 +- lib/librte_eal/common/eal_common_devargs.c | 9 +++-- lib/librte_eal/common/eal_common_options.c | 2 +- lib/librte_eal/common/include/rte_devargs.h | 54 +++++++++++++++++++++++++++++ lib/librte_eal/rte_eal_version.map | 3 ++ lib/librte_ether/rte_ethdev.c | 44 +++++++++++------------ test/test/commands.c | 2 +- 10 files changed, 89 insertions(+), 32 deletions(-)
app/test-pmd/cmdline.c +1 -1
@@ -8754,7 +8754,7 @@ static void cmd_dump_parsed(void *parsed_result,else if (!strcmp (res->dump,"dump_mempool" ))rte_mempool_list_dump (stdout);else if (!strcmp (res->dump,"dump_devargs" )) -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);else if (!strcmp (res->dump,"dump_log_types" ))rte_log_dump (stdout); }
drivers/net/vdev_netvsc/vdev_netvsc.c +1 -1
@@ -803,7 +803,7 @@ vdev_netvsc_scan_callback(__rte_unused void *arg) vdev_netvsc_cmp_rte_device, VDEV_NETVSC_DRIVER_NAME);if (dev)return ; -if (rte_eal_devargs_add (RTE_DEVTYPE_VIRTUAL ,VDEV_NETVSC_DRIVER_NAME )) +if (rte_devargs_add (RTE_DEVTYPE_VIRTUAL ,VDEV_NETVSC_DRIVER_NAME ))DRV_LOG (ERR ,"unable to add netvsc devargs." ); }
examples/bond/Makefile +1 -0
@@ -62,6 +62,7 @@ endif CFLAGS += -DALLOW_EXPERIMENTAL_API CFLAGS += -O3 +CFLAGS += -DALLOW_EXPERIMENTAL_API ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y) LDLIBS += -lrte_pmd_bond
examples/bond/main.c +1 -1
@@ -742,7 +742,7 @@ main(int argc, char *argv[])/* init EAL */ ret =rte_eal_init (argc, argv); -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);if (ret <0 )rte_exit (EXIT_FAILURE ,"Error with EAL initialization\n" ); argc -= ret;
lib/librte_eal/common/eal_common_devargs.c +6 -3
@@ -137,8 +137,9 @@ rte_devargs_insert(struct rte_devargs *da) }/* store a whitelist parameter for later parsing */ +__rte_experimental int -rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) +rte_devargs_add (enum rte_devtype devtype,const char *devargs_str) {struct rte_devargs *devargs =NULL ;struct rte_bus *bus =NULL ;
@@ -192,8 +193,9 @@ rte_devargs_remove(const char *busname, const char *devname) }/* count the number of devices of a specified type */ +__rte_experimental unsigned int -rte_eal_devargs_type_count (enum rte_devtype devtype) +rte_devargs_type_count (enum rte_devtype devtype) {struct rte_devargs *devargs;unsigned int count =0 ;
@@ -207,8 +209,9 @@ rte_eal_devargs_type_count(enum rte_devtype devtype) }/* dump the user devices on the console */ +__rte_experimental void -rte_eal_devargs_dump (FILE *f) +rte_devargs_dump (FILE *f) {struct rte_devargs *devargs;
lib/librte_eal/common/eal_common_options.c +1 -1
@@ -153,7 +153,7 @@ eal_option_device_parse(void) TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {if (ret ==0 ) { - ret =rte_eal_devargs_add (devopt->type, devopt->arg); + ret =rte_devargs_add (devopt->type, devopt->arg);if (ret)RTE_LOG (ERR ,EAL ,"Unable to parse device '%s'\n" , devopt->arg);
lib/librte_eal/common/include/rte_devargs.h +54 -0
84 lines not on this page · raw message
lib/librte_eal/rte_eal_version.map +3 -0
@@ -217,10 +217,13 @@ EXPERIMENTAL { rte_dev_event_monitor_start; rte_dev_event_monitor_stop; rte_eal_cleanup; + rte_devargs_add; + rte_devargs_dump; rte_devargs_insert; rte_devargs_next; rte_devargs_parse; rte_devargs_remove; + rte_devargs_type_count; rte_eal_hotplug_add; rte_eal_hotplug_remove; rte_eal_mbuf_user_pool_ops;
lib/librte_ether/rte_ethdev.c +20 -24
92 lines not on this page · raw message
test/test/commands.c +1 -1
@@ -132,7 +132,7 @@ static void cmd_dump_parsed(void *parsed_result,else if (!strcmp (res->dump,"dump_mempool" ))rte_mempool_list_dump (stdout);else if (!strcmp (res->dump,"dump_devargs" )) -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);else if (!strcmp (res->dump,"dump_log_types" ))rte_log_dump (stdout);else if (!strcmp (res->dump,"dump_malloc_stats" ))
Signature · 1 line
2.11.0
Re: [PATCH v4 01/10] devargs: introduce iterator
In reply to Gaetan RivetQuoted 32 lines · On Tue, 24 Apr 2018 00:41:01 +0200 Gaetan Rivet <[email protected]> wrote:
In preparation to making
devargs_listprivate.Bus drivers generally need to access
rte_devargspertaining to their operations. This match is a common operation for bus drivers.Add a new accessor for the
rte_devargslist.Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_devargs.c | 20 ++++++++++++++++++++ lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 41 insertions(+)lib/librte_eal/common/eal_common_devargs.c +10
-0@@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f) devargs->name, devargs->args); } } + +/* bus-aware rte_devargs iterator. */ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +{ +struct rte_devargs *da; + +if (start !=NULL ) + da =TAILQ_NEXT (start, next);
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_dump(FILE *f) · line 229
Quoted 10 lines
lib/librte_eal/common/eal_common_devargs.c +10
-0+else + da =TAILQ_FIRST (&devargs_list); +while (da !=NULL ) { +if (busname ==NULL || + (strcmp (busname, da->bus->name) ==0 )) +return da; + da =TAILQ_NEXT (da, next); + } +return NULL ; +}
Can this be made to return a const pointer? It seems unsymmetrical to get the next value as a non-const pointer when passed a const based on last value.
v5 cover devargs cleanup
In reply to Gaetan RivetCleanup the rte_devargs API.
This is a continuous process.
The previous version of this patchset was dependent on changes on the PCI bus and the rte_bus library. Not all these changes were integrated. This patchset has thus been limited to elements that are available right now, without dependencies.
The devargs list is made private. All devargs users are meant to use them using the proper API. Devargs will allocate dynamic memory. With device hotplug, this memory is usually mishandled and lost by libraries not following the evolutions of rte_devargs.
v4:
* Rebased on master.
v5:
* Merged iterator patches
* Fixed rte_devargs_add documentation in its own patch.
* Updated deprecation notice when relevant.
Gaetan Rivet (10):
devargs: introduce iterator
bus/vdev: do not reference devargs list
bus/pci: do not reference devargs list
net/vdev_netvsc: do not reference devargs list
test: remove devargs unit tests
devargs: make devargs list private
devargs: make parsing variadic
devargs: update devargs add documentation
devargs: use proper namespace prefix
devargs: rename legacy API
MAINTAINERS | 1 - app/test-pmd/cmdline.c | 2 +- doc/guides/rel_notes/deprecation.rst | 5 -- drivers/bus/pci/Makefile | 1 + drivers/bus/pci/pci_common.c | 6 +- drivers/bus/vdev/Makefile | 1 + drivers/bus/vdev/vdev.c | 13 +-- drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- drivers/net/vdev_netvsc/vdev_netvsc.c | 4 +- examples/bond/Makefile | 1 + examples/bond/main.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 39 ++------- lib/librte_eal/common/eal_common_devargs.c | 53 ++++++++++-- lib/librte_eal/common/eal_common_options.c | 2 +- lib/librte_eal/common/include/rte_devargs.h | 130 ++++++++++++++++++++++------ lib/librte_eal/rte_eal_version.map | 11 ++- lib/librte_ether/rte_ethdev.c | 44 +++++----- test/test/Makefile | 1 - test/test/commands.c | 2 +- test/test/test_devargs.c | 103 ---------------------- 21 files changed, 199 insertions(+), 226 deletions(-) delete mode 100644 test/test/test_devargs.c
Signature · 1 line
2.11.0
PATCH v5 1/10 devargs: introduce iterator
In preparation to making devargs_list private.
Bus drivers generally need to access rte_devargs pertaining to their operations. This match is a common operation for bus drivers.
Add a new accessor for the rte_devargs list.
--- lib/librte_eal/common/eal_common_devargs.c | 20 ++++++++++++++++++++ lib/librte_eal/common/include/rte_devargs.h | 28 ++++++++++++++++++++++++++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 49 insertions(+)
lib/librte_eal/common/eal_common_devargs.c +20 -0
@@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f) devargs->name, devargs->args); } } + +/* bus-aware rte_devargs iterator. */ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +{ +struct rte_devargs *da; + +if (start !=NULL ) + da =TAILQ_NEXT (start, next); +else + da =TAILQ_FIRST (&devargs_list); +while (da !=NULL ) { +if (busname ==NULL || + (strcmp (busname, da->bus->name) ==0 )) +return da; + da =TAILQ_NEXT (da, next); + } +return NULL ; +}
lib/librte_eal/common/include/rte_devargs.h +28 -0
@@ -189,6 +189,34 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);*/ void rte_eal_devargs_dump (FILE *f); +/** +* Find next rte_devargs matching the provided bus name. +* +* @param busname +* Limit the iteration to devargs related to buses +* matching this name. +* Will return any next rte_devargs if NULL. +* +* @param start +* Starting iteration point. The iteration will start at +* the first rte_devargs if NULL. +* +* @return +* Next rte_devargs entry matching the requested bus, +* NULL if there is none. +*/ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); + +/** +* Iterate over all rte_devargs for a specific bus. +*/ +#define RTE_EAL_DEVARGS_FOREACH (busname, da) \ + for (da = rte_eal_devargs_next(busname, NULL); \ + da != NULL; \ + da = rte_eal_devargs_next(busname, da)) \ + #ifdef __cplusplus }#endif
lib/librte_eal/rte_eal_version.map +1 -0
@@ -219,6 +219,7 @@ EXPERIMENTAL { rte_dev_event_monitor_stop; rte_eal_cleanup; rte_eal_devargs_insert; + rte_eal_devargs_next; rte_eal_devargs_parse; rte_eal_devargs_remove; rte_eal_hotplug_add;
Signature · 1 line
2.11.0
PATCH v5 2/10 bus/vdev: do not reference devargs list
In reply to Gaetan RivetThis list should not be operated upon by drivers. Use the public API to achieve the same functionalities.
--- drivers/bus/vdev/Makefile | 1 + drivers/bus/vdev/vdev.c | 11 +++-------- 2 files changed, 4 insertions(+), 8 deletions(-)
drivers/bus/vdev/Makefile +1 -0
@@ -10,6 +10,7 @@ LIB = librte_bus_vdev.a CFLAGS += -O3 CFLAGS += $(WERROR_FLAGS) +CFLAGS += -DALLOW_EXPERIMENTAL_API # versioning export map EXPORT_MAP := rte_bus_vdev_version.map
drivers/bus/vdev/vdev.c +3 -8
@@ -228,7 +228,7 @@ rte_vdev_init(const char *name, const char *args) goto fail; } -TAILQ_INSERT_TAIL (&devargs_list, devargs, next); +rte_eal_devargs_insert (devargs);TAILQ_INSERT_TAIL (&vdev_device_list, dev, next);return 0 ;
@@ -278,10 +278,8 @@ rte_vdev_uninit(const char *name)TAILQ_REMOVE (&vdev_device_list, dev, next); -TAILQ_REMOVE (&devargs_list, devargs, next); +rte_eal_devargs_remove (devargs->bus->name, devargs->name); -free (devargs->args); -free (devargs);free (dev);return 0 ; }
@@ -309,10 +307,7 @@ vdev_scan(void)rte_spinlock_unlock (&vdev_custom_scan_lock);/* for virtual devices we scan the devargs_list populated via cmdline */ - TAILQ_FOREACH(devargs, &devargs_list, next) { - -if (devargs->bus != &rte_vdev_bus) -continue ; + RTE_EAL_DEVARGS_FOREACH("vdev", devargs) { dev =find_vdev (devargs->name);if (dev)
Signature · 1 line
2.11.0
PATCH v5 3/10 bus/pci: do not reference devargs list
In reply to Gaetan RivetThis list should not be used by drivers.
Use the public API instead.
--- drivers/bus/pci/Makefile | 1 + drivers/bus/pci/pci_common.c | 6 +----- 2 files changed, 2 insertions(+), 5 deletions(-)
drivers/bus/pci/Makefile +1 -0
@@ -37,6 +37,7 @@ EXPORT_MAP := rte_bus_pci_version.map CFLAGS := -I$(SRCDIR) $(CFLAGS) CFLAGS += -O3 $(WERROR_FLAGS) +CFLAGS += -DALLOW_EXPERIMENTAL_API ifneq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),) SYSTEM := linux
drivers/bus/pci/pci_common.c +1 -5
@@ -45,12 +45,8 @@ static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev) {struct rte_devargs *devargs;struct rte_pci_addr addr; -struct rte_bus *pbus; - pbus =rte_bus_find_by_name ("pci" ); - TAILQ_FOREACH(devargs, &devargs_list, next) { -if (devargs->bus != pbus) -continue ; + RTE_EAL_DEVARGS_FOREACH("pci", devargs) { devargs->bus->parse (devargs->name, &addr);if (!rte_pci_addr_cmp (&dev->addr, &addr))return devargs;
Signature · 1 line
2.11.0
PATCH v5 4/10 net/vdev_netvsc: do not reference devargs list
In reply to Gaetan RivetThis list should not be used by drivers.
Use the public API instead.
--- drivers/net/vdev_netvsc/vdev_netvsc.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/net/vdev_netvsc/vdev_netvsc.c +1 -1
@@ -796,7 +796,7 @@ vdev_netvsc_scan_callback(__rte_unused void *arg)struct rte_devargs *devargs;struct rte_bus *vbus =rte_bus_find_by_name ("vdev" ); - TAILQ_FOREACH(devargs, &devargs_list, next) + RTE_EAL_DEVARGS_FOREACH("vdev", devargs)if (!strcmp (devargs->name,VDEV_NETVSC_DRIVER_NAME ))return ; dev = (struct rte_vdev_device *)vbus->find_device(NULL,
Signature · 1 line
2.11.0
PATCH v5 5/10 test: remove devargs unit tests
In reply to Gaetan RivetThe current test will not be compatible anymore with a private devargs list.
Moreover, the new functions should have new tests, while the existing API will be removed.
The current unit tests are thus obsolete and hereby removed.
--- MAINTAINERS | 1 - test/test/Makefile | 1 - test/test/test_devargs.c | 103 ----------------------------------------------- 3 files changed, 105 deletions(-) delete mode 100644 test/test/test_devargs.c
MAINTAINERS +0 -1
@@ -145,7 +145,6 @@ F: test/test/test_common.c F: test/test/test_cpuflags.c F: test/test/test_cycles.c F: test/test/test_debug.c -F: test/test/test_devargs.c F: test/test/test_eal* F: test/test/test_errno.c F: test/test/test_interrupts.c
test/test/Makefile +0 -1
@@ -161,7 +161,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += test_distributor_perf.c SRCS-$(CONFIG_RTE_LIBRTE_REORDER) += test_reorder.c -SRCS-y += test_devargs.c SRCS-y += virtual_pmd.c SRCS-y += packet_burst_generator.c SRCS-$(CONFIG_RTE_LIBRTE_ACL) += test_acl.c
test/test/test_devargs.c deleted +0 -103
103 lines not on this page · raw message
Signature · 1 line
2.11.0
PATCH v5 6/10 devargs: make devargs list private
In reply to Gaetan RivetInitially, rte_devargs was meant to be populated once and sometimes accessed, then never emptied.
With the new hotplug functionality having better standing, new usage appeared with repeated addition of devices and their subsequent removal.
Exposing devargs_list pushed bus drivers and libraries to be careless and inconsistent in their memory management. Making it private will allow to rationalize this part of the EAL and ensure that fewer memory leaks occur during operations.
--- doc/guides/rel_notes/deprecation.rst | 1 - lib/librte_eal/common/eal_common_devargs.c | 3 +++ lib/librte_eal/common/include/rte_devargs.h | 6 ------ lib/librte_eal/rte_eal_version.map | 1 - 4 files changed, 3 insertions(+), 8 deletions(-)
doc/guides/rel_notes/deprecation.rst +0 -1
@@ -16,7 +16,6 @@ Deprecation Notices - removal of ``name`` and ``args`` fields. - The enum ``rte_devtype`` was used to identify a bus and will disappear. - - The ``rte_devargs_list`` will be made private. - Functions previously deprecated will change or disappear: + ``rte_eal_devargs_add``
lib/librte_eal/common/eal_common_devargs.c +3 -0
@@ -18,6 +18,9 @@#include <rte_tailq.h> #include "eal_private.h" +/** user device double-linked queue type definition */ +TAILQ_HEAD (rte_devargs_list, rte_devargs); +/** Global list of user devices */ struct rte_devargs_list devargs_list =TAILQ_HEAD_INITIALIZER (devargs_list);
lib/librte_eal/common/include/rte_devargs.h +0 -6
@@ -59,12 +59,6 @@ struct rte_devargs {char *args; }; -/** user device double-linked queue type definition */ -TAILQ_HEAD (rte_devargs_list, rte_devargs); - -/** Global list of user devices */ -extern struct rte_devargs_list devargs_list; -/** * Parse a devargs string. *
lib/librte_eal/rte_eal_version.map +0 -1
@@ -2,7 +2,6 @@ DPDK_2.0 { global: __rte_panic; - devargs_list; eal_parse_sysfs_value; eal_timer_source; lcore_config;
Signature · 1 line
2.11.0
PATCH v5 7/10 devargs: make parsing variadic
In reply to Gaetan Rivetrte_eal_devargs_parse can be used by EAL subsystems, drivers, applications alike.
Device parameters may be presented with different structure each time; as a single declaration string or several strings each describing different parts of the declaration.
To simplify the use of this parsing facility, its parameters are made variadic.
--- drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 33 ++++------------------------- lib/librte_eal/common/eal_common_devargs.c | 15 ++++++++++--- lib/librte_eal/common/include/rte_devargs.h | 11 ++++++---- 5 files changed, 25 insertions(+), 38 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_eal_devargs_parse (args, d); + ret =rte_eal_devargs_parse (d,"%s" , args);if (ret) {DEBUG ("devargs parsing failed with code %d" , ret);return ret;
drivers/net/failsafe/failsafe_eal.c +1 -1
@@ -69,7 +69,7 @@ fs_bus_init(struct rte_eth_dev *dev)else snprintf (devstr,sizeof (devstr),"%s" , rte_eth_devices[pid].device->name); - ret =rte_eal_devargs_parse (devstr, da); + ret =rte_eal_devargs_parse (da,"%s" , devstr);if (ret) {ERROR ("Probed devargs parsing failed with code" " %d" , ret);
lib/librte_eal/common/eal_common_dev.c +4 -29
@@ -114,29 +114,12 @@ int rte_eal_dev_detach(struct rte_device *dev)return ret; } -static char * -full_dev_name (const char *bus,const char *dev,const char *args) -{ -char *name; -size_t len; - - len =snprintf (NULL ,0 ,"%s:%s,%s" , bus, dev, args) +1 ; - name =calloc (1 , len); -if (name ==NULL ) { -RTE_LOG (ERR ,EAL ,"Could not allocate full device name\n" ); -return NULL ; - } -snprintf (name, len,"%s:%s,%s" , bus, dev, args); -return name; -} - int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,const char *devargs) {struct rte_bus *bus;struct rte_device *dev;struct rte_devargs *da; -char *name;int ret; bus =rte_bus_find_by_name (busname);
@@ -151,17 +134,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnreturn -ENOTSUP ; } - name =full_dev_name (busname, devname, devargs); -if (name ==NULL ) + da =calloc (1 ,sizeof (*da)); +if (da ==NULL )return -ENOMEM ; - da =calloc (1 ,sizeof (*da)); -if (da ==NULL ) { - ret = -ENOMEM ; - goto err_name; - } - - ret =rte_eal_devargs_parse (name, da); + ret =rte_eal_devargs_parse (da,"%s:%s,%s" , + busname, devname, devargs);if (ret) goto err_devarg;
@@ -187,7 +165,6 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn dev->name); goto err_devarg; } -free (name);return 0 ; err_devarg:
@@ -195,8 +172,6 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnfree (da->args);free (da); } -err_name: -free (name);return ret; }
lib/librte_eal/common/eal_common_devargs.c +12 -3
@@ -11,6 +11,7 @@#include <stdio.h> #include <string.h> +#include <stdarg.h> #include <rte_compat.h> #include <rte_dev.h>
@@ -62,15 +63,23 @@ bus_name_cmp(const struct rte_bus *bus, const void *name) } int __rte_experimental -rte_eal_devargs_parse (const char *dev,struct rte_devargs *da) +rte_eal_devargs_parse (struct rte_devargs *da,const char *format, ...) {struct rte_bus *bus =NULL ; +va_list ap; +va_start (ap, format); +char dev[vsnprintf (NULL ,0 , format, ap) +1 ];const char *devname;const size_t maxlen =sizeof (da->name);size_t i; -if (dev ==NULL || da ==NULL ) +va_end (ap); +if (da ==NULL )return -EINVAL ; + +va_start (ap, format); +vsnprintf (dev,sizeof (dev), format, ap); +va_end (ap);/* Retrieve eventual bus info */ do { devname = dev;
@@ -140,7 +149,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)if (devargs ==NULL ) goto fail; -if (rte_eal_devargs_parse (dev, devargs)) +if (rte_eal_devargs_parse (devargs,"%s" , dev)) goto fail; devargs->type = devtype; bus = devargs->bus;
lib/librte_eal/common/include/rte_devargs.h +7 -4
@@ -94,18 +94,21 @@ int rte_eal_parse_devargs_str(const char *devargs_str,* in argument. Store which bus will handle the device, its name * and the eventual device parameters. * -* @param dev -* The device declaration string. +* The device string is built with a printf-like syntax. +* * @param da * The devargs structure holding the device information. +* @param format +* Format string describing a device. * * @return * - 0 on success. * - Negative errno on error. */ int __rte_experimental -rte_eal_devargs_parse (const char *dev, -struct rte_devargs *da); +rte_eal_devargs_parse (struct rte_devargs *da, +const char *format, ...) +__attribute__((format (printf,2 ,0 )));/** * Insert an rte_devargs in the global list.
Signature · 1 line
2.11.0
PATCH v5 8/10 devargs: update devargs add documentation
In reply to Gaetan RivetDevice syntax documentation is out of date.
--- lib/librte_eal/common/include/rte_devargs.h | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-)
lib/librte_eal/common/include/rte_devargs.h +16 -10
@@ -96,6 +96,21 @@ int rte_eal_parse_devargs_str(const char *devargs_str,* * The device string is built with a printf-like syntax. * +* 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 format
@@ -125,16 +140,7 @@ rte_eal_devargs_insert(struct rte_devargs *da);/** * Add a device to the user device list -* -* For PCI devices, the format of arguments string is "PCI_ADDR" or -* "PCI_ADDR,key=val,key2=val2,...". Examples: "08:00.1", "0000:5:00.0", -* "04:00.0,arg=val". -* -* For virtual devices, the format of arguments string is "DRIVER_NAME*" -* or "DRIVER_NAME*,key=val,key2=val2,...". Examples: "net_ring", -* "net_ring0", "net_pmdAnything,arg=0:arg2=1". The validity of the -* driver name is not checked by this function, it is done when probing -* the drivers. +* See rte_eal_devargs_parse() for details. * * @param devtype * The type of the device.
Signature · 1 line
2.11.0
PATCH v5 9/10 devargs: use proper namespace prefix
In reply to Gaetan Rivetrte_eal_devargs is useless, rte_devargs is sufficient.
Only experimental functions are changed for now.
--- drivers/bus/vdev/vdev.c | 6 +++--- drivers/net/failsafe/failsafe_args.c | 2 +- drivers/net/failsafe/failsafe_eal.c | 2 +- lib/librte_eal/common/eal_common_dev.c | 8 ++++---- lib/librte_eal/common/eal_common_devargs.c | 12 ++++++------ lib/librte_eal/common/include/rte_devargs.h | 25 ++++++++++++++----------- lib/librte_eal/rte_eal_version.map | 8 ++++---- 7 files changed, 33 insertions(+), 30 deletions(-)
drivers/bus/vdev/vdev.c +3 -3
@@ -228,7 +228,7 @@ rte_vdev_init(const char *name, const char *args) goto fail; } -rte_eal_devargs_insert (devargs); +rte_devargs_insert (devargs);TAILQ_INSERT_TAIL (&vdev_device_list, dev, next);return 0 ;
@@ -278,7 +278,7 @@ rte_vdev_uninit(const char *name)TAILQ_REMOVE (&vdev_device_list, dev, next); -rte_eal_devargs_remove (devargs->bus->name, devargs->name); +rte_devargs_remove (devargs->bus->name, devargs->name);free (dev);return 0 ;
@@ -297,7 +297,7 @@ vdev_scan(void)if (custom_scan->callback !=NULL )/* * the callback should update devargs list -* by calling rte_eal_devargs_insert() with +* by calling rte_devargs_insert() with * devargs.bus = rte_bus_find_by_name("vdev"); * devargs.type = RTE_DEVTYPE_VIRTUAL; * devargs.policy = RTE_DEV_WHITELISTED;
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_eal_devargs_parse (d,"%s" , args); + ret =rte_devargs_parse (d,"%s" , args);if (ret) {DEBUG ("devargs parsing failed with code %d" , ret);return ret;
drivers/net/failsafe/failsafe_eal.c +1 -1
@@ -69,7 +69,7 @@ fs_bus_init(struct rte_eth_dev *dev)else snprintf (devstr,sizeof (devstr),"%s" , rte_eth_devices[pid].device->name); - ret =rte_eal_devargs_parse (da,"%s" , devstr); + ret =rte_devargs_parse (da,"%s" , devstr);if (ret) {ERROR ("Probed devargs parsing failed with code" " %d" , ret);
lib/librte_eal/common/eal_common_dev.c +4 -4
@@ -138,12 +138,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnif (da ==NULL )return -ENOMEM ; - ret =rte_eal_devargs_parse (da,"%s:%s,%s" , + ret =rte_devargs_parse (da,"%s:%s,%s" , busname, devname, devargs);if (ret) goto err_devarg; - ret =rte_eal_devargs_insert (da); + ret =rte_devargs_insert (da);if (ret) goto err_devarg;
@@ -168,7 +168,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devnreturn 0 ; err_devarg: -if (rte_eal_devargs_remove (busname, devname)) { +if (rte_devargs_remove (busname, devname)) {free (da->args);free (da); }
@@ -204,7 +204,7 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)if (ret)RTE_LOG (ERR ,EAL ,"Driver cannot detach the device (%s)\n" , dev->name); -rte_eal_devargs_remove (busname, devname); +rte_devargs_remove (busname, devname);return ret; }
lib/librte_eal/common/eal_common_devargs.c +6 -6
@@ -63,7 +63,7 @@ bus_name_cmp(const struct rte_bus *bus, const void *name) } int __rte_experimental -rte_eal_devargs_parse (struct rte_devargs *da,const char *format, ...) +rte_devargs_parse (struct rte_devargs *da,const char *format, ...) {struct rte_bus *bus =NULL ;va_list ap;
@@ -125,11 +125,11 @@ rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...) } int __rte_experimental -rte_eal_devargs_insert (struct rte_devargs *da) +rte_devargs_insert (struct rte_devargs *da) {int ret; - ret =rte_eal_devargs_remove (da->bus->name, da->name); + ret =rte_devargs_remove (da->bus->name, da->name);if (ret <0 )return ret;TAILQ_INSERT_TAIL (&devargs_list, da, next);
@@ -149,7 +149,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)if (devargs ==NULL ) goto fail; -if (rte_eal_devargs_parse (devargs,"%s" , dev)) +if (rte_devargs_parse (devargs,"%s" , dev)) goto fail; devargs->type = devtype; bus = devargs->bus;
@@ -174,7 +174,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str) } int __rte_experimental -rte_eal_devargs_remove (const char *busname,const char *devname) +rte_devargs_remove (const char *busname,const char *devname) {struct rte_devargs *d;void *tmp;
@@ -223,7 +223,7 @@ rte_eal_devargs_dump(FILE *f)/* bus-aware rte_devargs iterator. */ __rte_experimental struct rte_devargs * -rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +rte_devargs_next (const char *busname,const struct rte_devargs *start) {struct rte_devargs *da;
lib/librte_eal/common/include/rte_devargs.h +14 -11
@@ -120,9 +120,10 @@ int rte_eal_parse_devargs_str(const char *devargs_str,* - 0 on success. * - Negative errno on error. */ -int __rte_experimental -rte_eal_devargs_parse (struct rte_devargs *da, -const char *format, ...) +__rte_experimental +int +rte_devargs_parse (struct rte_devargs *da, +const char *format, ...) __attribute__((format (printf,2 ,0 )));/**
@@ -135,12 +136,13 @@ __attribute__((format(printf, 2, 0)));* - 0 on success * - Negative on error. */ -int __rte_experimental -rte_eal_devargs_insert (struct rte_devargs *da); +__rte_experimental +int +rte_devargs_insert (struct rte_devargs *da);/** * Add a device to the user device list -* See rte_eal_devargs_parse() for details. +* See rte_devargs_parse() for details. * * @param devtype * The type of the device.
@@ -169,8 +171,9 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str);* <0 on error. * >0 if the devargs was not within the user device list. */ -int __rte_experimental rte_eal_devargs_remove(const char *busname, -const char *devname); +__rte_experimental +int rte_devargs_remove(const char *busname, +const char *devname);/** * Count the number of user devices of a specified type
@@ -210,15 +213,15 @@ void rte_eal_devargs_dump(FILE *f);*/ __rte_experimental struct rte_devargs * -rte_eal_devargs_next (const char *busname,const struct rte_devargs *start); +rte_devargs_next (const char *busname,const struct rte_devargs *start);/** * Iterate over all rte_devargs for a specific bus. */ #define RTE_EAL_DEVARGS_FOREACH (busname, da) \ - for (da = rte_eal_devargs_next(busname, NULL); \ + for (da = rte_devargs_next(busname, NULL); \ da != NULL; \ - da = rte_eal_devargs_next(busname, da)) \ + da = rte_devargs_next(busname, da)) \ #ifdef __cplusplus }
lib/librte_eal/rte_eal_version.map +4 -4
@@ -216,11 +216,11 @@ EXPERIMENTAL { rte_dev_event_callback_unregister; rte_dev_event_monitor_start; rte_dev_event_monitor_stop; + rte_devargs_insert; + rte_devargs_next; + rte_devargs_parse; + rte_devargs_remove; rte_eal_cleanup; - rte_eal_devargs_insert; - rte_eal_devargs_next; - rte_eal_devargs_parse; - rte_eal_devargs_remove; rte_eal_hotplug_add; rte_eal_hotplug_remove; rte_eal_mbuf_user_pool_ops;
Signature · 1 line
2.11.0
PATCH v5 10/10 devargs: rename legacy API
In reply to Gaetan RivetThe previous symbols were deprecated for two releases. They are now marked as such and cannot be used anymore.
They are replaced by ones respecting the new namespace that are marked experimental.
As a result, eth_dev attach and detach are slightly reworked to follow the changes.
--- app/test-pmd/cmdline.c | 2 +- doc/guides/rel_notes/deprecation.rst | 4 --- drivers/net/vdev_netvsc/vdev_netvsc.c | 2 +- examples/bond/Makefile | 1 + examples/bond/main.c | 2 +- lib/librte_eal/common/eal_common_devargs.c | 9 ++++-- lib/librte_eal/common/eal_common_options.c | 2 +- lib/librte_eal/common/include/rte_devargs.h | 46 +++++++++++++++++++++++++++++ lib/librte_eal/rte_eal_version.map | 3 ++ lib/librte_ether/rte_ethdev.c | 44 +++++++++++++-------------- test/test/commands.c | 2 +- 11 files changed, 81 insertions(+), 36 deletions(-)
app/test-pmd/cmdline.c +1 -1
@@ -8754,7 +8754,7 @@ static void cmd_dump_parsed(void *parsed_result,else if (!strcmp (res->dump,"dump_mempool" ))rte_mempool_list_dump (stdout);else if (!strcmp (res->dump,"dump_devargs" )) -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);else if (!strcmp (res->dump,"dump_log_types" ))rte_log_dump (stdout); }
doc/guides/rel_notes/deprecation.rst +0 -4
@@ -18,12 +18,8 @@ Deprecation Notices - The enum ``rte_devtype`` was used to identify a bus and will disappear. - Functions previously deprecated will change or disappear: - + ``rte_eal_devargs_add`` + ``rte_eal_devargs_type_count`` - + ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse`` + ``rte_eal_devargs_parse`` will change its format and use. - + all ``rte_devargs`` related functions will be renamed, changing the - ``rte_eal_devargs_`` prefix to ``rte_devargs_``. * pci: Several exposed functions are misnamed. The following functions are deprecated starting from v17.11 and are replaced:
drivers/net/vdev_netvsc/vdev_netvsc.c +1 -1
@@ -803,7 +803,7 @@ vdev_netvsc_scan_callback(__rte_unused void *arg) vdev_netvsc_cmp_rte_device, VDEV_NETVSC_DRIVER_NAME);if (dev)return ; -if (rte_eal_devargs_add (RTE_DEVTYPE_VIRTUAL ,VDEV_NETVSC_DRIVER_NAME )) +if (rte_devargs_add (RTE_DEVTYPE_VIRTUAL ,VDEV_NETVSC_DRIVER_NAME ))DRV_LOG (ERR ,"unable to add netvsc devargs." ); }
examples/bond/Makefile +1 -0
@@ -62,6 +62,7 @@ endif CFLAGS += -DALLOW_EXPERIMENTAL_API CFLAGS += -O3 +CFLAGS += -DALLOW_EXPERIMENTAL_API ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y) LDLIBS += -lrte_pmd_bond
examples/bond/main.c +1 -1
@@ -742,7 +742,7 @@ main(int argc, char *argv[])/* init EAL */ ret =rte_eal_init (argc, argv); -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);if (ret <0 )rte_exit (EXIT_FAILURE ,"Error with EAL initialization\n" ); argc -= ret;
lib/librte_eal/common/eal_common_devargs.c +6 -3
@@ -137,8 +137,9 @@ rte_devargs_insert(struct rte_devargs *da) }/* store a whitelist parameter for later parsing */ +__rte_experimental int -rte_eal_devargs_add (enum rte_devtype devtype,const char *devargs_str) +rte_devargs_add (enum rte_devtype devtype,const char *devargs_str) {struct rte_devargs *devargs =NULL ;struct rte_bus *bus =NULL ;
@@ -192,8 +193,9 @@ rte_devargs_remove(const char *busname, const char *devname) }/* count the number of devices of a specified type */ +__rte_experimental unsigned int -rte_eal_devargs_type_count (enum rte_devtype devtype) +rte_devargs_type_count (enum rte_devtype devtype) {struct rte_devargs *devargs;unsigned int count =0 ;
@@ -207,8 +209,9 @@ rte_eal_devargs_type_count(enum rte_devtype devtype) }/* dump the user devices on the console */ +__rte_experimental void -rte_eal_devargs_dump (FILE *f) +rte_devargs_dump (FILE *f) {struct rte_devargs *devargs;
lib/librte_eal/common/eal_common_options.c +1 -1
@@ -153,7 +153,7 @@ eal_option_device_parse(void) TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {if (ret ==0 ) { - ret =rte_eal_devargs_add (devopt->type, devopt->arg); + ret =rte_devargs_add (devopt->type, devopt->arg);if (ret)RTE_LOG (ERR ,EAL ,"Unable to parse device '%s'\n" , devopt->arg);
lib/librte_eal/common/include/rte_devargs.h +46 -0
76 lines not on this page · raw message
lib/librte_eal/rte_eal_version.map +3 -0
@@ -216,10 +216,13 @@ EXPERIMENTAL { rte_dev_event_callback_unregister; rte_dev_event_monitor_start; rte_dev_event_monitor_stop; + rte_devargs_add; + rte_devargs_dump; rte_devargs_insert; rte_devargs_next; rte_devargs_parse; rte_devargs_remove; + rte_devargs_type_count; rte_eal_cleanup; rte_eal_hotplug_add; rte_eal_hotplug_remove;
lib/librte_ether/rte_ethdev.c +20 -24
92 lines not on this page · raw message
test/test/commands.c +1 -1
@@ -132,7 +132,7 @@ static void cmd_dump_parsed(void *parsed_result,else if (!strcmp (res->dump,"dump_mempool" ))rte_mempool_list_dump (stdout);else if (!strcmp (res->dump,"dump_devargs" )) -rte_eal_devargs_dump (stdout); +rte_devargs_dump (stdout);else if (!strcmp (res->dump,"dump_log_types" ))rte_log_dump (stdout);else if (!strcmp (res->dump,"dump_malloc_stats" ))
Signature · 1 line
2.11.0
Re: [PATCH v4 02/10] devargs: introduce foreach macro
In reply to Gaetan RivetQuoted 5 lines · On Tue, 24 Apr 2018 00:41:02 +0200 Gaetan Rivet <[email protected]> wrote:
Introduce new
rte_devargsaccessor allowing to iterate over allrte_devargspertaining to a bus.Signed-off-by Gaetan Rivet <[email protected]>---
Aren't devargs intended to be internal and not part of ABI. If so maybe the experimental tag is not necessary, and you want to say that in the comments.
Re: [PATCH v4 01/10] devargs: introduce iterator
In reply to Stephen HemmingerQuoted 40 lines · Stephen Hemminger
Quoted 37 lines · On Tue, 24 Apr 2018 00:41:01 +0200 Gaetan Rivet <[email protected]> wrote:
In preparation to making
devargs_listprivate.Bus drivers generally need to access
rte_devargspertaining to their operations. This match is a common operation for bus drivers.Add a new accessor for the
rte_devargslist.Signed-off-by Gaetan Rivet <[email protected]>--- lib/librte_eal/common/eal_common_devargs.c | 20 ++++++++++++++++++++ lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++ lib/librte_eal/rte_eal_version.map | 1 + 3 files changed, 41 insertions(+)lib/librte_eal/common/eal_common_devargs.c +15
-0@@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f) devargs->name, devargs->args); } } + +/* bus-aware rte_devargs iterator. */ +__rte_experimental +struct rte_devargs * +rte_eal_devargs_next (const char *busname,const struct rte_devargs *start) +{ +struct rte_devargs *da; + +if (start !=NULL ) + da =TAILQ_NEXT (start, next); +else + da =TAILQ_FIRST (&devargs_list); +while (da !=NULL ) { +if (busname ==NULL || + (strcmp (busname, da->bus->name) ==0 ))
Quoted 9 lines
lib/librte_eal/common/eal_common_devargs.c rte_eal_devargs_dump(FILE *f) · line 229
Quoted 5 lines
lib/librte_eal/common/eal_common_devargs.c +5
-0+return da; + da =TAILQ_NEXT (da, next); + } +return NULL ; +}Can this be made to return a const pointer? It seems unsymmetrical to get the next value as a non-const pointer when passed a const based on last value.
I would like to, but this iterator is used in buses to match devargs and assign it to an rte_devive. The devargs field there is not const.
We don't have a deprecation notice running on this. This might be worth it, though, but I think this could come after integrating this series.
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v4 02/10] devargs: introduce foreach macro
In reply to Stephen HemmingerQuoted 6 lines · Stephen Hemminger
Quoted 3 lines · On Tue, 24 Apr 2018 00:41:02 +0200 Gaetan Rivet <[email protected]> wrote:
Introduce new
rte_devargsaccessor allowing to iterate over allrte_devargspertaining to a bus.
Quoted 6 lines
Quoted 2 lines
Signed-off-by Gaetan Rivet <[email protected]>---
Aren't devargs intended to be internal and not part of ABI. If so maybe the experimental tag is not necessary, and you want to say that in the comments.
The ABI is exposed, so any third-party declaring rte_devargs would be impacted by changes to the structure. I don't think it's internal.
In any case, the API itself is not internal, and the rte_experimental tag is meant for API I think? This function is new and prone to changes as well as part of the public EAL API, I think it is correct to tag it experimental.
Signature · 2 lines
Gaëtan Rivet 6WIND
Re: [PATCH v5 05/10] test: remove devargs unit tests
In reply to Gaetan Rivet24/04/2018 01:54, Gaetan Rivet:
Quoted 6 lines
--- MAINTAINERS | 1 - test/test/Makefile | 1 - test/test/test_devargs.c | 103 ----------------------------------------------- 3 files changed, 105 deletions(-) delete mode 100644 test/test/test_devargs.c
The file must be removed from meson.build too.
Re: [PATCH v5 10/10] devargs: rename legacy API
In reply to Gaetan Rivet24/04/2018 01:54, Gaetan Rivet:
Quoted 1 line
+ struct
rte_devargsda = {0};
It fails with clang.
I suggest replacing with memset.
Quoted 11 lines
Gaetan Rivet (10):
devargs: introduce iterator
bus/vdev: do not reference devargs list
bus/pci: do not reference devargs list
net/vdev_netvsc: do not reference devargs list
test: remove devargs unit tests
devargs: make devargs list private
devargs: make parsing variadic
devargs: update devargs add documentation
devargs: use proper namespace prefix
devargs: rename legacy API
Applied with small fixes and rebase, thanks