DPDK-dev Archive on lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH v3 04/12] eal/bus: add scan, match and insert support
From: Shreyansh Jain @ 2016-12-16 13:25 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas.monjalon, ferruh.yigit, jianbo.liu
In-Reply-To: <1481893853-31790-5-git-send-email-shreyansh.jain@nxp.com>

On Friday 16 December 2016 06:40 PM, Shreyansh Jain wrote:
> When a PMD is registred, it will associate itself with a bus.
>
> A bus is responsible for 'scan' of all the devices attached to it.
> All the scanned devices are attached to bus specific device_list.
> During the probe operation, 'match' of the drivers and devices would
> be done.
>
> Also, rather than adding a device to tail, a new device might be added to
> the list (pivoted on bus) at a predefined position, for example, adding it
> in order of addressing. Support for this is added as '*bus_insert'.
>
> This patch also adds necessary test framework to test the scan and
> match callbacks.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>  app/test/test_bus.c                     | 265 ++++++++++++++++++++++++++++++++
>  lib/librte_eal/common/eal_common_bus.c  |  15 ++
>  lib/librte_eal/common/include/rte_bus.h |  64 ++++++++
>  3 files changed, 344 insertions(+)
>
> diff --git a/app/test/test_bus.c b/app/test/test_bus.c
> index 760d40a..ed95479 100644
> --- a/app/test/test_bus.c
> +++ b/app/test/test_bus.c
> @@ -80,12 +80,32 @@ struct dummy_bus {
>  struct rte_bus_list orig_bus_list =
>  	TAILQ_HEAD_INITIALIZER(orig_bus_list);
>
> +/* Forward declarations for callbacks from bus */
> +
> +/* Bus A
> + * Scan would register devA1 and devA2 to bus
> + */
> +int scan_fn_for_busA(struct rte_bus *bus);
> +
> +/* Bus B
> + * Scan would register devB1 and devB2 to bus
> + */
> +int scan_fn_for_busB(struct rte_bus *bus);
> +
> +/* generic implementations wrapped around by above declarations */
> +static int generic_scan_fn(struct rte_bus *bus);
> +static int generic_match_fn(struct rte_driver *drv, struct rte_device *dev);
> +
>  struct rte_bus busA = {
>  	.name = "busA", /* "busA" */
> +	.scan = scan_fn_for_busA,
> +	.match = generic_match_fn,
>  };
>
>  struct rte_bus busB = {
>  	.name = "busB", /* "busB */
> +	.scan = scan_fn_for_busB,
> +	.match = generic_match_fn,
>  };
>
>  struct rte_driver driverA = {
> @@ -184,6 +204,92 @@ dump_device_tree(void)
>  	printf("------>8-------\n");
>  }
>
> +/* @internal
> + * Move over the dummy_buses and find the entry matching the bus object
> + * passed as argument.
> + * For each device in that dummy_buses list, register.
> + *
> + * @param bus
> + *	bus to scan againt test entry
> + * @return
> + *	0 for successful scan, even if no devices are found
> + *	!0 for any error in scanning (like, invalid bus)
> + */
> +static int
> +generic_scan_fn(struct rte_bus *bus)
> +{
> +	int i = 0;
> +	struct rte_device *dev = NULL;
> +	struct dummy_bus *db = NULL;
> +
> +	if (!bus)
> +		return -1;
> +
> +	/* Extract the device tree node using the bus passed */
> +	for (i = 0; dummy_buses[i].name; i++) {
> +		if (!strcmp(dummy_buses[i].name, bus->name)) {
> +			db = &dummy_buses[i];
> +			break;
> +		}
> +	}
> +
> +	if (!db)
> +		return -1;
> +
> +	/* For all the devices in the device tree (dummy_buses), add device */
> +	for (i = 0; db->devices[i]; i++) {
> +		dev = &(db->devices[i]->dev);
> +		rte_eal_bus_add_device(bus, dev);
> +	}
> +
> +	return 0;
> +}
> +
> +/* @internal
> + * Obtain bus from driver object. Match the address of rte_device object
> + * with all the devices associated with that bus.
> + *
> + * Being a test function, all this does is validate that device object
> + * provided is available on the same bus to which driver is registered.
> + *
> + * @param drv
> + *	driver to which matching is to be performed
> + * @param dev
> + *	device object to match with driver
> + * @return
> + *	0 for successful match
> + *	!0 for failed match
> + */
> +static int
> +generic_match_fn(struct rte_driver *drv, struct rte_device *dev)
> +{
> +	struct rte_bus *bus;
> +	struct rte_device *dev_p = NULL;
> +
> +	/* Match is based entirely on address of 'dev' and 'dev_p' extracted
> +	 * from bus->device_list.
> +	 */
> +
> +	/* a driver is registered with the bus *before* the scan. */
> +	bus = drv->bus;
> +	TAILQ_FOREACH(dev_p, &bus->device_list, next) {
> +		if (dev == dev_p)
> +			return 0;
> +	}
> +
> +	return 1;
> +}
> +
> +int
> +scan_fn_for_busA(struct rte_bus *bus) {
> +	return generic_scan_fn(bus);
> +}
> +
> +int
> +scan_fn_for_busB(struct rte_bus *bus) {
> +	return generic_scan_fn(bus);
> +}
> +
>  static int
>  test_bus_setup(void)
>  {
> @@ -391,6 +497,155 @@ test_driver_unregistration_on_bus(void)
>
>  }
>
> +static int
> +test_device_unregistration_on_bus(void)
> +{
> +	int i;
> +	struct rte_bus *bus = NULL;
> +	struct rte_device *dev;
> +
> +	for (i = 0; dummy_buses[i].name; i++) {
> +		bus = rte_eal_get_bus(dummy_buses[i].name);
> +		if (!bus) {
> +			printf("Unable to find bus (%s)\n",
> +			       dummy_buses[i].name);
> +			return -1;
> +		}
> +
> +		/* For bus 'bus', unregister all devices */
> +		TAILQ_FOREACH(dev, &bus->device_list, next) {
> +			rte_eal_bus_remove_device(dev);
> +		}
> +	}
> +
> +	for (i = 0; dummy_buses[i].name; i++) {
> +		bus = rte_eal_get_bus(dummy_buses[i].name);
> +
> +		if (!TAILQ_EMPTY(&bus->device_list)) {
> +			printf("Unable to remove all devices on bus (%s)\n",
> +			       bus->name);
> +			return -1;
> +		}
> +	}
> +
> +	/* All devices from all buses have been removed */
> +	printf("All devices on all buses unregistered.\n");
> +	dump_device_tree();
> +
> +	return 0;
> +}
> +
> +/* @internal
> + * For each bus registered, call the scan function to identify devices
> + * on the bus.
> + *
> + * @param void
> + * @return
> + *	0 for successful scan
> + *	!0 for unsuccessful scan
> + *
> + */
> +static int
> +test_bus_scan(void)
> +{
> +	int ret;
> +	struct rte_bus *bus;
> +
> +	TAILQ_FOREACH(bus, &rte_bus_list, next) {
> +		/* Call the scan function for each bus */
> +		ret = bus->scan(bus);
> +		if (ret) {
> +			printf("Scan of buses failed.\n");
> +			return -1;
> +		}
> +	}
> +
> +	printf("Scan of all buses completed.\n");
> +	dump_device_tree();
> +
> +	return 0;
> +}
> +
> +/* @internal
> + * Function to perform 'probe' and link devices and drivers on a bus.
> + * This would work over all the buses registered, and all devices and drivers
> + * registered with it - call match on each pair.
> + * Aim is to test the match_fn for each bus.
> + *
> + * @param void
> + * @return
> + *	0 for successful probe
> + *	!0 for failure in probe
> + *
> + */
> +static int
> +test_probe_on_bus(void)
> +{
> +	int ret = 0;
> +	int i, j;
> +	struct rte_bus *bus = NULL;
> +	struct rte_device *dev = NULL;
> +	struct rte_driver *drv = NULL;
> +
> +	/* In case of this test:
> +	* 1. for each bus in rte_bus_list
> +	* 2.  for each device in bus->device_list
> +	* 3.   for each driver in bus->driver_list
> +	* 4.    call match
> +	* 5.    link driver and device
> +	* 6. Verify the linkage.
> +	*/

This comment style is indeed in a warning from checkpatch. My bad -
somehow I missed fixing this before sending out. v4, along with any
other major comments/reviews.

<snip>

-
Shreyansh

^ permalink raw reply

* Re: [PATCH v3 11/12] eal: enable PCI bus and PCI test framework
From: Shreyansh Jain @ 2016-12-16 13:20 UTC (permalink / raw)
  To: dev; +Cc: david.marchand, thomas.monjalon, ferruh.yigit, jianbo.liu
In-Reply-To: <1481893853-31790-12-git-send-email-shreyansh.jain@nxp.com>

On Friday 16 December 2016 06:40 PM, Shreyansh Jain wrote:
> Register a PCI bus with Scan/match and probe callbacks. Necessary changes
> in EAL layer for enabling bus interfaces. PCI devices and drivers now
> reside within the Bus object.
>
> Now that PCI bus handles the scan/probe methods, independent calls to
> PCI scan and probe can be removed from the code.
> PCI device and driver list are also removed.
>
> rte_device and rte_driver list continue to exist. As does the VDEV lists.
>
> Changes to test_pci:
> - use a dummy test_pci_bus for all PCI test driver registrations
> - this reduces the need for cleaning global list
> - add necessary callbacks for invoking scan and probing/matching
>   using EAL PCI scan code
>
> Note: With this patch, all PCI PMDs would cease to work because of lack
>       rte_driver->probe/remove implementations. Next patch would do that.
>
> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> ---
>  app/test/test_pci.c                             | 154 +++++++++++------
>  lib/librte_eal/bsdapp/eal/eal.c                 |   7 -
>  lib/librte_eal/bsdapp/eal/eal_pci.c             |  52 +++---
>  lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   7 +-
>  lib/librte_eal/common/eal_common_pci.c          | 212 ++++++++++++++----------
>  lib/librte_eal/common/eal_private.h             |  14 +-
>  lib/librte_eal/common/include/rte_pci.h         |  53 +++---
>  lib/librte_eal/linuxapp/eal/eal.c               |   7 -
>  lib/librte_eal/linuxapp/eal/eal_pci.c           |  57 ++++---
>  lib/librte_eal/linuxapp/eal/rte_eal_version.map |   7 +-
>  10 files changed, 330 insertions(+), 240 deletions(-)
>
<snip>

This is a relatively large patch. I would love to split it but
currently I am unable to find a nice/clean way. Any suggestions would
be really appreciated:

This currently does 3 broad things:
  - Move the PCI device/driver registration to PCI Bus
   -- So, introduce PCI bus
   -- change EAL to point to this new bus
  - remove/refactor existing EAL/*/*pci* code to accommodate this shift
  - Test PCI framework.

I could have split test_pci changes into a new patch, but that breaks
compilation after this patchset if test_pci compilation is enabled
(disabled by default). Is that acceptable? (if not, I am stuck with
keeping this set of changes into a single patch).

And, I would appreciate some help in confirming if the changes to
test_pci are OK or not from PCI testing perspective.

-
Shreyansh

^ permalink raw reply

* [PATCH v3 11/12] eal: enable PCI bus and PCI test framework
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

Register a PCI bus with Scan/match and probe callbacks. Necessary changes
in EAL layer for enabling bus interfaces. PCI devices and drivers now
reside within the Bus object.

Now that PCI bus handles the scan/probe methods, independent calls to
PCI scan and probe can be removed from the code.
PCI device and driver list are also removed.

rte_device and rte_driver list continue to exist. As does the VDEV lists.

Changes to test_pci:
- use a dummy test_pci_bus for all PCI test driver registrations
- this reduces the need for cleaning global list
- add necessary callbacks for invoking scan and probing/matching
  using EAL PCI scan code

Note: With this patch, all PCI PMDs would cease to work because of lack
      rte_driver->probe/remove implementations. Next patch would do that.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 app/test/test_pci.c                             | 154 +++++++++++------
 lib/librte_eal/bsdapp/eal/eal.c                 |   7 -
 lib/librte_eal/bsdapp/eal/eal_pci.c             |  52 +++---
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |   7 +-
 lib/librte_eal/common/eal_common_pci.c          | 212 ++++++++++++++----------
 lib/librte_eal/common/eal_private.h             |  14 +-
 lib/librte_eal/common/include/rte_pci.h         |  53 +++---
 lib/librte_eal/linuxapp/eal/eal.c               |   7 -
 lib/librte_eal/linuxapp/eal/eal_pci.c           |  57 ++++---
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |   7 +-
 10 files changed, 330 insertions(+), 240 deletions(-)

diff --git a/app/test/test_pci.c b/app/test/test_pci.c
index f9b84db..e95b758 100644
--- a/app/test/test_pci.c
+++ b/app/test/test_pci.c
@@ -38,6 +38,7 @@
 #include <sys/queue.h>
 
 #include <rte_interrupts.h>
+#include <rte_bus.h>
 #include <rte_pci.h>
 #include <rte_ethdev.h>
 #include <rte_devargs.h>
@@ -61,10 +62,18 @@
 
 int test_pci_run = 0; /* value checked by the multiprocess test */
 static unsigned pci_dev_count;
+static struct rte_bus *pci_bus; /* global reference to a Test PCI bus */
 
 static int my_driver_init(struct rte_pci_driver *dr,
 			  struct rte_pci_device *dev);
 
+/* Test PCI bus. */
+struct rte_bus test_pci_bus = {
+	.name = "test_pci_bus",
+	.scan = rte_eal_pci_scan,
+	.match = rte_eal_pci_match,
+};
+
 /* IXGBE NICS */
 struct rte_pci_id my_driver_id[] = {
 	{RTE_PCI_DEVICE(0x0001, 0x1234)},
@@ -79,7 +88,9 @@ struct rte_pci_id my_driver_id2[] = {
 
 struct rte_pci_driver my_driver = {
 	.driver = {
-		.name = "test_driver"
+		.name = "test_driver",
+		.probe = rte_eal_pci_probe,
+		.remove = rte_eal_pci_remove,
 	},
 	.probe = my_driver_init,
 	.id_table = my_driver_id,
@@ -88,7 +99,9 @@ struct rte_pci_driver my_driver = {
 
 struct rte_pci_driver my_driver2 = {
 	.driver = {
-		.name = "test_driver2"
+		.name = "test_driver2",
+		.probe = rte_eal_pci_probe,
+		.remove = rte_eal_pci_remove,
 	},
 	.probe = my_driver_init,
 	.id_table = my_driver_id2,
@@ -108,14 +121,67 @@ my_driver_init(__attribute__((unused)) struct rte_pci_driver *dr,
 	return 0;
 }
 
+/* dump devices on the bus */
+static void
+do_pci_device_dump(FILE *f)
+{
+	int i;
+	struct rte_pci_device *dev = NULL;
+	struct rte_device *r_dev = NULL;
+
+	TAILQ_FOREACH(r_dev, &pci_bus->device_list, next) {
+		dev = container_of(r_dev, struct rte_pci_device, device);
+
+		fprintf(f, PCI_PRI_FMT, dev->addr.domain, dev->addr.bus,
+		       dev->addr.devid, dev->addr.function);
+		fprintf(f, " - vendor:%x device:%x\n", dev->id.vendor_id,
+		       dev->id.device_id);
+
+		for (i = 0; i != sizeof(dev->mem_resource) /
+			sizeof(dev->mem_resource[0]); i++) {
+			fprintf(f, "   %16.16"PRIx64" %16.16"PRIx64"\n",
+				dev->mem_resource[i].phys_addr,
+				dev->mem_resource[i].len);
+		}
+	}
+}
+
+/* Dummy implementation for rte_eal_pci_probe() over test_pci_bus */
+static int
+do_pci_bus_probe(void)
+{
+	int ret;
+	struct rte_device *device;
+	struct rte_driver *driver;
+
+	TAILQ_FOREACH(device, &pci_bus->device_list, next) {
+		TAILQ_FOREACH(driver, &pci_bus->driver_list, next) {
+			ret = pci_bus->match(driver, device);
+			if (!ret) {
+				if (!driver->probe)
+					continue;
+
+				device->driver = driver;
+				ret = driver->probe(driver, device);
+				if (ret != 0)
+					return ret;
+			}
+		}
+	}
+
+	return 0;
+}
+
 static void
 blacklist_all_devices(void)
 {
 	struct rte_pci_device *dev = NULL;
+	struct rte_device *device = NULL;
 	unsigned i = 0;
 	char pci_addr_str[16];
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
+	TAILQ_FOREACH(device, &(pci_bus->device_list), next) {
+		dev = container_of(device, struct rte_pci_device, device);
 		snprintf(pci_addr_str, sizeof(pci_addr_str), PCI_PRI_FMT,
 			dev->addr.domain, dev->addr.bus, dev->addr.devid,
 			dev->addr.function);
@@ -142,19 +208,11 @@ static void free_devargs_list(void)
 	}
 }
 
-/* backup real devices & drivers (not used for testing) */
-struct pci_driver_list real_pci_driver_list =
-	TAILQ_HEAD_INITIALIZER(real_pci_driver_list);
-struct pci_device_list real_pci_device_list =
-	TAILQ_HEAD_INITIALIZER(real_pci_device_list);
-
 REGISTER_LINKED_RESOURCE(test_pci_sysfs);
 
 static int
 test_pci_setup(void)
 {
-	struct rte_pci_device *dev;
-	struct rte_pci_driver *dr;
 	const struct resource *r;
 	int ret;
 
@@ -167,22 +225,19 @@ test_pci_setup(void)
 	ret = setenv("SYSFS_PCI_DEVICES", "test_pci_sysfs/bus/pci/devices", 1);
 	TEST_ASSERT_SUCCESS(ret, "failed to setenv");
 
-	/* Unregister original devices & drivers lists */
-	while (!TAILQ_EMPTY(&pci_driver_list)) {
-		dr = TAILQ_FIRST(&pci_driver_list);
-		rte_eal_pci_unregister(dr);
-		TAILQ_INSERT_TAIL(&real_pci_driver_list, dr, next);
-	}
+	/* Create a new Bus called 'test_pci_bus' */
+	/* Bus doesn't exist; Create the test bus */
+	printf("Creating a Test PCI bus\n");
+	rte_eal_bus_register(&test_pci_bus);
+	pci_bus = &test_pci_bus;
 
-	while (!TAILQ_EMPTY(&pci_device_list)) {
-		dev = TAILQ_FIRST(&pci_device_list);
-		TAILQ_REMOVE(&pci_device_list, dev, next);
-		TAILQ_INSERT_TAIL(&real_pci_device_list, dev, next);
-	}
+	printf("Scan for Test devices and add to bus\n");
+	ret = pci_bus->scan(pci_bus);
 
-	ret = rte_eal_pci_scan(NULL);
 	TEST_ASSERT_SUCCESS(ret, "failed to scan PCI bus");
-	rte_eal_pci_dump(stdout);
+
+	printf("Dump of all devices scanned:\n");
+	do_pci_device_dump(stdout);
 
 	return 0;
 }
@@ -190,8 +245,8 @@ test_pci_setup(void)
 static int
 test_pci_cleanup(void)
 {
-	struct rte_pci_device *dev;
-	struct rte_pci_driver *dr;
+	struct rte_device *dev = NULL;
+	struct rte_driver *dr = NULL;
 	const struct resource *r;
 	int ret;
 
@@ -203,28 +258,23 @@ test_pci_cleanup(void)
 	ret = resource_rm_by_tar(r);
 	TEST_ASSERT_SUCCESS(ret, "Failed to delete resource %s", r->name);
 
+	TEST_ASSERT_NOT_NULL(pci_bus, "Invalid bus specified");
+
 	/*
 	 * FIXME: there is no API in DPDK to free a rte_pci_device so we
 	 * cannot free the devices in the right way. Let's assume that we
 	 * don't care for tests.
 	 */
-	while (!TAILQ_EMPTY(&pci_device_list)) {
-		dev = TAILQ_FIRST(&pci_device_list);
-		TAILQ_REMOVE(&pci_device_list, dev, next);
+	TAILQ_FOREACH(dev, &(pci_bus->device_list), next) {
+		rte_eal_bus_remove_device(dev);
+		dev->driver = NULL;
 	}
 
-	/* Restore original devices & drivers lists */
-	while (!TAILQ_EMPTY(&real_pci_driver_list)) {
-		dr = TAILQ_FIRST(&real_pci_driver_list);
-		TAILQ_REMOVE(&real_pci_driver_list, dr, next);
-		rte_eal_pci_register(dr);
+	TAILQ_FOREACH(dr, &(pci_bus->driver_list), next) {
+		rte_eal_bus_remove_driver(dr);
 	}
 
-	while (!TAILQ_EMPTY(&real_pci_device_list)) {
-		dev = TAILQ_FIRST(&real_pci_device_list);
-		TAILQ_REMOVE(&real_pci_device_list, dev, next);
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
-	}
+	rte_eal_bus_unregister(pci_bus);
 
 	return 0;
 }
@@ -234,16 +284,19 @@ test_pci_blacklist(void)
 {
 	struct rte_devargs_list save_devargs_list;
 
-	printf("Dump all devices\n");
-	TEST_ASSERT(TAILQ_EMPTY(&pci_driver_list),
-			"pci_driver_list not empty");
+	TEST_ASSERT_NOT_NULL(pci_bus, "Invalid bus specified");
+
+	TEST_ASSERT(TAILQ_EMPTY(&pci_bus->driver_list),
+		    "PCI Driver list not empty");
 
-	rte_eal_pci_register(&my_driver);
-	rte_eal_pci_register(&my_driver2);
+	/* Add test drivers to Bus */
+	rte_eal_bus_add_driver(pci_bus, &(my_driver.driver));
+	rte_eal_bus_add_driver(pci_bus, &(my_driver2.driver));
 
 	pci_dev_count = 0;
-	printf("Scan bus\n");
-	rte_eal_pci_probe();
+
+	printf("Probe the Test Bus\n");
+	do_pci_bus_probe();
 
 	if (pci_dev_count == 0) {
 		printf("no device detected\n");
@@ -257,8 +310,8 @@ test_pci_blacklist(void)
 	blacklist_all_devices();
 
 	pci_dev_count = 0;
-	printf("Scan bus with all devices blacklisted\n");
-	rte_eal_pci_probe();
+	printf("Probe bus with all devices blacklisted\n");
+	do_pci_bus_probe();
 
 	free_devargs_list();
 	devargs_list = save_devargs_list;
@@ -270,8 +323,9 @@ test_pci_blacklist(void)
 
 	test_pci_run = 1;
 
-	rte_eal_pci_unregister(&my_driver);
-	rte_eal_pci_unregister(&my_driver2);
+	/* Clear the test drivers added to Test Bus */
+	rte_eal_bus_remove_driver(&(my_driver.driver));
+	rte_eal_bus_remove_driver(&(my_driver2.driver));
 
 	return 0;
 }
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 30afc6b..79d82d4 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -562,9 +562,6 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_timer_init() < 0)
 		rte_panic("Cannot init HPET or TSC timers\n");
 
-	if (rte_eal_pci_init() < 0)
-		rte_panic("Cannot init PCI\n");
-
 	eal_check_mem_on_local_socket();
 
 	if (eal_plugins_init() < 0)
@@ -616,10 +613,6 @@ rte_eal_init(int argc, char **argv)
 	rte_eal_mp_remote_launch(sync_func, NULL, SKIP_MASTER);
 	rte_eal_mp_wait_lcore();
 
-	/* Probe & Initialize PCI devices */
-	if (rte_eal_pci_probe())
-		rte_panic("Cannot probe PCI\n");
-
 	if (rte_eal_bus_probe())
 		rte_panic("Cannot probe devices\n");
 
diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 10b234e..ab9f9d2 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -58,6 +58,7 @@
 
 #include <rte_interrupts.h>
 #include <rte_log.h>
+#include <rte_bus.h>
 #include <rte_pci.h>
 #include <rte_common.h>
 #include <rte_launch.h>
@@ -249,7 +250,7 @@ pci_uio_map_resource_by_index(struct rte_pci_device *dev, int res_idx,
 }
 
 static int
-pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
+pci_scan_one(struct rte_bus *bus, int dev_pci_fd, struct pci_conf *conf)
 {
 	struct rte_pci_device *dev;
 	struct pci_bar_io bar;
@@ -322,19 +323,23 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 	}
 
 	/* device is valid, add in list (sorted) */
-	if (TAILQ_EMPTY(&pci_device_list)) {
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
+	if (TAILQ_EMPTY(&bus->device_list)) {
+		rte_eal_bus_add_device(bus, &dev->device);
 	}
 	else {
 		struct rte_pci_device *dev2 = NULL;
+		struct rte_device *r_dev2;
 		int ret;
 
-		TAILQ_FOREACH(dev2, &pci_device_list, next) {
+		TAILQ_FOREACH(r_dev2, &bus->device_list, next) {
+			dev2 = container_of(r_dev2, struct rte_pci_device,
+					    device);
 			ret = rte_eal_compare_pci_addr(&dev->addr, &dev2->addr);
 			if (ret > 0)
 				continue;
 			else if (ret < 0) {
-				TAILQ_INSERT_BEFORE(dev2, dev, next);
+				rte_eal_bus_insert_device(bus, &dev2->device,
+							  &dev->device);
 				return 0;
 			} else { /* already registered */
 				dev2->kdrv = dev->kdrv;
@@ -346,7 +351,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 				return 0;
 			}
 		}
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
+		rte_eal_bus_add_device(bus, &dev->device);
 	}
 
 	return 0;
@@ -361,7 +366,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
  * list. Call pci_scan_one() for each pci entry found.
  */
 int
-rte_eal_pci_scan(struct rte_bus *bus __rte_unused)
+rte_eal_pci_scan(struct rte_bus *bus)
 {
 	int fd;
 	unsigned dev_count = 0;
@@ -374,6 +379,10 @@ rte_eal_pci_scan(struct rte_bus *bus __rte_unused)
 			.matches = &matches[0],
 	};
 
+	/* for debug purposes, PCI can be disabled */
+	if (internal_config.no_pci)
+		return 0;
+
 	fd = open("/dev/pci", O_RDONLY);
 	if (fd < 0) {
 		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
@@ -389,7 +398,7 @@ rte_eal_pci_scan(struct rte_bus *bus __rte_unused)
 		}
 
 		for (i = 0; i < conf_io.num_matches; i++)
-			if (pci_scan_one(fd, &matches[i]) < 0)
+			if (pci_scan_one(bus, fd, &matches[i]) < 0)
 				goto error;
 
 		dev_count += conf_io.num_matches;
@@ -407,9 +416,9 @@ rte_eal_pci_scan(struct rte_bus *bus __rte_unused)
 }
 
 int
-pci_update_device(const struct rte_pci_addr *addr)
+pci_update_device(struct rte_bus *bus, const struct rte_pci_addr *addr)
 {
-	int fd;
+	int fd = -1;
 	struct pci_conf matches[2];
 	struct pci_match_conf match = {
 		.pc_sel = {
@@ -427,6 +436,9 @@ pci_update_device(const struct rte_pci_addr *addr)
 		.matches = &matches[0],
 	};
 
+	if (!bus)
+		goto error;
+
 	fd = open("/dev/pci", O_RDONLY);
 	if (fd < 0) {
 		RTE_LOG(ERR, EAL, "%s(): error opening /dev/pci\n", __func__);
@@ -442,7 +454,7 @@ pci_update_device(const struct rte_pci_addr *addr)
 	if (conf_io.num_matches != 1)
 		goto error;
 
-	if (pci_scan_one(fd, &matches[0]) < 0)
+	if (pci_scan_one(bus, fd, &matches[0]) < 0)
 		goto error;
 
 	close(fd);
@@ -668,17 +680,9 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p)
 	return ret;
 }
 
-/* Init the PCI EAL subsystem */
-int
-rte_eal_pci_init(void)
-{
-	/* for debug purposes, PCI can be disabled */
-	if (internal_config.no_pci)
-		return 0;
+struct rte_bus pci_bus = {
+	.scan = rte_eal_pci_scan,
+	.match = rte_eal_pci_match,
+};
 
-	if (rte_eal_pci_scan(NULL) < 0) {
-		RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
-		return -1;
-	}
-	return 0;
-}
+RTE_REGISTER_BUS(pci, pci_bus);
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 23fc1c1..3dcf439 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -6,8 +6,6 @@ DPDK_2.0 {
 	eal_parse_sysfs_value;
 	eal_timer_source;
 	lcore_config;
-	pci_device_list;
-	pci_driver_list;
 	per_lcore__lcore_id;
 	per_lcore__rte_errno;
 	rte_calloc;
@@ -41,7 +39,6 @@ DPDK_2.0 {
 	rte_eal_mp_wait_lcore;
 	rte_eal_parse_devargs_str;
 	rte_eal_pci_dump;
-	rte_eal_pci_probe;
 	rte_eal_pci_probe_one;
 	rte_eal_pci_register;
 	rte_eal_pci_scan;
@@ -187,5 +184,9 @@ DPDK_17.02 {
 	rte_eal_bus_remove_device;
 	rte_eal_bus_remove_driver;
 	rte_eal_bus_unregister;
+	rte_eal_pci_match;
+	rte_eal_pci_probe;
+	rte_eal_pci_remove;
+	rte_eal_pci_scan;
 
 } DPDK_16.11;
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 562c0fd..e6d8d87 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -71,6 +71,7 @@
 
 #include <rte_interrupts.h>
 #include <rte_log.h>
+#include <rte_bus.h>
 #include <rte_pci.h>
 #include <rte_per_lcore.h>
 #include <rte_memory.h>
@@ -82,11 +83,6 @@
 
 #include "eal_private.h"
 
-struct pci_driver_list pci_driver_list =
-	TAILQ_HEAD_INITIALIZER(pci_driver_list);
-struct pci_device_list pci_device_list =
-	TAILQ_HEAD_INITIALIZER(pci_device_list);
-
 #define SYSFS_PCI_DEVICES "/sys/bus/pci/devices"
 
 const char *pci_get_sysfs_path(void)
@@ -206,39 +202,10 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
 			     struct rte_pci_device *dev)
 {
 	int ret;
-	struct rte_driver *driver;
-	struct rte_device *device;
-	struct rte_pci_addr *loc;
 
 	if ((dr == NULL) || (dev == NULL))
 		return -EINVAL;
 
-	driver = &dr->driver;
-	device = &dev->device;
-	loc = &dev->addr;
-
-	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
-			loc->domain, loc->bus, loc->devid, loc->function,
-			dev->device.numa_node);
-
-	/* no initialization when blacklisted, return without error */
-	if (dev->device.devargs != NULL &&
-		dev->device.devargs->type ==
-			RTE_DEVTYPE_BLACKLISTED_PCI) {
-		RTE_LOG(INFO, EAL, "  Device is blacklisted, not"
-			" initializing\n");
-		return 1;
-	}
-
-	/* The device is not blacklisted; Check if driver supports it */
-	ret = rte_eal_pci_match(driver, device);
-	if (ret) {
-		/* Match of device and driver failed */
-		RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
-			driver->name);
-		return 1;
-	}
-
 	RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
 			dev->id.device_id, dr->driver.name);
 
@@ -276,23 +243,11 @@ static int
 rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
 		struct rte_pci_device *dev)
 {
-	int ret;
-	struct rte_driver *driver = NULL;
-	struct rte_device *device;
 	struct rte_pci_addr *loc;
 
 	if ((dr == NULL) || (dev == NULL))
 		return -EINVAL;
 
-	driver = &(dr->driver);
-	device = &(dev->device);
-
-	ret = rte_eal_pci_match(driver, device);
-	if (ret) {
-		/* Device and driver don't match */
-		return 1;
-	}
-
 	loc = &dev->addr;
 
 	RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
@@ -321,9 +276,9 @@ rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
  * failed, return 1 if no driver is found for this device.
  */
 static int
-pci_probe_all_drivers(struct rte_pci_device *dev)
+pci_probe_all_drivers(struct rte_bus *bus, struct rte_pci_device *dev)
 {
-	struct rte_pci_driver *dr = NULL;
+	struct rte_driver *r_dr = NULL;
 	int rc = 0;
 
 	if (dev == NULL)
@@ -333,8 +288,8 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
 	if (dev->driver != NULL)
 		return 0;
 
-	TAILQ_FOREACH(dr, &pci_driver_list, next) {
-		rc = rte_eal_pci_probe_one_driver(dr, dev);
+	TAILQ_FOREACH(r_dr, &bus->driver_list, next) {
+		rc = rte_eal_pci_probe(r_dr, &dev->device);
 		if (rc < 0)
 			/* negative value is an error */
 			return -1;
@@ -352,15 +307,17 @@ pci_probe_all_drivers(struct rte_pci_device *dev)
  * failed, return 1 if no driver is found for this device.
  */
 static int
-pci_detach_all_drivers(struct rte_pci_device *dev)
+pci_detach_all_drivers(struct rte_bus *bus, struct rte_pci_device *dev)
 {
 	struct rte_pci_driver *dr = NULL;
+	struct rte_driver *r_dr = NULL;
 	int rc = 0;
 
 	if (dev == NULL)
 		return -1;
 
-	TAILQ_FOREACH(dr, &pci_driver_list, next) {
+	TAILQ_FOREACH(r_dr, &bus->driver_list, next) {
+		dr = container_of(r_dr, struct rte_pci_driver, driver);
 		rc = rte_eal_pci_detach_dev(dr, dev);
 		if (rc < 0)
 			/* negative value is an error */
@@ -381,22 +338,31 @@ int
 rte_eal_pci_probe_one(const struct rte_pci_addr *addr)
 {
 	struct rte_pci_device *dev = NULL;
+	struct rte_device *r_dev = NULL;
+	struct rte_bus *bus;
 	int ret = 0;
 
 	if (addr == NULL)
 		return -1;
 
+	bus = rte_eal_get_bus("pci");
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "PCI Bus not registered\n");
+		return -1;
+	}
+
 	/* update current pci device in global list, kernel bindings might have
 	 * changed since last time we looked at it.
 	 */
-	if (pci_update_device(addr) < 0)
+	if (pci_update_device(bus, addr) < 0)
 		goto err_return;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
+	TAILQ_FOREACH(r_dev, &bus->device_list, next) {
+		dev = container_of(r_dev, struct rte_pci_device, device);
 		if (rte_eal_compare_pci_addr(&dev->addr, addr))
 			continue;
 
-		ret = pci_probe_all_drivers(dev);
+		ret = pci_probe_all_drivers(bus, dev);
 		if (ret)
 			goto err_return;
 		return 0;
@@ -417,20 +383,29 @@ int
 rte_eal_pci_detach(const struct rte_pci_addr *addr)
 {
 	struct rte_pci_device *dev = NULL;
+	struct rte_device *r_dev = NULL;
+	struct rte_bus *bus;
 	int ret = 0;
 
 	if (addr == NULL)
 		return -1;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
+	bus = rte_eal_get_bus("pci");
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "PCI Bus is not registered\n");
+		return -1;
+	}
+
+	TAILQ_FOREACH(r_dev, &bus->device_list, next) {
+		dev = container_of(r_dev, struct rte_pci_device, device);
 		if (rte_eal_compare_pci_addr(&dev->addr, addr))
 			continue;
 
-		ret = pci_detach_all_drivers(dev);
+		ret = pci_detach_all_drivers(bus, dev);
 		if (ret < 0)
 			goto err_return;
 
-		TAILQ_REMOVE(&pci_device_list, dev, next);
+		rte_eal_bus_remove_device(r_dev);
 		free(dev);
 		return 0;
 	}
@@ -443,41 +418,73 @@ rte_eal_pci_detach(const struct rte_pci_addr *addr)
 	return -1;
 }
 
-/*
- * Scan the content of the PCI bus, and call the probe() function for
- * all registered drivers that have a matching entry in its id_table
- * for discovered devices.
- */
 int
-rte_eal_pci_probe(void)
+rte_eal_pci_probe(struct rte_driver *driver, struct rte_device *device)
 {
-	struct rte_pci_device *dev = NULL;
-	struct rte_devargs *devargs;
-	int probe_all = 0;
 	int ret = 0;
+	struct rte_devargs *devargs;
+	struct rte_pci_device *pci_dev;
+	struct rte_pci_driver *pci_drv;
+	struct rte_pci_addr *loc;
 
-	if (rte_eal_devargs_type_count(RTE_DEVTYPE_WHITELISTED_PCI) == 0)
-		probe_all = 1;
+	if (!driver || !device)
+		return -1;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
+	pci_dev = container_of(device, struct rte_pci_device, device);
+	pci_drv = container_of(driver, struct rte_pci_driver, driver);
 
-		/* set devargs in PCI structure */
-		devargs = pci_devargs_lookup(dev);
-		if (devargs != NULL)
-			dev->device.devargs = devargs;
+	loc = &pci_dev->addr;
 
-		/* probe all or only whitelisted devices */
-		if (probe_all)
-			ret = pci_probe_all_drivers(dev);
-		else if (devargs != NULL &&
-			devargs->type == RTE_DEVTYPE_WHITELISTED_PCI)
-			ret = pci_probe_all_drivers(dev);
-		if (ret < 0)
-			rte_exit(EXIT_FAILURE, "Requested device " PCI_PRI_FMT
-				 " cannot be used\n", dev->addr.domain, dev->addr.bus,
-				 dev->addr.devid, dev->addr.function);
+	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+			loc->domain, loc->bus, loc->devid, loc->function,
+			pci_dev->device.numa_node);
+
+	/* Fetch the devargs associated with the device */
+	devargs = pci_devargs_lookup(pci_dev);
+	if (devargs != NULL)
+		pci_dev->device.devargs = devargs;
+
+	/* no initialization when blacklisted, return without error */
+	if (pci_dev->device.devargs != NULL &&
+	    pci_dev->device.devargs->type == RTE_DEVTYPE_BLACKLISTED_PCI) {
+		RTE_LOG(INFO, EAL, "  Device is blacklisted, not"
+			" initializing\n");
+		return 1;
 	}
 
+	ret = rte_eal_pci_probe_one_driver(pci_drv, pci_dev);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMT
+			" cannot be used\n", pci_dev->addr.domain,
+			pci_dev->addr.bus, pci_dev->addr.devid,
+			pci_dev->addr.function);
+		return ret;
+	}
+	return 0;
+}
+
+int
+rte_eal_pci_remove(struct rte_device *device)
+{
+	int ret = 0;
+	struct rte_pci_device *pci_dev;
+
+	if (!device)
+		return -1;
+
+	pci_dev = container_of(device, struct rte_pci_device, device);
+
+	if (!pci_dev->driver)
+		return -1;
+
+	ret = rte_eal_pci_detach_dev(pci_dev->driver, pci_dev);
+	if (ret < 0) {
+		RTE_LOG(ERR, EAL, "Requested device " PCI_PRI_FMT
+			" cannot be used\n", pci_dev->addr.domain,
+			pci_dev->addr.bus, pci_dev->addr.devid,
+			pci_dev->addr.function);
+		return ret;
+	}
 	return 0;
 }
 
@@ -506,8 +513,17 @@ void
 rte_eal_pci_dump(FILE *f)
 {
 	struct rte_pci_device *dev = NULL;
+	struct rte_device *r_dev = NULL;
+	struct rte_bus *bus;
 
-	TAILQ_FOREACH(dev, &pci_device_list, next) {
+	bus = rte_eal_get_bus("pci");
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "PCI Bus not registered\n");
+		return;
+	}
+
+	TAILQ_FOREACH(r_dev, &bus->device_list, next) {
+		dev = container_of(r_dev, struct rte_pci_device, device);
 		pci_dump_one_device(f, dev);
 	}
 }
@@ -516,14 +532,32 @@ rte_eal_pci_dump(FILE *f)
 void
 rte_eal_pci_register(struct rte_pci_driver *driver)
 {
-	TAILQ_INSERT_TAIL(&pci_driver_list, driver, next);
-	rte_eal_driver_register(&driver->driver);
+	struct rte_bus *bus;
+
+	RTE_VERIFY(driver);
+
+	bus = rte_eal_get_bus("pci");
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "PCI Bus not registered\n");
+		return;
+	}
+
+	rte_eal_bus_add_driver(bus, &driver->driver);
 }
 
 /* unregister a driver */
 void
 rte_eal_pci_unregister(struct rte_pci_driver *driver)
 {
-	rte_eal_driver_unregister(&driver->driver);
-	TAILQ_REMOVE(&pci_driver_list, driver, next);
+	struct rte_bus *bus;
+
+	RTE_VERIFY(driver);
+
+	bus = driver->driver.bus;
+	if (!bus) {
+		RTE_LOG(ERR, EAL, "PCI Bus not registered\n");
+		return;
+	}
+
+	rte_eal_bus_remove_driver(&driver->driver);
 }
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 9e7d8f6..06ec172 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -108,16 +108,6 @@ int rte_eal_timer_init(void);
  */
 int rte_eal_log_init(const char *id, int facility);
 
-/**
- * Init the PCI infrastructure
- *
- * This function is private to EAL.
- *
- * @return
- *   0 on success, negative on error
- */
-int rte_eal_pci_init(void);
-
 struct rte_pci_driver;
 struct rte_pci_device;
 
@@ -126,13 +116,15 @@ struct rte_pci_device;
  *
  * This function is private to EAL.
  *
+ * @param bus
+ *	The PCI bus on which device is connected
  * @param addr
  *	The PCI Bus-Device-Function address to look for
  * @return
  *   - 0 on success.
  *   - negative on error.
  */
-int pci_update_device(const struct rte_pci_addr *addr);
+int pci_update_device(struct rte_bus *bus, const struct rte_pci_addr *addr);
 
 /**
  * Unbind kernel driver for this device
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 10108a4..ec8f672 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -86,12 +86,6 @@ extern "C" {
 #include <rte_interrupts.h>
 #include <rte_dev.h>
 
-TAILQ_HEAD(pci_device_list, rte_pci_device); /**< PCI devices in D-linked Q. */
-TAILQ_HEAD(pci_driver_list, rte_pci_driver); /**< PCI drivers in D-linked Q. */
-
-extern struct pci_driver_list pci_driver_list; /**< Global list of PCI drivers. */
-extern struct pci_device_list pci_device_list; /**< Global list of PCI devices. */
-
 /** Pathname of PCI devices directory. */
 const char *pci_get_sysfs_path(void);
 
@@ -372,6 +366,40 @@ rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
 int rte_eal_pci_scan(struct rte_bus *bus);
 
 /**
+ * Probe callback for the PCI bus
+ *
+ * For each matched pair of PCI device and driver on PCI bus, perform devargs
+ * check, and call a series of callbacks to allocate ethdev/cryptodev instances
+ * and intializing them.
+ *
+ * @param driver
+ *	Generic driver object matched with the device
+ * @param device
+ *	Generic device object to initialize
+ * @return
+ *   - 0 on success.
+ *   - !0 on error.
+ */
+int
+rte_eal_pci_probe(struct rte_driver *driver, struct rte_device *device);
+
+/**
+ * Remove callback for the PCI bus
+ *
+ * Called when a device needs to be removed from a bus; wraps around the
+ * PCI specific implementation layered over rte_pci_driver->remove. Default
+ * handler used by PCI PMDs
+ *
+ * @param device
+ *	rte_device object referring to device to be removed
+ * @return
+ *	- 0 for successful removal
+ *	- !0 for failure in removal of device
+ */
+int
+rte_eal_pci_remove(struct rte_device *device);
+
+/**
  * Match the PCI Driver and Device using the ID Table
  *
  * @param drv
@@ -387,19 +415,6 @@ rte_eal_pci_match(struct rte_driver *drv,
 		  struct rte_device *dev);
 
 /**
- * Probe the PCI bus for registered drivers.
- *
- * Scan the content of the PCI bus, and call the probe() function for
- * all registered drivers that have a matching entry in its id_table
- * for discovered devices.
- *
- * @return
- *   - 0 on success.
- *   - Negative on error.
- */
-int rte_eal_pci_probe(void);
-
-/**
  * Map the PCI device resources in user space virtual memory address
  *
  * Note that driver should not call this function when flag
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 01d0cee..ff92de2 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -803,9 +803,6 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_log_init(logid, internal_config.syslog_facility) < 0)
 		rte_panic("Cannot init logs\n");
 
-	if (rte_eal_pci_init() < 0)
-		rte_panic("Cannot init PCI\n");
-
 #ifdef VFIO_PRESENT
 	if (rte_eal_vfio_setup() < 0)
 		rte_panic("Cannot init VFIO\n");
@@ -887,10 +884,6 @@ rte_eal_init(int argc, char **argv)
 	rte_eal_mp_remote_launch(sync_func, NULL, SKIP_MASTER);
 	rte_eal_mp_wait_lcore();
 
-	/* Probe & Initialize PCI devices */
-	if (rte_eal_pci_probe())
-		rte_panic("Cannot probe PCI\n");
-
 	if (rte_eal_bus_probe())
 		rte_panic("Cannot probe devices\n");
 
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index cbd25df..5c66aa7 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -35,6 +35,7 @@
 #include <dirent.h>
 
 #include <rte_log.h>
+#include <rte_bus.h>
 #include <rte_pci.h>
 #include <rte_eal_memconfig.h>
 #include <rte_malloc.h>
@@ -267,7 +268,8 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
 
 /* Scan one pci sysfs entry, and fill the devices list from it. */
 static int
-pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
+pci_scan_one(struct rte_bus *bus, const char *dirname,
+	     const struct rte_pci_addr *addr)
 {
 	char filename[PATH_MAX];
 	unsigned long tmp;
@@ -385,21 +387,23 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 		dev->kdrv = RTE_KDRV_NONE;
 
 	/* device is valid, add in list (sorted) */
-	if (TAILQ_EMPTY(&pci_device_list)) {
-		rte_eal_device_insert(&dev->device);
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
+	if (TAILQ_EMPTY(&bus->device_list)) {
+		rte_eal_bus_add_device(bus, &dev->device);
 	} else {
 		struct rte_pci_device *dev2;
+		struct rte_device *r_dev2;
 		int ret;
 
-		TAILQ_FOREACH(dev2, &pci_device_list, next) {
+		TAILQ_FOREACH(r_dev2, &bus->device_list, next) {
+			dev2 = container_of(r_dev2, struct rte_pci_device,
+					    device);
 			ret = rte_eal_compare_pci_addr(&dev->addr, &dev2->addr);
 			if (ret > 0)
 				continue;
 
 			if (ret < 0) {
-				TAILQ_INSERT_BEFORE(dev2, dev, next);
-				rte_eal_device_insert(&dev->device);
+				rte_eal_bus_insert_device(bus, &dev2->device,
+							  &dev->device);
 			} else { /* already registered */
 				dev2->kdrv = dev->kdrv;
 				dev2->max_vfs = dev->max_vfs;
@@ -409,15 +413,14 @@ pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 			}
 			return 0;
 		}
-		rte_eal_device_insert(&dev->device);
-		TAILQ_INSERT_TAIL(&pci_device_list, dev, next);
+		rte_eal_bus_add_device(bus, &dev->device);
 	}
 
 	return 0;
 }
 
 int
-pci_update_device(const struct rte_pci_addr *addr)
+pci_update_device(struct rte_bus *bus, const struct rte_pci_addr *addr)
 {
 	char filename[PATH_MAX];
 
@@ -425,7 +428,7 @@ pci_update_device(const struct rte_pci_addr *addr)
 		 pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
 		 addr->function);
 
-	return pci_scan_one(filename, addr);
+	return pci_scan_one(bus, filename, addr);
 }
 
 /*
@@ -479,13 +482,22 @@ parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
  * list
  */
 int
-rte_eal_pci_scan(struct rte_bus *bus_p __rte_unused)
+rte_eal_pci_scan(struct rte_bus *bus_p)
 {
 	struct dirent *e;
 	DIR *dir;
 	char dirname[PATH_MAX];
 	struct rte_pci_addr addr;
 
+	if (!bus_p) {
+		RTE_LOG(ERR, EAL, "PCI Bus is not registered\n");
+		return -1;
+	}
+
+	/* for debug purposes, PCI can be disabled */
+	if (internal_config.no_pci)
+		return 0;
+
 	dir = opendir(pci_get_sysfs_path());
 	if (dir == NULL) {
 		RTE_LOG(ERR, EAL, "%s(): opendir failed: %s\n",
@@ -504,7 +516,7 @@ rte_eal_pci_scan(struct rte_bus *bus_p __rte_unused)
 
 		snprintf(dirname, sizeof(dirname), "%s/%s",
 				pci_get_sysfs_path(), e->d_name);
-		if (pci_scan_one(dirname, &addr) < 0)
+		if (pci_scan_one(bus_p, dirname, &addr) < 0)
 			goto error;
 	}
 	closedir(dir);
@@ -750,18 +762,9 @@ rte_eal_pci_ioport_unmap(struct rte_pci_ioport *p)
 	return ret;
 }
 
-/* Init the PCI EAL subsystem */
-int
-rte_eal_pci_init(void)
-{
-	/* for debug purposes, PCI can be disabled */
-	if (internal_config.no_pci)
-		return 0;
-
-	if (rte_eal_pci_scan(NULL) < 0) {
-		RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
-		return -1;
-	}
+struct rte_bus pci_bus = {
+	.scan = rte_eal_pci_scan,
+	.match = rte_eal_pci_match,
+};
 
-	return 0;
-}
+RTE_REGISTER_BUS(pci, pci_bus);
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index c873a7f..6e0ec51 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -6,8 +6,6 @@ DPDK_2.0 {
 	eal_parse_sysfs_value;
 	eal_timer_source;
 	lcore_config;
-	pci_device_list;
-	pci_driver_list;
 	per_lcore__lcore_id;
 	per_lcore__rte_errno;
 	rte_calloc;
@@ -41,7 +39,6 @@ DPDK_2.0 {
 	rte_eal_mp_wait_lcore;
 	rte_eal_parse_devargs_str;
 	rte_eal_pci_dump;
-	rte_eal_pci_probe;
 	rte_eal_pci_probe_one;
 	rte_eal_pci_register;
 	rte_eal_pci_scan;
@@ -191,5 +188,9 @@ DPDK_17.02 {
 	rte_eal_bus_remove_device;
 	rte_eal_bus_remove_driver;
 	rte_eal_bus_unregister;
+	rte_eal_pci_match;
+	rte_eal_pci_probe;
+	rte_eal_pci_remove;
+	rte_eal_pci_scan;
 
 } DPDK_16.11;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 12/12] drivers: update PMDs to use rte_driver probe and remove
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

These callbacks now act as first layer of PCI interfaces from the Bus.
Bus probe would enter the PMDs through the rte_driver->probe/remove
callbacks, falling to rte_xxx_driver->probe/remove (Currently, all the
drivers are rte_pci_driver).

This patch also changes QAT which is the only crypto PMD based on PCI.
All others would be changed in a separate patch focused on VDEV.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 drivers/crypto/qat/rte_qat_cryptodev.c  | 4 ++++
 drivers/net/bnx2x/bnx2x_ethdev.c        | 8 ++++++++
 drivers/net/bnxt/bnxt_ethdev.c          | 4 ++++
 drivers/net/cxgbe/cxgbe_ethdev.c        | 4 ++++
 drivers/net/e1000/em_ethdev.c           | 4 ++++
 drivers/net/e1000/igb_ethdev.c          | 8 ++++++++
 drivers/net/ena/ena_ethdev.c            | 4 ++++
 drivers/net/enic/enic_ethdev.c          | 4 ++++
 drivers/net/fm10k/fm10k_ethdev.c        | 4 ++++
 drivers/net/i40e/i40e_ethdev.c          | 4 ++++
 drivers/net/i40e/i40e_ethdev_vf.c       | 4 ++++
 drivers/net/ixgbe/ixgbe_ethdev.c        | 8 ++++++++
 drivers/net/mlx4/mlx4.c                 | 4 +++-
 drivers/net/mlx5/mlx5.c                 | 1 +
 drivers/net/nfp/nfp_net.c               | 4 ++++
 drivers/net/qede/qede_ethdev.c          | 8 ++++++++
 drivers/net/szedata2/rte_eth_szedata2.c | 4 ++++
 drivers/net/thunderx/nicvf_ethdev.c     | 4 ++++
 drivers/net/virtio/virtio_ethdev.c      | 2 ++
 drivers/net/vmxnet3/vmxnet3_ethdev.c    | 4 ++++
 20 files changed, 90 insertions(+), 1 deletion(-)

diff --git a/drivers/crypto/qat/rte_qat_cryptodev.c b/drivers/crypto/qat/rte_qat_cryptodev.c
index 1e7ee61..bc1a9c6 100644
--- a/drivers/crypto/qat/rte_qat_cryptodev.c
+++ b/drivers/crypto/qat/rte_qat_cryptodev.c
@@ -120,6 +120,10 @@ crypto_qat_dev_init(__attribute__((unused)) struct rte_cryptodev_driver *crypto_
 
 static struct rte_cryptodev_driver rte_qat_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_qat_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
 		.probe = rte_cryptodev_pci_probe,
diff --git a/drivers/net/bnx2x/bnx2x_ethdev.c b/drivers/net/bnx2x/bnx2x_ethdev.c
index 0eae433..9f3b3f2 100644
--- a/drivers/net/bnx2x/bnx2x_ethdev.c
+++ b/drivers/net/bnx2x/bnx2x_ethdev.c
@@ -618,6 +618,10 @@ eth_bnx2xvf_dev_init(struct rte_eth_dev *eth_dev)
 
 static struct eth_driver rte_bnx2x_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_bnx2x_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 		.probe = rte_eth_dev_pci_probe,
@@ -632,6 +636,10 @@ static struct eth_driver rte_bnx2x_pmd = {
  */
 static struct eth_driver rte_bnx2xvf_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_bnx2xvf_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
 		.probe = rte_eth_dev_pci_probe,
diff --git a/drivers/net/bnxt/bnxt_ethdev.c b/drivers/net/bnxt/bnxt_ethdev.c
index 035fe07..c8671c8 100644
--- a/drivers/net/bnxt/bnxt_ethdev.c
+++ b/drivers/net/bnxt/bnxt_ethdev.c
@@ -1160,6 +1160,10 @@ bnxt_dev_uninit(struct rte_eth_dev *eth_dev) {
 
 static struct eth_driver bnxt_rte_pmd = {
 	.pci_drv = {
+		    .driver = {
+			    .probe = rte_eal_pci_probe,
+			    .remove = rte_eal_pci_remove,
+		    },
 		    .id_table = bnxt_pci_id_map,
 		    .drv_flags = RTE_PCI_DRV_NEED_MAPPING |
 			    RTE_PCI_DRV_DETACHABLE | RTE_PCI_DRV_INTR_LSC,
diff --git a/drivers/net/cxgbe/cxgbe_ethdev.c b/drivers/net/cxgbe/cxgbe_ethdev.c
index b7f28eb..67714fa 100644
--- a/drivers/net/cxgbe/cxgbe_ethdev.c
+++ b/drivers/net/cxgbe/cxgbe_ethdev.c
@@ -1039,6 +1039,10 @@ static int eth_cxgbe_dev_init(struct rte_eth_dev *eth_dev)
 
 static struct eth_driver rte_cxgbe_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = cxgb4_pci_tbl,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 		.probe = rte_eth_dev_pci_probe,
diff --git a/drivers/net/e1000/em_ethdev.c b/drivers/net/e1000/em_ethdev.c
index aee3d34..7be5da3 100644
--- a/drivers/net/e1000/em_ethdev.c
+++ b/drivers/net/e1000/em_ethdev.c
@@ -391,6 +391,10 @@ eth_em_dev_uninit(struct rte_eth_dev *eth_dev)
 
 static struct eth_driver rte_em_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_em_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
 			RTE_PCI_DRV_DETACHABLE,
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 2fddf0c..70dd24c 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -1078,6 +1078,10 @@ eth_igbvf_dev_uninit(struct rte_eth_dev *eth_dev)
 
 static struct eth_driver rte_igb_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_igb_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
 			RTE_PCI_DRV_DETACHABLE,
@@ -1094,6 +1098,10 @@ static struct eth_driver rte_igb_pmd = {
  */
 static struct eth_driver rte_igbvf_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_igbvf_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 		.probe = rte_eth_dev_pci_probe,
diff --git a/drivers/net/ena/ena_ethdev.c b/drivers/net/ena/ena_ethdev.c
index ab9a178..54fc8de 100644
--- a/drivers/net/ena/ena_ethdev.c
+++ b/drivers/net/ena/ena_ethdev.c
@@ -1705,6 +1705,10 @@ static uint16_t eth_ena_xmit_pkts(void *tx_queue, struct rte_mbuf **tx_pkts,
 
 static struct eth_driver rte_ena_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_ena_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING,
 		.probe = rte_eth_dev_pci_probe,
diff --git a/drivers/net/enic/enic_ethdev.c b/drivers/net/enic/enic_ethdev.c
index 2b154ec..c2783db 100644
--- a/drivers/net/enic/enic_ethdev.c
+++ b/drivers/net/enic/enic_ethdev.c
@@ -634,6 +634,10 @@ static int eth_enicpmd_dev_init(struct rte_eth_dev *eth_dev)
 
 static struct eth_driver rte_enic_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_enic_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 		.probe = rte_eth_dev_pci_probe,
diff --git a/drivers/net/fm10k/fm10k_ethdev.c b/drivers/net/fm10k/fm10k_ethdev.c
index 923690c..d1a2efa 100644
--- a/drivers/net/fm10k/fm10k_ethdev.c
+++ b/drivers/net/fm10k/fm10k_ethdev.c
@@ -3061,6 +3061,10 @@ static const struct rte_pci_id pci_id_fm10k_map[] = {
 
 static struct eth_driver rte_pmd_fm10k = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_fm10k_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
 			RTE_PCI_DRV_DETACHABLE,
diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index 67778ba..9c5d50f 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -670,6 +670,10 @@ static const struct rte_i40e_xstats_name_off rte_i40e_txq_prio_strings[] = {
 
 static struct eth_driver rte_i40e_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_i40e_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
 			RTE_PCI_DRV_DETACHABLE,
diff --git a/drivers/net/i40e/i40e_ethdev_vf.c b/drivers/net/i40e/i40e_ethdev_vf.c
index aa306d6..10bf6ab 100644
--- a/drivers/net/i40e/i40e_ethdev_vf.c
+++ b/drivers/net/i40e/i40e_ethdev_vf.c
@@ -1527,6 +1527,10 @@ i40evf_dev_uninit(struct rte_eth_dev *eth_dev)
  */
 static struct eth_driver rte_i40evf_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_i40evf_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 		.probe = rte_eth_dev_pci_probe,
diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c b/drivers/net/ixgbe/ixgbe_ethdev.c
index edc9b22..80ee232 100644
--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c
@@ -1564,6 +1564,10 @@ eth_ixgbevf_dev_uninit(struct rte_eth_dev *eth_dev)
 
 static struct eth_driver rte_ixgbe_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_ixgbe_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
 			RTE_PCI_DRV_DETACHABLE,
@@ -1580,6 +1584,10 @@ static struct eth_driver rte_ixgbe_pmd = {
  */
 static struct eth_driver rte_ixgbevf_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_ixgbevf_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 		.probe = rte_eth_dev_pci_probe,
diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
index da61a85..e3dcd41 100644
--- a/drivers/net/mlx4/mlx4.c
+++ b/drivers/net/mlx4/mlx4.c
@@ -5907,7 +5907,9 @@ static const struct rte_pci_id mlx4_pci_id_map[] = {
 static struct eth_driver mlx4_driver = {
 	.pci_drv = {
 		.driver = {
-			.name = MLX4_DRIVER_NAME
+			.name = MLX4_DRIVER_NAME,
+			.probe = rte_eal_pci_probe,
+		},
 		},
 		.id_table = mlx4_pci_id_map,
 		.probe = mlx4_pci_probe,
diff --git a/drivers/net/mlx5/mlx5.c b/drivers/net/mlx5/mlx5.c
index 90cc35e..76dda13 100644
--- a/drivers/net/mlx5/mlx5.c
+++ b/drivers/net/mlx5/mlx5.c
@@ -731,6 +731,7 @@ static struct eth_driver mlx5_driver = {
 	.pci_drv = {
 		.driver = {
 			.name = MLX5_DRIVER_NAME
+			.probe = rte_eal_pci_probe,
 		},
 		.id_table = mlx5_pci_id_map,
 		.probe = mlx5_pci_probe,
diff --git a/drivers/net/nfp/nfp_net.c b/drivers/net/nfp/nfp_net.c
index de80b46..125ba86 100644
--- a/drivers/net/nfp/nfp_net.c
+++ b/drivers/net/nfp/nfp_net.c
@@ -2469,6 +2469,10 @@ static struct rte_pci_id pci_id_nfp_net_map[] = {
 
 static struct eth_driver rte_nfp_net_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_nfp_net_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC |
 			     RTE_PCI_DRV_DETACHABLE,
diff --git a/drivers/net/qede/qede_ethdev.c b/drivers/net/qede/qede_ethdev.c
index d106dd0..31f6733 100644
--- a/drivers/net/qede/qede_ethdev.c
+++ b/drivers/net/qede/qede_ethdev.c
@@ -1642,6 +1642,10 @@ static struct rte_pci_id pci_id_qede_map[] = {
 
 static struct eth_driver rte_qedevf_pmd = {
 	.pci_drv = {
+		    .driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		    },
 		    .id_table = pci_id_qedevf_map,
 		    .drv_flags =
 		    RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
@@ -1655,6 +1659,10 @@ static struct eth_driver rte_qedevf_pmd = {
 
 static struct eth_driver rte_qede_pmd = {
 	.pci_drv = {
+		    .driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		    },
 		    .id_table = pci_id_qede_map,
 		    .drv_flags =
 		    RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
diff --git a/drivers/net/szedata2/rte_eth_szedata2.c b/drivers/net/szedata2/rte_eth_szedata2.c
index f3cd52d..a649e60 100644
--- a/drivers/net/szedata2/rte_eth_szedata2.c
+++ b/drivers/net/szedata2/rte_eth_szedata2.c
@@ -1572,6 +1572,10 @@ static const struct rte_pci_id rte_szedata2_pci_id_table[] = {
 
 static struct eth_driver szedata2_eth_driver = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = rte_szedata2_pci_id_table,
 		.probe = rte_eth_dev_pci_probe,
 		.remove = rte_eth_dev_pci_remove,
diff --git a/drivers/net/thunderx/nicvf_ethdev.c b/drivers/net/thunderx/nicvf_ethdev.c
index 466e49c..72ac748 100644
--- a/drivers/net/thunderx/nicvf_ethdev.c
+++ b/drivers/net/thunderx/nicvf_ethdev.c
@@ -2110,6 +2110,10 @@ static const struct rte_pci_id pci_id_nicvf_map[] = {
 
 static struct eth_driver rte_nicvf_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_nicvf_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_INTR_LSC,
 		.probe = rte_eth_dev_pci_probe,
diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index 079fd6c..4d5d1bb 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1377,6 +1377,8 @@ static struct eth_driver rte_virtio_pmd = {
 	.pci_drv = {
 		.driver = {
 			.name = "net_virtio",
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
 		},
 		.id_table = pci_id_virtio_map,
 		.drv_flags = RTE_PCI_DRV_DETACHABLE,
diff --git a/drivers/net/vmxnet3/vmxnet3_ethdev.c b/drivers/net/vmxnet3/vmxnet3_ethdev.c
index 8bb13e5..57f66cb 100644
--- a/drivers/net/vmxnet3/vmxnet3_ethdev.c
+++ b/drivers/net/vmxnet3/vmxnet3_ethdev.c
@@ -335,6 +335,10 @@ eth_vmxnet3_dev_uninit(struct rte_eth_dev *eth_dev)
 
 static struct eth_driver rte_vmxnet3_pmd = {
 	.pci_drv = {
+		.driver = {
+			.probe = rte_eal_pci_probe,
+			.remove = rte_eal_pci_remove,
+		},
 		.id_table = pci_id_vmxnet3_map,
 		.drv_flags = RTE_PCI_DRV_NEED_MAPPING | RTE_PCI_DRV_DETACHABLE,
 		.probe = rte_eth_dev_pci_probe,
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 10/12] pci: Pass rte_pci_addr to functions instead of separate args
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Ben Walker,
	Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

From: Ben Walker <benjamin.walker@intel.com>

Instead of passing domain, bus, devid, func, just pass
an rte_pci_addr.

Signed-off-by: Ben Walker <benjamin.walker@intel.com>
[Shreyansh: Checkpatch error fix]
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c | 33 ++++++++++++++-------------------
 1 file changed, 14 insertions(+), 19 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index bafb7fb..cbd25df 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -267,8 +267,7 @@ pci_parse_sysfs_resource(const char *filename, struct rte_pci_device *dev)
 
 /* Scan one pci sysfs entry, and fill the devices list from it. */
 static int
-pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
-	     uint8_t devid, uint8_t function)
+pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 {
 	char filename[PATH_MAX];
 	unsigned long tmp;
@@ -281,10 +280,7 @@ pci_scan_one(const char *dirname, uint16_t domain, uint8_t bus,
 		return -1;
 
 	memset(dev, 0, sizeof(*dev));
-	dev->addr.domain = domain;
-	dev->addr.bus = bus;
-	dev->addr.devid = devid;
-	dev->addr.function = function;
+	dev->addr = *addr;
 
 	/* get vendor id */
 	snprintf(filename, sizeof(filename), "%s/vendor", dirname);
@@ -429,16 +425,14 @@ pci_update_device(const struct rte_pci_addr *addr)
 		 pci_get_sysfs_path(), addr->domain, addr->bus, addr->devid,
 		 addr->function);
 
-	return pci_scan_one(filename, addr->domain, addr->bus, addr->devid,
-				addr->function);
+	return pci_scan_one(filename, addr);
 }
 
 /*
  * split up a pci address into its constituent parts.
  */
 static int
-parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
-		uint8_t *bus, uint8_t *devid, uint8_t *function)
+parse_pci_addr_format(const char *buf, int bufsize, struct rte_pci_addr *addr)
 {
 	/* first split on ':' */
 	union splitaddr {
@@ -466,10 +460,10 @@ parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
 
 	/* now convert to int values */
 	errno = 0;
-	*domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
-	*bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
-	*devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
-	*function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
+	addr->domain = (uint16_t)strtoul(splitaddr.domain, NULL, 16);
+	addr->bus = (uint8_t)strtoul(splitaddr.bus, NULL, 16);
+	addr->devid = (uint8_t)strtoul(splitaddr.devid, NULL, 16);
+	addr->function = (uint8_t)strtoul(splitaddr.function, NULL, 10);
 	if (errno != 0)
 		goto error;
 
@@ -490,8 +484,7 @@ rte_eal_pci_scan(struct rte_bus *bus_p __rte_unused)
 	struct dirent *e;
 	DIR *dir;
 	char dirname[PATH_MAX];
-	uint16_t domain;
-	uint8_t bus, devid, function;
+	struct rte_pci_addr addr;
 
 	dir = opendir(pci_get_sysfs_path());
 	if (dir == NULL) {
@@ -500,20 +493,22 @@ rte_eal_pci_scan(struct rte_bus *bus_p __rte_unused)
 		return -1;
 	}
 
+
 	while ((e = readdir(dir)) != NULL) {
 		if (e->d_name[0] == '.')
 			continue;
 
-		if (parse_pci_addr_format(e->d_name, sizeof(e->d_name), &domain,
-				&bus, &devid, &function) != 0)
+		if (parse_pci_addr_format(e->d_name,
+					  sizeof(e->d_name), &addr) != 0)
 			continue;
 
 		snprintf(dirname, sizeof(dirname), "%s/%s",
 				pci_get_sysfs_path(), e->d_name);
-		if (pci_scan_one(dirname, domain, bus, devid, function) < 0)
+		if (pci_scan_one(dirname, &addr) < 0)
 			goto error;
 	}
 	closedir(dir);
+
 	return 0;
 
 error:
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 09/12] eal/pci: generalize args of PCI scan/match towards RTE device/driver
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

PCI scan and match now work on rte_device/rte_driver rather than PCI
specific objects. These functions can now be plugged to the generic
bus callbacks for scanning and matching devices/drivers.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 app/test/test_pci.c                     |  2 +-
 lib/librte_eal/bsdapp/eal/eal_pci.c     |  4 ++--
 lib/librte_eal/common/eal_common_pci.c  | 28 +++++++++++++++++++++-------
 lib/librte_eal/common/include/rte_pci.h | 17 ++++++++++-------
 lib/librte_eal/linuxapp/eal/eal_pci.c   |  4 ++--
 5 files changed, 36 insertions(+), 19 deletions(-)

diff --git a/app/test/test_pci.c b/app/test/test_pci.c
index cda186d..f9b84db 100644
--- a/app/test/test_pci.c
+++ b/app/test/test_pci.c
@@ -180,7 +180,7 @@ test_pci_setup(void)
 		TAILQ_INSERT_TAIL(&real_pci_device_list, dev, next);
 	}
 
-	ret = rte_eal_pci_scan();
+	ret = rte_eal_pci_scan(NULL);
 	TEST_ASSERT_SUCCESS(ret, "failed to scan PCI bus");
 	rte_eal_pci_dump(stdout);
 
diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 8b3ed88..10b234e 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -361,7 +361,7 @@ pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
  * list. Call pci_scan_one() for each pci entry found.
  */
 int
-rte_eal_pci_scan(void)
+rte_eal_pci_scan(struct rte_bus *bus __rte_unused)
 {
 	int fd;
 	unsigned dev_count = 0;
@@ -676,7 +676,7 @@ rte_eal_pci_init(void)
 	if (internal_config.no_pci)
 		return 0;
 
-	if (rte_eal_pci_scan() < 0) {
+	if (rte_eal_pci_scan(NULL) < 0) {
 		RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
 		return -1;
 	}
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 706f91c..562c0fd 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -153,17 +153,22 @@ pci_unmap_resource(void *requested_addr, size_t size)
 }
 
 int
-rte_eal_pci_match(struct rte_pci_driver *pci_drv,
-		  struct rte_pci_device *pci_dev)
+rte_eal_pci_match(struct rte_driver *drv,
+		  struct rte_device *dev)
 {
 	int match = 1;
 	const struct rte_pci_id *id_table;
+	struct rte_pci_driver *pci_drv;
+	struct rte_pci_device *pci_dev;
 
-	if (!pci_drv || !pci_dev || !pci_drv->id_table) {
-		RTE_LOG(DEBUG, EAL, "Invalid PCI Driver object\n");
+	if (!drv || !dev) {
+		RTE_LOG(DEBUG, EAL, "Invalid Device/Driver\n");
 		return -1;
 	}
 
+	pci_drv = container_of(drv, struct rte_pci_driver, driver);
+	pci_dev = container_of(dev, struct rte_pci_device, device);
+
 	for (id_table = pci_drv->id_table; id_table->vendor_id != 0;
 	     id_table++) {
 		/* check if device's identifiers match the driver's ones */
@@ -201,11 +206,15 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
 			     struct rte_pci_device *dev)
 {
 	int ret;
+	struct rte_driver *driver;
+	struct rte_device *device;
 	struct rte_pci_addr *loc;
 
 	if ((dr == NULL) || (dev == NULL))
 		return -EINVAL;
 
+	driver = &dr->driver;
+	device = &dev->device;
 	loc = &dev->addr;
 
 	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
@@ -222,11 +231,11 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
 	}
 
 	/* The device is not blacklisted; Check if driver supports it */
-	ret = rte_eal_pci_match(dr, dev);
+	ret = rte_eal_pci_match(driver, device);
 	if (ret) {
 		/* Match of device and driver failed */
 		RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
-			dr->driver.name);
+			driver->name);
 		return 1;
 	}
 
@@ -268,12 +277,17 @@ rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
 		struct rte_pci_device *dev)
 {
 	int ret;
+	struct rte_driver *driver = NULL;
+	struct rte_device *device;
 	struct rte_pci_addr *loc;
 
 	if ((dr == NULL) || (dev == NULL))
 		return -EINVAL;
 
-	ret = rte_eal_pci_match(dr, dev);
+	driver = &(dr->driver);
+	device = &(dev->device);
+
+	ret = rte_eal_pci_match(driver, device);
 	if (ret) {
 		/* Device and driver don't match */
 		return 1;
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index c9b113d..10108a4 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -363,25 +363,28 @@ rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
  * Scan the content of the PCI bus, and the devices in the devices
  * list
  *
+ * @param bus
+ *	Reference to the PCI bus
+ *
  * @return
  *  0 on success, negative on error
  */
-int rte_eal_pci_scan(void);
+int rte_eal_pci_scan(struct rte_bus *bus);
 
 /**
  * Match the PCI Driver and Device using the ID Table
  *
- * @param pci_drv
- *	PCI driver from which ID table would be extracted
- * @param pci_dev
- *	PCI device to match against the driver
+ * @param drv
+ *	driver from which ID table would be extracted
+ * @param dev
+ *	device to match against the driver
  * @return
  *	0 for successful match
  *	!0 for unsuccessful match
  */
 int
-rte_eal_pci_match(struct rte_pci_driver *pci_drv,
-		  struct rte_pci_device *pci_dev);
+rte_eal_pci_match(struct rte_driver *drv,
+		  struct rte_device *dev);
 
 /**
  * Probe the PCI bus for registered drivers.
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 876ba38..bafb7fb 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -485,7 +485,7 @@ parse_pci_addr_format(const char *buf, int bufsize, uint16_t *domain,
  * list
  */
 int
-rte_eal_pci_scan(void)
+rte_eal_pci_scan(struct rte_bus *bus_p __rte_unused)
 {
 	struct dirent *e;
 	DIR *dir;
@@ -763,7 +763,7 @@ rte_eal_pci_init(void)
 	if (internal_config.no_pci)
 		return 0;
 
-	if (rte_eal_pci_scan() < 0) {
+	if (rte_eal_pci_scan(NULL) < 0) {
 		RTE_LOG(ERR, EAL, "%s(): Cannot scan PCI bus\n", __func__);
 		return -1;
 	}
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 08/12] pci: split match and probe function
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

Matching of PCI device address and driver ID table is being done at two
discreet locations duplicating the code. (rte_eal_pci_probe_one_driver
and rte_eal_pci_detach_dev).

Splitting the matching function into rte_eal_pci_match.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/common/eal_common_pci.c  | 198 ++++++++++++++++++--------------
 lib/librte_eal/common/include/rte_pci.h |  15 +++
 2 files changed, 125 insertions(+), 88 deletions(-)

diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 6bff675..706f91c 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -152,131 +152,153 @@ pci_unmap_resource(void *requested_addr, size_t size)
 				requested_addr);
 }
 
-/*
- * If vendor/device ID match, call the probe() function of the
- * driver.
- */
-static int
-rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *dev)
+int
+rte_eal_pci_match(struct rte_pci_driver *pci_drv,
+		  struct rte_pci_device *pci_dev)
 {
-	int ret;
+	int match = 1;
 	const struct rte_pci_id *id_table;
 
-	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
+	if (!pci_drv || !pci_dev || !pci_drv->id_table) {
+		RTE_LOG(DEBUG, EAL, "Invalid PCI Driver object\n");
+		return -1;
+	}
 
+	for (id_table = pci_drv->id_table; id_table->vendor_id != 0;
+	     id_table++) {
 		/* check if device's identifiers match the driver's ones */
-		if (id_table->vendor_id != dev->id.vendor_id &&
+		if (id_table->vendor_id != pci_dev->id.vendor_id &&
 				id_table->vendor_id != PCI_ANY_ID)
 			continue;
-		if (id_table->device_id != dev->id.device_id &&
+		if (id_table->device_id != pci_dev->id.device_id &&
 				id_table->device_id != PCI_ANY_ID)
 			continue;
-		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
-				id_table->subsystem_vendor_id != PCI_ANY_ID)
+		if (id_table->subsystem_vendor_id !=
+		    pci_dev->id.subsystem_vendor_id &&
+		    id_table->subsystem_vendor_id != PCI_ANY_ID)
 			continue;
-		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
-				id_table->subsystem_device_id != PCI_ANY_ID)
+		if (id_table->subsystem_device_id !=
+		    pci_dev->id.subsystem_device_id &&
+		    id_table->subsystem_device_id != PCI_ANY_ID)
 			continue;
-		if (id_table->class_id != dev->id.class_id &&
+		if (id_table->class_id != pci_dev->id.class_id &&
 				id_table->class_id != RTE_CLASS_ANY_ID)
 			continue;
 
-		struct rte_pci_addr *loc = &dev->addr;
-
-		RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
-				loc->domain, loc->bus, loc->devid, loc->function,
-				dev->device.numa_node);
-
-		/* no initialization when blacklisted, return without error */
-		if (dev->device.devargs != NULL &&
-			dev->device.devargs->type ==
-				RTE_DEVTYPE_BLACKLISTED_PCI) {
-			RTE_LOG(INFO, EAL, "  Device is blacklisted, not initializing\n");
-			return 1;
-		}
-
-		RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
-				dev->id.device_id, dr->driver.name);
-
-		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
-			/* map resources for devices that use igb_uio */
-			ret = rte_eal_pci_map_device(dev);
-			if (ret != 0)
-				return ret;
-		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
-				rte_eal_process_type() == RTE_PROC_PRIMARY) {
-			/* unbind current driver */
-			if (pci_unbind_kernel_driver(dev) < 0)
-				return -1;
-		}
-
-		/* reference driver structure */
-		dev->driver = dr;
-
-		/* call the driver probe() function */
-		ret = dr->probe(dr, dev);
-		if (ret)
-			dev->driver = NULL;
-
-		return ret;
+		match = 0;
+		break;
 	}
-	/* return positive value if driver doesn't support this device */
-	return 1;
+
+	return match;
 }
 
 /*
- * If vendor/device ID match, call the remove() function of the
+ * If vendor/device ID match, call the probe() function of the
  * driver.
  */
 static int
-rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
-		struct rte_pci_device *dev)
+rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr,
+			     struct rte_pci_device *dev)
 {
-	const struct rte_pci_id *id_table;
+	int ret;
+	struct rte_pci_addr *loc;
 
 	if ((dr == NULL) || (dev == NULL))
 		return -EINVAL;
 
-	for (id_table = dr->id_table; id_table->vendor_id != 0; id_table++) {
+	loc = &dev->addr;
 
-		/* check if device's identifiers match the driver's ones */
-		if (id_table->vendor_id != dev->id.vendor_id &&
-				id_table->vendor_id != PCI_ANY_ID)
-			continue;
-		if (id_table->device_id != dev->id.device_id &&
-				id_table->device_id != PCI_ANY_ID)
-			continue;
-		if (id_table->subsystem_vendor_id != dev->id.subsystem_vendor_id &&
-				id_table->subsystem_vendor_id != PCI_ANY_ID)
-			continue;
-		if (id_table->subsystem_device_id != dev->id.subsystem_device_id &&
-				id_table->subsystem_device_id != PCI_ANY_ID)
-			continue;
+	RTE_LOG(INFO, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+			loc->domain, loc->bus, loc->devid, loc->function,
+			dev->device.numa_node);
 
-		struct rte_pci_addr *loc = &dev->addr;
+	/* no initialization when blacklisted, return without error */
+	if (dev->device.devargs != NULL &&
+		dev->device.devargs->type ==
+			RTE_DEVTYPE_BLACKLISTED_PCI) {
+		RTE_LOG(INFO, EAL, "  Device is blacklisted, not"
+			" initializing\n");
+		return 1;
+	}
 
-		RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
-				loc->domain, loc->bus, loc->devid,
-				loc->function, dev->device.numa_node);
+	/* The device is not blacklisted; Check if driver supports it */
+	ret = rte_eal_pci_match(dr, dev);
+	if (ret) {
+		/* Match of device and driver failed */
+		RTE_LOG(DEBUG, EAL, "Driver (%s) doesn't match the device\n",
+			dr->driver.name);
+		return 1;
+	}
 
-		RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
-				dev->id.device_id, dr->driver.name);
+	RTE_LOG(INFO, EAL, "  probe driver: %x:%x %s\n", dev->id.vendor_id,
+			dev->id.device_id, dr->driver.name);
+
+	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING) {
+		/* map resources for devices that use igb_uio */
+		ret = rte_eal_pci_map_device(dev);
+		if (ret != 0)
+			return ret;
+	} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
+			rte_eal_process_type() == RTE_PROC_PRIMARY) {
+		/* unbind current driver */
+		if (pci_unbind_kernel_driver(dev) < 0)
+			return -1;
+	}
 
-		if (dr->remove && (dr->remove(dev) < 0))
-			return -1;	/* negative value is an error */
+	/* reference driver structure */
+	dev->driver = dr;
 
-		/* clear driver structure */
+	/* call the driver probe() function */
+	ret = dr->probe(dr, dev);
+	if (ret) {
+		RTE_LOG(DEBUG, EAL, "Driver (%s) probe failed.\n",
+			dr->driver.name);
 		dev->driver = NULL;
+	}
 
-		if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
-			/* unmap resources for devices that use igb_uio */
-			rte_eal_pci_unmap_device(dev);
+	return ret;
+}
 
-		return 0;
+/*
+ * If vendor/device ID match, call the remove() function of the
+ * driver.
+ */
+static int
+rte_eal_pci_detach_dev(struct rte_pci_driver *dr,
+		struct rte_pci_device *dev)
+{
+	int ret;
+	struct rte_pci_addr *loc;
+
+	if ((dr == NULL) || (dev == NULL))
+		return -EINVAL;
+
+	ret = rte_eal_pci_match(dr, dev);
+	if (ret) {
+		/* Device and driver don't match */
+		return 1;
 	}
 
-	/* return positive value if driver doesn't support this device */
-	return 1;
+	loc = &dev->addr;
+
+	RTE_LOG(DEBUG, EAL, "PCI device "PCI_PRI_FMT" on NUMA socket %i\n",
+			loc->domain, loc->bus, loc->devid,
+			loc->function, dev->device.numa_node);
+
+	RTE_LOG(DEBUG, EAL, "  remove driver: %x:%x %s\n", dev->id.vendor_id,
+			dev->id.device_id, dr->driver.name);
+
+	if (dr->remove && (dr->remove(dev) < 0))
+		return -1;	/* negative value is an error */
+
+	/* clear driver structure */
+	dev->driver = NULL;
+
+	if (dr->drv_flags & RTE_PCI_DRV_NEED_MAPPING)
+		/* unmap resources for devices that use igb_uio */
+		rte_eal_pci_unmap_device(dev);
+
+	return 0;
 }
 
 /*
diff --git a/lib/librte_eal/common/include/rte_pci.h b/lib/librte_eal/common/include/rte_pci.h
index 9ce8847..c9b113d 100644
--- a/lib/librte_eal/common/include/rte_pci.h
+++ b/lib/librte_eal/common/include/rte_pci.h
@@ -369,6 +369,21 @@ rte_eal_compare_pci_addr(const struct rte_pci_addr *addr,
 int rte_eal_pci_scan(void);
 
 /**
+ * Match the PCI Driver and Device using the ID Table
+ *
+ * @param pci_drv
+ *	PCI driver from which ID table would be extracted
+ * @param pci_dev
+ *	PCI device to match against the driver
+ * @return
+ *	0 for successful match
+ *	!0 for unsuccessful match
+ */
+int
+rte_eal_pci_match(struct rte_pci_driver *pci_drv,
+		  struct rte_pci_device *pci_dev);
+
+/**
  * Probe the PCI bus for registered drivers.
  *
  * Scan the content of the PCI bus, and call the probe() function for
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 07/12] eal: enable probe from bus infrastructure
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

The model is:
 rte_eal_init
 `--> calls rte_eal_bus_probe()
      This iterates over all the drivers and devices and matches them. For
      matched bus specific device-driver and calls:
      `-> rte_driver->probe()
          for all matched device/drivers (rte_bus->match() successful)
          This would be responsible for devargs related checks, eventually
          calling:
          `-> rte_xxx_driver->probe()
              which does all the work from eth_dev allocation to init.
              (Currently, eth_dev init is done by eth_driver->eth_dev_init,
                which would be removed soon)

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/common/eal_common_bus.c | 51 +++++++++++++++++++++++++++++++++-
 1 file changed, 50 insertions(+), 1 deletion(-)

diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 5fbfdcc..0dfa800 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -196,11 +196,60 @@ rte_eal_bus_scan(void)
 	return 0;
 }
 
+static int
+perform_probe(struct rte_bus *bus __rte_unused, struct rte_driver *driver,
+	      struct rte_device *device)
+{
+	int ret;
+
+	if (!driver->probe) {
+		RTE_LOG(ERR, EAL, "Driver (%s) doesn't support probe.\n",
+			driver->name);
+		/* This is not an error - just a badly implemented PMD */
+		return 0;
+	}
+
+	ret = driver->probe(driver, device);
+	if (ret < 0)
+		/* One of the probes failed */
+		RTE_LOG(ERR, EAL, "Probe failed for (%s).\n", driver->name);
+
+	/* In either case, ret <0 (error), ret > 0 (not supported) and ret = 0
+	 * success, return ret
+	 */
+	return ret;
+}
+
 /* Match driver<->device and call driver->probe() */
 int
 rte_eal_bus_probe(void)
 {
-	/* Until driver->probe is available, this is dummy implementation */
+	int ret;
+	struct rte_bus *bus;
+	struct rte_device *device;
+	struct rte_driver *driver;
+
+	/* For each bus registered with EAL */
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		TAILQ_FOREACH(device, &bus->device_list, next) {
+			TAILQ_FOREACH(driver, &bus->driver_list, next) {
+				ret = bus->match(driver, device);
+				if (!ret) {
+					ret = perform_probe(bus, driver,
+							    device);
+					if (ret < 0)
+						return ret;
+
+					device->driver = driver;
+					/* ret == 0 is success; ret >0 implies
+					 * driver doesn't support the device.
+					 * in either case, continue
+					 */
+				}
+			}
+		}
+	}
+
 	return 0;
 }
 
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 06/12] eal: add probe and remove support for rte_driver
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

rte_driver now supports probe and remove. These would be used for generic
device type (PCI, etc) probe and remove.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/common/include/rte_dev.h | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index 4004f9a..7d2ab16 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -145,6 +145,16 @@ void rte_eal_device_insert(struct rte_device *dev);
 void rte_eal_device_remove(struct rte_device *dev);
 
 /**
+ * Initialisation function for the driver called during probing.
+ */
+typedef int (driver_probe_t)(struct rte_driver *, struct rte_device *);
+
+/**
+ * Uninitialisation function for the driver called during hotplugging.
+ */
+typedef int (driver_remove_t)(struct rte_device *);
+
+/**
  * A structure describing a device driver.
  */
 struct rte_driver {
@@ -152,6 +162,8 @@ struct rte_driver {
 	struct rte_bus *bus;           /**< Bus serviced by this driver */
 	const char *name;                   /**< Driver name. */
 	const char *alias;              /**< Driver alias. */
+	driver_probe_t *probe;         /**< Probe the device */
+	driver_remove_t *remove;       /**< Remove/hotplugging the device */
 };
 
 /**
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 05/12] eal: integrate bus scan and probe with EAL
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

Still a dummy implementation as no real bus driver exists. This adds calls
from EAL to bus specific scan, match functions.
Once driver->probe is in place, and a bus handler has been installed,
the code would become effective.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 lib/librte_eal/bsdapp/eal/eal.c         |  7 +++++++
 lib/librte_eal/common/eal_common_bus.c  | 30 ++++++++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h | 19 +++++++++++++++++++
 lib/librte_eal/linuxapp/eal/eal.c       |  7 +++++++
 4 files changed, 63 insertions(+)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 35e3117..30afc6b 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -64,6 +64,7 @@
 #include <rte_string_fns.h>
 #include <rte_cpuflags.h>
 #include <rte_interrupts.h>
+#include <rte_bus.h>
 #include <rte_pci.h>
 #include <rte_dev.h>
 #include <rte_devargs.h>
@@ -580,6 +581,9 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_dev_init() < 0)
 		rte_panic("Cannot init pmd devices\n");
 
+	if (rte_eal_bus_scan())
+		rte_panic("Cannot scan the buses for devices\n");
+
 	RTE_LCORE_FOREACH_SLAVE(i) {
 
 		/*
@@ -616,6 +620,9 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_pci_probe())
 		rte_panic("Cannot probe PCI\n");
 
+	if (rte_eal_bus_probe())
+		rte_panic("Cannot probe devices\n");
+
 	rte_eal_mcfg_complete();
 
 	return fctret;
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 162a16f..5fbfdcc 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -174,6 +174,36 @@ rte_eal_bus_unregister(struct rte_bus *bus)
 	RTE_LOG(INFO, EAL, "Unregistered [%s] bus.\n", bus->name);
 }
 
+/* Scan all the buses for registering devices */
+int
+rte_eal_bus_scan(void)
+{
+	int ret;
+	struct rte_bus *bus = NULL;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		ret = bus->scan(bus);
+		if (ret) {
+			RTE_LOG(ERR, EAL, "Scan for (%s) bus failed.\n",
+				bus->name);
+			/* TODO: Should error on a particular bus block scan
+			 * for all others?
+			 */
+			return ret;
+		}
+	}
+
+	return 0;
+}
+
+/* Match driver<->device and call driver->probe() */
+int
+rte_eal_bus_probe(void)
+{
+	/* Until driver->probe is available, this is dummy implementation */
+	return 0;
+}
+
 /* dump one bus info */
 static int
 bus_dump_one(FILE *f, struct rte_bus *bus)
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index d1bd2e8..9bd8650 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -209,6 +209,25 @@ void rte_eal_bus_unregister(struct rte_bus *bus);
  */
 struct rte_bus *rte_eal_get_bus(const char *bus_name);
 
+/** @internal
+ * Scan all the buses attached to the framework.
+ *
+ * @param void
+ * @return void
+ */
+int rte_eal_bus_scan(void);
+
+/** @internal
+ * For each device on the bus, perform a driver 'match' and call the
+ * driver's probe for device initialization.
+ *
+ * @param void
+ * @return
+ *	0 for successful match/probe
+ *	!0 otherwise
+ */
+int rte_eal_bus_probe(void);
+
 /**
  * Dump information of all the buses registered with EAL.
  *
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index 2075282..01d0cee 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -69,6 +69,7 @@
 #include <rte_string_fns.h>
 #include <rte_cpuflags.h>
 #include <rte_interrupts.h>
+#include <rte_bus.h>
 #include <rte_pci.h>
 #include <rte_dev.h>
 #include <rte_devargs.h>
@@ -847,6 +848,9 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_intr_init() < 0)
 		rte_panic("Cannot init interrupt-handling thread\n");
 
+	if (rte_eal_bus_scan())
+		rte_panic("Cannot scan the buses for devices\n");
+
 	RTE_LCORE_FOREACH_SLAVE(i) {
 
 		/*
@@ -887,6 +891,9 @@ rte_eal_init(int argc, char **argv)
 	if (rte_eal_pci_probe())
 		rte_panic("Cannot probe PCI\n");
 
+	if (rte_eal_bus_probe())
+		rte_panic("Cannot probe devices\n");
+
 	rte_eal_mcfg_complete();
 
 	return fctret;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 04/12] eal/bus: add scan, match and insert support
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

When a PMD is registred, it will associate itself with a bus.

A bus is responsible for 'scan' of all the devices attached to it.
All the scanned devices are attached to bus specific device_list.
During the probe operation, 'match' of the drivers and devices would
be done.

Also, rather than adding a device to tail, a new device might be added to
the list (pivoted on bus) at a predefined position, for example, adding it
in order of addressing. Support for this is added as '*bus_insert'.

This patch also adds necessary test framework to test the scan and
match callbacks.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 app/test/test_bus.c                     | 265 ++++++++++++++++++++++++++++++++
 lib/librte_eal/common/eal_common_bus.c  |  15 ++
 lib/librte_eal/common/include/rte_bus.h |  64 ++++++++
 3 files changed, 344 insertions(+)

diff --git a/app/test/test_bus.c b/app/test/test_bus.c
index 760d40a..ed95479 100644
--- a/app/test/test_bus.c
+++ b/app/test/test_bus.c
@@ -80,12 +80,32 @@ struct dummy_bus {
 struct rte_bus_list orig_bus_list =
 	TAILQ_HEAD_INITIALIZER(orig_bus_list);
 
+/* Forward declarations for callbacks from bus */
+
+/* Bus A
+ * Scan would register devA1 and devA2 to bus
+ */
+int scan_fn_for_busA(struct rte_bus *bus);
+
+/* Bus B
+ * Scan would register devB1 and devB2 to bus
+ */
+int scan_fn_for_busB(struct rte_bus *bus);
+
+/* generic implementations wrapped around by above declarations */
+static int generic_scan_fn(struct rte_bus *bus);
+static int generic_match_fn(struct rte_driver *drv, struct rte_device *dev);
+
 struct rte_bus busA = {
 	.name = "busA", /* "busA" */
+	.scan = scan_fn_for_busA,
+	.match = generic_match_fn,
 };
 
 struct rte_bus busB = {
 	.name = "busB", /* "busB */
+	.scan = scan_fn_for_busB,
+	.match = generic_match_fn,
 };
 
 struct rte_driver driverA = {
@@ -184,6 +204,92 @@ dump_device_tree(void)
 	printf("------>8-------\n");
 }
 
+/* @internal
+ * Move over the dummy_buses and find the entry matching the bus object
+ * passed as argument.
+ * For each device in that dummy_buses list, register.
+ *
+ * @param bus
+ *	bus to scan againt test entry
+ * @return
+ *	0 for successful scan, even if no devices are found
+ *	!0 for any error in scanning (like, invalid bus)
+ */
+static int
+generic_scan_fn(struct rte_bus *bus)
+{
+	int i = 0;
+	struct rte_device *dev = NULL;
+	struct dummy_bus *db = NULL;
+
+	if (!bus)
+		return -1;
+
+	/* Extract the device tree node using the bus passed */
+	for (i = 0; dummy_buses[i].name; i++) {
+		if (!strcmp(dummy_buses[i].name, bus->name)) {
+			db = &dummy_buses[i];
+			break;
+		}
+	}
+
+	if (!db)
+		return -1;
+
+	/* For all the devices in the device tree (dummy_buses), add device */
+	for (i = 0; db->devices[i]; i++) {
+		dev = &(db->devices[i]->dev);
+		rte_eal_bus_add_device(bus, dev);
+	}
+
+	return 0;
+}
+
+/* @internal
+ * Obtain bus from driver object. Match the address of rte_device object
+ * with all the devices associated with that bus.
+ *
+ * Being a test function, all this does is validate that device object
+ * provided is available on the same bus to which driver is registered.
+ *
+ * @param drv
+ *	driver to which matching is to be performed
+ * @param dev
+ *	device object to match with driver
+ * @return
+ *	0 for successful match
+ *	!0 for failed match
+ */
+static int
+generic_match_fn(struct rte_driver *drv, struct rte_device *dev)
+{
+	struct rte_bus *bus;
+	struct rte_device *dev_p = NULL;
+
+	/* Match is based entirely on address of 'dev' and 'dev_p' extracted
+	 * from bus->device_list.
+	 */
+
+	/* a driver is registered with the bus *before* the scan. */
+	bus = drv->bus;
+	TAILQ_FOREACH(dev_p, &bus->device_list, next) {
+		if (dev == dev_p)
+			return 0;
+	}
+
+	return 1;
+}
+
+int
+scan_fn_for_busA(struct rte_bus *bus) {
+	return generic_scan_fn(bus);
+}
+
+int
+scan_fn_for_busB(struct rte_bus *bus) {
+	return generic_scan_fn(bus);
+}
+
 static int
 test_bus_setup(void)
 {
@@ -391,6 +497,155 @@ test_driver_unregistration_on_bus(void)
 
 }
 
+static int
+test_device_unregistration_on_bus(void)
+{
+	int i;
+	struct rte_bus *bus = NULL;
+	struct rte_device *dev;
+
+	for (i = 0; dummy_buses[i].name; i++) {
+		bus = rte_eal_get_bus(dummy_buses[i].name);
+		if (!bus) {
+			printf("Unable to find bus (%s)\n",
+			       dummy_buses[i].name);
+			return -1;
+		}
+
+		/* For bus 'bus', unregister all devices */
+		TAILQ_FOREACH(dev, &bus->device_list, next) {
+			rte_eal_bus_remove_device(dev);
+		}
+	}
+
+	for (i = 0; dummy_buses[i].name; i++) {
+		bus = rte_eal_get_bus(dummy_buses[i].name);
+
+		if (!TAILQ_EMPTY(&bus->device_list)) {
+			printf("Unable to remove all devices on bus (%s)\n",
+			       bus->name);
+			return -1;
+		}
+	}
+
+	/* All devices from all buses have been removed */
+	printf("All devices on all buses unregistered.\n");
+	dump_device_tree();
+
+	return 0;
+}
+
+/* @internal
+ * For each bus registered, call the scan function to identify devices
+ * on the bus.
+ *
+ * @param void
+ * @return
+ *	0 for successful scan
+ *	!0 for unsuccessful scan
+ *
+ */
+static int
+test_bus_scan(void)
+{
+	int ret;
+	struct rte_bus *bus;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		/* Call the scan function for each bus */
+		ret = bus->scan(bus);
+		if (ret) {
+			printf("Scan of buses failed.\n");
+			return -1;
+		}
+	}
+
+	printf("Scan of all buses completed.\n");
+	dump_device_tree();
+
+	return 0;
+}
+
+/* @internal
+ * Function to perform 'probe' and link devices and drivers on a bus.
+ * This would work over all the buses registered, and all devices and drivers
+ * registered with it - call match on each pair.
+ * Aim is to test the match_fn for each bus.
+ *
+ * @param void
+ * @return
+ *	0 for successful probe
+ *	!0 for failure in probe
+ *
+ */
+static int
+test_probe_on_bus(void)
+{
+	int ret = 0;
+	int i, j;
+	struct rte_bus *bus = NULL;
+	struct rte_device *dev = NULL;
+	struct rte_driver *drv = NULL;
+
+	/* In case of this test:
+	* 1. for each bus in rte_bus_list
+	* 2.  for each device in bus->device_list
+	* 3.   for each driver in bus->driver_list
+	* 4.    call match
+	* 5.    link driver and device
+	* 6. Verify the linkage.
+	*/
+	for (i = 0; dummy_buses[i].name; i++) {
+		/* get bus pointer from dummy_buses itself rather than
+		 * rte_eal_get_bus
+		 */
+		bus = dummy_buses[i].bus;
+
+		TAILQ_FOREACH(dev, &bus->device_list, next) {
+			TAILQ_FOREACH(drv, &bus->driver_list, next) {
+				if (!bus->match) {
+					printf("Incorrect bus without match "
+					       "fn: (%s).\n", bus->name);
+					return -1;
+				}
+
+				ret = bus->match(drv, dev);
+				if (ret) {
+					printf("Device and driver don't "
+					       "belong to same bus.\n");
+					return -1;
+				}
+				dev->driver = drv;
+
+				/* As match is generic, it always results in
+				 * dev->drv pointing to first driver entry in
+				 * dummy_buses[i]
+				 */
+			}
+		}
+	}
+
+	/* Verify the linkage. All devices belonging to a dummy_buses[i]
+	 * should have same driver (first driver entry of dummy_buses[i])
+	 */
+	for (i = 0; dummy_buses[i].name; i++) {
+		drv = dummy_buses[i].drivers[0];
+
+		for (j = 0; dummy_buses[i].devices[j]; j++) {
+			dev = &(dummy_buses[i].devices[j]->dev);
+			if (dev->driver != drv) {
+				printf("Incorrect driver<->device linkage.\n");
+				return -1;
+			}
+		}
+	}
+
+	printf("Probe on all buses successful.\n");
+	dump_device_tree();
+
+	return 0;
+}
+
 int
 test_bus(void)
 {
@@ -407,6 +662,16 @@ test_bus(void)
 	if (test_driver_registration_on_bus())
 		return -1;
 
+	if (test_bus_scan())
+		return -1;
+
+	/* Now that the devices and drivers are registered, perform probe */
+	if (test_probe_on_bus())
+		return -1;
+
+	if (test_device_unregistration_on_bus())
+		return -1;
+
 	if (test_driver_unregistration_on_bus())
 		return -1;
 
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
index 612f64e..162a16f 100644
--- a/lib/librte_eal/common/eal_common_bus.c
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -58,6 +58,18 @@ rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev)
 	dev->bus = bus;
 }
 
+void
+rte_eal_bus_insert_device(struct rte_bus *bus, struct rte_device *old_dev,
+			  struct rte_device *new_dev)
+{
+	RTE_VERIFY(bus);
+	RTE_VERIFY(old_dev);
+	RTE_VERIFY(new_dev);
+
+	TAILQ_INSERT_BEFORE(old_dev, new_dev, next);
+	new_dev->bus = bus;
+}
+
 /** @internal
  * Remove a device from its bus.
  */
@@ -132,6 +144,9 @@ rte_eal_bus_register(struct rte_bus *bus)
 {
 	RTE_VERIFY(bus);
 	RTE_VERIFY(bus->name && strlen(bus->name));
+	/* A bus should mandatorily have the scan and match implemented */
+	RTE_VERIFY(bus->scan);
+	RTE_VERIFY(bus->match);
 
 	/* Initialize the driver and device list associated with the bus */
 	TAILQ_INIT(&(bus->driver_list));
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
index f0297a9..d1bd2e8 100644
--- a/lib/librte_eal/common/include/rte_bus.h
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -59,6 +59,49 @@ TAILQ_HEAD(rte_bus_list, rte_bus);
 /* Global Bus list */
 extern struct rte_bus_list rte_bus_list;
 
+/**
+ * Bus specific scan for devices attached on the bus.
+ * For each bus object, the scan would be reponsible for finding devices and
+ * adding them to its private device list.
+ *
+ * Successful detection of a device results in rte_device object which is
+ * embedded within the respective device type (rte_pci_device, for example).
+ * Thereafter, PCI specific bus would need to perform
+ * container_of(rte_pci_device) to obtain PCI device object.
+ *
+ * Scan failure of a bus is not treated as exit criteria for application. Scan
+ * for all other buses would still continue.
+ *
+ * A bus should mandatorily implement this method.
+ *
+ * @param bus
+ *	Reference to the bus on which device is added
+ * @return
+ *	0 for successful scan
+ *	!0 (<0) for unsuccessful scan with error value
+ */
+typedef int (*bus_scan_t)(struct rte_bus *bus);
+
+/**
+ * Bus specific match for devices and drivers which can service them.
+ * For each scanned device, rte_driver->probe would be called for driver
+ * specific initialization of the device.
+ *
+ * It is the work of each bus handler to obtain the specific device object
+ * using container_of (or typecasting, as a less preferred way).
+ *
+ * A bus should mandatorily implement this method.
+ *
+ * @param drv
+ *	Driver object attached to the bus
+ * @param dev
+ *	Device object which is being probed.
+ * @return
+ *	0 for successful match
+ *	!0 for unsuccessful match
+ */
+typedef int (*bus_match_t)(struct rte_driver *drv, struct rte_device *dev);
+
 struct rte_bus {
 	TAILQ_ENTRY(rte_bus) next;   /**< Next bus object in linked list */
 	struct rte_driver_list driver_list;
@@ -66,6 +109,9 @@ struct rte_bus {
 	struct rte_device_list device_list;
 				     /**< List of all devices on bus */
 	const char *name;            /**< Name of the bus */
+	bus_scan_t scan;            /**< Scan for devices attached to bus */
+	bus_match_t match;
+	/**< Match device with drivers associated with the bus */
 };
 
 /** @internal
@@ -82,6 +128,24 @@ void
 rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev);
 
 /** @internal
+ * Rather than adding a device to tail, insert at a predefined location.
+ * This is specifically useful for update device cases, or where addition
+ * of devices in the list needs to be ordered (addressing, for example).
+ *
+ * @param bus
+ *	Handle for bus on which device is to be added
+ * @param old_dev
+ *	Existing rte_device object before which new device needs to be added
+ * @param new_dev
+ *	Object for device to be added before old_dev
+ * @return
+ *	void
+ */
+void
+rte_eal_bus_insert_device(struct rte_bus *bus, struct rte_device *old_device,
+			  struct rte_device *new_device);
+
+/** @internal
  * Remove a device from its bus.
  *
  * @param dev
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 03/12] test: add basic bus infrastructure tests
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

Verification of bus registration, driver registration on a bus.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
---
 app/test/Makefile   |   2 +-
 app/test/test.h     |   2 +
 app/test/test_bus.c | 423 ++++++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 426 insertions(+), 1 deletion(-)
 create mode 100644 app/test/test_bus.c

diff --git a/app/test/Makefile b/app/test/Makefile
index 5be023a..ca0f106 100644
--- a/app/test/Makefile
+++ b/app/test/Makefile
@@ -94,7 +94,7 @@ SRCS-y += test_cycles.c
 SRCS-y += test_spinlock.c
 SRCS-y += test_memory.c
 SRCS-y += test_memzone.c
-
+SRCS-y += test_bus.c
 SRCS-y += test_ring.c
 SRCS-y += test_ring_perf.c
 SRCS-y += test_pmd_perf.c
diff --git a/app/test/test.h b/app/test/test.h
index 82831f4..c8ec43f 100644
--- a/app/test/test.h
+++ b/app/test/test.h
@@ -236,6 +236,8 @@ int commands_init(void);
 int test_pci(void);
 int test_pci_run;
 
+int test_bus(void);
+
 int test_mp_secondary(void);
 
 int test_set_rxtx_conf(cmdline_fixed_string_t mode);
diff --git a/app/test/test_bus.c b/app/test/test_bus.c
new file mode 100644
index 0000000..760d40a
--- /dev/null
+++ b/app/test/test_bus.c
@@ -0,0 +1,423 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 NXP.
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of NXP nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <stdint.h>
+#include <sys/queue.h>
+
+#include <rte_bus.h>
+#include <rte_devargs.h>
+
+#include "test.h"
+#include "resource.h"
+
+/* Visualizing following bus-device-driver model for test
+ *
+ *  ===.===========.===========.=========,= busA
+ *     |           |           .         |
+ *   devA1       devA2         .         '____CPU_
+ *    `-----------`-------> driverA      |
+ *                                       '
+ *  ===.===========.===========.=========|= busB
+ *     |           |           .
+ *   devB1       devB2         .
+ *    `-----------`-------> driverB
+ *
+ */
+
+#define MAX_DEVICES_ON_BUS	10
+#define MAX_DRIVERS_ON_BUS	10
+
+/* A structure representing a ethernet/crypto device, embedding
+ * the rte_device.
+ */
+struct dummy_device {
+	const char *name;
+	struct rte_device dev;
+};
+
+/* Structure representing a Bus with devices attached to it, and drivers
+ * for those devices
+ */
+struct dummy_bus {
+	const char *name;
+	struct rte_bus *bus;
+	struct rte_driver *drivers[MAX_DRIVERS_ON_BUS];
+	struct dummy_device *devices[MAX_DEVICES_ON_BUS];
+};
+
+struct rte_bus_list orig_bus_list =
+	TAILQ_HEAD_INITIALIZER(orig_bus_list);
+
+struct rte_bus busA = {
+	.name = "busA", /* "busA" */
+};
+
+struct rte_bus busB = {
+	.name = "busB", /* "busB */
+};
+
+struct rte_driver driverA = {
+	.name = "driverA",
+};
+
+struct dummy_device devA1 = {
+	.name = "devA1",
+	.dev = {
+		.bus = NULL,
+		.driver = NULL,
+	}
+};
+
+struct dummy_device devA2 = {
+	.name = "devA2",
+	.dev = {
+		.bus = NULL,
+		.driver = NULL,
+	}
+};
+
+struct rte_driver driverB = {
+	.name = "driverB",
+};
+
+struct dummy_device devB1 = {
+	.name = "devB1",
+	.dev = {
+		.bus = NULL,
+		.driver = NULL,
+	}
+};
+
+struct dummy_device devB2 = {
+	.name = "devB2",
+	.dev = {
+		.bus = NULL,
+		.driver = NULL,
+	}
+};
+
+struct dummy_bus dummy_buses[] = {
+	{
+		.name = "busA",
+		.bus = &busA,
+		.drivers = {&driverA, NULL},
+		.devices = {&devA1, &devA2, NULL},
+	},
+	{
+		.name = "busB",
+		.bus = &busB,
+		.drivers = {&driverB, NULL},
+		.devices = {&devB1, &devB2, NULL},
+	},
+	{NULL, NULL, {NULL,}, {NULL,}, },
+};
+
+/* @internal
+ * Dump the device tree
+ */
+static void
+dump_device_tree(void)
+{
+	int i;
+	struct dummy_bus *db;
+	struct rte_bus *bus;
+	struct rte_driver *drv;
+	struct rte_device *dev;
+
+	printf("------>8-------\n");
+	printf("Device Tree:\n");
+	for (i = 0; dummy_buses[i].name; i++) {
+		db = &dummy_buses[i];
+
+		bus = rte_eal_get_bus(db->name);
+		if (!bus)
+			return;
+
+		printf(" Bus: %s\n", bus->name);
+
+		printf("  Drivers on bus:\n");
+		TAILQ_FOREACH(drv, &bus->driver_list, next) {
+			printf("    %s\n", drv->name);
+		}
+
+		printf("  Devices on bus:\n");
+		TAILQ_FOREACH(dev, &bus->device_list, next) {
+			printf("    Addr: %p\n", dev);
+			if (dev->driver)
+				printf("    Driver = %s\n", dev->driver->name);
+			else
+				printf("    Driver = None\n");
+		}
+	}
+	printf("------>8-------\n");
+}
+
+static int
+test_bus_setup(void)
+{
+	struct rte_bus *bus_p = NULL;
+
+	/* Preserve the original bus list before executing test */
+	while (!TAILQ_EMPTY(&rte_bus_list)) {
+		bus_p = TAILQ_FIRST(&rte_bus_list);
+		rte_eal_bus_unregister(bus_p);
+		TAILQ_INSERT_TAIL(&orig_bus_list, bus_p, next);
+	}
+
+	return 0;
+}
+
+static int
+test_bus_cleanup(void)
+{
+	struct rte_bus *bus_p = NULL;
+
+	/* Cleanup rte_bus_list before restoring entries */
+	while (!TAILQ_EMPTY(&rte_bus_list)) {
+		bus_p = TAILQ_FIRST(&rte_bus_list);
+		rte_eal_bus_unregister(bus_p);
+	}
+
+	bus_p = NULL;
+	/* Restore original entries */
+	while (!TAILQ_EMPTY(&orig_bus_list)) {
+		bus_p = TAILQ_FIRST(&orig_bus_list);
+		TAILQ_REMOVE(&orig_bus_list, bus_p, next);
+		rte_eal_bus_register(bus_p);
+	}
+
+	dump_device_tree();
+	return 0;
+}
+
+
+static int
+test_bus_registration(void)
+{
+	int i;
+	int ret;
+	struct rte_bus *bus = NULL;
+
+	for (i = 0; dummy_buses[i].name != NULL; i++) {
+		bus = dummy_buses[i].bus;
+		rte_eal_bus_register(bus);
+		printf("Registered Bus %s\n", dummy_buses[i].name);
+	}
+
+	/* Verify that all buses have been successfully registered */
+	i = 0;
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		/* Or, if the name of the bus is NULL */
+		if (!bus->name) {
+			/* Incorrect entry in list */
+			printf("Incorrect bus registered.\n");
+			return -1;
+		}
+
+		/* Or, if the bus name doesn't match that of dummy_buses */
+		ret = strcmp(bus->name, dummy_buses[i].name);
+		if (ret) {
+			/* Bus name doesn't match */
+			printf("Unable to correctly register bus (%s).\n",
+			       dummy_buses[i].name);
+			return -1;
+		}
+		i++;
+	}
+
+	/* Current value of dummy_buses[i] should be the NULL entry */
+	if (dummy_buses[i].name != NULL) {
+		printf("Not all buses were registered. For e.g. (%s)\n",
+		       dummy_buses[i].name);
+		return -1;
+	}
+
+	printf("Buses registered are:\n");
+	rte_eal_bus_dump(stdout);
+
+	return 0;
+}
+
+static int
+test_bus_unregistration(void)
+{
+	int i;
+	struct rte_bus *bus = NULL;
+
+	for (i = 0; dummy_buses[i].name != NULL; i++) {
+		bus = rte_eal_get_bus(dummy_buses[i].name);
+		if (bus) {
+			printf("Unregistering bus: '%s'\n", bus->name);
+			rte_eal_bus_unregister(bus);
+		}
+	}
+
+	if (!TAILQ_EMPTY(&rte_bus_list)) {
+		/* Unable to unregister all dummy buses */
+		printf("Unable to unregister all buses\n");
+		return -1;
+	}
+
+	printf("All buses have been unregistered.\n");
+	dump_device_tree();
+	return 0;
+}
+
+/* Positive case: For each driver in dummy_buses, perform
+ * registration
+ */
+static int
+test_driver_registration_on_bus(void)
+{
+	int i, j;
+	struct rte_bus *bus = NULL;
+	struct rte_driver *drv, *drv2;
+
+	/* For each bus on the dummy_buses list:
+	 * 1. get the bus reference
+	 * 2. register all drivers from dummy_buses
+	 */
+	for (i = 0; dummy_buses[i].name; i++) {
+		bus = rte_eal_get_bus(dummy_buses[i].name);
+		if (!bus) {
+			printf("Unable to find bus (%s)\n",
+			       dummy_buses[i].name);
+			return -1;
+		}
+
+		/* For bus 'bus', register all drivers */
+		for (j = 0; dummy_buses[i].drivers[j]; j++) {
+			drv = dummy_buses[i].drivers[j];
+			rte_eal_bus_add_driver(bus, drv);
+		}
+	}
+
+	/* Drivers have been registered. Verify by parsing the list */
+	drv = NULL;
+	for (i = 0; dummy_buses[i].name; i++) {
+		bus = rte_eal_get_bus(dummy_buses[i].name);
+		if (!bus) {
+			printf("Unable to find bus (%s)\n",
+			       dummy_buses[i].name);
+			return -1;
+		}
+
+		j = 0;
+		TAILQ_FOREACH(drv, &bus->driver_list, next) {
+			drv2 = dummy_buses[i].drivers[j++];
+			if (strcmp(drv2->name, drv->name)) {
+				printf("Incorrectly registered drivers."
+				       " Expected: %s; Available: %s\n",
+				       drv2->name, drv->name);
+				return -1;
+			}
+		}
+	}
+
+	printf("Driver registration test successful.\n");
+	dump_device_tree();
+
+	return 0;
+}
+
+static int
+test_driver_unregistration_on_bus(void)
+{
+	int i;
+	struct rte_bus *bus = NULL;
+	struct rte_driver *drv;
+
+	for (i = 0; dummy_buses[i].name; i++) {
+		bus = rte_eal_get_bus(dummy_buses[i].name);
+		if (!bus) {
+			printf("Unable to find bus (%s)\n",
+			       dummy_buses[i].name);
+			return -1;
+		}
+
+		/* For bus 'bus', unregister all drivers */
+		TAILQ_FOREACH(drv, &bus->driver_list, next) {
+			rte_eal_bus_remove_driver(drv);
+		}
+	}
+
+	/* Verifying that all drivers have been removed */
+	for (i = 0; dummy_buses[i].name; i++) {
+		bus = rte_eal_get_bus(dummy_buses[i].name);
+
+		if (!TAILQ_EMPTY(&bus->driver_list)) {
+			printf("Unable to remove all drivers on bus (%s)\n",
+			       bus->name);
+			return -1;
+		}
+	}
+
+	printf("Unregistration of drivers on all buses is successful.\n");
+	/* All devices from all buses have been removed */
+	dump_device_tree();
+	return 0;
+
+}
+
+int
+test_bus(void)
+{
+	/* Make necessary arrangements before starting test */
+	if (test_bus_setup())
+		return -1;
+
+	if (test_bus_registration())
+		return -1;
+
+	/* Assuming that buses are already registered, register drivers
+	 * with them.
+	 */
+	if (test_driver_registration_on_bus())
+		return -1;
+
+	if (test_driver_unregistration_on_bus())
+		return -1;
+
+	if (test_bus_unregistration())
+		return -1;
+
+	/* Restore the original environment/settings */
+	if (test_bus_cleanup())
+		return -1;
+
+	return 0;
+}
+
+REGISTER_TEST_COMMAND(bus_autotest, test_bus);
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 02/12] eal/bus: introduce bus abstraction
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

This patch introduces the rte_bus abstraction for devices and drivers in
EAL framework. The model is:
 - One or more buses are connected to a CPU (or core)
 - One or more devices are conneted to a Bus
 - Drivers are running instances which manage one or more devices
 - Bus is responsible for identifying devices (and interrupt propogation)
 - Driver is responsible for initializing the device

This patch adds a 'rte_bus' class which rte_driver and rte_device refer.
This way, each device (rte_xxx_device) would have reference to the bus
it is based on. As well as, each driver (rte_xxx_driver) would have link
to the bus and devices on it for servicing.

                                  __ rte_bus_list
                                 /
                     +----------'---+
                     |rte_bus       |
                     | driver_list------> List of rte_bus specific
                     | device_list----    devices
                     |              | `-> List of rte_bus associated
                     |              |     drivers
                     +--|------|----+
              _________/        \_________
    +--------/----+                     +-\---------------+
    |rte_device   |                     |rte_driver       |
    | rte_bus     |                     | rte_bus         |
    | rte_driver  |                     | ...             |
    | ...         |                     +---------...-----+
    |             |                               |||
    +---||--------+                               |||
        ||                                        |||
        | \                                        \\\
        |  \_____________                           \\\
        |                \                          |||
 +------|---------+ +----|----------+               |||
 |rte_pci_device  | |rte_xxx_device |               |||
 | ....           | | ....          |               |||
 +----------------+ +---------------+              / | \
                                                  /  |  \
                            _____________________/  /    \
                           /                    ___/      \
            +-------------'--+    +------------'---+    +--'------------+
            |rte_pci_driver  |    |rte_vdev_driver |    |rte_xxx_driver |
            | ....           |    | ....           |    | ....          |
            +----------------+    +----------------+    +---------------+

This patch only enables the bus references on rte_driver and rte_driver.
EAL wide global device and driver list continue to exist until an instance
of bus is added in subsequent patches.

This patch also introduces RTE_REGISTER_BUS macro on the lines of
RTE_PMD_REGISTER_XXX. Key difference is that the constructor priority has
been explicitly set to 101 so as to execute bus registration before PMD.

Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>

--
v2:
 - fix bsdapp compilation issue because of missing export symbols in map
   file
---
 lib/librte_eal/bsdapp/eal/Makefile              |   1 +
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  15 ++
 lib/librte_eal/common/Makefile                  |   2 +-
 lib/librte_eal/common/eal_common_bus.c          | 192 ++++++++++++++++++++++++
 lib/librte_eal/common/include/rte_bus.h         | 174 +++++++++++++++++++++
 lib/librte_eal/common/include/rte_dev.h         |   2 +
 lib/librte_eal/linuxapp/eal/Makefile            |   1 +
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  15 ++
 8 files changed, 401 insertions(+), 1 deletion(-)
 create mode 100644 lib/librte_eal/common/eal_common_bus.c
 create mode 100644 lib/librte_eal/common/include/rte_bus.h

diff --git a/lib/librte_eal/bsdapp/eal/Makefile b/lib/librte_eal/bsdapp/eal/Makefile
index a15b762..cce99f7 100644
--- a/lib/librte_eal/bsdapp/eal/Makefile
+++ b/lib/librte_eal/bsdapp/eal/Makefile
@@ -78,6 +78,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_cpuflags.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_string_fns.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_hexdump.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_devargs.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_bus.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_dev.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_options.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_BSDAPP) += eal_common_thread.c
diff --git a/lib/librte_eal/bsdapp/eal/rte_eal_version.map b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
index 2f81f7c..23fc1c1 100644
--- a/lib/librte_eal/bsdapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/bsdapp/eal/rte_eal_version.map
@@ -174,3 +174,18 @@ DPDK_16.11 {
 	rte_eal_vdrv_unregister;
 
 } DPDK_16.07;
+
+DPDK_17.02 {
+	global:
+
+	rte_bus_list;
+	rte_eal_bus_add_device;
+	rte_eal_bus_add_driver;
+	rte_eal_get_bus;
+	rte_eal_bus_dump;
+	rte_eal_bus_register;
+	rte_eal_bus_remove_device;
+	rte_eal_bus_remove_driver;
+	rte_eal_bus_unregister;
+
+} DPDK_16.11;
diff --git a/lib/librte_eal/common/Makefile b/lib/librte_eal/common/Makefile
index a92c984..0c39414 100644
--- a/lib/librte_eal/common/Makefile
+++ b/lib/librte_eal/common/Makefile
@@ -38,7 +38,7 @@ INC += rte_per_lcore.h rte_random.h
 INC += rte_tailq.h rte_interrupts.h rte_alarm.h
 INC += rte_string_fns.h rte_version.h
 INC += rte_eal_memconfig.h rte_malloc_heap.h
-INC += rte_hexdump.h rte_devargs.h rte_dev.h rte_vdev.h
+INC += rte_hexdump.h rte_devargs.h rte_bus.h rte_dev.h rte_vdev.h
 INC += rte_pci_dev_feature_defs.h rte_pci_dev_features.h
 INC += rte_malloc.h rte_keepalive.h rte_time.h
 
diff --git a/lib/librte_eal/common/eal_common_bus.c b/lib/librte_eal/common/eal_common_bus.c
new file mode 100644
index 0000000..612f64e
--- /dev/null
+++ b/lib/librte_eal/common/eal_common_bus.c
@@ -0,0 +1,192 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 NXP
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of NXP nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#include <stdio.h>
+#include <string.h>
+#include <inttypes.h>
+#include <sys/queue.h>
+
+#include <rte_bus.h>
+#include <rte_devargs.h>
+#include <rte_debug.h>
+
+#include "eal_private.h"
+
+struct rte_bus_list rte_bus_list =
+	TAILQ_HEAD_INITIALIZER(rte_bus_list);
+
+/** @internal
+ * Add a device to a bus.
+ */
+void
+rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev)
+{
+	RTE_VERIFY(bus);
+	RTE_VERIFY(dev);
+
+	TAILQ_INSERT_TAIL(&bus->device_list, dev, next);
+	dev->bus = bus;
+}
+
+/** @internal
+ * Remove a device from its bus.
+ */
+void
+rte_eal_bus_remove_device(struct rte_device *dev)
+{
+	struct rte_bus *bus;
+
+	RTE_VERIFY(dev);
+	RTE_VERIFY(dev->bus);
+
+	bus = dev->bus;
+	TAILQ_REMOVE(&bus->device_list, dev, next);
+	dev->bus = NULL;
+}
+
+/** @internal
+ * Associate a driver with a bus.
+ */
+void
+rte_eal_bus_add_driver(struct rte_bus *bus, struct rte_driver *drv)
+{
+	RTE_VERIFY(bus);
+	RTE_VERIFY(drv);
+
+	TAILQ_INSERT_TAIL(&bus->driver_list, drv, next);
+	drv->bus = bus;
+}
+
+/** @internal
+ * Disassociate a driver from bus.
+ */
+void
+rte_eal_bus_remove_driver(struct rte_driver *drv)
+{
+	struct rte_bus *bus;
+
+	RTE_VERIFY(drv);
+	RTE_VERIFY(drv->bus);
+
+	bus = drv->bus;
+	TAILQ_REMOVE(&bus->driver_list, drv, next);
+	drv->bus = NULL;
+}
+
+/**
+ * Get the bus handle using its name
+ */
+struct rte_bus *
+rte_eal_get_bus(const char *bus_name)
+{
+	struct rte_bus *bus;
+
+	RTE_VERIFY(bus_name);
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		RTE_VERIFY(bus->name);
+
+		if (!strcmp(bus_name, bus->name)) {
+			RTE_LOG(DEBUG, EAL, "Returning Bus object %p\n", bus);
+			return bus;
+		}
+	}
+
+	/* Unable to find bus requested */
+	return NULL;
+}
+
+/* register a bus */
+void
+rte_eal_bus_register(struct rte_bus *bus)
+{
+	RTE_VERIFY(bus);
+	RTE_VERIFY(bus->name && strlen(bus->name));
+
+	/* Initialize the driver and device list associated with the bus */
+	TAILQ_INIT(&(bus->driver_list));
+	TAILQ_INIT(&(bus->device_list));
+
+	TAILQ_INSERT_TAIL(&rte_bus_list, bus, next);
+	RTE_LOG(INFO, EAL, "Registered [%s] bus.\n", bus->name);
+}
+
+/* unregister a bus */
+void
+rte_eal_bus_unregister(struct rte_bus *bus)
+{
+	/* All devices and drivers associated with the bus should have been
+	 * 'device->uninit' and 'driver->remove()' already.
+	 */
+	RTE_VERIFY(TAILQ_EMPTY(&(bus->driver_list)));
+	RTE_VERIFY(TAILQ_EMPTY(&(bus->device_list)));
+
+	/* TODO: For each device, call its rte_device->driver->remove()
+	 * and rte_eal_bus_remove_driver()
+	 */
+
+	TAILQ_REMOVE(&rte_bus_list, bus, next);
+	RTE_LOG(INFO, EAL, "Unregistered [%s] bus.\n", bus->name);
+}
+
+/* dump one bus info */
+static int
+bus_dump_one(FILE *f, struct rte_bus *bus)
+{
+	int ret;
+
+	/* For now, dump only the bus name */
+	ret = fprintf(f, " %s\n", bus->name);
+
+	/* Error in case of inability in writing to stream */
+	if (ret < 0)
+		return ret;
+
+	return 0;
+}
+
+void
+rte_eal_bus_dump(FILE *f)
+{
+	int ret;
+	struct rte_bus *bus;
+
+	TAILQ_FOREACH(bus, &rte_bus_list, next) {
+		ret = bus_dump_one(f, bus);
+		if (ret) {
+			RTE_LOG(ERR, EAL, "Unable to write to stream (%d)\n",
+				ret);
+			break;
+		}
+	}
+}
diff --git a/lib/librte_eal/common/include/rte_bus.h b/lib/librte_eal/common/include/rte_bus.h
new file mode 100644
index 0000000..f0297a9
--- /dev/null
+++ b/lib/librte_eal/common/include/rte_bus.h
@@ -0,0 +1,174 @@
+/*-
+ *   BSD LICENSE
+ *
+ *   Copyright(c) 2016 NXP
+ *   All rights reserved.
+ *
+ *   Redistribution and use in source and binary forms, with or without
+ *   modification, are permitted provided that the following conditions
+ *   are met:
+ *
+ *     * Redistributions of source code must retain the above copyright
+ *       notice, this list of conditions and the following disclaimer.
+ *     * Redistributions in binary form must reproduce the above copyright
+ *       notice, this list of conditions and the following disclaimer in
+ *       the documentation and/or other materials provided with the
+ *       distribution.
+ *     * Neither the name of NXP nor the names of its
+ *       contributors may be used to endorse or promote products derived
+ *       from this software without specific prior written permission.
+ *
+ *   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+ *   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+ *   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+ *   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+ *   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+ *   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+ *   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+ *   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+ *   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ *   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ *   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef _RTE_BUS_H_
+#define _RTE_BUS_H_
+
+/**
+ * @file
+ *
+ * RTE PMD Bus Abstraction interfaces
+ *
+ * This file exposes APIs and Interfaces for Bus Abstraction over the devices
+ * drivers in EAL.
+ */
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include <stdio.h>
+#include <sys/queue.h>
+
+#include <rte_log.h>
+#include <rte_dev.h>
+
+/** Double linked list of buses */
+TAILQ_HEAD(rte_bus_list, rte_bus);
+
+/* Global Bus list */
+extern struct rte_bus_list rte_bus_list;
+
+struct rte_bus {
+	TAILQ_ENTRY(rte_bus) next;   /**< Next bus object in linked list */
+	struct rte_driver_list driver_list;
+				     /**< List of all drivers on bus */
+	struct rte_device_list device_list;
+				     /**< List of all devices on bus */
+	const char *name;            /**< Name of the bus */
+};
+
+/** @internal
+ * Add a device to a bus.
+ *
+ * @param bus
+ *	Bus on which device is to be added
+ * @param dev
+ *	Device handle
+ * @return
+ *	None
+ */
+void
+rte_eal_bus_add_device(struct rte_bus *bus, struct rte_device *dev);
+
+/** @internal
+ * Remove a device from its bus.
+ *
+ * @param dev
+ *	Device handle to remove
+ * @return
+ *	None
+ */
+void
+rte_eal_bus_remove_device(struct rte_device *dev);
+
+/** @internal
+ * Associate a driver with a bus.
+ *
+ * @param bus
+ *	Bus on which driver is to be added
+ * @param dev
+ *	Driver handle
+ * @return
+ *	None
+ */
+void
+rte_eal_bus_add_driver(struct rte_bus *bus, struct rte_driver *drv);
+
+/** @internal
+ * Disassociate a driver from its bus.
+ *
+ * @param dev
+ *	Driver handle to remove
+ * @return
+ *	None
+ */
+void
+rte_eal_bus_remove_driver(struct rte_driver *drv);
+
+/**
+ * Register a Bus handler.
+ *
+ * @param driver
+ *   A pointer to a rte_bus structure describing the bus
+ *   to be registered.
+ */
+void rte_eal_bus_register(struct rte_bus *bus);
+
+/**
+ * Unregister a Bus handler.
+ *
+ * @param driver
+ *   A pointer to a rte_bus structure describing the bus
+ *   to be unregistered.
+ */
+void rte_eal_bus_unregister(struct rte_bus *bus);
+
+/**
+ * Obtain handle for bus given its name.
+ *
+ * @param bus_name
+ *	Name of the bus handle to search
+ * @return
+ *	Pointer to Bus object if name matches any registered bus object
+ *	NULL, if no matching bus found
+ */
+struct rte_bus *rte_eal_get_bus(const char *bus_name);
+
+/**
+ * Dump information of all the buses registered with EAL.
+ *
+ * @param f
+ *	A valid and open output stream handle
+ *
+ * @return
+ *	 0 in case of success
+ *	!0 in case there is error in opening the output stream
+ */
+void rte_eal_bus_dump(FILE *f);
+
+/** Helper for Bus registration. The constructor has higher priority than
+ * PMD constructors
+ */
+#define RTE_REGISTER_BUS(nm, bus) \
+static void __attribute__((constructor(101), used)) businitfn_ ##nm(void) \
+{\
+	(bus).name = RTE_STR(nm);\
+	rte_eal_bus_register(&bus); \
+}
+
+#ifdef __cplusplus
+}
+#endif
+
+#endif /* _RTE_BUS_H */
diff --git a/lib/librte_eal/common/include/rte_dev.h b/lib/librte_eal/common/include/rte_dev.h
index 8840380..4004f9a 100644
--- a/lib/librte_eal/common/include/rte_dev.h
+++ b/lib/librte_eal/common/include/rte_dev.h
@@ -122,6 +122,7 @@ struct rte_driver;
  */
 struct rte_device {
 	TAILQ_ENTRY(rte_device) next; /**< Next device */
+	struct rte_bus *bus;          /**< Device connected to this bus */
 	struct rte_driver *driver;    /**< Associated driver */
 	int numa_node;                /**< NUMA node connection */
 	struct rte_devargs *devargs;  /**< Device user arguments */
@@ -148,6 +149,7 @@ void rte_eal_device_remove(struct rte_device *dev);
  */
 struct rte_driver {
 	TAILQ_ENTRY(rte_driver) next;  /**< Next in list. */
+	struct rte_bus *bus;           /**< Bus serviced by this driver */
 	const char *name;                   /**< Driver name. */
 	const char *alias;              /**< Driver alias. */
 };
diff --git a/lib/librte_eal/linuxapp/eal/Makefile b/lib/librte_eal/linuxapp/eal/Makefile
index 4e206f0..aa874a5 100644
--- a/lib/librte_eal/linuxapp/eal/Makefile
+++ b/lib/librte_eal/linuxapp/eal/Makefile
@@ -87,6 +87,7 @@ SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_cpuflags.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_string_fns.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_hexdump.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_devargs.c
+SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_bus.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_dev.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_options.c
 SRCS-$(CONFIG_RTE_EXEC_ENV_LINUXAPP) += eal_common_thread.c
diff --git a/lib/librte_eal/linuxapp/eal/rte_eal_version.map b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
index 83721ba..c873a7f 100644
--- a/lib/librte_eal/linuxapp/eal/rte_eal_version.map
+++ b/lib/librte_eal/linuxapp/eal/rte_eal_version.map
@@ -178,3 +178,18 @@ DPDK_16.11 {
 	rte_eal_vdrv_unregister;
 
 } DPDK_16.07;
+
+DPDK_17.02 {
+	global:
+
+	rte_bus_list;
+	rte_eal_bus_add_device;
+	rte_eal_bus_add_driver;
+	rte_eal_get_bus;
+	rte_eal_bus_dump;
+	rte_eal_bus_register;
+	rte_eal_bus_remove_device;
+	rte_eal_bus_remove_driver;
+	rte_eal_bus_unregister;
+
+} DPDK_16.11;
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 01/12] eal: define container_of macro
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Jan Blunck,
	Jan Viktorin, Shreyansh Jain
In-Reply-To: <1481893853-31790-1-git-send-email-shreyansh.jain@nxp.com>

From: Jan Blunck <jblunck@infradead.org>

This macro is based on Jan Viktorin's original patch but also checks the
type of the passed pointer against the type of the member.

Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
[jblunck@infradead.org: add type checking and __extension__]
Signed-off-by: Jan Blunck <jblunck@infradead.org>
---
 lib/librte_eal/common/include/rte_common.h | 20 ++++++++++++++++++++
 1 file changed, 20 insertions(+)

diff --git a/lib/librte_eal/common/include/rte_common.h b/lib/librte_eal/common/include/rte_common.h
index db5ac91..8dda3e2 100644
--- a/lib/librte_eal/common/include/rte_common.h
+++ b/lib/librte_eal/common/include/rte_common.h
@@ -331,6 +331,26 @@ rte_bsf32(uint32_t v)
 #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
 #endif
 
+/**
+ * Return pointer to the wrapping struct instance.
+ *
+ * Example:
+ *
+ *  struct wrapper {
+ *      ...
+ *      struct child c;
+ *      ...
+ *  };
+ *
+ *  struct child *x = obtain(...);
+ *  struct wrapper *w = container_of(x, struct wrapper, c);
+ */
+#ifndef container_of
+#define container_of(ptr, type, member)	__extension__ ({		\
+			typeof(((type *)0)->member) *_ptr = (ptr);	\
+			(type *)(((char *)_ptr) - offsetof(type, member)); })
+#endif
+
 #define _RTE_STR(x) #x
 /** Take a macro value and get a string version of it */
 #define RTE_STR(x) _RTE_STR(x)
-- 
2.7.4

^ permalink raw reply related

* [PATCH v3 00/12] Introducing EAL Bus-Device-Driver Model
From: Shreyansh Jain @ 2016-12-16 13:10 UTC (permalink / raw)
  To: dev, david.marchand
  Cc: thomas.monjalon, ferruh.yigit, jianbo.liu, Shreyansh Jain
In-Reply-To: <1481636232-2300-1-git-send-email-shreyansh.jain@nxp.com>

Link to v1: [10]
Link to v2: [11]

:: Introduction ::

DPDK has been inherently a PCI inclined framework. Because of this, the
design of device tree (or list) within DPDK is also PCI inclined. A
non-PCI device doesn't have a way of being expressed without using hooks
started from EAL to PMD.

(Check 'Version Changes' section for changes)

:: Overview of the Proposed Changes ::

Assuming the below graph for a computing node:

        device A1
          |
  +==.===='==============.============+ Bus A.
     |                    `--> driver A11     \
  device A2                `-> driver A12      \______
                                                |CPU |
                                                /`````
        device B1                              /
          |                                   /
  +==.===='==============.============+ Bus B`
     |                    `--> driver B11
  device B2                `-> driver B12


 - One or more buses are connected to a CPU (or core)
 - One or more devices are conneted to a Bus
 - Drivers are running instances which manage one or more devices
 - Bus is responsible for identifying devices (and interrupt propogation)
 - Driver is responsible for initializing the device

In context of DPDK EAL:
 - rte_bus, represents a Bus. An implementation of a physical bus would
   instantiate this class.
 - Buses are registered just like a PMD - RTE_REGISTER_BUS()
   `- Thus, implementation for PCI would instantiate a rte_bus, give it a
      name and provide scan/match hooks.
    - Currently, priority of RTE_REGISTER_BUS constructor has been set to
      101 to make sure bus is registered *before* drivers are.
 - Each registered bus is part of a doubly list.
   -- Each device refers to rte_bus on which it belongs
   -- Each driver refers to rte_bus with which it is associated
   -- Device and Drivers lists are part of rte_bus
   -- NO global device/driver list would exist
 - When a PMD wants to register itself, it would 'add' itself to an
   existing bus. Which essentially converts to adding the driver to
   a bus specific driver_list.
 - Bus would perform a scan and 'add' devices scanned to its list.
 - Bus would perform a probe and link devices and drivers on each bus and
   invoking a series of probes
   `-- There are some parallel work for combining scan/probe in EAL [5]
       and also for doing away with a independent scan function all
       together [6].


The view would be almost like:

                                  __ rte_bus_list
                                 /
                     +----------'---+
                     |rte_bus       |
                     | driver_list------> device_list for this bus
                     | device_list----    
                     | scan()       | `-> driver_list for this bus
                     | match()      |
                     | probe()      |
                     |              |
                     +--|------|----+
              _________/        \_________
    +--------/----+                     +-\---------------+
    |rte_device   |                     |rte_driver       |
    | *rte_bus    |                     | *rte_bus        |
    | rte_driver  |                     | probe()         |
    |             |                     | remove()        |
    |  devargs    |                     |                 |
    +---||--------+                     +---------|||-----+
        ||                                        '''      
        | \                                        \\\
        |  \_____________                           \\\
        |                \                          |||
 +------|---------+ +----|----------+               |||
 |rte_pci_device  | |rte_xxx_device |               |||
 | PCI specific   | | xxx device    |               |||
 | info (mem,)    | | specific fns  |              / | \
 +----------------+ +---------------+             /  |  \
                            _____________________/  /    \
                           /                    ___/      \
            +-------------'--+    +------------'---+    +--'------------+
            |rte_pci_driver  |    |rte_vdev_driver |    |rte_xxx_driver |
            | PCI id table,  |    | <probably,     |    | ....          |
            | other driver   |    |  nothing>      |    +---------------+
            | data           |    |  ...           |
            |  probe()       |    +----------------+
            |  remove()      |
            +----------------+

In continuation to the RFC posted on 17/Nov [9],
A series of patches is being posted which attempts to create:
 1. A basic bus model
    `- define rte_bus and associated methods/helpers
    `- test infrastructure to test the Bus infra
 2. Changes in EAL to support PCI as a bus
    `- a "pci" bus is registered
    `- existing scan/match/probe are modified to allow for bus integration
    `- PCI Device and Driver list, which were global entities, have been
       moved to rte_bus->[device/driver]_list

For v2 as well, I have sanity tested this patch over a XeonD X552 available
with me, as well as part of PoC for verifying NXP's DPAA2 PMD (being pushed
out in a separate series). Exhaustive testing is still pending.
 -> Please help in MLX & BSD related changes.

:: Brief about Patch Layout ::

0001:      Container_of patch from [3]
0002~0003: Introducing the basic Bus model and associated test case
0004~0005: Add scan, match and insert support for devices on bus
0006:      Add probe and remove for rte_driver
0007:      Enable probing of PCI Bus (devices) from EAL
0008:      Split the existing PCI probe into match and probe
0009:      Make PCI probe/match work on rte_driver/device rather than
           rte_pci_device/rte_pci_driver
0010:      Patch from Ben [8], part of series [2]
0011:      Enable Scan/Match/probe on Bus from EAL and remove unused
           functions and lists. PMDs still don't work (in fact, PCI PMD
           don't work after this patch - but without any compilation
           issues). Also, fix PCI test framework to reflect the bus
           integration.
0012:      Change PMDs to integrate with PCI bus

:: Pending Changes/Caveats ::

0. eth_dev still contains rte_pci_device. I am banking on Jan's patches [1]
   for conversion to a macro (ETH_DEV_PCI_DEV) and subsequent replacement
   of all pci_dev usage in eth_dev.

1. One of the major changes pending, as against proposed in RFC, is the
   removal of eth_driver.
   Being a large change, and independent one, this would be done in a
   separate series of patches.

2. This patchset only moves the PCI into a bus. And, that movement is also
   currently part of the EAL (lib/librte_eal/linux)
   - there was an open question in RFC about where to place the PCI bus
     instance - whether in drivers/bus/... or in lib/librte_bus/... or
     lib/librte_eal/...; This patch uses the last option. But, movement
     only impacts placement of Makefiles. Please convey your reservations
     for current placement.
   - It also impacts the point (8) about priority use in constructor

3. Though the implementation for bus is common for Linux and BSD, the PCI
   bus implementation has been done/tested only for Linux.

4. There was a suggestion from Jan Blunk about a helper iterator within the
   rte_bus. That is still pending.

5. The overall layout for driver probing has changed a little.
   earlier, it was:
    rte_eal_init()
     `-> rte_eal_pci_probe() (and parallel for VDEV)
         `-> rte_pci_driver->probe()
             `-> eth_driver->eth_dev_init()

   now, it would be:
   rte_eal_init()
     `-> rte_eal_bus_probe() <- Iterator for PCI device/driver
         `-> rte_driver->probe() <- devargs handling
             |                      old rte_eal_pci_probe()
             `-> rte_xxx_driver->probe() <- eth_dev allocation
                 `-> eth_driver->eth_dev_init <- eth_dev init

   Open Questions:
       Also, rte_driver->probe() creating eth_dev certainly sounds a little
       wrong - but, I would like to get your opinion on how to lay this
       order of which layer ethernet device corresponds to.
       1) Which layer should allocate eth_dev?
          `-> My take: rte_driver->probe()
       2) which layer should fill the eth_dev?
          `-> My take: rte_xxx_driver->probe()
       3) Is init/uninit better name for rte_xxx_driver()->probe() if all
          they do is initialize the ethernet device?

 8. RTE_REGISTER_BUS has been declared with contructor priority of 101
    It is important that Bus is registered *before* drivers are registered.
    Only way I could find to assure that was via 
    __attribute(contructor(priority)) of GCC. I am not sure how it would
    behave on other compilers. Any suggestions?
    - One suggestion from David Marchand was to use global bus object
      handles, which I have not implemented for now. If that is common
      choice, I will change in v3.

:: ToDo list ::

 - Bump to librte_eal version
 - Documentation continues to have references to some _old_ PCI symbols
 - vdev changes
 - eth_device, eth_driver changes

:: References ::

[1] http://dpdk.org/ml/archives/dev/2016-November/050186.html
[2] http://dpdk.org/ml/archives/dev/2016-November/050622.html
[3] http://dpdk.org/ml/archives/dev/2016-November/050416.html
[4] http://dpdk.org/ml/archives/dev/2016-November/050567.html
[5] http://dpdk.org/ml/archives/dev/2016-November/050628.html
[6] http://dpdk.org/ml/archives/dev/2016-November/050415.html
[7] http://dpdk.org/ml/archives/dev/2016-November/050443.html
[8] http://dpdk.org/ml/archives/dev/2016-November/050624.html
[9] http://dpdk.org/ml/archives/dev/2016-November/050296.html
[10] http://dpdk.org/ml/archives/dev/2016-December/051349.html
[12] http://dpdk.org/ml/archives/dev/2016-December/052092.html

:: Version Changes ::
v3:
 - rebase over master (c431384c8f)
 - revert patch 0001 changes for checkpatch (container_of macro)
 - qat/rte_qat_cryptodev update for rte_driver->probe
 - test_pci update for using a test_pci_bus for verification
 - some bug fixes based on internal testing.
 -- rte_eal_dev_attach not handling devargs
 -- blacklisting not working

v2:
 - No more bus->probe()
   Now, rte_eal_bus_probe() calls rte_driver->probe based on match output
 - new functions, rte_eal_pci_probe and rte_eal_pci_remove have been
   added as glue code between PCI PMDs and PCI Bus
   `-> PMDs are updated to use these new functions as callbacks for
       rte_driver
 - 'default' keyword has been removed from match and scan
 - Fix for incorrect changes in mlx* and nicvf*
 - Checkpatch fixes
 - Some variable checks have been removed from internal functions;
   functions which are externally visible continue to have such checks
 - Some rearrangement of patches:
   -- changes to drivers have been separated from EAL changes (but this
      does make PCI PMDs non-working for a particular patch)

Ben Walker (1):
  pci: Pass rte_pci_addr to functions instead of separate args

Jan Blunck (1):
  eal: define container_of macro

Shreyansh Jain (10):
  eal/bus: introduce bus abstraction
  test: add basic bus infrastructure tests
  eal/bus: add scan, match and insert support
  eal: integrate bus scan and probe with EAL
  eal: add probe and remove support for rte_driver
  eal: enable probe from bus infrastructure
  pci: split match and probe function
  eal/pci: generalize args of PCI scan/match towards RTE device/driver
  eal: enable PCI bus and PCI test framework
  drivers: update PMDs to use rte_driver probe and remove

 app/test/Makefile                               |   2 +-
 app/test/test.h                                 |   2 +
 app/test/test_bus.c                             | 688 ++++++++++++++++++++++++
 app/test/test_pci.c                             | 154 ++++--
 drivers/crypto/qat/rte_qat_cryptodev.c          |   4 +
 drivers/net/bnx2x/bnx2x_ethdev.c                |   8 +
 drivers/net/bnxt/bnxt_ethdev.c                  |   4 +
 drivers/net/cxgbe/cxgbe_ethdev.c                |   4 +
 drivers/net/e1000/em_ethdev.c                   |   4 +
 drivers/net/e1000/igb_ethdev.c                  |   8 +
 drivers/net/ena/ena_ethdev.c                    |   4 +
 drivers/net/enic/enic_ethdev.c                  |   4 +
 drivers/net/fm10k/fm10k_ethdev.c                |   4 +
 drivers/net/i40e/i40e_ethdev.c                  |   4 +
 drivers/net/i40e/i40e_ethdev_vf.c               |   4 +
 drivers/net/ixgbe/ixgbe_ethdev.c                |   8 +
 drivers/net/mlx4/mlx4.c                         |   4 +-
 drivers/net/mlx5/mlx5.c                         |   1 +
 drivers/net/nfp/nfp_net.c                       |   4 +
 drivers/net/qede/qede_ethdev.c                  |   8 +
 drivers/net/szedata2/rte_eth_szedata2.c         |   4 +
 drivers/net/thunderx/nicvf_ethdev.c             |   4 +
 drivers/net/virtio/virtio_ethdev.c              |   2 +
 drivers/net/vmxnet3/vmxnet3_ethdev.c            |   4 +
 lib/librte_eal/bsdapp/eal/Makefile              |   1 +
 lib/librte_eal/bsdapp/eal/eal.c                 |  12 +-
 lib/librte_eal/bsdapp/eal/eal_pci.c             |  52 +-
 lib/librte_eal/bsdapp/eal/rte_eal_version.map   |  22 +-
 lib/librte_eal/common/Makefile                  |   2 +-
 lib/librte_eal/common/eal_common_bus.c          | 286 ++++++++++
 lib/librte_eal/common/eal_common_pci.c          | 344 +++++++-----
 lib/librte_eal/common/eal_private.h             |  14 +-
 lib/librte_eal/common/include/rte_bus.h         | 257 +++++++++
 lib/librte_eal/common/include/rte_common.h      |  20 +
 lib/librte_eal/common/include/rte_dev.h         |  14 +
 lib/librte_eal/common/include/rte_pci.h         |  59 +-
 lib/librte_eal/linuxapp/eal/Makefile            |   1 +
 lib/librte_eal/linuxapp/eal/eal.c               |  12 +-
 lib/librte_eal/linuxapp/eal/eal_pci.c           |  84 ++-
 lib/librte_eal/linuxapp/eal/rte_eal_version.map |  22 +-
 40 files changed, 1840 insertions(+), 299 deletions(-)
 create mode 100644 app/test/test_bus.c
 create mode 100644 lib/librte_eal/common/eal_common_bus.c
 create mode 100644 lib/librte_eal/common/include/rte_bus.h

-- 
2.7.4

^ permalink raw reply

* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Bruce Richardson @ 2016-12-16 13:04 UTC (permalink / raw)
  To: Neil Horman
  Cc: Olivier Matz, Stephen Hemminger, dev, thomas.monjalon, vido,
	fiona.trahe, adrien.mazarguil
In-Reply-To: <20161216123738.GA2255@hmsreliant.think-freely.org>

On Fri, Dec 16, 2016 at 07:37:38AM -0500, Neil Horman wrote:
> On Fri, Dec 16, 2016 at 10:22:08AM +0100, Olivier Matz wrote:
> > Hi,
> > 
> > On Thu, 15 Dec 2016 09:22:07 -0800, Stephen Hemminger
> > <stephen@networkplumber.org> wrote:
> > > On Thu, 15 Dec 2016 11:09:12 -0500
> > > Neil Horman <nhorman@tuxdriver.com> wrote:
> > > 
> > > > On Thu, Dec 15, 2016 at 02:46:39PM +0100, Olivier Matz wrote:  
> > > > > Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver
> > > > > to declare the list of kernel modules required to run properly.
> > > > > 
> > > > > Today, most PCI drivers require uio/vfio.
> > > > > 
> > > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > > Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> > > > > ---
> > > > > 
> > > > > v2 -> v3:
> > > > > - fix kmods deps advertised by mellanox drivers as pointed out
> > > > >   by Adrien
> > > > > 
> > > > > v1 ->
> > > > > v2:                                                                                                
> > > > > - do not advertise uio_pci_generic for vf drivers
> > > > > - rebase on top of head: use new driver names and prefix
> > > > >   macro with
> > > > > RTE_                                                                                       
> > > > > 
> > > > > rfc -> v1:
> > > > > - the kmod information can be per-device using a modalias-like
> > > > >   pattern
> > > > > - change syntax to use '&' and '|' instead of ',' and ':'
> > > > > - remove useless prerequisites in kmod lis: no need to
> > > > >   specify both uio and uio_pci_generic, only the latter is
> > > > >   required
> > > > > - update kmod list in szedata2 driver
> > > > > - remove kmod list in qat driver: it requires more than just
> > > > > loading a kmod, which is described in documentation
> > > > > 
> > > > >  buildtools/pmdinfogen/pmdinfogen.c      |  1 +
> > > > >  buildtools/pmdinfogen/pmdinfogen.h      |  1 +
> > > > >  drivers/net/bnx2x/bnx2x_ethdev.c        |  2 ++
> > > > >  drivers/net/bnxt/bnxt_ethdev.c          |  1 +
> > > > >  drivers/net/cxgbe/cxgbe_ethdev.c        |  1 +
> > > > >  drivers/net/e1000/em_ethdev.c           |  1 +
> > > > >  drivers/net/e1000/igb_ethdev.c          |  2 ++
> > > > >  drivers/net/ena/ena_ethdev.c            |  1 +
> > > > >  drivers/net/enic/enic_ethdev.c          |  1 +
> > > > >  drivers/net/fm10k/fm10k_ethdev.c        |  1 +
> > > > >  drivers/net/i40e/i40e_ethdev.c          |  1 +
> > > > >  drivers/net/i40e/i40e_ethdev_vf.c       |  1 +
> > > > >  drivers/net/ixgbe/ixgbe_ethdev.c        |  2 ++
> > > > >  drivers/net/mlx4/mlx4.c                 |  2 ++
> > > > >  drivers/net/mlx5/mlx5.c                 |  1 +
> > > > >  drivers/net/nfp/nfp_net.c               |  1 +
> > > > >  drivers/net/qede/qede_ethdev.c          |  2 ++
> > > > >  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
> > > > >  drivers/net/thunderx/nicvf_ethdev.c     |  1 +
> > > > >  drivers/net/virtio/virtio_ethdev.c      |  1 +
> > > > >  drivers/net/vmxnet3/vmxnet3_ethdev.c    |  1 +
> > > > >  lib/librte_eal/common/include/rte_dev.h | 25
> > > > > +++++++++++++++++++++++++ tools/dpdk-pmdinfo.py
> > > > > |  5 ++++- 23 files changed, 56 insertions(+), 1 deletion(-)
> > > > >     
> > > > Its odd that all devices, regardless of vendor should depend on the
> > > > igb_uio module.  It seems to me that depending on uio_pci_generic
> > > > or vfio is sufficient.
> > 
> > igb_uio is the historical uio module of dpdk. Although it is called
> > igb_uio, it is not specific to Intel drivers.
> > 
> > Most drivers declare "igb_uio | uio_pci_generic | vfio", which means
> > that any of the 3 kernel modules can be used.
> > 
> > I think there are some cases where people will prefer using igb_uio,
> > for instance to use a vf pmd in a vm where there is no iommu,
> > and where the kernel vfio module does not support the no-iommu mode.
> > 
> > 
> > > 
> > > Yes it seems just a special case extension for Mellanox drivers.
> > 
> > Kmod deps are different whether it's a vf driver or not.
> > Mellanox drivers are not the only drivers that do not require uio,
> > there is also szedata2.
> > 
> > Is it an argument for not including this patch?
> > 
> Speaking only for myself, I'm not suggesting the patch not be included, only
> questioning the need to list igb_uio as an optional dependency.  From what I
> understand uio_pci_generic is equaly capable of being used in a vf as igb_uio,
> and so it seems like its sufficient to list in the deps alone, or am I missing
> something?
> 

Unfortunately uio_pci_generic cannot be used with VFs either inside or
outside a VM, as VFs use MSI/MSI-X rather than legacy interrupts. For
newer kernels with vfio-noiommu, that is an option, but for any other
case, igb_uio is needed.

/Bruce

^ permalink raw reply

* [PATCH 3/3] driver: vHost support to free consumed buffers
From: Billy McFall @ 2016-12-16 12:48 UTC (permalink / raw)
  To: thomas.monjalon, wenzhuo.lu; +Cc: dev, Billy McFall
In-Reply-To: <20161216124851.2640-1-bmcfall@redhat.com>

Add support to the vHostdriver for the new API to force free consumed
buffers on TX ring. vHost does not cache the mbufs so there is no work
to do.

Signed-off-by: Billy McFall <bmcfall@redhat.com>
---
 drivers/net/vhost/rte_eth_vhost.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/net/vhost/rte_eth_vhost.c b/drivers/net/vhost/rte_eth_vhost.c
index 766d4ef..6493d56 100644
--- a/drivers/net/vhost/rte_eth_vhost.c
+++ b/drivers/net/vhost/rte_eth_vhost.c
@@ -939,6 +939,16 @@ eth_queue_release(void *q)
 }
 
 static int
+eth_tx_done_cleanup(void *txq __rte_unused, uint32_t free_cnt __rte_unused)
+{
+	/*
+	 * vHost does not hang onto mbuf. eth_vhost_tx() copies packet data
+	 * and releases mbuf, so nothing to cleanup.
+	 */
+	return 0;
+}
+
+static int
 eth_link_update(struct rte_eth_dev *dev __rte_unused,
 		int wait_to_complete __rte_unused)
 {
@@ -979,6 +989,7 @@ static const struct eth_dev_ops ops = {
 	.tx_queue_setup = eth_tx_queue_setup,
 	.rx_queue_release = eth_queue_release,
 	.tx_queue_release = eth_queue_release,
+	.tx_done_cleanup = eth_tx_done_cleanup,
 	.link_update = eth_link_update,
 	.stats_get = eth_stats_get,
 	.stats_reset = eth_stats_reset,
-- 
2.9.3

^ permalink raw reply related

* [PATCH 2/3] driver: e1000 igb support to free consumed buffers
From: Billy McFall @ 2016-12-16 12:48 UTC (permalink / raw)
  To: thomas.monjalon, wenzhuo.lu; +Cc: dev, Billy McFall
In-Reply-To: <20161216124851.2640-1-bmcfall@redhat.com>

Add support to the e1000 igb driver for the new API to force free
consumed buffers on TX ring. e1000 igb driver does not implement a
tx_rs_thresh to free mbufs, it frees a slot in the ring as needed, so
a new function needed to be written.

Signed-off-by: Billy McFall <bmcfall@redhat.com>
---
 drivers/net/e1000/e1000_ethdev.h |   2 +
 drivers/net/e1000/igb_ethdev.c   |   1 +
 drivers/net/e1000/igb_rxtx.c     | 126 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 129 insertions(+)

diff --git a/drivers/net/e1000/e1000_ethdev.h b/drivers/net/e1000/e1000_ethdev.h
index 6c25c8d..fce0278 100644
--- a/drivers/net/e1000/e1000_ethdev.h
+++ b/drivers/net/e1000/e1000_ethdev.h
@@ -308,6 +308,8 @@ int eth_igb_tx_queue_setup(struct rte_eth_dev *dev, uint16_t tx_queue_id,
 		uint16_t nb_tx_desc, unsigned int socket_id,
 		const struct rte_eth_txconf *tx_conf);
 
+int eth_igb_tx_done_cleanup(void *txq, uint32_t free_cnt);
+
 int eth_igb_rx_init(struct rte_eth_dev *dev);
 
 void eth_igb_tx_init(struct rte_eth_dev *dev);
diff --git a/drivers/net/e1000/igb_ethdev.c b/drivers/net/e1000/igb_ethdev.c
index 2fddf0c..4010dc4 100644
--- a/drivers/net/e1000/igb_ethdev.c
+++ b/drivers/net/e1000/igb_ethdev.c
@@ -402,6 +402,7 @@ static const struct eth_dev_ops eth_igb_ops = {
 	.rx_descriptor_done   = eth_igb_rx_descriptor_done,
 	.tx_queue_setup       = eth_igb_tx_queue_setup,
 	.tx_queue_release     = eth_igb_tx_queue_release,
+	.tx_done_cleanup      = eth_igb_tx_done_cleanup,
 	.dev_led_on           = eth_igb_led_on,
 	.dev_led_off          = eth_igb_led_off,
 	.flow_ctrl_get        = eth_igb_flow_ctrl_get,
diff --git a/drivers/net/e1000/igb_rxtx.c b/drivers/net/e1000/igb_rxtx.c
index dbd37ac..1d4fbcb 100644
--- a/drivers/net/e1000/igb_rxtx.c
+++ b/drivers/net/e1000/igb_rxtx.c
@@ -1227,6 +1227,132 @@ eth_igb_tx_queue_release(void *txq)
 	igb_tx_queue_release(txq);
 }
 
+static int
+igb_tx_done_cleanup(struct igb_tx_queue *txq, uint32_t free_cnt)
+{
+	struct igb_tx_entry *sw_ring;
+	volatile union e1000_adv_tx_desc *txr;
+	uint16_t tx_first; /* First segment analyzed. */
+	uint16_t tx_id;    /* Current segment being processed. */
+	uint16_t tx_last;  /* Last segment in the current packet. */
+	uint16_t tx_next;  /* First segment of the next packet. */
+	int count;
+
+	if (txq != NULL) {
+		count = 0;
+		sw_ring = txq->sw_ring;
+		txr = txq->tx_ring;
+
+		/*
+		 * tx_tail is the last sent packet on the sw_ring. Goto the end
+		 * of that packet (the last segment in the packet chain) and
+		 * then the next segment will be the start of the oldest segment
+		 * in the sw_ring. This is the first packet that will be
+		 * attempted to be freed.
+		 */
+
+		/* Get last segment in most recently added packet. */
+		tx_first = sw_ring[txq->tx_tail].last_id;
+
+		/* Get the next segment, which is the oldest segment in ring. */
+		tx_first = sw_ring[tx_first].next_id;
+
+		/* Set the current index to the first. */
+		tx_id = tx_first;
+
+		/*
+		 * Loop through each packet. For each packet, verify that an
+		 * mbuf exists and that the last segment is free. If so, free
+		 * it and move on.
+		 */
+		while (1) {
+			tx_last = sw_ring[tx_id].last_id;
+
+			if (sw_ring[tx_last].mbuf) {
+				if (txr[tx_last].wb.status &
+						E1000_TXD_STAT_DD) {
+					/*
+					 * Increment the number of packets
+					 * freed.
+					 */
+					count++;
+
+					/* Get the start of the next packet. */
+					tx_next = sw_ring[tx_last].next_id;
+
+					/*
+					 * Loop through all segments in a
+					 * packet.
+					 */
+					do {
+						rte_pktmbuf_free_seg(sw_ring[tx_id].mbuf);
+						sw_ring[tx_id].mbuf = NULL;
+						sw_ring[tx_id].last_id = tx_id;
+
+						/* Move to next segemnt. */
+						tx_id = sw_ring[tx_id].next_id;
+
+					} while (tx_id != tx_next);
+
+					if (unlikely(count == (int)free_cnt))
+						break;
+				} else
+					/*
+					 * mbuf still in use, nothing left to
+					 * free.
+					 */
+					break;
+			} else {
+				/*
+				 * There are multiple reasons to be here:
+				 * 1) All the packets on the ring have been
+				 *    freed - tx_id is equal to tx_first
+				 *    and some packets have been freed.
+				 *    - Done, exit
+				 * 2) Interfaces has not sent a rings worth of
+				 *    packets yet, so the segment after tail is
+				 *    still empty. Or a previous call to this
+				 *    function freed some of the segments but
+				 *    not all so there is a hole in the list.
+				 *    Hopefully this is a rare case.
+				 *    - Walk the list and find the next mbuf. If
+				 *      there isn't one, then done.
+				 */
+				if (likely((tx_id == tx_first) && (count != 0)))
+					break;
+
+				/*
+				 * Walk the list and find the next mbuf, if any.
+				 */
+				do {
+					/* Move to next segemnt. */
+					tx_id = sw_ring[tx_id].next_id;
+
+					if (sw_ring[tx_id].mbuf)
+						break;
+
+				} while (tx_id != tx_first);
+
+				/*
+				 * Determine why previous loop bailed. If there
+				 * is not an mbuf, done.
+				 */
+				if (sw_ring[tx_id].mbuf == NULL)
+					break;
+			}
+		}
+	} else
+		count = -ENODEV;
+
+	return count;
+}
+
+int
+eth_igb_tx_done_cleanup(void *txq, uint32_t free_cnt)
+{
+	return igb_tx_done_cleanup(txq, free_cnt);
+}
+
 static void
 igb_reset_tx_queue_stat(struct igb_tx_queue *txq)
 {
-- 
2.9.3

^ permalink raw reply related

* [PATCH 1/3] ethdev: New API to free consumed buffers in TX ring
From: Billy McFall @ 2016-12-16 12:48 UTC (permalink / raw)
  To: thomas.monjalon, wenzhuo.lu; +Cc: dev, Billy McFall
In-Reply-To: <20161216124851.2640-1-bmcfall@redhat.com>

Add a new API to force free consumed buffers on TX ring. API will return
the number of packets freed (0-n) or error code if feature not supported
(-ENOTSUP) or input invalid (-ENODEV).

Because rte_eth_tx_buffer() may be used, and mbufs may still be held
in local buffer, the API also accepts *buffer and *sent. Before
attempting to free, rte_eth_tx_buffer_flush() is called to make sure
all mbufs are sent to Tx ring. rte_eth_tx_buffer_flush() is called even
if threshold is not met.

Signed-off-by: Billy McFall <bmcfall@redhat.com>
---
 lib/librte_ether/rte_ethdev.h | 56 +++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 56 insertions(+)

diff --git a/lib/librte_ether/rte_ethdev.h b/lib/librte_ether/rte_ethdev.h
index 9678179..e3f2be4 100644
--- a/lib/librte_ether/rte_ethdev.h
+++ b/lib/librte_ether/rte_ethdev.h
@@ -1150,6 +1150,9 @@ typedef uint32_t (*eth_rx_queue_count_t)(struct rte_eth_dev *dev,
 typedef int (*eth_rx_descriptor_done_t)(void *rxq, uint16_t offset);
 /**< @internal Check DD bit of specific RX descriptor */
 
+typedef int (*eth_tx_done_cleanup_t)(void *txq, uint32_t free_cnt);
+/**< @internal Force mbufs to be from TX ring. */
+
 typedef void (*eth_rxq_info_get_t)(struct rte_eth_dev *dev,
 	uint16_t rx_queue_id, struct rte_eth_rxq_info *qinfo);
 
@@ -1467,6 +1470,7 @@ struct eth_dev_ops {
 	eth_rx_disable_intr_t      rx_queue_intr_disable;
 	eth_tx_queue_setup_t       tx_queue_setup;/**< Set up device TX queue.*/
 	eth_queue_release_t        tx_queue_release;/**< Release TX queue.*/
+	eth_tx_done_cleanup_t      tx_done_cleanup;/**< Free tx ring mbufs */
 	eth_dev_led_on_t           dev_led_on;    /**< Turn on LED. */
 	eth_dev_led_off_t          dev_led_off;   /**< Turn off LED. */
 	flow_ctrl_get_t            flow_ctrl_get; /**< Get flow control. */
@@ -2943,6 +2947,58 @@ rte_eth_tx_buffer(uint8_t port_id, uint16_t queue_id,
 }
 
 /**
+ * Request the driver to free mbufs currently cached by the driver. The
+ * driver will only free the mbuf if it is no longer in use.
+ *
+ * @param port_id
+ *   The port identifier of the Ethernet device.
+ * @param queue_id
+ *   The index of the transmit queue through which output packets must be
+ *   sent.
+ *   The value must be in the range [0, nb_tx_queue - 1] previously supplied
+ *   to rte_eth_dev_configure().
+ * @param free_cnt
+ *   Maximum number of packets to free. Use 0 to indicate all possible packets
+ *   should be freed. Note that a packet may be using multiple mbufs.
+ * @param buffer
+ *   Buffer used to collect packets to be sent. If provided, the buffer will
+ *   be flushed, even if the current length is less than buffer->size. Pass NULL
+ *   if buffer has already been flushed.
+ * @param sent
+ *   Pointer to return number of packets sent if buffer has packets to be sent.
+ *   If *buffer is supplied, *sent must also be supplied.
+ * @return
+ *   Failure: < 0
+ *     -ENODEV: Invalid interface
+ *     -ENOTSUP: Driver does not support function
+ *   Success: >= 0
+ *     0-n: Number of packets freed. More packets may still remain in ring that
+ *     are in use.
+ */
+
+static inline int
+rte_eth_tx_done_cleanup(uint8_t port_id, uint16_t queue_id,  uint32_t free_cnt,
+		struct rte_eth_dev_tx_buffer *buffer, uint16_t *sent)
+{
+	struct rte_eth_dev *dev = &rte_eth_devices[port_id];
+
+	/* Validate Input Data. Bail if not valid or not supported. */
+	RTE_ETH_VALID_PORTID_OR_ERR_RET(port_id, -ENODEV);
+	RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->tx_done_cleanup, -ENOTSUP);
+
+	/*
+	 * If transmit buffer is provided and there are still packets to be
+	 * sent, then send them before attempting to free pending mbufs.
+	 */
+	if (buffer && sent)
+		*sent = rte_eth_tx_buffer_flush(port_id, queue_id, buffer);
+
+	/* Call driver to free pending mbufs. */
+	return (*dev->dev_ops->tx_done_cleanup)(dev->data->tx_queues[queue_id],
+			free_cnt);
+}
+
+/**
  * Configure a callback for buffered packets which cannot be sent
  *
  * Register a specific callback to be called when an attempt is made to send
-- 
2.9.3

^ permalink raw reply related

* [PATCH 0/3] New API to free consumed buffers in TX ring
From: Billy McFall @ 2016-12-16 12:48 UTC (permalink / raw)
  To: thomas.monjalon, wenzhuo.lu; +Cc: dev, Billy McFall

Based on a request from Damjan Marion and seconded by Keith Wiles, see
dpdk-dev mailling list from 11/21/2016, add a new API to free consumed
buffers on TX ring. This addresses two scenarios:
1) Flooding a packet and want to reuse existing mbuf to avoid a packet
copy. Increment the reference count of the packet and poll new API until
reference count is decremented.
2) Application runs out of mbufs so call API to free consumed packets so
processing can continue.

API will return the number of packets freed (0-n) or error code if
feature not supported (-ENOTSUP) or input invalid (-ENODEV).

API for e1000 igb driver and vHost driver have been implemented. Other
drivers can be implemented over time. Some drivers implement a TX done
flush routine that should be reused where possible. e1000 igb driver
and vHost driver do not have such functions.

Billy McFall (3):
  ethdev: New API to free consumed buffers in TX ring
  driver: e1000 igb support to free consumed buffers
  driver: vHost support to free consumed buffers

 drivers/net/e1000/e1000_ethdev.h  |   2 +
 drivers/net/e1000/igb_ethdev.c    |   1 +
 drivers/net/e1000/igb_rxtx.c      | 126 ++++++++++++++++++++++++++++++++++++++
 drivers/net/vhost/rte_eth_vhost.c |  11 ++++
 lib/librte_ether/rte_ethdev.h     |  56 +++++++++++++++++
 5 files changed, 196 insertions(+)

-- 
2.9.3

^ permalink raw reply

* Re: [PATCH v3] drivers: advertise kmod dependencies in pmdinfo
From: Neil Horman @ 2016-12-16 12:37 UTC (permalink / raw)
  To: Olivier Matz
  Cc: Stephen Hemminger, dev, thomas.monjalon, vido, fiona.trahe,
	adrien.mazarguil
In-Reply-To: <20161216102208.08955321@platinum>

On Fri, Dec 16, 2016 at 10:22:08AM +0100, Olivier Matz wrote:
> Hi,
> 
> On Thu, 15 Dec 2016 09:22:07 -0800, Stephen Hemminger
> <stephen@networkplumber.org> wrote:
> > On Thu, 15 Dec 2016 11:09:12 -0500
> > Neil Horman <nhorman@tuxdriver.com> wrote:
> > 
> > > On Thu, Dec 15, 2016 at 02:46:39PM +0100, Olivier Matz wrote:  
> > > > Add a new macro RTE_PMD_REGISTER_KMOD_DEP() that allows a driver
> > > > to declare the list of kernel modules required to run properly.
> > > > 
> > > > Today, most PCI drivers require uio/vfio.
> > > > 
> > > > Signed-off-by: Olivier Matz <olivier.matz@6wind.com>
> > > > Acked-by: Fiona Trahe <fiona.trahe@intel.com>
> > > > ---
> > > > 
> > > > v2 -> v3:
> > > > - fix kmods deps advertised by mellanox drivers as pointed out
> > > >   by Adrien
> > > > 
> > > > v1 ->
> > > > v2:                                                                                                
> > > > - do not advertise uio_pci_generic for vf drivers
> > > > - rebase on top of head: use new driver names and prefix
> > > >   macro with
> > > > RTE_                                                                                       
> > > > 
> > > > rfc -> v1:
> > > > - the kmod information can be per-device using a modalias-like
> > > >   pattern
> > > > - change syntax to use '&' and '|' instead of ',' and ':'
> > > > - remove useless prerequisites in kmod lis: no need to
> > > >   specify both uio and uio_pci_generic, only the latter is
> > > >   required
> > > > - update kmod list in szedata2 driver
> > > > - remove kmod list in qat driver: it requires more than just
> > > > loading a kmod, which is described in documentation
> > > > 
> > > >  buildtools/pmdinfogen/pmdinfogen.c      |  1 +
> > > >  buildtools/pmdinfogen/pmdinfogen.h      |  1 +
> > > >  drivers/net/bnx2x/bnx2x_ethdev.c        |  2 ++
> > > >  drivers/net/bnxt/bnxt_ethdev.c          |  1 +
> > > >  drivers/net/cxgbe/cxgbe_ethdev.c        |  1 +
> > > >  drivers/net/e1000/em_ethdev.c           |  1 +
> > > >  drivers/net/e1000/igb_ethdev.c          |  2 ++
> > > >  drivers/net/ena/ena_ethdev.c            |  1 +
> > > >  drivers/net/enic/enic_ethdev.c          |  1 +
> > > >  drivers/net/fm10k/fm10k_ethdev.c        |  1 +
> > > >  drivers/net/i40e/i40e_ethdev.c          |  1 +
> > > >  drivers/net/i40e/i40e_ethdev_vf.c       |  1 +
> > > >  drivers/net/ixgbe/ixgbe_ethdev.c        |  2 ++
> > > >  drivers/net/mlx4/mlx4.c                 |  2 ++
> > > >  drivers/net/mlx5/mlx5.c                 |  1 +
> > > >  drivers/net/nfp/nfp_net.c               |  1 +
> > > >  drivers/net/qede/qede_ethdev.c          |  2 ++
> > > >  drivers/net/szedata2/rte_eth_szedata2.c |  2 ++
> > > >  drivers/net/thunderx/nicvf_ethdev.c     |  1 +
> > > >  drivers/net/virtio/virtio_ethdev.c      |  1 +
> > > >  drivers/net/vmxnet3/vmxnet3_ethdev.c    |  1 +
> > > >  lib/librte_eal/common/include/rte_dev.h | 25
> > > > +++++++++++++++++++++++++ tools/dpdk-pmdinfo.py
> > > > |  5 ++++- 23 files changed, 56 insertions(+), 1 deletion(-)
> > > >     
> > > Its odd that all devices, regardless of vendor should depend on the
> > > igb_uio module.  It seems to me that depending on uio_pci_generic
> > > or vfio is sufficient.
> 
> igb_uio is the historical uio module of dpdk. Although it is called
> igb_uio, it is not specific to Intel drivers.
> 
> Most drivers declare "igb_uio | uio_pci_generic | vfio", which means
> that any of the 3 kernel modules can be used.
> 
> I think there are some cases where people will prefer using igb_uio,
> for instance to use a vf pmd in a vm where there is no iommu,
> and where the kernel vfio module does not support the no-iommu mode.
> 
> 
> > 
> > Yes it seems just a special case extension for Mellanox drivers.
> 
> Kmod deps are different whether it's a vf driver or not.
> Mellanox drivers are not the only drivers that do not require uio,
> there is also szedata2.
> 
> Is it an argument for not including this patch?
> 
Speaking only for myself, I'm not suggesting the patch not be included, only
questioning the need to list igb_uio as an optional dependency.  From what I
understand uio_pci_generic is equaly capable of being used in a vf as igb_uio,
and so it seems like its sufficient to list in the deps alone, or am I missing
something?

Additionally, in regards to the comment about rebasing on net-next here, I don't
think thats needed.  This patch is built such that you will be able to apply
this tag to additional drivers later, as they get merged into thomas's tree, you
don't need to get them all in one shot.  More to the point, there are crypto
drivers that may make use of this module dependency tag, and those are also
missing.  I would suggest taking the patch based on it current state (once the
above igb_uio issue is settled), and then adding the tag to new drivers in
subsequent releases as they get merged.

Neil

> 
> Regards,
> Olivier
> 

^ permalink raw reply

* Re: [PATCH 12/22] app/testpmd: add rte_flow item spec handler
From: Xing, Beilei @ 2016-12-16 12:22 UTC (permalink / raw)
  To: Adrien Mazarguil, Pei, Yulong
  Cc: dev@dpdk.org, Thomas Monjalon, De Lara Guarch, Pablo,
	Olivier Matz
In-Reply-To: <20161216091746.GC10340@6wind.com>

Thanks Adrien.

I have two questions:
1.  when I set " / vlan tci fix 10" with testpmd, I find the mask of tci is 0xFFFF.
     Actually tci includes PRI, CFI, and Vlan_id which holds 12 bits, so is it possible
     to set the mask to 0xFFF? 
     Our driver will check the mask only covers vlan_id instead of the whole tci.

2. When we test destroy function, we find the pointer provided to PMD is NULL
    instead of the pointer PMD returned to RTE during creating flow. Could you
    please have double check? Thanks.

Best Regards
Beilei

> -----Original Message-----
> From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com]
> Sent: Friday, December 16, 2016 5:18 PM
> To: Pei, Yulong <yulong.pei@intel.com>
> Cc: dev@dpdk.org; Thomas Monjalon <thomas.monjalon@6wind.com>; De
> Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Olivier Matz
> <olivier.matz@6wind.com>; Xing, Beilei <beilei.xing@intel.com>
> Subject: Re: [dpdk-dev] [PATCH 12/22] app/testpmd: add rte_flow item spec
> handler
> 
> Hi Yulong,
> 
> On Fri, Dec 16, 2016 at 03:01:15AM +0000, Pei, Yulong wrote:
> > Hi Adrien,
> >
> > I try to setup the following rule, but it seems that after set 'spec'  param,
> can not set 'mask' param,  is it an issue here or am I wrong to use it ?
> >
> > testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00
> >  dst [TOKEN]: destination MAC
> >  src [TOKEN]: source MAC
> >  type [TOKEN]: EtherType
> >  / [TOKEN]: specify next pattern item
> 
> You need to re-specify dst with "mask" instead of "spec". You can specify it
> as many times you like to update each structure in turn, e.g.:
> 
>  testpmd> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst
> mask 00:00:00:00:ff:ff
> 
> If you want to specify both spec and mask at once assuming you want it full,
> these commands yield the same result:
> 
>  testpmd> flow create 0 ingress pattern eth dst fix 00:00:00:00:09:00  testpmd>
> flow create 0 ingress pattern eth dst spec 00:00:00:00:09:00 dst mask
> ff:ff:ff:ff:ff:ff  testpmd> flow create 0 ingress pattern eth dst spec
> 00:00:00:00:09:00 dst prefix 48
> 
> You are even allowed to change your mind:
> 
>  testpmd> flow create 0 ingress pattern eth dst fix 00:00:2a:2a:2a:2a dst fix
> 00:00:00:00:09:00
> 
> All these will be properly documented in the v2 patchset. Note, this version
> will replace the "fix" keyword with "is" ("fix" made no sense according to
> feedback).
> 
> --
> Adrien Mazarguil
> 6WIND

^ permalink raw reply

* Re: [PATCH v2 01/12] eal: define container_of macro
From: Shreyansh Jain @ 2016-12-16 11:54 UTC (permalink / raw)
  To: Adrien Mazarguil, Jan Blunck
  Cc: dev, David Marchand, Thomas Monjalon, Ferruh Yigit, jianbo.liu,
	Jan Viktorin
In-Reply-To: <20161216112140.GE10340@6wind.com>

On Friday 16 December 2016 04:51 PM, Adrien Mazarguil wrote:
> On Fri, Dec 16, 2016 at 11:47:14AM +0100, Jan Blunck wrote:
>> On Fri, Dec 16, 2016 at 10:23 AM, Adrien Mazarguil
>> <adrien.mazarguil@6wind.com> wrote:
>>> On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
>>>> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
>>>>> On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
>>>>>>
>>>>>> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
>>>>>> wrote:
>>>>>>>
>>>>>>> From: Jan Blunck <jblunck@infradead.org>
>>>>>>>
>>>>>>> This macro is based on Jan Viktorin's original patch but also checks the
>>>>>>> type of the passed pointer against the type of the member.
>>>>>>>
>>>>>>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
>>>>>>> [shreyansh.jain@nxp.com: Fix checkpatch error]
>>>>>>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
>>>>>>> [jblunck@infradead.org: add type checking and __extension__]
>>>>>>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
>>>>>>>
>>>>>>> --
>>>>>>> v2:
>>>>>>>  - fix checkpatch error
>>>>>>> ---
>>>>>>>  lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
>>>>>>>  1 file changed, 21 insertions(+)
>>>>>>>
>>>>>>> diff --git a/lib/librte_eal/common/include/rte_common.h
>>>>>>> b/lib/librte_eal/common/include/rte_common.h
>>>>>>> index db5ac91..3eb8d11 100644
>>>>>>> --- a/lib/librte_eal/common/include/rte_common.h
>>>>>>> +++ b/lib/librte_eal/common/include/rte_common.h
>>>>>>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
>>>>>>>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
>>>>>>>  #endif
>>>>>>>
>>>>>>> +/**
>>>>>>> + * Return pointer to the wrapping struct instance.
>>>>>>> + *
>>>>>>> + * Example:
>>>>>>> + *
>>>>>>> + *  struct wrapper {
>>>>>>> + *      ...
>>>>>>> + *      struct child c;
>>>>>>> + *      ...
>>>>>>> + *  };
>>>>>>> + *
>>>>>>> + *  struct child *x = obtain(...);
>>>>>>> + *  struct wrapper *w = container_of(x, struct wrapper, c);
>>>>>>> + */
>>>>>>> +#ifndef container_of
>>>>>>> +#define container_of(ptr, type, member)        (__extension__  ({
>>>>>>> \
>>>>>>> +                       typeof(((type *)0)->member) * _ptr = (ptr);     \
>>>>>>> +                       (type *)(((char *)_ptr) - offsetof(type,
>>>>>>> member));\
>>>>>>> +                       }))
>>>>>>
>>>>>>
>>>>>> This is a checkpatch false positive. It should be fine to ignore this.
>>>>>> IIRC we already discussed this before.
>>>>>
>>>>>
>>>>> I too thought something similar was discussed. I tried searching the
>>>>> archives but couldn't find anything - thus, I thought probably I was
>>>>> hallucinating :P
>>>>>
>>>>> So, you want me to revert back the '()' change? Does it impact the expansion
>>>>> of this macro?
>>>>
>>>> We haven't added this on any other usage of the __extension__ keyword
>>>> in the existing code. From my perspective it is more consistent to
>>>> revert it.
>>>>
>>>> Anyone else with an opinion here? David? Thomas?
>>>
>>> As an exported header, rte_common.h must pass check-includes.sh. Both
>>> typeof() and the ({ ... }) construct are non-standard GCC extensions and
>>> would fail to compile with pedantic options.
>>>
>>
>> Thanks Adrien. These extensions are already in use by rte_common.h and
>> other headers. I don't believe we can remove the usage of typeof()
>> that easily without making the code really ugly.
>
> Sure, no problem with that, I misread and thought you wanted to drop
> __extension__ as well.
>
> Parentheses may perhaps cause more accurate warnings in case of syntax
> errors. That is not significant so either way is fine by me.
>

If that is the case, and it is OK to ignore the checkpatche warnings 
because of missing '(' and ')' (or, something else), I too prefer not to 
touch the patch unnecessarily.

I will remove my changes and revert back to original patch as created by 
Jan Blunck.

Thanks for clarifications.

-
Shreyansh

^ permalink raw reply

* Re: [PATCH 1/4] eal/common: introduce rte_memset on IA platform
From: Ananyev, Konstantin @ 2016-12-16 11:47 UTC (permalink / raw)
  To: Yang, Zhiyong, Thomas Monjalon
  Cc: dev@dpdk.org, yuanhan.liu@linux.intel.com, Richardson, Bruce,
	De Lara Guarch, Pablo
In-Reply-To: <E182254E98A5DA4EB1E657AC7CB9BD2A3EB59CAF@BGSMSX101.gar.corp.intel.com>

Hi Zhiyong,

> > > > > >
> > > > > extern void *(*__rte_memset_vector)( (void *s, int c, size_t n);
> > > > >
> > > > > static inline void*
> > > > > rte_memset_huge(void *s, int c, size_t n) {
> > > > >    return __rte_memset_vector(s, c, n); }
> > > > >
> > > > > static inline void *
> > > > > rte_memset(void *s, int c, size_t n) {
> > > > > 	If (n < XXX)
> > > > > 		return rte_memset_scalar(s, c, n);
> > > > > 	else
> > > > > 		return rte_memset_huge(s, c, n); }
> > > > >
> > > > > XXX could be either a define, or could also be a variable, so it
> > > > > can be setuped at startup, depending on the architecture.
> > > > >
> > > > > Would that work?
> > > > > Konstantin
> > > > >
> > > I have implemented the code for  choosing the functions at run time.
> > > rte_memcpy is used more frequently, So I test it at run time.
> > >
> > > typedef void *(*rte_memcpy_vector_t)(void *dst, const void *src,
> > > size_t n); extern rte_memcpy_vector_t rte_memcpy_vector; static inline
> > > void * rte_memcpy(void *dst, const void *src, size_t n) {
> > >         return rte_memcpy_vector(dst, src, n); } In order to reduce
> > > the overhead at run time, I assign the function address to var
> > > rte_memcpy_vector before main() starts to init the var.
> > >
> > > static void __attribute__((constructor))
> > > rte_memcpy_init(void)
> > > {
> > > 	if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_AVX2))
> > > 	{
> > > 		rte_memcpy_vector = rte_memcpy_avx2;
> > > 	}
> > > 	else if (rte_cpu_get_flag_enabled(RTE_CPUFLAG_SSE4_1))
> > > 	{
> > > 		rte_memcpy_vector = rte_memcpy_sse;
> > > 	}
> > > 	else
> > > 	{
> > > 		rte_memcpy_vector = memcpy;
> > > 	}
> > >
> > > }
> >
> > I thought we discussed a bit different approach.
> > In which rte_memcpy_vector() (rte_memeset_vector) would be called  only
> > after some cutoff point, i.e:
> >
> > void
> > rte_memcpy(void *dst, const void *src, size_t len) {
> > 	if (len < N) memcpy(dst, src, len);
> > 	else rte_memcpy_vector(dst, src, len);
> > }
> >
> > If you just always call rte_memcpy_vector() for every len, then it means that
> > compiler most likely has always to generate a proper call (not inlining
> > happening).
> 
> > For small length(s) price of extra function would probably overweight any
> > potential gain with SSE/AVX2 implementation.
> >
> > Konstantin
> 
> Yes, in fact,  from my tests, For small length(s)  rte_memset is far better than glibc memset,
> For large lengths, rte_memset is only a bit better than memset.
> because memset use the AVX2/SSE, too. Of course, it will use AVX512 on future machine.

Ok, thanks for clarification.
>From previous mails I got a wrong  impression that on big lengths
rte_memset_vector() is significantly faster than memset().

> 
> >For small length(s) price of extra function would probably overweight any
>  >potential gain.
> This is the key point. I think it should include the scalar optimization, not only vector optimization.
> 
> The value of rte_memset is always inlined and for small lengths it will be better.
> when in some case We are not sure that memset is always inlined by compiler.

Ok, so do you know in what cases memset() is not get inlined?
Is it when len parameter can't be precomputed by the compiler
(is not a constant)?

So to me it sounds like:
- We don't need to have an optimized verision of rte_memset() for big sizes.
- Which probably means we don't need an arch specific versions of rte_memset_vector() at all -
   for small sizes (<= 32B) scalar version would be good enough. 
- For big sizes we can just rely on memset().
Is that so?

> It seems that choosing function at run time will lose the gains.
> The following is tested on haswell by patch code.

Not sure what columns 2 and 3 in the table below mean? 
Konstantin

> ** rte_memset() - memset perf tests
>         (C = compile-time constant) **
> ======== ======= ======== ======= ========
>    Size memset in cache  memset in mem
> (bytes)        (ticks)        (ticks)
> ------- -------------- ---------------
> ============= 32B aligned ================
>       3            3 -    8       19 -  128
>       4            4 -    8       13 -  128
>       8            2 -    7       19 -  128
>       9            2 -    7       19 -  127
>      12           2 -    7       19 -  127
>      17          3 -    8        19 -  132
>      64          3 -    8        28 -  168
>     128        7 -   13       54 -  200
>     255        8 -   20       100 -  223
>     511        14 -   20     187 -  314
>    1024      24 -   29     328 -  379
>    8192     198 -  225   1829 - 2193
> 
> Thanks
> Zhiyong

^ permalink raw reply

* Re: [PATCH v2 01/12] eal: define container_of macro
From: Adrien Mazarguil @ 2016-12-16 11:21 UTC (permalink / raw)
  To: Jan Blunck
  Cc: Shreyansh Jain, dev, David Marchand, Thomas Monjalon,
	Ferruh Yigit, jianbo.liu, Jan Viktorin
In-Reply-To: <CALe+Z02WkXzbz+wA=ysvjf5SvU2bMCEY0vHTm0oGxXG=Jhs34Q@mail.gmail.com>

On Fri, Dec 16, 2016 at 11:47:14AM +0100, Jan Blunck wrote:
> On Fri, Dec 16, 2016 at 10:23 AM, Adrien Mazarguil
> <adrien.mazarguil@6wind.com> wrote:
> > On Fri, Dec 16, 2016 at 09:14:29AM +0100, Jan Blunck wrote:
> >> On Wed, Dec 14, 2016 at 6:12 AM, Shreyansh Jain <shreyansh.jain@nxp.com> wrote:
> >> > On Wednesday 14 December 2016 03:54 AM, Jan Blunck wrote:
> >> >>
> >> >> On Tue, Dec 13, 2016 at 2:37 PM, Shreyansh Jain <shreyansh.jain@nxp.com>
> >> >> wrote:
> >> >>>
> >> >>> From: Jan Blunck <jblunck@infradead.org>
> >> >>>
> >> >>> This macro is based on Jan Viktorin's original patch but also checks the
> >> >>> type of the passed pointer against the type of the member.
> >> >>>
> >> >>> Signed-off-by: Jan Viktorin <viktorin@rehivetech.com>
> >> >>> [shreyansh.jain@nxp.com: Fix checkpatch error]
> >> >>> Signed-off-by: Shreyansh Jain <shreyansh.jain@nxp.com>
> >> >>> [jblunck@infradead.org: add type checking and __extension__]
> >> >>> Signed-off-by: Jan Blunck <jblunck@infradead.org>
> >> >>>
> >> >>> --
> >> >>> v2:
> >> >>>  - fix checkpatch error
> >> >>> ---
> >> >>>  lib/librte_eal/common/include/rte_common.h | 21 +++++++++++++++++++++
> >> >>>  1 file changed, 21 insertions(+)
> >> >>>
> >> >>> diff --git a/lib/librte_eal/common/include/rte_common.h
> >> >>> b/lib/librte_eal/common/include/rte_common.h
> >> >>> index db5ac91..3eb8d11 100644
> >> >>> --- a/lib/librte_eal/common/include/rte_common.h
> >> >>> +++ b/lib/librte_eal/common/include/rte_common.h
> >> >>> @@ -331,6 +331,27 @@ rte_bsf32(uint32_t v)
> >> >>>  #define offsetof(TYPE, MEMBER)  __builtin_offsetof (TYPE, MEMBER)
> >> >>>  #endif
> >> >>>
> >> >>> +/**
> >> >>> + * Return pointer to the wrapping struct instance.
> >> >>> + *
> >> >>> + * Example:
> >> >>> + *
> >> >>> + *  struct wrapper {
> >> >>> + *      ...
> >> >>> + *      struct child c;
> >> >>> + *      ...
> >> >>> + *  };
> >> >>> + *
> >> >>> + *  struct child *x = obtain(...);
> >> >>> + *  struct wrapper *w = container_of(x, struct wrapper, c);
> >> >>> + */
> >> >>> +#ifndef container_of
> >> >>> +#define container_of(ptr, type, member)        (__extension__  ({
> >> >>> \
> >> >>> +                       typeof(((type *)0)->member) * _ptr = (ptr);     \
> >> >>> +                       (type *)(((char *)_ptr) - offsetof(type,
> >> >>> member));\
> >> >>> +                       }))
> >> >>
> >> >>
> >> >> This is a checkpatch false positive. It should be fine to ignore this.
> >> >> IIRC we already discussed this before.
> >> >
> >> >
> >> > I too thought something similar was discussed. I tried searching the
> >> > archives but couldn't find anything - thus, I thought probably I was
> >> > hallucinating :P
> >> >
> >> > So, you want me to revert back the '()' change? Does it impact the expansion
> >> > of this macro?
> >>
> >> We haven't added this on any other usage of the __extension__ keyword
> >> in the existing code. From my perspective it is more consistent to
> >> revert it.
> >>
> >> Anyone else with an opinion here? David? Thomas?
> >
> > As an exported header, rte_common.h must pass check-includes.sh. Both
> > typeof() and the ({ ... }) construct are non-standard GCC extensions and
> > would fail to compile with pedantic options.
> >
> 
> Thanks Adrien. These extensions are already in use by rte_common.h and
> other headers. I don't believe we can remove the usage of typeof()
> that easily without making the code really ugly.

Sure, no problem with that, I misread and thought you wanted to drop
__extension__ as well.

Parentheses may perhaps cause more accurate warnings in case of syntax
errors. That is not significant so either way is fine by me.

-- 
Adrien Mazarguil
6WIND

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox