[PATCH 0/9] bus: attach / detach API

[PATCH 0/9] bus: attach / detach API

[Gaetan Rivet]

This patchset introduces the attach / detach API to rte_bus.
The rte_device structure is used as the generic device representation.

This API is implemented for the virtual bus.
The functions rte_eal_dev_attach and rte_eal_dev_detach are updated to
use this new interface.

Jan Blunck (9):
  bus: add bus iterator to find a particular bus
  bus: add device iterator
  bus: add helper to find bus for a particular device
  bus: add bus helper iterator to find a particular device
  bus: introduce attach/detach functionality
  vdev: implement find_device bus operation
  vdev: implement detach bus operation
  eal: make virtual driver probe and remove take rte_vdev_device
  ethdev: Use embedded rte_device to detach driver

 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   4 +
 lib/librte_eal/common/eal_common_bus.c          |  66 ++++++++++++++++
 lib/librte_eal/common/eal_common_dev.c          | 100 ++++++++++++++++++------
 lib/librte_eal/common/eal_common_vdev.c         |  27 +++++++
 lib/librte_eal/common/include/rte_bus.h         |  75 ++++++++++++++++++
 lib/librte_eal/common/include/rte_dev.h         |  11 +++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   3 +
 lib/librte_ether/rte_ethdev.c                   |   3 +-
 8 files changed, 266 insertions(+), 23 deletions(-)

-- 
2.1.4

[PATCH 1/9] bus: add bus iterator to find a particular bus

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 14 ++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 20 ++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 36 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 2e48a73..ed09ab2 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -162,6 +162,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_bus_find;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 8f9baf8..5e33129 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -145,3 +145,17 @@ rte_bus_dump(FILE *f)
 		}
 	}
 }
+
+struct rte_bus *
+rte_bus_find(int (*match)(const struct rte_bus *bus, const void *data),
+	const void *data)
+{
+	struct rte_bus *bus = NULL;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		if (match(bus, data))
+			break;
+	}
+
+	return bus;
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 7c36969..66856bb 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -141,6 +141,26 @@ int rte_bus_probe(void);
 void rte_bus_dump(FILE *f);
 
 /**
+ * Bus iterator to find a particular bus.
+ *
+ * The callback function should return 0 if the bus doesn't match and non-zero
+ * if it does. If the callback returns non-zero this function will stop
+ * iterating over any more buses.
+ *
+ * @param match
+ *	 Callback function to check bus
+ *
+ * @param data
+ *	 Data to pass to match callback
+ *
+ * @return
+ *	 A pointer to a rte_bus structure or NULL in case no bus matches
+ */
+struct rte_bus *rte_bus_find(
+	int (*match)(const struct rte_bus *bus, const void *data),
+	const void *data);
+
+/**
  * Helper for Bus registration.
  * The constructor has higher priority than PMD constructors.
  */
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 670bab3..6efa517 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -166,6 +166,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_bus_find;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH 2/9] bus: add device iterator

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/common/eal_common_bus.c  | 1 +
 lib/librte_eal/common/include/rte_bus.h | 8 ++++++++
 2 files changed, 9 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 5e33129..41926fb 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -50,6 +50,7 @@ rte_bus_register(struct rte_bus *bus)
 	/* A bus should mandatorily have the scan implemented */
 	RTE_VERIFY(bus->scan);
 	RTE_VERIFY(bus->probe);
+	RTE_VERIFY(bus->find_device);
 
 	TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
 	RTE_LOG(DEBUG, EAL, "Registered [%s] bus.\n", bus->name);
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 66856bb..fe31f03 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -82,6 +82,13 @@ typedef int (*rte_bus_scan_t)(void);
 typedef int (*rte_bus_probe_t)(void);
 
 /**
+ * Device iterator to find a particular device on a bus.
+ */
+typedef struct rte_device * (*rte_bus_find_device_t)(
+	int (*match)(const struct rte_device *dev, const void *data),
+	const void *data);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -89,6 +96,7 @@ struct rte_bus {
 	const char *name;            /**< Name of the bus */
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
+	rte_bus_find_device_t find_device; /**< Find device on bus */
 };
 
 /**
-- 
2.1.4

[PATCH 3/9] bus: add helper to find bus for a particular device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 22 ++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         |  5 +++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 29 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index ed09ab2..f1a0765 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -163,6 +163,7 @@ DPDK_17.05 {
 	global:
 
 	rte_bus_find;
+	rte_bus_find_by_device;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 41926fb..97bcd65 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -160,3 +160,25 @@ rte_bus_find(int (*match)(const struct rte_bus *bus, const void *data),
 
 	return bus;
 }
+
+static int
+cmp_rte_device(const struct rte_device *dev, const void *_dev2)
+{
+	const struct rte_device *dev2 = _dev2;
+
+	return dev == dev2;
+}
+
+static int
+bus_find_device(const struct rte_bus *bus, const void *_dev)
+{
+	struct rte_device *dev;
+
+	dev = bus->find_device(cmp_rte_device, _dev);
+	return !!dev;
+}
+
+struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
+{
+	return rte_bus_find(bus_find_device, (const void *)dev);
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index fe31f03..1707a1f 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -169,6 +169,11 @@ struct rte_bus *rte_bus_find(
 	const void *data);
 
 /**
+ * Find the registered bus for a particular device.
+ */
+struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
+
+/**
  * Helper for Bus registration.
  * The constructor has higher priority than PMD constructors.
  */
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 6efa517..6f77222 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -167,6 +167,7 @@ DPDK_17.05 {
 	global:
 
 	rte_bus_find;
+	rte_bus_find_by_device;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH 4/9] bus: add bus helper iterator to find a particular device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 26 +++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 26 +++++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 54 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index f1a0765..21640d6 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -164,6 +164,7 @@ DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 97bcd65..b8f8384 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -182,3 +182,29 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
 {
 	return rte_bus_find(bus_find_device, (const void *)dev);
 }
+
+struct rte_device *
+rte_bus_find_device(const struct rte_device *start,
+	int (*match)(const struct rte_device *dev, const void *data),
+	const void *data)
+{
+	struct rte_bus *bus;
+	struct rte_device *dev = NULL;
+
+	if (start) {
+		bus = rte_bus_find_by_device(start);
+		if (!bus)
+			return NULL;
+	} else
+		bus = TAILQ_FIRST(&rte_bus_list);
+
+	while (bus) {
+		dev = bus->find_device(match, data);
+		if (dev)
+			break;
+
+		TAILQ_NEXT(bus, next);
+	}
+
+	return dev;
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 1707a1f..3b22fb8 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -169,6 +169,32 @@ struct rte_bus *rte_bus_find(
 	const void *data);
 
 /**
+ * Bus iterator to find a particular device.
+ *
+ * The callback function should return 0 if the device doesn't match and
+ * non-zero if it does. If the callback returns non-zero this function will
+ * stop iterating over any more buses and devices.
+ * To continue a search the device of a previous search is passed via the start
+ * parameters.
+ *
+ * @param start
+ *	 Start device of the iteration
+ *
+ * @param match
+ *	 Callback function to check device
+ *
+ * @param data
+ *	 Data to pass to match callback
+ *
+ * @return
+ *	 A pointer to a rte_bus structure or NULL in case no bus matches
+ */
+struct rte_device *
+rte_bus_find_device(const struct rte_device *start,
+	int (*match)(const struct rte_device *dev, const void *data),
+	const void *data);
+
+/**
  * Find the registered bus for a particular device.
  */
 struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 6f77222..e0a056d 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -168,6 +168,7 @@ DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH 5/9] bus: introduce attach/detach functionality

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/common/eal_common_bus.c  |  3 +++
 lib/librte_eal/common/include/rte_bus.h | 16 ++++++++++++++++
 2 files changed, 19 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index b8f8384..9fda287 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -52,6 +52,9 @@ rte_bus_register(struct rte_bus *bus)
 	RTE_VERIFY(bus->probe);
 	RTE_VERIFY(bus->find_device);
 
+	/* Buses supporting attach also require detach */
+	RTE_VERIFY(!bus->attach || bus->detach);
+
 	TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
 	RTE_LOG(DEBUG, EAL, "Registered [%s] bus.\n", bus->name);
 }
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 3b22fb8..201b0ff 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -89,6 +89,20 @@ typedef struct rte_device * (*rte_bus_find_device_t)(
 	const void *data);
 
 /**
+ * Implementation specific probe function which is responsible for linking
+ * devices on that bus with applicable drivers.
+ *
+ */
+typedef int (*rte_bus_attach_t)(struct rte_device *dev);
+
+/**
+ * Implementation specific remove function which is responsible for unlinking
+ * devices on that bus from assigned driver.
+ *
+ */
+typedef int (*rte_bus_detach_t)(struct rte_device *dev);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -97,6 +111,8 @@ struct rte_bus {
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
 	rte_bus_find_device_t find_device; /**< Find device on bus */
+	rte_bus_attach_t attach;     /**< Probe single device for drivers */
+	rte_bus_detach_t detach;     /**< Remove single device from driver */
 };
 
 /**
-- 
2.1.4

[PATCH 6/9] vdev: implement find_device bus operation

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/common/eal_common_vdev.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index 0037a64..5fc516f 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -338,9 +338,24 @@ vdev_probe(void)
 	return 0;
 }
 
+static struct rte_device *
+vdev_find_device(int (*match)(const struct rte_device *dev, const void *data),
+	const void *data)
+{
+	struct rte_vdev_device *dev;
+
+	TAILQ_FOREACH(dev, &vdev_device_list, next) {
+		if (match(&dev->device, data))
+			return &dev->device;
+	}
+
+	return NULL;
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
+	.find_device = vdev_find_device,
 };
 
 RTE_INIT(rte_vdev_bus_register);
-- 
2.1.4

[PATCH 7/9] vdev: implement detach bus operation

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/common/eal_common_vdev.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index 5fc516f..2095b01 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -352,10 +352,22 @@ vdev_find_device(int (*match)(const struct rte_device *dev, const void *data),
 	return NULL;
 }
 
+static int
+vdev_detach(struct rte_device *dev)
+{
+	/*
+	 * The virtual bus doesn't support 'unattached' devices so this is
+	 * actually equal to hotplugging removal of it.
+	 */
+	return rte_vdev_uninit(dev->name);
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
 	.find_device = vdev_find_device,
+	/* .attach = NULL, see comment on vdev_detach */
+	.detach = vdev_detach,
 };
 
 RTE_INIT(rte_vdev_bus_register);
-- 
2.1.4

[PATCH 8/9] eal: make virtual driver probe and remove take rte_vdev_device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

This is a preparation to embed the generic rte_device into the rte_eth_dev
also for virtual devices.

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/common/eal_common_dev.c | 93 ++++++++++++++++++++++++++--------
 1 file changed, 71 insertions(+), 22 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index a400ddd..c2d0f8d 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -37,6 +37,7 @@
 #include <inttypes.h>
 #include <sys/queue.h>
 
+#include <rte_bus.h>
 #include <rte_dev.h>
 #include <rte_devargs.h>
 #include <rte_debug.h>
@@ -45,50 +46,98 @@
 
 #include "eal_private.h"
 
+static int cmp_detached_dev_name(const struct rte_device *dev,
+	const void *_name)
+{
+	const char *name = _name;
+
+	/* skip attached devices */
+	if (dev->driver)
+		return 0;
+
+	return !strcmp(dev->name, name);
+}
+
 int rte_eal_dev_attach(const char *name, const char *devargs)
 {
-	struct rte_pci_addr addr;
+	struct rte_device *dev;
+	int ret;
 
 	if (name == NULL || devargs == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device or arguments provided\n");
 		return -EINVAL;
 	}
 
-	if (eal_parse_pci_DomBDF(name, &addr) == 0) {
-		if (rte_pci_probe_one(&addr) < 0)
-			goto err;
+	dev = rte_bus_find_device(NULL, cmp_detached_dev_name, name);
+	if (dev) {
+		struct rte_bus *bus;
+
+		bus = rte_bus_find_by_device(dev);
+		if (!bus) {
+			RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n",
+				name);
+			return -EINVAL;
+		}
 
-	} else {
-		if (rte_vdev_init(name, devargs))
-			goto err;
+		if (!bus->attach) {
+			RTE_LOG(ERR, EAL, "Bus function not supported\n");
+			return -ENOTSUP;
+		}
+
+		ret = bus->attach(dev);
+		goto out;
 	}
 
-	return 0;
+	/*
+	 * If we haven't found a bus device the user meant to "hotplug" a
+	 * virtual device instead.
+	 */
+	ret = rte_vdev_init(name, devargs);
+out:
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
+			name);
+	return ret;
+}
+
+static int cmp_dev_name(const struct rte_device *dev, const void *_name)
+{
+	const char *name = _name;
 
-err:
-	RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", name);
-	return -EINVAL;
+	return !strcmp(dev->name, name);
 }
 
 int rte_eal_dev_detach(const char *name)
 {
-	struct rte_pci_addr addr;
+	struct rte_device *dev;
+	struct rte_bus *bus;
+	int ret;
 
 	if (name == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device provided.\n");
 		return -EINVAL;
 	}
 
-	if (eal_parse_pci_DomBDF(name, &addr) == 0) {
-		if (rte_pci_detach(&addr) < 0)
-			goto err;
-	} else {
-		if (rte_vdev_uninit(name))
-			goto err;
+	dev = rte_bus_find_device(NULL, cmp_dev_name, name);
+	if (!dev) {
+		RTE_LOG(ERR, EAL, "Cannot find device (%s)\n", name);
+		return -EINVAL;
+	}
+
+	bus = rte_bus_find_by_device(dev);
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n", name);
+		return -EINVAL;
+	}
+
+	if (!bus->detach) {
+		RTE_LOG(ERR, EAL, "Bus function not supported\n");
+		return -ENOTSUP;
 	}
-	return 0;
 
-err:
-	RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n", name);
-	return -EINVAL;
+	ret = bus->detach(dev);
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
+			name);
+	return ret;
 }
-- 
2.1.4

[PATCH 9/9] ethdev: Use embedded rte_device to detach driver

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map |  1 +
 lib/librte_eal/common/eal_common_dev.c        | 43 ++++++++++++++++-----------
 lib/librte_eal/common/include/rte_dev.h       | 11 +++++++
 lib/librte_ether/rte_ethdev.c                 |  3 +-
 4 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 21640d6..150b0f7 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -162,6 +162,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_eal_device_detach;
 	rte_bus_find;
 	rte_bus_find_by_device;
 	rte_bus_find_device;
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index c2d0f8d..829d7f3 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -100,6 +100,30 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
 	return ret;
 }
 
+int rte_eal_device_detach(struct rte_device *dev)
+{
+	struct rte_bus *bus;
+	int ret;
+
+	bus = rte_bus_find_by_device(dev);
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n",
+			dev->name);
+		return -EINVAL;
+	}
+
+	if (!bus->detach) {
+		RTE_LOG(ERR, EAL, "Bus function not supported\n");
+		return -ENOTSUP;
+	}
+
+	ret = bus->detach(dev);
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
+			dev->name);
+	return ret;
+}
+
 static int cmp_dev_name(const struct rte_device *dev, const void *_name)
 {
 	const char *name = _name;
@@ -110,8 +134,6 @@ static int cmp_dev_name(const struct rte_device *dev, const void *_name)
 int rte_eal_dev_detach(const char *name)
 {
 	struct rte_device *dev;
-	struct rte_bus *bus;
-	int ret;
 
 	if (name == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device provided.\n");
@@ -124,20 +146,5 @@ int rte_eal_dev_detach(const char *name)
 		return -EINVAL;
 	}
 
-	bus = rte_bus_find_by_device(dev);
-	if (!bus) {
-		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n", name);
-		return -EINVAL;
-	}
-
-	if (!bus->detach) {
-		RTE_LOG(ERR, EAL, "Bus function not supported\n");
-		return -ENOTSUP;
-	}
-
-	ret = bus->detach(dev);
-	if (ret)
-		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
-			name);
-	return ret;
+	return rte_eal_device_detach(dev);
 }
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index de20c06..7092b16 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -123,6 +123,17 @@ struct rte_mem_resource {
 	void *addr;         /**< Virtual address, NULL when not mapped. */
 };
 
+/* FIXME: Forward declare */
+struct rte_device;
+
+/**
+ * Unplug the device from the device driver.
+ *
+ * @param dev
+ *   A pointer to a rte_device structure.
+ */
+int rte_eal_device_detach(struct rte_device *dev);
+
 /**
  * A structure describing a device driver.
  */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 83898a8..ff4f5ab 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -440,7 +440,8 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
 
 	snprintf(name, sizeof(rte_eth_devices[port_id].data->name),
 		 "%s", rte_eth_devices[port_id].data->name);
-	ret = rte_eal_dev_detach(name);
+
+	ret = rte_eal_device_detach(rte_eth_devices[0].device);
 	if (ret < 0)
 		goto err;
 
-- 
2.1.4

[PATCH v2 00/11] bus: attach / detach API

[Gaetan Rivet]

Following the work from Jan:

This patchset introduces the attach / detach API to rte_bus.
The rte_device structure is used as the generic device representation.

This API is implemented for the virtual bus.
The functions rte_eal_dev_attach and rte_eal_dev_detach are updated to
use this new interface.

--

0. API rework
-------------

