[dpdk-dev] [RFC] eal: simplify parameters of hotplug functions

[dpdk-dev] [RFC] eal: simplify parameters of hotplug functions

[Thomas Monjalon]

All informations about a device to probe can be grouped
in a common string, which is what we usually call devargs.
That's why the bus name and device name can be removed from
rte_eal_hotplug_add().

When removing a device, we already know its rte_device handle
which can be directly passed as parameter of rte_eal_hotplug_remove().
If the rte_device is not known, it can be retrieved from the devargs,
thanks to RTE_DEV_FOREACH().

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
This patch contains only the change in the API as RFC.

This idea was presented at Dublin during the "hotplug talk".
---
 lib/librte_eal/common/include/rte_dev.h | 19 +++++--------------
 1 file changed, 5 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index b80a80598..2f4212d01 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -194,18 +194,12 @@ int rte_eal_dev_detach(struct rte_device *dev);
  *
  * Hotplug add a given device to a specific bus.
  *
- * @param busname
- *   The bus name the device is added to.
- * @param devname
- *   The device name. Based on this device name, eal will identify a driver
- *   capable of handling it and pass it to the driver probing function.
  * @param devargs
- *   Device arguments to be passed to the driver.
+ *   Device arguments including bus, class and driver properties
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs);
+int __rte_experimental rte_eal_hotplug_add(const char *devargs);
 
 /**
  * @warning
@@ -213,15 +207,12 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
  *
  * Hotplug remove a given device from a specific bus.
  *
- * @param busname
- *   The bus name the device is removed from.
- * @param devname
- *   The device name being removed.
+ * @param dev
+ *   Data structure of the device to remove
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_remove(const char *busname,
-					  const char *devname);
+int __rte_experimental rte_eal_hotplug_remove(struct rte_device *dev);
 
 /**
  * Device comparison function.
-- 
2.18.0

[dpdk-dev] [PATCH v2 0/4] eal: simplify devargs and hotplug functions

[Thomas Monjalon]

This is a follow-up of an idea presented at Dublin
during the "hotplug talk".

Instead of changing the existing hotplug functions, as in the RFC,
some new experimental functions are added.
The old functions lose their experimental status in order to provide
a non-experimental replacement for deprecated attach/detach functions.

It has been discussed briefly in the technical board meeting today.

Thomas Monjalon (4):
  devargs: remove deprecated functions
  devargs: simplify parameters of removal function
  eal: remove experimental flag of hotplug functions
  eal: simplify parameters of hotplug functions

 drivers/bus/ifpga/ifpga_bus.c               |  5 +-
 drivers/bus/vdev/vdev.c                     |  8 +-
 drivers/net/failsafe/failsafe_eal.c         |  3 +-
 drivers/net/failsafe/failsafe_ether.c       |  3 +-
 lib/librte_eal/common/eal_common_dev.c      | 85 +++++++++++++--------
 lib/librte_eal/common/eal_common_devargs.c  | 38 +--------
 lib/librte_eal/common/include/rte_dev.h     | 35 +++++++--
 lib/librte_eal/common/include/rte_devargs.h | 81 +-------------------
 lib/librte_eal/rte_eal_version.map          | 10 +--
 9 files changed, 96 insertions(+), 172 deletions(-)

-- 
2.19.0

[dpdk-dev] [PATCH v2 1/4] devargs: remove deprecated functions

[Thomas Monjalon]

rte_eal_parse_devargs_str() does not support parsing the bus name
at the start of devargs. So it has been renamed and deprecated.

rte_eal_devargs_add(), rte_eal_devargs_type_count() and
rte_eal_devargs_dump() were declared deprecated and had their
implementation body renamed.

All these functions were deprecated in release 18.05.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
The library version is not updated because it should be done
in earlier patch https://patches.dpdk.org/patch/43903/
---
 lib/librte_eal/common/eal_common_devargs.c  | 30 ---------
 lib/librte_eal/common/include/rte_devargs.h | 71 ---------------------
 lib/librte_eal/rte_eal_version.map          |  4 --
 3 files changed, 105 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index dac2402a4..f63d2c663 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -31,36 +31,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 size_t
 devargs_layer_count(const char *s)
 {
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 097a4ce7b..0eef6e9c4 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -66,36 +66,6 @@ struct rte_devargs {
 	const char *data; /**< Device string storage. */
 };
 
-/**
- * @deprecated
- * 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
- */
-__rte_deprecated
-int rte_eal_parse_devargs_str(const char *devargs_str,
-				char **drvname, char **drvargs);
-
 /**
  * Parse a device string.
  *
@@ -201,23 +171,6 @@ rte_devargs_insert(struct rte_devargs *da);
 __rte_experimental
 int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
 
-/**
- * @deprecated
- * Add a device to the user device list
- * See rte_devargs_parse() for details.
- *
- * @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);
-
 /**
  * Remove a device from the user device list.
  * Its resources are freed.
@@ -251,20 +204,6 @@ __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);
-
 /**
  * This function dumps the list of user device and their arguments.
  *
@@ -274,16 +213,6 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
 __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);
-
 /**
  * Find next rte_devargs matching the provided bus name.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 73282bbb0..3df7f9831 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -19,9 +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_devargs_type_count;
 	rte_eal_get_configuration;
 	rte_eal_get_lcore_state;
 	rte_eal_get_physmem_size;
@@ -32,7 +29,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.19.0

[dpdk-dev] [PATCH v2 2/4] devargs: simplify parameters of removal function

[Thomas Monjalon]

The function rte_devargs_remove(), which is intended to be internal,
can take a devargs structure as argument.
The matching is still using string comparison of bus name and
device name.
It is simpler and may allow a different devargs matching in future.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/bus/ifpga/ifpga_bus.c               |  5 +----
 drivers/bus/vdev/vdev.c                     |  8 ++------
 lib/librte_eal/common/eal_common_dev.c      |  4 ++--
 lib/librte_eal/common/eal_common_devargs.c  |  8 ++++----
 lib/librte_eal/common/include/rte_devargs.h | 10 +++-------
 5 files changed, 12 insertions(+), 23 deletions(-)

diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index b324872ee..0e17ea9a3 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -363,7 +363,6 @@ static int
 ifpga_unplug(struct rte_device *dev)
 {
 	struct rte_afu_device *afu_dev = NULL;
-	struct rte_devargs *devargs = NULL;
 	int ret;
 
 	if (dev == NULL)
@@ -373,15 +372,13 @@ ifpga_unplug(struct rte_device *dev)
 	if (!afu_dev)
 		return -ENOENT;
 
-	devargs = dev->devargs;
-
 	ret = ifpga_remove_driver(afu_dev);
 	if (ret)
 		return ret;
 
 	TAILQ_REMOVE(&ifpga_afu_dev_list, afu_dev, next);
 
-	rte_devargs_remove(devargs->bus->name, devargs->name);
+	rte_devargs_remove(dev->devargs);
 	free(afu_dev);
 	return 0;
 
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 69dee89a8..390c2ce70 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -248,7 +248,6 @@ int
 rte_vdev_init(const char *name, const char *args)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
 	int ret;
 
 	rte_spinlock_recursive_lock(&vdev_device_list_lock);
@@ -259,9 +258,8 @@ rte_vdev_init(const char *name, const char *args)
 			if (ret > 0)
 				VDEV_LOG(ERR, "no driver found for %s", name);
 			/* If fails, remove it from vdev list */
-			devargs = dev->device.devargs;
 			TAILQ_REMOVE(&vdev_device_list, dev, next);
-			rte_devargs_remove(devargs->bus->name, devargs->name);
+			rte_devargs_remove(dev->device.devargs);
 			free(dev);
 		}
 	}
@@ -289,7 +287,6 @@ int
 rte_vdev_uninit(const char *name)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
 	int ret;
 
 	if (name == NULL)
@@ -308,8 +305,7 @@ rte_vdev_uninit(const char *name)
 		goto unlock;
 
 	TAILQ_REMOVE(&vdev_device_list, dev, next);
-	devargs = dev->device.devargs;
-	rte_devargs_remove(devargs->bus->name, devargs->name);
+	rte_devargs_remove(dev->device.devargs);
 	free(dev);
 
 unlock:
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 678dbcac7..e81ff4258 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -186,7 +186,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return 0;
 
 err_devarg:
-	if (rte_devargs_remove(busname, devname)) {
+	if (rte_devargs_remove(da)) {
 		free(da->args);
 		free(da);
 	}
@@ -227,7 +227,7 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
 			dev->name);
-	rte_devargs_remove(busname, devname);
+	rte_devargs_remove(dev->devargs);
 	return ret;
 }
 
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index f63d2c663..2f2bb4d90 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -266,7 +266,7 @@ rte_devargs_insert(struct rte_devargs *da)
 {
 	int ret;
 
-	ret = rte_devargs_remove(da->bus->name, da->name);
+	ret = rte_devargs_remove(da);
 	if (ret < 0)
 		return ret;
 	TAILQ_INSERT_TAIL(&devargs_list, da, next);
@@ -312,14 +312,14 @@ rte_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 }
 
 int __rte_experimental
