All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] eal: read and parse device option separately
@ 2017-08-01 18:13 Gaetan Rivet
  2017-08-02  8:56 ` [PATCH v2] " Gaetan Rivet
  0 siblings, 1 reply; 6+ messages in thread
From: Gaetan Rivet @ 2017-08-01 18:13 UTC (permalink / raw)
  To: dev; +Cc: Gaetan Rivet, Dirk-Holger Lenz

When the EAL parses the common options given to the application,
not all subsystems are available. Some device drivers are registered
afterward upon dynamic plugin loading.

Devices using those drivers are thus unable to be parsed by any drivers
and are rejected.

Store the device options first and keep them for later processing.
Parse these right before initializing the buses, the drivers must have
been stabilized at this point.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal.c            |  5 +++
 lib/librte_eal/common/eal_common_options.c | 63 ++++++++++++++++++++++++++++--
 lib/librte_eal/common/eal_options.h        |  1 +
 lib/librte_eal/linuxapp/eal/eal.c          |  5 +++
 4 files changed, 71 insertions(+), 3 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 80fe21d..5fa5988 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -614,6 +614,11 @@ rte_eal_init(int argc, char **argv)
 		rte_config.master_lcore, thread_id, cpuset,
 		ret == 0 ? "" : "...");
 
+	if (eal_option_device_parse()) {
+		rte_errno = ENODEV;
+		return -1;
+	}
+
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices\n");
 		rte_errno = ENODEV;
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 56c368c..ead089e 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -47,6 +47,7 @@
 #include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_lcore.h>
+#include <rte_tailq.h>
 #include <rte_version.h>
 #include <rte_devargs.h>
 #include <rte_memcpy.h>
@@ -125,11 +126,67 @@ static const char *default_solib_dir = RTE_EAL_PMD_PATH;
 static const char dpdk_solib_path[] __attribute__((used)) =
 "DPDK_PLUGIN_PATH=" RTE_EAL_PMD_PATH;
 
+TAILQ_HEAD(device_option_list, device_option);
+
+struct device_option {
+	TAILQ_ENTRY(device_option) next;
+
+	enum rte_devtype type;
+	char optarg[];
+};
+
+static struct device_option_list devopt_list =
+TAILQ_HEAD_INITIALIZER(devopt_list);
 
 static int master_lcore_parsed;
 static int mem_parsed;
 static int core_parsed;
 
+static int
+eal_option_device_add(enum rte_devtype type, const char *optarg)
+{
+	struct device_option *deo;
+	size_t optlen;
+	int ret;
+
+	optlen = strlen(optarg) + 1;
+	deo = calloc(1, sizeof(*deo) + optlen);
+	if (deo == NULL) {
+		RTE_LOG(ERR, EAL, "Unable to allocate device option\n");
+		return -ENOMEM;
+	}
+
+	deo->type = type;
+	ret = snprintf(deo->optarg, optlen, "%s", optarg);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL, "Unable to copy device option\n");
+		free(deo);
+		return -EINVAL;
+	}
+	TAILQ_INSERT_TAIL(&devopt_list, deo, next);
+	return 0;
+}
+
+int
+eal_option_device_parse(void)
+{
+	struct device_option *deo;
+	void *tmp;
+	int ret = 0;
+
+	TAILQ_FOREACH_SAFE(deo, &devopt_list, next, tmp) {
+		if (ret == 0) {
+			ret = rte_eal_devargs_add(deo->type, deo->optarg);
+			if (ret)
+				RTE_LOG(ERR, EAL, "Unable to parse device '%s'\n",
+					deo->optarg);
+		}
+		TAILQ_REMOVE(&devopt_list, deo, next);
+		free(deo);
+	}
+	return ret;
+}
+
 void
 eal_reset_internal_config(struct internal_config *internal_cfg)
 {
@@ -944,14 +1001,14 @@ eal_parse_common_option(int opt, const char *optarg,
 	switch (opt) {
 	/* blacklist */
 	case 'b':
-		if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI,
+		if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
 				optarg) < 0) {
 			return -1;
 		}
 		break;
 	/* whitelist */
 	case 'w':
-		if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI,
+		if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
 				optarg) < 0) {
 			return -1;
 		}
@@ -1061,7 +1118,7 @@ eal_parse_common_option(int opt, const char *optarg,
 		break;
 
 	case OPT_VDEV_NUM:
-		if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,
+		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
 				optarg) < 0) {
 			return -1;
 		}
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index a881c62..439a261 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -91,6 +91,7 @@ extern const struct option eal_long_options[];
 
 int eal_parse_common_option(int opt, const char *argv,
 			    struct internal_config *conf);
+int eal_option_device_parse(void);
 int eal_adjust_config(struct internal_config *internal_cfg);
 int eal_check_common_options(struct internal_config *internal_cfg);
 void eal_common_usage(void);
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index b28bbab..48f12f4 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -889,6 +889,11 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
+	if (eal_option_device_parse()) {
+		rte_errno = ENODEV;
+		return -1;
+	}
+
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices\n");
 		rte_errno = ENODEV;
-- 
2.1.4

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

* [PATCH v2] eal: read and parse device option separately
  2017-08-01 18:13 [PATCH] eal: read and parse device option separately Gaetan Rivet
@ 2017-08-02  8:56 ` Gaetan Rivet
  2017-08-02 14:29   ` Thomas Monjalon
  2017-08-02 17:10   ` [PATCH v3] " Gaetan Rivet
  0 siblings, 2 replies; 6+ messages in thread