I would like to propose an evolution on the API developed by Jan.

The attach / detach rte_bus API is necessary to support the attach/detach
rte_dev API. Those are two different levels for one similar functionality.

Attach / detach does not allow true hotplugging, because the attach
function expects the devices operated upon to already exist within the
buses / sub-layers. This means that this API expects devices meta-datas
(bus-internal device representation and associated device information
read from the system) to be present upon attach. This part of the work
is done during scanning.

While it is best to avoid changing the public rte_dev API as it already
exists, nothing prevents this new rte_bus API from superseeding it.
It has been said during the previous release cycle that device hotplug
was a feature that interested users. True hotplug is not allowed by the
current attach / detach API. Worse, this API hinders the effort to bring
this new functionality by squatting its semantic field.

Thus, I propose to rename rte_bus attach / detach; plug / unplug. As it
is a superset of the attach / detach functionality, it can be used to
implement rte_dev attach / detach. Now is the right time to pivot to
this new feature.

This should help maintainers understanding the aim of this API and the
differences with the APIs higher-up, clarify the field and allow a new
functionality to be proposed.

The vdev bus is inherently supporting the new API, however it has been
made explicit. My implementation in the PCI bus in further patchset also
follows the rte_bus hotplug API instead of only attach / detach.

One remaining problem with the vdev bus is the rte_dev attach
implementation, which needs the rte_devargs rework to be properly fixed.

1. Additional evolutions in the patchset
----------------------------------------

The RTE_VERIFY on the find_device is too stringent I think and forces
all buses to implement a public device iterator. While it could be
argued that it would push for quicker support for the functionality, I
think it's possible that some buses are not interested at all in it and
should simply be ignored.

The bus devices iterator has been fixed.

The internal rte_device handle was not properly setup within the
net_ring PMD.

Gaetan Rivet (2):
  vdev: implement hotplug functionality
  net/ring: fix dev handle in eth_dev

Jan Blunck (9):
  bus: add bus iterator to find a particular bus
  bus: add device iterator
  bus: add helper to find bus for a particular device
  bus: add bus helper iterator to find a particular device
  bus: introduce hotplug functionality
  vdev: implement find_device bus operation
  vdev: implement unplug bus operation
  eal: make virtual driver probe and remove take rte_vdev_device
  ethdev: Use embedded rte_device to detach driver

 drivers/net/ring/rte_eth_ring.c                 |   7 ++
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   4 +
 lib/librte_eal/common/eal_common_bus.c          |  65 +++++++++++++++
 lib/librte_eal/common/eal_common_dev.c          | 100 ++++++++++++++++++------
 lib/librte_eal/common/eal_common_vdev.c         |  27 +++++++
 lib/librte_eal/common/include/rte_bus.h         |  87 +++++++++++++++++++++
 lib/librte_eal/common/include/rte_dev.h         |  26 ++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   3 +
 lib/librte_ether/rte_ethdev.c                   |   3 +-
 9 files changed, 299 insertions(+), 23 deletions(-)

-- 
2.1.4

[PATCH v2 01/11] bus: add bus iterator to find a particular bus

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 13 ++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 32 +++++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 47 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 2e48a73..ed09ab2 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -162,6 +162,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_bus_find;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 8f9baf8..68f70d0 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -145,3 +145,16 @@ rte_bus_dump(FILE *f)
 		}
 	}
 }
+
+struct rte_bus *
+rte_bus_find(rte_bus_match_t match, const void *data)
+{
+	struct rte_bus *bus = NULL;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		if (match(bus, data))
+			break;
+	}
+
+	return bus;
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 7c36969..006feca 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -141,6 +141,38 @@ int rte_bus_probe(void);
 void rte_bus_dump(FILE *f);
 
 /**
+ * Bus match function.
+ *
+ * @param bus
+ *	bus under test.
+ *
+ * @param data
+ *	data matched
+ *
+ * @return
+ *	0 if the bus does not match.
+ *	!0 if the bus matches.
+ */
+typedef int (*rte_bus_match_t)(const struct rte_bus *bus, const void *data);
+
+/**
+ * Bus iterator to find a particular bus.
+ *
+ * If the callback returns non-zero this function will stop iterating over
+ * any more buses.
+ *
+ * @param match
+ *	 Callback function to check bus
+ *
+ * @param data
+ *	 Data to pass to match callback
+ *
+ * @return
+ *	 A pointer to a rte_bus structure or NULL in case no bus matches
+ */
+struct rte_bus *rte_bus_find(rte_bus_match_t match, const void *data);
+
+/**
  * Helper for Bus registration.
  * The constructor has higher priority than PMD constructors.
  */
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 670bab3..6efa517 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -166,6 +166,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_bus_find;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH v2 02/11] bus: add device iterator

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/include/rte_bus.h |  7 +++++++
 lib/librte_eal/common/include/rte_dev.h | 15 +++++++++++++++
 2 files changed, 22 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 006feca..4545b03 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -82,6 +82,12 @@ typedef int (*rte_bus_scan_t)(void);
 typedef int (*rte_bus_probe_t)(void);
 
 /**
+ * Device iterator to find a particular device on a bus.
+ */
+typedef struct rte_device * (*rte_bus_find_device_t)(rte_dev_match_t match,
+						     const void *data);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -89,6 +95,7 @@ struct rte_bus {
 	const char *name;            /**< Name of the bus */
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
+	rte_bus_find_device_t find_device; /**< Find device on bus */
 };
 
 /**
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index de20c06..f017814 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -191,6 +191,21 @@ int rte_eal_dev_attach(const char *name, const char *devargs);
  */
 int rte_eal_dev_detach(const char *name);
 
+/**
+ * Device match function.
+ *
+ * @param dev
+ *   Device handle.
+ *
+ * @param data
+ *   Data to match against.
+ *
+ * @return
+ *   0 if the device does not match.
+ *   !0 otherwise.
+ */
+typedef int (*rte_dev_match_t)(const struct rte_device *dev, const void *data);
+
 #define RTE_PMD_EXPORT_NAME_ARRAY(n, idx) n##idx[]
 
 #define RTE_PMD_EXPORT_NAME(name, idx) \
-- 
2.1.4

[PATCH v2 03/11] bus: add helper to find bus for a particular device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 24 ++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         |  5 +++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 31 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index ed09ab2..f1a0765 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -163,6 +163,7 @@ DPDK_17.05 {
 	global:
 
 	rte_bus_find;
+	rte_bus_find_by_device;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 68f70d0..811aff3 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -158,3 +158,27 @@ rte_bus_find(rte_bus_match_t match, const void *data)
 
 	return bus;
 }
+
+static int
+cmp_rte_device(const struct rte_device *dev, const void *_dev2)
+{
+	const struct rte_device *dev2 = _dev2;
+
+	return dev == dev2;
+}
+
+static int
+bus_find_device(const struct rte_bus *bus, const void *_dev)
+{
+	struct rte_device *dev;
+
+	if (!bus->find_device)
+		return 0;
+	dev = bus->find_device(cmp_rte_device, _dev);
+	return !!dev;
+}
+
+struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
+{
+	return rte_bus_find(bus_find_device, (const void *)dev);
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 4545b03..0664662 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -180,6 +180,11 @@ typedef int (*rte_bus_match_t)(const struct rte_bus *bus, const void *data);
 struct rte_bus *rte_bus_find(rte_bus_match_t match, const void *data);
 
 /**
+ * Find the registered bus for a particular device.
+ */
+struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
+
+/**
  * Helper for Bus registration.
  * The constructor has higher priority than PMD constructors.
  */
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 6efa517..6f77222 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -167,6 +167,7 @@ DPDK_17.05 {
 	global:
 
 	rte_bus_find;
+	rte_bus_find_by_device;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH v2 04/11] bus: add bus helper iterator to find a particular device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 25 +++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 23 +++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 50 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index f1a0765..21640d6 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -164,6 +164,7 @@ DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 811aff3..632a6e7 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -182,3 +182,28 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
 {
 	return rte_bus_find(bus_find_device, (const void *)dev);
 }
+
+struct rte_device *
+rte_bus_find_device(const struct rte_device *start,
+		    rte_dev_match_t match, const void *data)
+{
+	struct rte_bus *bus;
+	struct rte_device *dev = NULL;
+	int start_seen = 0;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		if (!bus->find_device)
+			continue;
+		if (!!start ^ start_seen) {
+			dev = bus->find_device(cmp_rte_device, start);
+			if (dev)
+				start_seen = 1;
+			else
+				continue;
+		}
+		dev = bus->find_device(match, data);
+		if (dev)
+			break;
+	}
+	return dev;
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 0664662..b56018f 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -180,6 +180,29 @@ typedef int (*rte_bus_match_t)(const struct rte_bus *bus, const void *data);
 struct rte_bus *rte_bus_find(rte_bus_match_t match, const void *data);
 
 /**
+ * Bus iterator to find a particular device.
+ *
+ * If the callback returns non-zero this function will stop iterating over any
+ * more buses and devices. To continue a search the device of a previous search
+ * is passed via the start parameters.
+ *
+ * @param start
+ *	 Start device of the iteration
+ *
+ * @param match
+ *	 Callback function to check device
+ *
+ * @param data
+ *	 Data to pass to match callback
+ *
+ * @return
+ *	 A pointer to a rte_bus structure or NULL in case no bus matches
+ */
+struct rte_device *
+rte_bus_find_device(const struct rte_device *start,
+		    rte_dev_match_t match, const void *data);
+
+/**
  * Find the registered bus for a particular device.
  */
 struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 6f77222..e0a056d 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -168,6 +168,7 @@ DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH v2 05/11] bus: introduce hotplug functionality

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_bus.c  |  3 +++
 lib/librte_eal/common/include/rte_bus.h | 20 ++++++++++++++++++++
 2 files changed, 23 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 632a6e7..4063897 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -51,6 +51,9 @@ rte_bus_register(struct rte_bus *bus)
 	RTE_VERIFY(bus->scan);
 	RTE_VERIFY(bus->probe);
 
+	/* Buses supporting hotplug also require unplug */
+	RTE_VERIFY(!bus->plug || bus->unplug);
+
 	TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
 	RTE_LOG(DEBUG, EAL, "Registered [%s] bus.\n", bus->name);
 }
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index b56018f..3c59fc2 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -88,6 +88,24 @@ typedef struct rte_device * (*rte_bus_find_device_t)(rte_dev_match_t match,
 						     const void *data);
 
 /**
+ * Implementation specific probe function which is responsible for linking
+ * devices on that bus with applicable drivers.
+ * The plugged device might already have been used previously by the bus,
+ * in which case some buses might prefer to detect and re-use the relevant
+ * information pertaining to this device.
+ */
+typedef int (*rte_bus_plug_t)(struct rte_devargs *da);
+
+/**
+ * Implementation specific remove function which is responsible for unlinking
+ * devices on that bus from assigned driver.
+ * Some buses will expect the same device to be plugged-back in eventually.
+ * The decision to release the associated meta-data (full-fledged bus-specific
+ * device representation) is left to the discretion of the bus.
+ */
+typedef int (*rte_bus_unplug_t)(struct rte_devargs *da);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -96,6 +114,8 @@ struct rte_bus {
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
 	rte_bus_find_device_t find_device; /**< Find device on bus */
+	rte_bus_plug_t plug;         /**< Probe single device for drivers */
+	rte_bus_unplug_t unplug;     /**< Remove single device from driver */
 };
 
 /**
-- 
2.1.4

[PATCH v2 06/11] vdev: implement find_device bus operation

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_vdev.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index 0037a64..46f2b74 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -338,9 +338,22 @@ vdev_probe(void)
 	return 0;
 }
 
+static struct rte_device *
+vdev_find_device(rte_dev_match_t match, const void *data)
+{
+	struct rte_vdev_device *dev;
+
+	TAILQ_FOREACH(dev, &vdev_device_list, next) {
+		if (match(&dev->device, data))
+			return &dev->device;
+	}
+	return NULL;
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
+	.find_device = vdev_find_device,
 };
 
 RTE_INIT(rte_vdev_bus_register);
-- 
2.1.4

[PATCH v2 07/11] vdev: implement hotplug functionality

[Gaetan Rivet]

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

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index 46f2b74..813955e 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -350,10 +350,17 @@ vdev_find_device(rte_dev_match_t match, const void *data)
 	return NULL;
 }
 
+static int
+vdev_plug(struct rte_devargs *da)
+{
+	return rte_vdev_init(da->virt.drv_name, da->args);
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
 	.find_device = vdev_find_device,
+	.plug = vdev_plug,
 };
 
 RTE_INIT(rte_vdev_bus_register);
-- 
2.1.4

[PATCH v2 08/11] vdev: implement unplug bus operation

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_vdev.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index 813955e..67c6fff 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -356,11 +356,18 @@ vdev_plug(struct rte_devargs *da)
 	return rte_vdev_init(da->virt.drv_name, da->args);
 }
 
+static int
+vdev_unplug(struct rte_devargs *da)
+{
+	return rte_vdev_uninit(da->virt.drv_name);
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
 	.find_device = vdev_find_device,
 	.plug = vdev_plug,
+	.unplug = vdev_unplug,
 };
 
 RTE_INIT(rte_vdev_bus_register);
-- 
2.1.4

[PATCH v2 09/11] eal: make virtual driver probe and remove take rte_vdev_device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

This is a preparation to embed the generic rte_device into the rte_eth_dev
also for virtual devices.

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_dev.c | 93 ++++++++++++++++++++++++++--------
 1 file changed, 71 insertions(+), 22 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index a400ddd..7508b70 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -37,6 +37,7 @@
 #include <inttypes.h>
 #include <sys/queue.h>
 
+#include <rte_bus.h>
 #include <rte_dev.h>
 #include <rte_devargs.h>
 #include <rte_debug.h>
@@ -45,50 +46,98 @@
 
 #include "eal_private.h"
 
+static int cmp_detached_dev_name(const struct rte_device *dev,
+	const void *_name)
+{
+	const char *name = _name;
+
+	/* skip attached devices */
+	if (dev->driver)
+		return 0;
+
+	return !strcmp(dev->name, name);
+}
+
 int rte_eal_dev_attach(const char *name, const char *devargs)
 {
-	struct rte_pci_addr addr;
+	struct rte_device *dev;
+	int ret;
 
 	if (name == NULL || devargs == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device or arguments provided\n");
 		return -EINVAL;
 	}
 
-	if (eal_parse_pci_DomBDF(name, &addr) == 0) {
-		if (rte_pci_probe_one(&addr) < 0)
-			goto err;
+	dev = rte_bus_find_device(NULL, cmp_detached_dev_name, name);
+	if (dev) {
+		struct rte_bus *bus;
+
+		bus = rte_bus_find_by_device(dev);
+		if (!bus) {
+			RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n",
+				name);
+			return -EINVAL;
+		}
 
-	} else {
-		if (rte_vdev_init(name, devargs))
-			goto err;
+		if (!bus->plug) {
+			RTE_LOG(ERR, EAL, "Bus function not supported\n");
+			return -ENOTSUP;
+		}
+
+		ret = bus->plug(dev->devargs);
+		goto out;
 	}
 
-	return 0;
+	/*
+	 * If we haven't found a bus device the user meant to "hotplug" a
+	 * virtual device instead.
+	 */
+	ret = rte_vdev_init(name, devargs);
+out:
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
+			name);
+	return ret;
+}
+
+static int cmp_dev_name(const struct rte_device *dev, const void *_name)
+{
+	const char *name = _name;
 
-err:
-	RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", name);
-	return -EINVAL;
+	return !strcmp(dev->name, name);
 }
 
 int rte_eal_dev_detach(const char *name)
 {
-	struct rte_pci_addr addr;
+	struct rte_device *dev;
+	struct rte_bus *bus;
+	int ret;
 
 	if (name == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device provided.\n");
 		return -EINVAL;
 	}
 
-	if (eal_parse_pci_DomBDF(name, &addr) == 0) {
-		if (rte_pci_detach(&addr) < 0)
-			goto err;
-	} else {
-		if (rte_vdev_uninit(name))
-			goto err;
+	dev = rte_bus_find_device(NULL, cmp_dev_name, name);
+	if (!dev) {
+		RTE_LOG(ERR, EAL, "Cannot find device (%s)\n", name);
+		return -EINVAL;
+	}
+
+	bus = rte_bus_find_by_device(dev);
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n", name);
+		return -EINVAL;
+	}
+
+	if (!bus->unplug) {
+		RTE_LOG(ERR, EAL, "Bus function not supported\n");
+		return -ENOTSUP;
 	}
-	return 0;
 
-err:
-	RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n", name);
-	return -EINVAL;
+	ret = bus->unplug(dev->devargs);
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
+			name);
+	return ret;
 }
-- 
2.1.4

[PATCH v2 10/11] ethdev: Use embedded rte_device to detach driver

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map |  1 +
 lib/librte_eal/common/eal_common_dev.c        | 43 ++++++++++++++++-----------
 lib/librte_eal/common/include/rte_dev.h       | 11 +++++++
 lib/librte_ether/rte_ethdev.c                 |  3 +-
 4 files changed, 39 insertions(+), 19 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 21640d6..150b0f7 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -162,6 +162,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_eal_device_detach;
 	rte_bus_find;
 	rte_bus_find_by_device;
 	rte_bus_find_device;
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 7508b70..968c66e 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -100,6 +100,30 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
 	return ret;
 }
 
