All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] eal/common: fix inconsistent representation of PCI numbers
@ 2024-07-01 20:01 Shani Peretz
  2024-07-01 22:00 ` Stephen Hemminger
                   ` (2 more replies)
  0 siblings, 3 replies; 60+ messages in thread
From: Shani Peretz @ 2024-07-01 20:01 UTC (permalink / raw)
  To: dev
  Cc: shperetz, mkashani, jblunck, Dariusz Sosnowski, Thomas Monjalon,
	Chenbo Xia, Nipun Gupta, Tyler Retzlaff

DPDK allows for two ways to specify PCI device numbers:
a full version ("0000:08:00.0") and a short version ("08:00.0").
The problem arises when the application uses one format (e.g., full)
when running testpmd, but then tries to use the other format (e.g., short)
in a subsequent command, leading to a failure.

The cmp_dev_name func, which is responsible for comparing PCI device names,
is not handling the inconsistent PCI number representations correctly.
The suggested fix is to use the pci_parse function, which can parse
the PCI device name and fill a struct rte_pci_addr with the standardized
representation of the PCI number.
By comparing the struct rte_pci_addr instances instead of the string
representations, the application can ensure consistent handling of
PCI device numbers, regardless of the format used.

Fixes: a3ee360f4440 ("eal: add hotplug add/remove device")
Cc: jblunck@infradead.org

Signed-off-by: Shani Peretz <shperetz@nvidia.com>
Acked-by: Dariusz Sosnowski <dsosnowski@nvidia.com>
Acked-by: Thomas Monjalon <thomas@monjalon.net>
---
 app/test/test_vdev.c            | 10 ++--------
 drivers/bus/pci/pci_common.c    | 10 ++++++++++
 lib/eal/common/eal_common_dev.c | 11 ++++++-----
 lib/eal/common/hotplug_mp.c     | 11 ++---------
 lib/eal/include/bus_driver.h    | 18 ++++++++++++++++++
 lib/eal/include/rte_dev.h       | 16 ++++++++++++++++
 lib/eal/linux/eal_dev.c         | 10 +---------
 lib/eal/version.map             |  3 +++
 8 files changed, 58 insertions(+), 31 deletions(-)

diff --git a/app/test/test_vdev.c b/app/test/test_vdev.c
index 3e262f30bc..860fa260af 100644
--- a/app/test/test_vdev.c
+++ b/app/test/test_vdev.c
@@ -20,12 +20,6 @@ static const char * const valid_keys[] = {
 	NULL,
 };
 
-static int
-cmp_dev_name(const struct rte_device *dev, const void *name)
-{
-	return strcmp(rte_dev_name(dev), name);
-}
-
 static int
 cmp_dev_match(const struct rte_device *dev, const void *_kvlist)
 {
@@ -82,7 +76,7 @@ test_vdev_bus(void)
 		printf("Failed to create vdev net_null_test0\n");
 		goto fail;
 	}
-	dev0 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test0");
+	dev0 = vdev_bus->find_device(NULL, rte_cmp_dev_name, "net_null_test0");
 	if (dev0 == NULL) {
 		printf("Cannot find net_null_test0 vdev\n");
 		goto fail;
@@ -93,7 +87,7 @@ test_vdev_bus(void)
 		printf("Failed to create vdev net_null_test1\n");
 		goto fail;
 	}
-	dev1 = vdev_bus->find_device(NULL, cmp_dev_name, "net_null_test1");
+	dev1 = vdev_bus->find_device(NULL, rte_cmp_dev_name, "net_null_test1");
 	if (dev1 == NULL) {
 		printf("Cannot find net_null_test1 vdev\n");
 		goto fail;
diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 889a48d2af..6b3bf070ae 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -501,6 +501,15 @@ rte_pci_dump(FILE *f)
 		pci_dump_one_device(f, dev);
 	}
 }
+static int
+pci_cmp_name(const struct rte_device *dev1, const void *name2)
+{
+	const struct rte_pci_device *dev = RTE_DEV_TO_PCI_CONST(dev1);
+	char name2_addr[sizeof(struct rte_pci_addr)] = {0};
+	dev1->bus->parse(name2, (void *)&name2_addr);
+
+	return memcmp(&name2_addr, &(dev->addr), sizeof(struct rte_pci_addr));
+}
 
 static int
 pci_parse(const char *name, void *addr)