-rte_devargs_remove(const char *busname, const char *devname)
+rte_devargs_remove(struct rte_devargs *devargs)
 {
 	struct rte_devargs *d;
 	void *tmp;
 
 	TAILQ_FOREACH_SAFE(d, &devargs_list, next, tmp) {
-		if (strcmp(d->bus->name, busname) == 0 &&
-		    strcmp(d->name, devname) == 0) {
+		if (strcmp(d->bus->name, devargs->bus->name) == 0 &&
+		    strcmp(d->name, devargs->name) == 0) {
 			TAILQ_REMOVE(&devargs_list, d, next);
 			free(d->args);
 			free(d);
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 0eef6e9c4..b1f121f83 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -176,11 +176,8 @@ int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
  * Its resources are freed.
  * If the devargs cannot be found, nothing happens.
  *
- * @param busname
- *   bus name of the devargs to remove.
- *
- * @param devname
- *   device name of the devargs to remove.
+ * @param devargs
+ *   The instance or a copy of devargs to remove.
  *
  * @return
  *   0 on success.
@@ -188,8 +185,7 @@ int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
  *   >0 if the devargs was not within the user device list.
  */
 __rte_experimental
-int rte_devargs_remove(const char *busname,
-		       const char *devname);
+int rte_devargs_remove(struct rte_devargs *devargs);
 
 /**
  * Count the number of user devices of a specified type
-- 
2.19.0

Re: [dpdk-dev] [PATCH v2 2/4] devargs: simplify parameters of removal function

[Ophir Munk]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> Sent: Thursday, September 27, 2018 12:48 AM
> To: dev@dpdk.org
> Cc: gaetan.rivet@6wind.com; Ophir Munk <ophirmu@mellanox.com>;
> qi.z.zhang@intel.com; ferruh.yigit@intel.com; ktraynor@redhat.com
> Subject: [PATCH v2 2/4] devargs: simplify parameters of removal function
> 
> The function rte_devargs_remove(), which is intended to be internal, can
> take a devargs structure as argument.
> The matching is still using string comparison of bus name and device name.
> It is simpler and may allow a different devargs matching in future.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  drivers/bus/ifpga/ifpga_bus.c               |  5 +----
>  drivers/bus/vdev/vdev.c                     |  8 ++------
>  lib/librte_eal/common/eal_common_dev.c      |  4 ++--
>  lib/librte_eal/common/eal_common_devargs.c  |  8 ++++----
> lib/librte_eal/common/include/rte_devargs.h | 10 +++-------
>  5 files changed, 12 insertions(+), 23 deletions(-)
> 
> diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
> index b324872ee..0e17ea9a3 100644
> --- a/drivers/bus/ifpga/ifpga_bus.c
> +++ b/drivers/bus/ifpga/ifpga_bus.c
> @@ -363,7 +363,6 @@ static int
>  ifpga_unplug(struct rte_device *dev)
>  {
>  	struct rte_afu_device *afu_dev = NULL;
> -	struct rte_devargs *devargs = NULL;
>  	int ret;
> 
>  	if (dev == NULL)
> @@ -373,15 +372,13 @@ ifpga_unplug(struct rte_device *dev)
>  	if (!afu_dev)
>  		return -ENOENT;
> 
> -	devargs = dev->devargs;
> -
>  	ret = ifpga_remove_driver(afu_dev);
>  	if (ret)
>  		return ret;
> 
>  	TAILQ_REMOVE(&ifpga_afu_dev_list, afu_dev, next);
> 
> -	rte_devargs_remove(devargs->bus->name, devargs->name);
> +	rte_devargs_remove(dev->devargs);
>  	free(afu_dev);
>  	return 0;
> 
> diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c index
> 69dee89a8..390c2ce70 100644
> --- a/drivers/bus/vdev/vdev.c
> +++ b/drivers/bus/vdev/vdev.c
> @@ -248,7 +248,6 @@ int
>  rte_vdev_init(const char *name, const char *args)  {
>  	struct rte_vdev_device *dev;
> -	struct rte_devargs *devargs;
>  	int ret;
> 
>  	rte_spinlock_recursive_lock(&vdev_device_list_lock);
> @@ -259,9 +258,8 @@ rte_vdev_init(const char *name, const char *args)
>  			if (ret > 0)
>  				VDEV_LOG(ERR, "no driver found for %s",
> name);
>  			/* If fails, remove it from vdev list */
> -			devargs = dev->device.devargs;
>  			TAILQ_REMOVE(&vdev_device_list, dev, next);
> -			rte_devargs_remove(devargs->bus->name, devargs-
> >name);
> +			rte_devargs_remove(dev->device.devargs);
>  			free(dev);
>  		}
>  	}
> @@ -289,7 +287,6 @@ int
>  rte_vdev_uninit(const char *name)
>  {
>  	struct rte_vdev_device *dev;
> -	struct rte_devargs *devargs;
>  	int ret;
> 
>  	if (name == NULL)
> @@ -308,8 +305,7 @@ rte_vdev_uninit(const char *name)
>  		goto unlock;
> 
>  	TAILQ_REMOVE(&vdev_device_list, dev, next);
> -	devargs = dev->device.devargs;
> -	rte_devargs_remove(devargs->bus->name, devargs->name);
> +	rte_devargs_remove(dev->device.devargs);
>  	free(dev);
> 
>  unlock:
> diff --git a/lib/librte_eal/common/eal_common_dev.c
> b/lib/librte_eal/common/eal_common_dev.c
> index 678dbcac7..e81ff4258 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -186,7 +186,7 @@ int __rte_experimental rte_eal_hotplug_add(const
> char *busname, const char *devn
>  	return 0;
> 
>  err_devarg:
> -	if (rte_devargs_remove(busname, devname)) {
> +	if (rte_devargs_remove(da)) {

Can you please explain why calling with 'da' parameter (which may have different internal fields of busname and devname than the explicit busname and devname parameters) - is correct?

>  		free(da->args);
>  		free(da);
>  	}
> @@ -227,7 +227,7 @@ rte_eal_hotplug_remove(const char *busname,
> const char *devname)
>  	if (ret)
>  		RTE_LOG(ERR, EAL, "Driver cannot detach the device
> (%s)\n",
>  			dev->name);
> -	rte_devargs_remove(busname, devname);
> +	rte_devargs_remove(dev->devargs);

Can you please explain why calling with 'dev->devargs' parameter (which may have different internal fields of busname and devname than the explicit busname and devname parameters) - is correct?

>  	return ret;
>  }
> 
> diff --git a/lib/librte_eal/common/eal_common_devargs.c
> b/lib/librte_eal/common/eal_common_devargs.c
> index f63d2c663..2f2bb4d90 100644
> --- a/lib/librte_eal/common/eal_common_devargs.c
> +++ b/lib/librte_eal/common/eal_common_devargs.c
> @@ -266,7 +266,7 @@ rte_devargs_insert(struct rte_devargs *da)  {
>  	int ret;
> 
> -	ret = rte_devargs_remove(da->bus->name, da->name);
> +	ret = rte_devargs_remove(da);
>  	if (ret < 0)
>  		return ret;
>  	TAILQ_INSERT_TAIL(&devargs_list, da, next); @@ -312,14 +312,14
> @@ rte_devargs_add(enum rte_devtype devtype, const char *devargs_str)  }
> 
>  int __rte_experimental
> -rte_devargs_remove(const char *busname, const char *devname)
> +rte_devargs_remove(struct rte_devargs *devargs)
>  {
>  	struct rte_devargs *d;
>  	void *tmp;
> 
>  	TAILQ_FOREACH_SAFE(d, &devargs_list, next, tmp) {
> -		if (strcmp(d->bus->name, busname) == 0 &&
> -		    strcmp(d->name, devname) == 0) {
> +		if (strcmp(d->bus->name, devargs->bus->name) == 0 &&
> +		    strcmp(d->name, devargs->name) == 0) {
>  			TAILQ_REMOVE(&devargs_list, d, next);
>  			free(d->args);
>  			free(d);
> diff --git a/lib/librte_eal/common/include/rte_devargs.h
> b/lib/librte_eal/common/include/rte_devargs.h
> index 0eef6e9c4..b1f121f83 100644
> --- a/lib/librte_eal/common/include/rte_devargs.h
> +++ b/lib/librte_eal/common/include/rte_devargs.h
> @@ -176,11 +176,8 @@ int rte_devargs_add(enum rte_devtype devtype,
> const char *devargs_str);
>   * Its resources are freed.
>   * If the devargs cannot be found, nothing happens.
>   *
> - * @param busname
> - *   bus name of the devargs to remove.
> - *
> - * @param devname
> - *   device name of the devargs to remove.
> + * @param devargs
> + *   The instance or a copy of devargs to remove.
>   *
>   * @return
>   *   0 on success.
> @@ -188,8 +185,7 @@ int rte_devargs_add(enum rte_devtype devtype,
> const char *devargs_str);
>   *   >0 if the devargs was not within the user device list.
>   */
>  __rte_experimental
> -int rte_devargs_remove(const char *busname,
> -		       const char *devname);
> +int rte_devargs_remove(struct rte_devargs *devargs);
> 
>  /**
>   * Count the number of user devices of a specified type
> --
> 2.19.0

Re: [dpdk-dev] [PATCH v2 2/4] devargs: simplify parameters of removal function

[Thomas Monjalon]

Hi,

27/09/2018 10:24, Ophir Munk:
> From: Thomas Monjalon [mailto:thomas@monjalon.net]
> > The function rte_devargs_remove(), which is intended to be internal, can
> > take a devargs structure as argument.
> > The matching is still using string comparison of bus name and device name.
> > It is simpler and may allow a different devargs matching in future.
[...]
> > --- a/lib/librte_eal/common/eal_common_dev.c
> > +++ b/lib/librte_eal/common/eal_common_dev.c
> > @@ -186,7 +186,7 @@ int __rte_experimental rte_eal_hotplug_add(const
> > char *busname, const char *devn
> >  	return 0;
> > 
> >  err_devarg:
> > -	if (rte_devargs_remove(busname, devname)) {
> > +	if (rte_devargs_remove(da)) {
> 
> Can you please explain why calling with 'da' parameter
> (which may have different internal fields of busname and devname
> than the explicit busname and devname parameters) - is correct?

Before devargs is parsed and inserted in the global list,
devargs values are NULL, so nothing is found (as before this patch).
rte_devargs_remove(da) will have no effect and will return 1,
so the devargs will be freed by code in the if block.

When devargs is inserted in the list, it will have the values
of busname and devname, so it is the same as before this patch.

However, there may be an issue dereferencing devargs->bus->name
when devargs is not parsed. I must add this code in rte_devargs_remove:
    if (devargs->bus == NULL)
        return 1;

[...]
> > @@ -227,7 +227,7 @@ rte_eal_hotplug_remove(const char *busname,
> > const char *devname)
> >  	if (ret)
> >  		RTE_LOG(ERR, EAL, "Driver cannot detach the device
> > (%s)\n",
> >  			dev->name);
> > -	rte_devargs_remove(busname, devname);
> > +	rte_devargs_remove(dev->devargs);
> 
> Can you please explain why calling with 'dev->devargs' parameter
> (which may have different internal fields of busname and devname
> than the explicit busname and devname parameters) - is correct?

The device is found by checking the busname and devname.
I don't see how the devargs values may be different.

[dpdk-dev] [PATCH v2 3/4] eal: remove experimental flag of hotplug functions

[Thomas Monjalon]

These functions are quite old and are the only available replacement
for the deprecated attach/detach functions.

Note: some new functions may (again) replace these hotplug functions,
in future, with better parameters.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_eal/common/eal_common_dev.c  |  7 ++++---
 lib/librte_eal/common/include/rte_dev.h | 13 +++----------
 lib/librte_eal/rte_eal_version.map      |  4 ++--
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index e81ff4258..43fdab395 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -127,8 +127,9 @@ int rte_eal_dev_detach(struct rte_device *dev)
 	return ret;
 }
 
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs)
+int
+rte_eal_hotplug_add(const char *busname, const char *devname,
+                    const char *devargs)
 {
 	struct rte_bus *bus;
 	struct rte_device *dev;
@@ -193,7 +194,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return ret;
 }
 
-int __rte_experimental
+int
 rte_eal_hotplug_remove(const char *busname, const char *devname)
 {
 	struct rte_bus *bus;
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index b80a80598..4f5282459 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -189,9 +189,6 @@ __rte_deprecated
 int rte_eal_dev_detach(struct rte_device *dev);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Hotplug add a given device to a specific bus.
  *
  * @param busname
@@ -204,13 +201,10 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs);
+int rte_eal_hotplug_add(const char *busname, const char *devname,
+                        const char *devargs);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Hotplug remove a given device from a specific bus.
  *
  * @param busname
@@ -220,8 +214,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_remove(const char *busname,
-					  const char *devname);
+int rte_eal_hotplug_remove(const char *busname, const char *devname);
 
 /**
  * Device comparison function.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 3df7f9831..6bff37f4b 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -261,6 +261,8 @@ DPDK_18.08 {
 DPDK_18.11 {
 	global:
 
+	rte_eal_hotplug_add;
+	rte_eal_hotplug_remove;
 	rte_strscpy;
 
 } DPDK_18.08;
@@ -288,8 +290,6 @@ EXPERIMENTAL {
 	rte_devargs_remove;
 	rte_devargs_type_count;
 	rte_eal_cleanup;
-	rte_eal_hotplug_add;
-	rte_eal_hotplug_remove;
 	rte_fbarray_attach;
 	rte_fbarray_destroy;
 	rte_fbarray_detach;
-- 
2.19.0

[dpdk-dev] [PATCH v2 4/4] eal: simplify parameters of hotplug functions

[Thomas Monjalon]

All information about a device to probe can be grouped
in a common string, which is what we usually call devargs.
An application should not have to parse this string before
calling the EAL probe function.
And the syntax could evolve to be more complex and support
matching multiple devices in one string.
That's why the bus name and device name should be removed from
rte_eal_hotplug_add().
Instead of changing this function, a simpler one is added
and used in the old one, which may be deprecated later.

When removing a device, we already know its rte_device handle
which can be directly passed as parameter of rte_eal_hotplug_remove().
If the rte_device is not known, it can be retrieved with the devargs,
by iterating in the device list (future RTE_DEV_FOREACH()).
Similarly to the probing case, a new function is added
and used in the old one, which may be deprecated later.
The new function is used in failsafe, because the replacement is easy.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/net/failsafe/failsafe_eal.c     |  3 +-
 drivers/net/failsafe/failsafe_ether.c   |  3 +-
 lib/librte_eal/common/eal_common_dev.c  | 76 ++++++++++++++++---------
 lib/librte_eal/common/include/rte_dev.h | 30 +++++++++-
 lib/librte_eal/rte_eal_version.map      |  2 +
 5 files changed, 80 insertions(+), 34 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index ce1633f13..8a888b1ff 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -144,8 +144,7 @@ fs_bus_uninit(struct rte_eth_dev *dev)
 	int ret = 0;
 
 	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
-		sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
-							sdev->dev->name);
+		sdev_ret = rte_dev_remove(sdev->dev);
 		if (sdev_ret) {
 			ERROR("Failed to remove requested device %s (err: %d)",
 			      sdev->dev->name, sdev_ret);
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 5b5cb3b49..cd7b97e1f 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -265,8 +265,7 @@ fs_dev_remove(struct sub_device *sdev)
 		sdev->state = DEV_PROBED;
 		/* fallthrough */
 	case DEV_PROBED:
-		ret = rte_eal_hotplug_remove(sdev->bus->name,
-					     sdev->dev->name);
+		ret = rte_dev_remove(sdev->dev);
 		if (ret) {
 			ERROR("Bus detach failed for sub_device %u",
 			      SUB_ID(sdev));
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 43fdab395..85eb1569f 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -129,46 +129,57 @@ int rte_eal_dev_detach(struct rte_device *dev)
 
 int
 rte_eal_hotplug_add(const char *busname, const char *devname,
-                    const char *devargs)
+                    const char *drvargs)
+{
+	char *devargs = NULL;
+	int size, length = -1;
+
+	do { /* 2 iterations: first is to know string length */
+		size = length + 1;
+		length = snprintf(devargs, size, "%s:%s,%s", busname, devname, drvargs);
+		if (length >= size)
+			devargs = malloc(length + 1);
+		if (devargs == NULL)
+			return -ENOMEM;
+	} while (size == 0);
+
+	return rte_dev_probe(devargs);
+}
+
+int __rte_experimental
+rte_dev_probe(const char *devargs)
 {
-	struct rte_bus *bus;
 	struct rte_device *dev;
 	struct rte_devargs *da;
 	int ret;
 
-	bus = rte_bus_find_by_name(busname);
-	if (bus == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n", busname);
-		return -ENOENT;
-	}
-
-	if (bus->plug == NULL) {
-		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
-			bus->name);
-		return -ENOTSUP;
-	}
-
 	da = calloc(1, sizeof(*da));
 	if (da == NULL)
 		return -ENOMEM;
 
-	ret = rte_devargs_parsef(da, "%s:%s,%s",
-				 busname, devname, devargs);
+	ret = rte_devargs_parse(da, devargs);
 	if (ret)
 		goto err_devarg;
 
+	if (da->bus->plug == NULL) {
+		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
+			da->bus->name);
+		ret = -ENOTSUP;
+		goto err_devarg;
+	}
+
 	ret = rte_devargs_insert(da);
 	if (ret)
 		goto err_devarg;
 
-	ret = bus->scan();
+	ret = da->bus->scan();
 	if (ret)
 		goto err_devarg;
 
-	dev = bus->find_device(NULL, cmp_dev_name, devname);
+	dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find device (%s)\n",
-			devname);
+			da->name);
 		ret = -ENODEV;
 		goto err_devarg;
 	}
@@ -178,7 +189,7 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 		return -EEXIST;
 	}
 
-	ret = bus->plug(dev);
+	ret = da->bus->plug(dev);
 	if (ret) {
 		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
 			dev->name);
@@ -197,9 +208,8 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 int
 rte_eal_hotplug_remove(const char *busname, const char *devname)
 {
-	struct rte_bus *bus;
 	struct rte_device *dev;
-	int ret;
+	struct rte_bus *bus;
 
 	bus = rte_bus_find_by_name(busname);
 	if (bus == NULL) {
@@ -207,23 +217,33 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 		return -ENOENT;
 	}
 
-	if (bus->unplug == NULL) {
-		RTE_LOG(ERR, EAL, "Function unplug not supported by bus (%s)\n",
-			bus->name);
-		return -ENOTSUP;
-	}
-
 	dev = bus->find_device(NULL, cmp_dev_name, devname);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find plugged device (%s)\n", devname);
 		return -EINVAL;
 	}
 
+	return rte_dev_remove(dev);
+}
+
+int __rte_experimental
+rte_dev_remove(struct rte_device *dev)
+{
+	struct rte_bus *bus;
+	int ret;
+
 	if (dev->driver == NULL) {
 		RTE_LOG(ERR, EAL, "Device is already unplugged\n");
 		return -ENOENT;
 	}
 
+	bus = dev->devargs->bus;
+	if (bus->unplug == NULL) {
+		RTE_LOG(ERR, EAL, "Function unplug not supported by bus (%s)\n",
+			bus->name);
+		return -ENOTSUP;
+	}
+
 	ret = bus->unplug(dev);
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index 4f5282459..7a30362c0 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -196,13 +196,26 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @param devname
  *   The device name. Based on this device name, eal will identify a driver
  *   capable of handling it and pass it to the driver probing function.
- * @param devargs
+ * @param drvargs
  *   Device arguments to be passed to the driver.
  * @return
  *   0 on success, negative on error.
  */
 int rte_eal_hotplug_add(const char *busname, const char *devname,
-                        const char *devargs);
+                        const char *drvargs);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Add matching devices.
+ *
+ * @param devargs
+ *   Device arguments including bus, class and driver properties.
+ * @return
+ *   0 on success, negative on error.
+ */
+int __rte_experimental rte_dev_probe(const char *devargs);
 
 /**
  * Hotplug remove a given device from a specific bus.
@@ -216,6 +229,19 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
  */
 int rte_eal_hotplug_remove(const char *busname, const char *devname);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Remove one device.
+ *
+ * @param dev
+ *   Data structure of the device to remove.
+ * @return
+ *   0 on success, negative on error.
+ */
+int __rte_experimental rte_dev_remove(struct rte_device *dev);
+
 /**
  * Device comparison function.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 6bff37f4b..2ea7a870a 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -281,6 +281,8 @@ EXPERIMENTAL {
 	rte_dev_event_monitor_stop;
 	rte_dev_iterator_init;
 	rte_dev_iterator_next;
+	rte_dev_probe;
+	rte_dev_remove;
 	rte_devargs_add;
 	rte_devargs_dump;
 	rte_devargs_insert;
-- 
2.19.0

[dpdk-dev] [PATCH v3 0/4] eal: simplify devargs and hotplug functions

[Thomas Monjalon]

This is a follow-up of an idea presented at Dublin
during the "hotplug talk".

Instead of changing the existing hotplug functions, as in the RFC,
some new experimental functions are added.
The old functions lose their experimental status in order to provide
a non-experimental replacement for deprecated attach/detach functions.

It has been discussed briefly in the technical board meeting this week.


Change in v3:
  - fix null dereferencing in error path (patch 2)


Thomas Monjalon (4):
  devargs: remove deprecated functions
  devargs: simplify parameters of removal function
  eal: remove experimental flag of hotplug functions
  eal: simplify parameters of hotplug functions

 drivers/bus/ifpga/ifpga_bus.c               |  5 +-
 drivers/bus/vdev/vdev.c                     |  8 +-
 drivers/net/failsafe/failsafe_eal.c         |  3 +-
 drivers/net/failsafe/failsafe_ether.c       |  3 +-
 lib/librte_eal/common/eal_common_dev.c      | 85 +++++++++++++--------
 lib/librte_eal/common/eal_common_devargs.c  | 41 ++--------
 lib/librte_eal/common/include/rte_dev.h     | 35 +++++++--
 lib/librte_eal/common/include/rte_devargs.h | 81 +-------------------
 lib/librte_eal/rte_eal_version.map          | 10 +--
 9 files changed, 99 insertions(+), 172 deletions(-)

-- 
2.19.0

[dpdk-dev] [PATCH v3 1/4] devargs: remove deprecated functions

[Thomas Monjalon]

rte_eal_parse_devargs_str() does not support parsing the bus name
at the start of devargs. So it has been renamed and deprecated.

rte_eal_devargs_add(), rte_eal_devargs_type_count() and
rte_eal_devargs_dump() were declared deprecated and had their
implementation body renamed.

All these functions were deprecated in release 18.05.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
The library version is not updated because it should be done
in earlier patch https://patches.dpdk.org/patch/43903/
---
 lib/librte_eal/common/eal_common_devargs.c  | 30 ---------
 lib/librte_eal/common/include/rte_devargs.h | 71 ---------------------
 lib/librte_eal/rte_eal_version.map          |  4 --
 3 files changed, 105 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index dac2402a4..f63d2c663 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -31,36 +31,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 size_t
 devargs_layer_count(const char *s)
 {
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 097a4ce7b..0eef6e9c4 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -66,36 +66,6 @@ struct rte_devargs {
 	const char *data; /**< Device string storage. */
 };
 
-/**
- * @deprecated
- * 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
- */
-__rte_deprecated
-int rte_eal_parse_devargs_str(const char *devargs_str,
-				char **drvname, char **drvargs);
-
 /**
  * Parse a device string.
  *
@@ -201,23 +171,6 @@ rte_devargs_insert(struct rte_devargs *da);
 __rte_experimental
 int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
 
-/**
- * @deprecated
- * Add a device to the user device list
- * See rte_devargs_parse() for details.
- *
- * @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);
-
 /**
  * Remove a device from the user device list.
  * Its resources are freed.
@@ -251,20 +204,6 @@ __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);
-
 /**
  * This function dumps the list of user device and their arguments.
  *
@@ -274,16 +213,6 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
 __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);
-
 /**
  * Find next rte_devargs matching the provided bus name.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 73282bbb0..3df7f9831 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -19,9 +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_devargs_type_count;
 	rte_eal_get_configuration;
 	rte_eal_get_lcore_state;
 	rte_eal_get_physmem_size;
@@ -32,7 +29,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.19.0

Re: [dpdk-dev] [PATCH v3 1/4] devargs: remove deprecated functions

[Andrew Rybchenko]

On 9/28/18 7:21 PM, Thomas Monjalon wrote:
> rte_eal_parse_devargs_str() does not support parsing the bus name
> at the start of devargs. So it has been renamed and deprecated.
>
> rte_eal_devargs_add(), rte_eal_devargs_type_count() and
> rte_eal_devargs_dump() were declared deprecated and had their
> implementation body renamed.
>
> All these functions were deprecated in release 18.05.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

I would say that it is more fair to say that changes related to
rte_eal_devargs_add(), rte_eal_devargs_type_count() and
rte_eal_devargs_dump() fixes b65ecf19 since the changeset
simply removes these functions. May be the ideal solution
is to add changeset which adds these deprecated functions
and simply calls replacement and remove in the next changeset.
But it is too many efforts for nothing, so good to go as is.

Should it be mentioned in release notes (API removal)?

Re: [dpdk-dev] [PATCH v3 1/4] devargs: remove deprecated functions

[Thomas Monjalon]

01/10/2018 13:24, Andrew Rybchenko:
> On 9/28/18 7:21 PM, Thomas Monjalon wrote:
> > rte_eal_parse_devargs_str() does not support parsing the bus name
> > at the start of devargs. So it has been renamed and deprecated.
> >
> > rte_eal_devargs_add(), rte_eal_devargs_type_count() and
> > rte_eal_devargs_dump() were declared deprecated and had their
> > implementation body renamed.
> >
> > All these functions were deprecated in release 18.05.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> I would say that it is more fair to say that changes related to
> rte_eal_devargs_add(), rte_eal_devargs_type_count() and
> rte_eal_devargs_dump() fixes b65ecf19 since the changeset
> simply removes these functions. May be the ideal solution
> is to add changeset which adds these deprecated functions
> and simply calls replacement and remove in the next changeset.
> But it is too many efforts for nothing, so good to go as is.

Yes, not worth the effort.

> Should it be mentioned in release notes (API removal)?

Yes, you're right, will add a note in v4.
Thanks

[dpdk-dev] [PATCH v3 2/4] devargs: simplify parameters of removal function

[Thomas Monjalon]

The function rte_devargs_remove(), which is intended to be internal,
can take a devargs structure as argument.
The matching is still using string comparison of bus name and
device name.
It is simpler and may allow a different devargs matching in future.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/bus/ifpga/ifpga_bus.c               |  5 +----
 drivers/bus/vdev/vdev.c                     |  8 ++------
 lib/librte_eal/common/eal_common_dev.c      |  4 ++--
 lib/librte_eal/common/eal_common_devargs.c  | 11 +++++++----
 lib/librte_eal/common/include/rte_devargs.h | 10 +++-------
 5 files changed, 15 insertions(+), 23 deletions(-)

diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index c54b59db2..3ef035b7e 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -361,7 +361,6 @@ static int
 ifpga_unplug(struct rte_device *dev)
 {
 	struct rte_afu_device *afu_dev = NULL;
-	struct rte_devargs *devargs = NULL;
 	int ret;
 
 	if (dev == NULL)
@@ -371,15 +370,13 @@ ifpga_unplug(struct rte_device *dev)
 	if (!afu_dev)
 		return -ENOENT;
 
-	devargs = dev->devargs;
-
 	ret = ifpga_remove_driver(afu_dev);
 	if (ret)
 		return ret;
 
 	TAILQ_REMOVE(&ifpga_afu_dev_list, afu_dev, next);
 
-	rte_devargs_remove(devargs->bus->name, devargs->name);
+	rte_devargs_remove(dev->devargs);
 	free(afu_dev);
 	return 0;
 
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 69dee89a8..390c2ce70 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -248,7 +248,6 @@ int
 rte_vdev_init(const char *name, const char *args)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
 	int ret;
 
 	rte_spinlock_recursive_lock(&vdev_device_list_lock);
@@ -259,9 +258,8 @@ rte_vdev_init(const char *name, const char *args)
 			if (ret > 0)
 				VDEV_LOG(ERR, "no driver found for %s", name);
 			/* If fails, remove it from vdev list */
-			devargs = dev->device.devargs;
 			TAILQ_REMOVE(&vdev_device_list, dev, next);
-			rte_devargs_remove(devargs->bus->name, devargs->name);
+			rte_devargs_remove(dev->device.devargs);
 			free(dev);
 		}
 	}
@@ -289,7 +287,6 @@ int
 rte_vdev_uninit(const char *name)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
 	int ret;
 
 	if (name == NULL)
@@ -308,8 +305,7 @@ rte_vdev_uninit(const char *name)
 		goto unlock;
 
 	TAILQ_REMOVE(&vdev_device_list, dev, next);
-	devargs = dev->device.devargs;
-	rte_devargs_remove(devargs->bus->name, devargs->name);
+	rte_devargs_remove(dev->device.devargs);
 	free(dev);
 
 unlock:
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 678dbcac7..e81ff4258 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -186,7 +186,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return 0;
 
 err_devarg:
-	if (rte_devargs_remove(busname, devname)) {
+	if (rte_devargs_remove(da)) {
 		free(da->args);
 		free(da);
 	}
@@ -227,7 +227,7 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
 			dev->name);
-	rte_devargs_remove(busname, devname);
+	rte_devargs_remove(dev->devargs);
 	return ret;
 }
 
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index f63d2c663..d8db45a23 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -266,7 +266,7 @@ rte_devargs_insert(struct rte_devargs *da)
 {
 	int ret;
 
-	ret = rte_devargs_remove(da->bus->name, da->name);
+	ret = rte_devargs_remove(da);
 	if (ret < 0)
 		return ret;
 	TAILQ_INSERT_TAIL(&devargs_list, da, next);
@@ -312,14 +312,17 @@ rte_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 }
 
 int __rte_experimental
-rte_devargs_remove(const char *busname, const char *devname)
+rte_devargs_remove(struct rte_devargs *devargs)
 {
 	struct rte_devargs *d;
 	void *tmp;
 
+	if (devargs->bus == NULL)
+		return -1;
+
 	TAILQ_FOREACH_SAFE(d, &devargs_list, next, tmp) {
-		if (strcmp(d->bus->name, busname) == 0 &&
-		    strcmp(d->name, devname) == 0) {
+		if (strcmp(d->bus->name, devargs->bus->name) == 0 &&
+		    strcmp(d->name, devargs->name) == 0) {
 			TAILQ_REMOVE(&devargs_list, d, next);
 			free(d->args);
 			free(d);
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 0eef6e9c4..b1f121f83 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -176,11 +176,8 @@ int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
  * Its resources are freed.
  * If the devargs cannot be found, nothing happens.
  *
- * @param busname
- *   bus name of the devargs to remove.
- *
- * @param devname
- *   device name of the devargs to remove.
+ * @param devargs
+ *   The instance or a copy of devargs to remove.
  *
  * @return
  *   0 on success.
@@ -188,8 +185,7 @@ int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
  *   >0 if the devargs was not within the user device list.
  */
 __rte_experimental
-int rte_devargs_remove(const char *busname,
-		       const char *devname);
+int rte_devargs_remove(struct rte_devargs *devargs);
 
 /**
  * Count the number of user devices of a specified type
-- 
2.19.0

Re: [dpdk-dev] [PATCH v3 2/4] devargs: simplify parameters of removal function

[Andrew Rybchenko]

On 9/28/18 7:21 PM, Thomas Monjalon wrote:
> The function rte_devargs_remove(), which is intended to be internal,
> can take a devargs structure as argument.
> The matching is still using string comparison of bus name and
> device name.
> It is simpler and may allow a different devargs matching in future.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

What about release notes? It is API change, yes, experimental, but still.

Re: [dpdk-dev] [PATCH v3 2/4] devargs: simplify parameters of removal function

[Thomas Monjalon]

01/10/2018 13:25, Andrew Rybchenko:
> On 9/28/18 7:21 PM, Thomas Monjalon wrote:
> > The function rte_devargs_remove(), which is intended to be internal,
> > can take a devargs structure as argument.
> > The matching is still using string comparison of bus name and
> > device name.
> > It is simpler and may allow a different devargs matching in future.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> What about release notes? It is API change, yes, experimental, but still.

Yes, you're right.
Will fix in v4, thanks.

[dpdk-dev] [PATCH v3 3/4] eal: remove experimental flag of hotplug functions

[Thomas Monjalon]

These functions are quite old and are the only available replacement
for the deprecated attach/detach functions.

Note: some new functions may (again) replace these hotplug functions,
in future, with better parameters.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 lib/librte_eal/common/eal_common_dev.c  |  7 ++++---
 lib/librte_eal/common/include/rte_dev.h | 11 ++---------
 lib/librte_eal/rte_eal_version.map      |  4 ++--
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index e81ff4258..a9be58edf 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -127,8 +127,9 @@ int rte_eal_dev_detach(struct rte_device *dev)
 	return ret;
 }
 
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs)
+int
+rte_eal_hotplug_add(const char *busname, const char *devname,
+		    const char *devargs)
 {
 	struct rte_bus *bus;
 	struct rte_device *dev;
@@ -193,7 +194,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return ret;
 }
 
-int __rte_experimental
+int
 rte_eal_hotplug_remove(const char *busname, const char *devname)
 {
 	struct rte_bus *bus;
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index b80a80598..d440c4e58 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -189,9 +189,6 @@ __rte_deprecated
 int rte_eal_dev_detach(struct rte_device *dev);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Hotplug add a given device to a specific bus.
  *
  * @param busname
@@ -204,13 +201,10 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
+int rte_eal_hotplug_add(const char *busname, const char *devname,
 			const char *devargs);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Hotplug remove a given device from a specific bus.
  *
  * @param busname
@@ -220,8 +214,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_remove(const char *busname,
-					  const char *devname);
+int rte_eal_hotplug_remove(const char *busname, const char *devname);
 
 /**
  * Device comparison function.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 3df7f9831..6bff37f4b 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -261,6 +261,8 @@ DPDK_18.08 {
 DPDK_18.11 {
 	global:
 
+	rte_eal_hotplug_add;
+	rte_eal_hotplug_remove;
 	rte_strscpy;
 
 } DPDK_18.08;
@@ -288,8 +290,6 @@ EXPERIMENTAL {
 	rte_devargs_remove;
 	rte_devargs_type_count;
 	rte_eal_cleanup;
-	rte_eal_hotplug_add;
-	rte_eal_hotplug_remove;
 	rte_fbarray_attach;
 	rte_fbarray_destroy;
 	rte_fbarray_detach;
-- 
2.19.0

Re: [dpdk-dev] [PATCH v3 3/4] eal: remove experimental flag of hotplug functions

[Andrew Rybchenko]

On 9/28/18 7:21 PM, Thomas Monjalon wrote:
> These functions are quite old and are the only available replacement
> for the deprecated attach/detach functions.
>
> Note: some new functions may (again) replace these hotplug functions,
> in future, with better parameters.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

Should it be mentioned in release notes?

Re: [dpdk-dev] [PATCH v3 3/4] eal: remove experimental flag of hotplug functions

[Thomas Monjalon]

01/10/2018 13:26, Andrew Rybchenko:
> On 9/28/18 7:21 PM, Thomas Monjalon wrote:
> > These functions are quite old and are the only available replacement
> > for the deprecated attach/detach functions.
> >
> > Note: some new functions may (again) replace these hotplug functions,
> > in future, with better parameters.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> Should it be mentioned in release notes?

There is no dedicated section for functions becoming "stable".
I am afraid it may become too much administrative if we want to note
all functions moving from experimental.

Re: [dpdk-dev] [PATCH v3 3/4] eal: remove experimental flag of hotplug functions

[Ferruh Yigit]

On 9/28/2018 5:21 PM, Thomas Monjalon wrote:
> These functions are quite old and are the only available replacement
> for the deprecated attach/detach functions.
> 
> Note: some new functions may (again) replace these hotplug functions,
> in future, with better parameters.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  lib/librte_eal/common/eal_common_dev.c  |  7 ++++---
>  lib/librte_eal/common/include/rte_dev.h | 11 ++---------
>  lib/librte_eal/rte_eal_version.map      |  4 ++--

Can remove "-DALLOW_EXPERIMENTAL_API" (or "allow_experimental_apis = true" for
meson) from drivers/raw/ifpga_rawdev when these APIs are not experimental anymore.
For reference: https://patches.dpdk.org/patch/45836/

It is easy to know when to add "-DALLOW_EXPERIMENTAL_API" but it is hard to know
when to remove one, some helper should be good there.

Re: [dpdk-dev] [PATCH v3 3/4] eal: remove experimental flag of hotplug functions

[Thomas Monjalon]

02/10/2018 12:28, Ferruh Yigit:
> On 9/28/2018 5:21 PM, Thomas Monjalon wrote:
> > These functions are quite old and are the only available replacement
> > for the deprecated attach/detach functions.
> > 
> > Note: some new functions may (again) replace these hotplug functions,
> > in future, with better parameters.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >  lib/librte_eal/common/eal_common_dev.c  |  7 ++++---
> >  lib/librte_eal/common/include/rte_dev.h | 11 ++---------
> >  lib/librte_eal/rte_eal_version.map      |  4 ++--
> 
> Can remove "-DALLOW_EXPERIMENTAL_API" (or "allow_experimental_apis = true" for
> meson) from drivers/raw/ifpga_rawdev when these APIs are not experimental anymore.
> For reference: https://patches.dpdk.org/patch/45836/

Yes we could remove this flag in this patch.
But the next series would add it again:
	https://patches.dpdk.org/patch/45601/

So I think it is not worth for now.

> It is easy to know when to add "-DALLOW_EXPERIMENTAL_API" but it is hard to know
> when to remove one, some helper should be good there.

Yes, we should try to clean up those flags regularly after each -rc1 release.
But we don't have any tooling for that.

[dpdk-dev] [PATCH v3 4/4] eal: simplify parameters of hotplug functions

[Thomas Monjalon]

All information about a device to probe can be grouped
in a common string, which is what we usually call devargs.
An application should not have to parse this string before
calling the EAL probe function.
And the syntax could evolve to be more complex and support
matching multiple devices in one string.
That's why the bus name and device name should be removed from
rte_eal_hotplug_add().
Instead of changing this function, a simpler one is added
and used in the old one, which may be deprecated later.

When removing a device, we already know its rte_device handle
which can be directly passed as parameter of rte_eal_hotplug_remove().
If the rte_device is not known, it can be retrieved with the devargs,
by iterating in the device list (future RTE_DEV_FOREACH()).
Similarly to the probing case, a new function is added
and used in the old one, which may be deprecated later.
The new function is used in failsafe, because the replacement is easy.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/net/failsafe/failsafe_eal.c     |  3 +-
 drivers/net/failsafe/failsafe_ether.c   |  3 +-
 lib/librte_eal/common/eal_common_dev.c  | 76 ++++++++++++++++---------
 lib/librte_eal/common/include/rte_dev.h | 30 +++++++++-
 lib/librte_eal/rte_eal_version.map      |  2 +
 5 files changed, 80 insertions(+), 34 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index ce1633f13..8a888b1ff 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -144,8 +144,7 @@ fs_bus_uninit(struct rte_eth_dev *dev)
 	int ret = 0;
 
 	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
-		sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
-							sdev->dev->name);
+		sdev_ret = rte_dev_remove(sdev->dev);
 		if (sdev_ret) {
 			ERROR("Failed to remove requested device %s (err: %d)",
 			      sdev->dev->name, sdev_ret);
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 5b5cb3b49..cd7b97e1f 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -265,8 +265,7 @@ fs_dev_remove(struct sub_device *sdev)
 		sdev->state = DEV_PROBED;
 		/* fallthrough */
 	case DEV_PROBED:
-		ret = rte_eal_hotplug_remove(sdev->bus->name,
-					     sdev->dev->name);
+		ret = rte_dev_remove(sdev->dev);
 		if (ret) {
 			ERROR("Bus detach failed for sub_device %u",
 			      SUB_ID(sdev));
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index a9be58edf..b40e4c0d0 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -129,46 +129,57 @@ int rte_eal_dev_detach(struct rte_device *dev)
 
 int
 rte_eal_hotplug_add(const char *busname, const char *devname,
-		    const char *devargs)
+		    const char *drvargs)
+{
+	char *devargs = NULL;
+	int size, length = -1;
+
+	do { /* 2 iterations: first is to know string length */
+		size = length + 1;
+		length = snprintf(devargs, size, "%s:%s,%s", busname, devname, drvargs);
+		if (length >= size)
+			devargs = malloc(length + 1);
+		if (devargs == NULL)
+			return -ENOMEM;
+	} while (size == 0);
+
+	return rte_dev_probe(devargs);
+}
+
+int __rte_experimental
+rte_dev_probe(const char *devargs)
 {
-	struct rte_bus *bus;
 	struct rte_device *dev;
 	struct rte_devargs *da;
 	int ret;
 
-	bus = rte_bus_find_by_name(busname);
-	if (bus == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n", busname);
-		return -ENOENT;
-	}
-
-	if (bus->plug == NULL) {
-		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
-			bus->name);
-		return -ENOTSUP;
-	}
-
 	da = calloc(1, sizeof(*da));
 	if (da == NULL)
 		return -ENOMEM;
 
-	ret = rte_devargs_parsef(da, "%s:%s,%s",
-				 busname, devname, devargs);
+	ret = rte_devargs_parse(da, devargs);
 	if (ret)
 		goto err_devarg;
 
+	if (da->bus->plug == NULL) {
+		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
+			da->bus->name);
+		ret = -ENOTSUP;
+		goto err_devarg;
+	}
+
 	ret = rte_devargs_insert(da);
 	if (ret)
 		goto err_devarg;
 
-	ret = bus->scan();
+	ret = da->bus->scan();
 	if (ret)
 		goto err_devarg;
 
-	dev = bus->find_device(NULL, cmp_dev_name, devname);
+	dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find device (%s)\n",
-			devname);
+			da->name);
 		ret = -ENODEV;
 		goto err_devarg;
 	}
@@ -178,7 +189,7 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 		return -EEXIST;
 	}
 
-	ret = bus->plug(dev);
+	ret = da->bus->plug(dev);
 	if (ret) {
 		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
 			dev->name);
@@ -197,9 +208,8 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 int
 rte_eal_hotplug_remove(const char *busname, const char *devname)
 {
-	struct rte_bus *bus;
 	struct rte_device *dev;
-	int ret;
+	struct rte_bus *bus;
 
 	bus = rte_bus_find_by_name(busname);
 	if (bus == NULL) {
@@ -207,23 +217,33 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 		return -ENOENT;
 	}
 
-	if (bus->unplug == NULL) {
-		RTE_LOG(ERR, EAL, "Function unplug not supported by bus (%s)\n",
-			bus->name);
-		return -ENOTSUP;
-	}
-
 	dev = bus->find_device(NULL, cmp_dev_name, devname);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find plugged device (%s)\n", devname);
 		return -EINVAL;
 	}
 
+	return rte_dev_remove(dev);
+}
+
+int __rte_experimental
+rte_dev_remove(struct rte_device *dev)
+{
+	struct rte_bus *bus;
+	int ret;
+
 	if (dev->driver == NULL) {
 		RTE_LOG(ERR, EAL, "Device is already unplugged\n");
 		return -ENOENT;
 	}
 
+	bus = dev->devargs->bus;
+	if (bus->unplug == NULL) {
+		RTE_LOG(ERR, EAL, "Function unplug not supported by bus (%s)\n",
+			bus->name);
+		return -ENOTSUP;
+	}
+
 	ret = bus->unplug(dev);
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index d440c4e58..ee77e4006 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -196,13 +196,26 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @param devname
  *   The device name. Based on this device name, eal will identify a driver
  *   capable of handling it and pass it to the driver probing function.
- * @param devargs
+ * @param drvargs
  *   Device arguments to be passed to the driver.
  * @return
  *   0 on success, negative on error.
  */
 int rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs);
+			const char *drvargs);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Add matching devices.
+ *
+ * @param devargs
+ *   Device arguments including bus, class and driver properties.
+ * @return
+ *   0 on success, negative on error.
+ */
+int __rte_experimental rte_dev_probe(const char *devargs);
 
 /**
  * Hotplug remove a given device from a specific bus.
@@ -216,6 +229,19 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
  */
 int rte_eal_hotplug_remove(const char *busname, const char *devname);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Remove one device.
+ *
+ * @param dev
+ *   Data structure of the device to remove.
+ * @return
+ *   0 on success, negative on error.
+ */
+int __rte_experimental rte_dev_remove(struct rte_device *dev);
+
 /**
  * Device comparison function.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 6bff37f4b..2ea7a870a 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -281,6 +281,8 @@ EXPERIMENTAL {
 	rte_dev_event_monitor_stop;
 	rte_dev_iterator_init;
 	rte_dev_iterator_next;
+	rte_dev_probe;
+	rte_dev_remove;
 	rte_devargs_add;
 	rte_devargs_dump;
 	rte_devargs_insert;
-- 
2.19.0

Re: [dpdk-dev] [PATCH v3 4/4] eal: simplify parameters of hotplug functions

[Andrew Rybchenko]

On 9/28/18 7:21 PM, Thomas Monjalon wrote:
> All information about a device to probe can be grouped
> in a common string, which is what we usually call devargs.
> An application should not have to parse this string before
> calling the EAL probe function.
> And the syntax could evolve to be more complex and support
> matching multiple devices in one string.
> That's why the bus name and device name should be removed from
> rte_eal_hotplug_add().
> Instead of changing this function, a simpler one is added
> and used in the old one, which may be deprecated later.
>
> When removing a device, we already know its rte_device handle
> which can be directly passed as parameter of rte_eal_hotplug_remove().
> If the rte_device is not known, it can be retrieved with the devargs,
> by iterating in the device list (future RTE_DEV_FOREACH()).
> Similarly to the probing case, a new function is added
> and used in the old one, which may be deprecated later.
> The new function is used in failsafe, because the replacement is easy.
>
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

Minor memory leak below, when fixed:

Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

<...>

> diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
> index a9be58edf..b40e4c0d0 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -129,46 +129,57 @@ int rte_eal_dev_detach(struct rte_device *dev)
>   
>   int
>   rte_eal_hotplug_add(const char *busname, const char *devname,
> -		    const char *devargs)
> +		    const char *drvargs)
> +{
> +	char *devargs = NULL;
> +	int size, length = -1;
> +
> +	do { /* 2 iterations: first is to know string length */
> +		size = length + 1;
> +		length = snprintf(devargs, size, "%s:%s,%s", busname, devname, drvargs);
> +		if (length >= size)
> +			devargs = malloc(length + 1);
> +		if (devargs == NULL)
> +			return -ENOMEM;
> +	} while (size == 0);
> +
> +	return rte_dev_probe(devargs);

I think we should free devargs after the call.

> +}

<...>

Re: [dpdk-dev] [PATCH v3 4/4] eal: simplify parameters of hotplug functions

[Thomas Monjalon]

01/10/2018 13:26, Andrew Rybchenko:
> On 9/28/18 7:21 PM, Thomas Monjalon wrote:
> > All information about a device to probe can be grouped
> > in a common string, which is what we usually call devargs.
> > An application should not have to parse this string before
> > calling the EAL probe function.
> > And the syntax could evolve to be more complex and support
> > matching multiple devices in one string.
> > That's why the bus name and device name should be removed from
> > rte_eal_hotplug_add().
> > Instead of changing this function, a simpler one is added
> > and used in the old one, which may be deprecated later.
> >
> > When removing a device, we already know its rte_device handle
> > which can be directly passed as parameter of rte_eal_hotplug_remove().
> > If the rte_device is not known, it can be retrieved with the devargs,
> > by iterating in the device list (future RTE_DEV_FOREACH()).
> > Similarly to the probing case, a new function is added
> > and used in the old one, which may be deprecated later.
> > The new function is used in failsafe, because the replacement is easy.
> >
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> Minor memory leak below, when fixed:
> 
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
> 
> <...>
> 
> > diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
> > index a9be58edf..b40e4c0d0 100644
> > --- a/lib/librte_eal/common/eal_common_dev.c
> > +++ b/lib/librte_eal/common/eal_common_dev.c
> > @@ -129,46 +129,57 @@ int rte_eal_dev_detach(struct rte_device *dev)
> >   
> >   int
> >   rte_eal_hotplug_add(const char *busname, const char *devname,
> > -		    const char *devargs)
> > +		    const char *drvargs)
> > +{
> > +	char *devargs = NULL;
> > +	int size, length = -1;
> > +
> > +	do { /* 2 iterations: first is to know string length */
> > +		size = length + 1;
> > +		length = snprintf(devargs, size, "%s:%s,%s", busname, devname, drvargs);
> > +		if (length >= size)
> > +			devargs = malloc(length + 1);
> > +		if (devargs == NULL)
> > +			return -ENOMEM;
> > +	} while (size == 0);
> > +
> > +	return rte_dev_probe(devargs);
> 
> I think we should free devargs after the call.

Good catch!
Will fix in v4, thank you.

[dpdk-dev] [PATCH v4 0/4] eal: simplify devargs and hotplug functions

[Thomas Monjalon]

This is a follow-up of an idea presented at Dublin
during the "hotplug talk".

Instead of changing the existing hotplug functions, as in the RFC,
some new experimental functions are added.
The old functions lose their experimental status in order to provide
a non-experimental replacement for deprecated attach/detach functions.

It has been discussed briefly in the latest technical board meeting.


Changes in v4 - after Andrew's review:
  - add API changes in release notes (patches 1 & 2)
  - fix memory leak in rte_eal_hotplug_add (patch 4)

Change in v3:
  - fix null dereferencing in error path (patch 2)


Thomas Monjalon (4):
  devargs: remove deprecated functions
  devargs: simplify parameters of removal function
  eal: remove experimental flag of hotplug functions
  eal: simplify parameters of hotplug functions

 doc/guides/rel_notes/release_18_11.rst      |  8 ++
 drivers/bus/ifpga/ifpga_bus.c               |  5 +-
 drivers/bus/vdev/vdev.c                     |  8 +-
 drivers/net/failsafe/failsafe_eal.c         |  3 +-
 drivers/net/failsafe/failsafe_ether.c       |  3 +-
 lib/librte_eal/common/eal_common_dev.c      | 89 +++++++++++++--------
 lib/librte_eal/common/eal_common_devargs.c  | 41 ++--------
 lib/librte_eal/common/include/rte_dev.h     | 35 ++++++--
 lib/librte_eal/common/include/rte_devargs.h | 81 +------------------
 lib/librte_eal/rte_eal_version.map          | 10 +--
 10 files changed, 111 insertions(+), 172 deletions(-)

-- 
2.19.0

[dpdk-dev] [PATCH v4 1/4] devargs: remove deprecated functions

[Thomas Monjalon]

rte_eal_parse_devargs_str() does not support parsing the bus name
at the start of devargs. So it was renamed and deprecated.

rte_eal_devargs_add(), rte_eal_devargs_type_count() and
rte_eal_devargs_dump() were declared deprecated and had their
implementation body renamed.

All these functions were deprecated in release 18.05.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
The library version is not updated because it should be done
in earlier patch https://patches.dpdk.org/patch/43903/
---
 doc/guides/rel_notes/release_18_11.rst      |  5 ++
 lib/librte_eal/common/eal_common_devargs.c  | 30 ---------
 lib/librte_eal/common/include/rte_devargs.h | 71 ---------------------
 lib/librte_eal/rte_eal_version.map          |  4 --
 4 files changed, 5 insertions(+), 105 deletions(-)

diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 9c00e33cc..ef902b028 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -102,6 +102,11 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* eal: The following devargs functions, which were deprecated in 18.05,
+  are removed in 18.11:
+  ``rte_eal_parse_devargs_str()``, ``rte_eal_devargs_add()``,
+  ``rte_eal_devargs_type_count()``, and ``rte_eal_devargs_dump()``.
+
 * mbuf: The ``__rte_mbuf_raw_free()`` and ``__rte_pktmbuf_prefree_seg()``
   functions were deprecated since 17.05 and are replaced by
   ``rte_mbuf_raw_free()`` and ``rte_pktmbuf_prefree_seg()``.
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index dac2402a4..f63d2c663 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -31,36 +31,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 size_t
 devargs_layer_count(const char *s)
 {
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 097a4ce7b..0eef6e9c4 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -66,36 +66,6 @@ struct rte_devargs {
 	const char *data; /**< Device string storage. */
 };
 
-/**
- * @deprecated
- * 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
- */
-__rte_deprecated
-int rte_eal_parse_devargs_str(const char *devargs_str,
-				char **drvname, char **drvargs);
-
 /**
  * Parse a device string.
  *
@@ -201,23 +171,6 @@ rte_devargs_insert(struct rte_devargs *da);
 __rte_experimental
 int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
 
-/**
- * @deprecated
- * Add a device to the user device list
- * See rte_devargs_parse() for details.
- *
- * @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);
-
 /**
  * Remove a device from the user device list.
  * Its resources are freed.
@@ -251,20 +204,6 @@ __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);
-
 /**
  * This function dumps the list of user device and their arguments.
  *
@@ -274,16 +213,6 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
 __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);
-
 /**
  * Find next rte_devargs matching the provided bus name.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 73282bbb0..3df7f9831 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -19,9 +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_devargs_type_count;
 	rte_eal_get_configuration;
 	rte_eal_get_lcore_state;
 	rte_eal_get_physmem_size;
@@ -32,7 +29,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.19.0

[dpdk-dev] [PATCH v4 2/4] devargs: simplify parameters of removal function

[Thomas Monjalon]

The function rte_devargs_remove(), which is intended to be internal,
can take a devargs structure as argument.
The matching is still using string comparison of bus name and
device name.
It is simpler and may allow a different devargs matching in future.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/rel_notes/release_18_11.rst      |  3 +++
 drivers/bus/ifpga/ifpga_bus.c               |  5 +----
 drivers/bus/vdev/vdev.c                     |  8 ++------
 lib/librte_eal/common/eal_common_dev.c      |  4 ++--
 lib/librte_eal/common/eal_common_devargs.c  | 11 +++++++----
 lib/librte_eal/common/include/rte_devargs.h | 10 +++-------
 6 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index ef902b028..54f475fd7 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -107,6 +107,9 @@ API Changes
   ``rte_eal_parse_devargs_str()``, ``rte_eal_devargs_add()``,
   ``rte_eal_devargs_type_count()``, and ``rte_eal_devargs_dump()``.
 
+* eal: The parameters of the function ``rte_devargs_remove()`` have changed
+  from bus and device names to ``struct rte_devargs``.
+
 * mbuf: The ``__rte_mbuf_raw_free()`` and ``__rte_pktmbuf_prefree_seg()``
   functions were deprecated since 17.05 and are replaced by
   ``rte_mbuf_raw_free()`` and ``rte_pktmbuf_prefree_seg()``.
diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index c54b59db2..3ef035b7e 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -361,7 +361,6 @@ static int
 ifpga_unplug(struct rte_device *dev)
 {
 	struct rte_afu_device *afu_dev = NULL;
-	struct rte_devargs *devargs = NULL;
 	int ret;
 
 	if (dev == NULL)
@@ -371,15 +370,13 @@ ifpga_unplug(struct rte_device *dev)
 	if (!afu_dev)
 		return -ENOENT;
 
-	devargs = dev->devargs;
-
 	ret = ifpga_remove_driver(afu_dev);
 	if (ret)
 		return ret;
 
 	TAILQ_REMOVE(&ifpga_afu_dev_list, afu_dev, next);
 
-	rte_devargs_remove(devargs->bus->name, devargs->name);
+	rte_devargs_remove(dev->devargs);
 	free(afu_dev);
 	return 0;
 
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 69dee89a8..390c2ce70 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -248,7 +248,6 @@ int
 rte_vdev_init(const char *name, const char *args)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
 	int ret;
 
 	rte_spinlock_recursive_lock(&vdev_device_list_lock);
@@ -259,9 +258,8 @@ rte_vdev_init(const char *name, const char *args)
 			if (ret > 0)
 				VDEV_LOG(ERR, "no driver found for %s", name);
 			/* If fails, remove it from vdev list */
-			devargs = dev->device.devargs;
 			TAILQ_REMOVE(&vdev_device_list, dev, next);
-			rte_devargs_remove(devargs->bus->name, devargs->name);
+			rte_devargs_remove(dev->device.devargs);
 			free(dev);
 		}
 	}
@@ -289,7 +287,6 @@ int
 rte_vdev_uninit(const char *name)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
 	int ret;
 
 	if (name == NULL)
@@ -308,8 +305,7 @@ rte_vdev_uninit(const char *name)
 		goto unlock;
 
 	TAILQ_REMOVE(&vdev_device_list, dev, next);
-	devargs = dev->device.devargs;
-	rte_devargs_remove(devargs->bus->name, devargs->name);
+	rte_devargs_remove(dev->device.devargs);
 	free(dev);
 
 unlock:
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 678dbcac7..e81ff4258 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -186,7 +186,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return 0;
 
 err_devarg:
-	if (rte_devargs_remove(busname, devname)) {
+	if (rte_devargs_remove(da)) {
 		free(da->args);
 		free(da);
 	}
@@ -227,7 +227,7 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
 			dev->name);
-	rte_devargs_remove(busname, devname);
+	rte_devargs_remove(dev->devargs);
 	return ret;
 }
 
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index f63d2c663..d8db45a23 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -266,7 +266,7 @@ rte_devargs_insert(struct rte_devargs *da)
 {
 	int ret;
 
-	ret = rte_devargs_remove(da->bus->name, da->name);
+	ret = rte_devargs_remove(da);
 	if (ret < 0)
 		return ret;
 	TAILQ_INSERT_TAIL(&devargs_list, da, next);
@@ -312,14 +312,17 @@ rte_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 }
 
 int __rte_experimental
-rte_devargs_remove(const char *busname, const char *devname)
+rte_devargs_remove(struct rte_devargs *devargs)
 {
 	struct rte_devargs *d;
 	void *tmp;
 
+	if (devargs->bus == NULL)
+		return -1;
+
 	TAILQ_FOREACH_SAFE(d, &devargs_list, next, tmp) {
-		if (strcmp(d->bus->name, busname) == 0 &&
-		    strcmp(d->name, devname) == 0) {
+		if (strcmp(d->bus->name, devargs->bus->name) == 0 &&
+		    strcmp(d->name, devargs->name) == 0) {
 			TAILQ_REMOVE(&devargs_list, d, next);
 			free(d->args);
 			free(d);
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 0eef6e9c4..b1f121f83 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -176,11 +176,8 @@ int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
  * Its resources are freed.
  * If the devargs cannot be found, nothing happens.
  *
- * @param busname
- *   bus name of the devargs to remove.
- *
- * @param devname
- *   device name of the devargs to remove.
+ * @param devargs
+ *   The instance or a copy of devargs to remove.
  *
  * @return
  *   0 on success.
@@ -188,8 +185,7 @@ int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
  *   >0 if the devargs was not within the user device list.
  */
 __rte_experimental
-int rte_devargs_remove(const char *busname,
-		       const char *devname);
+int rte_devargs_remove(struct rte_devargs *devargs);
 
 /**
  * Count the number of user devices of a specified type
-- 
2.19.0

[dpdk-dev] [PATCH v4 3/4] eal: remove experimental flag of hotplug functions

[Thomas Monjalon]

These functions are quite old and are the only available replacement
for the deprecated attach/detach functions.

Note: some new functions may (again) replace these hotplug functions,
in future, with better parameters.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_eal/common/eal_common_dev.c  |  7 ++++---
 lib/librte_eal/common/include/rte_dev.h | 11 ++---------
 lib/librte_eal/rte_eal_version.map      |  4 ++--
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index e81ff4258..a9be58edf 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -127,8 +127,9 @@ int rte_eal_dev_detach(struct rte_device *dev)
 	return ret;
 }
 
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs)
+int
+rte_eal_hotplug_add(const char *busname, const char *devname,
+		    const char *devargs)
 {
 	struct rte_bus *bus;
 	struct rte_device *dev;
@@ -193,7 +194,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return ret;
 }
 
-int __rte_experimental
+int
 rte_eal_hotplug_remove(const char *busname, const char *devname)
 {
 	struct rte_bus *bus;
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index b80a80598..d440c4e58 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -189,9 +189,6 @@ __rte_deprecated
 int rte_eal_dev_detach(struct rte_device *dev);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Hotplug add a given device to a specific bus.
  *
  * @param busname
@@ -204,13 +201,10 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
+int rte_eal_hotplug_add(const char *busname, const char *devname,
 			const char *devargs);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Hotplug remove a given device from a specific bus.
  *
  * @param busname
@@ -220,8 +214,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_remove(const char *busname,
-					  const char *devname);
+int rte_eal_hotplug_remove(const char *busname, const char *devname);
 
 /**
  * Device comparison function.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 3df7f9831..6bff37f4b 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -261,6 +261,8 @@ DPDK_18.08 {
 DPDK_18.11 {
 	global:
 
+	rte_eal_hotplug_add;
+	rte_eal_hotplug_remove;
 	rte_strscpy;
 
 } DPDK_18.08;
@@ -288,8 +290,6 @@ EXPERIMENTAL {
 	rte_devargs_remove;
 	rte_devargs_type_count;
 	rte_eal_cleanup;
-	rte_eal_hotplug_add;
-	rte_eal_hotplug_remove;
 	rte_fbarray_attach;
 	rte_fbarray_destroy;
 	rte_fbarray_detach;
-- 
2.19.0

[dpdk-dev] [PATCH v4 4/4] eal: simplify parameters of hotplug functions

[Thomas Monjalon]

All information about a device to probe can be grouped
in a common string, which is what we usually call devargs.
An application should not have to parse this string before
calling the EAL probe function.
And the syntax could evolve to be more complex and support
matching multiple devices in one string.
That's why the bus name and device name should be removed from
rte_eal_hotplug_add().
Instead of changing this function, a simpler one is added
and used in the old one, which may be deprecated later.

When removing a device, we already know its rte_device handle
which can be directly passed as parameter of rte_eal_hotplug_remove().
If the rte_device is not known, it can be retrieved with the devargs,
by iterating in the device list (future RTE_DEV_FOREACH()).
Similarly to the probing case, a new function is added
and used in the old one, which may be deprecated later.
The new function is used in failsafe, because the replacement is easy.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/failsafe/failsafe_eal.c     |  3 +-
 drivers/net/failsafe/failsafe_ether.c   |  3 +-
 lib/librte_eal/common/eal_common_dev.c  | 80 ++++++++++++++++---------
 lib/librte_eal/common/include/rte_dev.h | 30 +++++++++-
 lib/librte_eal/rte_eal_version.map      |  2 +
 5 files changed, 84 insertions(+), 34 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index ce1633f13..8a888b1ff 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -144,8 +144,7 @@ fs_bus_uninit(struct rte_eth_dev *dev)
 	int ret = 0;
 
 	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
-		sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
-							sdev->dev->name);
+		sdev_ret = rte_dev_remove(sdev->dev);
 		if (sdev_ret) {
 			ERROR("Failed to remove requested device %s (err: %d)",
 			      sdev->dev->name, sdev_ret);
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 51c96f78b..f2512c430 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -282,8 +282,7 @@ fs_dev_remove(struct sub_device *sdev)
 		sdev->state = DEV_PROBED;
 		/* fallthrough */
 	case DEV_PROBED:
-		ret = rte_eal_hotplug_remove(sdev->bus->name,
-					     sdev->dev->name);
+		ret = rte_dev_remove(sdev->dev);
 		if (ret) {
 			ERROR("Bus detach failed for sub_device %u",
 			      SUB_ID(sdev));
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index a9be58edf..713707b9a 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -129,46 +129,61 @@ int rte_eal_dev_detach(struct rte_device *dev)
 
 int
 rte_eal_hotplug_add(const char *busname, const char *devname,
-		    const char *devargs)
+		    const char *drvargs)
 {
-	struct rte_bus *bus;
-	struct rte_device *dev;
-	struct rte_devargs *da;
 	int ret;
+	char *devargs = NULL;
+	int size, length = -1;
 
-	bus = rte_bus_find_by_name(busname);
-	if (bus == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n", busname);
-		return -ENOENT;
-	}
+	do { /* 2 iterations: first is to know string length */
+		size = length + 1;
+		length = snprintf(devargs, size, "%s:%s,%s", busname, devname, drvargs);
+		if (length >= size)
+			devargs = malloc(length + 1);
+		if (devargs == NULL)
+			return -ENOMEM;
+	} while (size == 0);
 
-	if (bus->plug == NULL) {
-		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
-			bus->name);
-		return -ENOTSUP;
-	}
+	ret = rte_dev_probe(devargs);
+
+	free(devargs);
+	return ret;
+}
+
+int __rte_experimental
+rte_dev_probe(const char *devargs)
+{
+	struct rte_device *dev;
+	struct rte_devargs *da;
+	int ret;
 
 	da = calloc(1, sizeof(*da));
 	if (da == NULL)
 		return -ENOMEM;
 
-	ret = rte_devargs_parsef(da, "%s:%s,%s",
-				 busname, devname, devargs);
+	ret = rte_devargs_parse(da, devargs);
 	if (ret)
 		goto err_devarg;
 
+	if (da->bus->plug == NULL) {
+		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
+			da->bus->name);
+		ret = -ENOTSUP;
+		goto err_devarg;
+	}
+
 	ret = rte_devargs_insert(da);
 	if (ret)
 		goto err_devarg;
 
-	ret = bus->scan();
+	ret = da->bus->scan();
 	if (ret)
 		goto err_devarg;
 
-	dev = bus->find_device(NULL, cmp_dev_name, devname);
+	dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find device (%s)\n",
-			devname);
+			da->name);
 		ret = -ENODEV;
 		goto err_devarg;
 	}
@@ -178,7 +193,7 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 		return -EEXIST;
 	}
 
-	ret = bus->plug(dev);
+	ret = da->bus->plug(dev);
 	if (ret) {
 		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
 			dev->name);
@@ -197,9 +212,8 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 int
 rte_eal_hotplug_remove(const char *busname, const char *devname)
 {
-	struct rte_bus *bus;
 	struct rte_device *dev;
-	int ret;
+	struct rte_bus *bus;
 
 	bus = rte_bus_find_by_name(busname);
 	if (bus == NULL) {
@@ -207,23 +221,33 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 		return -ENOENT;
 	}
 
-	if (bus->unplug == NULL) {
-		RTE_LOG(ERR, EAL, "Function unplug not supported by bus (%s)\n",
-			bus->name);
-		return -ENOTSUP;
-	}
-
 	dev = bus->find_device(NULL, cmp_dev_name, devname);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find plugged device (%s)\n", devname);
 		return -EINVAL;
 	}
 
+	return rte_dev_remove(dev);
+}
+
+int __rte_experimental
+rte_dev_remove(struct rte_device *dev)
+{
+	struct rte_bus *bus;
+	int ret;
+
 	if (dev->driver == NULL) {
 		RTE_LOG(ERR, EAL, "Device is already unplugged\n");
 		return -ENOENT;
 	}
 
+	bus = dev->devargs->bus;
+	if (bus->unplug == NULL) {
+		RTE_LOG(ERR, EAL, "Function unplug not supported by bus (%s)\n",
+			bus->name);
+		return -ENOTSUP;
+	}
+
 	ret = bus->unplug(dev);
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index d440c4e58..ee77e4006 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -196,13 +196,26 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @param devname
  *   The device name. Based on this device name, eal will identify a driver
  *   capable of handling it and pass it to the driver probing function.
- * @param devargs
+ * @param drvargs
  *   Device arguments to be passed to the driver.
  * @return
  *   0 on success, negative on error.
  */
 int rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs);
+			const char *drvargs);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Add matching devices.
+ *
+ * @param devargs
+ *   Device arguments including bus, class and driver properties.
+ * @return
+ *   0 on success, negative on error.
+ */
+int __rte_experimental rte_dev_probe(const char *devargs);
 
 /**
  * Hotplug remove a given device from a specific bus.
@@ -216,6 +229,19 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
  */
 int rte_eal_hotplug_remove(const char *busname, const char *devname);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Remove one device.
+ *
+ * @param dev
+ *   Data structure of the device to remove.
+ * @return
+ *   0 on success, negative on error.
+ */
+int __rte_experimental rte_dev_remove(struct rte_device *dev);
+
 /**
  * Device comparison function.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 6bff37f4b..2ea7a870a 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -281,6 +281,8 @@ EXPERIMENTAL {
 	rte_dev_event_monitor_stop;
 	rte_dev_iterator_init;
 	rte_dev_iterator_next;
+	rte_dev_probe;
+	rte_dev_remove;
 	rte_devargs_add;
 	rte_devargs_dump;
 	rte_devargs_insert;
-- 
2.19.0

[dpdk-dev] [PATCH v5 0/5] eal: simplify devargs and hotplug functions

[Thomas Monjalon]

This is a follow-up of an idea presented at Dublin
during the "hotplug talk".

Instead of changing the existing hotplug functions, as in the RFC,
some new experimental functions are added.
The old functions lose their experimental status in order to provide
a non-experimental replacement for deprecated attach/detach functions.

It has been discussed briefly in the latest technical board meeting.


Changes in v5:
  - rte_devargs_remove is fixed in case of null devargs (patch 2)
  - a pointer to the bus is added in rte_device (patch 3)
  - rte_dev_remove is fixed in case of no devargs (patch 5)

Changes in v4 - after Andrew's review:
  - add API changes in release notes (patches 1 & 2)
  - fix memory leak in rte_eal_hotplug_add (patch 4)

Change in v3:
  - fix null dereferencing in error path (patch 2)


Thomas Monjalon (5):
  devargs: remove deprecated functions
  devargs: simplify parameters of removal function
  eal: add bus pointer in device structure
  eal: remove experimental flag of hotplug functions
  eal: simplify parameters of hotplug functions

 doc/guides/rel_notes/release_18_11.rst      | 10 +++
 drivers/bus/dpaa/dpaa_bus.c                 |  2 +
 drivers/bus/fslmc/fslmc_bus.c               |  2 +
 drivers/bus/ifpga/ifpga_bus.c               |  6 +-
 drivers/bus/pci/bsd/pci.c                   |  2 +
 drivers/bus/pci/linux/pci.c                 |  1 +
 drivers/bus/pci/private.h                   |  2 +
 drivers/bus/vdev/vdev.c                     |  9 +--
 drivers/bus/vmbus/linux/vmbus_bus.c         |  1 +
 drivers/bus/vmbus/private.h                 |  3 +
 drivers/net/failsafe/failsafe_eal.c         |  3 +-
 drivers/net/failsafe/failsafe_ether.c       |  3 +-
 lib/librte_eal/common/eal_common_dev.c      | 89 +++++++++++++--------
 lib/librte_eal/common/eal_common_devargs.c  | 41 ++--------
 lib/librte_eal/common/include/rte_dev.h     | 36 +++++++--
 lib/librte_eal/common/include/rte_devargs.h | 81 +------------------
 lib/librte_eal/rte_eal_version.map          | 10 +--
 17 files changed, 128 insertions(+), 173 deletions(-)

-- 
2.19.0

[dpdk-dev] [PATCH v5 1/5] devargs: remove deprecated functions

[Thomas Monjalon]

rte_eal_parse_devargs_str() does not support parsing the bus name
at the start of devargs. So it was renamed and deprecated.

rte_eal_devargs_add(), rte_eal_devargs_type_count() and
rte_eal_devargs_dump() were declared deprecated and had their
implementation body renamed.

All these functions were deprecated in release 18.05.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
The library version is not updated because it should be done
in earlier patch https://patches.dpdk.org/patch/43903/
---
 doc/guides/rel_notes/release_18_11.rst      |  5 ++
 lib/librte_eal/common/eal_common_devargs.c  | 30 ---------
 lib/librte_eal/common/include/rte_devargs.h | 71 ---------------------
 lib/librte_eal/rte_eal_version.map          |  4 --
 4 files changed, 5 insertions(+), 105 deletions(-)

diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 2133a5b9b..4f4420ad3 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -118,6 +118,11 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* eal: The following devargs functions, which were deprecated in 18.05,
+  were removed in 18.11:
+  ``rte_eal_parse_devargs_str()``, ``rte_eal_devargs_add()``,
+  ``rte_eal_devargs_type_count()``, and ``rte_eal_devargs_dump()``.
+
 * mbuf: The ``__rte_mbuf_raw_free()`` and ``__rte_pktmbuf_prefree_seg()``
   functions were deprecated since 17.05 and are replaced by
   ``rte_mbuf_raw_free()`` and ``rte_pktmbuf_prefree_seg()``.
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 88afece10..5cb5c624d 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -28,36 +28,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 size_t
 devargs_layer_count(const char *s)
 {
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 097a4ce7b..0eef6e9c4 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -66,36 +66,6 @@ struct rte_devargs {
 	const char *data; /**< Device string storage. */
 };
 