+int rte_eal_device_detach(struct rte_device *dev)
+{
+	struct rte_bus *bus;
+	int ret;
+
+	bus = rte_bus_find_by_device(dev);
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n",
+			dev->name);
+		return -EINVAL;
+	}
+
+	if (!bus->unplug) {
+		RTE_LOG(ERR, EAL, "Bus function not supported\n");
+		return -ENOTSUP;
+	}
+
+	ret = bus->unplug(dev->devargs);
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
+			dev->name);
+	return ret;
+}
+
 static int cmp_dev_name(const struct rte_device *dev, const void *_name)
 {
 	const char *name = _name;
@@ -110,8 +134,6 @@ static int cmp_dev_name(const struct rte_device *dev, const void *_name)
 int rte_eal_dev_detach(const char *name)
 {
 	struct rte_device *dev;
-	struct rte_bus *bus;
-	int ret;
 
 	if (name == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device provided.\n");
@@ -124,20 +146,5 @@ int rte_eal_dev_detach(const char *name)
 		return -EINVAL;
 	}
 
-	bus = rte_bus_find_by_device(dev);
-	if (!bus) {
-		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n", name);
-		return -EINVAL;
-	}
-
-	if (!bus->unplug) {
-		RTE_LOG(ERR, EAL, "Bus function not supported\n");
-		return -ENOTSUP;
-	}
-
-	ret = bus->unplug(dev->devargs);
-	if (ret)
-		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
-			name);
-	return ret;
+	return rte_eal_device_detach(dev);
 }
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index f017814..c530797 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -123,6 +123,17 @@ struct rte_mem_resource {
 	void *addr;         /**< Virtual address, NULL when not mapped. */
 };
 
+/* FIXME: Forward declare */
+struct rte_device;
+
+/**
+ * Unplug the device from the device driver.
+ *
+ * @param dev
+ *   A pointer to a rte_device structure.
+ */
+int rte_eal_device_detach(struct rte_device *dev);
+
 /**
  * A structure describing a device driver.
  */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 83898a8..ff4f5ab 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -440,7 +440,8 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
 
 	snprintf(name, sizeof(rte_eth_devices[port_id].data->name),
 		 "%s", rte_eth_devices[port_id].data->name);
-	ret = rte_eal_dev_detach(name);
+
+	ret = rte_eal_device_detach(rte_eth_devices[0].device);
 	if (ret < 0)
 		goto err;
 
-- 
2.1.4

[PATCH v2 11/11] net/ring: fix dev handle in eth_dev

[Gaetan Rivet]

The ring PMD uses special eth_dev allocators, which cannot be updated to
accept an rte_vdev_device.
Circumvent the limitation and store the rte_device handle in the
rte_eth_dev structure.

Fixes: 050fe6e9ff97 ("drivers/net: use ethdev allocation helper for
vdev")
Cc: stable@dpdk.org

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
Cc: Bruce Richardson <bruce.richardson@intel.com>
---
 drivers/net/ring/rte_eth_ring.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index 87d2258..9ebc60e 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -515,6 +515,8 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev)
 	RTE_LOG(INFO, PMD, "Initializing pmd_ring for %s\n", name);
 
 	if (params == NULL || params[0] == '\0') {
+		struct rte_eth_dev *eth_dev;
+
 		ret = eth_dev_ring_create(name, rte_socket_id(), DEV_CREATE);
 		if (ret == -1) {
 			RTE_LOG(INFO, PMD,
@@ -522,6 +524,11 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev)
 			ret = eth_dev_ring_create(name, rte_socket_id(),
 						  DEV_ATTACH);
 		}
+		/* find an ethdev entry */
+		eth_dev = rte_eth_dev_allocated(name);
+		if (eth_dev == NULL)
+			return -ENODEV;
+		eth_dev->device = &dev->device;
 	}
 	else {
 		kvlist = rte_kvargs_parse(params, valid_arguments);
-- 
2.1.4

Re: [PATCH v2 00/11] bus: attach / detach API

[Stephen Hemminger]

On Wed, 31 May 2017 15:17:45 +0200
Gaetan Rivet <gaetan.rivet@6wind.com> wrote:

> Following the work from Jan:
> 
> This patchset introduces the attach / detach API to rte_bus.
> The rte_device structure is used as the generic device representation.
> 
> This API is implemented for the virtual bus.
> The functions rte_eal_dev_attach and rte_eal_dev_detach are updated to
> use this new interface.
> 
> --
> 
> 0. API rework
> -------------
> 
> I would like to propose an evolution on the API developed by Jan.
> 
> The attach / detach rte_bus API is necessary to support the attach/detach
> rte_dev API. Those are two different levels for one similar functionality.
> 
> Attach / detach does not allow true hotplugging, because the attach
> function expects the devices operated upon to already exist within the
> buses / sub-layers. This means that this API expects devices meta-datas
> (bus-internal device representation and associated device information
> read from the system) to be present upon attach. This part of the work
> is done during scanning.
> 
> While it is best to avoid changing the public rte_dev API as it already
> exists, nothing prevents this new rte_bus API from superseeding it.
> It has been said during the previous release cycle that device hotplug
> was a feature that interested users. True hotplug is not allowed by the
> current attach / detach API. Worse, this API hinders the effort to bring
> this new functionality by squatting its semantic field.
> 
> Thus, I propose to rename rte_bus attach / detach; plug / unplug. As it
> is a superset of the attach / detach functionality, it can be used to
> implement rte_dev attach / detach. Now is the right time to pivot to
> this new feature.
> 
> This should help maintainers understanding the aim of this API and the
> differences with the APIs higher-up, clarify the field and allow a new
> functionality to be proposed.
> 
> The vdev bus is inherently supporting the new API, however it has been
> made explicit. My implementation in the PCI bus in further patchset also
> follows the rte_bus hotplug API instead of only attach / detach.
> 
> One remaining problem with the vdev bus is the rte_dev attach
> implementation, which needs the rte_devargs rework to be properly fixed.
> 
> 1. Additional evolutions in the patchset
> ----------------------------------------
> 
> The RTE_VERIFY on the find_device is too stringent I think and forces
> all buses to implement a public device iterator. While it could be
> argued that it would push for quicker support for the functionality, I
> think it's possible that some buses are not interested at all in it and
> should simply be ignored.
> 
> The bus devices iterator has been fixed.
> 
> The internal rte_device handle was not properly setup within the
> net_ring PMD.
> 
> Gaetan Rivet (2):
>   vdev: implement hotplug functionality
>   net/ring: fix dev handle in eth_dev
> 
> Jan Blunck (9):
>   bus: add bus iterator to find a particular bus
>   bus: add device iterator
>   bus: add helper to find bus for a particular device
>   bus: add bus helper iterator to find a particular device
>   bus: introduce hotplug functionality
>   vdev: implement find_device bus operation
>   vdev: implement unplug bus operation
>   eal: make virtual driver probe and remove take rte_vdev_device
>   ethdev: Use embedded rte_device to detach driver
> 
>  drivers/net/ring/rte_eth_ring.c                 |   7 ++
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   4 +
>  lib/librte_eal/common/eal_common_bus.c          |  65 +++++++++++++++
>  lib/librte_eal/common/eal_common_dev.c          | 100 ++++++++++++++++++------
>  lib/librte_eal/common/eal_common_vdev.c         |  27 +++++++
>  lib/librte_eal/common/include/rte_bus.h         |  87 +++++++++++++++++++++
>  lib/librte_eal/common/include/rte_dev.h         |  26 ++++++
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |   3 +
>  lib/librte_ether/rte_ethdev.c                   |   3 +-
>  9 files changed, 299 insertions(+), 23 deletions(-)
> 

LGTM

Maybe we should evolve it by having both rte_bus and  rte_dev API for one release and mark
the rte_dev API for attach/detach as deprecated?

Re: [PATCH v2 01/11] bus: add bus iterator to find a particular bus

[Shreyansh Jain]

Hello Gaetan,

On Wednesday 31 May 2017 06:47 PM, Gaetan Rivet wrote:
> From: Jan Blunck <jblunck@infradead.org>
> 
> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>   lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
>   lib/librte_eal/common/eal_common_bus.c          | 13 ++++++++++
>   lib/librte_eal/common/include/rte_bus.h         | 32 +++++++++++++++++++++++++
>   lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
>   4 files changed, 47 insertions(+)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 2e48a73..ed09ab2 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -162,6 +162,7 @@ DPDK_17.02 {
>   DPDK_17.05 {
>   	global:
>   
> +	rte_bus_find;
>   	rte_cpu_is_supported;
>   	rte_log_dump;
>   	rte_log_register;
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index 8f9baf8..68f70d0 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -145,3 +145,16 @@ rte_bus_dump(FILE *f)
>   		}
>   	}
>   }
> +
> +struct rte_bus *
> +rte_bus_find(rte_bus_match_t match, const void *data)
> +{
> +	struct rte_bus *bus = NULL;
> +
> +	TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +		if (match(bus, data))
> +			break;
> +	}
> +
> +	return bus;
> +}
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> index 7c36969..006feca 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -141,6 +141,38 @@ int rte_bus_probe(void);
>   void rte_bus_dump(FILE *f);
>   
>   /**
> + * Bus match function.
> + *
> + * @param bus
> + *	bus under test.
> + *
> + * @param data
> + *	data matched
> + *
> + * @return
> + *	0 if the bus does not match.
> + *	!0 if the bus matches.

One of the common match function implementation could be simply to match
a string. strcmp itself returns '0' for a successful match.
On the same lines, should this function return value be reversed?
-
0 if match
!0 if not a match
-
That way, people would not have to change either the way strcmp works,
for example, or the way various APIs expect '0' as success.

same for rte_device_match_t as well. (in next patch)

> + */
> +typedef int (*rte_bus_match_t)(const struct rte_bus *bus, const void *data);
> +
> +/**
> + * Bus iterator to find a particular bus.
> + *
> + * If the callback returns non-zero this function will stop iterating over
> + * any more buses.
> + *
> + * @param match
> + *	 Callback function to check bus
> + *
> + * @param data
> + *	 Data to pass to match callback
> + *
> + * @return
> + *	 A pointer to a rte_bus structure or NULL in case no bus matches
> + */
> +struct rte_bus *rte_bus_find(rte_bus_match_t match, const void *data);
> +
> +/**
>    * Helper for Bus registration.
>    * The constructor has higher priority than PMD constructors.
>    */
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 670bab3..6efa517 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -166,6 +166,7 @@ DPDK_17.02 {
>   DPDK_17.05 {
>   	global:
>   
> +	rte_bus_find;
>   	rte_cpu_is_supported;
>   	rte_intr_free_epoll_fd;
>   	rte_log_dump;
> 

Re: [PATCH v2 01/11] bus: add bus iterator to find a particular bus

[Gaëtan Rivet]

On Wed, Jun 07, 2017 at 12:36:53PM +0530, Shreyansh Jain wrote:
> Hello Gaetan,
> 
> On Wednesday 31 May 2017 06:47 PM, Gaetan Rivet wrote:
> >From: Jan Blunck <jblunck@infradead.org>
> >
> >Signed-off-by: Jan Blunck <jblunck@infradead.org>
> >Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> >---
> >  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
> >  lib/librte_eal/common/eal_common_bus.c          | 13 ++++++++++
> >  lib/librte_eal/common/include/rte_bus.h         | 32 +++++++++++++++++++++++++
> >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
> >  4 files changed, 47 insertions(+)
> >
> >diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> >index 2e48a73..ed09ab2 100644
> >--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> >+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> >@@ -162,6 +162,7 @@ DPDK_17.02 {
> >  DPDK_17.05 {
> >  	global:
> >+	rte_bus_find;
> >  	rte_cpu_is_supported;
> >  	rte_log_dump;
> >  	rte_log_register;
> >diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> >index 8f9baf8..68f70d0 100644
> >--- a/lib/librte_eal/common/eal_common_bus.c
> >+++ b/lib/librte_eal/common/eal_common_bus.c
> >@@ -145,3 +145,16 @@ rte_bus_dump(FILE *f)
> >  		}
> >  	}
> >  }
> >+
> >+struct rte_bus *
> >+rte_bus_find(rte_bus_match_t match, const void *data)
> >+{
> >+	struct rte_bus *bus = NULL;
> >+
> >+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
> >+		if (match(bus, data))
> >+			break;
> >+	}
> >+
> >+	return bus;
> >+}
> >diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> >index 7c36969..006feca 100644
> >--- a/lib/librte_eal/common/include/rte_bus.h
> >+++ b/lib/librte_eal/common/include/rte_bus.h
> >@@ -141,6 +141,38 @@ int rte_bus_probe(void);
> >  void rte_bus_dump(FILE *f);
> >  /**
> >+ * Bus match function.
> >+ *
> >+ * @param bus
> >+ *	bus under test.
> >+ *
> >+ * @param data
> >+ *	data matched
> >+ *
> >+ * @return
> >+ *	0 if the bus does not match.
> >+ *	!0 if the bus matches.
> 
> One of the common match function implementation could be simply to match
> a string. strcmp itself returns '0' for a successful match.
> On the same lines, should this function return value be reversed?
> -
> 0 if match
> !0 if not a match
> -
> That way, people would not have to change either the way strcmp works,
> for example, or the way various APIs expect '0' as success.
> 
> same for rte_device_match_t as well. (in next patch)
> 

It was actually a point I hesitated a little before submitting this
version.

The logic behind strcmp is that you can express three states: greater
than, equal, lower than, thus having total order within the string set.

Here, buses are not ordered (logically). Having a bus lower or greater
than some arbitrary data does not mean much.

Anyway, this was my reasoning for following Jan's proposal on this, but
I'm not against changing this API. Maybe having to possibility to
express total order could be useful eventually. I don't have a strong
opinion on this so unless someone shouts about it, I will follow your
remark.

> >+ */
> >+typedef int (*rte_bus_match_t)(const struct rte_bus *bus, const void *data);
> >+
> >+/**
> >+ * Bus iterator to find a particular bus.
> >+ *
> >+ * If the callback returns non-zero this function will stop iterating over
> >+ * any more buses.
> >+ *
> >+ * @param match
> >+ *	 Callback function to check bus
> >+ *
> >+ * @param data
> >+ *	 Data to pass to match callback
> >+ *
> >+ * @return
> >+ *	 A pointer to a rte_bus structure or NULL in case no bus matches
> >+ */
> >+struct rte_bus *rte_bus_find(rte_bus_match_t match, const void *data);
> >+
> >+/**
> >   * Helper for Bus registration.
> >   * The constructor has higher priority than PMD constructors.
> >   */
> >diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> >index 670bab3..6efa517 100644
> >--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> >+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> >@@ -166,6 +166,7 @@ DPDK_17.02 {
> >  DPDK_17.05 {
> >  	global:
> >+	rte_bus_find;
> >  	rte_cpu_is_supported;
> >  	rte_intr_free_epoll_fd;
> >  	rte_log_dump;
> >

-- 
Gaëtan Rivet
6WIND

Re: [PATCH v2 04/11] bus: add bus helper iterator to find a particular device

[Jan Blunck]

On Wed, May 31, 2017 at 3:17 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> From: Jan Blunck <jblunck@infradead.org>
>
> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
>  lib/librte_eal/common/eal_common_bus.c          | 25 +++++++++++++++++++++++++
>  lib/librte_eal/common/include/rte_bus.h         | 23 +++++++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
>  4 files changed, 50 insertions(+)
>
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index f1a0765..21640d6 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -164,6 +164,7 @@ DPDK_17.05 {
>
>         rte_bus_find;
>         rte_bus_find_by_device;
> +       rte_bus_find_device;
>         rte_cpu_is_supported;
>         rte_log_dump;
>         rte_log_register;
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index 811aff3..632a6e7 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -182,3 +182,28 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
>  {
>         return rte_bus_find(bus_find_device, (const void *)dev);
>  }
> +
> +struct rte_device *
> +rte_bus_find_device(const struct rte_device *start,
> +                   rte_dev_match_t match, const void *data)
> +{
> +       struct rte_bus *bus;
> +       struct rte_device *dev = NULL;
> +       int start_seen = 0;
> +
> +       TAILQ_FOREACH(bus, &rte_bus_list, next) {

I don't think that this should use the TAILQ_FOREACH() macro since it
makes the code nesting deeper unnecessarily.


> +               if (!bus->find_device)
> +                       continue;
> +               if (!!start ^ start_seen) {
> +                       dev = bus->find_device(cmp_rte_device, start);
> +                       if (dev)
> +                               start_seen = 1;
> +                       else
> +                               continue;
> +               }
> +               dev = bus->find_device(match, data);
> +               if (dev)
> +                       break;
> +       }
> +       return dev;
> +}
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> index 0664662..b56018f 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -180,6 +180,29 @@ typedef int (*rte_bus_match_t)(const struct rte_bus *bus, const void *data);
>  struct rte_bus *rte_bus_find(rte_bus_match_t match, const void *data);
>
>  /**
> + * Bus iterator to find a particular device.
> + *
> + * If the callback returns non-zero this function will stop iterating over any
> + * more buses and devices. To continue a search the device of a previous search
> + * is passed via the start parameters.
> + *
> + * @param start
> + *      Start device of the iteration
> + *
> + * @param match
> + *      Callback function to check device
> + *
> + * @param data
> + *      Data to pass to match callback
> + *
> + * @return
> + *      A pointer to a rte_bus structure or NULL in case no bus matches
> + */
> +struct rte_device *
> +rte_bus_find_device(const struct rte_device *start,
> +                   rte_dev_match_t match, const void *data);
> +
> +/**
>   * Find the registered bus for a particular device.
>   */
>  struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 6f77222..e0a056d 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -168,6 +168,7 @@ DPDK_17.05 {
>
>         rte_bus_find;
>         rte_bus_find_by_device;
> +       rte_bus_find_device;
>         rte_cpu_is_supported;
>         rte_intr_free_epoll_fd;
>         rte_log_dump;
> --
> 2.1.4
>

Re: [PATCH v2 01/11] bus: add bus iterator to find a particular bus

[Jan Blunck]

On Wed, Jun 7, 2017 at 3:27 PM, Gaëtan Rivet <gaetan.rivet@6wind.com> wrote:
> On Wed, Jun 07, 2017 at 12:36:53PM +0530, Shreyansh Jain wrote:
>> >diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
>> >index 7c36969..006feca 100644
>> >--- a/lib/librte_eal/common/include/rte_bus.h
>> >+++ b/lib/librte_eal/common/include/rte_bus.h
>> >@@ -141,6 +141,38 @@ int rte_bus_probe(void);
>> >  void rte_bus_dump(FILE *f);
>> >  /**
>> >+ * Bus match function.
>> >+ *
>> >+ * @param bus
>> >+ *  bus under test.
>> >+ *
>> >+ * @param data
>> >+ *  data matched
>> >+ *
>> >+ * @return
>> >+ *  0 if the bus does not match.
>> >+ *  !0 if the bus matches.
>>
>> One of the common match function implementation could be simply to match
>> a string. strcmp itself returns '0' for a successful match.
>> On the same lines, should this function return value be reversed?
>> -
>> 0 if match
>> !0 if not a match
>> -
>> That way, people would not have to change either the way strcmp works,
>> for example, or the way various APIs expect '0' as success.
>>
>> same for rte_device_match_t as well. (in next patch)
>>
>
> It was actually a point I hesitated a little before submitting this
> version.
>
> The logic behind strcmp is that you can express three states: greater
> than, equal, lower than, thus having total order within the string set.
>
> Here, buses are not ordered (logically). Having a bus lower or greater
> than some arbitrary data does not mean much.
>
> Anyway, this was my reasoning for following Jan's proposal on this, but
> I'm not against changing this API. Maybe having to possibility to
> express total order could be useful eventually. I don't have a strong
> opinion on this so unless someone shouts about it, I will follow your
> remark.
>

It is better to have consistency on the match/comparator behavior.
Also if it comes as a surprise to users lets change it.

Re: [PATCH v2 04/11] bus: add bus helper iterator to find a particular device

[Gaëtan Rivet]

On Wed, Jun 07, 2017 at 06:41:58PM +0200, Jan Blunck wrote:
> On Wed, May 31, 2017 at 3:17 PM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > From: Jan Blunck <jblunck@infradead.org>
> >
> > Signed-off-by: Jan Blunck <jblunck@infradead.org>
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> >  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
> >  lib/librte_eal/common/eal_common_bus.c          | 25 +++++++++++++++++++++++++
> >  lib/librte_eal/common/include/rte_bus.h         | 23 +++++++++++++++++++++++
> >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
> >  4 files changed, 50 insertions(+)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > index f1a0765..21640d6 100644
> > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > @@ -164,6 +164,7 @@ DPDK_17.05 {
> >
> >         rte_bus_find;
> >         rte_bus_find_by_device;
> > +       rte_bus_find_device;
> >         rte_cpu_is_supported;
> >         rte_log_dump;
> >         rte_log_register;
> > diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> > index 811aff3..632a6e7 100644
> > --- a/lib/librte_eal/common/eal_common_bus.c
> > +++ b/lib/librte_eal/common/eal_common_bus.c
> > @@ -182,3 +182,28 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
> >  {
> >         return rte_bus_find(bus_find_device, (const void *)dev);
> >  }
> > +
> > +struct rte_device *
> > +rte_bus_find_device(const struct rte_device *start,
> > +                   rte_dev_match_t match, const void *data)
> > +{
> > +       struct rte_bus *bus;
> > +       struct rte_device *dev = NULL;
> > +       int start_seen = 0;
> > +
> > +       TAILQ_FOREACH(bus, &rte_bus_list, next) {
> 
> I don't think that this should use the TAILQ_FOREACH() macro since it
> makes the code nesting deeper unnecessarily.
> 
> 

While debugging the previous version, I found rather difficult to follow
the nesting of iterators / callback calls.

I preferred laying all of it down and simplifying the flow. The nesting
is either within the same scope or divided among several scopes. I
proposed the former as I found it easier to write and read, but that's
subjective.

> > +               if (!bus->find_device)
> > +                       continue;
> > +               if (!!start ^ start_seen) {
> > +                       dev = bus->find_device(cmp_rte_device, start);
> > +                       if (dev)
> > +                               start_seen = 1;
> > +                       else
> > +                               continue;
> > +               }
> > +               dev = bus->find_device(match, data);
> > +               if (dev)
> > +                       break;
> > +       }
> > +       return dev;
> > +}
> > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > index 0664662..b56018f 100644
> > --- a/lib/librte_eal/common/include/rte_bus.h
> > +++ b/lib/librte_eal/common/include/rte_bus.h
> > @@ -180,6 +180,29 @@ typedef int (*rte_bus_match_t)(const struct rte_bus *bus, const void *data);
> >  struct rte_bus *rte_bus_find(rte_bus_match_t match, const void *data);
> >
> >  /**
> > + * Bus iterator to find a particular device.
> > + *
> > + * If the callback returns non-zero this function will stop iterating over any
> > + * more buses and devices. To continue a search the device of a previous search
> > + * is passed via the start parameters.
> > + *
> > + * @param start
> > + *      Start device of the iteration
> > + *
> > + * @param match
> > + *      Callback function to check device
> > + *
> > + * @param data
> > + *      Data to pass to match callback
> > + *
> > + * @return
> > + *      A pointer to a rte_bus structure or NULL in case no bus matches
> > + */
> > +struct rte_device *
> > +rte_bus_find_device(const struct rte_device *start,
> > +                   rte_dev_match_t match, const void *data);
> > +
> > +/**
> >   * Find the registered bus for a particular device.
> >   */
> >  struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
> > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > index 6f77222..e0a056d 100644
> > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > @@ -168,6 +168,7 @@ DPDK_17.05 {
> >
> >         rte_bus_find;
> >         rte_bus_find_by_device;
> > +       rte_bus_find_device;
> >         rte_cpu_is_supported;
> >         rte_intr_free_epoll_fd;
> >         rte_log_dump;
> > --
> > 2.1.4
> >

-- 
Gaëtan Rivet
6WIND

[PATCH v3 00/10] bus: attach / detach API

[Gaetan Rivet]

Following the work from Jan:

This patchset introduces the attach / detach API to rte_bus.
The rte_device structure is used as the generic device representation.

This API is implemented for the virtual bus.
The functions rte_eal_dev_attach and rte_eal_dev_detach are updated to
use this new interface.

-- v2

0. API rework
-------------

I would like to propose an evolution on the API developed by Jan.

The attach / detach rte_bus API is necessary to support the attach/detach
rte_dev API. Those are two different levels for one similar functionality.

Attach / detach does not allow true hotplugging, because the attach
function expects the devices operated upon to already exist within the
buses / sub-layers. This means that this API expects devices meta-datas
(bus-internal device representation and associated device information
read from the system) to be present upon attach. This part of the work
is done during scanning.

While it is best to avoid changing the public rte_dev API as it already
exists, nothing prevents this new rte_bus API from superseeding it.
It has been said during the previous release cycle that device hotplug
was a feature that interested users. True hotplug is not allowed by the
current attach / detach API. Worse, this API hinders the effort to bring
this new functionality by squatting its semantic field.

Thus, I propose to rename rte_bus attach / detach; plug / unplug. As it
is a superset of the attach / detach functionality, it can be used to
implement rte_dev attach / detach. Now is the right time to pivot to
this new feature.

This should help maintainers understanding the aim of this API and the
differences with the APIs higher-up, clarify the field and allow a new
functionality to be proposed.

The vdev bus is inherently supporting the new API, however it has been
made explicit. My implementation in the PCI bus in further patchset also
follows the rte_bus hotplug API instead of only attach / detach.

One remaining problem with the vdev bus is the rte_dev attach
implementation, which needs the rte_devargs rework to be properly fixed.

1. Additional evolutions in the patchset
----------------------------------------

The RTE_VERIFY on the find_device is too stringent I think and forces
all buses to implement a public device iterator. While it could be
argued that it would push for quicker support for the functionality, I
think it's possible that some buses are not interested at all in it and
should simply be ignored.

The bus devices iterator has been fixed.

The internal rte_device handle was not properly setup within the
net_ring PMD.

-- v3

The new API is now

typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);
typedef int (*rte_bus_unplug_t)(struct rte_device *dev);

So, plugging a device takes an rte_devargs as input and returns an rte_device.
While implementing related subsystems, I found that I usually needed
this rte_device handle upon a successful device plugging. This seems the
sensible and useful thing to do.
As such, on error NULL is returned and rte_errno is set by the bus.

Unplugging a device however now returns to the first version, which used
an rte_device. The explicit contract here is that if one has an
rte_device that has been obtained by calling bus->plug(), then this
handle can be used for bus->unplug().

Additionally, bus and device comparators now returns 0 on match,
following strcmp-like behavior.

Gaetan Rivet (2):
  vdev: implement hotplug functionality
  net/ring: fix dev handle in eth_dev

Jan Blunck (8):
  bus: add bus iterator to find a particular bus
  bus: add device iterator
  bus: add helper to find bus for a particular device
  bus: add bus helper iterator to find a particular device
  bus: introduce hotplug functionality
  vdev: implement find_device bus operation
  eal: make virtual driver probe and remove take rte_vdev_device
  ethdev: use embedded rte_device to detach driver

 drivers/net/ring/rte_eth_ring.c                 |   7 ++
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   4 +
 lib/librte_eal/common/eal_common_bus.c          |  62 +++++++++++++++
 lib/librte_eal/common/eal_common_dev.c          | 100 ++++++++++++++++++------
 lib/librte_eal/common/eal_common_vdev.c         |  49 ++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 100 ++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_dev.h         |  28 +++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   3 +
 lib/librte_ether/rte_ethdev.c                   |  31 +++-----
 9 files changed, 341 insertions(+), 43 deletions(-)

-- 
2.1.4

[PATCH v3 01/10] bus: add bus iterator to find a particular bus

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 11 ++++++++
 lib/librte_eal/common/include/rte_bus.h         | 34 +++++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 47 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 2e48a73..ed09ab2 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -162,6 +162,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_bus_find;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 8f9baf8..a54aeb4 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -145,3 +145,14 @@ rte_bus_dump(FILE *f)
 		}
 	}
 }
+
+struct rte_bus *
+rte_bus_find(rte_bus_cmp_t cmp, const void *data)
+{
+	struct rte_bus *bus = NULL;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next)
+		if (cmp(bus, data) == 0)
+			break;
+	return bus;
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 7c36969..16bcfd9 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -141,6 +141,40 @@ int rte_bus_probe(void);
 void rte_bus_dump(FILE *f);
 
 /**
+ * Bus comparison function.
+ *
+ * @param bus
+ *	Bus under test.
+ *
+ * @param data
+ *	Data to compare against.
+ *
+ * @return
+ *	0 if the bus matches the data.
+ *	!0 if the bus does not match.
+ *	<0 if ordering is possible and the bus is lower than the data.
+ *	>0 if ordering is possible and the bus is greater than the data.
+ */
+typedef int (*rte_bus_cmp_t)(const struct rte_bus *bus, const void *data);
+
+/**
+ * Bus iterator to find a particular bus.
+ *
+ * If the callback returns non-zero this function will stop iterating over
+ * any more buses.
+ *
+ * @param cmp
+ *	Comparison function.
+ *
+ * @param data
+ *	 Data to pass to cmp callback
+ *
+ * @return
+ *	 A pointer to a rte_bus structure or NULL in case no bus matches
+ */
+struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp, const void *data);
+
+/**
  * Helper for Bus registration.
  * The constructor has higher priority than PMD constructors.
  */
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 670bab3..6efa517 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -166,6 +166,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_bus_find;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH v3 02/10] bus: add device iterator

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/include/rte_bus.h |  7 +++++++
 lib/librte_eal/common/include/rte_dev.h | 17 +++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 16bcfd9..e3f867f 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -82,6 +82,12 @@ typedef int (*rte_bus_scan_t)(void);
 typedef int (*rte_bus_probe_t)(void);
 
 /**
+ * Device iterator to find a particular device on a bus.
+ */
+typedef struct rte_device * (*rte_bus_find_device_t)(rte_dev_cmp_t match,
+						     const void *data);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -89,6 +95,7 @@ struct rte_bus {
 	const char *name;            /**< Name of the bus */
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
+	rte_bus_find_device_t find_device; /**< Find device on bus */
 };
 
 /**
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index de20c06..724855e 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -191,6 +191,23 @@ int rte_eal_dev_attach(const char *name, const char *devargs);
  */
 int rte_eal_dev_detach(const char *name);
 
