DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] devargs: restore rte_devtype API
@ 2017-07-15 17:59 Gaetan Rivet
  2017-07-19 21:32 ` Thomas Monjalon
  0 siblings, 1 reply; 2+ messages in thread
From: Gaetan Rivet @ 2017-07-15 17:59 UTC (permalink / raw)
  To: dev; +Cc: Gaetan Rivet

Revert "devargs: make device types generic"

This commit broke the rte_devargs API by changing the meaning of
the rte_devtype enum.

Restore the previous API, unit tests and function calls.
Introduce parallel enum that acts as translation between previous API
and current structures.

Restoring the previous API means that -w and -b are not usable anymore
with any bus having implemented the "parse" operation. Only PCI devices
can be used with -w and -b, virtual devices are declared using vdev.

This (partially) reverts commit bd279a79366f50a4893fb84db91bbf64b56f9fb1.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---

As correctly pointed out by Jan Blunck in [1], the previous change breaks an existing API.
This commit only deals with this API. Doing so however, restore the previous behavior as well.
This means that the devargs behavior is iso-functional with that of the v17.05.
It is possible, however, to explicitly set the bus in a device declaration.

[1]: http://dpdk.org/ml/archives/dev/2017-July/071318.html

 lib/librte_eal/common/eal_common_devargs.c  | 11 +++++----
 lib/librte_eal/common/eal_common_options.c  | 13 ++++++++---
 lib/librte_eal/common/eal_common_pci.c      |  6 ++---
 lib/librte_eal/common/include/rte_dev.h     |  8 +++++++
 lib/librte_eal/common/include/rte_devargs.h |  8 ++++---
 test/test/test_devargs.c                    | 36 ++++++++++++++---------------
 6 files changed, 50 insertions(+), 32 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index ff6c2a8..33e9f0a 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -40,6 +40,7 @@
 #include <stdio.h>
 #include <string.h>
 
+#include <rte_dev.h>
 #include <rte_devargs.h>
 #include <rte_tailq.h>
 #include "eal_private.h"
@@ -167,18 +168,20 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 		goto fail;
 	devargs->type = devtype;
 	bus = devargs->bus;
-	if (devargs->type == RTE_DEVTYPE_WHITELISTED) {
+	if (devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI)
+		devargs->policy = RTE_DEV_BLACKLISTED;
+	if (devargs->policy == RTE_DEV_WHITELISTED) {
 		if (bus->conf.scan_mode == RTE_BUS_SCAN_UNDEFINED) {
 			bus->conf.scan_mode = RTE_BUS_SCAN_WHITELIST;
 		} else if (bus->conf.scan_mode == RTE_BUS_SCAN_BLACKLIST) {
-			fprintf(stderr, "ERROR: incompatible device type and bus scan mode\n");
+			fprintf(stderr, "ERROR: incompatible device policy and bus scan mode\n");
 			goto fail;
 		}
-	} else if (devargs->type == RTE_DEVTYPE_BLACKLISTED) {
+	} else if (devargs->policy == RTE_DEV_BLACKLISTED) {
 		if (bus->conf.scan_mode == RTE_BUS_SCAN_UNDEFINED) {
 			bus->conf.scan_mode = RTE_BUS_SCAN_BLACKLIST;
 		} else if (bus->conf.scan_mode == RTE_BUS_SCAN_WHITELIST) {
-			fprintf(stderr, "ERROR: incompatible device type and bus scan mode\n");
+			fprintf(stderr, "ERROR: incompatible device policy and bus scan mode\n");
 			goto fail;
 		}
 	}
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 075b0ea..f470195 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -798,14 +798,14 @@ eal_parse_common_option(int opt, const char *optarg,
 	switch (opt) {
 	/* blacklist */
 	case 'b':
-		if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED,
+		if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI,
 				optarg) < 0) {
 			return -1;
 		}
 		break;
 	/* whitelist */
 	case 'w':
-		if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED,
+		if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI,
 				optarg) < 0) {
 			return -1;
 		}
@@ -901,7 +901,7 @@ eal_parse_common_option(int opt, const char *optarg,
 		break;
 
 	case OPT_VDEV_NUM:
-		if (rte_eal_devargs_add(RTE_DEVTYPE_UNDEFINED,
+		if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,
 				optarg) < 0) {
 			return -1;
 		}
@@ -1025,6 +1025,13 @@ eal_check_common_options(struct internal_config *internal_cfg)
 		return -1;
 	}
 
+	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 &&
+		rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 0) {
+		RTE_LOG(ERR, EAL, "Options blacklist (-b) and whitelist (-w) "
+			"cannot be used at the same time\n");
+		return -1;
+	}
+
 	return 0;
 }
 
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 9ad1bf1..eaa041e 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -219,8 +219,8 @@ rte_pci_probe_one_driver(struct rte_pci_driver *dr,
 
 	/* no initialization when blacklisted, return without error */
 	if (dev->device.devargs != NULL &&
-		dev->device.devargs->type ==
-			RTE_DEVTYPE_BLACKLISTED) {
+		dev->device.devargs->policy ==
+			RTE_DEV_BLACKLISTED) {
 		RTE_LOG(INFO, EAL, "  Device is blacklisted, not"
 			" initializing\n");
 		return 1;
@@ -424,7 +424,7 @@ rte_pci_probe(void)
 		if (probe_all)
 			ret = pci_probe_all_drivers(dev);
 		else if (devargs != NULL &&
-			devargs->type == RTE_DEVTYPE_WHITELISTED)
+			devargs->policy == RTE_DEV_WHITELISTED)
 			ret = pci_probe_all_drivers(dev);
 		if (ret < 0) {
 			RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMT
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index bcd8b1e..5386d3a 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -127,6 +127,14 @@ 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 7b63fa3..58d585d 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -56,9 +56,9 @@ extern "C" {
  * Type of generic device
  */
 enum rte_devtype {
-	RTE_DEVTYPE_UNDEFINED,
-	RTE_DEVTYPE_WHITELISTED,
-	RTE_DEVTYPE_BLACKLISTED,
+	RTE_DEVTYPE_WHITELISTED_PCI,
+	RTE_DEVTYPE_BLACKLISTED_PCI,
+	RTE_DEVTYPE_VIRTUAL,
 };
 
 /**
@@ -76,6 +76,8 @@ struct rte_devargs {
 	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. */
diff --git a/test/test/test_devargs.c b/test/test/test_devargs.c
index 149c9c9..18f54ed 100644
--- a/test/test/test_devargs.c
+++ b/test/test/test_devargs.c
@@ -64,32 +64,30 @@ test_devargs(void)
 	TAILQ_INIT(&devargs_list);
 
 	/* test valid cases */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED, "08:00.1") < 0)
+	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "08:00.1") < 0)
 		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED, "0000:5:00.0") < 0)
+	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "0000:5:00.0") < 0)
 		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED, "04:00.0,arg=val") < 0)
+	if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI, "04:00.0,arg=val") < 0)
 		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED, "0000:01:00.1") < 0)
+	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) != 4)
+	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 2)
 		goto fail;
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED) != 0)
+	if (rte_eal_devargs_type_count(RTE_DEVTYPE_BLACKLISTED_PCI) != 2)
 		goto fail;
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_UNDEFINED) != 0)
+	if (rte_eal_devargs_type_count(RTE_DEVTYPE_VIRTUAL) != 0)
 		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_UNDEFINED, "net_ring0") < 0)
+	if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, "net_ring0") < 0)
 		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_UNDEFINED,
-				"net_ring1,key=val,k2=val2") < 0)
+	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_UNDEFINED) != 2)
+	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_UNDEFINED,
-				"net_ring1,k1=val,k2=val2") < 0)
+	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",
@@ -100,7 +98,7 @@ test_devargs(void)
 	free_devargs_list();
 
 	/* check PCI device with empty argument parsing */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED, "04:00.1") < 0)
+	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)
@@ -110,15 +108,15 @@ test_devargs(void)
 	free_devargs_list();
 
 	/* test error case: bad PCI address */
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED, "08:1") == 0)
+	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "08:1") == 0)
 		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED, "00.1") == 0)
+	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "00.1") == 0)
 		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED, "foo") == 0)
+	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "foo") == 0)
 		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED, ",") == 0)
+	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, ",") == 0)
 		goto fail;
-	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED, "000f:0:0") == 0)
+	if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI, "000f:0:0") == 0)
 		goto fail;
 
 	devargs_list = save_devargs_list;
-- 
2.1.4

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

* Re: [dpdk-dev] [PATCH] devargs: restore rte_devtype API
  2017-07-15 17:59 [dpdk-dev] [PATCH] devargs: restore rte_devtype API Gaetan Rivet
@ 2017-07-19 21:32 ` Thomas Monjalon
  0 siblings, 0 replies; 2+ messages in thread
From: Thomas Monjalon @ 2017-07-19 21:32 UTC (permalink / raw)
  To: Gaetan Rivet; +Cc: dev, jblunck

15/07/2017 20:59, Gaetan Rivet:
> Revert "devargs: make device types generic"
> 
> This commit broke the rte_devargs API by changing the meaning of
> the rte_devtype enum.
> 
> Restore the previous API, unit tests and function calls.
> Introduce parallel enum that acts as translation between previous API
> and current structures.
> 
> Restoring the previous API means that -w and -b are not usable anymore
> with any bus having implemented the "parse" operation. Only PCI devices
> can be used with -w and -b, virtual devices are declared using vdev.
> 
> This (partially) reverts commit bd279a79366f50a4893fb84db91bbf64b56f9fb1.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
> 
> As correctly pointed out by Jan Blunck in [1], the previous change breaks an existing API.
> This commit only deals with this API. Doing so however, restore the previous behavior as well.
> This means that the devargs behavior is iso-functional with that of the v17.05.
> It is possible, however, to explicitly set the bus in a device declaration.
> 
> [1]: http://dpdk.org/ml/archives/dev/2017-July/071318.html

Applied, thanks

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

end of thread, other threads:[~2017-07-19 21:32 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-15 17:59 [dpdk-dev] [PATCH] devargs: restore rte_devtype API Gaetan Rivet
2017-07-19 21:32 ` Thomas Monjalon

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).