DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/6] devargs cleanup
@ 2017-08-25 16:07 Gaetan Rivet
  2017-08-25 16:07 ` [PATCH 1/6] devargs: introduce iterator Gaetan Rivet
                   ` (8 more replies)
  0 siblings, 9 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-08-25 16:07 UTC (permalink / raw)
  To: dev; +Cc: 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. 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

-- 
2.1.4

^ permalink raw reply	[flat|nested] 91+ messages in thread

* [PATCH 1/6] devargs: introduce iterator
  2017-08-25 16:07 [PATCH 0/6] devargs cleanup Gaetan Rivet
@ 2017-08-25 16:07 ` Gaetan Rivet
  2017-08-25 16:07 ` [PATCH 2/6] devargs: introduce foreach macro Gaetan Rivet
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-08-25 16:07 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 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(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index aac6fd7..610db67 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -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;
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 6ac88d6..e0e47e8 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -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;
+}
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 58d585d..226a082 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -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
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 5cfd934..c1bc704 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -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;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH 2/6] devargs: introduce foreach macro
  2017-08-25 16:07 [PATCH 0/6] devargs cleanup Gaetan Rivet
  2017-08-25 16:07 ` [PATCH 1/6] devargs: introduce iterator Gaetan Rivet
@ 2017-08-25 16:07 ` Gaetan Rivet
  2017-08-25 16:07 ` [PATCH 3/6] vdev: do not reference devargs_list Gaetan Rivet
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-08-25 16:07 UTC (permalink / raw)
  To: dev; +Cc: Gaetan Rivet

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

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/include/rte_devargs.h | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 226a082..5ca5a32 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -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
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH 3/6] vdev: do not reference devargs_list
  2017-08-25 16:07 [PATCH 0/6] devargs cleanup Gaetan Rivet
  2017-08-25 16:07 ` [PATCH 1/6] devargs: introduce iterator Gaetan Rivet
  2017-08-25 16:07 ` [PATCH 2/6] devargs: introduce foreach macro Gaetan Rivet
@ 2017-08-25 16:07 ` Gaetan Rivet
  2017-08-25 16:07 ` [PATCH 4/6] bus/pci: " Gaetan Rivet
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-08-25 16:07 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_vdev.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index f7e547a..a7410a6 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -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)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH 4/6] bus/pci: do not reference devargs_list
  2017-08-25 16:07 [PATCH 0/6] devargs cleanup Gaetan Rivet
                   ` (2 preceding siblings ...)
  2017-08-25 16:07 ` [PATCH 3/6] vdev: do not reference devargs_list Gaetan Rivet
@ 2017-08-25 16:07 ` Gaetan Rivet
  2017-08-25 16:07 ` [PATCH 5/6] test: remove rte_devargs unit tests Gaetan Rivet
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-08-25 16:07 UTC (permalink / raw)
  To: dev; +Cc: Gaetan Rivet

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

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 drivers/bus/pci/rte_pci_common.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/bus/pci/rte_pci_common.c b/drivers/bus/pci/rte_pci_common.c
index 459ae42..c4a2131 100644
--- a/drivers/bus/pci/rte_pci_common.c
+++ b/drivers/bus/pci/rte_pci_common.c
@@ -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;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH 5/6] test: remove rte_devargs unit tests
  2017-08-25 16:07 [PATCH 0/6] devargs cleanup Gaetan Rivet
                   ` (3 preceding siblings ...)
  2017-08-25 16:07 ` [PATCH 4/6] bus/pci: " Gaetan Rivet
@ 2017-08-25 16:07 ` Gaetan Rivet
  2017-08-25 16:07 ` [PATCH 6/6] devargs: make devargs_list private Gaetan Rivet
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-08-25 16:07 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 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

diff --git a/MAINTAINERS b/MAINTAINERS
index a0cd75e..f6a096a 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -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
diff --git a/test/test/Makefile b/test/test/Makefile
index 42d9a49..42c9cea 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -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
diff --git a/test/test/test_devargs.c b/test/test/test_devargs.c
deleted file mode 100644
index 18f54ed..0000000
--- a/test/test/test_devargs.c
+++ /dev/null
@@ -1,131 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright 2014 6WIND S.A.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of 6WIND S.A nor the names of its contributors
- *       may be used to endorse or promote products derived from this
- *       software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <sys/queue.h>
-
-#include <rte_debug.h>
-#include <rte_devargs.h>
-
-#include "test.h"
-
-/* clear devargs list that was modified by the test */
-static void free_devargs_list(void)
-{
-	struct rte_devargs *devargs;
-
-	while (!TAILQ_EMPTY(&devargs_list)) {
-		devargs = TAILQ_FIRST(&devargs_list);
-		TAILQ_REMOVE(&devargs_list, devargs, next);
-		free(devargs->args);
-		free(devargs);
-	}
-}
-
-static int
-test_devargs(void)
-{
-	struct rte_devargs_list save_devargs_list;
-	struct rte_devargs *devargs;
-
-	/* save the real devargs_list, it is restored at the end of the test */
-	save_devargs_list = devargs_list;
-	TAILQ_INIT(&devargs_list);
-
-	/* test valid cases */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "08:00.1") < 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "0000:5:00.0") < 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI, "04:00.0,arg=val") < 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI, "0000:01:00.1") < 0)
-		goto fail;
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 2)
-		goto fail;
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 2)
-		goto fail;
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_VIRTUAL) != 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, "net_ring0") < 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, "net_ring1,key=val,k2=val2") < 0)
-		goto fail;
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_VIRTUAL) != 2)
-		goto fail;
-	free_devargs_list();
-
-	/* check virtual device with argument parsing */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, "net_ring1,k1=val,k2=val2") < 0)
-		goto fail;
-	devargs = TAILQ_FIRST(&devargs_list);
-	if (strncmp(devargs->name, "net_ring1",
-			sizeof(devargs->name)) != 0)
-		goto fail;
-	if (!devargs->args || strcmp(devargs->args, "k1=val,k2=val2") != 0)
-		goto fail;
-	free_devargs_list();
-
-	/* check PCI device with empty argument parsing */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "04:00.1") < 0)
-		goto fail;
-	devargs = TAILQ_FIRST(&devargs_list);
-	if (strcmp(devargs->name, "04:00.1") != 0)
-		goto fail;
-	if (!devargs->args || strcmp(devargs->args, "") != 0)
-		goto fail;
-	free_devargs_list();
-
-	/* test error case: bad PCI address */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "08:1") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "00.1") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "foo") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, ",") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "000f:0:0") == 0)
-		goto fail;
-
-	devargs_list = save_devargs_list;
-	return 0;
-
- fail:
-	free_devargs_list();
-	devargs_list = save_devargs_list;
-	return -1;
-}
-
-REGISTER_TEST_COMMAND(devargs_autotest, test_devargs);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH 6/6] devargs: make devargs_list private
  2017-08-25 16:07 [PATCH 0/6] devargs cleanup Gaetan Rivet
                   ` (4 preceding siblings ...)
  2017-08-25 16:07 ` [PATCH 5/6] test: remove rte_devargs unit tests Gaetan Rivet
@ 2017-08-25 16:07 ` Gaetan Rivet
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-08-25 16:07 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 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(-)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 610db67..91621b1 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -2,7 +2,6 @@ DPDK_2.0 {
 	global:
 
 	__rte_panic;
-	devargs_list;
 	eal_parse_sysfs_value;
 	eal_timer_source;
 	lcore_config;
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index e0e47e8..2b20ce0 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -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);
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 5ca5a32..d07810f 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -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.
  *
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index c1bc704..6595b64 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -2,7 +2,6 @@ DPDK_2.0 {
 	global:
 
 	__rte_panic;
-	devargs_list;
 	eal_parse_sysfs_value;
 	eal_timer_source;
 	lcore_config;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 00/18] devargs cleanup
  2017-08-25 16:07 [PATCH 0/6] devargs cleanup Gaetan Rivet
                   ` (5 preceding siblings ...)
  2017-08-25 16:07 ` [PATCH 6/6] devargs: make devargs_list private Gaetan Rivet