+/**
+ * Device comparison function.
+ *
+ * @param dev
+ *   Device handle.
+ *
+ * @param data
+ *   Data to compare against.
+ *
+ * @return
+ *   0 if the device matches the data.
+ *   !0 if the device does not match.
+ *   <0 if ordering is possible and the device is lower than the data.
+ *   >0 if ordering is possible and the device is greater than the data.
+ */
+typedef int (*rte_dev_cmp_t)(const struct rte_device *dev, const void *data);
+
 #define RTE_PMD_EXPORT_NAME_ARRAY(n, idx) n##idx[]
 
 #define RTE_PMD_EXPORT_NAME(name, idx) \
-- 
2.1.4

[PATCH v3 03/10] bus: add helper to find bus for a particular device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 24 ++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         |  5 +++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 31 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index ed09ab2..f1a0765 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -163,6 +163,7 @@ DPDK_17.05 {
 	global:
 
 	rte_bus_find;
+	rte_bus_find_by_device;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index a54aeb4..42f1076 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -156,3 +156,27 @@ rte_bus_find(rte_bus_cmp_t cmp, const void *data)
 			break;
 	return bus;
 }
+
+static int
+cmp_rte_device(const struct rte_device *dev, const void *_dev2)
+{
+	const struct rte_device *dev2 = _dev2;
+
+	return !(dev == dev2);
+}
+
+static int
+bus_find_device(const struct rte_bus *bus, const void *_dev)
+{
+	struct rte_device *dev;
+
+	if (!bus->find_device)
+		return -1;
+	dev = bus->find_device(cmp_rte_device, _dev);
+	return !dev;
+}
+
+struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
+{
+	return rte_bus_find(bus_find_device, (const void *)dev);
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index e3f867f..b3bc6e8 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -182,6 +182,11 @@ typedef int (*rte_bus_cmp_t)(const struct rte_bus *bus, const void *data);
 struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp, const void *data);
 
 /**
+ * Find the registered bus for a particular device.
+ */
+struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
+
+/**
  * Helper for Bus registration.
  * The constructor has higher priority than PMD constructors.
  */
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 6efa517..6f77222 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -167,6 +167,7 @@ DPDK_17.05 {
 	global:
 
 	rte_bus_find;
+	rte_bus_find_by_device;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH v3 04/10] bus: add bus helper iterator to find a particular device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 25 +++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 23 +++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 50 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index f1a0765..21640d6 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -164,6 +164,7 @@ DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 42f1076..451884f 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -180,3 +180,28 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
 {
 	return rte_bus_find(bus_find_device, (const void *)dev);
 }
+
+struct rte_device *
+rte_bus_find_device(const struct rte_device *start,
+		    rte_dev_cmp_t cmp, const void *data)
+{
+	struct rte_bus *bus;
+	struct rte_device *dev = NULL;
+	int start_seen = 0;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		if (!bus->find_device)
+			continue;
+		if (!!start ^ start_seen) {
+			dev = bus->find_device(cmp_rte_device, start);
+			if (dev)
+				start_seen = 1;
+			else
+				continue;
+		}
+		dev = bus->find_device(cmp, data);
+		if (dev)
+			break;
+	}
+	return dev;
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index b3bc6e8..eba407d 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -182,6 +182,29 @@ typedef int (*rte_bus_cmp_t)(const struct rte_bus *bus, const void *data);
 struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp, const void *data);
 
 /**
+ * Bus iterator to find a particular device.
+ *
+ * If the callback returns non-zero this function will stop iterating over any
+ * more buses and devices. To continue a search the device of a previous search
+ * is passed via the start parameters.
+ *
+ * @param start
+ *	 Start device of the iteration.
+ *
+ * @param cmp
+ *	 Callback function to check device.
+ *
+ * @param data
+ *	 Data to pass to match callback.
+ *
+ * @return
+ *	 A pointer to a rte_bus structure or NULL in case no bus matches.
+ */
+struct rte_device *
+rte_bus_find_device(const struct rte_device *start,
+		    rte_dev_cmp_t cmp, const void *data);
+
+/**
  * Find the registered bus for a particular device.
  */
 struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 6f77222..e0a056d 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -168,6 +168,7 @@ DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH v3 05/10] bus: introduce hotplug functionality

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_bus.c  |  2 ++
 lib/librte_eal/common/include/rte_bus.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 451884f..381a0b6 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -50,6 +50,8 @@ rte_bus_register(struct rte_bus *bus)
 	/* A bus should mandatorily have the scan implemented */
 	RTE_VERIFY(bus->scan);
 	RTE_VERIFY(bus->probe);