-/**
- * @deprecated
- * 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
- */
-__rte_deprecated
-int rte_eal_parse_devargs_str(const char *devargs_str,
-				char **drvname, char **drvargs);
-
 /**
  * Parse a device string.
  *
@@ -201,23 +171,6 @@ rte_devargs_insert(struct rte_devargs *da);
 __rte_experimental
 int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
 
-/**
- * @deprecated
- * Add a device to the user device list
- * See rte_devargs_parse() for details.
- *
- * @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);
-
 /**
  * Remove a device from the user device list.
  * Its resources are freed.
@@ -251,20 +204,6 @@ __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);
-
 /**
  * This function dumps the list of user device and their arguments.
  *
@@ -274,16 +213,6 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
 __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);
-
 /**
  * Find next rte_devargs matching the provided bus name.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 73282bbb0..3df7f9831 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -19,9 +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_devargs_type_count;
 	rte_eal_get_configuration;
 	rte_eal_get_lcore_state;
 	rte_eal_get_physmem_size;
@@ -32,7 +29,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.19.0

Re: [dpdk-dev] [PATCH v5 1/5] devargs: remove deprecated functions

[Gaëtan Rivet]

Hi Thomas,

On Thu, Oct 04, 2018 at 01:10:42AM +0200, Thomas Monjalon wrote:
> rte_eal_parse_devargs_str() does not support parsing the bus name
> at the start of devargs. So it was renamed and deprecated.
> 
> rte_eal_devargs_add(), rte_eal_devargs_type_count() and
> rte_eal_devargs_dump() were declared deprecated and had their
> implementation body renamed.
> 
> All these functions were deprecated in release 18.05.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

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

> ---
> The library version is not updated because it should be done
> in earlier patch https://patches.dpdk.org/patch/43903/
> ---
>  doc/guides/rel_notes/release_18_11.rst      |  5 ++
>  lib/librte_eal/common/eal_common_devargs.c  | 30 ---------
>  lib/librte_eal/common/include/rte_devargs.h | 71 ---------------------
>  lib/librte_eal/rte_eal_version.map          |  4 --
>  4 files changed, 5 insertions(+), 105 deletions(-)
> 

-- 
Gaëtan Rivet
6WIND

[dpdk-dev] [PATCH v5 2/5] devargs: simplify parameters of removal function

[Thomas Monjalon]

The function rte_devargs_remove(), which is intended to be internal,
can take a devargs structure as argument.
The matching is still using string comparison of bus name and
device name.
It is simpler and may allow a different devargs matching in future.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 doc/guides/rel_notes/release_18_11.rst      |  3 +++
 drivers/bus/ifpga/ifpga_bus.c               |  5 +----
 drivers/bus/vdev/vdev.c                     |  8 ++------
 lib/librte_eal/common/eal_common_dev.c      |  4 ++--
 lib/librte_eal/common/eal_common_devargs.c  | 11 +++++++----
 lib/librte_eal/common/include/rte_devargs.h | 10 +++-------
 6 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 4f4420ad3..d534bb71c 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -123,6 +123,9 @@ API Changes
   ``rte_eal_parse_devargs_str()``, ``rte_eal_devargs_add()``,
   ``rte_eal_devargs_type_count()``, and ``rte_eal_devargs_dump()``.
 
+* eal: The parameters of the function ``rte_devargs_remove()`` have changed
+  from bus and device names to ``struct rte_devargs``.
+
 * mbuf: The ``__rte_mbuf_raw_free()`` and ``__rte_pktmbuf_prefree_seg()``
   functions were deprecated since 17.05 and are replaced by
   ``rte_mbuf_raw_free()`` and ``rte_pktmbuf_prefree_seg()``.
diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index c54b59db2..3ef035b7e 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -361,7 +361,6 @@ static int
 ifpga_unplug(struct rte_device *dev)
 {
 	struct rte_afu_device *afu_dev = NULL;
-	struct rte_devargs *devargs = NULL;
 	int ret;
 
 	if (dev == NULL)
@@ -371,15 +370,13 @@ ifpga_unplug(struct rte_device *dev)
 	if (!afu_dev)
 		return -ENOENT;
 
-	devargs = dev->devargs;
-
 	ret = ifpga_remove_driver(afu_dev);
 	if (ret)
 		return ret;
 
 	TAILQ_REMOVE(&ifpga_afu_dev_list, afu_dev, next);
 
-	rte_devargs_remove(devargs->bus->name, devargs->name);
+	rte_devargs_remove(dev->devargs);
 	free(afu_dev);
 	return 0;
 
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index ef3ad6d99..efca962f7 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -249,7 +249,6 @@ int
 rte_vdev_init(const char *name, const char *args)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
 	int ret;
 
 	rte_spinlock_recursive_lock(&vdev_device_list_lock);
@@ -260,9 +259,8 @@ rte_vdev_init(const char *name, const char *args)
 			if (ret > 0)
 				VDEV_LOG(ERR, "no driver found for %s", name);
 			/* If fails, remove it from vdev list */
-			devargs = dev->device.devargs;
 			TAILQ_REMOVE(&vdev_device_list, dev, next);
-			rte_devargs_remove(devargs->bus->name, devargs->name);
+			rte_devargs_remove(dev->device.devargs);
 			free(dev);
 		}
 	}
@@ -290,7 +288,6 @@ int
 rte_vdev_uninit(const char *name)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
 	int ret;
 
 	if (name == NULL)
@@ -309,8 +306,7 @@ rte_vdev_uninit(const char *name)
 		goto unlock;
 
 	TAILQ_REMOVE(&vdev_device_list, dev, next);
-	devargs = dev->device.devargs;
-	rte_devargs_remove(devargs->bus->name, devargs->name);
+	rte_devargs_remove(dev->device.devargs);
 	free(dev);
 
 unlock:
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 678dbcac7..e1d9e8ec7 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -186,7 +186,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return 0;
 
 err_devarg:
-	if (rte_devargs_remove(busname, devname)) {
+	if (rte_devargs_remove(da) != 0) {
 		free(da->args);
 		free(da);
 	}
@@ -227,7 +227,7 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
 			dev->name);
-	rte_devargs_remove(busname, devname);
+	rte_devargs_remove(dev->devargs);
 	return ret;
 }
 
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 5cb5c624d..69e9e32e9 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -263,7 +263,7 @@ rte_devargs_insert(struct rte_devargs *da)
 {
 	int ret;
 
-	ret = rte_devargs_remove(da->bus->name, da->name);
+	ret = rte_devargs_remove(da);
 	if (ret < 0)
 		return ret;
 	TAILQ_INSERT_TAIL(&devargs_list, da, next);
@@ -309,14 +309,17 @@ rte_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 }
 
 int __rte_experimental
-rte_devargs_remove(const char *busname, const char *devname)
+rte_devargs_remove(struct rte_devargs *devargs)
 {
 	struct rte_devargs *d;
 	void *tmp;
 
+	if (devargs == NULL || devargs->bus == NULL)
+		return -1;
+
 	TAILQ_FOREACH_SAFE(d, &devargs_list, next, tmp) {
-		if (strcmp(d->bus->name, busname) == 0 &&
-		    strcmp(d->name, devname) == 0) {
+		if (strcmp(d->bus->name, devargs->bus->name) == 0 &&
+		    strcmp(d->name, devargs->name) == 0) {
 			TAILQ_REMOVE(&devargs_list, d, next);
 			free(d->args);
 			free(d);
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 0eef6e9c4..b1f121f83 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -176,11 +176,8 @@ int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
  * Its resources are freed.
  * If the devargs cannot be found, nothing happens.
  *
- * @param busname
- *   bus name of the devargs to remove.
- *
- * @param devname
- *   device name of the devargs to remove.
+ * @param devargs
+ *   The instance or a copy of devargs to remove.
  *
  * @return
  *   0 on success.
@@ -188,8 +185,7 @@ int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
  *   >0 if the devargs was not within the user device list.
  */
 __rte_experimental
-int rte_devargs_remove(const char *busname,
-		       const char *devname);
+int rte_devargs_remove(struct rte_devargs *devargs);
 
 /**
  * Count the number of user devices of a specified type
-- 
2.19.0

Re: [dpdk-dev] [PATCH v5 2/5] devargs: simplify parameters of removal function

[Gaëtan Rivet]

On Thu, Oct 04, 2018 at 01:10:43AM +0200, Thomas Monjalon wrote:
> The function rte_devargs_remove(), which is intended to be internal,
> can take a devargs structure as argument.
> The matching is still using string comparison of bus name and
> device name.
> It is simpler and may allow a different devargs matching in future.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  doc/guides/rel_notes/release_18_11.rst      |  3 +++
>  drivers/bus/ifpga/ifpga_bus.c               |  5 +----
>  drivers/bus/vdev/vdev.c                     |  8 ++------
>  lib/librte_eal/common/eal_common_dev.c      |  4 ++--
>  lib/librte_eal/common/eal_common_devargs.c  | 11 +++++++----
>  lib/librte_eal/common/include/rte_devargs.h | 10 +++-------
>  6 files changed, 18 insertions(+), 23 deletions(-)
> 

-- 
Gaëtan Rivet
6WIND

[dpdk-dev] [PATCH v5 3/5] eal: add bus pointer in device structure

[Thomas Monjalon]

When a device is added with a devargs (hotplug or whitelist),
the bus pointer can be retrieved via its devargs.
But there is no such devargs.bus in case of standard scan.

A pointer to the rte_bus handle is added to rte_device.
When a device is allocated (during a scan),
the pointer to its bus is assigned.

It will make possible to remove a rte_device,
using the function pointer from its bus.

The function rte_bus_find_by_device() becomes useless,
and may be removed later.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 doc/guides/rel_notes/release_18_11.rst  | 2 ++
 drivers/bus/dpaa/dpaa_bus.c             | 2 ++
 drivers/bus/fslmc/fslmc_bus.c           | 2 ++
 drivers/bus/ifpga/ifpga_bus.c           | 1 +
 drivers/bus/pci/bsd/pci.c               | 2 ++
 drivers/bus/pci/linux/pci.c             | 1 +
 drivers/bus/pci/private.h               | 2 ++
 drivers/bus/vdev/vdev.c                 | 1 +
 drivers/bus/vmbus/linux/vmbus_bus.c     | 1 +
 drivers/bus/vmbus/private.h             | 3 +++
 lib/librte_eal/common/include/rte_dev.h | 1 +
 11 files changed, 18 insertions(+)

diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index d534bb71c..2c6791e5e 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -164,6 +164,8 @@ ABI Changes
        ``rte_config`` structure on account of improving DPDK usability when
        using either ``--legacy-mem`` or ``--single-file-segments`` flags.
 
+* eal: The structure ``rte_device`` got a new field to reference a ``rte_bus``.
+
 
 Removed Items
 -------------
diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 49cd04dbb..138e0f98d 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -165,6 +165,8 @@ dpaa_create_device_list(void)
 			goto cleanup;
 		}
 
+		dev->device.bus = &rte_dpaa_bus.bus;
+
 		cfg = &dpaa_netcfg->port_cfg[i];
 		fman_intf = cfg->fman_if;
 
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index bfe81e236..960f55071 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -161,6 +161,8 @@ scan_one_fslmc_device(char *dev_name)
 		return -ENOMEM;
 	}
 
+	dev->device.bus = &rte_fslmc_bus.bus;
+
 	/* Parse the device name and ID */
 	t_ptr = strtok(dup_dev_name, ".");
 	if (!t_ptr) {
diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index 3ef035b7e..80663328a 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -142,6 +142,7 @@ ifpga_scan_one(struct rte_rawdev *rawdev,
 	if (!afu_dev)
 		goto end;
 
+	afu_dev->device.bus = &rte_ifpga_bus;
 	afu_dev->device.devargs = devargs;
 	afu_dev->device.numa_node = SOCKET_ID_ANY;
 	afu_dev->device.name = devargs->name;
diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
index 655b34b7e..40641cad4 100644
--- a/drivers/bus/pci/bsd/pci.c
+++ b/drivers/bus/pci/bsd/pci.c
@@ -223,6 +223,8 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 	}
 
 	memset(dev, 0, sizeof(*dev));
+	dev->device.bus = &rte_pci_bus.bus;
+
 	dev->addr.domain = conf->pc_sel.pc_domain;
 	dev->addr.bus = conf->pc_sel.pc_bus;
 	dev->addr.devid = conf->pc_sel.pc_dev;
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 04648ac93..e31bbb370 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -228,6 +228,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 		return -1;
 
 	memset(dev, 0, sizeof(*dev));
+	dev->device.bus = &rte_pci_bus.bus;
 	dev->addr = *addr;
 
 	/* get vendor id */
diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h
index 0e689fa74..04bffa6e7 100644
--- a/drivers/bus/pci/private.h
+++ b/drivers/bus/pci/private.h
@@ -15,6 +15,8 @@ extern struct rte_pci_bus rte_pci_bus;
 struct rte_pci_driver;
 struct rte_pci_device;
 
+extern struct rte_pci_bus rte_pci_bus;
+
 /**
  * Probe the PCI bus
  *
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index efca962f7..0142fb2c8 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -456,6 +456,7 @@ vdev_scan(void)
 			continue;
 		}
 
+		dev->device.bus = &rte_vdev_bus;
 		dev->device.devargs = devargs;
 		dev->device.numa_node = SOCKET_ID_ANY;
 		dev->device.name = devargs->name;
diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 527a6a39f..a4755a387 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -229,6 +229,7 @@ vmbus_scan_one(const char *name)
 	if (dev == NULL)
 		return -1;
 
+	dev->device.bus = &rte_vmbus_bus.bus;
 	dev->device.name = strdup(name);
 	if (!dev->device.name)
 		goto error;
diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h
index f2022a68c..211127dd8 100644
--- a/drivers/bus/vmbus/private.h
+++ b/drivers/bus/vmbus/private.h
@@ -10,11 +10,14 @@
 #include <sys/uio.h>
 #include <rte_log.h>
 #include <rte_vmbus_reg.h>
+#include <rte_bus_vmbus.h>
 
 #ifndef PAGE_SIZE
 #define PAGE_SIZE	4096
 #endif
 
+extern struct rte_vmbus_bus rte_vmbus_bus;
+
 extern int vmbus_logtype_bus;
 #define VMBUS_LOG(level, fmt, args...) \
 	rte_log(RTE_LOG_ ## level, vmbus_logtype_bus, "%s(): " fmt "\n", \
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index b80a80598..d82cba847 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -157,6 +157,7 @@ struct rte_device {
 	TAILQ_ENTRY(rte_device) next; /**< Next device */
 	const char *name;             /**< Device name */
 	const struct rte_driver *driver;/**< Associated driver */
+	const struct rte_bus *bus;    /**< Bus handle assigned on scan */
 	int numa_node;                /**< NUMA node connection */
 	struct rte_devargs *devargs;  /**< Device user arguments */
 };
-- 
2.19.0

Re: [dpdk-dev] [PATCH v5 3/5] eal: add bus pointer in device structure

[Gaëtan Rivet]

On Thu, Oct 04, 2018 at 01:10:44AM +0200, Thomas Monjalon wrote:
> When a device is added with a devargs (hotplug or whitelist),
> the bus pointer can be retrieved via its devargs.
> But there is no such devargs.bus in case of standard scan.
> 
> A pointer to the rte_bus handle is added to rte_device.
> When a device is allocated (during a scan),
> the pointer to its bus is assigned.
> 
> It will make possible to remove a rte_device,
> using the function pointer from its bus.
> 
> The function rte_bus_find_by_device() becomes useless,
> and may be removed later.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>

I agree with this change, but I think this can break ABI of
buses defining their structure by composition with rte_device (e.g. PCI
bus). Have you checked ABI?

Personally I don't care, I prefer a clean framework to a littered lib.

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

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH v5 3/5] eal: add bus pointer in device structure

[Thomas Monjalon]

04/10/2018 11:31, Gaëtan Rivet:
> On Thu, Oct 04, 2018 at 01:10:44AM +0200, Thomas Monjalon wrote:
> > When a device is added with a devargs (hotplug or whitelist),
> > the bus pointer can be retrieved via its devargs.
> > But there is no such devargs.bus in case of standard scan.
> > 
> > A pointer to the rte_bus handle is added to rte_device.
> > When a device is allocated (during a scan),
> > the pointer to its bus is assigned.
> > 
> > It will make possible to remove a rte_device,
> > using the function pointer from its bus.
> > 
> > The function rte_bus_find_by_device() becomes useless,
> > and may be removed later.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> 
> I agree with this change, but I think this can break ABI of
> buses defining their structure by composition with rte_device (e.g. PCI
> bus). Have you checked ABI?

Yes I forgot it changes the size of the bus structures.
I can spin a v6 with a bump of ABI version of the bus drivers,
and an additional note in release notes.

> Personally I don't care, I prefer a clean framework to a littered lib.
> 
> Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>

Adding bus drivers maintainers to get more opinions.

[dpdk-dev] [PATCH v5 4/5] eal: remove experimental flag of hotplug functions

[Thomas Monjalon]

These functions are quite old and are the only available replacement
for the deprecated attach/detach functions.

Note: some new functions may (again) replace these hotplug functions,
in future, with better parameters.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 lib/librte_eal/common/eal_common_dev.c  |  7 ++++---
 lib/librte_eal/common/include/rte_dev.h | 11 ++---------
 lib/librte_eal/rte_eal_version.map      |  4 ++--
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index e1d9e8ec7..ce6d145fb 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -127,8 +127,9 @@ int rte_eal_dev_detach(struct rte_device *dev)
 	return ret;
 }
 
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs)
+int
+rte_eal_hotplug_add(const char *busname, const char *devname,
+		    const char *devargs)
 {
 	struct rte_bus *bus;
 	struct rte_device *dev;
@@ -193,7 +194,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return ret;
 }
 
-int __rte_experimental
+int
 rte_eal_hotplug_remove(const char *busname, const char *devname)
 {
 	struct rte_bus *bus;
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index d82cba847..d6c5d48a9 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -190,9 +190,6 @@ __rte_deprecated
 int rte_eal_dev_detach(struct rte_device *dev);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Hotplug add a given device to a specific bus.
  *
  * @param busname
@@ -205,13 +202,10 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
+int rte_eal_hotplug_add(const char *busname, const char *devname,
 			const char *devargs);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Hotplug remove a given device from a specific bus.
  *
  * @param busname
@@ -221,8 +215,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_remove(const char *busname,
-					  const char *devname);
+int rte_eal_hotplug_remove(const char *busname, const char *devname);
 
 /**
  * Device comparison function.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 3df7f9831..6bff37f4b 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -261,6 +261,8 @@ DPDK_18.08 {
 DPDK_18.11 {
 	global:
 
+	rte_eal_hotplug_add;
+	rte_eal_hotplug_remove;
 	rte_strscpy;
 
 } DPDK_18.08;
@@ -288,8 +290,6 @@ EXPERIMENTAL {
 	rte_devargs_remove;
 	rte_devargs_type_count;
 	rte_eal_cleanup;
-	rte_eal_hotplug_add;
-	rte_eal_hotplug_remove;
 	rte_fbarray_attach;
 	rte_fbarray_destroy;
 	rte_fbarray_detach;
-- 
2.19.0

Re: [dpdk-dev] [PATCH v5 4/5] eal: remove experimental flag of hotplug functions

[Gaëtan Rivet]

On Thu, Oct 04, 2018 at 01:10:45AM +0200, Thomas Monjalon wrote:
> These functions are quite old and are the only available replacement
> for the deprecated attach/detach functions.
> 
> Note: some new functions may (again) replace these hotplug functions,
> in future, with better parameters.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

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

> ---
>  lib/librte_eal/common/eal_common_dev.c  |  7 ++++---
>  lib/librte_eal/common/include/rte_dev.h | 11 ++---------
>  lib/librte_eal/rte_eal_version.map      |  4 ++--
>  3 files changed, 8 insertions(+), 14 deletions(-)
> 

-- 
Gaëtan Rivet
6WIND

[dpdk-dev] [PATCH v5 5/5] eal: simplify parameters of hotplug functions

[Thomas Monjalon]

All information about a device to probe can be grouped
in a common string, which is what we usually call devargs.
An application should not have to parse this string before
calling the EAL probe function.
And the syntax could evolve to be more complex and support
matching multiple devices in one string.
That's why the bus name and device name should be removed from
rte_eal_hotplug_add().
Instead of changing this function, a simpler one is added
and used in the old one, which may be deprecated later.

When removing a device, we already know its rte_device handle
which can be directly passed as parameter of rte_eal_hotplug_remove().
If the rte_device is not known, it can be retrieved with the devargs,
by iterating in the device list (future RTE_DEV_FOREACH()).
Similarly to the probing case, a new function is added
and used in the old one, which may be deprecated later.
The new function is used in failsafe, because the replacement is easy.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
---
 drivers/net/failsafe/failsafe_eal.c     |  3 +-
 drivers/net/failsafe/failsafe_ether.c   |  3 +-
 lib/librte_eal/common/eal_common_dev.c  | 80 ++++++++++++++++---------
 lib/librte_eal/common/include/rte_dev.h | 30 +++++++++-
 lib/librte_eal/rte_eal_version.map      |  2 +
 5 files changed, 83 insertions(+), 35 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index ce1633f13..8a888b1ff 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -144,8 +144,7 @@ fs_bus_uninit(struct rte_eth_dev *dev)
 	int ret = 0;
 
 	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
-		sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
-							sdev->dev->name);
+		sdev_ret = rte_dev_remove(sdev->dev);
 		if (sdev_ret) {
 			ERROR("Failed to remove requested device %s (err: %d)",
 			      sdev->dev->name, sdev_ret);
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 51c96f78b..f2512c430 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -282,8 +282,7 @@ fs_dev_remove(struct sub_device *sdev)
 		sdev->state = DEV_PROBED;
 		/* fallthrough */
 	case DEV_PROBED:
-		ret = rte_eal_hotplug_remove(sdev->bus->name,
-					     sdev->dev->name);
+		ret = rte_dev_remove(sdev->dev);
 		if (ret) {
 			ERROR("Bus detach failed for sub_device %u",
 			      SUB_ID(sdev));
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index ce6d145fb..5dcc1403f 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -129,46 +129,61 @@ int rte_eal_dev_detach(struct rte_device *dev)
 
 int
 rte_eal_hotplug_add(const char *busname, const char *devname,
-		    const char *devargs)
+		    const char *drvargs)
 {
-	struct rte_bus *bus;
-	struct rte_device *dev;
-	struct rte_devargs *da;
 	int ret;
+	char *devargs = NULL;
+	int size, length = -1;
 
-	bus = rte_bus_find_by_name(busname);
-	if (bus == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n", busname);
-		return -ENOENT;
-	}
+	do { /* 2 iterations: first is to know string length */
+		size = length + 1;
+		length = snprintf(devargs, size, "%s:%s,%s", busname, devname, drvargs);
+		if (length >= size)
+			devargs = malloc(length + 1);
+		if (devargs == NULL)
+			return -ENOMEM;
+	} while (size == 0);
 
-	if (bus->plug == NULL) {
-		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
-			bus->name);
-		return -ENOTSUP;
-	}
+	ret = rte_dev_probe(devargs);
+
+	free(devargs);
+	return ret;
+}
+
+int __rte_experimental
+rte_dev_probe(const char *devargs)
+{
+	struct rte_device *dev;
+	struct rte_devargs *da;
+	int ret;
 
 	da = calloc(1, sizeof(*da));
 	if (da == NULL)
 		return -ENOMEM;
 
-	ret = rte_devargs_parsef(da, "%s:%s,%s",
-				 busname, devname, devargs);
+	ret = rte_devargs_parse(da, devargs);
 	if (ret)
 		goto err_devarg;
 
+	if (da->bus->plug == NULL) {
+		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
+			da->bus->name);
+		ret = -ENOTSUP;
+		goto err_devarg;
+	}
+
 	ret = rte_devargs_insert(da);
 	if (ret)
 		goto err_devarg;
 
-	ret = bus->scan();
+	ret = da->bus->scan();
 	if (ret)
 		goto err_devarg;
 
-	dev = bus->find_device(NULL, cmp_dev_name, devname);
+	dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find device (%s)\n",
-			devname);
+			da->name);
 		ret = -ENODEV;
 		goto err_devarg;
 	}