From: Gaetan Rivet @ 2017-08-02  8:56 UTC (permalink / raw)
  To: dev; +Cc: Gaetan Rivet, Dirk-Holger Lenz

When the EAL parses the common options given to the application,
not all subsystems are available. Some device drivers are registered
afterward upon dynamic plugin loading.

Devices using those drivers are thus unable to be parsed by any drivers
and are rejected.

Store the device options first and keep them for later processing.
Parse these right before initializing the buses, the drivers must have
been stabilized at this point.

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

v2:

  - Fix the -w / -b incompatibility check.
    It was checking that no two incompatible rte_devargs were inserted.
    As rte_devargs are now generated right before buses scan, this check
    was always correct, even when it should have failed.

 lib/librte_eal/bsdapp/eal/eal.c            |  5 ++
 lib/librte_eal/common/eal_common_options.c | 79 ++++++++++++++++++++++++++++--
 lib/librte_eal/common/eal_options.h        |  1 +
 lib/librte_eal/linuxapp/eal/eal.c          |  5 ++
 4 files changed, 85 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 80fe21d..5fa5988 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -614,6 +614,11 @@ rte_eal_init(int argc, char **argv)
 		rte_config.master_lcore, thread_id, cpuset,
 		ret == 0 ? "" : "...");
 
+	if (eal_option_device_parse()) {
+		rte_errno = ENODEV;
+		return -1;
+	}
+
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices\n");
 		rte_errno = ENODEV;
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 56c368c..01fcc18 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -47,6 +47,7 @@
 #include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_lcore.h>
+#include <rte_tailq.h>
 #include <rte_version.h>
 #include <rte_devargs.h>
 #include <rte_memcpy.h>
@@ -125,11 +126,79 @@ static const char *default_solib_dir = RTE_EAL_PMD_PATH;
 static const char dpdk_solib_path[] __attribute__((used)) =
 "DPDK_PLUGIN_PATH=" RTE_EAL_PMD_PATH;
 
+TAILQ_HEAD(device_option_list, device_option);
+
+struct device_option {
+	TAILQ_ENTRY(device_option) next;
+
+	enum rte_devtype type;
+	char optarg[];
+};
+
+static struct device_option_list devopt_list =
+TAILQ_HEAD_INITIALIZER(devopt_list);
 
 static int master_lcore_parsed;
 static int mem_parsed;
 static int core_parsed;
 
