devargs cleanup

PATCH Superseded Gaetan Rivet 7 participants 91 messages Aug 25, 2017 – Apr 25, 2018 Network drivers

Share & tools

Gaetan Rivet

author

2017-08-25 16:08 UTC #0 lore raw

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

Gaetan Rivet

author

2017-08-25 16:08 UTC #1 lore raw

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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index aac6fd7..610db67 100644
@@ -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

index 6ac88d6..e0e47e8 100644
@@ -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

index 58d585d..226a082 100644
@@ -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

index 5cfd934..c1bc704 100644
@@ -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

Gaetan Rivet

author

2017-08-25 16:08 UTC #2 lore raw

PATCH 2/6 devargs: introduce foreach macro

In reply to Gaetan Rivet

Introduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining to a bus.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_eal/common/include/rte_devargs.h | 9 +++++++++
 1 file changed, 9 insertions(+)

lib/librte_eal/common/include/rte_devargs.h +9 -0

index 226a082..5ca5a32 100644
@@ -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

Gaetan Rivet

author

2017-08-25 16:08 UTC #3 lore raw

PATCH 3/6 vdev: do not reference devargs_list

In reply to 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]>

---
 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

index f7e547a..a7410a6 100644
@@ -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

Gaetan Rivet

author

2017-08-25 16:08 UTC #4 lore raw

PATCH 4/6 bus/pci: do not reference devargs_list

In reply to Gaetan Rivet

This list should not be used by drivers.
Use the public API instead.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/pci/rte_pci_common.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

drivers/bus/pci/rte_pci_common.c +1 -5

index 459ae42..c4a2131 100644
@@ -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

Gaetan Rivet

author

2017-08-25 16:08 UTC #5 lore raw

PATCH 5/6 test: remove rte_devargs unit tests

In reply to Gaetan Rivet

The 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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index a0cd75e..f6a096a 100644
@@ -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

index 42d9a49..42c9cea 100644
@@ -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

Gaetan Rivet

author

2017-08-25 16:08 UTC #6 lore raw

PATCH 6/6 devargs: make devargs_list private

In reply to Gaetan Rivet

Initially, 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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 610db67..91621b1 100644
@@ -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

index e0e47e8..2b20ce0 100644
@@ -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

index 5ca5a32..d07810f 100644
@@ -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

index c1bc704..6595b64 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #7 lore raw

v2 cover devargs cleanup

In reply to Gaetan Rivet

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.

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

Gaetan Rivet

author

2017-10-12 08:21 UTC #8 lore raw

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.

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 +12 -5

index 630c9d2..d57cb5d 100644
@@ -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

Shreyansh Jain

2017-12-11 13:43 UTC #9 lore raw

Re: [PATCH v2 01/18] eal: prepend busname on legacy device declaration