+	/* Buses supporting hotplug also require unplug. */
+	RTE_VERIFY(!bus->plug || bus->unplug);
 
 	TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
 	RTE_LOG(DEBUG, EAL, "Registered [%s] bus.\n", bus->name);
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index eba407d..cb44e46 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -88,6 +88,35 @@ typedef struct rte_device * (*rte_bus_find_device_t)(rte_dev_cmp_t match,
 						     const void *data);
 
 /**
+ * Implementation specific probe function which is responsible for linking
+ * devices on that bus with applicable drivers.
+ * The plugged device might already have been used previously by the bus,
+ * in which case some buses might prefer to detect and re-use the relevant
+ * information pertaining to this device.
+ *
+ * @param da
+ *	Device declaration.
+ *
+ * @return
+ *	The pointer to a valid rte_device usable by the bus on success.
+ *	NULL on error. rte_errno is then set.
+ */
+typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);
+
+/**
+ * Implementation specific remove function which is responsible for unlinking
+ * devices on that bus from assigned driver.
+ *
+ * @param dev
+ *	Device pointer that was returned by a previous device plug call.
+ *
+ * @return
+ *	0 on success.
+ *	!0 on error. rte_errno is then set.
+ */
+typedef int (*rte_bus_unplug_t)(struct rte_device *dev);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -96,6 +125,8 @@ struct rte_bus {
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
 	rte_bus_find_device_t find_device; /**< Find device on bus */
+	rte_bus_plug_t plug;         /**< Probe single device for drivers */
+	rte_bus_unplug_t unplug;     /**< Remove single device from driver */
 };
 
 /**
-- 
2.1.4

[PATCH v3 06/10] vdev: implement find_device bus operation

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_vdev.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index 0037a64..52528ef 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -338,9 +338,22 @@ vdev_probe(void)
 	return 0;
 }
 
+static struct rte_device *
+vdev_find_device(rte_dev_cmp_t cmp, const void *data)
+{
+	struct rte_vdev_device *dev;
+
+	TAILQ_FOREACH(dev, &vdev_device_list, next) {
+		if (cmp(&dev->device, data) == 0)
+			return &dev->device;
+	}
+	return NULL;
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
+	.find_device = vdev_find_device,
 };
 
 RTE_INIT(rte_vdev_bus_register);
-- 
2.1.4

[PATCH v3 07/10] vdev: implement hotplug functionality

[Gaetan Rivet]

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

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index 52528ef..22e4640 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -38,11 +38,13 @@
 #include <sys/queue.h>
 
 #include <rte_eal.h>
+#include <rte_dev.h>
 #include <rte_bus.h>
 #include <rte_vdev.h>
 #include <rte_common.h>
 #include <rte_devargs.h>
 #include <rte_memory.h>
+#include <rte_errno.h>
 
 /** Double linked list of virtual device drivers. */
 TAILQ_HEAD(vdev_device_list, rte_vdev_device);
@@ -350,10 +352,44 @@ vdev_find_device(rte_dev_cmp_t cmp, const void *data)
 	return NULL;
 }
 
+static struct rte_device *
+vdev_plug(struct rte_devargs *da)
+{
+	struct rte_vdev_device *dev;
+	int ret;
+
+	ret = rte_vdev_init(da->virt.drv_name, da->args);
+	if (ret) {
+		rte_errno = -ret;
+		return NULL;
+	}
+	dev = find_vdev(da->virt.drv_name);
+	return &dev->device;
+}
+
+static int
+vdev_unplug(struct rte_device *dev)
+{
+	struct rte_devargs *da;
+	int ret;
+
+	da = dev->devargs;
+	if (da == NULL) {
+		rte_errno = EINVAL;
+		return -1;
+	}
+	ret = rte_vdev_uninit(da->virt.drv_name);
+	if (ret)
+		rte_errno = -ret;
+	return ret;
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
 	.find_device = vdev_find_device,
+	.plug = vdev_plug,
+	.unplug = vdev_unplug,
 };
 
 RTE_INIT(rte_vdev_bus_register);
-- 
2.1.4

[PATCH v3 08/10] eal: make virtual driver probe and remove take rte_vdev_device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

This is a preparation to embed the generic rte_device into the rte_eth_dev
also for virtual devices.

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_dev.c | 93 ++++++++++++++++++++++++++--------
 1 file changed, 71 insertions(+), 22 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index a400ddd..733da91 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -37,6 +37,7 @@
 #include <inttypes.h>
 #include <sys/queue.h>
 
+#include <rte_bus.h>
 #include <rte_dev.h>
 #include <rte_devargs.h>
 #include <rte_debug.h>
@@ -45,50 +46,98 @@
 
 #include "eal_private.h"
 
+static int cmp_detached_dev_name(const struct rte_device *dev,
+	const void *_name)
+{
+	const char *name = _name;
+
+	/* skip attached devices */
+	if (dev->driver)
+		return 0;
+
+	return strcmp(dev->name, name);
+}
+
 int rte_eal_dev_attach(const char *name, const char *devargs)
 {
-	struct rte_pci_addr addr;
+	struct rte_device *dev;
+	int ret;
 
 	if (name == NULL || devargs == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device or arguments provided\n");
 		return -EINVAL;
 	}
 
-	if (eal_parse_pci_DomBDF(name, &addr) == 0) {
-		if (rte_pci_probe_one(&addr) < 0)
-			goto err;
+	dev = rte_bus_find_device(NULL, cmp_detached_dev_name, name);
+	if (dev) {
+		struct rte_bus *bus;
+
+		bus = rte_bus_find_by_device(dev);
+		if (!bus) {
+			RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n",
+				name);
+			return -EINVAL;
+		}
 
-	} else {
-		if (rte_vdev_init(name, devargs))
-			goto err;
+		if (!bus->plug) {
+			RTE_LOG(ERR, EAL, "Bus function not supported\n");
+			return -ENOTSUP;
+		}
+
+		ret = (bus->plug(dev->devargs) == NULL);
+		goto out;
 	}
 
-	return 0;
+	/*
+	 * If we haven't found a bus device the user meant to "hotplug" a
+	 * virtual device instead.
+	 */
+	ret = rte_vdev_init(name, devargs);
+out:
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
+			name);
+	return ret;
+}
+
+static int cmp_dev_name(const struct rte_device *dev, const void *_name)
+{
+	const char *name = _name;
 
-err:
-	RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", name);
-	return -EINVAL;
+	return strcmp(dev->name, name);
 }
 
 int rte_eal_dev_detach(const char *name)
 {
-	struct rte_pci_addr addr;
+	struct rte_device *dev;
+	struct rte_bus *bus;
+	int ret;
 
 	if (name == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device provided.\n");
 		return -EINVAL;
 	}
 
-	if (eal_parse_pci_DomBDF(name, &addr) == 0) {
-		if (rte_pci_detach(&addr) < 0)
-			goto err;
-	} else {
-		if (rte_vdev_uninit(name))
-			goto err;
+	dev = rte_bus_find_device(NULL, cmp_dev_name, name);
+	if (!dev) {
+		RTE_LOG(ERR, EAL, "Cannot find device (%s)\n", name);
+		return -EINVAL;
+	}
+
+	bus = rte_bus_find_by_device(dev);
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n", name);
+		return -EINVAL;
+	}
+
+	if (!bus->unplug) {
+		RTE_LOG(ERR, EAL, "Bus function not supported\n");
+		return -ENOTSUP;
 	}
-	return 0;
 
-err:
-	RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n", name);
-	return -EINVAL;
+	ret = bus->unplug(dev);
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
+			name);
+	return ret;
 }
-- 
2.1.4

[PATCH v3 09/10] ethdev: use embedded rte_device to detach driver

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map |  1 +
 lib/librte_eal/common/eal_common_dev.c        | 43 ++++++++++++++++-----------
 lib/librte_eal/common/include/rte_dev.h       | 11 +++++++
 lib/librte_ether/rte_ethdev.c                 | 31 +++++++------------
 4 files changed, 47 insertions(+), 39 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 21640d6..150b0f7 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -162,6 +162,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_eal_device_detach;
 	rte_bus_find;
 	rte_bus_find_by_device;
 	rte_bus_find_device;
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 733da91..8ef9b98 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -100,6 +100,30 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
 	return ret;
 }
 
+int rte_eal_device_detach(struct rte_device *dev)
+{
+	struct rte_bus *bus;
+	int ret;
+
+	bus = rte_bus_find_by_device(dev);
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n",
+			dev->name);
+		return -EINVAL;
+	}
+
+	if (!bus->unplug) {
+		RTE_LOG(ERR, EAL, "Bus function not supported\n");
+		return -ENOTSUP;
+	}
+
+	ret = bus->unplug(dev);
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
+			dev->name);
+	return ret;
+}
+
 static int cmp_dev_name(const struct rte_device *dev, const void *_name)
 {
 	const char *name = _name;
@@ -110,8 +134,6 @@ static int cmp_dev_name(const struct rte_device *dev, const void *_name)
 int rte_eal_dev_detach(const char *name)
 {
 	struct rte_device *dev;
-	struct rte_bus *bus;
-	int ret;
 
 	if (name == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device provided.\n");
@@ -124,20 +146,5 @@ int rte_eal_dev_detach(const char *name)
 		return -EINVAL;
 	}
 
-	bus = rte_bus_find_by_device(dev);
-	if (!bus) {
-		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n", name);
-		return -EINVAL;
-	}
-
-	if (!bus->unplug) {
-		RTE_LOG(ERR, EAL, "Bus function not supported\n");
-		return -ENOTSUP;
-	}
-
-	ret = bus->unplug(dev);
-	if (ret)
-		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
-			name);
-	return ret;
+	return rte_eal_device_detach(dev);
 }
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index 724855e..9f35b1f 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -123,6 +123,17 @@ struct rte_mem_resource {
 	void *addr;         /**< Virtual address, NULL when not mapped. */
 };
 
+/* FIXME: Forward declare */
+struct rte_device;
+
+/**
+ * Unplug the device from the device driver.
+ *
+ * @param dev
+ *   A pointer to a rte_device structure.
+ */
+int rte_eal_device_detach(struct rte_device *dev);
+
 /**
  * A structure describing a device driver.
  */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 606252f..bc603c1 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -355,26 +355,14 @@ rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id)
 static int
 rte_eth_dev_is_detachable(uint8_t port_id)
 {
-	uint32_t dev_flags;
+	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
-	switch (rte_eth_devices[port_id].data->kdrv) {
-	case RTE_KDRV_IGB_UIO:
-	case RTE_KDRV_UIO_GENERIC:
-	case RTE_KDRV_NIC_UIO:
-	case RTE_KDRV_NONE:
-	case RTE_KDRV_VFIO:
-		break;
-	default:
-		return -ENOTSUP;
-	}
-	dev_flags = rte_eth_devices[port_id].data->dev_flags;
-	if ((dev_flags & RTE_ETH_DEV_DETACHABLE) &&
-		(!(dev_flags & RTE_ETH_DEV_BONDED_SLAVE)))
+	dev = &rte_eth_devices[port_id];
+	if (dev->data->dev_flags & RTE_ETH_DEV_BONDED_SLAVE)
 		return 0;
-	else
-		return 1;
+	return !!dev->device->devargs->bus->unplug;
 }
 
 /* attach the new device, then store port_id of the device */