@@ -178,7 +193,7 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 		return -EEXIST;
 	}
 
-	ret = bus->plug(dev);
+	ret = dev->bus->plug(dev);
 	if (ret) {
 		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
 			dev->name);
@@ -197,9 +212,8 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 int
 rte_eal_hotplug_remove(const char *busname, const char *devname)
 {
-	struct rte_bus *bus;
 	struct rte_device *dev;
-	int ret;
+	struct rte_bus *bus;
 
 	bus = rte_bus_find_by_name(busname);
 	if (bus == NULL) {
@@ -207,24 +221,32 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 		return -ENOENT;
 	}
 
-	if (bus->unplug == NULL) {
-		RTE_LOG(ERR, EAL, "Function unplug not supported by bus (%s)\n",
-			bus->name);
-		return -ENOTSUP;
-	}
-
 	dev = bus->find_device(NULL, cmp_dev_name, devname);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find plugged device (%s)\n", devname);
 		return -EINVAL;
 	}
 
+	return rte_dev_remove(dev);
+}
+
+int __rte_experimental
+rte_dev_remove(struct rte_device *dev)
+{
+	int ret;
+
 	if (dev->driver == NULL) {
 		RTE_LOG(ERR, EAL, "Device is already unplugged\n");
 		return -ENOENT;
 	}
 
-	ret = bus->unplug(dev);
+	if (dev->bus->unplug == NULL) {
+		RTE_LOG(ERR, EAL, "Function unplug not supported by bus (%s)\n",
+			dev->bus->name);
+		return -ENOTSUP;
+	}
+
+	ret = dev->bus->unplug(dev);
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
 			dev->name);
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index d6c5d48a9..036180ff3 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -197,13 +197,26 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @param devname
  *   The device name. Based on this device name, eal will identify a driver
  *   capable of handling it and pass it to the driver probing function.
- * @param devargs
+ * @param drvargs
  *   Device arguments to be passed to the driver.
  * @return
  *   0 on success, negative on error.
  */
 int rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs);
+			const char *drvargs);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Add matching devices.
+ *
+ * @param devargs
+ *   Device arguments including bus, class and driver properties.
+ * @return
+ *   0 on success, negative on error.
+ */
+int __rte_experimental rte_dev_probe(const char *devargs);
 
 /**
  * Hotplug remove a given device from a specific bus.
@@ -217,6 +230,19 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
  */
 int rte_eal_hotplug_remove(const char *busname, const char *devname);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Remove one device.
+ *
+ * @param dev
+ *   Data structure of the device to remove.
+ * @return
+ *   0 on success, negative on error.
+ */
+int __rte_experimental rte_dev_remove(struct rte_device *dev);
+
 /**
  * Device comparison function.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 6bff37f4b..2ea7a870a 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -281,6 +281,8 @@ EXPERIMENTAL {
 	rte_dev_event_monitor_stop;
 	rte_dev_iterator_init;
 	rte_dev_iterator_next;
+	rte_dev_probe;
+	rte_dev_remove;
 	rte_devargs_add;
 	rte_devargs_dump;
 	rte_devargs_insert;
-- 
2.19.0

Re: [dpdk-dev] [PATCH v5 5/5] eal: simplify parameters of hotplug functions

[Gaëtan Rivet]

I much prefer this API,

I have one remark inline however.

On Thu, Oct 04, 2018 at 01:10:46AM +0200, Thomas Monjalon wrote:
> All information about a device to probe can be grouped
> in a common string, which is what we usually call devargs.
> An application should not have to parse this string before
> calling the EAL probe function.
> And the syntax could evolve to be more complex and support
> matching multiple devices in one string.
> That's why the bus name and device name should be removed from
> rte_eal_hotplug_add().
> Instead of changing this function, a simpler one is added
> and used in the old one, which may be deprecated later.
> 
> When removing a device, we already know its rte_device handle
> which can be directly passed as parameter of rte_eal_hotplug_remove().
> If the rte_device is not known, it can be retrieved with the devargs,
> by iterating in the device list (future RTE_DEV_FOREACH()).
> Similarly to the probing case, a new function is added
> and used in the old one, which may be deprecated later.
> The new function is used in failsafe, because the replacement is easy.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>

With the following remark being taken care of,

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

> ---
>  drivers/net/failsafe/failsafe_eal.c     |  3 +-
>  drivers/net/failsafe/failsafe_ether.c   |  3 +-
>  lib/librte_eal/common/eal_common_dev.c  | 80 ++++++++++++++++---------
>  lib/librte_eal/common/include/rte_dev.h | 30 +++++++++-
>  lib/librte_eal/rte_eal_version.map      |  2 +
>  5 files changed, 83 insertions(+), 35 deletions(-)
> 

<snip>

> diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
> index ce6d145fb..5dcc1403f 100644
> --- a/lib/librte_eal/common/eal_common_dev.c
> +++ b/lib/librte_eal/common/eal_common_dev.c
> @@ -129,46 +129,61 @@ int rte_eal_dev_detach(struct rte_device *dev)
>  
>  int
>  rte_eal_hotplug_add(const char *busname, const char *devname,
> -		    const char *devargs)
> +		    const char *drvargs)
>  {
> -	struct rte_bus *bus;
> -	struct rte_device *dev;
> -	struct rte_devargs *da;
>  	int ret;
> +	char *devargs = NULL;
> +	int size, length = -1;
>  
> -	bus = rte_bus_find_by_name(busname);
> -	if (bus == NULL) {
> -		RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n", busname);
> -		return -ENOENT;
> -	}
> +	do { /* 2 iterations: first is to know string length */
> +		size = length + 1;
> +		length = snprintf(devargs, size, "%s:%s,%s", busname, devname, drvargs);
> +		if (length >= size)
> +			devargs = malloc(length + 1);
> +		if (devargs == NULL)
> +			return -ENOMEM;
> +	} while (size == 0);

I don't see a good reason for writing it this way.
You use an additional state (size) and complicate something that is
pretty straightforward. To make sure no error was written here, I had to
do step-by-step careful reading, this should not be required by clean
code.

You should remove this loop, which then allow removing size, and forces using
simple code-flow.

>  
> -	if (bus->plug == NULL) {
> -		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
> -			bus->name);
> -		return -ENOTSUP;
> -	}
> +	ret = rte_dev_probe(devargs);
> +
> +	free(devargs);
> +	return ret;
> +}
> +
> +int __rte_experimental
> +rte_dev_probe(const char *devargs)
> +{
> +	struct rte_device *dev;
> +	struct rte_devargs *da;
> +	int ret;
>  
>  	da = calloc(1, sizeof(*da));
>  	if (da == NULL)
>  		return -ENOMEM;
>  
> -	ret = rte_devargs_parsef(da, "%s:%s,%s",
> -				 busname, devname, devargs);
> +	ret = rte_devargs_parse(da, devargs);
>  	if (ret)
>  		goto err_devarg;
>  
> +	if (da->bus->plug == NULL) {
> +		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
> +			da->bus->name);
> +		ret = -ENOTSUP;
> +		goto err_devarg;
> +	}
> +
>  	ret = rte_devargs_insert(da);
>  	if (ret)
>  		goto err_devarg;
>  
> -	ret = bus->scan();
> +	ret = da->bus->scan();
>  	if (ret)
>  		goto err_devarg;
>  
> -	dev = bus->find_device(NULL, cmp_dev_name, devname);
> +	dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
>  	if (dev == NULL) {
>  		RTE_LOG(ERR, EAL, "Cannot find device (%s)\n",
> -			devname);
> +			da->name);
>  		ret = -ENODEV;
>  		goto err_devarg;
>  	}

<snip>

-- 
Gaëtan Rivet
6WIND

Re: [dpdk-dev] [PATCH v5 5/5] eal: simplify parameters of hotplug functions

[Thomas Monjalon]

04/10/2018 11:44, Gaëtan Rivet:
> On Thu, Oct 04, 2018 at 01:10:46AM +0200, Thomas Monjalon wrote:
> > --- a/lib/librte_eal/common/eal_common_dev.c
> > +++ b/lib/librte_eal/common/eal_common_dev.c
> > @@ -129,46 +129,61 @@ int rte_eal_dev_detach(struct rte_device *dev)
> >  
> >  int
> >  rte_eal_hotplug_add(const char *busname, const char *devname,
> > -		    const char *devargs)
> > +		    const char *drvargs)
> >  {
> > -	struct rte_bus *bus;
> > -	struct rte_device *dev;
> > -	struct rte_devargs *da;
> >  	int ret;
> > +	char *devargs = NULL;
> > +	int size, length = -1;
> >  
> > -	bus = rte_bus_find_by_name(busname);
> > -	if (bus == NULL) {
> > -		RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n", busname);
> > -		return -ENOENT;
> > -	}
> > +	do { /* 2 iterations: first is to know string length */
> > +		size = length + 1;
> > +		length = snprintf(devargs, size, "%s:%s,%s", busname, devname, drvargs);
> > +		if (length >= size)
> > +			devargs = malloc(length + 1);
> > +		if (devargs == NULL)
> > +			return -ENOMEM;
> > +	} while (size == 0);
> 
> I don't see a good reason for writing it this way.
> You use an additional state (size) and complicate something that is
> pretty straightforward. To make sure no error was written here, I had to
> do step-by-step careful reading, this should not be required by clean
> code.
> 
> You should remove this loop, which then allow removing size, and forces using
> simple code-flow.

The only reason for this loop is to avoid duplicating the snprintf format
in two calls.

It could be replaced by this:

	length = snprintf(devargs, 0, "%s:%s,%s", busname, devname, drvargs);
	if (length < 0)
		return -EINVAL;
	devargs = malloc(length + 1);
	if (devargs == NULL)
		return -ENOMEM;
	snprintf(devargs, length + 1, "%s:%s,%s", busname, devname, drvargs);

Any preference?

Re: [dpdk-dev] [PATCH v5 5/5] eal: simplify parameters of hotplug functions

[Gaëtan Rivet]

On Thu, Oct 04, 2018 at 01:46:54PM +0200, Thomas Monjalon wrote:
> 04/10/2018 11:44, Gaëtan Rivet:
> > On Thu, Oct 04, 2018 at 01:10:46AM +0200, Thomas Monjalon wrote:
> > > --- a/lib/librte_eal/common/eal_common_dev.c
> > > +++ b/lib/librte_eal/common/eal_common_dev.c
> > > @@ -129,46 +129,61 @@ int rte_eal_dev_detach(struct rte_device *dev)
> > >  
> > >  int
> > >  rte_eal_hotplug_add(const char *busname, const char *devname,
> > > -		    const char *devargs)
> > > +		    const char *drvargs)
> > >  {
> > > -	struct rte_bus *bus;
> > > -	struct rte_device *dev;
> > > -	struct rte_devargs *da;
> > >  	int ret;
> > > +	char *devargs = NULL;
> > > +	int size, length = -1;
> > >  
> > > -	bus = rte_bus_find_by_name(busname);
> > > -	if (bus == NULL) {
> > > -		RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n", busname);
> > > -		return -ENOENT;
> > > -	}
> > > +	do { /* 2 iterations: first is to know string length */
> > > +		size = length + 1;
> > > +		length = snprintf(devargs, size, "%s:%s,%s", busname, devname, drvargs);
> > > +		if (length >= size)
> > > +			devargs = malloc(length + 1);
> > > +		if (devargs == NULL)
> > > +			return -ENOMEM;
> > > +	} while (size == 0);
> > 
> > I don't see a good reason for writing it this way.
> > You use an additional state (size) and complicate something that is
> > pretty straightforward. To make sure no error was written here, I had to
> > do step-by-step careful reading, this should not be required by clean
> > code.
> > 
> > You should remove this loop, which then allow removing size, and forces using
> > simple code-flow.
> 
> The only reason for this loop is to avoid duplicating the snprintf format
> in two calls.
> 
> It could be replaced by this:
> 
> 	length = snprintf(devargs, 0, "%s:%s,%s", busname, devname, drvargs);
> 	if (length < 0)
> 		return -EINVAL;
> 	devargs = malloc(length + 1);
> 	if (devargs == NULL)
> 		return -ENOMEM;
> 	snprintf(devargs, length + 1, "%s:%s,%s", busname, devname, drvargs);
> 
> Any preference?
> 
> 

Yes, the second is cleaner IMO.

-- 
Gaëtan Rivet
6WIND

[dpdk-dev] [PATCH v6 0/5] eal: simplify devargs and hotplug functions

[Thomas Monjalon]

This is a follow-up of an idea presented at Dublin
during the "hotplug talk".

Instead of changing the existing hotplug functions, as in the RFC,
some new experimental functions are added.
The old functions lose their experimental status in order to provide
a non-experimental replacement for deprecated attach/detach functions.

It has been discussed briefly in the latest technical board meeting.


Changes in v6 - after Gaetan's review:
  - bump ABI version of all buses (because of rte_device change)
  - unroll snprintf loop in rte_eal_hotplug_add

Changes in v5:
  - rte_devargs_remove is fixed in case of null devargs (patch 2)
  - a pointer to the bus is added in rte_device (patch 3)
  - rte_dev_remove is fixed in case of no devargs (patch 5)

Changes in v4 - after Andrew's review:
  - add API changes in release notes (patches 1 & 2)
  - fix memory leak in rte_eal_hotplug_add (patch 4)

Change in v3:
  - fix null dereferencing in error path (patch 2)


Thomas Monjalon (5):
  devargs: remove deprecated functions
  devargs: simplify parameters of removal function
  eal: add bus pointer in device structure
  eal: remove experimental flag of hotplug functions
  eal: simplify parameters of hotplug functions

 doc/guides/rel_notes/release_18_11.rst      | 23 ++++--
 drivers/bus/dpaa/Makefile                   |  2 +-
 drivers/bus/dpaa/dpaa_bus.c                 |  2 +
 drivers/bus/dpaa/meson.build                |  2 +
 drivers/bus/fslmc/Makefile                  |  2 +-
 drivers/bus/fslmc/fslmc_bus.c               |  2 +
 drivers/bus/fslmc/meson.build               |  2 +
 drivers/bus/ifpga/Makefile                  |  2 +-
 drivers/bus/ifpga/ifpga_bus.c               |  6 +-
 drivers/bus/ifpga/meson.build               |  2 +
 drivers/bus/pci/Makefile                    |  2 +-
 drivers/bus/pci/bsd/pci.c                   |  2 +
 drivers/bus/pci/linux/pci.c                 |  1 +
 drivers/bus/pci/meson.build                 |  2 +
 drivers/bus/pci/private.h                   |  2 +
 drivers/bus/vdev/Makefile                   |  2 +-
 drivers/bus/vdev/meson.build                |  2 +
 drivers/bus/vdev/vdev.c                     |  9 +--
 drivers/bus/vmbus/Makefile                  |  2 +-
 drivers/bus/vmbus/linux/vmbus_bus.c         |  1 +
 drivers/bus/vmbus/meson.build               |  2 +
 drivers/bus/vmbus/private.h                 |  3 +
 drivers/net/failsafe/failsafe_eal.c         |  3 +-
 drivers/net/failsafe/failsafe_ether.c       |  3 +-
 lib/librte_eal/common/eal_common_dev.c      | 90 +++++++++++++--------
 lib/librte_eal/common/eal_common_devargs.c  | 41 ++--------
 lib/librte_eal/common/include/rte_dev.h     | 36 +++++++--
 lib/librte_eal/common/include/rte_devargs.h | 81 +------------------
 lib/librte_eal/rte_eal_version.map          | 10 +--
 29 files changed, 155 insertions(+), 184 deletions(-)

-- 
2.19.0

[dpdk-dev] [PATCH v6 1/5] devargs: remove deprecated functions

[Thomas Monjalon]

rte_eal_parse_devargs_str() does not support parsing the bus name
at the start of devargs. So it was renamed and deprecated.

rte_eal_devargs_add(), rte_eal_devargs_type_count() and
rte_eal_devargs_dump() were declared deprecated and had their
implementation body renamed.

All these functions were deprecated in release 18.05.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
The library version is not updated because it should be done
in earlier patch https://patches.dpdk.org/patch/43903/
---
 doc/guides/rel_notes/release_18_11.rst      |  5 ++
 lib/librte_eal/common/eal_common_devargs.c  | 30 ---------
 lib/librte_eal/common/include/rte_devargs.h | 71 ---------------------
 lib/librte_eal/rte_eal_version.map          |  4 --
 4 files changed, 5 insertions(+), 105 deletions(-)

diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 2133a5b9b..4f4420ad3 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -118,6 +118,11 @@ API Changes
    Also, make sure to start the actual text at the margin.
    =========================================================
 
+* eal: The following devargs functions, which were deprecated in 18.05,
+  were removed in 18.11:
+  ``rte_eal_parse_devargs_str()``, ``rte_eal_devargs_add()``,
+  ``rte_eal_devargs_type_count()``, and ``rte_eal_devargs_dump()``.
+
 * mbuf: The ``__rte_mbuf_raw_free()`` and ``__rte_pktmbuf_prefree_seg()``
   functions were deprecated since 17.05 and are replaced by
   ``rte_mbuf_raw_free()`` and ``rte_pktmbuf_prefree_seg()``.
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 88afece10..5cb5c624d 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -28,36 +28,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 size_t
 devargs_layer_count(const char *s)
 {
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 097a4ce7b..0eef6e9c4 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -66,36 +66,6 @@ struct rte_devargs {
 	const char *data; /**< Device string storage. */
 };
 
-/**
- * @deprecated
- * 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
- */
-__rte_deprecated
-int rte_eal_parse_devargs_str(const char *devargs_str,
-				char **drvname, char **drvargs);
-
 /**
  * Parse a device string.
  *
@@ -201,23 +171,6 @@ rte_devargs_insert(struct rte_devargs *da);
 __rte_experimental
 int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
 
-/**
- * @deprecated
- * Add a device to the user device list
- * See rte_devargs_parse() for details.
- *
- * @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);
-
 /**
  * Remove a device from the user device list.
  * Its resources are freed.
@@ -251,20 +204,6 @@ __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);
-
 /**
  * This function dumps the list of user device and their arguments.
  *
@@ -274,16 +213,6 @@ rte_eal_devargs_type_count(enum rte_devtype devtype);
 __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);
-
 /**
  * Find next rte_devargs matching the provided bus name.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 73282bbb0..3df7f9831 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -19,9 +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_devargs_type_count;
 	rte_eal_get_configuration;
 	rte_eal_get_lcore_state;
 	rte_eal_get_physmem_size;
@@ -32,7 +29,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.19.0

[dpdk-dev] [PATCH v6 2/5] devargs: simplify parameters of removal function

[Thomas Monjalon]

The function rte_devargs_remove(), which is intended to be internal,
can take a devargs structure as argument.
The matching is still using string comparison of bus name and
device name.
It is simpler and may allow a different devargs matching in future.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 doc/guides/rel_notes/release_18_11.rst      |  3 +++
 drivers/bus/ifpga/ifpga_bus.c               |  5 +----
 drivers/bus/vdev/vdev.c                     |  8 ++------
 lib/librte_eal/common/eal_common_dev.c      |  4 ++--
 lib/librte_eal/common/eal_common_devargs.c  | 11 +++++++----
 lib/librte_eal/common/include/rte_devargs.h | 10 +++-------
 6 files changed, 18 insertions(+), 23 deletions(-)

diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index 4f4420ad3..d534bb71c 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -123,6 +123,9 @@ API Changes
   ``rte_eal_parse_devargs_str()``, ``rte_eal_devargs_add()``,
   ``rte_eal_devargs_type_count()``, and ``rte_eal_devargs_dump()``.
 
+* eal: The parameters of the function ``rte_devargs_remove()`` have changed
+  from bus and device names to ``struct rte_devargs``.
+
 * mbuf: The ``__rte_mbuf_raw_free()`` and ``__rte_pktmbuf_prefree_seg()``
   functions were deprecated since 17.05 and are replaced by
   ``rte_mbuf_raw_free()`` and ``rte_pktmbuf_prefree_seg()``.
diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index c54b59db2..3ef035b7e 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -361,7 +361,6 @@ static int
 ifpga_unplug(struct rte_device *dev)
 {
 	struct rte_afu_device *afu_dev = NULL;
-	struct rte_devargs *devargs = NULL;
 	int ret;
 
 	if (dev == NULL)
@@ -371,15 +370,13 @@ ifpga_unplug(struct rte_device *dev)
 	if (!afu_dev)
 		return -ENOENT;
 
-	devargs = dev->devargs;
-
 	ret = ifpga_remove_driver(afu_dev);
 	if (ret)
 		return ret;
 
 	TAILQ_REMOVE(&ifpga_afu_dev_list, afu_dev, next);
 
-	rte_devargs_remove(devargs->bus->name, devargs->name);
+	rte_devargs_remove(dev->devargs);
 	free(afu_dev);
 	return 0;
 
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index ef3ad6d99..efca962f7 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -249,7 +249,6 @@ int
 rte_vdev_init(const char *name, const char *args)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
 	int ret;
 
 	rte_spinlock_recursive_lock(&vdev_device_list_lock);
@@ -260,9 +259,8 @@ rte_vdev_init(const char *name, const char *args)
 			if (ret > 0)
 				VDEV_LOG(ERR, "no driver found for %s", name);
 			/* If fails, remove it from vdev list */
-			devargs = dev->device.devargs;
 			TAILQ_REMOVE(&vdev_device_list, dev, next);
-			rte_devargs_remove(devargs->bus->name, devargs->name);
+			rte_devargs_remove(dev->device.devargs);
 			free(dev);
 		}
 	}