One 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
index 630c9d2..d57cb5d 100644
@@ -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 -0

  	devopt = calloc(1, sizeof(*devopt) + optlen);
  	if (devopt == NULL) {
  		RTE_LOG(ERR, EAL, "Unable to allocate device option\n");

[...]

Gaetan Rivet

author

2017-10-12 08:21 UTC #10 lore raw

PATCH v2 2/18 eal: remove generic devtype

In reply to 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_devtype enum can disappear, along with crutches added during this transition.

rte_eal_devargs_type_count becomes 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 +2 ~~-14~~

index bbe862b..5fbcf11 100644
@@ -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

index 573869a..47416a5 100644
@@ -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~~

index e371456..2fddbfa 100644
@@ -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~~

index d57cb5d..603df27 100644
@@ -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

index 4c4ac7e..5f090ed 100644
@@ -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~~

index 58d585d..e50c166 100644
@@ -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

index a2709e3..e1e2a50 100644
@@ -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

Aaron Conole

2017-10-17 18:17 UTC #11 lore raw

Re: [PATCH v2 02/18] eal: remove generic devtype

Quoted 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_devtype enum can disappear, along with crutches added during this transition.

rte_eal_devargs_type_count becomes 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

index bbe862b..5fbcf11 100644
@@ -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 -0

 			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

index 573869a..47416a5 100644
@@ -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~~

index e371456..2fddbfa 100644
@@ -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~~

index d57cb5d..603df27 100644
@@ -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

index 4c4ac7e..5f090ed 100644
@@ -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~~

index 58d585d..e50c166 100644
@@ -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

index a2709e3..e1e2a50 100644
@@ -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;

Gaëtan Rivet

author

2017-10-18 08:20 UTC #12 lore raw

Re: [PATCH v2 02/18] eal: remove generic devtype

Hello 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_devtype enum can disappear, along with crutches added during this transition.
rte_eal_devargs_type_count becomes 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_all is 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

drivers/bus/pci/pci_common.c +0 -0

 			ret = pci_probe_all_drivers(dev);
 		if (ret < 0) {
 			RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMT

< ... >

Signature · 2 lines

Gaëtan Rivet
6WIND

Gaetan Rivet

author

2017-10-12 08:21 UTC #13 lore raw

PATCH v2 3/18 devargs: introduce iterator

In reply to Gaetan Rivet

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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 47416a5..01ae0c7 100644
@@ -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

index 2fddbfa..614f1c5 100644
@@ -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

index e50c166..0eec406 100644
@@ -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

index e1e2a50..576de56 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #14 lore raw

PATCH v2 4/18 devargs: introduce foreach macro

In reply to Gaetan Rivet

Introduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining to a bus.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_eal/common/include/rte_devargs.h | 8 ++++++++
 1 file changed, 8 insertions(+)

lib/librte_eal/common/include/rte_devargs.h +8 -0

index 0eec406..6222677 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #15 lore raw

PATCH v2 5/18 vdev: do not reference devargs list

In reply to 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]>

---
 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

index f7e547a..a7410a6 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #16 lore raw

PATCH v2 6/18 bus/pci: do not reference devargs list

In reply to Gaetan Rivet

This list should not be used by drivers.
Use the public API instead.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/bus/pci/pci_common.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

drivers/bus/pci/pci_common.c +1 -5

index 5fbcf11..0b64d20 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #17 lore raw

PATCH v2 7/18 test: remove devargs unit tests

In reply to Gaetan Rivet

The 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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index b8b5441..6c174ef 100644
@@ -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

index 61e4699..3d76e5e 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #18 lore raw

PATCH v2 8/18 devargs: make devargs list private

In reply to Gaetan Rivet

Initially, 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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 01ae0c7..0d693c8 100644
@@ -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

index 614f1c5..0f81f22 100644
@@ -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

index 6222677..5f4ad33 100644
@@ -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

index 576de56..9c0251e 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #19 lore raw

PATCH v2 9/18 devargs: make parsing variadic

In reply to Gaetan Rivet

rte_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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index cfc83e3..08ce4ad 100644
@@ -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~~

index e251275..b965e56 100644
@@ -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

index 0f81f22..a21cc1a 100644
@@ -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~~

index 5f4ad33..1fe03d6 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #20 lore raw

PATCH v2 10/18 devargs: require bus name prefix

In reply to Gaetan Rivet

The EAL now requires the bus to be prepended to the device declaration string.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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~~

index a21cc1a..49cc3b8 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #21 lore raw

PATCH v2 11/18 devargs: simplify implementation

In reply to 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 +3 -5

index 49cc3b8..1d87cd9 100644
@@ -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

Shreyansh Jain

2017-10-16 11:26 UTC #22 lore raw

Re: [PATCH v2 11/18] devargs: simplify implementation

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 -0
index 49cc3b8..1d87cd9 100644

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 -0
  	return 0;
 
  fail:

Maybe I am doing something wrong here - any ideas? Can you send an updated/rebased version on current master HEAD?

-
Shreyansh

Shreyansh Jain

2017-10-16 11:30 UTC #23 lore raw

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 -0
index 49cc3b8..1d87cd9 100644
lib/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)
{
      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)
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

Gaëtan Rivet

author

2017-10-16 13:42 UTC #24 lore raw

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

Shreyansh Jain

2017-10-17 05:46 UTC #25 lore raw

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

Gaetan Rivet

author

2017-10-12 08:21 UTC #26 lore raw

PATCH v2 12/18 eal: add generic device declaration parameter

In reply to Gaetan Rivet

Add 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

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 603df27..b7591fd 100644
@@ -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

index 30e6bb4..d50eff7 100644
@@ -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

Shreyansh Jain

2017-12-13 14:13 UTC #27 lore raw

Re: [PATCH v2 12/18] eal: add generic device declaration parameter

Quoted 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

index 603df27..b7591fd 100644
@@ -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;