+static int
+eal_option_device_add(enum rte_devtype type, const char *optarg)
+{
+	struct device_option *deo;
+	size_t optlen;
+	int ret;
+
+	optlen = strlen(optarg) + 1;
+	deo = calloc(1, sizeof(*deo) + optlen);
+	if (deo == NULL) {
+		RTE_LOG(ERR, EAL, "Unable to allocate device option\n");
+		return -ENOMEM;
+	}
+
+	deo->type = type;
+	ret = snprintf(deo->optarg, optlen, "%s", optarg);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL, "Unable to copy device option\n");
+		free(deo);
+		return -EINVAL;
+	}
+	TAILQ_INSERT_TAIL(&devopt_list, deo, next);
+	return 0;
+}
+
+static unsigned int
+eal_option_device_type_count(enum rte_devtype type)
+{
+	struct device_option *deo;
+	unsigned int count = 0;
+
+	TAILQ_FOREACH(deo, &devopt_list, next)
+		if (deo->type == type)
+			count++;
+	return count;
+}
+
+int
+eal_option_device_parse(void)
+{
+	struct device_option *deo;
+	void *tmp;
+	int ret = 0;
+
+	TAILQ_FOREACH_SAFE(deo, &devopt_list, next, tmp) {
+		if (ret == 0) {
+			ret = rte_eal_devargs_add(deo->type, deo->optarg);
+			if (ret)
+				RTE_LOG(ERR, EAL, "Unable to parse device '%s'\n",
+					deo->optarg);
+		}
+		TAILQ_REMOVE(&devopt_list, deo, next);
+		free(deo);
+	}
+	return ret;
+}
+
 void
 eal_reset_internal_config(struct internal_config *internal_cfg)
 {
@@ -944,14 +1013,14 @@ eal_parse_common_option(int opt, const char *optarg,
 	switch (opt) {
 	/* blacklist */
 	case 'b':
-		if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI,
+		if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
 				optarg) < 0) {
 			return -1;
 		}
 		break;
 	/* whitelist */
 	case 'w':
-		if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI,
+		if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
 				optarg) < 0) {
 			return -1;
 		}
@@ -1061,7 +1130,7 @@ eal_parse_common_option(int opt, const char *optarg,
 		break;
 
 	case OPT_VDEV_NUM:
-		if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,
+		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
 				optarg) < 0) {
 			return -1;
 		}
