* [dpdk-dev] [PATCH v2 01/18] eal: prepend busname on legacy device declaration
2017-10-12 8:21 ` [dpdk-dev] [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 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 01/18] eal: prepend busname on legacy device declaration
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 01/18] eal: prepend busname on legacy device declaration Gaetan Rivet
@ 2017-12-11 13:57 ` Shreyansh Jain
0 siblings, 0 replies; 91+ messages in thread
From: Shreyansh Jain @ 2017-12-11 13:57 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev
One very quick comment:
On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
> 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;
I think you want "optlen += strlen(busname) + 1";
> devopt = calloc(1, sizeof(*devopt) + optlen);
> if (devopt == NULL) {
> RTE_LOG(ERR, EAL, "Unable to allocate device option\n");
[...]
^ permalink raw reply [flat|nested] 91+ messages in thread
* [dpdk-dev] [PATCH v2 02/18] eal: remove generic devtype
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [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 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 02/18] eal: remove generic devtype
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 02/18] eal: remove generic devtype Gaetan Rivet
@ 2017-10-17 18:16 ` Aaron Conole
2017-10-18 8:20 ` Gaëtan Rivet
0 siblings, 1 reply; 91+ messages in thread
From: Aaron Conole @ 2017-10-17 18:16 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev
Gaetan Rivet <gaetan.rivet@6wind.com> writes:
> 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))
What is the intent of this? If probe_all is true, and devargs != null,
I think this branch isn't taken.
Shouldn't this be ||? Maybe I missed something?
> 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;
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 02/18] eal: remove generic devtype
2017-10-17 18:16 ` Aaron Conole
@ 2017-10-18 8:20 ` Gaëtan Rivet
0 siblings, 0 replies; 91+ messages in thread
From: Gaëtan Rivet @ 2017-10-18 8:20 UTC (permalink / raw)
To: Aaron Conole; +Cc: dev
Hello Aaron,
On Tue, Oct 17, 2017 at 02:16:59PM -0400, Aaron Conole wrote:
> Gaetan Rivet <gaetan.rivet@6wind.com> writes:
>
> > 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>
> > ---
< ... >
> > @@ -380,11 +371,8 @@ rte_pci_probe(void)
> > probed++;
> >
> > devargs = dev->device.devargs;
> > - /* probe all or only whitelisted devices */
> > - if (probe_all)
> > - ret = pci_probe_all_drivers(dev);
> > - else if (devargs != NULL &&
> > - devargs->policy == RTE_DEV_WHITELISTED)
> > + /* probe all or only declared devices */
> > + if (probe_all ^ (devargs != NULL))
>
> What is the intent of this? If probe_all is true, and devargs != null,
> I think this branch isn't taken.
>
> Shouldn't this be ||? Maybe I missed something?
>
Here are the possible choices:
probe_all \ devargs !NULL NULL
--------------------------+----------------------+----------------------------+
true |blacklist and the |blacklist mode and no |
|devargs describes a |devargs, meaning the device |
|blacklisted device |is not blacklisted |
|--> do not probe |--> do probe |
--------------------------+----------------------+----------------------------+
false |whitelist mode and |whitelist mode and no |
|whitelisted device |devargs, device has been |
|using a devargs |scanned but not whitelisted |
|--> do probe |--> do not probe |
--------------------------+----------------------+----------------------------+
A XOR is thus the logical expression of this decision.
What I could be doubtful about here is that using a xor anywhere can be
confusing for some people and it could all be expressed in two simpler
ifs.
Also, probe_all could be renamed as "blacklist_mode" for example to be
more descriptive.
But unless I'm mistaken, the logic should be correct.
> > ret = pci_probe_all_drivers(dev);
> > if (ret < 0) {
> > RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMT
< ... >
--
Gaëtan Rivet
6WIND
^ permalink raw reply [flat|nested] 91+ messages in thread
* [dpdk-dev] [PATCH v2 03/18] devargs: introduce iterator
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 01/18] eal: prepend busname on legacy device declaration Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 02/18] eal: remove generic devtype Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 04/18] devargs: introduce foreach macro
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (2 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 03/18] devargs: introduce iterator Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 05/18] vdev: do not reference devargs list
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (3 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 04/18] devargs: introduce foreach macro Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 06/18] bus/pci: do not reference devargs list
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (4 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 05/18] vdev: do not reference devargs list Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 07/18] test: remove devargs unit tests
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (5 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 06/18] bus/pci: " Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 08/18] devargs: make devargs list private
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (6 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 07/18] test: remove devargs unit tests Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 09/18] devargs: make parsing variadic
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (7 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 08/18] devargs: make devargs list private Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 10/18] devargs: require bus name prefix
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (8 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 09/18] devargs: make parsing variadic Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 11/18] devargs: simplify implementation
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (9 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [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 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 11/18] devargs: simplify implementation
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 11/18] devargs: simplify implementation Gaetan Rivet
@ 2017-10-16 11:39 ` Shreyansh Jain
2017-10-16 11:42 ` Shreyansh Jain
0 siblings, 1 reply; 91+ messages in thread
From: Shreyansh Jain @ 2017-10-16 11:39 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev
Hello Gaetan,
On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
> 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;
> }
>
While trying to work on this patch, I noticed that the complete series
(including "Move PCI away from EAL") is not cleanly applicable on
current master (17.11 RC1). I thought it would be some tiny issues.
But there are some issues which I couldn't pass, Like...
> -/* store a whitelist parameter for later parsing */
> int
In the this function
> -rte_eal_devargs_add(const char *devargs_str)
> +rte_eal_devargs_add(const char *dev)
> {
> struct rte_devargs *devargs = NULL;
> - const char *dev = devargs_str;
>
> - /* use calloc instead of rte_zmalloc as it's called early at init */
> devargs = calloc(1, sizeof(*devargs));
> if (devargs == NULL)
> goto fail;
>
> if (rte_eal_devargs_parse(devargs, "%s", dev))
> goto fail;
These lines don't exist in your patch
---
59c2ba6c 172) if (bus->conf.probe_mode == RTE_BUS_PROBE_UNDEFINED) {
b631f3b0 173) if (devargs->policy == RTE_DEV_WHITELISTED)
59c2ba6c 174) bus->conf.probe_mode =
RTE_BUS_PROBE_WHITELIST;
b631f3b0 175) else if (devargs->policy == RTE_DEV_BLACKLISTED)
59c2ba6c 176) bus->conf.probe_mode =
RTE_BUS_PROBE_BLACKLIST;
02823c1d 177) }
bf6dea0e 178) TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
bf6dea0e 179) return 0;
0215a4c6 180)
---
(Some introduced by the move PCI series, but others like b631f3b0 are
very old ~17.08)
> - TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
> + if (rte_eal_devargs_insert(devargs))
> + goto fail;
And hence, I don't know whether you intend to insert the above line
after or before checking PROBE.
> return 0;
>
> fail:
>
Maybe I am doing something wrong here - any ideas? Can you send an
updated/rebased version on current master HEAD?
-
Shreyansh
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 11/18] devargs: simplify implementation
2017-10-16 11:39 ` Shreyansh Jain
@ 2017-10-16 11:42 ` Shreyansh Jain
2017-10-16 13:42 ` Gaëtan Rivet
0 siblings, 1 reply; 91+ messages in thread
From: Shreyansh Jain @ 2017-10-16 11:42 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev
Hello Gaetan,
Please ignore this email (reason inline)
On Monday 16 October 2017 05:09 PM, Shreyansh Jain wrote:
> Hello Gaetan,
>
> On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
>> 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;
>> }
>
> While trying to work on this patch, I noticed that the complete series
> (including "Move PCI away from EAL") is not cleanly applicable on
> current master (17.11 RC1). I thought it would be some tiny issues.
>
> But there are some issues which I couldn't pass, Like...
>
>> -/* store a whitelist parameter for later parsing */
>> int
>
> In the this function
>
>> -rte_eal_devargs_add(const char *devargs_str)
>> +rte_eal_devargs_add(const char *dev)
>> {
>> struct rte_devargs *devargs = NULL;
>> - const char *dev = devargs_str;
>> - /* use calloc instead of rte_zmalloc as it's called early at init */
>> devargs = calloc(1, sizeof(*devargs));
>> if (devargs == NULL)
>> goto fail;
>> if (rte_eal_devargs_parse(devargs, "%s", dev))
>> goto fail;
>
> These lines don't exist in your patch
>
> ---
> 59c2ba6c 172) if (bus->conf.probe_mode == RTE_BUS_PROBE_UNDEFINED) {
> b631f3b0 173) if (devargs->policy == RTE_DEV_WHITELISTED)
> 59c2ba6c 174) bus->conf.probe_mode =
> RTE_BUS_PROBE_WHITELIST;
> b631f3b0 175) else if (devargs->policy == RTE_DEV_BLACKLISTED)
> 59c2ba6c 176) bus->conf.probe_mode =
> RTE_BUS_PROBE_BLACKLIST;
> 02823c1d 177) }
> bf6dea0e 178) TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
> bf6dea0e 179) return 0;
> 0215a4c6 180)
> ---
> (Some introduced by the move PCI series, but others like b631f3b0 are
> very old ~17.08)
>
>
>> - TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
>> + if (rte_eal_devargs_insert(devargs))
>> + goto fail;
>
> And hence, I don't know whether you intend to insert the above line
> after or before checking PROBE.
>
>> return 0;
>> fail:
>>
>
> Maybe I am doing something wrong here - any ideas? Can you send an
> updated/rebased version on current master HEAD?
Just after sending this, I noticed that I had not applied the "Bus
control framework" patch set which the "devargs..." cover letter talks
about.
I will try with that and confirm if there is still any issue.
>
> -
> Shreyansh
>
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 11/18] devargs: simplify implementation
2017-10-16 11:42 ` Shreyansh Jain
@ 2017-10-16 13:42 ` Gaëtan Rivet
2017-10-17 5:58 ` Shreyansh Jain
0 siblings, 1 reply; 91+ messages in thread
From: Gaëtan Rivet @ 2017-10-16 13:42 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: dev
On Mon, Oct 16, 2017 at 05:12:37PM +0530, Shreyansh Jain wrote:
> Hello Gaetan,
>
> Please ignore this email (reason inline)
>
Hello Shreyansh,
Thanks for reading this patchset, and sorry about the confusion (the
previous cover-letter only listed the PCI bus move, the bus control was
only added in this version).
I'd be very happy to see these series integrated for this release or the
next, and as such your opinion would matter a great deal.
However, I have found important issues that I am still working on with
the PCI bus move. I am trying to fix this in time, but I'm not sure yet
to succeed soon enough.
It would block the rest. I could still redo both series without the PCI
bus move but the bus control scheme is necessary for the devargs
cleanup.
I don't know if this bus control scheme is interesting enough though.
I need additional opinions about it.
Anyway, thanks,
--
Gaëtan Rivet
6WIND
^ permalink raw reply [flat|nested] 91+ messages in thread
* Re: [dpdk-dev] [PATCH v2 11/18] devargs: simplify implementation
2017-10-16 13:42 ` Gaëtan Rivet
@ 2017-10-17 5:58 ` Shreyansh Jain
0 siblings, 0 replies; 91+ messages in thread
From: Shreyansh Jain @ 2017-10-17 5:58 UTC (permalink / raw)
To: Gaëtan Rivet; +Cc: dev
Hello Gaetan,
On Monday 16 October 2017 07:12 PM, Gaëtan Rivet wrote:
> On Mon, Oct 16, 2017 at 05:12:37PM +0530, Shreyansh Jain wrote:
>> Hello Gaetan,
>>
>> Please ignore this email (reason inline)
>>
>
> Hello Shreyansh,
>
> Thanks for reading this patchset, and sorry about the confusion (the
> previous cover-letter only listed the PCI bus move, the bus control was
> only added in this version).
That's OK. I mixed up the cover-letters anyway.
Probably the complete series needs a rebase as some patches have changes
exactly same locations in EAL where this patch impacts (at least for PCI
movement patches, I found patches had fuzz factor)
>
> I'd be very happy to see these series integrated for this release or the
> next, and as such your opinion would matter a great deal.
I do understand the importance of this patch series - but I have not
been able to devote enough time to this lately. Even now, I am not sure
if I would be able to completely review all within 17.11 timeframe as it
is quite big. (And I have some personal time-off coming up soon!)
>
> However, I have found important issues that I am still working on with
> the PCI bus move. I am trying to fix this in time, but I'm not sure yet
> to succeed soon enough.
I understand.
It is a complex series - specially the build break in PCI movement on
per patch basis - that would be difficult to solve. Probably that
requires adding dummy functions/variables to allow compilation and then
moving them out. It is indeed tough choice.
>
> It would block the rest. I could still redo both series without the PCI
> bus move but the bus control scheme is necessary for the devargs
> cleanup.
Probably it is best to postpone for 18.02. It indeed is difficult to let
a proposal fall through a planned release - but this work is really
critical as it impacts a lot of people (PCI, Buses, args etc).
Just my personal opinion.
>
> I don't know if this bus control scheme is interesting enough though.
> I need additional opinions about it.
I haven't started reading the Bus control yet - I am still focusing on
PCI movement and then I had planned the devargs. But, I will start
looking into this as well. I would try to add whatever opinion I can.
>
> Anyway, thanks,
>
Always welcome! But, that's what the community is for. :D
-
Shreyansh
^ permalink raw reply [flat|nested] 91+ messages in thread
* [dpdk-dev] [PATCH v2 12/18] eal: add generic device declaration parameter
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (10 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [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 ` [dpdk-dev] [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 [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 12/18] eal: add generic device declaration parameter
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 12/18] eal: add generic device declaration parameter Gaetan Rivet
@ 2017-12-13 14:26 ` Shreyansh Jain
2017-12-13 14:47 ` Gaëtan Rivet
0 siblings, 1 reply; 91+ messages in thread
From: Shreyansh Jain @ 2017-12-13 14:26 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev
On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
> Add a new generic device declaration parameter:
>
> --dev=<device_declaration>
>
[...]
>
> 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;
> + }
Might be a naive question: Any specific reason why we don't add the
devices directly into devargs_list here (eal_parse_args ->
eal_parse_common_option -> OPT_DEV ->) rather than wait for eal to call
eal_option_device_parse again?
Is it to allow eal_plugins_init() to finish?
> + break;
> +
> case OPT_VDEV_NUM:
> if (eal_option_device_add("vdev", optarg) < 0)
> return -1;
[...]
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 12/18] eal: add generic device declaration parameter
2017-12-13 14:26 ` Shreyansh Jain
@ 2017-12-13 14:47 ` Gaëtan Rivet
2017-12-13 15:24 ` Shreyansh Jain
0 siblings, 1 reply; 91+ messages in thread
From: Gaëtan Rivet @ 2017-12-13 14:47 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: dev
On Wed, Dec 13, 2017 at 07:56:42PM +0530, Shreyansh Jain wrote:
> On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
> > Add a new generic device declaration parameter:
> >
> > --dev=<device_declaration>
> >
>
> [...]
>
> >
> > 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;
> > + }
>
> Might be a naive question: Any specific reason why we don't add the devices
> directly into devargs_list here (eal_parse_args -> eal_parse_common_option
> -> OPT_DEV ->) rather than wait for eal to call eal_option_device_parse
> again?
>
> Is it to allow eal_plugins_init() to finish?
>
Yes. And actually this makes me aware of an issue with this
implementation.
Calling rte_eal_devargs_parse here is premature, and
rte_bus_probe_mode_set as well.
eal_plugins_init() must be executed before calling rte_devargs to allow
for buses introduced as plugins to be able to recognize their devices.
I will reorder a few things in eal_options, thanks for catching this.
> > + break;
> > +
> > case OPT_VDEV_NUM:
> > if (eal_option_device_add("vdev", optarg) < 0)
> > return -1;
>
> [...]
>
--
Gaëtan Rivet
6WIND
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 12/18] eal: add generic device declaration parameter
2017-12-13 14:47 ` Gaëtan Rivet
@ 2017-12-13 15:24 ` Shreyansh Jain
0 siblings, 0 replies; 91+ messages in thread
From: Shreyansh Jain @ 2017-12-13 15:24 UTC (permalink / raw)
To: Gaëtan Rivet; +Cc: dev
On Wednesday 13 December 2017 08:17 PM, Gaëtan Rivet wrote:
> On Wed, Dec 13, 2017 at 07:56:42PM +0530, Shreyansh Jain wrote:
>> On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
>>> Add a new generic device declaration parameter:
>>>
>>> --dev=<device_declaration>
>>>
>>
>> [...]
>>
>>>
>>> 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;
>>> + }
>>
>> Might be a naive question: Any specific reason why we don't add the devices
>> directly into devargs_list here (eal_parse_args -> eal_parse_common_option
>> -> OPT_DEV ->) rather than wait for eal to call eal_option_device_parse
>> again?
>>
>> Is it to allow eal_plugins_init() to finish?
>>
>
> Yes. And actually this makes me aware of an issue with this
> implementation.
>
> Calling rte_eal_devargs_parse here is premature, and
> rte_bus_probe_mode_set as well.
>
> eal_plugins_init() must be executed before calling rte_devargs to allow
> for buses introduced as plugins to be able to recognize their devices.
There might be one more catch. Maybe eal_parse_args also finds all the
plugins to load (-d ...).
>
> I will reorder a few things in eal_options, thanks for catching this.
>
>>> + break;
>>> +
>>> case OPT_VDEV_NUM:
>>> if (eal_option_device_add("vdev", optarg) < 0)
>>> return -1;
>>
>> [...]
>>
>
^ permalink raw reply [flat|nested] 91+ messages in thread
* [dpdk-dev] [PATCH v2 13/18] bus: make device recognition function public
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (11 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 12/18] eal: add generic device declaration parameter Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [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/eal/rte_eal_version.map | 1 +
3 files changed, 14 insertions(+)
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 0d693c8..13c8450 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -223,6 +223,7 @@ EXPERIMENTAL {
DPDK_17.11 {
global:
+ rte_bus_find_by_device_name;
rte_bus_get_iommu_class;
rte_eal_iova_mode;
rte_eal_mbuf_default_mempool_ops;
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 1cae96e..6d3fb70 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -364,6 +364,18 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
struct rte_bus *rte_bus_find_by_name(const char *busname);
/**
+ * Find a bus capable of identifying a device.
+ *
+ * @param str
+ * A device identifier (PCI address, virtual PMD name, ...).
+ *
+ * @return
+ * A valid bus handle if found.
+ * NULL if no bus is able to parse this device.
+ */
+struct rte_bus *rte_bus_find_by_device_name(const char *str);
+
+/**
* Helper for Bus registration.
* The constructor has higher priority than PMD constructors.
*/
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 9c0251e..a6fe25c 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -227,6 +227,7 @@ EXPERIMENTAL {
DPDK_17.11 {
global:
+ rte_bus_find_by_device_name;
rte_bus_get_iommu_class;
rte_eal_iova_mode;
rte_eal_mbuf_default_mempool_ops;
--
2.1.4
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 14/18] net/failsafe: keep legacy sub-device declaration
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (12 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 13/18] bus: make device recognition function public Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 15/18] ether: use new devargs parsing function Gaetan Rivet
` (5 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
Allow users to use the same sub-device declaration format.
The rte_devargs library now requires the bus name to be prepended to
device declarations. While it is possible to use this new format, the
transition to the new one can be made smoother.
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
drivers/net/failsafe/failsafe_args.c | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c
index 08ce4ad..0c98264 100644
--- a/drivers/net/failsafe/failsafe_args.c
+++ b/drivers/net/failsafe/failsafe_args.c
@@ -34,6 +34,7 @@
#include <string.h>
#include <errno.h>
+#include <rte_bus.h>
#include <rte_debug.h>
#include <rte_devargs.h>
#include <rte_malloc.h>
@@ -83,12 +84,20 @@ closing_paren(const char *text)
static int
fs_parse_device(struct sub_device *sdev, char *args)
{
+ struct rte_bus *bus;
struct rte_devargs *d;
int ret;
d = &sdev->devargs;
DEBUG("%s", args);
- ret = rte_eal_devargs_parse(d, "%s", args);
+ bus = rte_bus_find_by_device_name(args);
+ if (bus == NULL)
+ /* args may contain the bus name */
+ ret = rte_eal_devargs_parse(d, "%s", args);
+ else
+ /* args is a device name */
+ ret = rte_eal_devargs_parse(d, "%s:%s",
+ bus->name, args);
if (ret) {
DEBUG("devargs parsing failed with code %d", ret);
return ret;
--
2.1.4
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 15/18] ether: use new devargs parsing function
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (13 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 14/18] net/failsafe: keep legacy sub-device declaration Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 16/18] devargs: remove old " Gaetan Rivet
` (4 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 previous one is being deprecated.
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
lib/librte_ether/rte_ethdev.c | 11 +++++++----
1 file changed, 7 insertions(+), 4 deletions(-)
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index bb08204..618f576 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -372,21 +372,24 @@ rte_eth_dev_is_detachable(uint16_t port_id)
/* attach the new device, then store port_id of the device */
int
-rte_eth_dev_attach(const char *devargs, uint16_t *port_id)
+rte_eth_dev_attach(const char *devstr, uint16_t *port_id)
{
int ret = -1;
int current = rte_eth_dev_count();
+ struct rte_devargs devargs;
char *name = NULL;
char *args = NULL;
- if ((devargs == NULL) || (port_id == NULL)) {
+ if ((devstr == NULL) || (port_id == NULL)) {
ret = -EINVAL;
goto err;
}
- /* parse devargs, then retrieve device name and args */
- if (rte_eal_parse_devargs_str(devargs, &name, &args))
+ /* parse device, then retrieve device name and args */
+ if (rte_eal_devargs_parse(&devargs, "%s", devstr))
goto err;
+ name = devargs.name;
+ args = devargs.args;
ret = rte_eal_dev_attach(name, args);
if (ret < 0)
--
2.1.4
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 16/18] devargs: remove old devargs parsing function
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (14 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 15/18] ether: use new devargs parsing function Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 17/18] devargs: use proper prefix Gaetan Rivet
` (3 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 functionality is already covered by rte_eal_devargs_parse.
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 | 30 -------------------------
lib/librte_eal/common/include/rte_devargs.h | 28 -----------------------
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 1 -
4 files changed, 60 deletions(-)
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 13c8450..aae8d32 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -32,7 +32,6 @@ DPDK_2.0 {
rte_eal_lcore_role;
rte_eal_mp_remote_launch;
rte_eal_mp_wait_lcore;
- rte_eal_parse_devargs_str;
rte_eal_process_type;
rte_eal_remote_launch;
rte_eal_tailq_lookup;
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 1d87cd9..82c8573 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -53,36 +53,6 @@ TAILQ_HEAD(rte_devargs_list, rte_devargs);
struct rte_devargs_list devargs_list =
TAILQ_HEAD_INITIALIZER(devargs_list);
-int
-rte_eal_parse_devargs_str(const char *devargs_str,
- char **drvname, char **drvargs)
-{
- char *sep;
-
- if ((devargs_str) == NULL || (drvname) == NULL || (drvargs == NULL))
- return -1;
-
- *drvname = strdup(devargs_str);
- if (*drvname == NULL)
- return -1;
-
- /* set the first ',' to '\0' to split name and arguments */
- sep = strchr(*drvname, ',');
- if (sep != NULL) {
- sep[0] = '\0';
- *drvargs = strdup(sep + 1);
- } else {
- *drvargs = strdup("");
- }
-
- if (*drvargs == NULL) {
- free(*drvname);
- *drvname = NULL;
- return -1;
- }
- return 0;
-}
-
static int
bus_name_cmp(const struct rte_bus *bus, const void *name)
{
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 1fe03d6..499f7e3 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -74,34 +74,6 @@ struct rte_devargs {
};
/**
- * Parse a devargs string.
- *
- * For PCI devices, the format of arguments string is "PCI_ADDR" or
- * "PCI_ADDR,key=val,key2=val2,...". Examples: "08:00.1", "0000:5:00.0",
- * "04:00.0,arg=val".
- *
- * For virtual devices, the format of arguments string is "DRIVER_NAME*"
- * or "DRIVER_NAME*,key=val,key2=val2,...". Examples: "net_ring",
- * "net_ring0", "net_pmdAnything,arg=0:arg2=1".
- *
- * The function parses the arguments string to get driver name and driver
- * arguments.
- *
- * @param devargs_str
- * The arguments as given by the user.
- * @param drvname
- * The pointer to the string to store parsed driver name.
- * @param drvargs
- * The pointer to the string to store parsed driver arguments.
- *
- * @return
- * - 0 on success
- * - A negative value on error
- */
-int rte_eal_parse_devargs_str(const char *devargs_str,
- char **drvname, char **drvargs);
-
-/**
* Parse a device string.
*
* Verify that a bus is capable of handling the device passed
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index a6fe25c..323f799 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -32,7 +32,6 @@ DPDK_2.0 {
rte_eal_lcore_role;
rte_eal_mp_remote_launch;
rte_eal_mp_wait_lcore;
- rte_eal_parse_devargs_str;
rte_eal_process_type;
rte_eal_remote_launch;
rte_eal_tailq_lookup;
--
2.1.4
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 17/18] devargs: use proper prefix
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (15 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 16/18] devargs: remove old " Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices Gaetan Rivet
` (2 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 is redundant.
Make it concise.
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
app/test-pmd/cmdline.c | 2 +-
drivers/net/failsafe/failsafe_args.c | 4 ++--
examples/bond/main.c | 2 +-
lib/librte_eal/bsdapp/eal/rte_eal_version.map | 12 ++++++------
lib/librte_eal/common/eal_common_dev.c | 8 ++++----
lib/librte_eal/common/eal_common_devargs.c | 18 +++++++++---------
lib/librte_eal/common/eal_common_options.c | 4 ++--
lib/librte_eal/common/eal_common_vdev.c | 4 ++--
lib/librte_eal/common/include/rte_devargs.h | 18 +++++++++---------
lib/librte_eal/linuxapp/eal/rte_eal_version.map | 12 ++++++------
lib/librte_ether/rte_ethdev.c | 2 +-
test/test/commands.c | 2 +-
12 files changed, 44 insertions(+), 44 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 1204604..3b338e5 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -8322,7 +8322,7 @@ static void cmd_dump_parsed(void *parsed_result,
else if (!strcmp(res->dump, "dump_mempool"))
rte_mempool_list_dump(stdout);
else if (!strcmp(res->dump, "dump_devargs"))
- rte_eal_devargs_dump(stdout);
+ rte_devargs_dump(stdout);
else if (!strcmp(res->dump, "dump_log_types"))
rte_log_dump(stdout);
}
diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c
index 0c98264..79c90cf 100644
--- a/drivers/net/failsafe/failsafe_args.c
+++ b/drivers/net/failsafe/failsafe_args.c
@@ -93,10 +93,10 @@ fs_parse_device(struct sub_device *sdev, char *args)
bus = rte_bus_find_by_device_name(args);
if (bus == NULL)
/* args may contain the bus name */
- ret = rte_eal_devargs_parse(d, "%s", args);
+ ret = rte_devargs_parse(d, "%s", args);
else
/* args is a device name */
- ret = rte_eal_devargs_parse(d, "%s:%s",
+ ret = rte_devargs_parse(d, "%s:%s",
bus->name, args);
if (ret) {
DEBUG("devargs parsing failed with code %d", ret);
diff --git a/examples/bond/main.c b/examples/bond/main.c
index cb55552..420ba1f 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -744,7 +744,7 @@ main(int argc, char *argv[])
/* init EAL */
ret = rte_eal_init(argc, argv);
- rte_eal_devargs_dump(stdout);
+ rte_devargs_dump(stdout);
if (ret < 0)
rte_exit(EXIT_FAILURE, "Error with EAL initialization\n");
argc -= ret;
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index aae8d32..0913a52 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -19,8 +19,6 @@ DPDK_2.0 {
rte_dump_tailq;
rte_eal_alarm_cancel;
rte_eal_alarm_set;
- rte_eal_devargs_add;
- rte_eal_devargs_dump;
rte_eal_get_configuration;
rte_eal_get_lcore_state;
rte_eal_get_physmem_layout;
@@ -186,10 +184,12 @@ DPDK_17.08 {
EXPERIMENTAL {
global:
- rte_eal_devargs_insert;
- rte_eal_devargs_next;
- rte_eal_devargs_parse;
- rte_eal_devargs_remove;
+ rte_devargs_add;
+ rte_devargs_dump;
+ rte_devargs_insert;
+ rte_devargs_next;
+ rte_devargs_parse;
+ rte_devargs_remove;
rte_eal_hotplug_add;
rte_eal_hotplug_remove;
rte_service_component_register;
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index b965e56..5608690 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -151,12 +151,12 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
if (da == NULL)
return -ENOMEM;
- ret = rte_eal_devargs_parse(da, "%s:%s,%s",
+ ret = rte_devargs_parse(da, "%s:%s,%s",
busname, devname, devargs);
if (ret)
goto err_devarg;
- ret = rte_eal_devargs_insert(da);
+ ret = rte_devargs_insert(da);
if (ret)
goto err_devarg;
@@ -181,7 +181,7 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
return 0;
err_devarg:
- if (rte_eal_devargs_remove(busname, devname)) {
+ if (rte_devargs_remove(busname, devname)) {
free(da->args);
free(da);
}
@@ -216,6 +216,6 @@ int rte_eal_hotplug_remove(const char *busname, const char *devname)
if (ret)
RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
dev->name);
- rte_eal_devargs_remove(busname, devname);
+ rte_devargs_remove(busname, devname);
return ret;
}
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 82c8573..d5e36d2 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -60,7 +60,7 @@ bus_name_cmp(const struct rte_bus *bus, const void *name)
}
int
-rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...)
+rte_devargs_parse(struct rte_devargs *da, const char *format, ...)
{
struct rte_bus *bus = NULL;
va_list ap;
@@ -112,11 +112,11 @@ rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...)
}
int
-rte_eal_devargs_insert(struct rte_devargs *da)
+rte_devargs_insert(struct rte_devargs *da)
{
int ret;
- ret = rte_eal_devargs_remove(da->bus->name, da->name);
+ ret = rte_devargs_remove(da->bus->name, da->name);
if (ret < 0)
return ret;
TAILQ_INSERT_TAIL(&devargs_list, da, next);
@@ -124,7 +124,7 @@ rte_eal_devargs_insert(struct rte_devargs *da)
}
int
-rte_eal_devargs_add(const char *dev)
+rte_devargs_add(const char *dev)
{
struct rte_devargs *devargs = NULL;
@@ -132,9 +132,9 @@ rte_eal_devargs_add(const char *dev)
if (devargs == NULL)
goto fail;
- if (rte_eal_devargs_parse(devargs, "%s", dev))
+ if (rte_devargs_parse(devargs, "%s", dev))
goto fail;
- if (rte_eal_devargs_insert(devargs))
+ if (rte_devargs_insert(devargs))
goto fail;
return 0;
@@ -148,7 +148,7 @@ rte_eal_devargs_add(const char *dev)
}
int
-rte_eal_devargs_remove(const char *busname, const char *devname)
+rte_devargs_remove(const char *busname, const char *devname)
{
struct rte_devargs *d;
void *tmp;
@@ -167,7 +167,7 @@ rte_eal_devargs_remove(const char *busname, const char *devname)
/* dump the user devices on the console */
void
-rte_eal_devargs_dump(FILE *f)
+rte_devargs_dump(FILE *f)
{
struct rte_devargs *devargs;
@@ -181,7 +181,7 @@ rte_eal_devargs_dump(FILE *f)
/* bus-aware rte_devargs iterator. */
struct rte_devargs *
-rte_eal_devargs_next(const char *busname, const struct rte_devargs *start)
+rte_devargs_next(const char *busname, const struct rte_devargs *start)
{
struct rte_devargs *da;
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index b7591fd..a3238fb 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -181,7 +181,7 @@ eal_option_device_parse(void)
TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {
if (ret == 0) {
- ret = rte_eal_devargs_add(devopt->arg);
+ ret = rte_devargs_add(devopt->arg);
if (ret)
RTE_LOG(ERR, EAL, "Unable to parse device '%s'\n",
devopt->arg);
@@ -1125,7 +1125,7 @@ eal_parse_common_option(int opt, const char *optarg,
struct rte_devargs da;
int ret;
- if (rte_eal_devargs_parse(&da, optarg) < 0)
+ if (rte_devargs_parse(&da, optarg) < 0)
return -1;
ret = rte_bus_probe_mode_set(da.bus->name,
RTE_BUS_PROBE_WHITELIST);
diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index a7410a6..583648d 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;
}
- rte_eal_devargs_insert(devargs);
+ rte_devargs_insert(devargs);
TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
return 0;
@@ -242,7 +242,7 @@ rte_vdev_uninit(const char *name)
TAILQ_REMOVE(&vdev_device_list, dev, next);
- rte_eal_devargs_remove(devargs->bus->name, devargs->name);
+ rte_devargs_remove(devargs->bus->name, devargs->name);
free(dev);
return 0;
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 499f7e3..e46686d 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -92,8 +92,8 @@ struct rte_devargs {
* - Negative errno on error.
*/
int
-rte_eal_devargs_parse(struct rte_devargs *da,
- const char *format, ...);
+rte_devargs_parse(struct rte_devargs *da,
+ const char *format, ...);
/**
* Insert an rte_devargs in the global list.
@@ -106,7 +106,7 @@ rte_eal_devargs_parse(struct rte_devargs *da,
* - Negative on error.
*/
int
-rte_eal_devargs_insert(struct rte_devargs *da);
+rte_devargs_insert(struct rte_devargs *da);
/**
* Add a device to the user device list
@@ -127,7 +127,7 @@ rte_eal_devargs_insert(struct rte_devargs *da);
* - 0 on success
* - A negative value on error
*/
-int rte_eal_devargs_add(const char *devargs_str);
+int rte_devargs_add(const char *devargs_str);
/**
* Remove a device from the user device list.
@@ -145,7 +145,7 @@ int rte_eal_devargs_add(const char *devargs_str);
* <0 on error.
* >0 if the devargs was not within the user device list.
*/
-int rte_eal_devargs_remove(const char *busname, const char *devname);
+int rte_devargs_remove(const char *busname, const char *devname);
/**
* This function dumps the list of user device and their arguments.
@@ -153,7 +153,7 @@ int rte_eal_devargs_remove(const char *busname, const char *devname);
* @param f
* A pointer to a file for output
*/
-void rte_eal_devargs_dump(FILE *f);
+void rte_devargs_dump(FILE *f);
/**
* Find next rte_devargs matching the provided bus name.
@@ -171,15 +171,15 @@ void rte_eal_devargs_dump(FILE *f);
* NULL if there is none.
*/
struct rte_devargs *
-rte_eal_devargs_next(const char *busname, const struct rte_devargs *start);
+rte_devargs_next(const char *busname, const struct rte_devargs *start);
/**
* Iterate over all rte_devargs for a specific bus.
*/
#define RTE_EAL_DEVARGS_FOREACH(busname, da) \
- for (da = rte_eal_devargs_next(busname, NULL); \
+ for (da = rte_devargs_next(busname, NULL); \
da != NULL; \
- da = rte_eal_devargs_next(busname, da)) \
+ da = rte_devargs_next(busname, da)) \
#ifdef __cplusplus
}
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 323f799..87b9a3b 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -19,8 +19,6 @@ DPDK_2.0 {
rte_dump_tailq;
rte_eal_alarm_cancel;
rte_eal_alarm_set;
- rte_eal_devargs_add;
- rte_eal_devargs_dump;
rte_eal_get_configuration;
rte_eal_get_lcore_state;
rte_eal_get_physmem_layout;
@@ -190,10 +188,12 @@ DPDK_17.08 {
EXPERIMENTAL {
global:
- rte_eal_devargs_insert;
- rte_eal_devargs_next;
- rte_eal_devargs_parse;
- rte_eal_devargs_remove;
+ rte_devargs_add;
+ rte_devargs_dump;
+ rte_devargs_insert;
+ rte_devargs_next;
+ rte_devargs_parse;
+ rte_devargs_remove;
rte_eal_hotplug_add;
rte_eal_hotplug_remove;
rte_service_component_register;
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 618f576..2f3f721 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -386,7 +386,7 @@ rte_eth_dev_attach(const char *devstr, uint16_t *port_id)
}
/* parse device, then retrieve device name and args */
- if (rte_eal_devargs_parse(&devargs, "%s", devstr))
+ if (rte_devargs_parse(&devargs, "%s", devstr))
goto err;
name = devargs.name;
args = devargs.args;
diff --git a/test/test/commands.c b/test/test/commands.c
index 4097a33..1670f75 100644
--- a/test/test/commands.c
+++ b/test/test/commands.c
@@ -157,7 +157,7 @@ static void cmd_dump_parsed(void *parsed_result,
else if (!strcmp(res->dump, "dump_mempool"))
rte_mempool_list_dump(stdout);
else if (!strcmp(res->dump, "dump_devargs"))
- rte_eal_devargs_dump(stdout);
+ rte_devargs_dump(stdout);
else if (!strcmp(res->dump, "dump_log_types"))
rte_log_dump(stdout);
}
--
2.1.4
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (16 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 17/18] devargs: use proper prefix Gaetan Rivet
@ 2017-10-12 8:21 ` Gaetan Rivet
2017-10-12 11:28 ` Mcnamara, John
2017-12-13 10:17 ` Shreyansh Jain
2017-10-17 18:18 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Aaron Conole
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
19 siblings, 2 replies; 91+ messages in thread
From: Gaetan Rivet @ 2017-10-12 8:21 UTC (permalink / raw)
To: dev; +Cc: Gaetan Rivet
These actions have been enacted.
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
doc/guides/rel_notes/deprecation.rst | 13 -------------
1 file changed, 13 deletions(-)
diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
index ef2264f..23faa19 100644
--- a/doc/guides/rel_notes/deprecation.rst
+++ b/doc/guides/rel_notes/deprecation.rst
@@ -16,19 +16,6 @@ Deprecation Notices
- ``rte_set_log_type``, replaced by ``rte_log_set_level``
- ``rte_get_log_type``, replaced by ``rte_log_get_level``
-* eal: several API and ABI changes are planned for ``rte_devargs`` in v17.11.
- The format of device command line parameters will change. The bus will need
- to be explicitly stated in the device declaration. The enum ``rte_devtype``
- was used to identify a bus and will disappear.
- The structure ``rte_devargs`` will change.
- The ``rte_devargs_list`` will be made private.
- The following functions are deprecated starting from 17.08 and will either be
- modified or removed in 17.11:
-
- - ``rte_eal_devargs_add``
- - ``rte_eal_devargs_type_count``
- - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse``
-
* eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA address
translation scheme.
Reference to phys address in EAL data-structure or functions may change to
--
2.1.4
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices Gaetan Rivet
@ 2017-10-12 11:28 ` Mcnamara, John
2017-12-13 10:17 ` Shreyansh Jain
1 sibling, 0 replies; 91+ messages in thread
From: Mcnamara, John @ 2017-10-12 11:28 UTC (permalink / raw)
To: Gaetan Rivet, dev
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Gaetan Rivet
> Sent: Thursday, October 12, 2017 9:21 AM
> To: dev@dpdk.org
> Cc: Gaetan Rivet <gaetan.rivet@6wind.com>
> Subject: [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation
> notices
>
> These actions have been enacted.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Acked-by: John McNamara <john.mcnamara@intel.com>
^ permalink raw reply [flat|nested] 91+ messages in thread
* Re: [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices Gaetan Rivet
2017-10-12 11:28 ` Mcnamara, John
@ 2017-12-13 10:17 ` Shreyansh Jain
2017-12-13 10:25 ` Gaëtan Rivet
1 sibling, 1 reply; 91+ messages in thread
From: Shreyansh Jain @ 2017-12-13 10:17 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev
Hello Gaetan,
On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
> These actions have been enacted.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
> doc/guides/rel_notes/deprecation.rst | 13 -------------
> 1 file changed, 13 deletions(-)
>
> diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> index ef2264f..23faa19 100644
> --- a/doc/guides/rel_notes/deprecation.rst
> +++ b/doc/guides/rel_notes/deprecation.rst
> @@ -16,19 +16,6 @@ Deprecation Notices
> - ``rte_set_log_type``, replaced by ``rte_log_set_level``
> - ``rte_get_log_type``, replaced by ``rte_log_get_level``
>
> -* eal: several API and ABI changes are planned for ``rte_devargs`` in v17.11.
> - The format of device command line parameters will change. The bus will need
> - to be explicitly stated in the device declaration. The enum ``rte_devtype``
> - was used to identify a bus and will disappear.
> - The structure ``rte_devargs`` will change.
> - The ``rte_devargs_list`` will be made private.
> - The following functions are deprecated starting from 17.08 and will either be
> - modified or removed in 17.11:
> -
> - - ``rte_eal_devargs_add``
> - - ``rte_eal_devargs_type_count``
> - - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse``
> -
> * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA address
> translation scheme.
> Reference to phys address in EAL data-structure or functions may change to
>
Once this patch is formalized, the documentation reference for
rte_devargs.h also needs to be changed as it still refers to RTE devargs as:
"...These devices can be PCI devices or virtual devices....".
Similarly, the rte_devargs_parse too has PCI traces.
Next step would be to remove the "pci"/"vdev" reference from
rte_eal_dev_attach.
Former can be part of this series, but the later needs to be a separate
patch, I think. Let me know if you want me to work on these (or later).
Other than that, I think I am OK with overall patch. If you can push the
final series (I am not sure it would be with or without bus control), I
can give it a spin (to vaildate if non-PCI like FSLMC bus can work fine).
^ permalink raw reply [flat|nested] 91+ messages in thread
* Re: [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices
2017-12-13 10:17 ` Shreyansh Jain
@ 2017-12-13 10:25 ` Gaëtan Rivet
2017-12-13 10:54 ` Shreyansh Jain
0 siblings, 1 reply; 91+ messages in thread
From: Gaëtan Rivet @ 2017-12-13 10:25 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: dev
Hello Shreyansh,
On Wed, Dec 13, 2017 at 03:47:04PM +0530, Shreyansh Jain wrote:
> Hello Gaetan,
>
> On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
> > These actions have been enacted.
> >
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> > doc/guides/rel_notes/deprecation.rst | 13 -------------
> > 1 file changed, 13 deletions(-)
> >
> > diff --git a/doc/guides/rel_notes/deprecation.rst b/doc/guides/rel_notes/deprecation.rst
> > index ef2264f..23faa19 100644
> > --- a/doc/guides/rel_notes/deprecation.rst
> > +++ b/doc/guides/rel_notes/deprecation.rst
> > @@ -16,19 +16,6 @@ Deprecation Notices
> > - ``rte_set_log_type``, replaced by ``rte_log_set_level``
> > - ``rte_get_log_type``, replaced by ``rte_log_get_level``
> > -* eal: several API and ABI changes are planned for ``rte_devargs`` in v17.11.
> > - The format of device command line parameters will change. The bus will need
> > - to be explicitly stated in the device declaration. The enum ``rte_devtype``
> > - was used to identify a bus and will disappear.
> > - The structure ``rte_devargs`` will change.
> > - The ``rte_devargs_list`` will be made private.
> > - The following functions are deprecated starting from 17.08 and will either be
> > - modified or removed in 17.11:
> > -
> > - - ``rte_eal_devargs_add``
> > - - ``rte_eal_devargs_type_count``
> > - - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse``
> > -
> > * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA address
> > translation scheme.
> > Reference to phys address in EAL data-structure or functions may change to
> >
>
> Once this patch is formalized, the documentation reference for rte_devargs.h
> also needs to be changed as it still refers to RTE devargs as:
>
> "...These devices can be PCI devices or virtual devices....".
>
> Similarly, the rte_devargs_parse too has PCI traces.
>
> Next step would be to remove the "pci"/"vdev" reference from
> rte_eal_dev_attach.
>
Noted, thanks.
> Former can be part of this series, but the later needs to be a separate
> patch, I think. Let me know if you want me to work on these (or later).
>
> Other than that, I think I am OK with overall patch. If you can push the
> final series (I am not sure it would be with or without bus control), I can
> give it a spin (to vaildate if non-PCI like FSLMC bus can work fine).
Indeed, I also think everything should be settled first.
I have mostly finished working on this series yesterday,
I will integrate your above remarks which will be short.
(Well, by finished I mean I finished the first 90%. The other 90% is
still in progress...)
I removed the rte_devargs unit test and am not too happy about it. There
are parsing functions there, which are extremely error-prone and would
like to have at least the basis for some tests, that we could populate
as we go. If I have the courage I will try to write it and send it with
this series.
I would certainly appreciate if you are able to fix the pci / vdev
limitation in rte_eal_dev_attach, as I am starting to be overwhelmed
with work (trying to finish a lot of things before the holidays).
Thanks for the review!
--
Gaëtan Rivet
6WIND
^ permalink raw reply [flat|nested] 91+ messages in thread
* Re: [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices
2017-12-13 10:25 ` Gaëtan Rivet
@ 2017-12-13 10:54 ` Shreyansh Jain
2017-12-22 4:59 ` Shreyansh Jain
0 siblings, 1 reply; 91+ messages in thread
From: Shreyansh Jain @ 2017-12-13 10:54 UTC (permalink / raw)
To: Gaëtan Rivet; +Cc: dev
Hello Gaetan,
> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Wednesday, December 13, 2017 3:56 PM
> To: Shreyansh Jain <shreyansh.jain@nxp.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
>
> Hello Shreyansh,
>
> On Wed, Dec 13, 2017 at 03:47:04PM +0530, Shreyansh Jain wrote:
> > Hello Gaetan,
> >
> > On Thursday 12 October 2017 01:51 PM, Gaetan Rivet wrote:
> > > These actions have been enacted.
> > >
> > > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > ---
> > > doc/guides/rel_notes/deprecation.rst | 13 -------------
> > > 1 file changed, 13 deletions(-)
> > >
> > > diff --git a/doc/guides/rel_notes/deprecation.rst
> b/doc/guides/rel_notes/deprecation.rst
> > > index ef2264f..23faa19 100644
> > > --- a/doc/guides/rel_notes/deprecation.rst
> > > +++ b/doc/guides/rel_notes/deprecation.rst
> > > @@ -16,19 +16,6 @@ Deprecation Notices
> > > - ``rte_set_log_type``, replaced by ``rte_log_set_level``
> > > - ``rte_get_log_type``, replaced by ``rte_log_get_level``
> > > -* eal: several API and ABI changes are planned for ``rte_devargs`` in
> v17.11.
> > > - The format of device command line parameters will change. The bus will
> need
> > > - to be explicitly stated in the device declaration. The enum
> ``rte_devtype``
> > > - was used to identify a bus and will disappear.
> > > - The structure ``rte_devargs`` will change.
> > > - The ``rte_devargs_list`` will be made private.
> > > - The following functions are deprecated starting from 17.08 and will
> either be
> > > - modified or removed in 17.11:
> > > -
> > > - - ``rte_eal_devargs_add``
> > > - - ``rte_eal_devargs_type_count``
> > > - - ``rte_eal_parse_devargs_str``, replaced by ``rte_eal_devargs_parse``
> > > -
> > > * eal: An ABI change is planned for 17.11 to make DPDK aware of IOVA
> address
> > > translation scheme.
> > > Reference to phys address in EAL data-structure or functions may
> change to
> > >
> >
> > Once this patch is formalized, the documentation reference for
> rte_devargs.h
> > also needs to be changed as it still refers to RTE devargs as:
> >
> > "...These devices can be PCI devices or virtual devices....".
> >
> > Similarly, the rte_devargs_parse too has PCI traces.
> >
> > Next step would be to remove the "pci"/"vdev" reference from
> > rte_eal_dev_attach.
> >
>
> Noted, thanks.
>
> > Former can be part of this series, but the later needs to be a separate
> > patch, I think. Let me know if you want me to work on these (or later).
> >
> > Other than that, I think I am OK with overall patch. If you can push the
> > final series (I am not sure it would be with or without bus control), I can
> > give it a spin (to vaildate if non-PCI like FSLMC bus can work fine).
>
> Indeed, I also think everything should be settled first.
> I have mostly finished working on this series yesterday,
> I will integrate your above remarks which will be short.
>
> (Well, by finished I mean I finished the first 90%. The other 90% is
> still in progress...)
>
> I removed the rte_devargs unit test and am not too happy about it. There
> are parsing functions there, which are extremely error-prone and would
> like to have at least the basis for some tests, that we could populate
> as we go. If I have the courage I will try to write it and send it with
> this series.
While reading through the code, I also had the same feeling - there can be corner cases in the parsing functions which I can't imagine. Anyways, those need to be runtime-verified - static reviews may not suffice.
>
> I would certainly appreciate if you are able to fix the pci / vdev
> limitation in rte_eal_dev_attach, as I am starting to be overwhelmed
> with work (trying to finish a lot of things before the holidays).
OK.
Once you give the devargs a push, I will start work on the PCI removal from rte_eal_dev_attach. Before that, I just want to be sure of devargs with non-PCI bus (non hotplug case).
And, thanks for tons of work you are handling. I saw the patches and really appreciate how you have split things up in sequential manner per-patch. It is difficult.
>
> Thanks for the review!
>
> --
> Gaëtan Rivet
> 6WIND
^ permalink raw reply [flat|nested] 91+ messages in thread
* Re: [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices
2017-12-13 10:54 ` Shreyansh Jain
@ 2017-12-22 4:59 ` Shreyansh Jain
2017-12-22 8:33 ` Gaëtan Rivet
0 siblings, 1 reply; 91+ messages in thread
From: Shreyansh Jain @ 2017-12-22 4:59 UTC (permalink / raw)
To: Gaëtan Rivet; +Cc: dev
Hello Gaetan,
On Wednesday 13 December 2017 04:24 PM, Shreyansh Jain wrote:
> Hello Gaetan,
>
>> -----Original Message-----
>> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
>> Sent: Wednesday, December 13, 2017 3:56 PM
>> To: Shreyansh Jain <shreyansh.jain@nxp.com>
>> Cc: dev@dpdk.org
>> Subject: Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
>>
[...]
>
> While reading through the code, I also had the same feeling - there can be corner cases in the parsing functions which I can't imagine. Anyways, those need to be runtime-verified - static reviews may not suffice.
>
>>
>> I would certainly appreciate if you are able to fix the pci / vdev
>> limitation in rte_eal_dev_attach, as I am starting to be overwhelmed
>> with work (trying to finish a lot of things before the holidays).
>
> OK.
> Once you give the devargs a push, I will start work on the PCI removal from rte_eal_dev_attach. Before that, I just want to be sure of devargs with non-PCI bus (non hotplug case).
>
> And, thanks for tons of work you are handling. I saw the patches and really appreciate how you have split things up in sequential manner per-patch. It is difficult.
Have you pushed the new version of the devargs patches?
Just wanted to check in case I have missed it.
-
Shreyansh
^ permalink raw reply [flat|nested] 91+ messages in thread
* Re: [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices
2017-12-22 4:59 ` Shreyansh Jain
@ 2017-12-22 8:33 ` Gaëtan Rivet
0 siblings, 0 replies; 91+ messages in thread
From: Gaëtan Rivet @ 2017-12-22 8:33 UTC (permalink / raw)
To: Shreyansh Jain; +Cc: dev
Hi Shreyansh,
On Fri, Dec 22, 2017 at 10:29:14AM +0530, Shreyansh Jain wrote:
> Hello Gaetan,
>
> On Wednesday 13 December 2017 04:24 PM, Shreyansh Jain wrote:
> > Hello Gaetan,
> >
> > > -----Original Message-----
> > > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > > Sent: Wednesday, December 13, 2017 3:56 PM
> > > To: Shreyansh Jain <shreyansh.jain@nxp.com>
> > > Cc: dev@dpdk.org
> > > Subject: Re: [PATCH v2 18/18] doc: remove devargs deprecation notices
> > >
>
> [...]
>
> > While reading through the code, I also had the same feeling - there can be corner cases in the parsing functions which I can't imagine. Anyways, those need to be runtime-verified - static reviews may not suffice.
> >
> > >
> > > I would certainly appreciate if you are able to fix the pci / vdev
> > > limitation in rte_eal_dev_attach, as I am starting to be overwhelmed
> > > with work (trying to finish a lot of things before the holidays).
> > OK.
> > Once you give the devargs a push, I will start work on the PCI removal from rte_eal_dev_attach. Before that, I just want to be sure of devargs with non-PCI bus (non hotplug case).
> >
> > And, thanks for tons of work you are handling. I saw the patches and really appreciate how you have split things up in sequential manner per-patch. It is difficult.
>
> Have you pushed the new version of the devargs patches?
> Just wanted to check in case I have missed it.
>
No sorry,
I am removing the dependency on the bus control framework from the
devargs patchset.
I finished the patchset otherwise (redid the unit test), but after some
thinking I found that the bus control was maybe not ideal.
I will send the devargs patchset soon, once it is made independent, and
will expose the issue with the bus control.
Best,
--
Gaëtan Rivet
6WIND
^ permalink raw reply [flat|nested] 91+ messages in thread
* Re: [dpdk-dev] [PATCH v2 00/18] devargs cleanup
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (17 preceding siblings ...)
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 18/18] doc: remove devargs deprecation notices Gaetan Rivet
@ 2017-10-17 18:18 ` Aaron Conole
2017-10-18 8:36 ` Gaëtan Rivet
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
19 siblings, 1 reply; 91+ messages in thread
From: Aaron Conole @ 2017-10-17 18:18 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev
Gaetan Rivet <gaetan.rivet@6wind.com> writes:
> 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:
It is weird to me that you introduce patch sets, and then introduce
cleanups later? Shouldn't we revise the existing patchsets?
Maybe I missed a discussion somewhere?
> 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
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v2 00/18] devargs cleanup
2017-10-17 18:18 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Aaron Conole
@ 2017-10-18 8:36 ` Gaëtan Rivet
0 siblings, 0 replies; 91+ messages in thread
From: Gaëtan Rivet @ 2017-10-18 8:36 UTC (permalink / raw)
To: Aaron Conole; +Cc: dev
On Tue, Oct 17, 2017 at 02:18:02PM -0400, Aaron Conole wrote:
> Gaetan Rivet <gaetan.rivet@6wind.com> writes:
>
> > 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:
>
> It is weird to me that you introduce patch sets, and then introduce
> cleanups later? Shouldn't we revise the existing patchsets?
>
> Maybe I missed a discussion somewhere?
>
No discussion missed :)
The way it happened here was that I started cleaning up the mess I did
last release in the rte_devargs to streamline somewhat the subsystem.
Several things needed to disappear:
1. List of devargs manipulated by buses. With hotplug feature they had
to be properly managed (i.e. freed on device unplug).
2. rte_devtype, which was meant to disappear last release but was kept
to smooth the transition to the new API.
3. Different usages of devargs_add among applications / PMDs
There had been some different last release about the exposed API,
thus I made a compromise that could potentially benefit everyone.
Points 1 and 3 did not require additional work on the buses.
Point 2 however, required a way for buses to be configured in whitelist
/ blacklist mode. To clean up the rte_devtype, a configuration facility
is necessary.
While working on this facility, I found that I was once again breaking
the ABI of rte_bus, along with the IOVA configuration. I thought it
could be streamlined in a more stable way, thus I wrote the separate
patchset for a cleaner integration of these changes in this release.
All in all, the PCI move is not necessary but was sent a few weeks ago
already and it could be skipped with a small rework.
The bus control could be done in a more hackish way as well, without
having a whole separate patchset to introduce the control facility. I
wanted to propose a cleaner approach that could possibly keep the
rte_bus ABI stable for some time.
> > Move PCI away from the EAL
> > http://dpdk.org/ml/archives/dev/2017-August/073512.html
> >
> > Bus control framework
> > http://dpdk.org/ml/archives/dev/2017-October/078752.html
> >
> > Gaetan Rivet (18):
> > eal: prepend busname on legacy device declaration
> > eal: remove generic devtype
> > devargs: introduce iterator
> > devargs: introduce foreach macro
> > vdev: do not reference devargs list
> > bus/pci: do not reference devargs list
> > test: remove devargs unit tests
> > devargs: make devargs list private
> > devargs: make parsing variadic
> > devargs: require bus name prefix
> > devargs: simplify implementation
> > eal: add generic device declaration parameter
> > bus: make device recognition function public
> > net/failsafe: keep legacy sub-device declaration
> > ether: use new devargs parsing function
> > devargs: remove old devargs parsing function
> > devargs: use proper prefix
> > doc: remove devargs deprecation notices
> >
> > MAINTAINERS | 1 -
> > app/test-pmd/cmdline.c | 2 +-
> > doc/guides/rel_notes/deprecation.rst | 13 ---
> > drivers/bus/pci/pci_common.c | 22 +---
> > drivers/net/failsafe/failsafe_args.c | 11 +-
> > examples/bond/main.c | 2 +-
> > lib/librte_eal/bsdapp/eal/rte_eal_version.map | 15 ++-
> > lib/librte_eal/common/eal_common_dev.c | 39 ++-----
> > lib/librte_eal/common/eal_common_devargs.c | 129 +++++++++--------------
> > lib/librte_eal/common/eal_common_options.c | 47 ++++++---
> > lib/librte_eal/common/eal_common_vdev.c | 11 +-
> > lib/librte_eal/common/eal_options.h | 2 +
> > lib/librte_eal/common/include/rte_bus.h | 12 +++
> > lib/librte_eal/common/include/rte_dev.h | 8 --
> > lib/librte_eal/common/include/rte_devargs.h | 120 ++++++++--------------
> > lib/librte_eal/linuxapp/eal/rte_eal_version.map | 15 ++-
> > lib/librte_ether/rte_ethdev.c | 11 +-
> > test/test/Makefile | 1 -
> > test/test/commands.c | 2 +-
> > test/test/test_devargs.c | 131 ------------------------
> > 20 files changed, 186 insertions(+), 408 deletions(-)
> > delete mode 100644 test/test/test_devargs.c
--
Gaëtan Rivet
6WIND
^ permalink raw reply [flat|nested] 91+ messages in thread
* [dpdk-dev] [PATCH v3 00/10] devargs cleanup
2017-10-12 8:21 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Gaetan Rivet
` (18 preceding siblings ...)
2017-10-17 18:18 ` [dpdk-dev] [PATCH v2 00/18] devargs cleanup Aaron Conole
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 01/10] devargs: introduce iterator Gaetan Rivet
` (10 more replies)
19 siblings, 11 replies; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 UTC (permalink / raw)
To: dev; +Cc: Gaetan Rivet
Cleanup the rte_devargs API.
This is a continuous process.
The previous version of this patchset was dependent on changes
on the PCI bus and the rte_bus library. Not all these changes were
integrated. This patchset has thus been limited to elements that are
available right now, without dependencies.
The devargs list is made private. All devargs users are meant to use them
using the proper API. Devargs will allocate dynamic memory. With device hotplug,
this memory is usually mishandled and lost by libraries not following the evolutions
of rte_devargs.
Gaetan Rivet (10):
devargs: introduce iterator
devargs: introduce foreach macro
bus/vdev: do not reference devargs list
bus/pci: do not reference devargs list
net/vdev_netvsc: do not reference devargs list
test: remove devargs unit tests
devargs: make devargs list private
devargs: make parsing variadic
devargs: use proper namespace prefix
devargs: rename legacy API
MAINTAINERS | 1 -
app/test-pmd/cmdline.c | 2 +-
drivers/bus/pci/Makefile | 1 +
drivers/bus/pci/pci_common.c | 6 +-
drivers/bus/vdev/Makefile | 1 +
drivers/bus/vdev/vdev.c | 13 +--
drivers/net/failsafe/failsafe_args.c | 2 +-
drivers/net/failsafe/failsafe_eal.c | 2 +-
drivers/net/vdev_netvsc/vdev_netvsc.c | 4 +-
examples/bond/Makefile | 1 +
examples/bond/main.c | 2 +-
lib/librte_eal/common/eal_common_dev.c | 39 ++-------
lib/librte_eal/common/eal_common_devargs.c | 53 ++++++++++--
lib/librte_eal/common/eal_common_options.c | 2 +-
lib/librte_eal/common/include/rte_devargs.h | 127 +++++++++++++++++++++++-----
lib/librte_eal/rte_eal_version.map | 11 ++-
lib/librte_ether/rte_ethdev.c | 44 +++++-----
test/test/Makefile | 1 -
test/test/commands.c | 2 +-
test/test/test_devargs.c | 103 ----------------------
20 files changed, 198 insertions(+), 219 deletions(-)
delete mode 100644 test/test/test_devargs.c
--
2.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v3 01/10] devargs: introduce iterator
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-03-21 5:43 ` Tan, Jianfeng
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 02/10] devargs: introduce foreach macro Gaetan Rivet
` (9 subsequent siblings)
10 siblings, 1 reply; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 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/common/eal_common_devargs.c | 20 ++++++++++++++++++++
lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++
lib/librte_eal/rte_eal_version.map | 1 +
3 files changed, 41 insertions(+)
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 810b3e18f..c6c5eabcf 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f)
devargs->name, devargs->args);
}
}
+
+/* bus-aware rte_devargs iterator. */
+__rte_experimental
+struct rte_devargs *
+rte_eal_devargs_next(const char *busname, const struct rte_devargs *start)
+{
+ struct rte_devargs *da;
+
+ if (start != NULL)
+ da = TAILQ_NEXT(start, next);
+ else
+ da = TAILQ_FIRST(&devargs_list);
+ while (da != NULL) {
+ if (busname == NULL ||
+ (strcmp(busname, da->bus->name) == 0))
+ return da;
+ da = TAILQ_NEXT(da, next);
+ }
+ return NULL;
+}
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 84e5e23c4..969a10449 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -189,6 +189,26 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
*/
void rte_eal_devargs_dump(FILE *f);
+/**
+ * Find next rte_devargs matching the provided bus name.
+ *
+ * @param busname
+ * Limit the iteration to devargs related to buses
+ * matching this name.
+ * Will return any next rte_devargs if NULL.
+ *
+ * @param start
+ * Starting iteration point. The iteration will start at
+ * the first rte_devargs if NULL.
+ *
+ * @return
+ * Next rte_devargs entry matching the requested bus,
+ * NULL if there is none.
+ */
+__rte_experimental
+struct rte_devargs *
+rte_eal_devargs_next(const char *busname, const struct rte_devargs *start);
+
#ifdef __cplusplus
}
#endif
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index d12360235..02a040a8b 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -216,6 +216,7 @@ EXPERIMENTAL {
rte_eal_cleanup;
rte_eal_devargs_insert;
+ rte_eal_devargs_next;
rte_eal_devargs_parse;
rte_eal_devargs_remove;
rte_eal_hotplug_add;
--
2.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v3 01/10] devargs: introduce iterator
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 01/10] devargs: introduce iterator Gaetan Rivet
@ 2018-03-21 5:43 ` Tan, Jianfeng
2018-03-21 8:50 ` Gaëtan Rivet
0 siblings, 1 reply; 91+ messages in thread
From: Tan, Jianfeng @ 2018-03-21 5:43 UTC (permalink / raw)
To: Gaetan Rivet, dev
Hi Gaetan,
On 3/21/2018 7:20 AM, Gaetan Rivet wrote:
> In preparation to making devargs_list private.
>
> Bus drivers generally need to access rte_devargs pertaining to their
> operations. This match is a common operation for bus drivers.
>
> Add a new accessor for the rte_devargs list.
>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
> lib/librte_eal/common/eal_common_devargs.c | 20 ++++++++++++++++++++
> lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++
> lib/librte_eal/rte_eal_version.map | 1 +
> 3 files changed, 41 insertions(+)
>
> diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> index 810b3e18f..c6c5eabcf 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f)
> devargs->name, devargs->args);
> }
> }
> +
> +/* bus-aware rte_devargs iterator. */
> +__rte_experimental
> +struct rte_devargs *
> +rte_eal_devargs_next(const char *busname, const struct rte_devargs *start)
> +{
> + struct rte_devargs *da;
> +
> + if (start != NULL)
> + da = TAILQ_NEXT(start, next);
> + else
> + da = TAILQ_FIRST(&devargs_list);
> + while (da != NULL) {
> + if (busname == NULL ||
> + (strcmp(busname, da->bus->name) == 0))
> + return da;
> + da = TAILQ_NEXT(da, next);
> + }
> + return NULL;
> +}
> diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> index 84e5e23c4..969a10449 100644
> --- a/lib/librte_eal/common/include/rte_devargs.h
> +++ b/lib/librte_eal/common/include/rte_devargs.h
> @@ -189,6 +189,26 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
> */
> void rte_eal_devargs_dump(FILE *f);
>
> +/**
> + * Find next rte_devargs matching the provided bus name.
> + *
> + * @param busname
> + * Limit the iteration to devargs related to buses
> + * matching this name.
> + * Will return any next rte_devargs if NULL.
> + *
> + * @param start
> + * Starting iteration point. The iteration will start at
> + * the first rte_devargs if NULL.
> + *
> + * @return
> + * Next rte_devargs entry matching the requested bus,
> + * NULL if there is none.
> + */
> +__rte_experimental
> +struct rte_devargs *
> +rte_eal_devargs_next(const char *busname, const struct rte_devargs *start);
> +
> #ifdef __cplusplus
> }
> #endif
> diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> index d12360235..02a040a8b 100644
> --- a/lib/librte_eal/rte_eal_version.map
> +++ b/lib/librte_eal/rte_eal_version.map
> @@ -216,6 +216,7 @@ EXPERIMENTAL {
>
> rte_eal_cleanup;
> rte_eal_devargs_insert;
> + rte_eal_devargs_next;
> rte_eal_devargs_parse;
> rte_eal_devargs_remove;
> rte_eal_hotplug_add;
Shall we change these APIs to be thread-safe?
Thanks,
Jianfeng
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v3 01/10] devargs: introduce iterator
2018-03-21 5:43 ` Tan, Jianfeng
@ 2018-03-21 8:50 ` Gaëtan Rivet
0 siblings, 0 replies; 91+ messages in thread
From: Gaëtan Rivet @ 2018-03-21 8:50 UTC (permalink / raw)
To: Tan, Jianfeng; +Cc: dev
Hello Jianfeng,
On Wed, Mar 21, 2018 at 01:43:49PM +0800, Tan, Jianfeng wrote:
> Hi Gaetan,
>
>
> On 3/21/2018 7:20 AM, Gaetan Rivet wrote:
> > In preparation to making devargs_list private.
> >
> > Bus drivers generally need to access rte_devargs pertaining to their
> > operations. This match is a common operation for bus drivers.
> >
> > Add a new accessor for the rte_devargs list.
> >
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> > lib/librte_eal/common/eal_common_devargs.c | 20 ++++++++++++++++++++
> > lib/librte_eal/common/include/rte_devargs.h | 20 ++++++++++++++++++++
> > lib/librte_eal/rte_eal_version.map | 1 +
> > 3 files changed, 41 insertions(+)
> >
> > diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
> > index 810b3e18f..c6c5eabcf 100644
> > --- a/lib/librte_eal/common/eal_common_devargs.c
> > +++ b/lib/librte_eal/common/eal_common_devargs.c
> > @@ -207,3 +207,23 @@ rte_eal_devargs_dump(FILE *f)
> > devargs->name, devargs->args);
> > }
> > }
> > +
> > +/* bus-aware rte_devargs iterator. */
> > +__rte_experimental
> > +struct rte_devargs *
> > +rte_eal_devargs_next(const char *busname, const struct rte_devargs *start)
> > +{
> > + struct rte_devargs *da;
> > +
> > + if (start != NULL)
> > + da = TAILQ_NEXT(start, next);
> > + else
> > + da = TAILQ_FIRST(&devargs_list);
> > + while (da != NULL) {
> > + if (busname == NULL ||
> > + (strcmp(busname, da->bus->name) == 0))
> > + return da;
> > + da = TAILQ_NEXT(da, next);
> > + }
> > + return NULL;
> > +}
> > diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
> > index 84e5e23c4..969a10449 100644
> > --- a/lib/librte_eal/common/include/rte_devargs.h
> > +++ b/lib/librte_eal/common/include/rte_devargs.h
> > @@ -189,6 +189,26 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
> > */
> > void rte_eal_devargs_dump(FILE *f);
> > +/**
> > + * Find next rte_devargs matching the provided bus name.
> > + *
> > + * @param busname
> > + * Limit the iteration to devargs related to buses
> > + * matching this name.
> > + * Will return any next rte_devargs if NULL.
> > + *
> > + * @param start
> > + * Starting iteration point. The iteration will start at
> > + * the first rte_devargs if NULL.
> > + *
> > + * @return
> > + * Next rte_devargs entry matching the requested bus,
> > + * NULL if there is none.
> > + */
> > +__rte_experimental
> > +struct rte_devargs *
> > +rte_eal_devargs_next(const char *busname, const struct rte_devargs *start);
> > +
> > #ifdef __cplusplus
> > }
> > #endif
> > diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
> > index d12360235..02a040a8b 100644
> > --- a/lib/librte_eal/rte_eal_version.map
> > +++ b/lib/librte_eal/rte_eal_version.map
> > @@ -216,6 +216,7 @@ EXPERIMENTAL {
> > rte_eal_cleanup;
> > rte_eal_devargs_insert;
> > + rte_eal_devargs_next;
> > rte_eal_devargs_parse;
> > rte_eal_devargs_remove;
> > rte_eal_hotplug_add;
>
> Shall we change these APIs to be thread-safe?
It was not planned, but I think you raise an interesting point.
At the moment, most devargs operations are happening when a single
thread is started (eal init).
However, hotplug means having insertion and deletion possibly happening
in additional threads. This API exists and could help making these
operations thread-safe.
So this is low-priority I think for now (unless shown otherwise), but
definitely something to look into.
--
Gaëtan Rivet
6WIND
^ permalink raw reply [flat|nested] 91+ messages in thread
* [dpdk-dev] [PATCH v3 02/10] devargs: introduce foreach macro
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 01/10] devargs: introduce iterator Gaetan Rivet
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-04-22 21:42 ` Thomas Monjalon
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 03/10] bus/vdev: do not reference devargs list Gaetan Rivet
` (8 subsequent siblings)
10 siblings, 1 reply; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 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 969a10449..aba31481b 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -209,6 +209,14 @@ __rte_experimental
struct rte_devargs *
rte_eal_devargs_next(const char *busname, const struct rte_devargs *start);
+/**
+ * Iterate over all rte_devargs for a specific bus.
+ */
+#define RTE_EAL_DEVARGS_FOREACH(busname, da) \
+ for (da = rte_eal_devargs_next(busname, NULL); \
+ da != NULL; \
+ da = rte_eal_devargs_next(busname, da)) \
+
#ifdef __cplusplus
}
#endif
--
2.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v3 03/10] bus/vdev: do not reference devargs list
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 01/10] devargs: introduce iterator Gaetan Rivet
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 02/10] devargs: introduce foreach macro Gaetan Rivet
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-03-21 5:41 ` Tan, Jianfeng
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 04/10] bus/pci: " Gaetan Rivet
` (7 subsequent siblings)
10 siblings, 1 reply; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 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>
---
drivers/bus/vdev/Makefile | 1 +
drivers/bus/vdev/vdev.c | 11 +++--------
2 files changed, 4 insertions(+), 8 deletions(-)
diff --git a/drivers/bus/vdev/Makefile b/drivers/bus/vdev/Makefile
index 24d424a38..bd0bb8955 100644
--- a/drivers/bus/vdev/Makefile
+++ b/drivers/bus/vdev/Makefile
@@ -10,6 +10,7 @@ LIB = librte_bus_vdev.a
CFLAGS += -O3
CFLAGS += $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
# versioning export map
EXPORT_MAP := rte_bus_vdev_version.map
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index e4bc72463..8ee264baf 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -256,7 +256,7 @@ rte_vdev_init(const char *name, const char *args)
goto fail;
}
- TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
+ rte_eal_devargs_insert(devargs);
TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
return 0;
@@ -306,10 +306,8 @@ rte_vdev_uninit(const char *name)
TAILQ_REMOVE(&vdev_device_list, dev, next);
- TAILQ_REMOVE(&devargs_list, devargs, next);
+ rte_eal_devargs_remove(devargs->bus->name, devargs->name);
- free(devargs->args);
- free(devargs);
free(dev);
return 0;
}
@@ -337,10 +335,7 @@ vdev_scan(void)
rte_spinlock_unlock(&vdev_custom_scan_lock);
/* for virtual devices we scan the devargs_list populated via cmdline */
- TAILQ_FOREACH(devargs, &devargs_list, next) {
-
- if (devargs->bus != &rte_vdev_bus)
- continue;
+ RTE_EAL_DEVARGS_FOREACH("vdev", devargs) {
dev = find_vdev(devargs->name);
if (dev)
--
2.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v3 03/10] bus/vdev: do not reference devargs list
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 03/10] bus/vdev: do not reference devargs list Gaetan Rivet
@ 2018-03-21 5:41 ` Tan, Jianfeng
0 siblings, 0 replies; 91+ messages in thread
From: Tan, Jianfeng @ 2018-03-21 5:41 UTC (permalink / raw)
To: Gaetan Rivet, dev
On 3/21/2018 7:20 AM, Gaetan Rivet wrote:
> 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>
Acked-by: Jianfeng Tan <jianfeng.tan@intel.com>
Thanks,
Jianfeng
> ---
> drivers/bus/vdev/Makefile | 1 +
> drivers/bus/vdev/vdev.c | 11 +++--------
> 2 files changed, 4 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/bus/vdev/Makefile b/drivers/bus/vdev/Makefile
> index 24d424a38..bd0bb8955 100644
> --- a/drivers/bus/vdev/Makefile
> +++ b/drivers/bus/vdev/Makefile
> @@ -10,6 +10,7 @@ LIB = librte_bus_vdev.a
>
> CFLAGS += -O3
> CFLAGS += $(WERROR_FLAGS)
> +CFLAGS += -DALLOW_EXPERIMENTAL_API
>
> # versioning export map
> EXPORT_MAP := rte_bus_vdev_version.map
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
> index e4bc72463..8ee264baf 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -256,7 +256,7 @@ rte_vdev_init(const char *name, const char *args)
> goto fail;
> }
>
> - TAILQ_INSERT_TAIL(&devargs_list, devargs, next);
> + rte_eal_devargs_insert(devargs);
>
> TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
> return 0;
> @@ -306,10 +306,8 @@ rte_vdev_uninit(const char *name)
>
> TAILQ_REMOVE(&vdev_device_list, dev, next);
>
> - TAILQ_REMOVE(&devargs_list, devargs, next);
> + rte_eal_devargs_remove(devargs->bus->name, devargs->name);
>
> - free(devargs->args);
> - free(devargs);
> free(dev);
> return 0;
> }
> @@ -337,10 +335,7 @@ vdev_scan(void)
> rte_spinlock_unlock(&vdev_custom_scan_lock);
>
> /* for virtual devices we scan the devargs_list populated via cmdline */
> - TAILQ_FOREACH(devargs, &devargs_list, next) {
> -
> - if (devargs->bus != &rte_vdev_bus)
> - continue;
> + RTE_EAL_DEVARGS_FOREACH("vdev", devargs) {
>
> dev = find_vdev(devargs->name);
> if (dev)
^ permalink raw reply [flat|nested] 91+ messages in thread
* [dpdk-dev] [PATCH v3 04/10] bus/pci: do not reference devargs list
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
` (2 preceding siblings ...)
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 03/10] bus/vdev: do not reference devargs list Gaetan Rivet
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 05/10] net/vdev_netvsc: " Gaetan Rivet
` (6 subsequent siblings)
10 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 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/Makefile | 1 +
drivers/bus/pci/pci_common.c | 6 +-----
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/drivers/bus/pci/Makefile b/drivers/bus/pci/Makefile
index f3df1c4ce..73796ec0e 100644
--- a/drivers/bus/pci/Makefile
+++ b/drivers/bus/pci/Makefile
@@ -37,6 +37,7 @@ EXPORT_MAP := rte_bus_pci_version.map
CFLAGS := -I$(SRCDIR) $(CFLAGS)
CFLAGS += -O3 $(WERROR_FLAGS)
+CFLAGS += -DALLOW_EXPERIMENTAL_API
ifneq ($(CONFIG_RTE_EXEC_ENV_LINUXAPP),)
SYSTEM := linux
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 2a00f365a..6bed0bc9d 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -45,12 +45,8 @@ static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev)
{
struct rte_devargs *devargs;
struct rte_pci_addr addr;
- struct rte_bus *pbus;
- pbus = rte_bus_find_by_name("pci");
- TAILQ_FOREACH(devargs, &devargs_list, next) {
- if (devargs->bus != pbus)
- continue;
+ RTE_EAL_DEVARGS_FOREACH("pci", devargs) {
devargs->bus->parse(devargs->name, &addr);
if (!rte_pci_addr_cmp(&dev->addr, &addr))
return devargs;
--
2.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v3 05/10] net/vdev_netvsc: do not reference devargs list
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
` (3 preceding siblings ...)
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 04/10] bus/pci: " Gaetan Rivet
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 06/10] test: remove devargs unit tests Gaetan Rivet
` (5 subsequent siblings)
10 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 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/net/vdev_netvsc/vdev_netvsc.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c b/drivers/net/vdev_netvsc/vdev_netvsc.c
index cbf4d590a..b0bbce750 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -733,7 +733,7 @@ vdev_netvsc_scan_callback(__rte_unused void *arg)
struct rte_devargs *devargs;
struct rte_bus *vbus = rte_bus_find_by_name("vdev");
- TAILQ_FOREACH(devargs, &devargs_list, next)
+ RTE_EAL_DEVARGS_FOREACH("vdev", devargs)
if (!strcmp(devargs->name, VDEV_NETVSC_DRIVER_NAME))
return;
dev = (struct rte_vdev_device *)vbus->find_device(NULL,
--
2.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v3 06/10] test: remove devargs unit tests
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
` (4 preceding siblings ...)
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 05/10] net/vdev_netvsc: " Gaetan Rivet
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 07/10] devargs: make devargs list private Gaetan Rivet
` (4 subsequent siblings)
10 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 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 | 103 -----------------------------------------------
3 files changed, 105 deletions(-)
delete mode 100644 test/test/test_devargs.c
diff --git a/MAINTAINERS b/MAINTAINERS
index a646ca3e1..c07931d13 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -113,7 +113,6 @@ F: test/test/test_common.c
F: test/test/test_cpuflags.c
F: test/test/test_cycles.c
F: test/test/test_debug.c
-F: test/test/test_devargs.c
F: test/test/test_eal*
F: test/test/test_errno.c
F: test/test/test_interrupts.c
diff --git a/test/test/Makefile b/test/test/Makefile
index a88cc38bf..035b073b7 100644
--- a/test/test/Makefile
+++ b/test/test/Makefile
@@ -161,7 +161,6 @@ SRCS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += test_distributor_perf.c
SRCS-$(CONFIG_RTE_LIBRTE_REORDER) += test_reorder.c
-SRCS-y += test_devargs.c
SRCS-y += virtual_pmd.c
SRCS-y += packet_burst_generator.c
SRCS-$(CONFIG_RTE_LIBRTE_ACL) += test_acl.c
diff --git a/test/test/test_devargs.c b/test/test/test_devargs.c
deleted file mode 100644
index b8f3146f6..000000000
--- a/test/test/test_devargs.c
+++ /dev/null
@@ -1,103 +0,0 @@
-/* SPDX-License-Identifier: BSD-3-Clause
- * Copyright 2014 6WIND S.A.
- */
-
-#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.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v3 07/10] devargs: make devargs list private
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
` (5 preceding siblings ...)
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 06/10] test: remove devargs unit tests Gaetan Rivet
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-04-22 21:46 ` Thomas Monjalon
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 08/10] devargs: make parsing variadic Gaetan Rivet
` (3 subsequent siblings)
10 siblings, 1 reply; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 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/common/eal_common_devargs.c | 3 +++
lib/librte_eal/common/include/rte_devargs.h | 6 ------
lib/librte_eal/rte_eal_version.map | 1 -
3 files changed, 3 insertions(+), 7 deletions(-)
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index c6c5eabcf..a7f374cfa 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -18,6 +18,9 @@
#include <rte_tailq.h>
#include "eal_private.h"
+/** user device double-linked queue type definition */
+TAILQ_HEAD(rte_devargs_list, rte_devargs);
+
/** Global list of user devices */
struct rte_devargs_list devargs_list =
TAILQ_HEAD_INITIALIZER(devargs_list);
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index aba31481b..e48cc3cf2 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -59,12 +59,6 @@ struct rte_devargs {
char *args;
};
-/** user device double-linked queue type definition */
-TAILQ_HEAD(rte_devargs_list, rte_devargs);
-
-/** Global list of user devices */
-extern struct rte_devargs_list devargs_list;
-
/**
* Parse a devargs string.
*
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 02a040a8b..8dab46ba6 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_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.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v3 07/10] devargs: make devargs list private
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 07/10] devargs: make devargs list private Gaetan Rivet
@ 2018-04-22 21:46 ` Thomas Monjalon
0 siblings, 0 replies; 91+ messages in thread
From: Thomas Monjalon @ 2018-04-22 21:46 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev
21/03/2018 00:20, 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/common/eal_common_devargs.c | 3 +++
> lib/librte_eal/common/include/rte_devargs.h | 6 ------
> lib/librte_eal/rte_eal_version.map | 1 -
> 3 files changed, 3 insertions(+), 7 deletions(-)
You must remove the related line in deprecation notice.
^ permalink raw reply [flat|nested] 91+ messages in thread
* [dpdk-dev] [PATCH v3 08/10] devargs: make parsing variadic
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
` (6 preceding siblings ...)
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 07/10] devargs: make devargs list private Gaetan Rivet
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-04-22 21:52 ` Thomas Monjalon
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 09/10] devargs: use proper namespace prefix Gaetan Rivet
` (2 subsequent siblings)
10 siblings, 1 reply; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 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 +-
drivers/net/failsafe/failsafe_eal.c | 2 +-
lib/librte_eal/common/eal_common_dev.c | 33 ++++-------------------------
lib/librte_eal/common/eal_common_devargs.c | 15 ++++++++++---
lib/librte_eal/common/include/rte_devargs.h | 26 ++++++++++++-----------
5 files changed, 32 insertions(+), 46 deletions(-)
diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c
index 366dbea16..f303668bb 100644
--- a/drivers/net/failsafe/failsafe_args.c
+++ b/drivers/net/failsafe/failsafe_args.c
@@ -62,7 +62,7 @@ fs_parse_device(struct sub_device *sdev, char *args)
d = &sdev->devargs;
DEBUG("%s", args);
- ret = rte_eal_devargs_parse(args, d);
+ ret = rte_eal_devargs_parse(d, "%s", args);
if (ret) {
DEBUG("devargs parsing failed with code %d", ret);
return ret;
diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index c3d673125..5adaabfeb 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -69,7 +69,7 @@ fs_bus_init(struct rte_eth_dev *dev)
else
snprintf(devstr, sizeof(devstr), "%s",
rte_eth_devices[pid].device->name);
- ret = rte_eal_devargs_parse(devstr, da);
+ ret = rte_eal_devargs_parse(da, "%s", devstr);
if (ret) {
ERROR("Probed devargs parsing failed with code"
" %d", ret);
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index cd071442f..43e1fb767 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -89,29 +89,12 @@ int rte_eal_dev_detach(struct rte_device *dev)
return ret;
}
-static char *
-full_dev_name(const char *bus, const char *dev, const char *args)
-{
- char *name;
- size_t len;
-
- len = snprintf(NULL, 0, "%s:%s,%s", bus, dev, args) + 1;
- name = calloc(1, len);
- if (name == NULL) {
- RTE_LOG(ERR, EAL, "Could not allocate full device name\n");
- return NULL;
- }
- snprintf(name, len, "%s:%s,%s", bus, dev, args);
- return name;
-}
-
int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
const char *devargs)
{
struct rte_bus *bus;
struct rte_device *dev;
struct rte_devargs *da;
- char *name;
int ret;
bus = rte_bus_find_by_name(busname);
@@ -126,17 +109,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
return -ENOTSUP;
}
- name = full_dev_name(busname, devname, devargs);
- if (name == NULL)
+ da = calloc(1, sizeof(*da));
+ if (da == NULL)
return -ENOMEM;
- da = calloc(1, sizeof(*da));
- if (da == NULL) {
- ret = -ENOMEM;
- goto err_name;
- }
-
- ret = rte_eal_devargs_parse(name, da);
+ ret = rte_eal_devargs_parse(da, "%s:%s,%s",
+ busname, devname, devargs);
if (ret)
goto err_devarg;
@@ -162,7 +140,6 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
dev->name);
goto err_devarg;
}
- free(name);
return 0;
err_devarg:
@@ -170,8 +147,6 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
free(da->args);
free(da);
}
-err_name:
- free(name);
return ret;
}
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index a7f374cfa..b251bb0a6 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -11,6 +11,7 @@
#include <stdio.h>
#include <string.h>
+#include <stdarg.h>
#include <rte_compat.h>
#include <rte_dev.h>
@@ -62,15 +63,23 @@ bus_name_cmp(const struct rte_bus *bus, const void *name)
}
int __rte_experimental
-rte_eal_devargs_parse(const char *dev, struct rte_devargs *da)
+rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...)
{
struct rte_bus *bus = NULL;
+ va_list ap;
+ va_start(ap, format);
+ char dev[vsnprintf(NULL, 0, format, ap) + 1];
const char *devname;
const size_t maxlen = sizeof(da->name);
size_t i;
- if (dev == NULL || da == NULL)
+ va_end(ap);
+ if (da == NULL)
return -EINVAL;
+
+ va_start(ap, format);
+ vsnprintf(dev, sizeof(dev), format, ap);
+ va_end(ap);
/* Retrieve eventual bus info */
do {
devname = dev;
@@ -140,7 +149,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
if (devargs == NULL)
goto fail;
- if (rte_eal_devargs_parse(dev, devargs))
+ if (rte_eal_devargs_parse(devargs, "%s", dev))
goto fail;
devargs->type = devtype;
bus = devargs->bus;
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index e48cc3cf2..e261f7c69 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -94,18 +94,21 @@ int rte_eal_parse_devargs_str(const char *devargs_str,
* in argument. Store which bus will handle the device, its name
* and the eventual device parameters.
*
- * @param dev
- * The device declaration string.
+ * The device string is built with a printf-like syntax.
+ *
* @param da
* The devargs structure holding the device information.
+ * @param format
+ * Format string describing a device.
*
* @return
* - 0 on success.
* - Negative errno on error.
*/
int __rte_experimental
-rte_eal_devargs_parse(const char *dev,
- struct rte_devargs *da);
+rte_eal_devargs_parse(struct rte_devargs *da,
+ const char *format, ...)
+__attribute__((format(printf, 2, 0)));
/**
* Insert an rte_devargs in the global list.
@@ -123,15 +126,14 @@ rte_eal_devargs_insert(struct rte_devargs *da);
/**
* Add a device to the user device list
*
- * For PCI devices, the format of arguments string is "PCI_ADDR" or
- * "PCI_ADDR,key=val,key2=val2,...". Examples: "08:00.1", "0000:5:00.0",
- * "04:00.0,arg=val".
+ * The format is
*
- * For virtual devices, the format of arguments string is "DRIVER_NAME*"
- * or "DRIVER_NAME*,key=val,key2=val2,...". Examples: "net_ring",
- * "net_ring0", "net_pmdAnything,arg=0:arg2=1". The validity of the
- * driver name is not checked by this function, it is done when probing
- * the drivers.
+ * bus:device_identifier,arg1=val1,arg2=val2
+ *
+ * Examples:
+ *
+ * pci:0000:05.00.0,arg=val
+ * vdev:net_ring0
*
* @param devtype
* The type of the device.
--
2.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v3 08/10] devargs: make parsing variadic
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 08/10] devargs: make parsing variadic Gaetan Rivet
@ 2018-04-22 21:52 ` Thomas Monjalon
0 siblings, 0 replies; 91+ messages in thread
From: Thomas Monjalon @ 2018-04-22 21:52 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev
21/03/2018 00:20, Gaetan Rivet:
> /**
> * 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
I think this part should not be in the patch.
Or I miss something?
^ permalink raw reply [flat|nested] 91+ messages in thread
* [dpdk-dev] [PATCH v3 09/10] devargs: use proper namespace prefix
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
` (7 preceding siblings ...)
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 08/10] devargs: make parsing variadic Gaetan Rivet
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 10/10] devargs: rename legacy API Gaetan Rivet
2018-04-22 22:00 ` [dpdk-dev] [PATCH v3 00/10] devargs cleanup Thomas Monjalon
10 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 UTC (permalink / raw)
To: dev; +Cc: Gaetan Rivet
rte_eal_devargs is useless, rte_devargs is sufficient.
Only experimental functions are changed for now.
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
drivers/bus/vdev/vdev.c | 6 +++---
drivers/net/failsafe/failsafe_args.c | 2 +-
drivers/net/failsafe/failsafe_eal.c | 2 +-
lib/librte_eal/common/eal_common_dev.c | 8 ++++----
lib/librte_eal/common/eal_common_devargs.c | 12 ++++++------
lib/librte_eal/common/include/rte_devargs.h | 23 +++++++++++++----------
lib/librte_eal/rte_eal_version.map | 8 ++++----
7 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 8ee264baf..4fcbd9c79 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -256,7 +256,7 @@ rte_vdev_init(const char *name, const char *args)
goto fail;
}
- rte_eal_devargs_insert(devargs);
+ rte_devargs_insert(devargs);
TAILQ_INSERT_TAIL(&vdev_device_list, dev, next);
return 0;
@@ -306,7 +306,7 @@ rte_vdev_uninit(const char *name)
TAILQ_REMOVE(&vdev_device_list, dev, next);
- rte_eal_devargs_remove(devargs->bus->name, devargs->name);
+ rte_devargs_remove(devargs->bus->name, devargs->name);
free(dev);
return 0;
@@ -325,7 +325,7 @@ vdev_scan(void)
if (custom_scan->callback != NULL)
/*
* the callback should update devargs list
- * by calling rte_eal_devargs_insert() with
+ * by calling rte_devargs_insert() with
* devargs.bus = rte_bus_find_by_name("vdev");
* devargs.type = RTE_DEVTYPE_VIRTUAL;
* devargs.policy = RTE_DEV_WHITELISTED;
diff --git a/drivers/net/failsafe/failsafe_args.c b/drivers/net/failsafe/failsafe_args.c
index f303668bb..05773556c 100644
--- a/drivers/net/failsafe/failsafe_args.c
+++ b/drivers/net/failsafe/failsafe_args.c
@@ -62,7 +62,7 @@ fs_parse_device(struct sub_device *sdev, char *args)
d = &sdev->devargs;
DEBUG("%s", args);
- ret = rte_eal_devargs_parse(d, "%s", args);
+ ret = rte_devargs_parse(d, "%s", args);
if (ret) {
DEBUG("devargs parsing failed with code %d", ret);
return ret;
diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index 5adaabfeb..f382f5709 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -69,7 +69,7 @@ fs_bus_init(struct rte_eth_dev *dev)
else
snprintf(devstr, sizeof(devstr), "%s",
rte_eth_devices[pid].device->name);
- ret = rte_eal_devargs_parse(da, "%s", devstr);
+ ret = rte_devargs_parse(da, "%s", devstr);
if (ret) {
ERROR("Probed devargs parsing failed with code"
" %d", ret);
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 43e1fb767..07e3e1647 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -113,12 +113,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
if (da == NULL)
return -ENOMEM;
- ret = rte_eal_devargs_parse(da, "%s:%s,%s",
+ ret = rte_devargs_parse(da, "%s:%s,%s",
busname, devname, devargs);
if (ret)
goto err_devarg;
- ret = rte_eal_devargs_insert(da);
+ ret = rte_devargs_insert(da);
if (ret)
goto err_devarg;
@@ -143,7 +143,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
return 0;
err_devarg:
- if (rte_eal_devargs_remove(busname, devname)) {
+ if (rte_devargs_remove(busname, devname)) {
free(da->args);
free(da);
}
@@ -179,6 +179,6 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
if (ret)
RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
dev->name);
- rte_eal_devargs_remove(busname, devname);
+ rte_devargs_remove(busname, devname);
return ret;
}
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index b251bb0a6..24d0f6f44 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -63,7 +63,7 @@ bus_name_cmp(const struct rte_bus *bus, const void *name)
}
int __rte_experimental
-rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...)
+rte_devargs_parse(struct rte_devargs *da, const char *format, ...)
{
struct rte_bus *bus = NULL;
va_list ap;
@@ -125,11 +125,11 @@ rte_eal_devargs_parse(struct rte_devargs *da, const char *format, ...)
}
int __rte_experimental
-rte_eal_devargs_insert(struct rte_devargs *da)
+rte_devargs_insert(struct rte_devargs *da)
{
int ret;
- ret = rte_eal_devargs_remove(da->bus->name, da->name);
+ ret = rte_devargs_remove(da->bus->name, da->name);
if (ret < 0)
return ret;
TAILQ_INSERT_TAIL(&devargs_list, da, next);
@@ -149,7 +149,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
if (devargs == NULL)
goto fail;
- if (rte_eal_devargs_parse(devargs, "%s", dev))
+ if (rte_devargs_parse(devargs, "%s", dev))
goto fail;
devargs->type = devtype;
bus = devargs->bus;
@@ -174,7 +174,7 @@ rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
}
int __rte_experimental
-rte_eal_devargs_remove(const char *busname, const char *devname)
+rte_devargs_remove(const char *busname, const char *devname)
{
struct rte_devargs *d;
void *tmp;
@@ -223,7 +223,7 @@ rte_eal_devargs_dump(FILE *f)
/* bus-aware rte_devargs iterator. */
__rte_experimental
struct rte_devargs *
-rte_eal_devargs_next(const char *busname, const struct rte_devargs *start)
+rte_devargs_next(const char *busname, const struct rte_devargs *start)
{
struct rte_devargs *da;
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index e261f7c69..07a4701b2 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -105,9 +105,10 @@ int rte_eal_parse_devargs_str(const char *devargs_str,
* - 0 on success.
* - Negative errno on error.
*/
-int __rte_experimental
-rte_eal_devargs_parse(struct rte_devargs *da,
- const char *format, ...)
+__rte_experimental
+int
+rte_devargs_parse(struct rte_devargs *da,
+ const char *format, ...)
__attribute__((format(printf, 2, 0)));
/**
@@ -120,8 +121,9 @@ __attribute__((format(printf, 2, 0)));
* - 0 on success
* - Negative on error.
*/
-int __rte_experimental
-rte_eal_devargs_insert(struct rte_devargs *da);
+__rte_experimental
+int
+rte_devargs_insert(struct rte_devargs *da);
/**
* Add a device to the user device list
@@ -162,8 +164,9 @@ int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str);
* <0 on error.
* >0 if the devargs was not within the user device list.
*/
-int __rte_experimental rte_eal_devargs_remove(const char *busname,
- const char *devname);
+__rte_experimental
+int rte_devargs_remove(const char *busname,
+ const char *devname);
/**
* Count the number of user devices of a specified type
@@ -203,15 +206,15 @@ void rte_eal_devargs_dump(FILE *f);
*/
__rte_experimental
struct rte_devargs *
-rte_eal_devargs_next(const char *busname, const struct rte_devargs *start);
+rte_devargs_next(const char *busname, const struct rte_devargs *start);
/**
* Iterate over all rte_devargs for a specific bus.
*/
#define RTE_EAL_DEVARGS_FOREACH(busname, da) \
- for (da = rte_eal_devargs_next(busname, NULL); \
+ for (da = rte_devargs_next(busname, NULL); \
da != NULL; \
- da = rte_eal_devargs_next(busname, da)) \
+ da = rte_devargs_next(busname, da)) \
#ifdef __cplusplus
}
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 8dab46ba6..1e614f088 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -214,10 +214,10 @@ EXPERIMENTAL {
global:
rte_eal_cleanup;
- rte_eal_devargs_insert;
- rte_eal_devargs_next;
- rte_eal_devargs_parse;
- rte_eal_devargs_remove;
+ rte_devargs_insert;
+ rte_devargs_next;
+ rte_devargs_parse;
+ rte_devargs_remove;
rte_eal_hotplug_add;
rte_eal_hotplug_remove;
rte_eal_mbuf_user_pool_ops;
--
2.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* [dpdk-dev] [PATCH v3 10/10] devargs: rename legacy API
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
` (8 preceding siblings ...)
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 09/10] devargs: use proper namespace prefix Gaetan Rivet
@ 2018-03-20 23:20 ` Gaetan Rivet
2018-04-22 22:00 ` [dpdk-dev] [PATCH v3 00/10] devargs cleanup Thomas Monjalon
10 siblings, 0 replies; 91+ messages in thread
From: Gaetan Rivet @ 2018-03-20 23:20 UTC (permalink / raw)
To: dev; +Cc: Gaetan Rivet
This previous symbols were deprecated for two releases.
They are now marked as such and cannot be used anymore.
They are replaced by ones respecting the new namespace that are marked
experimental.
As a result, eth_dev attach and detach is slightly reworked to follow
the changes.
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
app/test-pmd/cmdline.c | 2 +-
drivers/net/vdev_netvsc/vdev_netvsc.c | 2 +-
examples/bond/Makefile | 1 +
examples/bond/main.c | 2 +-
lib/librte_eal/common/eal_common_devargs.c | 9 +++--
lib/librte_eal/common/eal_common_options.c | 2 +-
lib/librte_eal/common/include/rte_devargs.h | 54 +++++++++++++++++++++++++++++
lib/librte_eal/rte_eal_version.map | 3 ++
lib/librte_ether/rte_ethdev.c | 44 +++++++++++------------
test/test/commands.c | 2 +-
10 files changed, 89 insertions(+), 32 deletions(-)
diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index 40b31ad7e..6e6b4e2e2 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -8621,7 +8621,7 @@ static void cmd_dump_parsed(void *parsed_result,
else if (!strcmp(res->dump, "dump_mempool"))
rte_mempool_list_dump(stdout);
else if (!strcmp(res->dump, "dump_devargs"))
- rte_eal_devargs_dump(stdout);
+ rte_devargs_dump(stdout);
else if (!strcmp(res->dump, "dump_log_types"))
rte_log_dump(stdout);
}
diff --git a/drivers/net/vdev_netvsc/vdev_netvsc.c b/drivers/net/vdev_netvsc/vdev_netvsc.c
index b0bbce750..f4e88689f 100644
--- a/drivers/net/vdev_netvsc/vdev_netvsc.c
+++ b/drivers/net/vdev_netvsc/vdev_netvsc.c
@@ -740,7 +740,7 @@ vdev_netvsc_scan_callback(__rte_unused void *arg)
vdev_netvsc_cmp_rte_device, VDEV_NETVSC_DRIVER_NAME);
if (dev)
return;
- if (rte_eal_devargs_add(RTE_DEVTYPE_VIRTUAL, VDEV_NETVSC_DRIVER_NAME))
+ if (rte_devargs_add(RTE_DEVTYPE_VIRTUAL, VDEV_NETVSC_DRIVER_NAME))
DRV_LOG(ERR, "unable to add netvsc devargs.");
}
diff --git a/examples/bond/Makefile b/examples/bond/Makefile
index 44d10d4f5..ce3f907de 100644
--- a/examples/bond/Makefile
+++ b/examples/bond/Makefile
@@ -59,6 +59,7 @@ CFLAGS_main.o += -Wno-return-type
endif
CFLAGS += -O3
+CFLAGS += -DALLOW_EXPERIMENTAL_API
ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),y)
LDLIBS += -lrte_pmd_bond
diff --git a/examples/bond/main.c b/examples/bond/main.c
index 455f108ee..ede1c591e 100644
--- a/examples/bond/main.c
+++ b/examples/bond/main.c
@@ -742,7 +742,7 @@ main(int argc, char *argv[])
/* init EAL */
ret = rte_eal_init(argc, argv);
- rte_eal_devargs_dump(stdout);
+ rte_devargs_dump(stdout);
if (ret < 0)
rte_exit(EXIT_FAILURE, "Error with EAL initialization\n");
argc -= ret;
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 24d0f6f44..b0434158b 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -137,8 +137,9 @@ rte_devargs_insert(struct rte_devargs *da)
}
/* store a whitelist parameter for later parsing */
+__rte_experimental
int
-rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str)
+rte_devargs_add(enum rte_devtype devtype, const char *devargs_str)
{
struct rte_devargs *devargs = NULL;
struct rte_bus *bus = NULL;
@@ -192,8 +193,9 @@ rte_devargs_remove(const char *busname, const char *devname)
}
/* count the number of devices of a specified type */
+__rte_experimental
unsigned int
-rte_eal_devargs_type_count(enum rte_devtype devtype)
+rte_devargs_type_count(enum rte_devtype devtype)
{
struct rte_devargs *devargs;
unsigned int count = 0;
@@ -207,8 +209,9 @@ rte_eal_devargs_type_count(enum rte_devtype devtype)
}
/* dump the user devices on the console */
+__rte_experimental
void
-rte_eal_devargs_dump(FILE *f)
+rte_devargs_dump(FILE *f)
{
struct rte_devargs *devargs;
diff --git a/lib/librte_eal/common/eal_common_options.c b/lib/librte_eal/common/eal_common_options.c
index 9f2f8d25a..63df21224 100644
--- a/lib/librte_eal/common/eal_common_options.c
+++ b/lib/librte_eal/common/eal_common_options.c
@@ -151,7 +151,7 @@ eal_option_device_parse(void)
TAILQ_FOREACH_SAFE(devopt, &devopt_list, next, tmp) {
if (ret == 0) {
- ret = rte_eal_devargs_add(devopt->type, devopt->arg);
+ ret = rte_devargs_add(devopt->type, devopt->arg);
if (ret)
RTE_LOG(ERR, EAL, "Unable to parse device '%s'\n",
devopt->arg);
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 07a4701b2..3ba1f2084 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -60,6 +60,7 @@ struct rte_devargs {
};
/**
+ * @deprecated
* Parse a devargs string.
*
* For PCI devices, the format of arguments string is "PCI_ADDR" or
@@ -84,6 +85,7 @@ struct rte_devargs {
* - 0 on success
* - A negative value on error
*/
+__rte_deprecated
int rte_eal_parse_devargs_str(const char *devargs_str,
char **drvname, char **drvargs);
@@ -146,6 +148,32 @@ rte_devargs_insert(struct rte_devargs *da);
* - 0 on success
* - A negative value on error
*/
+__rte_experimental
+int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
+
+/**
+ * @deprecated
+ * Add a device to the user device list
+ *
+ * The format is
+ *
+ * bus:device_identifier,arg1=val1,arg2=val2
+ *
+ * Examples:
+ *
+ * pci:0000:05.00.0,arg=val
+ * vdev:net_ring0
+ *
+ * @param devtype
+ * The type of the device.
+ * @param devargs_str
+ * The arguments as given by the user.
+ *
+ * @return
+ * - 0 on success
+ * - A negative value on error
+ */
+__rte_deprecated
int rte_eal_devargs_add(enum rte_devtype devtype, const char *devargs_str);
/**
@@ -177,6 +205,21 @@ int rte_devargs_remove(const char *busname,
* @return
* The number of devices.
*/
+__rte_experimental
+unsigned int
+rte_devargs_type_count(enum rte_devtype devtype);
+
+/**
+ * @deprecated
+ * Count the number of user devices of a specified type
+ *
+ * @param devtype
+ * The type of the devices to counted.
+ *
+ * @return
+ * The number of devices.
+ */
+__rte_deprecated
unsigned int
rte_eal_devargs_type_count(enum rte_devtype devtype);
@@ -186,6 +229,17 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
* @param f
* A pointer to a file for output
*/
+__rte_experimental
+void rte_devargs_dump(FILE *f);
+
+/**
+ * @deprecated
+ * This function dumps the list of user device and their arguments.
+ *
+ * @param f
+ * A pointer to a file for output
+ */
+__rte_deprecated
void rte_eal_devargs_dump(FILE *f);
/**
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 1e614f088..bc24237e1 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -214,10 +214,13 @@ EXPERIMENTAL {
global:
rte_eal_cleanup;
+ rte_devargs_add;
+ rte_devargs_dump;
rte_devargs_insert;
rte_devargs_next;
rte_devargs_parse;
rte_devargs_remove;
+ rte_devargs_type_count;
rte_eal_hotplug_add;
rte_eal_hotplug_remove;
rte_eal_mbuf_user_pool_ops;
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 0590f0c10..027d5b535 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -596,27 +596,26 @@ eth_err(uint16_t port_id, int ret)
int
rte_eth_dev_attach(const char *devargs, uint16_t *port_id)
{
- int ret = -1;
int current = rte_eth_dev_count();
- char *name = NULL;
- char *args = NULL;
+ struct rte_devargs da = {0};
+ int ret = -1;
if ((devargs == NULL) || (port_id == NULL)) {
ret = -EINVAL;
goto err;
}
- /* parse devargs, then retrieve device name and args */
- if (rte_eal_parse_devargs_str(devargs, &name, &args))
+ /* parse devargs */
+ if (rte_devargs_parse(&da, "%s", devargs))
goto err;
- ret = rte_eal_dev_attach(name, args);
+ ret = rte_eal_hotplug_add(da.bus->name, da.name, da.args);
if (ret < 0)
goto err;
/* no point looking at the port count if no port exists */
if (!rte_eth_dev_count()) {
- RTE_LOG(ERR, EAL, "No port found for device (%s)\n", name);
+ RTE_LOG(ERR, EAL, "No port found for device (%s)\n", da.name);
ret = -1;
goto err;
}
@@ -633,45 +632,42 @@ rte_eth_dev_attach(const char *devargs, uint16_t *port_id)
ret = 0;
err:
- free(name);
- free(args);
+ free(da.args);
return ret;
}
/* detach the device, then store the name of the device */
int
-rte_eth_dev_detach(uint16_t port_id, char *name)
+rte_eth_dev_detach(uint16_t port_id, char *name __rte_unused)
{
+ struct rte_device *dev;
+ struct rte_bus *bus;
uint32_t dev_flags;
int ret = -1;
RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
- if (name == NULL) {
- ret = -EINVAL;
- goto err;
- }
-
dev_flags = rte_eth_devices[port_id].data->dev_flags;
if (dev_flags & RTE_ETH_DEV_BONDED_SLAVE) {
RTE_LOG(ERR, EAL, "Port %" PRIu16 " is bonded, cannot detach\n",
port_id);
- ret = -ENOTSUP;
- goto err;
+ return -ENOTSUP;
}
- snprintf(name, sizeof(rte_eth_devices[port_id].data->name),
- "%s", rte_eth_devices[port_id].data->name);
+ dev = rte_eth_devices[port_id].device;
+ if (dev == NULL)
+ return -EINVAL;
- ret = rte_eal_dev_detach(rte_eth_devices[port_id].device);
+ bus = rte_bus_find_by_device(dev);
+ if (bus == NULL)
+ return -ENOENT;
+
+ ret = rte_eal_hotplug_remove(bus->name, dev->name);
if (ret < 0)
- goto err;
+ return ret;
rte_eth_dev_release_port(&rte_eth_devices[port_id]);
return 0;
-
-err:
- return ret;
}
static int
diff --git a/test/test/commands.c b/test/test/commands.c
index cf0b726bb..2ae78a3d0 100644
--- a/test/test/commands.c
+++ b/test/test/commands.c
@@ -132,7 +132,7 @@ static void cmd_dump_parsed(void *parsed_result,
else if (!strcmp(res->dump, "dump_mempool"))
rte_mempool_list_dump(stdout);
else if (!strcmp(res->dump, "dump_devargs"))
- rte_eal_devargs_dump(stdout);
+ rte_devargs_dump(stdout);
else if (!strcmp(res->dump, "dump_log_types"))
rte_log_dump(stdout);
else if (!strcmp(res->dump, "dump_malloc_stats"))
--
2.11.0
^ permalink raw reply [flat|nested] 91+ messages in thread* Re: [dpdk-dev] [PATCH v3 00/10] devargs cleanup
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 00/10] " Gaetan Rivet
` (9 preceding siblings ...)
2018-03-20 23:20 ` [dpdk-dev] [PATCH v3 10/10] devargs: rename legacy API Gaetan Rivet
@ 2018-04-22 22:00 ` Thomas Monjalon
10 siblings, 0 replies; 91+ messages in thread
From: Thomas Monjalon @ 2018-04-22 22:00 UTC (permalink / raw)
To: Gaetan Rivet; +Cc: dev
21/03/2018 00:20, Gaetan Rivet:
> Cleanup the rte_devargs API.
>
> This is a continuous process.
> The previous version of this patchset was dependent on changes
> on the PCI bus and the rte_bus library. Not all these changes were
> integrated. This patchset has thus been limited to elements that are
> available right now, without dependencies.
>
> The devargs list is made private. All devargs users are meant to use them
> using the proper API. Devargs will allocate dynamic memory. With device hotplug,
> this memory is usually mishandled and lost by libraries not following the evolutions
> of rte_devargs.
>
> Gaetan Rivet (10):
> devargs: introduce iterator
> devargs: introduce foreach macro
> bus/vdev: do not reference devargs list
> bus/pci: do not reference devargs list
> net/vdev_netvsc: do not reference devargs list
> test: remove devargs unit tests
> devargs: make devargs list private
> devargs: make parsing variadic
> devargs: use proper namespace prefix
> devargs: rename legacy API
Sorry, I missed this series sent one month ago.
I did few small comments.
When then they are addressed, you can rebase and add my
Acked-by: Thomas Monjalon <thomas@monjalon.net>
Thanks
^ permalink raw reply [flat|nested] 91+ messages in thread