@ 2017-10-12  8:21 ` Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 01/18] eal: prepend busname on legacy device declaration Gaetan Rivet
                     ` (19 more replies)
  2018-04-23 22:41 ` [PATCH v4 " Gaetan Rivet
  2018-04-23 23:54 ` [PATCH v5 00/10] devargs cleanup Gaetan Rivet
  8 siblings, 20 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: 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

-- 
2.1.4

^ permalink raw reply	[flat|nested] 91+ messages in thread

* [PATCH v2 01/18] eal: prepend busname on legacy device declaration
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-12-11 13:57     ` Shreyansh Jain
  2017-10-12  8:21   ` [PATCH v2 02/18] eal: remove generic devtype Gaetan Rivet
                     ` (18 subsequent siblings)
  19 siblings, 1 reply; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_options.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 630c9d2..d57cb5d 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -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;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 02/18] eal: remove generic devtype
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 01/18] eal: prepend busname on legacy device declaration Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-17 18:16     ` Aaron Conole
  2017-10-12  8:21   ` [PATCH v2 03/18] devargs: introduce iterator Gaetan Rivet
                     ` (17 subsequent siblings)
  19 siblings, 1 reply; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 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(-)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index bbe862b..5fbcf11 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -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
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 573869a..47416a5 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -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;
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index e371456..2fddbfa 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -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)
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index d57cb5d..603df27 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -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:
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index 4c4ac7e..5f090ed 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -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 {
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 58d585d..e50c166 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -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
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index a2709e3..e1e2a50 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -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;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 03/18] devargs: introduce iterator
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 01/18] eal: prepend busname on legacy device declaration Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 02/18] eal: remove generic devtype Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 04/18] devargs: introduce foreach macro Gaetan Rivet
                     ` (16 subsequent siblings)
  19 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 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(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 47416a5..01ae0c7 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -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;
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 2fddbfa..614f1c5 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -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;
+}
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index e50c166..0eec406 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -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
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index e1e2a50..576de56 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -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;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 04/18] devargs: introduce foreach macro
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                     ` (2 preceding siblings ...)
  2017-10-12  8:21   ` [PATCH v2 03/18] devargs: introduce iterator Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 05/18] vdev: do not reference devargs list Gaetan Rivet
                     ` (15 subsequent siblings)
  19 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: Gaetan Rivet

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

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/include/rte_devargs.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 0eec406..6222677 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -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
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 05/18] vdev: do not reference devargs list
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                     ` (3 preceding siblings ...)
  2017-10-12  8:21   ` [PATCH v2 04/18] devargs: introduce foreach macro Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 06/18] bus/pci: " Gaetan Rivet
                     ` (14 subsequent siblings)
  19 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_vdev.c | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index f7e547a..a7410a6 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -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)
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 06/18] bus/pci: do not reference devargs list
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                     ` (4 preceding siblings ...)
  2017-10-12  8:21   ` [PATCH v2 05/18] vdev: do not reference devargs list Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 07/18] test: remove devargs unit tests Gaetan Rivet
                     ` (13 subsequent siblings)
  19 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: Gaetan Rivet

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

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 drivers/bus/pci/pci_common.c | 6 +-----
 1 file changed, 1 insertion(+), 5 deletions(-)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 5fbcf11..0b64d20 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -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;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 07/18] test: remove devargs unit tests
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                     ` (5 preceding siblings ...)
  2017-10-12  8:21   ` [PATCH v2 06/18] bus/pci: " Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 08/18] devargs: make devargs list private Gaetan Rivet
                     ` (12 subsequent siblings)
  19 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 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

diff --git a/MAINTAINERS b/MAINTAINERS
index b8b5441..6c174ef 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -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
diff --git a/test/test/Makefile b/test/test/Makefile
index 61e4699..3d76e5e 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -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
diff --git a/test/test/test_devargs.c b/test/test/test_devargs.c
deleted file mode 100644
index 18f54ed..0000000
--- a/test/test/test_devargs.c
+++ /dev/null
@@ -1,131 +0,0 @@
-/*-
- *   BSD LICENSE
- *
- *   Copyright 2014 6WIND S.A.
- *
- *   Redistribution and use in source and binary forms, with or without
- *   modification, are permitted provided that the following conditions
- *   are met:
- *
- *     * Redistributions of source code must retain the above copyright
- *       notice, this list of conditions and the following disclaimer.
- *     * Redistributions in binary form must reproduce the above copyright
- *       notice, this list of conditions and the following disclaimer in
- *       the documentation and/or other materials provided with the
- *       distribution.
- *     * Neither the name of 6WIND S.A nor the names of its contributors
- *       may be used to endorse or promote products derived from this
- *       software without specific prior written permission.
- *
- *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
- *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
- *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
- *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
- *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
- *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
- *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
- *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- */
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <string.h>
-#include <sys/queue.h>
-
-#include <rte_debug.h>
-#include <rte_devargs.h>
-
-#include "test.h"
-
-/* clear devargs list that was modified by the test */
-static void free_devargs_list(void)
-{
-	struct rte_devargs *devargs;
-
-	while (!TAILQ_EMPTY(&devargs_list)) {
-		devargs = TAILQ_FIRST(&devargs_list);
-		TAILQ_REMOVE(&devargs_list, devargs, next);
-		free(devargs->args);
-		free(devargs);
-	}
-}
-
-static int
-test_devargs(void)
-{
-	struct rte_devargs_list save_devargs_list;
-	struct rte_devargs *devargs;
-
-	/* save the real devargs_list, it is restored at the end of the test */
-	save_devargs_list = devargs_list;
-	TAILQ_INIT(&devargs_list);
-
-	/* test valid cases */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "08:00.1") < 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "0000:5:00.0") < 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI, "04:00.0,arg=val") < 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI, "0000:01:00.1") < 0)
-		goto fail;
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 2)
-		goto fail;
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 2)
-		goto fail;
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_VIRTUAL) != 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, "net_ring0") < 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, "net_ring1,key=val,k2=val2") < 0)
-		goto fail;
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_VIRTUAL) != 2)
-		goto fail;
-	free_devargs_list();
-
-	/* check virtual device with argument parsing */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, "net_ring1,k1=val,k2=val2") < 0)
-		goto fail;
-	devargs = TAILQ_FIRST(&devargs_list);
-	if (strncmp(devargs->name, "net_ring1",
-			sizeof(devargs->name)) != 0)
-		goto fail;
-	if (!devargs->args || strcmp(devargs->args, "k1=val,k2=val2") != 0)
-		goto fail;
-	free_devargs_list();
-
-	/* check PCI device with empty argument parsing */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "04:00.1") < 0)
-		goto fail;
-	devargs = TAILQ_FIRST(&devargs_list);
-	if (strcmp(devargs->name, "04:00.1") != 0)
-		goto fail;
-	if (!devargs->args || strcmp(devargs->args, "") != 0)
-		goto fail;
-	free_devargs_list();
-
-	/* test error case: bad PCI address */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "08:1") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "00.1") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "foo") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, ",") == 0)
-		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "000f:0:0") == 0)
-		goto fail;
-
-	devargs_list = save_devargs_list;
-	return 0;
-
- fail:
-	free_devargs_list();
-	devargs_list = save_devargs_list;
-	return -1;
-}
-
-REGISTER_TEST_COMMAND(devargs_autotest, test_devargs);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 08/18] devargs: make devargs list private
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                     ` (6 preceding siblings ...)
  2017-10-12  8:21   ` [PATCH v2 07/18] test: remove devargs unit tests Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 09/18] devargs: make parsing variadic Gaetan Rivet
                     ` (11 subsequent siblings)
  19 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 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(-)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 01ae0c7..0d693c8 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -2,7 +2,6 @@ DPDK_2.0 {
 	global:
 
 	__rte_panic;
-	devargs_list;
 	eal_parse_sysfs_value;
 	eal_timer_source;
 	lcore_config;
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 614f1c5..0f81f22 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -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);
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 6222677..5f4ad33 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -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.
  *
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 576de56..9c0251e 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -2,7 +2,6 @@ DPDK_2.0 {
 	global:
 
 	__rte_panic;
-	devargs_list;
 	eal_parse_sysfs_value;
 	eal_timer_source;
 	lcore_config;
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 09/18] devargs: make parsing variadic
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                     ` (7 preceding siblings ...)
  2017-10-12  8:21   ` [PATCH v2 08/18] devargs: make devargs list private Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 10/18] devargs: require bus name prefix Gaetan Rivet
                     ` (10 subsequent siblings)
  19 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 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(-)

diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c
index cfc83e3..08ce4ad 100644
--- a/drivers/net/failsafe/failsafe_args.c
+++ b/drivers/net/failsafe/failsafe_args.c
@@ -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;
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index e251275..b965e56 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -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;
 }
 
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 0f81f22..a21cc1a 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -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;
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 5f4ad33..1fe03d6 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -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.
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 10/18] devargs: require bus name prefix
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                     ` (8 preceding siblings ...)
  2017-10-12  8:21   ` [PATCH v2 09/18] devargs: make parsing variadic Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 11/18] devargs: simplify implementation Gaetan Rivet
                     ` (9 subsequent siblings)
  19 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: Gaetan Rivet

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

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_devargs.c | 28 +++++++++-------------------
 1 file changed, 9 insertions(+), 19 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index a21cc1a..49cc3b8 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -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]);
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 11/18] devargs: simplify implementation
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                     ` (9 preceding siblings ...)
  2017-10-12  8:21   ` [PATCH v2 10/18] devargs: require bus name prefix Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-16 11:39     ` Shreyansh Jain
  2017-10-12  8:21   ` [PATCH v2 12/18] eal: add generic device declaration parameter Gaetan Rivet
                     ` (8 subsequent siblings)
  19 siblings, 1 reply; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: Gaetan Rivet

Re-use existing code, remove incorrect comments.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_devargs.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 49cc3b8..1d87cd9 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -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:
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 12/18] eal: add generic device declaration parameter
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                     ` (10 preceding siblings ...)
  2017-10-12  8:21   ` [PATCH v2 11/18] devargs: simplify implementation Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-12-13 14:26     ` Shreyansh Jain
  2017-10-12  8:21   ` [PATCH v2 13/18] bus: make device recognition function public Gaetan Rivet
                     ` (7 subsequent siblings)
  19 siblings, 1 reply; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_options.c | 19 +++++++++++++++++++
 lib/librte_eal/common/eal_options.h        |  2 ++
 2 files changed, 21 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 603df27..b7591fd 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -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"
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index 30e6bb4..d50eff7 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -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"
-- 
2.1.4

^ permalink raw reply related	[flat|nested] 91+ messages in thread

* [PATCH v2 13/18] bus: make device recognition function public
  2017-10-12  8:21 ` [PATCH v2 00/18] devargs cleanup Gaetan Rivet
                     ` (11 preceding siblings ...)
  2017-10-12  8:21   ` [PATCH v2 12/18] eal: add generic device declaration parameter Gaetan Rivet
@ 2017-10-12  8:21   ` Gaetan Rivet
  2017-10-12  8:21   ` [PATCH v2 14/18] net/failsafe: keep legacy sub-device declaration Gaetan Rivet
                     ` (6 subsequent siblings)
  19 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12  8:21 UTC (permalink / raw)
  To: dev; +Cc: 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 <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/include/rte_bus.h         | 12 ++++++++++++
 lib/librte_eal/linuxapp