@@ -956,6 +965,7 @@ struct rte_pci_bus rte_pci_bus = {
 		.plug = pci_plug,
 		.unplug = pci_unplug,
 		.parse = pci_parse,
+		.cmp_name = pci_cmp_name,
 		.devargs_parse = rte_pci_devargs_parse,
 		.dma_map = pci_dma_map,
 		.dma_unmap = pci_dma_unmap,
diff --git a/lib/eal/common/eal_common_dev.c b/lib/eal/common/eal_common_dev.c
index a99252b02f..12d68c3605 100644
--- a/lib/eal/common/eal_common_dev.c
+++ b/lib/eal/common/eal_common_dev.c
@@ -107,11 +107,12 @@ struct dev_next_ctx {
 #define CLSCTX(ptr) \
 	(((struct dev_next_ctx *)(intptr_t)ptr)->cls_str)
 
-static int cmp_dev_name(const struct rte_device *dev, const void *_name)
+int rte_cmp_dev_name(const struct rte_device *dev1, const void *name2)
 {
-	const char *name = _name;
+	if (dev1->bus->cmp_name)
+		return dev1->bus->cmp_name(dev1, name2);
 
-	return strcmp(dev->name, name);
+	return strcmp(dev1->name, (const char *)name2);
 }
 
 int
@@ -197,7 +198,7 @@ local_dev_probe(const char *devargs, struct rte_device **new_dev)
 	if (ret)
 		goto err_devarg;
 
-	dev = da->bus->find_device(NULL, cmp_dev_name, da->name);
+	dev = da->bus->find_device(NULL, rte_cmp_dev_name, da->name);
 	if (dev == NULL) {
 		EAL_LOG(ERR, "Cannot find device (%s)",
 			da->name);
@@ -335,7 +336,7 @@ rte_eal_hotplug_remove(const char *busname, const char *devname)
 		return -ENOENT;
 	}
 
-	dev = bus->find_device(NULL, cmp_dev_name, devname);
+	dev = bus->find_device(NULL, rte_cmp_dev_name, devname);
 	if (dev == NULL) {
 		EAL_LOG(ERR, "Cannot find plugged device (%s)", devname);
 		return -EINVAL;
diff --git a/lib/eal/common/hotplug_mp.c b/lib/eal/common/hotplug_mp.c
index 17089ca3db..a2623c96c3 100644
--- a/lib/eal/common/hotplug_mp.c
+++ b/lib/eal/common/hotplug_mp.c
@@ -21,13 +21,6 @@ struct mp_reply_bundle {
 	void *peer;
 };
 
-static int cmp_dev_name(const struct rte_device *dev, const void *_name)
-{
-	const char *name = _name;
-
-	return strcmp(dev->name, name);
-}
-
 /**
  * Secondary to primary request.
  * start from function eal_dev_hotplug_request_to_primary.
@@ -135,7 +128,7 @@ __handle_secondary_request(void *param)
 			goto finish;
 		}
 
-		dev = bus->find_device(NULL, cmp_dev_name, da.name);
+		dev = bus->find_device(NULL, rte_cmp_dev_name, da.name);
 		if (dev == NULL) {
 			EAL_LOG(ERR, "Cannot find plugged device (%s)", da.name);
 			ret = -ENOENT;
@@ -262,7 +255,7 @@ static void __handle_primary_request(void *param)
 			goto quit;
 		}
 
-		dev = bus->find_device(NULL, cmp_dev_name, da->name);
+		dev = bus->find_device(NULL, rte_cmp_dev_name, da->name);
 		if (dev == NULL) {
 			EAL_LOG(ERR, "Cannot find plugged device (%s)", da->name);
 			ret = -ENOENT;
diff --git a/lib/eal/include/bus_driver.h b/lib/eal/include/bus_driver.h
index 7b85a17a09..eba820f36c 100644
--- a/lib/eal/include/bus_driver.h
+++ b/lib/eal/include/bus_driver.h
@@ -118,6 +118,23 @@ typedef int (*rte_bus_unplug_t)(struct rte_device *dev);
  */
 typedef int (*rte_bus_parse_t)(const char *name, void *addr);
 
+/**
+ * Bus specific device name comparison function.
+ *
+ * This type of function is used to compare a bus name with an arbitrary
+ * name.
+ *
+ * @param dev
+ *	Device handle.
+ *
+ * @param name
+ *	Name to compare against.
+ *
+ * @return
+ *	0 if the device matches the name. Nonzero otherwise.
+ */
+typedef int (*rte_bus_cmp_name_t)(const struct rte_device *dev, const void *name);
+
 /**
  * Parse bus part of the device arguments.
  *
@@ -258,6 +275,7 @@ struct rte_bus {
 	rte_bus_plug_t plug;         /**< Probe single device for drivers */
 	rte_bus_unplug_t unplug;     /**< Remove single device from driver */
 	rte_bus_parse_t parse;       /**< Parse a device name */
+	rte_bus_cmp_name_t cmp_name; /**< Compare device name */
 	rte_bus_devargs_parse_t devargs_parse; /**< Parse bus devargs */
 	rte_dev_dma_map_t dma_map;   /**< DMA map for device in the bus */
 	rte_dev_dma_unmap_t dma_unmap; /**< DMA unmap for device in the bus */
diff --git a/lib/eal/include/rte_dev.h b/lib/eal/include/rte_dev.h
index cefa04f905..a3d666c110 100644
--- a/lib/eal/include/rte_dev.h
+++ b/lib/eal/include/rte_dev.h
@@ -21,6 +21,7 @@ extern "C" {
 
 #include <rte_config.h>
 #include <rte_common.h>
+#include <rte_compat.h>
 #include <rte_log.h>
 
 struct rte_bus;
@@ -170,6 +171,21 @@ int rte_dev_is_probed(const struct rte_device *dev);
 int rte_eal_hotplug_add(const char *busname, const char *devname,
 			const char *drvargs);
 
+/**
+ * General device name comparison. Will compare by using the specific bus
+ * compare function or by comparing the names directly.
+ *
+ * @param dev
+ *   Device handle.
+ * @param name
+ *   Name to compare against.
+ * @return
+ *   0 if the device matches the name. Nonzero otherwise.
+ */
+__rte_experimental
+int
+rte_cmp_dev_name(const struct rte_device *dev, const void *name);
+
 /**
  * Add matching devices.
  *
diff --git a/lib/eal/linux/eal_dev.c b/lib/eal/linux/eal_dev.c
index fff4e8fa83..5c8effd7be 100644
--- a/lib/eal/linux/eal_dev.c
+++ b/lib/eal/linux/eal_dev.c
@@ -91,14 +91,6 @@ static void sigbus_handler(int signum, siginfo_t *info,
 	EAL_LOG(DEBUG, "Success to handle SIGBUS for hot-unplug!");
 }
 
-static int cmp_dev_name(const struct rte_device *dev,
-	const void *_name)
-{
-	const char *name = _name;
-
-	return strcmp(dev->name, name);
-}
-
 static int
 dev_uev_socket_fd_create(void)
 {
@@ -280,7 +272,7 @@ dev_uev_handler(__rte_unused void *param)
 				goto failure_handle_err;
 			}
 
-			dev = bus->find_device(NULL, cmp_dev_name,
+			dev = bus->find_device(NULL, rte_cmp_dev_name,
 					       uevent.devname);
 			if (dev == NULL) {
 				EAL_LOG(ERR, "Cannot find device (%s) on "
diff --git a/lib/eal/version.map b/lib/eal/version.map
index 3df50c3fbb..e4355ef890 100644
--- a/lib/eal/version.map
+++ b/lib/eal/version.map
@@ -396,6 +396,9 @@ EXPERIMENTAL {
 
 	# added in 24.03
 	rte_vfio_get_device_info; # WINDOWS_NO_EXPORT
+
+	# added in 24.07
+	rte_cmp_dev_name;
 };
 
 INTERNAL {
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 60+ messages in thread

end of thread, other threads:[~2025-07-22  4:04 UTC | newest]

Thread overview: 60+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-07-01 20:01 [PATCH] eal/common: fix inconsistent representation of PCI numbers Shani Peretz
2024-07-01 22:00 ` Stephen Hemminger
2024-07-08 16:51 ` [PATCH v3] " Shani Peretz
2024-07-12 13:49   ` David Marchand
2024-07-12 17:55     ` Thomas Monjalon
2025-01-29  8:54   ` [PATCH v4] bus: " Shani Peretz
2025-01-29  9:45     ` Bruce Richardson
2025-01-29 16:25     ` Stephen Hemminger
2025-02-05 16:36       ` Shani Peretz
2025-02-05 16:42         ` Stephen Hemminger
2025-02-05 17:37           ` Shani Peretz
2025-02-05 18:46             ` Stephen Hemminger
2025-02-05 20:16               ` Shani Peretz
2025-02-06  0:40                 ` Stephen Hemminger
2025-01-29 17:17     ` Stephen Hemminger
2025-01-29 18:06       ` Bruce Richardson
2025-02-05  1:55         ` fengchengwen
2025-02-06  0:08     ` [PATCH v5 0/4] fix comparison between devices Shani Peretz
2025-02-06  0:08       ` [PATCH v5 1/4] bus/pci: fix registration of PCI device Shani Peretz
2025-02-06 11:22         ` Thomas Monjalon
2025-02-06  0:08       ` [PATCH v5 2/4] lib: fix comparison between devices Shani Peretz
2025-02-06  7:55         ` Hemant Agrawal
2025-02-06 11:25         ` Thomas Monjalon
2025-02-10  1:18         ` Xu, Rosen
2025-02-11 17:48         ` Stephen Hemminger
2025-02-11 17:54           ` Bruce Richardson
2025-02-11 18:04             ` Stephen Hemminger
2025-02-19 13:26               ` Shani Peretz
2025-02-06  0:08       ` [PATCH v5 3/4] app/test: add tests to find devices Shani Peretz
2025-02-06  1:03         ` Stephen Hemminger
2025-02-06  0:08       ` [PATCH v5 4/4] lib: change find device and cmp dev name functions Shani Peretz
2025-02-06 10:54         ` [PATCH v6 1/4] bus/pci: fix registration of PCI device Shani Peretz
2025-02-06 10:54           ` [PATCH v6 2/4] lib: fix comparison between devices Shani Peretz
2025-02-06 10:54           ` [PATCH v6 3/4] app/test: add tests to find devices Shani Peretz
2025-02-06 10:54           ` [PATCH v6 4/4] lib: change find device and cmp dev name functions Shani Peretz
2025-02-11 17:04           ` [PATCH v6 1/4] bus/pci: fix registration of PCI device Bruce Richardson
2025-02-12  0:39           ` Stephen Hemminger
2025-02-12 16:38           ` [PATCH v7 " Shani Peretz
2025-02-12 16:38             ` [PATCH v7 2/4] lib: fix comparison between devices Shani Peretz
2025-02-19 16:50               ` Stephen Hemminger
2025-02-20 18:33               ` Stephen Hemminger
2025-03-06 16:26                 ` Shani Peretz
2025-03-17 14:11                   ` Stephen Hemminger
2025-04-30 11:12                     ` Shani Peretz
2025-04-30 15:59                       ` Stephen Hemminger
2025-04-30 17:17                         ` Shani Peretz
2025-07-08  8:37               ` [PATCH v8 0/1] fix inconsistent representation of PCI device name Shani Peretz
2025-07-08  8:37                 ` [PATCH v8 1/1] app/testpmd: canonicalize short PCI name format Shani Peretz
2025-07-09 17:38                   ` Stephen Hemminger
2025-07-13  6:34                   ` [PATCH v9 0/1] fix inconsistent representation of PCI device name Shani Peretz
2025-07-13  6:34                     ` [PATCH v9 1/1] app/testpmd: canonicalize short PCI name format Shani Peretz
2025-07-22  4:00                       ` Stephen Hemminger
2025-02-12 16:38             ` [PATCH v7 3/4] app/test: add tests to find devices Shani Peretz
2025-02-19 16:47               ` Stephen Hemminger
2025-04-02 14:22               ` Stephen Hemminger
2025-02-12 16:38             ` [PATCH v7 4/4] lib: change find device and cmp dev name functions Shani Peretz
2025-02-19 16:44             ` [PATCH v7 1/4] bus/pci: fix registration of PCI device Stephen Hemminger
2025-02-19 16:48             ` Stephen Hemminger
2025-02-24 20:38             ` Stephen Hemminger
2024-10-04 22:21 ` [PATCH] eal/common: fix inconsistent representation of PCI numbers Stephen Hemminger

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.