@@ -427,6 +415,7 @@ rte_eth_dev_attach(const char *devargs, uint8_t *port_id)
 int
 rte_eth_dev_detach(uint8_t port_id, char *name)
 {
+	struct rte_eth_dev *dev;
 	int ret = -1;
 
 	if (name == NULL) {
@@ -435,15 +424,15 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
 	}
 
 	/* FIXME: move this to eal, once device flags are relocated there */
-	if (rte_eth_dev_is_detachable(port_id))
+	if (!rte_eth_dev_is_detachable(port_id))
 		goto err;
 
-	snprintf(name, sizeof(rte_eth_devices[port_id].data->name),
-		 "%s", rte_eth_devices[port_id].data->name);
-	ret = rte_eal_dev_detach(name);
+	dev = &rte_eth_devices[port_id];
+	snprintf(name, RTE_ETH_NAME_MAX_LEN - 1, "%s", dev->device->name);
+	ret = rte_eal_device_detach(dev->device);
 	if (ret < 0)
 		goto err;
-
+	dev->state = RTE_ETH_DEV_UNUSED;
 	return 0;
 
 err:
-- 
2.1.4

[PATCH v3 10/10] net/ring: fix dev handle in eth_dev

[Gaetan Rivet]

The ring PMD uses special eth_dev allocators, which cannot be updated to
accept an rte_vdev_device.
Circumvent the limitation and store the rte_device handle in the
rte_eth_dev structure.

Fixes: 050fe6e9ff97 ("drivers/net: use ethdev allocation helper for
vdev")
Cc: stable@dpdk.org

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 drivers/net/ring/rte_eth_ring.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ring/rte_eth_ring.c b/drivers/net/ring/rte_eth_ring.c
index d4dce95..2cd32a9 100644
--- a/drivers/net/ring/rte_eth_ring.c
+++ b/drivers/net/ring/rte_eth_ring.c
@@ -515,6 +515,8 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev)
 	RTE_LOG(INFO, PMD, "Initializing pmd_ring for %s\n", name);
 
 	if (params == NULL || params[0] == '\0') {
+		struct rte_eth_dev *eth_dev;
+
 		ret = eth_dev_ring_create(name, rte_socket_id(), DEV_CREATE);
 		if (ret == -1) {
 			RTE_LOG(INFO, PMD,
@@ -522,6 +524,11 @@ rte_pmd_ring_probe(struct rte_vdev_device *dev)
 			ret = eth_dev_ring_create(name, rte_socket_id(),
 						  DEV_ATTACH);
 		}
+		/* find an ethdev entry */
+		eth_dev = rte_eth_dev_allocated(name);
+		if (eth_dev == NULL)
+			return -ENODEV;
+		eth_dev->device = &dev->device;
 	}
 	else {
 		kvlist = rte_kvargs_parse(params, valid_arguments);
-- 
2.1.4

Re: [PATCH v2 01/11] bus: add bus iterator to find a particular bus

[Shreyansh Jain]

Hello Gaëtan,

> -----Original Message-----
> From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> Sent: Wednesday, June 07, 2017 6:58 PM
> To: Shreyansh Jain <shreyansh.jain@nxp.com>
> Cc: dev@dpdk.org; Jan Blunck <jblunck@infradead.org>
> Subject: Re: [PATCH v2 01/11] bus: add bus iterator to find a particular bus
> 
> On Wed, Jun 07, 2017 at 12:36:53PM +0530, Shreyansh Jain wrote:
> > Hello Gaetan,
> >
> > On Wednesday 31 May 2017 06:47 PM, Gaetan Rivet wrote:
> > >From: Jan Blunck <jblunck@infradead.org>
> > >
> > >Signed-off-by: Jan Blunck <jblunck@infradead.org>
> > >Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > >---
> > >  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
> > >  lib/librte_eal/common/eal_common_bus.c          | 13 ++++++++++
> > >  lib/librte_eal/common/include/rte_bus.h         | 32
> +++++++++++++++++++++++++
> > >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
> > >  4 files changed, 47 insertions(+)
> > >
> > >diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > >index 2e48a73..ed09ab2 100644
> > >--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > >+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > >@@ -162,6 +162,7 @@ DPDK_17.02 {
> > >  DPDK_17.05 {
> > >  	global:
> > >+	rte_bus_find;
> > >  	rte_cpu_is_supported;
> > >  	rte_log_dump;
> > >  	rte_log_register;
> > >diff --git a/lib/librte_eal/common/eal_common_bus.c
> b/lib/librte_eal/common/eal_common_bus.c
> > >index 8f9baf8..68f70d0 100644
> > >--- a/lib/librte_eal/common/eal_common_bus.c
> > >+++ b/lib/librte_eal/common/eal_common_bus.c
> > >@@ -145,3 +145,16 @@ rte_bus_dump(FILE *f)
> > >  		}
> > >  	}
> > >  }
> > >+
> > >+struct rte_bus *
> > >+rte_bus_find(rte_bus_match_t match, const void *data)
> > >+{
> > >+	struct rte_bus *bus = NULL;
> > >+
> > >+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
> > >+		if (match(bus, data))
> > >+			break;
> > >+	}
> > >+
> > >+	return bus;
> > >+}
> > >diff --git a/lib/librte_eal/common/include/rte_bus.h
> b/lib/librte_eal/common/include/rte_bus.h
> > >index 7c36969..006feca 100644
> > >--- a/lib/librte_eal/common/include/rte_bus.h
> > >+++ b/lib/librte_eal/common/include/rte_bus.h
> > >@@ -141,6 +141,38 @@ int rte_bus_probe(void);
> > >  void rte_bus_dump(FILE *f);
> > >  /**
> > >+ * Bus match function.
> > >+ *
> > >+ * @param bus
> > >+ *	bus under test.
> > >+ *
> > >+ * @param data
> > >+ *	data matched
> > >+ *
> > >+ * @return
> > >+ *	0 if the bus does not match.
> > >+ *	!0 if the bus matches.
> >
> > One of the common match function implementation could be simply to match
> > a string. strcmp itself returns '0' for a successful match.
> > On the same lines, should this function return value be reversed?
> > -
> > 0 if match
> > !0 if not a match
> > -
> > That way, people would not have to change either the way strcmp works,
> > for example, or the way various APIs expect '0' as success.
> >
> > same for rte_device_match_t as well. (in next patch)
> >
> 
> It was actually a point I hesitated a little before submitting this
> version.
> 
> The logic behind strcmp is that you can express three states: greater
> than, equal, lower than, thus having total order within the string set.
> 
> Here, buses are not ordered (logically). Having a bus lower or greater
> than some arbitrary data does not mean much.

I agree about the match() fn - but, this is not what I meant. 
My point was that ideally for implementation of callbacks, applications
would more often use the strcmp function (matching name of the bus) than
the match() callback.

Further, this semantic is being followed by other DPDK APIs as well - so,
my intent was to make this also inline with those.

> 
> Anyway, this was my reasoning for following Jan's proposal on this, but
> I'm not against changing this API. Maybe having to possibility to
> express total order could be useful eventually. I don't have a strong
> opinion on this so unless someone shouts about it, I will follow your
> remark.

Thank you!

> 
<...snip...>

Re: [PATCH v2 01/11] bus: add bus iterator to find a particular bus

[Gaëtan Rivet]

On Thu, Jun 08, 2017 at 04:34:50AM +0000, Shreyansh Jain wrote:
> Hello Gaëtan,
> 
> > -----Original Message-----
> > From: Gaëtan Rivet [mailto:gaetan.rivet@6wind.com]
> > Sent: Wednesday, June 07, 2017 6:58 PM
> > To: Shreyansh Jain <shreyansh.jain@nxp.com>
> > Cc: dev@dpdk.org; Jan Blunck <jblunck@infradead.org>
> > Subject: Re: [PATCH v2 01/11] bus: add bus iterator to find a particular bus
> > 
> > On Wed, Jun 07, 2017 at 12:36:53PM +0530, Shreyansh Jain wrote:
> > > Hello Gaetan,
> > >
> > > On Wednesday 31 May 2017 06:47 PM, Gaetan Rivet wrote:
> > > >From: Jan Blunck <jblunck@infradead.org>
> > > >
> > > >Signed-off-by: Jan Blunck <jblunck@infradead.org>
> > > >Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > > >---
> > > >  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
> > > >  lib/librte_eal/common/eal_common_bus.c          | 13 ++++++++++
> > > >  lib/librte_eal/common/include/rte_bus.h         | 32
> > +++++++++++++++++++++++++
> > > >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
> > > >  4 files changed, 47 insertions(+)
> > > >
> > > >diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > > >index 2e48a73..ed09ab2 100644
> > > >--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > > >+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > > >@@ -162,6 +162,7 @@ DPDK_17.02 {
> > > >  DPDK_17.05 {
> > > >  	global:
> > > >+	rte_bus_find;
> > > >  	rte_cpu_is_supported;
> > > >  	rte_log_dump;
> > > >  	rte_log_register;
> > > >diff --git a/lib/librte_eal/common/eal_common_bus.c
> > b/lib/librte_eal/common/eal_common_bus.c
> > > >index 8f9baf8..68f70d0 100644
> > > >--- a/lib/librte_eal/common/eal_common_bus.c
> > > >+++ b/lib/librte_eal/common/eal_common_bus.c
> > > >@@ -145,3 +145,16 @@ rte_bus_dump(FILE *f)
> > > >  		}
> > > >  	}
> > > >  }
> > > >+
> > > >+struct rte_bus *
> > > >+rte_bus_find(rte_bus_match_t match, const void *data)
> > > >+{
> > > >+	struct rte_bus *bus = NULL;
> > > >+
> > > >+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
> > > >+		if (match(bus, data))
> > > >+			break;
> > > >+	}
> > > >+
> > > >+	return bus;
> > > >+}
> > > >diff --git a/lib/librte_eal/common/include/rte_bus.h
> > b/lib/librte_eal/common/include/rte_bus.h
> > > >index 7c36969..006feca 100644
> > > >--- a/lib/librte_eal/common/include/rte_bus.h
> > > >+++ b/lib/librte_eal/common/include/rte_bus.h
> > > >@@ -141,6 +141,38 @@ int rte_bus_probe(void);
> > > >  void rte_bus_dump(FILE *f);
> > > >  /**
> > > >+ * Bus match function.
> > > >+ *
> > > >+ * @param bus
> > > >+ *	bus under test.
> > > >+ *
> > > >+ * @param data
> > > >+ *	data matched
> > > >+ *
> > > >+ * @return
> > > >+ *	0 if the bus does not match.
> > > >+ *	!0 if the bus matches.
> > >
> > > One of the common match function implementation could be simply to match
> > > a string. strcmp itself returns '0' for a successful match.
> > > On the same lines, should this function return value be reversed?
> > > -
> > > 0 if match
> > > !0 if not a match
> > > -
> > > That way, people would not have to change either the way strcmp works,
> > > for example, or the way various APIs expect '0' as success.
> > >
> > > same for rte_device_match_t as well. (in next patch)
> > >
> > 
> > It was actually a point I hesitated a little before submitting this
> > version.
> > 
> > The logic behind strcmp is that you can express three states: greater
> > than, equal, lower than, thus having total order within the string set.
> > 
> > Here, buses are not ordered (logically). Having a bus lower or greater
> > than some arbitrary data does not mean much.
> 
> I agree about the match() fn - but, this is not what I meant. 
> My point was that ideally for implementation of callbacks, applications
> would more often use the strcmp function (matching name of the bus) than
> the match() callback.
> 
> Further, this semantic is being followed by other DPDK APIs as well - so,
> my intent was to make this also inline with those.
> 

Ah, I see your point now. It makes sense.
The API is now the same.

> > 
> > Anyway, this was my reasoning for following Jan's proposal on this, but
> > I'm not against changing this API. Maybe having to possibility to
> > express total order could be useful eventually. I don't have a strong
> > opinion on this so unless someone shouts about it, I will follow your
> > remark.
> 
> Thank you!
> 
> > 
> <...snip...>

-- 
Gaëtan Rivet
6WIND

Re: [PATCH v3 01/10] bus: add bus iterator to find a particular bus

[Jan Blunck]

On Thu, Jun 8, 2017 at 1:53 AM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> From: Jan Blunck <jblunck@infradead.org>
>
> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> ---
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
>  lib/librte_eal/common/eal_common_bus.c          | 11 ++++++++
>  lib/librte_eal/common/include/rte_bus.h         | 34 +++++++++++++++++++++++++
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
>  4 files changed, 47 insertions(+)
>
> diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> index 2e48a73..ed09ab2 100644
> --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> @@ -162,6 +162,7 @@ DPDK_17.02 {
>  DPDK_17.05 {
>         global:
>
> +       rte_bus_find;
>         rte_cpu_is_supported;
>         rte_log_dump;
>         rte_log_register;
> diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> index 8f9baf8..a54aeb4 100644
> --- a/lib/librte_eal/common/eal_common_bus.c
> +++ b/lib/librte_eal/common/eal_common_bus.c
> @@ -145,3 +145,14 @@ rte_bus_dump(FILE *f)
>                 }
>         }
>  }
> +
> +struct rte_bus *
> +rte_bus_find(rte_bus_cmp_t cmp, const void *data)
> +{
> +       struct rte_bus *bus = NULL;
> +
> +       TAILQ_FOREACH(bus, &rte_bus_list, next)
> +               if (cmp(bus, data) == 0)
> +                       break;
> +       return bus;
> +}
> diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> index 7c36969..16bcfd9 100644
> --- a/lib/librte_eal/common/include/rte_bus.h
> +++ b/lib/librte_eal/common/include/rte_bus.h
> @@ -141,6 +141,40 @@ int rte_bus_probe(void);
>  void rte_bus_dump(FILE *f);
>
>  /**
> + * Bus comparison function.
> + *
> + * @param bus
> + *     Bus under test.
> + *
> + * @param data
> + *     Data to compare against.
> + *
> + * @return
> + *     0 if the bus matches the data.
> + *     !0 if the bus does not match.
> + *     <0 if ordering is possible and the bus is lower than the data.
> + *     >0 if ordering is possible and the bus is greater than the data.
> + */
> +typedef int (*rte_bus_cmp_t)(const struct rte_bus *bus, const void *data);

Gaetan,

Thanks for doing the adjustments. I believe we should also pass the
starting pointer down to the bus implementation to enable continuation
of a search after the given starting pointer. Does this make sense?

> +
> +/**
> + * Bus iterator to find a particular bus.
> + *
> + * If the callback returns non-zero this function will stop iterating over
> + * any more buses.
> + *
> + * @param cmp
> + *     Comparison function.
> + *
> + * @param data
> + *      Data to pass to cmp callback
> + *
> + * @return
> + *      A pointer to a rte_bus structure or NULL in case no bus matches
> + */
> +struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp, const void *data);
> +
> +/**
>   * Helper for Bus registration.
>   * The constructor has higher priority than PMD constructors.
>   */
> diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> index 670bab3..6efa517 100644
> --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> @@ -166,6 +166,7 @@ DPDK_17.02 {
>  DPDK_17.05 {
>         global:
>
> +       rte_bus_find;
>         rte_cpu_is_supported;
>         rte_intr_free_epoll_fd;
>         rte_log_dump;
> --
> 2.1.4
>

Re: [PATCH v3 01/10] bus: add bus iterator to find a particular bus

[Gaëtan Rivet]

On Sat, Jun 10, 2017 at 10:58:47AM +0200, Jan Blunck wrote:
> On Thu, Jun 8, 2017 at 1:53 AM, Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> > From: Jan Blunck <jblunck@infradead.org>
> >
> > Signed-off-by: Jan Blunck <jblunck@infradead.org>
> > Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
> > ---
> >  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
> >  lib/librte_eal/common/eal_common_bus.c          | 11 ++++++++
> >  lib/librte_eal/common/include/rte_bus.h         | 34 +++++++++++++++++++++++++
> >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
> >  4 files changed, 47 insertions(+)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > index 2e48a73..ed09ab2 100644
> > --- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
> > @@ -162,6 +162,7 @@ DPDK_17.02 {
> >  DPDK_17.05 {
> >         global:
> >
> > +       rte_bus_find;
> >         rte_cpu_is_supported;
> >         rte_log_dump;
> >         rte_log_register;
> > diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
> > index 8f9baf8..a54aeb4 100644
> > --- a/lib/librte_eal/common/eal_common_bus.c
> > +++ b/lib/librte_eal/common/eal_common_bus.c
> > @@ -145,3 +145,14 @@ rte_bus_dump(FILE *f)
> >                 }
> >         }
> >  }
> > +
> > +struct rte_bus *
> > +rte_bus_find(rte_bus_cmp_t cmp, const void *data)
> > +{
> > +       struct rte_bus *bus = NULL;
> > +
> > +       TAILQ_FOREACH(bus, &rte_bus_list, next)
> > +               if (cmp(bus, data) == 0)
> > +                       break;
> > +       return bus;
> > +}
> > diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
> > index 7c36969..16bcfd9 100644
> > --- a/lib/librte_eal/common/include/rte_bus.h
> > +++ b/lib/librte_eal/common/include/rte_bus.h
> > @@ -141,6 +141,40 @@ int rte_bus_probe(void);
> >  void rte_bus_dump(FILE *f);
> >
> >  /**
> > + * Bus comparison function.
> > + *
> > + * @param bus
> > + *     Bus under test.
> > + *
> > + * @param data
> > + *     Data to compare against.
> > + *
> > + * @return
> > + *     0 if the bus matches the data.
> > + *     !0 if the bus does not match.
> > + *     <0 if ordering is possible and the bus is lower than the data.
> > + *     >0 if ordering is possible and the bus is greater than the data.
> > + */
> > +typedef int (*rte_bus_cmp_t)(const struct rte_bus *bus, const void *data);
> 
> Gaetan,
> 
> Thanks for doing the adjustments. I believe we should also pass the
> starting pointer down to the bus implementation to enable continuation
> of a search after the given starting pointer. Does this make sense?
> 

Hi Jan,

Yes it makes sense, I see no reason to restrict the API, it will probably
be useful and is a pretty common functionality for iterators.

It seems I missed fixing the description to rte_bus_find anyway, I will
rework this part.

> > +
> > +/**
> > + * Bus iterator to find a particular bus.
> > + *
> > + * If the callback returns non-zero this function will stop iterating over
> > + * any more buses.

--> returns zero this function will stop **

> > + *
> > + * @param cmp
> > + *     Comparison function.
> > + *
> > + * @param data
> > + *      Data to pass to cmp callback
> > + *
> > + * @return
> > + *      A pointer to a rte_bus structure or NULL in case no bus matches
> > + */
> > +struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp, const void *data);
> > +
> > +/**
> >   * Helper for Bus registration.
> >   * The constructor has higher priority than PMD constructors.
> >   */
> > diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > index 670bab3..6efa517 100644
> > --- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > +++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
> > @@ -166,6 +166,7 @@ DPDK_17.02 {
> >  DPDK_17.05 {
> >         global:
> >
> > +       rte_bus_find;
> >         rte_cpu_is_supported;
> >         rte_intr_free_epoll_fd;
> >         rte_log_dump;
> > --
> > 2.1.4
> >

-- 
Gaëtan Rivet
6WIND

[PATCH v4 0/9] bus: attach / detach API

[Gaetan Rivet]

Following the work from Jan:

This patchset introduces the attach / detach API to rte_bus.
The rte_device structure is used as the generic device representation.

This API is implemented for the virtual bus.
The functions rte_eal_dev_attach and rte_eal_dev_detach are updated to
use this new interface.

-- v2

0. API rework
-------------

I would like to propose an evolution on the API developed by Jan.

The attach / detach rte_bus API is necessary to support the attach/detach
rte_dev API. Those are two different levels for one similar functionality.

Attach / detach does not allow true hotplugging, because the attach
function expects the devices operated upon to already exist within the
buses / sub-layers. This means that this API expects devices meta-datas
(bus-internal device representation and associated device information
read from the system) to be present upon attach. This part of the work
is done during scanning.

While it is best to avoid changing the public rte_dev API as it already
exists, nothing prevents this new rte_bus API from superseeding it.
It has been said during the previous release cycle that device hotplug
was a feature that interested users. True hotplug is not allowed by the
current attach / detach API. Worse, this API hinders the effort to bring
this new functionality by squatting its semantic field.

Thus, I propose to rename rte_bus attach / detach; plug / unplug. As it
is a superset of the attach / detach functionality, it can be used to
implement rte_dev attach / detach. Now is the right time to pivot to
this new feature.

This should help maintainers understanding the aim of this API and the
differences with the APIs higher-up, clarify the field and allow a new
functionality to be proposed.

The vdev bus is inherently supporting the new API, however it has been
made explicit. My implementation in the PCI bus in further patchset also
follows the rte_bus hotplug API instead of only attach / detach.

One remaining problem with the vdev bus is the rte_dev attach
implementation, which needs the rte_devargs rework to be properly fixed.

1. Additional evolutions in the patchset
----------------------------------------

The RTE_VERIFY on the find_device is too stringent I think and forces
all buses to implement a public device iterator. While it could be
argued that it would push for quicker support for the functionality, I
think it's possible that some buses are not interested at all in it and
should simply be ignored.

The bus devices iterator has been fixed.

The internal rte_device handle was not properly setup within the
net_ring PMD.

-- v3

The new API is now

typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);
typedef int (*rte_bus_unplug_t)(struct rte_device *dev);

So, plugging a device takes an rte_devargs as input and returns an rte_device.
While implementing related subsystems, I found that I usually needed
this rte_device handle upon a successful device plugging. This seems the
sensible and useful thing to do.
As such, on error NULL is returned and rte_errno is set by the bus.

Unplugging a device however now returns to the first version, which used
an rte_device. The explicit contract here is that if one has an
rte_device that has been obtained by calling bus->plug(), then this
handle can be used for bus->unplug().

Additionally, bus and device comparators now returns 0 on match,
following strcmp-like behavior.

-- v4

* rte_bus_find now takes a *start* parameter, that can be null.
  The bus search starts from this element if set.

* A few doc fixes.

* The rte_device field was fixed within the rte_ring PMD in a previous patch.
  This fix has been integrated by other means, it is not necessary anymore.

Gaetan Rivet (1):
  vdev: implement hotplug functionality

Jan Blunck (8):
  bus: add bus iterator to find a particular bus
  bus: add device iterator
  bus: add helper to find bus for a particular device
  bus: add bus helper iterator to find a particular device
  bus: introduce hotplug functionality
  vdev: implement find_device bus operation
  eal: make virtual driver probe and remove take rte_vdev_device
  ethdev: use embedded rte_device to detach driver

 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   4 +
 lib/librte_eal/common/eal_common_bus.c          |  70 ++++++++++++++++
 lib/librte_eal/common/eal_common_dev.c          | 100 +++++++++++++++++-----
 lib/librte_eal/common/eal_common_vdev.c         |  49 +++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 107 ++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_dev.h         |  28 +++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   3 +
 lib/librte_ether/rte_ethdev.c                   |  31 +++----
 8 files changed, 349 insertions(+), 43 deletions(-)

-- 
2.1.4

[PATCH v4 1/9] bus: add bus iterator to find a particular bus

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 20 ++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 41 +++++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 63 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 2e48a73..ed09ab2 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -162,6 +162,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_bus_find;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 8f9baf8..4619eb2 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -145,3 +145,23 @@ rte_bus_dump(FILE *f)
 		}
 	}
 }
