From: Shani Peretz <shperetz@nvidia.com>
To: <dev@dpdk.org>
Cc: <shperetz@nvidia.com>, <mkashani@nvidia.com>,
<jblunck@infradead.org>,
Dariusz Sosnowski <dsosnowski@nvidia.com>,
Thomas Monjalon <thomas@monjalon.net>,
Chenbo Xia <chenbox@nvidia.com>,
Nipun Gupta <nipun.gupta@amd.com>,
Tyler Retzlaff <roretzla@linux.microsoft.com>
Subject: [PATCH] eal/common: fix inconsistent representation of PCI numbers
Date: Mon, 1 Jul 2024 23:01:17 +0300 [thread overview]
Message-ID: <20240701200117.6349-1-shperetz@nvidia.com> (raw)
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
next reply other threads:[~2024-07-01 20:01 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-07-01 20:01 Shani Peretz [this message]
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
2024-10-04 22:21 ` [PATCH] " Stephen Hemminger
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20240701200117.6349-1-shperetz@nvidia.com \
--to=shperetz@nvidia.com \
--cc=chenbox@nvidia.com \
--cc=dev@dpdk.org \
--cc=dsosnowski@nvidia.com \
--cc=jblunck@infradead.org \
--cc=mkashani@nvidia.com \
--cc=nipun.gupta@amd.com \
--cc=roretzla@linux.microsoft.com \
--cc=thomas@monjalon.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).