[...]

Gaëtan Rivet

author

2017-12-13 14:47 UTC #28 lore raw

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
index 603df27..b7591fd 100644
@@ -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_list here (eal_parse_args -> eal_parse_common_option -> OPT_DEV ->) rather than wait for eal to call eal_option_device_parse again?

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

Shreyansh Jain

2017-12-13 15:10 UTC #29 lore raw

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
index 603df27..b7591fd 100644
@@ -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_list here (eal_parse_args -> eal_parse_common_option -> OPT_DEV ->) rather than wait for eal to call eal_option_device_parse again?
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_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.

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;
[...]

Gaetan Rivet

author

2017-10-12 08:21 UTC #30 lore raw

PATCH v2 13/18 bus: make device recognition function public

In reply to Gaetan Rivet

As 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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 0d693c8..13c8450 100644
@@ -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

index 1cae96e..6d3fb70 100644
@@ -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

index 9c0251e..a6fe25c 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #31 lore raw

PATCH v2 14/18 net/failsafe: keep legacy sub-device declaration

In reply to Gaetan Rivet

Allow 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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/net/failsafe/failsafe_args.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

drivers/net/failsafe/failsafe_args.c +10 -1

index 08ce4ad..0c98264 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:21 UTC #32 lore raw

PATCH v2 15/18 ether: use new devargs parsing function

In reply to Gaetan Rivet

The previous one is being deprecated.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_ether/rte_ethdev.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

lib/librte_ether/rte_ethdev.c +7 -4

index bb08204..618f576 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:22 UTC #33 lore raw

PATCH v2 16/18 devargs: remove old devargs parsing function

In reply to Gaetan Rivet

This functionality is already covered by rte_eal_devargs_parse.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 13c8450..aae8d32 100644
@@ -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~~

index 1d87cd9..82c8573 100644
@@ -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~~

index 1fe03d6..499f7e3 100644
@@ -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

index a6fe25c..323f799 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:22 UTC #34 lore raw

PATCH v2 17/18 devargs: use proper prefix

In reply to Gaetan Rivet

rte_eal_devargs is redundant.
Make it concise.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 1204604..3b338e5 100644
@@ -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

index 0c98264..79c90cf 100644
@@ -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

index cb55552..420ba1f 100644
@@ -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

index aae8d32..0913a52 100644
@@ -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

index b965e56..5608690 100644
@@ -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

index 82c8573..d5e36d2 100644
@@ -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

index b7591fd..a3238fb 100644
@@ -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

index a7410a6..583648d 100644
@@ -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

index 499f7e3..e46686d 100644
@@ -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

index 323f799..87b9a3b 100644
@@ -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

index 618f576..2f3f721 100644
@@ -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

index 4097a33..1670f75 100644
@@ -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

Gaetan Rivet

author

2017-10-12 08:22 UTC #35 lore raw

PATCH v2 18/18 doc: remove devargs deprecation notices

In reply to 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 ~~-13~~

index ef2264f..23faa19 100644
@@ -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

Mcnamara, John

Acked-by on v2 18/18

2017-10-12 11:28 UTC #36 lore raw

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]>

Acked-by John McNamara <[email protected]>

Shreyansh Jain

2017-12-13 10:03 UTC #37 lore raw

Re: [PATCH v2 18/18] doc: remove devargs deprecation notices

In reply to Gaetan Rivet

Hello 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

index ef2264f..23faa19 100644
@@ -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).

Gaëtan Rivet

author

2017-12-13 10:26 UTC #38 lore raw

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

index ef2264f..23faa19 100644
@@ -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

doc/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 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:

Quoted 7 lines

"...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.

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

Shreyansh Jain

2017-12-13 10:55 UTC #39 lore raw

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 notices
Hello 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.rst
Quoted 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`` in
v17.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 will
need
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 will
either 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 IOVA
address
Quoted 2 lines
doc/guides/rel_notes/deprecation.rst Deprecation Notices · line 20
Quoted 2 lines
doc/guides/rel_notes/deprecation.rst +0 -0
    translation scheme.
    Reference to phys address in EAL data-structure or functions may
change to
Quoted 3 lines
Quoted 1 line
Once this patch is formalized, the documentation reference for
rte_devargs.h
Quoted 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_parse too 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_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.

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

Shreyansh Jain

