[PATCH v1 0/2] Bus find_device fixes

[PATCH v1 0/2] Bus find_device fixes

[Gaetan Rivet]

These patches are extracted from the device querying series [1]
for earlier integration.

[1]: https://dpdk.org/ml/archives/dev/2018-March/092891.html

Gaetan Rivet (2):
  bus/pci: fix find device implementation
  bus/vdev: fix find device implementation

 drivers/bus/pci/pci_common.c    | 21 ++++++++++++---------
 drivers/bus/pci/rte_bus_pci.h   |  3 +++
 drivers/bus/vdev/rte_bus_vdev.h |  3 +++
 drivers/bus/vdev/vdev.c         | 14 +++++++++-----
 4 files changed, 27 insertions(+), 14 deletions(-)

-- 
2.11.0

[PATCH v1 1/2] bus/pci: fix find device implementation

[Gaetan Rivet]

If start is set, and a device before it matches the data
passed for comparison, then this first device is returned.

This induces potentially infinite loops.

Fixes: c7fe1eea8a74 ("bus: simplify finding starting point")
Cc: stable@dpdk.org

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 drivers/bus/pci/pci_common.c  | 21 ++++++++++++---------
 drivers/bus/pci/rte_bus_pci.h |  3 +++
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 2a00f365a..2c45f8151 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -459,17 +459,20 @@ static struct rte_device *
 pci_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
 		const void *data)
 {
-	struct rte_pci_device *dev;
+	const struct rte_pci_device *pstart;
+	struct rte_pci_device *pdev;
 
-	FOREACH_DEVICE_ON_PCIBUS(dev) {
-		if (start && &dev->device == start) {
-			start = NULL; /* starting point found */
-			continue;
-		}
-		if (cmp(&dev->device, data) == 0)
-			return &dev->device;
+	if (start != NULL) {
+		pstart = RTE_DEV_TO_PCI_CONST(start);
+		pdev = TAILQ_NEXT(pstart, next);
+	} else {
+		pdev = TAILQ_FIRST(&rte_pci_bus.device_list);
+	}
+	while (pdev != NULL) {
+		if (cmp(&pdev->device, data) == 0)
+			return &pdev->device;
+		pdev = TAILQ_NEXT(pdev, next);
 	}
-
 	return NULL;
 }
 
diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h
index 357afb912..458e6d076 100644
--- a/drivers/bus/pci/rte_bus_pci.h
+++ b/drivers/bus/pci/rte_bus_pci.h
@@ -74,6 +74,9 @@ struct rte_pci_device {
  */
 #define RTE_DEV_TO_PCI(ptr) container_of(ptr, struct rte_pci_device, device)
 
+#define RTE_DEV_TO_PCI_CONST(ptr) \
+	container_of(ptr, const struct rte_pci_device, device)
+
 #define RTE_ETH_DEV_TO_PCI(eth_dev)	RTE_DEV_TO_PCI((eth_dev)->device)
 
 /** Any PCI device identifier (vendor, device, ...) */
-- 
2.11.0

[PATCH v1 2/2] bus/vdev: fix find device implementation

[Gaetan Rivet]

If start is set and a device before it matches the data,
this device is returned.

This induces potentially infinite loops.

Fixes: c7fe1eea8a74 ("bus: simplify finding starting point")
Cc: stable@dpdk.org

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 drivers/bus/vdev/rte_bus_vdev.h |  3 +++
 drivers/bus/vdev/vdev.c         | 14 +++++++++-----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/bus/vdev/rte_bus_vdev.h b/drivers/bus/vdev/rte_bus_vdev.h
index 646d6c090..f9b5eb596 100644
--- a/drivers/bus/vdev/rte_bus_vdev.h
+++ b/drivers/bus/vdev/rte_bus_vdev.h
@@ -25,6 +25,9 @@ struct rte_vdev_device {
 #define RTE_DEV_TO_VDEV(ptr) \
 	container_of(ptr, struct rte_vdev_device, device)
 
+#define RTE_DEV_TO_VDEV_CONST(ptr) \
+	container_of(ptr, const struct rte_vdev_device, device)
+
 static inline const char *
 rte_vdev_device_name(const struct rte_vdev_device *dev)
 {
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index f8dd1f5e6..c135554c0 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -358,15 +358,19 @@ static struct rte_device *
 vdev_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
 		 const void *data)
 {
+	const struct rte_vdev_device *vstart;
 	struct rte_vdev_device *dev;
 
-	TAILQ_FOREACH(dev, &vdev_device_list, next) {
-		if (start && &dev->device == start) {
-			start = NULL;
-			continue;
-		}
+	if (start != NULL) {
+		vstart = RTE_DEV_TO_VDEV_CONST(start);
+		dev = TAILQ_NEXT(vstart, next);
+	} else {
+		dev = TAILQ_FIRST(&vdev_device_list);
+	}
+	while (dev != NULL) {
 		if (cmp(&dev->device, data) == 0)
 			return &dev->device;
+		dev = TAILQ_NEXT(dev, next);
 	}
 	return NULL;
 }
-- 
2.11.0

[PATCH v2 0/2] Bus find_device fixes

[Gaetan Rivet]

These patches are extracted from the device querying series [1]
for earlier integration.

[1]: https://dpdk.org/ml/archives/dev/2018-March/092891.html

v2:

  * Rebase on master

Gaetan Rivet (2):
  bus/pci: fix find device implementation
  bus/vdev: fix find device implementation

 drivers/bus/pci/pci_common.c    | 21 ++++++++++++---------
 drivers/bus/pci/rte_bus_pci.h   |  3 +++
 drivers/bus/vdev/rte_bus_vdev.h |  3 +++
 drivers/bus/vdev/vdev.c         | 14 +++++++++-----
 4 files changed, 27 insertions(+), 14 deletions(-)

-- 
2.11.0

[PATCH v2 1/2] bus/pci: fix find device implementation

[Gaetan Rivet]

If start is set, and a device before it matches the data
passed for comparison, then this first device is returned.

This induces potentially infinite loops.

Fixes: c7fe1eea8a74 ("bus: simplify finding starting point")
Cc: stable@dpdk.org

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 drivers/bus/pci/pci_common.c  | 21 ++++++++++++---------
 drivers/bus/pci/rte_bus_pci.h |  3 +++
 2 files changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 6bed0bc9d..7215aaec3 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -455,17 +455,20 @@ static struct rte_device *
 pci_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
 		const void *data)
 {
-	struct rte_pci_device *dev;
+	const struct rte_pci_device *pstart;
+	struct rte_pci_device *pdev;
 
-	FOREACH_DEVICE_ON_PCIBUS(dev) {
-		if (start && &dev->device == start) {
-			start = NULL; /* starting point found */
-			continue;
-		}
-		if (cmp(&dev->device, data) == 0)
-			return &dev->device;
+	if (start != NULL) {
+		pstart = RTE_DEV_TO_PCI_CONST(start);
+		pdev = TAILQ_NEXT(pstart, next);
+	} else {
+		pdev = TAILQ_FIRST(&rte_pci_bus.device_list);
+	}
+	while (pdev != NULL) {
+		if (cmp(&pdev->device, data) == 0)
+			return &pdev->device;
+		pdev = TAILQ_NEXT(pdev, next);
 	}
-
 	return NULL;
 }
 
diff --git a/drivers/bus/pci/rte_bus_pci.h b/drivers/bus/pci/rte_bus_pci.h
index 357afb912..458e6d076 100644
--- a/drivers/bus/pci/rte_bus_pci.h
+++ b/drivers/bus/pci/rte_bus_pci.h
@@ -74,6 +74,9 @@ struct rte_pci_device {
  */
 #define RTE_DEV_TO_PCI(ptr) container_of(ptr, struct rte_pci_device, device)
 
+#define RTE_DEV_TO_PCI_CONST(ptr) \
+	container_of(ptr, const struct rte_pci_device, device)
+
 #define RTE_ETH_DEV_TO_PCI(eth_dev)	RTE_DEV_TO_PCI((eth_dev)->device)
 
 /** Any PCI device identifier (vendor, device, ...) */
-- 
2.11.0

[PATCH v2 2/2] bus/vdev: fix find device implementation

[Gaetan Rivet]

If start is set and a device before it matches the data,
this device is returned.

This induces potentially infinite loops.

Fixes: c7fe1eea8a74 ("bus: simplify finding starting point")
Cc: stable@dpdk.org

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 drivers/bus/vdev/rte_bus_vdev.h |  3 +++
 drivers/bus/vdev/vdev.c         | 14 +++++++++-----
 2 files changed, 12 insertions(+), 5 deletions(-)

diff --git a/drivers/bus/vdev/rte_bus_vdev.h b/drivers/bus/vdev/rte_bus_vdev.h
index 646d6c090..f9b5eb596 100644
--- a/drivers/bus/vdev/rte_bus_vdev.h
+++ b/drivers/bus/vdev/rte_bus_vdev.h
@@ -25,6 +25,9 @@ struct rte_vdev_device {
 #define RTE_DEV_TO_VDEV(ptr) \
 	container_of(ptr, struct rte_vdev_device, device)
 
+#define RTE_DEV_TO_VDEV_CONST(ptr) \
+	container_of(ptr, const struct rte_vdev_device, device)
+
 static inline const char *
 rte_vdev_device_name(const struct rte_vdev_device *dev)
 {
diff --git a/drivers/bus/vdev/vdev.c b/drivers/bus/vdev/vdev.c
index 84fa51f98..9f576eb80 100644
--- a/drivers/bus/vdev/vdev.c
+++ b/drivers/bus/vdev/vdev.c
@@ -495,16 +495,20 @@ static struct rte_device *
 vdev_find_device(const struct rte_device *start, rte_dev_cmp_t cmp,
 		 const void *data)
 {
+	const struct rte_vdev_device *vstart;
 	struct rte_vdev_device *dev;
 
 	rte_spinlock_lock(&vdev_device_list_lock);
-	TAILQ_FOREACH(dev, &vdev_device_list, next) {
-		if (start && &dev->device == start) {
-			start = NULL;
-			continue;
-		}
+	if (start != NULL) {
+		vstart = RTE_DEV_TO_VDEV_CONST(start);
+		dev = TAILQ_NEXT(vstart, next);
+	} else {
+		dev = TAILQ_FIRST(&vdev_device_list);
+	}
+	while (dev != NULL) {
 		if (cmp(&dev->device, data) == 0)
 			break;
+		dev = TAILQ_NEXT(dev, next);
 	}
 	rte_spinlock_unlock(&vdev_device_list_lock);
 
-- 
2.11.0

Re: [PATCH v2 0/2] Bus find_device fixes

[Thomas Monjalon]

> Gaetan Rivet (2):
>   bus/pci: fix find device implementation
>   bus/vdev: fix find device implementation

Applied, thanks