@@ -290,7 +288,6 @@ int
 rte_vdev_uninit(const char *name)
 {
 	struct rte_vdev_device *dev;
-	struct rte_devargs *devargs;
 	int ret;
 
 	if (name == NULL)
@@ -309,8 +306,7 @@ rte_vdev_uninit(const char *name)
 		goto unlock;
 
 	TAILQ_REMOVE(&vdev_device_list, dev, next);
-	devargs = dev->device.devargs;
-	rte_devargs_remove(devargs->bus->name, devargs->name);
+	rte_devargs_remove(dev->device.devargs);
 	free(dev);
 
 unlock:
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 678dbcac7..e1d9e8ec7 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -186,7 +186,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return 0;
 
 err_devarg:
-	if (rte_devargs_remove(busname, devname)) {
+	if (rte_devargs_remove(da) != 0) {
 		free(da->args);
 		free(da);
 	}
@@ -227,7 +227,7 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
 			dev->name);
-	rte_devargs_remove(busname, devname);
+	rte_devargs_remove(dev->devargs);
 	return ret;
 }
 
diff --git a/lib/librte_eal/common/eal_common_devargs.c b/lib/librte_eal/common/eal_common_devargs.c
index 5cb5c624d..69e9e32e9 100644
--- a/lib/librte_eal/common/eal_common_devargs.c
+++ b/lib/librte_eal/common/eal_common_devargs.c
@@ -263,7 +263,7 @@ rte_devargs_insert(struct rte_devargs *da)
 {
 	int ret;
 
-	ret = rte_devargs_remove(da->bus->name, da->name);
+	ret = rte_devargs_remove(da);
 	if (ret < 0)
 		return ret;
 	TAILQ_INSERT_TAIL(&devargs_list, da, next);
@@ -309,14 +309,17 @@ rte_devargs_add(enum rte_devtype devtype, const char *devargs_str)
 }
 
 int __rte_experimental
-rte_devargs_remove(const char *busname, const char *devname)
+rte_devargs_remove(struct rte_devargs *devargs)
 {
 	struct rte_devargs *d;
 	void *tmp;
 
+	if (devargs == NULL || devargs->bus == NULL)
+		return -1;
+
 	TAILQ_FOREACH_SAFE(d, &devargs_list, next, tmp) {
-		if (strcmp(d->bus->name, busname) == 0 &&
-		    strcmp(d->name, devname) == 0) {
+		if (strcmp(d->bus->name, devargs->bus->name) == 0 &&
+		    strcmp(d->name, devargs->name) == 0) {
 			TAILQ_REMOVE(&devargs_list, d, next);
 			free(d->args);
 			free(d);
diff --git a/lib/librte_eal/common/include/rte_devargs.h b/lib/librte_eal/common/include/rte_devargs.h
index 0eef6e9c4..b1f121f83 100644
--- a/lib/librte_eal/common/include/rte_devargs.h
+++ b/lib/librte_eal/common/include/rte_devargs.h
@@ -176,11 +176,8 @@ int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
  * Its resources are freed.
  * If the devargs cannot be found, nothing happens.
  *
- * @param busname
- *   bus name of the devargs to remove.
- *
- * @param devname
- *   device name of the devargs to remove.
+ * @param devargs
+ *   The instance or a copy of devargs to remove.
  *
  * @return
  *   0 on success.
@@ -188,8 +185,7 @@ int rte_devargs_add(enum rte_devtype devtype, const char *devargs_str);
  *   >0 if the devargs was not within the user device list.
  */
 __rte_experimental
-int rte_devargs_remove(const char *busname,
-		       const char *devname);
+int rte_devargs_remove(struct rte_devargs *devargs);
 
 /**
  * Count the number of user devices of a specified type
-- 
2.19.0

[dpdk-dev] [PATCH v6 3/5] eal: add bus pointer in device structure

[Thomas Monjalon]

When a device is added with a devargs (hotplug or whitelist),
the bus pointer can be retrieved via its devargs.
But there is no such devargs.bus in case of standard scan.

A pointer to the rte_bus handle is added to rte_device.
When a device is allocated (during a scan),
the pointer to its bus is assigned.

It will make possible to remove a rte_device,
using the function pointer from its bus.

The function rte_bus_find_by_device() becomes useless,
and may be removed later.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 doc/guides/rel_notes/release_18_11.rst  | 15 ++++++++++-----
 drivers/bus/dpaa/Makefile               |  2 +-
 drivers/bus/dpaa/dpaa_bus.c             |  2 ++
 drivers/bus/dpaa/meson.build            |  2 ++
 drivers/bus/fslmc/Makefile              |  2 +-
 drivers/bus/fslmc/fslmc_bus.c           |  2 ++
 drivers/bus/fslmc/meson.build           |  2 ++
 drivers/bus/ifpga/Makefile              |  2 +-
 drivers/bus/ifpga/ifpga_bus.c           |  1 +
 drivers/bus/ifpga/meson.build           |  2 ++
 drivers/bus/pci/Makefile                |  2 +-
 drivers/bus/pci/bsd/pci.c               |  2 ++
 drivers/bus/pci/linux/pci.c             |  1 +
 drivers/bus/pci/meson.build             |  2 ++
 drivers/bus/pci/private.h               |  2 ++
 drivers/bus/vdev/Makefile               |  2 +-
 drivers/bus/vdev/meson.build            |  2 ++
 drivers/bus/vdev/vdev.c                 |  1 +
 drivers/bus/vmbus/Makefile              |  2 +-
 drivers/bus/vmbus/linux/vmbus_bus.c     |  1 +
 drivers/bus/vmbus/meson.build           |  2 ++
 drivers/bus/vmbus/private.h             |  3 +++
 lib/librte_eal/common/include/rte_dev.h |  1 +
 23 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/doc/guides/rel_notes/release_18_11.rst b/doc/guides/rel_notes/release_18_11.rst
index d534bb71c..c87522f27 100644
--- a/doc/guides/rel_notes/release_18_11.rst
+++ b/doc/guides/rel_notes/release_18_11.rst
@@ -164,6 +164,10 @@ ABI Changes
        ``rte_config`` structure on account of improving DPDK usability when
        using either ``--legacy-mem`` or ``--single-file-segments`` flags.
 
+* eal: The structure ``rte_device`` got a new field to reference a ``rte_bus``.
+  It is changing the size of the ``struct rte_device`` and the inherited
+  device structures of all buses.
+
 
 Removed Items
 -------------
@@ -199,11 +203,12 @@ The libraries prepended with a plus sign were incremented in this version.
      librte_bbdev.so.1
      librte_bitratestats.so.2
      librte_bpf.so.1
-     librte_bus_dpaa.so.1
-     librte_bus_fslmc.so.1
-     librte_bus_pci.so.1
-     librte_bus_vdev.so.1
-   + librte_bus_vmbus.so.1
+   + librte_bus_dpaa.so.2
+   + librte_bus_fslmc.so.2
+   + librte_bus_ifpga.so.2
+   + librte_bus_pci.so.2
+   + librte_bus_vdev.so.2
+   + librte_bus_vmbus.so.2
      librte_cfgfile.so.2
      librte_cmdline.so.2
      librte_common_octeontx.so.1
diff --git a/drivers/bus/dpaa/Makefile b/drivers/bus/dpaa/Makefile
index bffaa9d92..9337b5f92 100644
--- a/drivers/bus/dpaa/Makefile
+++ b/drivers/bus/dpaa/Makefile
@@ -24,7 +24,7 @@ CFLAGS += -I$(RTE_SDK)/lib/librte_eal/common/include
 # versioning export map
 EXPORT_MAP := rte_bus_dpaa_version.map
 
-LIBABIVER := 1
+LIBABIVER := 2
 
 # all source are stored in SRCS-y
 #
diff --git a/drivers/bus/dpaa/dpaa_bus.c b/drivers/bus/dpaa/dpaa_bus.c
index 49cd04dbb..138e0f98d 100644
--- a/drivers/bus/dpaa/dpaa_bus.c
+++ b/drivers/bus/dpaa/dpaa_bus.c
@@ -165,6 +165,8 @@ dpaa_create_device_list(void)
 			goto cleanup;
 		}
 
+		dev->device.bus = &rte_dpaa_bus.bus;
+
 		cfg = &dpaa_netcfg->port_cfg[i];
 		fman_intf = cfg->fman_if;
 
diff --git a/drivers/bus/dpaa/meson.build b/drivers/bus/dpaa/meson.build
index d10b62c03..5e7705571 100644
--- a/drivers/bus/dpaa/meson.build
+++ b/drivers/bus/dpaa/meson.build
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright 2018 NXP
 
+version = 2
+
 if host_machine.system() != 'linux'
         build = false
 endif
diff --git a/drivers/bus/fslmc/Makefile b/drivers/bus/fslmc/Makefile
index 515d0f534..e95551980 100644
--- a/drivers/bus/fslmc/Makefile
+++ b/drivers/bus/fslmc/Makefile
@@ -24,7 +24,7 @@ LDLIBS += -lrte_ethdev
 EXPORT_MAP := rte_bus_fslmc_version.map
 
 # library version
-LIBABIVER := 1
+LIBABIVER := 2
 
 SRCS-$(CONFIG_RTE_LIBRTE_FSLMC_BUS) += \
         qbman/qbman_portal.c \
diff --git a/drivers/bus/fslmc/fslmc_bus.c b/drivers/bus/fslmc/fslmc_bus.c
index bfe81e236..960f55071 100644
--- a/drivers/bus/fslmc/fslmc_bus.c
+++ b/drivers/bus/fslmc/fslmc_bus.c
@@ -161,6 +161,8 @@ scan_one_fslmc_device(char *dev_name)
 		return -ENOMEM;
 	}
 
+	dev->device.bus = &rte_fslmc_bus.bus;
+
 	/* Parse the device name and ID */
 	t_ptr = strtok(dup_dev_name, ".");
 	if (!t_ptr) {
diff --git a/drivers/bus/fslmc/meson.build b/drivers/bus/fslmc/meson.build
index 22a56a6fc..54ca92d0c 100644
--- a/drivers/bus/fslmc/meson.build
+++ b/drivers/bus/fslmc/meson.build
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright 2018 NXP
 
+version = 2
+
 if host_machine.system() != 'linux'
         build = false
 endif
diff --git a/drivers/bus/ifpga/Makefile b/drivers/bus/ifpga/Makefile
index 3ff3bdb81..514452b39 100644
--- a/drivers/bus/ifpga/Makefile
+++ b/drivers/bus/ifpga/Makefile
@@ -19,7 +19,7 @@ LDLIBS += -lrte_kvargs
 EXPORT_MAP := rte_bus_ifpga_version.map
 
 # library version
-LIBABIVER := 1
+LIBABIVER := 2
 
 SRCS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS) += ifpga_bus.c
 SRCS-$(CONFIG_RTE_LIBRTE_IFPGA_BUS) += ifpga_common.c
diff --git a/drivers/bus/ifpga/ifpga_bus.c b/drivers/bus/ifpga/ifpga_bus.c
index 3ef035b7e..80663328a 100644
--- a/drivers/bus/ifpga/ifpga_bus.c
+++ b/drivers/bus/ifpga/ifpga_bus.c
@@ -142,6 +142,7 @@ ifpga_scan_one(struct rte_rawdev *rawdev,
 	if (!afu_dev)
 		goto end;
 
+	afu_dev->device.bus = &rte_ifpga_bus;
 	afu_dev->device.devargs = devargs;
 	afu_dev->device.numa_node = SOCKET_ID_ANY;
 	afu_dev->device.name = devargs->name;
diff --git a/drivers/bus/ifpga/meson.build b/drivers/bus/ifpga/meson.build
index c9b08c862..0b5c38d54 100644
--- a/drivers/bus/ifpga/meson.build
+++ b/drivers/bus/ifpga/meson.build
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2010-2018 Intel Corporation
 
+version = 2
+
 deps += ['pci', 'kvargs', 'rawdev']
 install_headers('rte_bus_ifpga.h')
 sources = files('ifpga_common.c', 'ifpga_bus.c')
diff --git a/drivers/bus/pci/Makefile b/drivers/bus/pci/Makefile
index 4de953f8f..f33e0120f 100644
--- a/drivers/bus/pci/Makefile
+++ b/drivers/bus/pci/Makefile
@@ -4,7 +4,7 @@
 include $(RTE_SDK)/mk/rte.vars.mk
 
 LIB = librte_bus_pci.a
-LIBABIVER := 1
+LIBABIVER := 2
 EXPORT_MAP := rte_bus_pci_version.map
 
 CFLAGS := -I$(SRCDIR) $(CFLAGS)
diff --git a/drivers/bus/pci/bsd/pci.c b/drivers/bus/pci/bsd/pci.c
index 655b34b7e..40641cad4 100644
--- a/drivers/bus/pci/bsd/pci.c
+++ b/drivers/bus/pci/bsd/pci.c
@@ -223,6 +223,8 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 	}
 
 	memset(dev, 0, sizeof(*dev));
+	dev->device.bus = &rte_pci_bus.bus;
+
 	dev->addr.domain = conf->pc_sel.pc_domain;
 	dev->addr.bus = conf->pc_sel.pc_bus;
 	dev->addr.devid = conf->pc_sel.pc_dev;
diff --git a/drivers/bus/pci/linux/pci.c b/drivers/bus/pci/linux/pci.c
index 04648ac93..e31bbb370 100644
--- a/drivers/bus/pci/linux/pci.c
+++ b/drivers/bus/pci/linux/pci.c
@@ -228,6 +228,7 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 		return -1;
 
 	memset(dev, 0, sizeof(*dev));
+	dev->device.bus = &rte_pci_bus.bus;
 	dev->addr = *addr;
 
 	/* get vendor id */
diff --git a/drivers/bus/pci/meson.build b/drivers/bus/pci/meson.build
index 23d6a5fec..ef9492bb8 100644
--- a/drivers/bus/pci/meson.build
+++ b/drivers/bus/pci/meson.build
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
+version = 2
+
 deps += ['pci']
 install_headers('rte_bus_pci.h')
 sources = files('pci_common.c',
diff --git a/drivers/bus/pci/private.h b/drivers/bus/pci/private.h
index 0e689fa74..04bffa6e7 100644
--- a/drivers/bus/pci/private.h
+++ b/drivers/bus/pci/private.h
@@ -15,6 +15,8 @@ extern struct rte_pci_bus rte_pci_bus;
 struct rte_pci_driver;
 struct rte_pci_device;
 
+extern struct rte_pci_bus rte_pci_bus;
+
 /**
  * Probe the PCI bus
  *
diff --git a/drivers/bus/vdev/Makefile b/drivers/bus/vdev/Makefile
index 1f9cd7ebe..803b8ea7b 100644
--- a/drivers/bus/vdev/Makefile
+++ b/drivers/bus/vdev/Makefile
@@ -16,7 +16,7 @@ CFLAGS += -DALLOW_EXPERIMENTAL_API
 EXPORT_MAP := rte_bus_vdev_version.map
 
 # library version
-LIBABIVER := 1
+LIBABIVER := 2
 
 SRCS-y += vdev.c
 SRCS-y += vdev_params.c
diff --git a/drivers/bus/vdev/meson.build b/drivers/bus/vdev/meson.build
index 12605e5c7..803785f10 100644
--- a/drivers/bus/vdev/meson.build
+++ b/drivers/bus/vdev/meson.build
@@ -1,6 +1,8 @@
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright(c) 2017 Intel Corporation
 
+version = 2
+
 sources = files('vdev.c',
 	'vdev_params.c')
 install_headers('rte_bus_vdev.h')
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index efca962f7..0142fb2c8 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -456,6 +456,7 @@ vdev_scan(void)
 			continue;
 		}
 
+		dev->device.bus = &rte_vdev_bus;
 		dev->device.devargs = devargs;
 		dev->device.numa_node = SOCKET_ID_ANY;
 		dev->device.name = devargs->name;
diff --git a/drivers/bus/vmbus/Makefile b/drivers/bus/vmbus/Makefile
index deee9dd10..e54c557c6 100644
--- a/drivers/bus/vmbus/Makefile
+++ b/drivers/bus/vmbus/Makefile
@@ -3,7 +3,7 @@
 include $(RTE_SDK)/mk/rte.vars.mk
 
 LIB = librte_bus_vmbus.a
-LIBABIVER := 1
+LIBABIVER := 2
 EXPORT_MAP := rte_bus_vmbus_version.map
 
 CFLAGS += -I$(SRCDIR)
diff --git a/drivers/bus/vmbus/linux/vmbus_bus.c b/drivers/bus/vmbus/linux/vmbus_bus.c
index 527a6a39f..a4755a387 100644
--- a/drivers/bus/vmbus/linux/vmbus_bus.c
+++ b/drivers/bus/vmbus/linux/vmbus_bus.c
@@ -229,6 +229,7 @@ vmbus_scan_one(const char *name)
 	if (dev == NULL)
 		return -1;
 
+	dev->device.bus = &rte_vmbus_bus.bus;
 	dev->device.name = strdup(name);
 	if (!dev->device.name)
 		goto error;
diff --git a/drivers/bus/vmbus/meson.build b/drivers/bus/vmbus/meson.build
index 18daabecc..0e4d058ee 100644
--- a/drivers/bus/vmbus/meson.build
+++ b/drivers/bus/vmbus/meson.build
@@ -1,5 +1,7 @@
 # SPDX-License-Identifier: BSD-3-Clause
 
+version = 2
+
 allow_experimental_apis = true
 
 install_headers('rte_bus_vmbus.h','rte_vmbus_reg.h')
diff --git a/drivers/bus/vmbus/private.h b/drivers/bus/vmbus/private.h
index f2022a68c..211127dd8 100644
--- a/drivers/bus/vmbus/private.h
+++ b/drivers/bus/vmbus/private.h
@@ -10,11 +10,14 @@
 #include <sys/uio.h>
 #include <rte_log.h>
 #include <rte_vmbus_reg.h>
+#include <rte_bus_vmbus.h>
 
 #ifndef PAGE_SIZE
 #define PAGE_SIZE	4096
 #endif
 
+extern struct rte_vmbus_bus rte_vmbus_bus;
+
 extern int vmbus_logtype_bus;
 #define VMBUS_LOG(level, fmt, args...) \
 	rte_log(RTE_LOG_ ## level, vmbus_logtype_bus, "%s(): " fmt "\n", \
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index b80a80598..d82cba847 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -157,6 +157,7 @@ struct rte_device {
 	TAILQ_ENTRY(rte_device) next; /**< Next device */
 	const char *name;             /**< Device name */
 	const struct rte_driver *driver;/**< Associated driver */
+	const struct rte_bus *bus;    /**< Bus handle assigned on scan */
 	int numa_node;                /**< NUMA node connection */
 	struct rte_devargs *devargs;  /**< Device user arguments */
 };
-- 
2.19.0

[dpdk-dev] [PATCH v6 4/5] eal: remove experimental flag of hotplug functions

[Thomas Monjalon]

These functions are quite old and are the only available replacement
for the deprecated attach/detach functions.

Note: some new functions may (again) replace these hotplug functions,
in future, with better parameters.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_dev.c  |  7 ++++---
 lib/librte_eal/common/include/rte_dev.h | 11 ++---------
 lib/librte_eal/rte_eal_version.map      |  4 ++--
 3 files changed, 8 insertions(+), 14 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index e1d9e8ec7..ce6d145fb 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -127,8 +127,9 @@ int rte_eal_dev_detach(struct rte_device *dev)
 	return ret;
 }
 
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs)
+int
+rte_eal_hotplug_add(const char *busname, const char *devname,
+		    const char *devargs)
 {
 	struct rte_bus *bus;
 	struct rte_device *dev;
@@ -193,7 +194,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
 	return ret;
 }
 
-int __rte_experimental
+int
 rte_eal_hotplug_remove(const char *busname, const char *devname)
 {
 	struct rte_bus *bus;
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index d82cba847..d6c5d48a9 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -190,9 +190,6 @@ __rte_deprecated
 int rte_eal_dev_detach(struct rte_device *dev);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Hotplug add a given device to a specific bus.
  *
  * @param busname
@@ -205,13 +202,10 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devname,
+int rte_eal_hotplug_add(const char *busname, const char *devname,
 			const char *devargs);
 
 /**
- * @warning
- * @b EXPERIMENTAL: this API may change without prior notice
- *
  * Hotplug remove a given device from a specific bus.
  *
  * @param busname
@@ -221,8 +215,7 @@ int __rte_experimental rte_eal_hotplug_add(const char *busname, const char *devn
  * @return
  *   0 on success, negative on error.
  */
-int __rte_experimental rte_eal_hotplug_remove(const char *busname,
-					  const char *devname);
+int rte_eal_hotplug_remove(const char *busname, const char *devname);
 
 /**
  * Device comparison function.
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 3df7f9831..6bff37f4b 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -261,6 +261,8 @@ DPDK_18.08 {
 DPDK_18.11 {
 	global:
 
+	rte_eal_hotplug_add;
+	rte_eal_hotplug_remove;
 	rte_strscpy;
 
 } DPDK_18.08;
@@ -288,8 +290,6 @@ EXPERIMENTAL {
 	rte_devargs_remove;
 	rte_devargs_type_count;
 	rte_eal_cleanup;
-	rte_eal_hotplug_add;
-	rte_eal_hotplug_remove;
 	rte_fbarray_attach;
 	rte_fbarray_destroy;
 	rte_fbarray_detach;
-- 
2.19.0

[dpdk-dev] [PATCH v6 5/5] eal: simplify parameters of hotplug functions

[Thomas Monjalon]

All information about a device to probe can be grouped
in a common string, which is what we usually call devargs.
An application should not have to parse this string before
calling the EAL probe function.
And the syntax could evolve to be more complex and support
matching multiple devices in one string.
That's why the bus name and device name should be removed from
rte_eal_hotplug_add().
Instead of changing this function, a simpler one is added
and used in the old one, which may be deprecated later.

When removing a device, we already know its rte_device handle
which can be directly passed as parameter of rte_eal_hotplug_remove().
If the rte_device is not known, it can be retrieved with the devargs,
by iterating in the device list (future RTE_DEV_FOREACH()).
Similarly to the probing case, a new function is added
and used in the old one, which may be deprecated later.
The new function is used in failsafe, because the replacement is easy.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
Reviewed-by: Andrew Rybchenko <arybchenko@solarflare.com>
Acked-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 drivers/net/failsafe/failsafe_eal.c     |  3 +-
 drivers/net/failsafe/failsafe_ether.c   |  3 +-
 lib/librte_eal/common/eal_common_dev.c  | 81 ++++++++++++++++---------
 lib/librte_eal/common/include/rte_dev.h | 30 ++++++++-
 lib/librte_eal/rte_eal_version.map      |  2 +
 5 files changed, 84 insertions(+), 35 deletions(-)

diff --git a/drivers/net/failsafe/failsafe_eal.c b/drivers/net/failsafe/failsafe_eal.c
index ce1633f13..8a888b1ff 100644
--- a/drivers/net/failsafe/failsafe_eal.c
+++ b/drivers/net/failsafe/failsafe_eal.c
@@ -144,8 +144,7 @@ fs_bus_uninit(struct rte_eth_dev *dev)
 	int ret = 0;
 
 	FOREACH_SUBDEV_STATE(sdev, i, dev, DEV_PROBED) {
-		sdev_ret = rte_eal_hotplug_remove(sdev->bus->name,
-							sdev->dev->name);
+		sdev_ret = rte_dev_remove(sdev->dev);
 		if (sdev_ret) {
 			ERROR("Failed to remove requested device %s (err: %d)",
 			      sdev->dev->name, sdev_ret);
diff --git a/drivers/net/failsafe/failsafe_ether.c b/drivers/net/failsafe/failsafe_ether.c
index 51c96f78b..f2512c430 100644
--- a/drivers/net/failsafe/failsafe_ether.c
+++ b/drivers/net/failsafe/failsafe_ether.c
@@ -282,8 +282,7 @@ fs_dev_remove(struct sub_device *sdev)
 		sdev->state = DEV_PROBED;
 		/* fallthrough */
 	case DEV_PROBED:
-		ret = rte_eal_hotplug_remove(sdev->bus->name,
-					     sdev->dev->name);
+		ret = rte_dev_remove(sdev->dev);
 		if (ret) {
 			ERROR("Bus detach failed for sub_device %u",
 			      SUB_ID(sdev));
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index ce6d145fb..7663eaa3f 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -129,46 +129,62 @@ int rte_eal_dev_detach(struct rte_device *dev)
 
 int
 rte_eal_hotplug_add(const char *busname, const char *devname,
-		    const char *devargs)
+		    const char *drvargs)
 {
-	struct rte_bus *bus;
-	struct rte_device *dev;
-	struct rte_devargs *da;
 	int ret;
+	char *devargs = NULL;
+	int length;
 
-	bus = rte_bus_find_by_name(busname);
-	if (bus == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot find bus (%s)\n", busname);
-		return -ENOENT;
-	}
+	length = snprintf(NULL, 0, "%s:%s,%s", busname, devname, drvargs);
+	if (length < 0)
+		return -EINVAL;
+	devargs = malloc(length + 1);
+	if (devargs == NULL)
+		return -ENOMEM;
+	ret = snprintf(devargs, length + 1, "%s:%s,%s", busname, devname, drvargs);
+	if (ret < 0)
+		return -EINVAL;
 
-	if (bus->plug == NULL) {
-		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
-			bus->name);
-		return -ENOTSUP;
-	}
+	ret = rte_dev_probe(devargs);
+
+	free(devargs);
+	return ret;
+}
+
+int __rte_experimental
+rte_dev_probe(const char *devargs)
+{
+	struct rte_device *dev;
+	struct rte_devargs *da;
+	int ret;
 
 	da = calloc(1, sizeof(*da));
 	if (da == NULL)
 		return -ENOMEM;
 
-	ret = rte_devargs_parsef(da, "%s:%s,%s",
-				 busname, devname, devargs);
+	ret = rte_devargs_parse(da, devargs);
 	if (ret)
 		goto err_devarg;
 
+	if (da->bus->plug == NULL) {
+		RTE_LOG(ERR, EAL, "Function plug not supported by bus (%s)\n",
+			da->bus->name);
+		ret = -ENOTSUP;
+		goto err_devarg;
+	}
+
 	ret = rte_devargs_insert(da);
 	if (ret)
 		goto err_devarg;
 
-	ret = bus->scan();
+	ret = da->bus->scan();
 	if (ret)
 		goto err_devarg;
 
-	dev = bus->find_device(NULL, cmp_dev_name, devname);
+	dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find device (%s)\n",
-			devname);
+			da->name);
 		ret = -ENODEV;
 		goto err_devarg;
 	}