2017-12-22 04:45 UTC #40 lore raw

Re: [PATCH v2 18/18] doc: remove devargs deprecation notices

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 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 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.

Have you pushed the new version of the devargs patches? Just wanted to check in case I have missed it.

-
Shreyansh

Gaëtan Rivet

author

2017-12-22 08:33 UTC #41 lore raw

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 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.
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

Aaron Conole

2017-10-17 18:18 UTC #42 lore raw

In reply to Gaetan Rivet

Quoted 23 lines · Gaetan Rivet

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.
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.

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.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

Gaëtan Rivet

author

2017-10-18 08:36 UTC #43 lore raw

Quoted 25 lines · Aaron Conole

Quoted 23 lines · Gaetan Rivet
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.
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.

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.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 · 2 lines

Gaëtan Rivet
6WIND

Gaetan Rivet

author

2018-03-20 23:20 UTC #44 lore raw

v3 cover devargs cleanup

In reply to Gaetan Rivet

Cleanup 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

Gaetan Rivet

author

2018-03-20 23:20 UTC #45 lore raw

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.

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

index 810b3e18f..c6c5eabcf 100644
@@ -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

index 84e5e23c4..969a10449 100644
@@ -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

index d12360235..02a040a8b 100644
@@ -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

Tan, Jianfeng

2018-03-21 05:43 UTC #46 lore raw

Re: [PATCH v3 01/10] devargs: introduce iterator

Hi Gaetan,

Quoted 78 lines · Gaetan Rivet

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.

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

index 810b3e18f..c6c5eabcf 100644
@@ -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

index 84e5e23c4..969a10449 100644
@@ -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

index d12360235..02a040a8b 100644

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

Gaëtan Rivet

author

2018-03-21 08:50 UTC #47 lore raw

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_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.

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

index 810b3e18f..c6c5eabcf 100644
@@ -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

index 84e5e23c4..969a10449 100644
@@ -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

index d12360235..02a040a8b 100644
@@ -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 -0
  	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?

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

Gaetan Rivet

author

2018-03-20 23:20 UTC #48 lore raw

PATCH v3 2/10 devargs: introduce foreach macro

In reply to Gaetan Rivet

Introduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining to a bus.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_eal/common/include/rte_devargs.h | 8 ++++++++
 1 file changed, 8 insertions(+)

lib/librte_eal/common/include/rte_devargs.h +8 -0

index 969a10449..aba31481b 100644
@@ -209,6 +209,14 @@ __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

Signature · 1 line

2.11.0

Thomas Monjalon

2018-04-22 21:42 UTC #49 lore raw

Re: [PATCH v3 02/10] devargs: introduce foreach macro

21/03/2018 00:20, Gaetan Rivet:

Quoted 2 lines

Introduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining to a bus.

This patch can be merged with previous one.

Gaetan Rivet

author

2018-03-20 23:20 UTC #50 lore raw

PATCH v3 3/10 bus/vdev: do not reference devargs list

In reply to 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]>

---
 drivers/bus/vdev/Makefile |  1 +
 drivers/bus/vdev/vdev.c   | 11 +++--------
 2 files changed, 4 insertions(+), 8 deletions(-)

drivers/bus/vdev/Makefile +1 -0

index 24d424a38..bd0bb8955 100644
@@ -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

index e4bc72463..8ee264baf 100644
@@ -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

Tan, Jianfeng

Acked-by on v3 3/10

2018-03-21 05:41 UTC #51 lore raw

Re: [PATCH v3 03/10] bus/vdev: do not reference devargs list

Quoted 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]>

Acked-by Jianfeng Tan <[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

index 24d424a38..bd0bb8955 100644
@@ -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

index e4bc72463..8ee264baf 100644
@@ -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)

Gaetan Rivet

author

2018-03-20 23:20 UTC #52 lore raw

PATCH v3 4/10 bus/pci: do not reference devargs list

In reply to Gaetan Rivet

This list should not be used by drivers.
Use the public API instead.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index f3df1c4ce..73796ec0e 100644
@@ -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

index 2a00f365a..6bed0bc9d 100644
@@ -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

Gaetan Rivet

author

2018-03-20 23:20 UTC #53 lore raw

PATCH v3 5/10 net/vdev_netvsc: do not reference devargs list

In reply to Gaetan Rivet