+
+struct rte_bus *
+rte_bus_find(rte_bus_cmp_t cmp,
+	     const void *data,
+	     const struct rte_bus *start)
+{
+	struct rte_bus *bus = NULL;
+	int started = start == NULL;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		if (!started) {
+			if (bus == start)
+				started = 1;
+			continue;
+		}
+		if (cmp(bus, data) == 0)
+			break;
+	}
+	return bus;
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 5f47b82..3e26d4b 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -141,6 +141,47 @@ int rte_bus_probe(void);
 void rte_bus_dump(FILE *f);
 
 /**
+ * Bus comparison function.
+ *
+ * @param bus
+ *	Bus under test.
+ *
+ * @param data
+ *	Data to compare against.
+ *
+ * @return
+ *	0 if the bus matches the data.
+ *	!0 if the bus does not match.
+ *	<0 if ordering is possible and the bus is lower than the data.
+ *	>0 if ordering is possible and the bus is greater than the data.
+ */
+typedef int (*rte_bus_cmp_t)(const struct rte_bus *bus, const void *data);
+
+/**
+ * Bus iterator to find a particular bus.
+ *
+ * If the callback returns zero this function will stop iterating over
+ * any more buses.
+ * If the start parameter is non-NULL, the comparison will only be determined
+ * past this element.
+ *
+ * @param cmp
+ *	Comparison function.
+ *
+ * @param data
+ *	 Data to pass to cmp callback
+ *
+ * @param start
+ *	Starting point for the iteration.
+ *
+ * @return
+ *	 A pointer to a rte_bus structure or NULL in case no bus matches
+ */
+struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp,
+			     const void *data,
+			     const struct rte_bus *start);
+
+/**
  * Helper for Bus registration.
  * The constructor has higher priority than PMD constructors.
  */
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 670bab3..6efa517 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -166,6 +166,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_bus_find;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH v4 2/9] bus: add device iterator

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/include/rte_bus.h |  7 +++++++
 lib/librte_eal/common/include/rte_dev.h | 17 +++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index 3e26d4b..fe9573f 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -82,6 +82,12 @@ typedef int (*rte_bus_scan_t)(void);
 typedef int (*rte_bus_probe_t)(void);
 
 /**
+ * Device iterator to find a particular device on a bus.
+ */
+typedef struct rte_device * (*rte_bus_find_device_t)(rte_dev_cmp_t match,
+						     const void *data);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -89,6 +95,7 @@ struct rte_bus {
 	const char *name;            /**< Name of the bus */
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
+	rte_bus_find_device_t find_device; /**< Find device on bus */
 };
 
 /**
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index de20c06..724855e 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -191,6 +191,23 @@ int rte_eal_dev_attach(const char *name, const char *devargs);
  */
 int rte_eal_dev_detach(const char *name);
 
+/**
+ * Device comparison function.
+ *
+ * @param dev
+ *   Device handle.
+ *
+ * @param data
+ *   Data to compare against.
+ *
+ * @return
+ *   0 if the device matches the data.
+ *   !0 if the device does not match.
+ *   <0 if ordering is possible and the device is lower than the data.
+ *   >0 if ordering is possible and the device is greater than the data.
+ */
+typedef int (*rte_dev_cmp_t)(const struct rte_device *dev, const void *data);
+
 #define RTE_PMD_EXPORT_NAME_ARRAY(n, idx) n##idx[]
 
 #define RTE_PMD_EXPORT_NAME(name, idx) \
-- 
2.1.4

[PATCH v4 3/9] bus: add helper to find bus for a particular device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 24 ++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         |  5 +++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 31 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index ed09ab2..f1a0765 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -163,6 +163,7 @@ DPDK_17.05 {
 	global:
 
 	rte_bus_find;
+	rte_bus_find_by_device;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 4619eb2..a38b576 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -165,3 +165,27 @@ rte_bus_find(rte_bus_cmp_t cmp,
 	}
 	return bus;
 }
+
+static int
+cmp_rte_device(const struct rte_device *dev, const void *_dev2)
+{
+	const struct rte_device *dev2 = _dev2;
+
+	return !(dev == dev2);
+}
+
+static int
+bus_find_device(const struct rte_bus *bus, const void *_dev)
+{
+	struct rte_device *dev;
+
+	if (!bus->find_device)
+		return -1;
+	dev = bus->find_device(cmp_rte_device, _dev);
+	return !dev;
+}
+
+struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
+{
+	return rte_bus_find(bus_find_device, (const void *)dev, NULL);
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index fe9573f..fa12f55 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -189,6 +189,11 @@ struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp,
 			     const struct rte_bus *start);
 
 /**
+ * Find the registered bus for a particular device.
+ */
+struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
+
+/**
  * Helper for Bus registration.
  * The constructor has higher priority than PMD constructors.
  */
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 6efa517..6f77222 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -167,6 +167,7 @@ DPDK_17.05 {
 	global:
 
 	rte_bus_find;
+	rte_bus_find_by_device;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH v4 4/9] bus: add bus helper iterator to find a particular device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  1 +
 lib/librte_eal/common/eal_common_bus.c          | 24 ++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 23 +++++++++++++++++++++++
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  1 +
 4 files changed, 49 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index f1a0765..21640d6 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -164,6 +164,7 @@ DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_log_dump;
 	rte_log_register;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index a38b576..9d84727 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -189,3 +189,27 @@ struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
 {
 	return rte_bus_find(bus_find_device, (const void *)dev, NULL);
 }
+
+struct rte_device *
+rte_bus_find_device(const struct rte_device *start,
+		    rte_dev_cmp_t cmp, const void *data)
+{
+	struct rte_bus *bus;
+	struct rte_device *dev = NULL;
+	int started = start == NULL;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		if (!bus->find_device)
+			continue;
+		if (!started) {
+			dev = bus->find_device(cmp_rte_device, start);
+			if (dev)
+				started = 1;
+			continue;
+		}
+		dev = bus->find_device(cmp, data);
+		if (dev)
+			break;
+	}
+	return dev;
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index fa12f55..e078c00 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -189,6 +189,29 @@ struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp,
 			     const struct rte_bus *start);
 
 /**
+ * Bus iterator to find a particular device.
+ *
+ * If the callback returns non-zero this function will stop iterating over any
+ * more buses and devices. To continue a search the device of a previous search
+ * is passed via the start parameters.
+ *
+ * @param start
+ *	 Start device of the iteration.
+ *
+ * @param cmp
+ *	 Callback function to check device.
+ *
+ * @param data
+ *	 Data to pass to match callback.
+ *
+ * @return
+ *	 A pointer to a rte_bus structure or NULL in case no bus matches.
+ */
+struct rte_device *
+rte_bus_find_device(const struct rte_device *start,
+		    rte_dev_cmp_t cmp, const void *data);
+
+/**
  * Find the registered bus for a particular device.
  */
 struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev);
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 6f77222..e0a056d 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -168,6 +168,7 @@ DPDK_17.05 {
 
 	rte_bus_find;
 	rte_bus_find_by_device;
+	rte_bus_find_device;
 	rte_cpu_is_supported;
 	rte_intr_free_epoll_fd;
 	rte_log_dump;
-- 
2.1.4

[PATCH v4 5/9] bus: introduce hotplug functionality

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_bus.c  |  2 ++
 lib/librte_eal/common/include/rte_bus.h | 31 +++++++++++++++++++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 9d84727..b6bf57e 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -50,6 +50,8 @@ rte_bus_register(struct rte_bus *bus)
 	/* A bus should mandatorily have the scan implemented */
 	RTE_VERIFY(bus->scan);
 	RTE_VERIFY(bus->probe);
+	/* Buses supporting hotplug also require unplug. */
+	RTE_VERIFY(!bus->plug || bus->unplug);
 
 	TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
 	RTE_LOG(DEBUG, EAL, "Registered [%s] bus.\n", bus->name);
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index e078c00..fcc2442 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -88,6 +88,35 @@ typedef struct rte_device * (*rte_bus_find_device_t)(rte_dev_cmp_t match,
 						     const void *data);
 
 /**
+ * Implementation specific probe function which is responsible for linking
+ * devices on that bus with applicable drivers.
+ * The plugged device might already have been used previously by the bus,
+ * in which case some buses might prefer to detect and re-use the relevant
+ * information pertaining to this device.
+ *
+ * @param da
+ *	Device declaration.
+ *
+ * @return
+ *	The pointer to a valid rte_device usable by the bus on success.
+ *	NULL on error. rte_errno is then set.
+ */
+typedef struct rte_device * (*rte_bus_plug_t)(struct rte_devargs *da);
+
+/**
+ * Implementation specific remove function which is responsible for unlinking
+ * devices on that bus from assigned driver.
+ *
+ * @param dev
+ *	Device pointer that was returned by a previous device plug call.
+ *
+ * @return
+ *	0 on success.
+ *	!0 on error. rte_errno is then set.
+ */
+typedef int (*rte_bus_unplug_t)(struct rte_device *dev);
+
+/**
  * A structure describing a generic bus.
  */
 struct rte_bus {
@@ -96,6 +125,8 @@ struct rte_bus {
 	rte_bus_scan_t scan;         /**< Scan for devices attached to bus */
 	rte_bus_probe_t probe;       /**< Probe devices on bus */
 	rte_bus_find_device_t find_device; /**< Find device on bus */
+	rte_bus_plug_t plug;         /**< Probe single device for drivers */
+	rte_bus_unplug_t unplug;     /**< Remove single device from driver */
 };
 
 /**
-- 
2.1.4

[PATCH v4 6/9] vdev: implement find_device bus operation

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_vdev.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index 0037a64..52528ef 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -338,9 +338,22 @@ vdev_probe(void)
 	return 0;
 }
 
+static struct rte_device *
+vdev_find_device(rte_dev_cmp_t cmp, const void *data)
+{
+	struct rte_vdev_device *dev;
+
+	TAILQ_FOREACH(dev, &vdev_device_list, next) {
+		if (cmp(&dev->device, data) == 0)
+			return &dev->device;
+	}
+	return NULL;
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
+	.find_device = vdev_find_device,
 };
 
 RTE_INIT(rte_vdev_bus_register);
-- 
2.1.4

[PATCH v4 7/9] vdev: implement hotplug functionality

[Gaetan Rivet]

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

diff --git a/lib/librte_eal/common/eal_common_vdev.c b/lib/librte_eal/common/eal_common_vdev.c
index 52528ef..22e4640 100644
--- a/lib/librte_eal/common/eal_common_vdev.c
+++ b/lib/librte_eal/common/eal_common_vdev.c
@@ -38,11 +38,13 @@
 #include <sys/queue.h>
 
 #include <rte_eal.h>
+#include <rte_dev.h>
 #include <rte_bus.h>
 #include <rte_vdev.h>
 #include <rte_common.h>
 #include <rte_devargs.h>
 #include <rte_memory.h>
+#include <rte_errno.h>
 
 /** Double linked list of virtual device drivers. */
 TAILQ_HEAD(vdev_device_list, rte_vdev_device);
@@ -350,10 +352,44 @@ vdev_find_device(rte_dev_cmp_t cmp, const void *data)
 	return NULL;
 }
 
+static struct rte_device *
+vdev_plug(struct rte_devargs *da)
+{
+	struct rte_vdev_device *dev;
+	int ret;
+
+	ret = rte_vdev_init(da->virt.drv_name, da->args);
+	if (ret) {
+		rte_errno = -ret;
+		return NULL;
+	}
+	dev = find_vdev(da->virt.drv_name);
+	return &dev->device;
+}
+
+static int
+vdev_unplug(struct rte_device *dev)
+{
+	struct rte_devargs *da;
+	int ret;
+
+	da = dev->devargs;
+	if (da == NULL) {
+		rte_errno = EINVAL;
+		return -1;
+	}
+	ret = rte_vdev_uninit(da->virt.drv_name);
+	if (ret)
+		rte_errno = -ret;
+	return ret;
+}
+
 static struct rte_bus rte_vdev_bus = {
 	.scan = vdev_scan,
 	.probe = vdev_probe,
 	.find_device = vdev_find_device,
+	.plug = vdev_plug,
+	.unplug = vdev_unplug,
 };
 
 RTE_INIT(rte_vdev_bus_register);
-- 
2.1.4

[PATCH v4 8/9] eal: make virtual driver probe and remove take rte_vdev_device

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

This is a preparation to embed the generic rte_device into the rte_eth_dev
also for virtual devices.

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/common/eal_common_dev.c | 93 ++++++++++++++++++++++++++--------
 1 file changed, 71 insertions(+), 22 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index a400ddd..733da91 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -37,6 +37,7 @@
 #include <inttypes.h>
 #include <sys/queue.h>
 
+#include <rte_bus.h>
 #include <rte_dev.h>
 #include <rte_devargs.h>
 #include <rte_debug.h>
@@ -45,50 +46,98 @@
 
 #include "eal_private.h"
 
+static int cmp_detached_dev_name(const struct rte_device *dev,
+	const void *_name)
+{
+	const char *name = _name;
+
+	/* skip attached devices */
+	if (dev->driver)
+		return 0;
+
+	return strcmp(dev->name, name);
+}
+
 int rte_eal_dev_attach(const char *name, const char *devargs)
 {
-	struct rte_pci_addr addr;
+	struct rte_device *dev;
+	int ret;
 
 	if (name == NULL || devargs == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device or arguments provided\n");
 		return -EINVAL;
 	}
 
-	if (eal_parse_pci_DomBDF(name, &addr) == 0) {
-		if (rte_pci_probe_one(&addr) < 0)
-			goto err;
+	dev = rte_bus_find_device(NULL, cmp_detached_dev_name, name);
+	if (dev) {
+		struct rte_bus *bus;
+
+		bus = rte_bus_find_by_device(dev);
+		if (!bus) {
+			RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n",
+				name);
+			return -EINVAL;
+		}
 
-	} else {
-		if (rte_vdev_init(name, devargs))
-			goto err;
+		if (!bus->plug) {
+			RTE_LOG(ERR, EAL, "Bus function not supported\n");
+			return -ENOTSUP;
+		}
+
+		ret = (bus->plug(dev->devargs) == NULL);
+		goto out;
 	}
 
-	return 0;
+	/*
+	 * If we haven't found a bus device the user meant to "hotplug" a
+	 * virtual device instead.
+	 */
+	ret = rte_vdev_init(name, devargs);
+out:
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n",
+			name);
+	return ret;
+}
+
+static int cmp_dev_name(const struct rte_device *dev, const void *_name)
+{
+	const char *name = _name;
 
-err:
-	RTE_LOG(ERR, EAL, "Driver cannot attach the device (%s)\n", name);
-	return -EINVAL;
+	return strcmp(dev->name, name);
 }
 
 int rte_eal_dev_detach(const char *name)
 {
-	struct rte_pci_addr addr;
+	struct rte_device *dev;
+	struct rte_bus *bus;
+	int ret;
 
 	if (name == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device provided.\n");
 		return -EINVAL;
 	}
 
-	if (eal_parse_pci_DomBDF(name, &addr) == 0) {
-		if (rte_pci_detach(&addr) < 0)
-			goto err;
-	} else {
-		if (rte_vdev_uninit(name))
-			goto err;
+	dev = rte_bus_find_device(NULL, cmp_dev_name, name);
+	if (!dev) {
+		RTE_LOG(ERR, EAL, "Cannot find device (%s)\n", name);
+		return -EINVAL;
+	}
+
+	bus = rte_bus_find_by_device(dev);
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n", name);
+		return -EINVAL;
+	}
+
+	if (!bus->unplug) {
+		RTE_LOG(ERR, EAL, "Bus function not supported\n");
+		return -ENOTSUP;
 	}
-	return 0;
 
-err:
-	RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n", name);
-	return -EINVAL;
+	ret = bus->unplug(dev);
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
+			name);
+	return ret;
 }
-- 
2.1.4

[PATCH v4 9/9] ethdev: use embedded rte_device to detach driver

[Gaetan Rivet]

From: Jan Blunck <jblunck@infradead.org>