@@ -178,7 +194,7 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 		return -EEXIST;
 	}
 
-	ret = bus->plug(dev);
+	ret = dev->bus->plug(dev);
 	if (ret) {
 		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
 			dev->name);
@@ -197,9 +213,8 @@ rte_eal_hotplug_add(const char *busname, const char *devname,
 int
 rte_eal_hotplug_remove(const char *busname, const char *devname)
 {
-	struct rte_bus *bus;
 	struct rte_device *dev;
-	int ret;
+	struct rte_bus *bus;
 
 	bus = rte_bus_find_by_name(busname);
 	if (bus == NULL) {
@@ -207,24 +222,32 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 		return -ENOENT;
 	}
 
-	if (bus->unplug == NULL) {
-		RTE_LOG(ERR, EAL, "Function unplug not supported by bus (%s)\n",
-			bus->name);
-		return -ENOTSUP;
-	}
-
 	dev = bus->find_device(NULL, cmp_dev_name, devname);
 	if (dev == NULL) {
 		RTE_LOG(ERR, EAL, "Cannot find plugged device (%s)\n", devname);
 		return -EINVAL;
 	}
 
+	return rte_dev_remove(dev);
+}
+
+int __rte_experimental
+rte_dev_remove(struct rte_device *dev)
+{
+	int ret;
+
 	if (dev->driver == NULL) {
 		RTE_LOG(ERR, EAL, "Device is already unplugged\n");
 		return -ENOENT;
 	}
 
-	ret = bus->unplug(dev);
+	if (dev->bus->unplug == NULL) {
+		RTE_LOG(ERR, EAL, "Function unplug not supported by bus (%s)\n",
+			dev->bus->name);
+		return -ENOTSUP;
+	}
+
+	ret = dev->bus->unplug(dev);
 	if (ret)
 		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
 			dev->name);
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index d6c5d48a9..036180ff3 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -197,13 +197,26 @@ int rte_eal_dev_detach(struct rte_device *dev);
  * @param devname
  *   The device name. Based on this device name, eal will identify a driver
  *   capable of handling it and pass it to the driver probing function.
- * @param devargs
+ * @param drvargs
  *   Device arguments to be passed to the driver.
  * @return
  *   0 on success, negative on error.
  */
 int rte_eal_hotplug_add(const char *busname, const char *devname,
-			const char *devargs);
+			const char *drvargs);
+
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Add matching devices.
+ *
+ * @param devargs
+ *   Device arguments including bus, class and driver properties.
+ * @return
+ *   0 on success, negative on error.
+ */
+int __rte_experimental rte_dev_probe(const char *devargs);
 
 /**
  * Hotplug remove a given device from a specific bus.
@@ -217,6 +230,19 @@ int rte_eal_hotplug_add(const char *busname, const char *devname,
  */
 int rte_eal_hotplug_remove(const char *busname, const char *devname);
 
+/**
+ * @warning
+ * @b EXPERIMENTAL: this API may change without prior notice
+ *
+ * Remove one device.
+ *
+ * @param dev
+ *   Data structure of the device to remove.
+ * @return
+ *   0 on success, negative on error.
+ */
+int __rte_experimental rte_dev_remove(struct rte_device *dev);
+
 /**
  * Device comparison function.
  *
diff --git a/lib/librte_eal/rte_eal_version.map b/lib/librte_eal/rte_eal_version.map
index 6bff37f4b..2ea7a870a 100644
--- a/lib/librte_eal/rte_eal_version.map
+++ b/lib/librte_eal/rte_eal_version.map
@@ -281,6 +281,8 @@ EXPERIMENTAL {
 	rte_dev_event_monitor_stop;
 	rte_dev_iterator_init;
 	rte_dev_iterator_next;
+	rte_dev_probe;
+	rte_dev_remove;
 	rte_devargs_add;
 	rte_devargs_dump;
 	rte_devargs_insert;
-- 
2.19.0

Re: [dpdk-dev] [PATCH v6 0/5] eal: simplify devargs and hotplug functions

[Stephen Hemminger]

On Sun,  7 Oct 2018 11:32:39 +0200
Thomas Monjalon <thomas@monjalon.net> wrote:

> This is a follow-up of an idea presented at Dublin
> during the "hotplug talk".
> 
> Instead of changing the existing hotplug functions, as in the RFC,
> some new experimental functions are added.
> The old functions lose their experimental status in order to provide
> a non-experimental replacement for deprecated attach/detach functions.
> 
> It has been discussed briefly in the latest technical board meeting.
> 
> 
> Changes in v6 - after Gaetan's review:
>   - bump ABI version of all buses (because of rte_device change)
>   - unroll snprintf loop in rte_eal_hotplug_add
> 
> Changes in v5:
>   - rte_devargs_remove is fixed in case of null devargs (patch 2)
>   - a pointer to the bus is added in rte_device (patch 3)
>   - rte_dev_remove is fixed in case of no devargs (patch 5)
> 
> Changes in v4 - after Andrew's review:
>   - add API changes in release notes (patches 1 & 2)
>   - fix memory leak in rte_eal_hotplug_add (patch 4)
> 
> Change in v3:
>   - fix null dereferencing in error path (patch 2)
> 
> 
> Thomas Monjalon (5):
>   devargs: remove deprecated functions
>   devargs: simplify parameters of removal function
>   eal: add bus pointer in device structure
>   eal: remove experimental flag of hotplug functions
>   eal: simplify parameters of hotplug functions
> 
>  doc/guides/rel_notes/release_18_11.rst      | 23 ++++--
>  drivers/bus/dpaa/Makefile                   |  2 +-
>  drivers/bus/dpaa/dpaa_bus.c                 |  2 +
>  drivers/bus/dpaa/meson.build                |  2 +
>  drivers/bus/fslmc/Makefile                  |  2 +-
>  drivers/bus/fslmc/fslmc_bus.c               |  2 +
>  drivers/bus/fslmc/meson.build               |  2 +
>  drivers/bus/ifpga/Makefile                  |  2 +-
>  drivers/bus/ifpga/ifpga_bus.c               |  6 +-
>  drivers/bus/ifpga/meson.build               |  2 +
>  drivers/bus/pci/Makefile                    |  2 +-
>  drivers/bus/pci/bsd/pci.c                   |  2 +
>  drivers/bus/pci/linux/pci.c                 |  1 +
>  drivers/bus/pci/meson.build                 |  2 +
>  drivers/bus/pci/private.h                   |  2 +
>  drivers/bus/vdev/Makefile                   |  2 +-
>  drivers/bus/vdev/meson.build                |  2 +
>  drivers/bus/vdev/vdev.c                     |  9 +--
>  drivers/bus/vmbus/Makefile                  |  2 +-
>  drivers/bus/vmbus/linux/vmbus_bus.c         |  1 +
>  drivers/bus/vmbus/meson.build               |  2 +
>  drivers/bus/vmbus/private.h                 |  3 +
>  drivers/net/failsafe/failsafe_eal.c         |  3 +-
>  drivers/net/failsafe/failsafe_ether.c       |  3 +-
>  lib/librte_eal/common/eal_common_dev.c      | 90 +++++++++++++--------
>  lib/librte_eal/common/eal_common_devargs.c  | 41 ++--------
>  lib/librte_eal/common/include/rte_dev.h     | 36 +++++++--
>  lib/librte_eal/common/include/rte_devargs.h | 81 +------------------
>  lib/librte_eal/rte_eal_version.map          | 10 +--
>  29 files changed, 155 insertions(+), 184 deletions(-)
> 

I like these changes.

Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>


I noticed there is only minimal places that devargs appear in the documentation.
The relationship between whitelist and devargs is not obvious for new users.


The one place is in the documentation of the documentation! So you want to pull
rte_eth_dev_attach from documentation.rst.

Re: [dpdk-dev] [PATCH v6 0/5] eal: simplify devargs and hotplug functions

[Thomas Monjalon]

08/10/2018 23:45, Stephen Hemminger:
> On Sun,  7 Oct 2018 11:32:39 +0200
> Thomas Monjalon <thomas@monjalon.net> wrote:
> 
> > This is a follow-up of an idea presented at Dublin
> > during the "hotplug talk".
> > 
> > Instead of changing the existing hotplug functions, as in the RFC,
> > some new experimental functions are added.
> > The old functions lose their experimental status in order to provide
> > a non-experimental replacement for deprecated attach/detach functions.
> > 
> > It has been discussed briefly in the latest technical board meeting.
> > 
> > 
> > Changes in v6 - after Gaetan's review:
> >   - bump ABI version of all buses (because of rte_device change)
> >   - unroll snprintf loop in rte_eal_hotplug_add
> > 
> > Changes in v5:
> >   - rte_devargs_remove is fixed in case of null devargs (patch 2)
> >   - a pointer to the bus is added in rte_device (patch 3)
> >   - rte_dev_remove is fixed in case of no devargs (patch 5)
> > 
> > Changes in v4 - after Andrew's review:
> >   - add API changes in release notes (patches 1 & 2)
> >   - fix memory leak in rte_eal_hotplug_add (patch 4)
> > 
> > Change in v3:
> >   - fix null dereferencing in error path (patch 2)
> > 
> > 
> > Thomas Monjalon (5):
> >   devargs: remove deprecated functions
> >   devargs: simplify parameters of removal function
> >   eal: add bus pointer in device structure
> >   eal: remove experimental flag of hotplug functions
> >   eal: simplify parameters of hotplug functions
> 
> I like these changes.
> 
> Reviewed-by: Stephen Hemminger <stephen@networkplumber.org>

Applied