@@ -1187,8 +1256,8 @@ 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) {
+	if (eal_option_device_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 &&
+		eal_option_device_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;
diff --git a/lib/librte_eal/common/eal_options.h b/lib/librte_eal/common/eal_options.h
index a881c62..439a261 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -91,6 +91,7 @@ extern const struct option eal_long_options[];
 
 int eal_parse_common_option(int opt, const char *argv,
 			    struct internal_config *conf);
+int eal_option_device_parse(void);
 int eal_adjust_config(struct internal_config *internal_cfg);
 int eal_check_common_options(struct internal_config *internal_cfg);
 void eal_common_usage(void);
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index b28bbab..48f12f4 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -889,6 +889,11 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
+	if (eal_option_device_parse()) {
+		rte_errno = ENODEV;
+		return -1;
+	}
+
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices\n");
 		rte_errno = ENODEV;
-- 
2.1.4

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

* Re: [PATCH v2] eal: read and parse device option separately
  2017-08-02  8:56 ` [PATCH v2] " Gaetan Rivet
@ 2017-08-02 14:29   ` Thomas Monjalon
  2017-08-02 15:23     ` Gaëtan Rivet
  2017-08-02 17:10   ` [PATCH v3] " Gaetan Rivet
  1 sibling, 1 reply; 6+ messages in thread
From: Thomas Monjalon @ 2017-08-02 14:29 UTC (permalink / raw)
  To: Gaetan Rivet; +Cc: dev, Dirk-Holger Lenz

Hi Gaetan

02/08/2017 10:56, Gaetan Rivet:
> When the EAL parses the common options given to the application,
> not all subsystems are available. Some device drivers are registered
> afterward upon dynamic plugin loading.
> 
> Devices using those drivers are thus unable to be parsed by any drivers
> and are rejected.
> 
> Store the device options first and keep them for later processing.
> Parse these right before initializing the buses, the drivers must have
> been stabilized at this point.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
> 
> v2:
> 
>   - Fix the -w / -b incompatibility check.
>     It was checking that no two incompatible rte_devargs were inserted.
>     As rte_devargs are now generated right before buses scan, this check
>     was always correct, even when it should have failed.

You are doing this check in eal_check_common_options().
I think you can do the check earlier in eal_parse_common_option()
with 2 static variables, instead of implementing the
new function eal_option_device_type_count().

> +static unsigned int
> +eal_option_device_type_count(enum rte_devtype type)
[...]
> @@ -944,14 +1013,14 @@ eal_parse_common_option(int opt, const char *optarg,
>  	switch (opt) {
>  	/* blacklist */
>  	case 'b':
> -		if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI,
> +		if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
>  				optarg) < 0) {
>  			return -1;
>  		}
>  		break;
>  	/* whitelist */
>  	case 'w':
> -		if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI,
> +		if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
>  				optarg) < 0) {
>  			return -1;
>  		}
> @@ -1061,7 +1130,7 @@ eal_parse_common_option(int opt, const char *optarg,
>  		break;
>  
>  	case OPT_VDEV_NUM:
> -		if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,
> +		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
>  				optarg) < 0) {
>  			return -1;
>  		}
> @@ -1187,8 +1256,8 @@ 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) {
> +	if (eal_option_device_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 &&
> +		eal_option_device_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;

[...]
> +static int
> +eal_option_device_add(enum rte_devtype type, const char *optarg)
> +{
> +	struct device_option *deo;
> +	size_t optlen;

Just a last comment about variable name:
Instead of deo, opt or option would be more meaningful.

Thanks for the important fix!

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

* Re: [PATCH v2] eal: read and parse device option separately
  2017-08-02 14:29   ` Thomas Monjalon
@ 2017-08-02 15:23     ` Gaëtan Rivet
  0 siblings, 0 replies; 6+ messages in thread
From: Gaëtan Rivet @ 2017-08-02 15:23 UTC (permalink / raw)
  To: Thomas Monjalon; +Cc: dev, Dirk-Holger Lenz

On Wed, Aug 02, 2017 at 04:29:13PM +0200, Thomas Monjalon wrote:
> Hi Gaetan
> 
> 02/08/2017 10:56, Gaetan Rivet:
> > When the EAL parses the common options given to the application,
> > not all subsystems are available. Some device drivers are registered
> > afterward upon dynamic plugin loading.
> > 
> > Devices using those drivers are thus unable to be parsed by any drivers
> > and are rejected.
> > 
> > Store the device options first and keep them for later processing.
> > Parse these right before initializing the buses, the drivers must have
> > been stabilized at this point.
> > 
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> > 
> > v2:
> > 
> >   - Fix the -w / -b incompatibility check.
> >     It was checking that no two incompatible rte_devargs were inserted.
> >     As rte_devargs are now generated right before buses scan, this check
> >     was always correct, even when it should have failed.
> 
> You are doing this check in eal_check_common_options().
> I think you can do the check earlier in eal_parse_common_option()
> with 2 static variables, instead of implementing the
> new function eal_option_device_type_count().
> 

Okay, will simplify.

> > +static unsigned int
> > +eal_option_device_type_count(enum rte_devtype type)
> [...]
> > @@ -944,14 +1013,14 @@ eal_parse_common_option(int opt, const char *optarg,
> >  	switch (opt) {
> >  	/* blacklist */
> >  	case 'b':
> > -		if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI,
> > +		if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
> >  				optarg) < 0) {
> >  			return -1;
> >  		}
> >  		break;
> >  	/* whitelist */
> >  	case 'w':
> > -		if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI,
> > +		if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
> >  				optarg) < 0) {
> >  			return -1;
> >  		}
> > @@ -1061,7 +1130,7 @@ eal_parse_common_option(int opt, const char *optarg,
> >  		break;
> >  
> >  	case OPT_VDEV_NUM:
> > -		if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,
> > +		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
> >  				optarg) < 0) {
> >  			return -1;
> >  		}
> > @@ -1187,8 +1256,8 @@ 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) {
> > +	if (eal_option_device_type_count(RTE_DEVTYPE_WHITELISTED_PCI) != 0 &&
> > +		eal_option_device_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;
> 
> [...]
> > +static int
> > +eal_option_device_add(enum rte_devtype type, const char *optarg)
> > +{
> > +	struct device_option *deo;
> > +	size_t optlen;
> 
> Just a last comment about variable name:
> Instead of deo, opt or option would be more meaningful.
> 

Will do.

> Thanks for the important fix!

-- 
Gaëtan Rivet
6WIND

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

* [PATCH v3] eal: read and parse device option separately
  2017-08-02  8:56 ` [PATCH v2] " Gaetan Rivet
  2017-08-02 14:29   ` Thomas Monjalon
@ 2017-08-02 17:10   ` Gaetan Rivet
  2017-08-03 17:57     ` Thomas Monjalon
  1 sibling, 1 reply; 6+ messages in thread
From: Gaetan Rivet @ 2017-08-02 17:10 UTC (permalink / raw)
  To: dev; +Cc: Gaetan Rivet, Dirk-Holger Lenz

When the EAL parses the common options given to the application,
not all subsystems are available. Some device drivers are registered
afterward upon dynamic plugin loading.

Devices using those drivers are thus unable to be parsed by any drivers
and are rejected.

Store the device options first and keep them for later processing.
Parse these right before initializing the buses, the drivers must have
been stabilized at this point.

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Tested-by: Dirk-Holger Lenz <dirk.lenz@ng4t.com>
---

v2:

  - Fix the -w / -b incompatibility check.
    It was checking that no two incompatible rte_devargs were inserted.
    As rte_devargs are now generated right before buses scan, this check
    was always correct, even when it should have failed.

v3:

  - Fix variable name (deo --> devopt)
  - Use static variables to check for -w and -b concomitant use.

 lib/librte_eal/bsdapp/eal/eal.c            |  5 ++
 lib/librte_eal/common/eal_common_options.c | 83 ++++++++++++++++++++++++++----
 lib/librte_eal/common/eal_options.h        |  1 +
 lib/librte_eal/linuxapp/eal/eal.c          |  5 ++
 4 files changed, 84 insertions(+), 10 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 80fe21d..5fa5988 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -614,6 +614,11 @@ rte_eal_init(int argc, char **argv)
 		rte_config.master_lcore, thread_id, cpuset,
 		ret == 0 ? "" : "...");
 
+	if (eal_option_device_parse()) {
+		rte_errno = ENODEV;
+		return -1;
+	}
+
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices\n");
 		rte_errno = ENODEV;
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 56c368c..1da185e 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -47,6 +47,7 @@
 #include <rte_eal.h>
 #include <rte_log.h>
 #include <rte_lcore.h>
+#include <rte_tailq.h>
 #include <rte_version.h>
 #include <rte_devargs.h>
 #include <rte_memcpy.h>
@@ -125,11 +126,67 @@ static const char *default_solib_dir = RTE_EAL_PMD_PATH;
 static const char dpdk_solib_path[] __attribute__((used)) =
 "DPDK_PLUGIN_PATH=" RTE_EAL_PMD_PATH;
 
+TAILQ_HEAD(device_option_list, device_option);
+
+struct device_option {
+	TAILQ_ENTRY(device_option) next;
+
+	enum rte_devtype type;
+	char arg[];
+};
+
+static struct device_option_list devopt_list =
+TAILQ_HEAD_INITIALIZER(devopt_list);
 
 static int master_lcore_parsed;
 static int mem_parsed;
 static int core_parsed;
 
+static int
+eal_option_device_add(enum rte_devtype type, const char *optarg)
+{
+	struct device_option *devopt;
+	size_t optlen;
+	int ret;
+
+	optlen = strlen(optarg) + 1;
+	devopt = calloc(1, sizeof(*devopt) + optlen);
+	if (devopt == NULL) {
+		RTE_LOG(ERR, EAL, "Unable to allocate device option\n");
+		return -ENOMEM;
+	}
+
+	devopt->type = type;
+	ret = snprintf(devopt->arg, optlen, "%s", optarg);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL, "Unable to copy device option\n");
+		free(devopt);
+		return -EINVAL;
+	}
+	TAILQ_INSERT_TAIL(&devopt_list, devopt, next);
+	return 0;
+}
+
+int
+eal_option_device_parse(void)
+{
+	struct device_option *devopt;
+	void *tmp;
+	int ret = 0;
+
+	TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {
+		if (ret == 0) {
+			ret = rte_eal_devargs_add(devopt->type, devopt->arg);
+			if (ret)
+				RTE_LOG(ERR, EAL, "Unable to parse device '%s'\n",
+					devopt->arg);
+		}
+		TAILQ_REMOVE(&devopt_list, devopt, next);
+		free(devopt);
+	}
+	return ret;
+}
+
 void
 eal_reset_internal_config(struct internal_config *internal_cfg)
 {
@@ -941,20 +998,29 @@ int
 eal_parse_common_option(int opt, const char *optarg,
 			struct internal_config *conf)
 {
+	static int b_used;
+	static int w_used;
+
 	switch (opt) {
 	/* blacklist */
 	case 'b':
-		if (rte_eal_devargs_add(RTE_DEVTYPE_BLACKLISTED_PCI,
+		if (w_used)
+			goto bw_used;
+		if (eal_option_device_add(RTE_DEVTYPE_BLACKLISTED_PCI,
 				optarg) < 0) {
 			return -1;
 		}
+		b_used = 1;
 		break;
 	/* whitelist */
 	case 'w':
-		if (rte_eal_devargs_add(RTE_DEVTYPE_WHITELISTED_PCI,
+		if (b_used)
+			goto bw_used;
+		if (eal_option_device_add(RTE_DEVTYPE_WHITELISTED_PCI,
 				optarg) < 0) {
 			return -1;
 		}
+		w_used = 1;
 		break;
 	/* coremask */
 	case 'c':
@@ -1061,7 +1127,7 @@ eal_parse_common_option(int opt, const char *optarg,
 		break;
 
 	case OPT_VDEV_NUM:
-		if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL,
+		if (eal_option_device_add(RTE_DEVTYPE_VIRTUAL,
 				optarg) < 0) {
 			return -1;
 		}
@@ -1100,6 +1166,10 @@ eal_parse_common_option(int opt, const char *optarg,
 	}
 
 	return 0;
+bw_used:
+	RTE_LOG(ERR, EAL, "Options blacklist (-b) and whitelist (-w) "
+		"cannot be used at the same time\n");
+	return -1;
 }
 
 static void
@@ -1187,13 +1257,6 @@ 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_options.h b/lib/librte_eal/common/eal_options.h
index a881c62..439a261 100644
--- a/lib/librte_eal/common/eal_options.h
+++ b/lib/librte_eal/common/eal_options.h
@@ -91,6 +91,7 @@ extern const struct option eal_long_options[];
 
 int eal_parse_common_option(int opt, const char *argv,
 			    struct internal_config *conf);
+int eal_option_device_parse(void);
 int eal_adjust_config(struct internal_config *internal_cfg);
 int eal_check_common_options(struct internal_config *internal_cfg);
 void eal_common_usage(void);
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index b28bbab..48f12f4 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -889,6 +889,11 @@ rte_eal_init(int argc, char **argv)
 		return -1;
 	}
 
+	if (eal_option_device_parse()) {
+		rte_errno = ENODEV;
+		return -1;
+	}
+
 	if (rte_bus_scan()) {
 		rte_eal_init_alert("Cannot scan the buses for devices\n");
 		rte_errno = ENODEV;
-- 
2.1.4

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

* Re: [PATCH v3] eal: read and parse device option separately
  2017-08-02 17:10   ` [PATCH v3] " Gaetan Rivet
@ 2017-08-03 17:57     ` Thomas Monjalon
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2017-08-03 17:57 UTC (permalink / raw)
  To: Gaetan Rivet; +Cc: dev, Dirk-Holger Lenz

02/08/2017 19:10, Gaetan Rivet:
> When the EAL parses the common options given to the application,
> not all subsystems are available. Some device drivers are registered
> afterward upon dynamic plugin loading.
> 
> Devices using those drivers are thus unable to be parsed by any drivers
> and are rejected.
> 
> Store the device options first and keep them for later processing.
> Parse these right before initializing the buses, the drivers must have
> been stabilized at this point.
> 
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> Tested-by: Dirk-Holger Lenz <dirk.lenz@ng4t.com>

Applied, thanks

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

end of thread, other threads:[~2017-08-03 17:57 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-01 18:13 [PATCH] eal: read and parse device option separately Gaetan Rivet
2017-08-02  8:56 ` [PATCH v2] " Gaetan Rivet
2017-08-02 14:29   ` Thomas Monjalon
2017-08-02 15:23     ` Gaëtan Rivet
2017-08-02 17:10   ` [PATCH v3] " Gaetan Rivet
2017-08-03 17:57     ` Thomas Monjalon

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.