Signed-off-by: Jan Blunck <jblunck@infradead.org>
Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map |  1 +
 lib/librte_eal/common/eal_common_dev.c        | 43 ++++++++++++++++-----------
 lib/librte_eal/common/include/rte_dev.h       | 11 +++++++
 lib/librte_ether/rte_ethdev.c                 | 31 +++++++------------
 4 files changed, 47 insertions(+), 39 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 21640d6..150b0f7 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -162,6 +162,7 @@ DPDK_17.02 {
 DPDK_17.05 {
 	global:
 
+	rte_eal_device_detach;
 	rte_bus_find;
 	rte_bus_find_by_device;
 	rte_bus_find_device;
diff --git a/lib/librte_eal/common/eal_common_dev.c b/lib/librte_eal/common/eal_common_dev.c
index 733da91..8ef9b98 100644
--- a/lib/librte_eal/common/eal_common_dev.c
+++ b/lib/librte_eal/common/eal_common_dev.c
@@ -100,6 +100,30 @@ int rte_eal_dev_attach(const char *name, const char *devargs)
 	return ret;
 }
 
+int rte_eal_device_detach(struct rte_device *dev)
+{
+	struct rte_bus *bus;
+	int ret;
+
+	bus = rte_bus_find_by_device(dev);
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n",
+			dev->name);
+		return -EINVAL;
+	}
+
+	if (!bus->unplug) {
+		RTE_LOG(ERR, EAL, "Bus function not supported\n");
+		return -ENOTSUP;
+	}
+
+	ret = bus->unplug(dev);
+	if (ret)
+		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
+			dev->name);
+	return ret;
+}
+
 static int cmp_dev_name(const struct rte_device *dev, const void *_name)
 {
 	const char *name = _name;
@@ -110,8 +134,6 @@ static int cmp_dev_name(const struct rte_device *dev, const void *_name)
 int rte_eal_dev_detach(const char *name)
 {
 	struct rte_device *dev;
-	struct rte_bus *bus;
-	int ret;
 
 	if (name == NULL) {
 		RTE_LOG(ERR, EAL, "Invalid device provided.\n");
@@ -124,20 +146,5 @@ int rte_eal_dev_detach(const char *name)
 		return -EINVAL;
 	}
 
-	bus = rte_bus_find_by_device(dev);
-	if (!bus) {
-		RTE_LOG(ERR, EAL, "Cannot find bus for device (%s)\n", name);
-		return -EINVAL;
-	}
-
-	if (!bus->unplug) {
-		RTE_LOG(ERR, EAL, "Bus function not supported\n");
-		return -ENOTSUP;
-	}
-
-	ret = bus->unplug(dev);
-	if (ret)
-		RTE_LOG(ERR, EAL, "Driver cannot detach the device (%s)\n",
-			name);
-	return ret;
+	return rte_eal_device_detach(dev);
 }
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index 724855e..9f35b1f 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -123,6 +123,17 @@ struct rte_mem_resource {
 	void *addr;         /**< Virtual address, NULL when not mapped. */
 };
 
+/* FIXME: Forward declare */
+struct rte_device;
+
+/**
+ * Unplug the device from the device driver.
+ *
+ * @param dev
+ *   A pointer to a rte_device structure.
+ */
+int rte_eal_device_detach(struct rte_device *dev);
+
 /**
  * A structure describing a device driver.
  */
diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
index 03a42a6..4a1d0b9 100644
--- a/lib/librte_ether/rte_ethdev.c
+++ b/lib/librte_ether/rte_ethdev.c
@@ -354,26 +354,14 @@ rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id)
 static int
 rte_eth_dev_is_detachable(uint8_t port_id)
 {
-	uint32_t dev_flags;
+	struct rte_eth_dev *dev;
 
 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
 
-	switch (rte_eth_devices[port_id].data->kdrv) {
-	case RTE_KDRV_IGB_UIO:
-	case RTE_KDRV_UIO_GENERIC:
-	case RTE_KDRV_NIC_UIO:
-	case RTE_KDRV_NONE:
-	case RTE_KDRV_VFIO:
-		break;
-	default:
-		return -ENOTSUP;
-	}
-	dev_flags = rte_eth_devices[port_id].data->dev_flags;
-	if ((dev_flags & RTE_ETH_DEV_DETACHABLE) &&
-		(!(dev_flags & RTE_ETH_DEV_BONDED_SLAVE)))
+	dev = &rte_eth_devices[port_id];
+	if (dev->data->dev_flags & RTE_ETH_DEV_BONDED_SLAVE)
 		return 0;
-	else
-		return 1;
+	return !!dev->device->devargs->bus->unplug;
 }
 
 /* attach the new device, then store port_id of the device */
@@ -426,6 +414,7 @@ rte_eth_dev_attach(const char *devargs, uint8_t *port_id)
 int
 rte_eth_dev_detach(uint8_t port_id, char *name)
 {
+	struct rte_eth_dev *dev;
 	int ret = -1;
 
 	if (name == NULL) {
@@ -434,15 +423,15 @@ rte_eth_dev_detach(uint8_t port_id, char *name)
 	}
 
 	/* FIXME: move this to eal, once device flags are relocated there */
-	if (rte_eth_dev_is_detachable(port_id))
+	if (!rte_eth_dev_is_detachable(port_id))
 		goto err;
 
-	snprintf(name, sizeof(rte_eth_devices[port_id].data->name),
-		 "%s", rte_eth_devices[port_id].data->name);
-	ret = rte_eal_dev_detach(name);
+	dev = &rte_eth_devices[port_id];
+	snprintf(name, RTE_ETH_NAME_MAX_LEN - 1, "%s", dev->device->name);
+	ret = rte_eal_device_detach(dev->device);
 	if (ret < 0)
 		goto err;
-
+	dev->state = RTE_ETH_DEV_UNUSED;
 	return 0;
 
 err:
-- 
2.1.4

Re: [PATCH v4 2/9] bus: add device iterator

[Thomas Monjalon]

21/06/2017 01:29, Gaetan Rivet:
> +/**
> + * Device comparison function.
> + *
> + * @param dev
> + *   Device handle.
> + *
> + * @param data
> + *   Data to compare against.
> + *
> + * @return
> + *   0 if the device matches the data.
> + *   !0 if the device does not match.
> + *   <0 if ordering is possible and the device is lower than the data.
> + *   >0 if ordering is possible and the device is greater than the data.
> + */
> +typedef int (*rte_dev_cmp_t)(const struct rte_device *dev, const void *data);

data is really abstract.
Maybe a comment is missing to explain that data is better specified
in bus implementations?

Why not implement it for PCI?

Re: [PATCH v4 3/9] bus: add helper to find bus for a particular device

[Thomas Monjalon]

21/06/2017 01:29, Gaetan Rivet:
> +static int
> +cmp_rte_device(const struct rte_device *dev, const void *_dev2)

Better to rename dev into dev1.

> +{
> +	const struct rte_device *dev2 = _dev2;
> +
> +	return !(dev == dev2);

simpler: return dev1 != dev2

[...]
> +static int
> +bus_find_device(const struct rte_bus *bus, const void *_dev)
> +{
> +	struct rte_device *dev;
> +
> +	if (!bus->find_device)

It is preferred to check pointers against NULL.

> +		return -1;
> +	dev = bus->find_device(cmp_rte_device, _dev);
> +	return !dev;

Here also: return dev == NULL:

> +}
> +
> +struct rte_bus *rte_bus_find_by_device(const struct rte_device *dev)
> +{
> +	return rte_bus_find(bus_find_device, (const void *)dev, NULL);
> +}

Nice

Re: [PATCH v4 1/9] bus: add bus iterator to find a particular bus

[Thomas Monjalon]

21/06/2017 01:29, Gaetan Rivet:
> +/**
> + * Bus iterator to find a particular bus.
> + *
> + * If the callback returns zero this function will stop iterating over
> + * any more buses.
> + * If the start parameter is non-NULL, the comparison will only be determined
> + * past this element.
> + *
> + * @param cmp
> + *     Comparison function.
> + *
> + * @param data
> + *      Data to pass to cmp callback
> + *
> + * @param start
> + *     Starting point for the iteration.
> + *
> + * @return
> + *      A pointer to a rte_bus structure or NULL in case no bus matches
> + */
> +struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp,
> +                            const void *data,
> +                            const struct rte_bus *start);
> 

What will be the typical usage? find by name?
Does it make sense to implement a helper for find_by_name?

Or is it used only for rte_bus_find_by_device()?

Re: [PATCH v4 2/9] bus: add device iterator

[Gaëtan Rivet]

On Wed, Jun 21, 2017 at 01:55:39PM +0200, Thomas Monjalon wrote:
> 21/06/2017 01:29, Gaetan Rivet:
> > +/**
> > + * Device comparison function.
> > + *
> > + * @param dev
> > + *   Device handle.
> > + *
> > + * @param data
> > + *   Data to compare against.
> > + *
> > + * @return
> > + *   0 if the device matches the data.
> > + *   !0 if the device does not match.
> > + *   <0 if ordering is possible and the device is lower than the data.
> > + *   >0 if ordering is possible and the device is greater than the data.
> > + */
> > +typedef int (*rte_dev_cmp_t)(const struct rte_device *dev, const void *data);
> 
> data is really abstract.
> Maybe a comment is missing to explain that data is better specified
> in bus implementations?
> 

I'm not sure it is better specified in rte_bus though :).
However, the usage can be understood there, why it exists in the first
place.

I think bus iterators could benefit some more explanation about the why.

> Why not implement it for PCI?
> 

I sent this series with only the patches from Jan, initially in the
version he solely developed. Only afterward did I fix a few bugs,
reworked a few APIs.

As such, two other series complete this patchset:

[dpdk-dev] [PATCH v3] pci: implement find_device bus operation
http://dpdk.org/ml/archives/dev/2017-June/067485.html

And

[dpdk-dev] [PATCH v3 0/3] eal: complete attach / detach support
http://dpdk.org/ml/archives/dev/2017-June/067516.html

It might make sense to merge all three series together.
They are conceptually linked very closely. The only reason I did not do
so at first was because I was unsure about who would take responsibility
for the attach / detach patchset, and if it had not be me I did not want to
put undue responsibility of my patches on whomever would.

But that point is moot now.

-- 
Gaëtan Rivet
6WIND

Re: [PATCH v4 4/9] bus: add bus helper iterator to find a particular device

[Thomas Monjalon]

21/06/2017 01:29, Gaetan Rivet:
>  /**
> + * Bus iterator to find a particular device.

It should be said that it is iterating over every registered buses.

> + *
> + * If the callback returns non-zero this function will stop iterating over any
> + * more buses and devices. To continue a search the device of a previous search
> + * is passed via the start parameters.
> + *
> + * @param start
> + *	 Start device of the iteration.
> + *
> + * @param cmp
> + *	 Callback function to check device.
> + *
> + * @param data
> + *	 Data to pass to match callback.
> + *
> + * @return
> + *	 A pointer to a rte_bus structure or NULL in case no bus matches.
> + */
> +struct rte_device *
> +rte_bus_find_device(const struct rte_device *start,
> +		    rte_dev_cmp_t cmp, const void *data);

The order of the parameters is different of rte_bus_find():
    struct rte_bus *rte_bus_find(rte_bus_cmp_t cmp,
                                 const void *data,
                                 const struct rte_bus *start);

Re: [PATCH v4 9/9] ethdev: use embedded rte_device to detach driver

[Thomas Monjalon]

21/06/2017 01:29, Gaetan Rivet:
> --- a/lib/librte_ether/rte_ethdev.c
> +++ b/lib/librte_ether/rte_ethdev.c
> @@ -354,26 +354,14 @@ rte_eth_dev_get_port_by_name(const char *name, uint8_t *port_id)
>  static int
>  rte_eth_dev_is_detachable(uint8_t port_id)
>  {
> -	uint32_t dev_flags;
> +	struct rte_eth_dev *dev;
>  
>  	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -EINVAL);
>  
> -	switch (rte_eth_devices[port_id].data->kdrv) {
> -	case RTE_KDRV_IGB_UIO:
> -	case RTE_KDRV_UIO_GENERIC:
> -	case RTE_KDRV_NIC_UIO:
> -	case RTE_KDRV_NONE:
> -	case RTE_KDRV_VFIO:
> -		break;
> -	default:
> -		return -ENOTSUP;
> -	}
> -	dev_flags = rte_eth_devices[port_id].data->dev_flags;
> -	if ((dev_flags & RTE_ETH_DEV_DETACHABLE) &&
> -		(!(dev_flags & RTE_ETH_DEV_BONDED_SLAVE)))
> +	dev = &rte_eth_devices[port_id];
> +	if (dev->data->dev_flags & RTE_ETH_DEV_BONDED_SLAVE)
>  		return 0;
> -	else
> -		return 1;
> +	return !!dev->device->devargs->bus->unplug;
>  }

The changes in rte_eth_dev_is_detachable() deserves a separate commit.

Are you removing RTE_ETH_DEV_DETACHABLE flag?

I think this change cannot be done before implementing unplug in PCI.

Re: [PATCH v4 9/9] ethdev: use embedded rte_device to detach driver

[Thomas Monjalon]

21/06/2017 01:29, Gaetan Rivet:
> +	return !!dev->device->devargs->bus->unplug;

lib/librte_ether/rte_ethdev.c:364:33: fatal error:
no member named 'bus' in 'struct rte_devargs'

Please move this patch alone, specifying the right dependency.

Re: [PATCH v2 00/11] bus: attach / detach API

[Gaëtan Rivet]

Hi Stephen,

On Wed, May 31, 2017 at 08:34:26AM -0700, Stephen Hemminger wrote:
> On Wed, 31 May 2017 15:17:45 +0200
> Gaetan Rivet <gaetan.rivet@6wind.com> wrote:
> 
> > Following the work from Jan:
> > 
> > This patchset introduces the attach / detach API to rte_bus.
> > The rte_device structure is used as the generic device representation.
> > 
> > This API is implemented for the virtual bus.
> > The functions rte_eal_dev_attach and rte_eal_dev_detach are updated to
> > use this new interface.
> > 
> > --
> > 
> > 0. API rework
> > -------------
> > 
> > I would like to propose an evolution on the API developed by Jan.
> > 
> > The attach / detach rte_bus API is necessary to support the attach/detach
> > rte_dev API. Those are two different levels for one similar functionality.
> > 
> > Attach / detach does not allow true hotplugging, because the attach
> > function expects the devices operated upon to already exist within the
> > buses / sub-layers. This means that this API expects devices meta-datas
> > (bus-internal device representation and associated device information
> > read from the system) to be present upon attach. This part of the work
> > is done during scanning.
> > 
> > While it is best to avoid changing the public rte_dev API as it already
> > exists, nothing prevents this new rte_bus API from superseeding it.
> > It has been said during the previous release cycle that device hotplug
> > was a feature that interested users. True hotplug is not allowed by the
> > current attach / detach API. Worse, this API hinders the effort to bring
> > this new functionality by squatting its semantic field.
> > 
> > Thus, I propose to rename rte_bus attach / detach; plug / unplug. As it
> > is a superset of the attach / detach functionality, it can be used to
> > implement rte_dev attach / detach. Now is the right time to pivot to
> > this new feature.
> > 
> > This should help maintainers understanding the aim of this API and the
> > differences with the APIs higher-up, clarify the field and allow a new
> > functionality to be proposed.
> > 
> > The vdev bus is inherently supporting the new API, however it has been
> > made explicit. My implementation in the PCI bus in further patchset also
> > follows the rte_bus hotplug API instead of only attach / detach.
> > 
> > One remaining problem with the vdev bus is the rte_dev attach
> > implementation, which needs the rte_devargs rework to be properly fixed.
> > 
> > 1. Additional evolutions in the patchset
> > ----------------------------------------
> > 
> > The RTE_VERIFY on the find_device is too stringent I think and forces
> > all buses to implement a public device iterator. While it could be
> > argued that it would push for quicker support for the functionality, I
> > think it's possible that some buses are not interested at all in it and
> > should simply be ignored.
> > 
> > The bus devices iterator has been fixed.
> > 
> > The internal rte_device handle was not properly setup within the
> > net_ring PMD.
> > 
> > Gaetan Rivet (2):
> >   vdev: implement hotplug functionality
> >   net/ring: fix dev handle in eth_dev
> > 
> > Jan Blunck (9):
> >   bus: add bus iterator to find a particular bus
> >   bus: add device iterator
> >   bus: add helper to find bus for a particular device
> >   bus: add bus helper iterator to find a particular device
> >   bus: introduce hotplug functionality
> >   vdev: implement find_device bus operation
> >   vdev: implement unplug bus operation
> >   eal: make virtual driver probe and remove take rte_vdev_device
> >   ethdev: Use embedded rte_device to detach driver
> > 
> >  drivers/net/ring/rte_eth_ring.c                 |   7 ++
> >  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   4 +
> >  lib/librte_eal/common/eal_common_bus.c          |  65 +++++++++++++++
> >  lib/librte_eal/common/eal_common_dev.c          | 100 ++++++++++++++++++------
> >  lib/librte_eal/common/eal_common_vdev.c         |  27 +++++++
> >  lib/librte_eal/common/include/rte_bus.h         |  87 +++++++++++++++++++++
> >  lib/librte_eal/common/include/rte_dev.h         |  26 ++++++
> >  lib/librte_eal/linuxapp/eal/rte_eal_version.map |   3 +
> >  lib/librte_ether/rte_ethdev.c                   |   3 +-
> >  9 files changed, 299 insertions(+), 23 deletions(-)
> > 
> 
> LGTM
> 
> Maybe we should evolve it by having both rte_bus and  rte_dev API for one release and mark
> the rte_dev API for attach/detach as deprecated?

Sorry for the late response.

I think that if the hotplug API is correctly designed, the rte_dev API
could indeed be deprecated and then removed. I do not see a reason right
now to keep it.

-- 
Gaëtan Rivet
6WIND