This list should not be used by drivers.
Use the public API instead.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/net/vdev_netvsc/vdev_netvsc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/vdev_netvsc/vdev_netvsc.c +1 -1

index cbf4d590a..b0bbce750 100644
@@ -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

Gaetan Rivet

author

2018-03-20 23:20 UTC #54 lore raw

PATCH v3 6/10 test: remove devargs unit tests

In reply to Gaetan Rivet

The 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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index a646ca3e1..c07931d13 100644
@@ -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

index a88cc38bf..035b073b7 100644
@@ -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

Gaetan Rivet

author

2018-03-20 23:20 UTC #55 lore raw

PATCH v3 7/10 devargs: make devargs list private

In reply to Gaetan Rivet

Initially, 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.

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(-)

lib/librte_eal/common/eal_common_devargs.c +3 -0

index c6c5eabcf..a7f374cfa 100644
@@ -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

index aba31481b..e48cc3cf2 100644
@@ -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

index 02a040a8b..8dab46ba6 100644
@@ -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

Thomas Monjalon

2018-04-22 21:46 UTC #56 lore raw

Re: [PATCH v3 07/10] devargs: make devargs list private

21/03/2018 00:20, Gaetan Rivet:

Quoted 11 lines

Initially, 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.

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.

Gaetan Rivet

author

2018-03-20 23:20 UTC #57 lore raw

PATCH v3 8/10 devargs: make parsing variadic

In reply to Gaetan Rivet

rte_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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 366dbea16..f303668bb 100644
@@ -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

index c3d673125..5adaabfeb 100644
@@ -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~~

index cd071442f..43e1fb767 100644
@@ -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 *devn
 		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;

@@ -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 *devn
 		free(da->args);
 		free(da);
 	}
-err_name:
-	free(name);
 	return ret;
 }

lib/librte_eal/common/eal_common_devargs.c +12 -3

index a7f374cfa..b251bb0a6 100644
@@ -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~~

index e48cc3cf2..e261f7c69 100644
@@ -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

Thomas Monjalon

2018-04-22 21:52 UTC #58 lore raw

Re: [PATCH v3 08/10] devargs: make parsing variadic

21/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?

Gaetan Rivet

author

2018-03-20 23:20 UTC #59 lore raw

PATCH v3 9/10 devargs: use proper namespace prefix

In reply to Gaetan Rivet

rte_eal_devargs is useless, rte_devargs is sufficient.

Only experimental functions are changed for now.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 8ee264baf..4fcbd9c79 100644
@@ -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

index f303668bb..05773556c 100644
@@ -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

index 5adaabfeb..f382f5709 100644
@@ -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

index 43e1fb767..07e3e1647 100644
@@ -113,12 +113,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	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;

@@ -143,7 +143,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return 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

index b251bb0a6..24d0f6f44 100644
@@ -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~~

index e261f7c69..07a4701b2 100644
@@ -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

index 8dab46ba6..1e614f088 100644
@@ -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

Gaetan Rivet

author

2018-03-20 23:20 UTC #60 lore raw

PATCH v3 10/10 devargs: rename legacy API

In reply to Gaetan Rivet

This 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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 40b31ad7e..6e6b4e2e2 100644
@@ -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

index b0bbce750..f4e88689f 100644
@@ -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

index 44d10d4f5..ce3f907de 100644
@@ -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

index 455f108ee..ede1c591e 100644
@@ -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

index 24d0f6f44..b0434158b 100644
@@ -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

index 9f2f8d25a..63df21224 100644
@@ -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

index 1e614f088..bc24237e1 100644
@@ -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

index cf0b726bb..2ae78a3d0 100644
@@ -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

Thomas Monjalon

Acked-by on the v3 series

2018-04-22 22:00 UTC #61 lore raw

In reply to Gaetan Rivet

21/03/2018 00:20, Gaetan Rivet:

Quoted 13 lines

Cleanup 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.

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

Acked-by Thomas Monjalon <[email protected]>

Thanks

Gaetan Rivet

author

2018-04-23 22:41 UTC #62 lore raw

v4 cover devargs cleanup

In reply to Gaetan Rivet

Cleanup the rte_devargs API.

v4:

* Rebased on master.

 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

Gaetan Rivet

author

2018-04-23 22:41 UTC #63 lore raw

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.

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

index 810b3e18f..c6c5eabcf 100644
@@ -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

index 84e5e23c4..969a10449 100644
@@ -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

index d02d80b8a..74c779068 100644
@@ -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

Stephen Hemminger

2018-04-23 23:54 UTC #64 lore raw

Re: [PATCH v4 01/10] devargs: introduce iterator

Quoted 32 lines · On Tue, 24 Apr 2018 00:41:01 +0200 Gaetan Rivet <[email protected]> wrote:

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.
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
index 810b3e18f..c6c5eabcf 100644
@@ -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.

Gaëtan Rivet

author

2018-04-24 10:22 UTC #65 lore raw

Re: [PATCH v4 01/10] devargs: introduce iterator

Quoted 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_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.
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
index 810b3e18f..c6c5eabcf 100644
@@ -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

Gaetan Rivet

author

2018-04-23 22:41 UTC #66 lore raw

PATCH v4 2/10 devargs: introduce foreach macro

In reply to Gaetan Rivet

Introduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining to a bus.

Signed-off-by Gaetan Rivet <[email protected]>

---
 lib/librte_eal/common/include/rte_devargs.h | 8 ++++++++
 1 file changed, 8 insertions(+)

lib/librte_eal/common/include/rte_devargs.h +8 -0

index 969a10449..aba31481b 100644
@@ -209,6 +209,14 @@ __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

Signature · 1 line

2.11.0

Stephen Hemminger

2018-04-23 23:56 UTC #67 lore raw

Re: [PATCH v4 02/10] devargs: introduce foreach macro

Quoted 5 lines · On Tue, 24 Apr 2018 00:41:02 +0200 Gaetan Rivet <[email protected]> wrote:

Introduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining 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.

Gaëtan Rivet

author

2018-04-24 10:26 UTC #68 lore raw

Re: [PATCH v4 02/10] devargs: introduce foreach macro

Quoted 6 lines · Stephen Hemminger

Quoted 3 lines · On Tue, 24 Apr 2018 00:41:02 +0200 Gaetan Rivet <[email protected]> wrote:
Introduce new rte_devargs accessor allowing to iterate over all rte_devargs pertaining 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

Gaetan Rivet

author

2018-04-23 22:41 UTC #69 lore raw

PATCH v4 3/10 bus/vdev: do not reference devargs list

In reply to 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]>

---
 drivers/bus/vdev/Makefile |  1 +
 drivers/bus/vdev/vdev.c   | 11 +++--------
 2 files changed, 4 insertions(+), 8 deletions(-)

drivers/bus/vdev/Makefile +1 -0

index 24d424a38..bd0bb8955 100644
@@ -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

index f8dd1f5e6..d100f3232 100644
@@ -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

Gaetan Rivet

author

2018-04-23 22:41 UTC #70 lore raw

PATCH v4 4/10 bus/pci: do not reference devargs list

In reply to Gaetan Rivet

This list should not be used by drivers.
Use the public API instead.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 804a198d1..c03722c13 100644
@@ -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

index 2a00f365a..6bed0bc9d 100644
@@ -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

Gaetan Rivet

author

2018-04-23 22:41 UTC #71 lore raw

PATCH v4 5/10 net/vdev_netvsc: do not reference devargs list

In reply to Gaetan Rivet

This list should not be used by drivers.
Use the public API instead.

Signed-off-by Gaetan Rivet <[email protected]>

---
 drivers/net/vdev_netvsc/vdev_netvsc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/vdev_netvsc/vdev_netvsc.c +1 -1

index f7102d768..8b85711d4 100644
@@ -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

Gaetan Rivet

author

2018-04-23 22:41 UTC #72 lore raw

PATCH v4 6/10 test: remove devargs unit tests

In reply to Gaetan Rivet

The 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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 645bd5263..1857bfd64 100644
@@ -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

index c9c007c9b..2630ab484 100644
@@ -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

Gaetan Rivet

author

2018-04-23 22:41 UTC #73 lore raw

PATCH v4 7/10 devargs: make devargs list private

In reply to Gaetan Rivet

Initially, 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.

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(-)

lib/librte_eal/common/eal_common_devargs.c +3 -0

index c6c5eabcf..a7f374cfa 100644
@@ -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

index aba31481b..e48cc3cf2 100644
@@ -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

index 74c779068..4054fc111 100644
@@ -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

Gaetan Rivet

author

2018-04-23 22:41 UTC #74 lore raw

PATCH v4 8/10 devargs: make parsing variadic

In reply to Gaetan Rivet

rte_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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 20d2f520a..5edc061e8 100644
@@ -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

index 4078fdbb0..c11621e93 100644
@@ -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~~

index 149e9ad72..809d6649d 100644
@@ -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 *devn
 		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;

@@ -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 *devn
 		free(da->args);
 		free(da);
 	}
-err_name:
-	free(name);
 	return ret;
 }

lib/librte_eal/common/eal_common_devargs.c +12 -3

index a7f374cfa..b251bb0a6 100644
@@ -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~~

index e48cc3cf2..e261f7c69 100644
@@ -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

Gaetan Rivet

author

2018-04-23 22:41 UTC #75 lore raw

PATCH v4 9/10 devargs: use proper namespace prefix

In reply to Gaetan Rivet

rte_eal_devargs is useless, rte_devargs is sufficient.

Only experimental functions are changed for now.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index d100f3232..f7a2148ef 100644
@@ -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

index 5edc061e8..2c002b164 100644
@@ -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

index c11621e93..ee89236f1 100644
@@ -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

index 809d6649d..61cb3b162 100644
@@ -138,12 +138,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	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;

@@ -168,7 +168,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return 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

index b251bb0a6..24d0f6f44 100644
@@ -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~~

index e261f7c69..07a4701b2 100644
@@ -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

index 4054fc111..526b0d95c 100644
@@ -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

Gaetan Rivet

author

2018-04-23 22:41 UTC #76 lore raw

PATCH v4 10/10 devargs: rename legacy API

In reply to Gaetan Rivet

This 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.

Signed-off-by Gaetan Rivet <[email protected]>

---
 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

index 0b442c3a6..18470c6fc 100644
@@ -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

index 8b85711d4..f902dbe27 100644
@@ -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

index e7afce358..6b0324a42 100644
@@ -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

index d8edc642b..65e0edd25 100644
@@ -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

index 24d0f6f44..b0434158b 100644
@@ -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

index 40c5b26f3..e1a1a3c56 100644
@@ -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

index 526b0d95c..d744312ec 100644
@@ -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

index 6bfdc0272..94fbc310e 100644
@@ -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

Gaetan Rivet

author

2018-04-23 23:55 UTC #77 lore raw

v5 cover devargs cleanup

In reply to Gaetan Rivet

Cleanup the rte_devargs API.

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

Gaetan Rivet

author

2018-04-23 23:55 UTC #78 lore raw

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.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Thomas Monjalon <[email protected]>

---
 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

index 810b3e18f..c6c5eabcf 100644
@@ -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

index 84e5e23c4..aba31481b 100644
@@ -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

index d02d80b8a..74c779068 100644
@@ -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

Gaetan Rivet

author

2018-04-23 23:55 UTC #79 lore raw

PATCH v5 2/10 bus/vdev: do not reference devargs list

In reply to 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]>

Acked-by Jianfeng Tan <[email protected]>

Acked-by Thomas Monjalon <[email protected]>

---
 drivers/bus/vdev/Makefile |  1 +
 drivers/bus/vdev/vdev.c   | 11 +++--------
 2 files changed, 4 insertions(+), 8 deletions(-)

drivers/bus/vdev/Makefile +1 -0

index 24d424a38..bd0bb8955 100644
@@ -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

index f8dd1f5e6..d100f3232 100644
@@ -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

Gaetan Rivet

author

2018-04-23 23:55 UTC #80 lore raw

PATCH v5 3/10 bus/pci: do not reference devargs list

In reply to Gaetan Rivet

This list should not be used by drivers.
Use the public API instead.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Thomas Monjalon <[email protected]>

---
 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

index 804a198d1..c03722c13 100644
@@ -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

index 2a00f365a..6bed0bc9d 100644
@@ -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

Gaetan Rivet

author

2018-04-23 23:55 UTC #81 lore raw

PATCH v5 4/10 net/vdev_netvsc: do not reference devargs list

In reply to Gaetan Rivet

This list should not be used by drivers.
Use the public API instead.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Thomas Monjalon <[email protected]>

---
 drivers/net/vdev_netvsc/vdev_netvsc.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/net/vdev_netvsc/vdev_netvsc.c +1 -1

index f7102d768..8b85711d4 100644
@@ -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

Gaetan Rivet

author

2018-04-23 23:55 UTC #82 lore raw

PATCH v5 5/10 test: remove devargs unit tests

In reply to Gaetan Rivet

The 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.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Thomas Monjalon <[email protected]>

---
 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

index 645bd5263..1857bfd64 100644
@@ -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

index c9c007c9b..2630ab484 100644
@@ -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

Thomas Monjalon

2018-04-25 00:45 UTC #83 lore raw

Re: [PATCH v5 05/10] test: remove devargs unit tests

24/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.

Gaetan Rivet

author

2018-04-23 23:55 UTC #84 lore raw

PATCH v5 6/10 devargs: make devargs list private

In reply to Gaetan Rivet

Initially, 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.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Thomas Monjalon <[email protected]>

---
 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

index 1f814b4b1..2165fe4c6 100644
@@ -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

index c6c5eabcf..a7f374cfa 100644
@@ -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

index aba31481b..e48cc3cf2 100644
@@ -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

index 74c779068..4054fc111 100644
@@ -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

Gaetan Rivet

author

2018-04-23 23:55 UTC #85 lore raw

PATCH v5 7/10 devargs: make parsing variadic

In reply to Gaetan Rivet

rte_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.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Thomas Monjalon <[email protected]>

---
 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

index 20d2f520a..5edc061e8 100644
@@ -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

index 4078fdbb0..c11621e93 100644
@@ -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~~

index 149e9ad72..809d6649d 100644
@@ -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 *devn
 		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;

@@ -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 *devn
 		free(da->args);
 		free(da);
 	}
-err_name:
-	free(name);
 	return ret;
 }

lib/librte_eal/common/eal_common_devargs.c +12 -3

index a7f374cfa..b251bb0a6 100644
@@ -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

index e48cc3cf2..eff7a3f8c 100644
@@ -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

Gaetan Rivet

author

2018-04-23 23:55 UTC #86 lore raw

PATCH v5 8/10 devargs: update devargs add documentation

In reply to Gaetan Rivet

Device syntax documentation is out of date.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Thomas Monjalon <[email protected]>

---
 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~~

index eff7a3f8c..bfe4da4fd 100644
@@ -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

Gaetan Rivet

author

2018-04-23 23:55 UTC #87 lore raw

PATCH v5 9/10 devargs: use proper namespace prefix

In reply to Gaetan Rivet

rte_eal_devargs is useless, rte_devargs is sufficient.

Only experimental functions are changed for now.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Thomas Monjalon <[email protected]>

---
 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

index d100f3232..f7a2148ef 100644
@@ -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

index 5edc061e8..2c002b164 100644
@@ -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

index c11621e93..ee89236f1 100644
@@ -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

index 809d6649d..61cb3b162 100644
@@ -138,12 +138,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	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;

@@ -168,7 +168,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return 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

index b251bb0a6..24d0f6f44 100644
@@ -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~~

index bfe4da4fd..0806f2319 100644
@@ -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

index 4054fc111..67f8dd4fa 100644
@@ -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

Gaetan Rivet

author

2018-04-23 23:55 UTC #88 lore raw

PATCH v5 10/10 devargs: rename legacy API

In reply to Gaetan Rivet

The 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.

Signed-off-by Gaetan Rivet <[email protected]>

Acked-by Thomas Monjalon <[email protected]>

---
 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

index 0b442c3a6..18470c6fc 100644
@@ -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

index 2165fe4c6..50959025e 100644
@@ -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

index 8b85711d4..f902dbe27 100644
@@ -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

index e7afce358..6b0324a42 100644
@@ -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

index d8edc642b..65e0edd25 100644
@@ -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

index 24d0f6f44..b0434158b 100644
@@ -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

index 40c5b26f3..e1a1a3c56 100644
@@ -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

index 67f8dd4fa..21346c703 100644
@@ -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

index 6bfdc0272..94fbc310e 100644
@@ -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

Thomas Monjalon

2018-04-25 01:51 UTC #89 lore raw

Re: [PATCH v5 10/10] devargs: rename legacy API

24/04/2018 01:54, Gaetan Rivet:

Quoted 1 line

+ struct rte_devargs da = {0};

It fails with clang.
I suggest replacing with memset.

Thomas Monjalon

2018-04-25 01:57 UTC #90 lore raw

In reply to Gaetan Rivet

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