[PATCH v6 1/4] bus/pci: fix registration of PCI device

[PATCH v6 1/4] bus/pci: fix registration of PCI device

[Shani Peretz]

When registering a new PCI device, the device->name field stored
the user-provided string from devargs (e.g., "08:00.0" or "0000:08:00.0").
This approach led to inconsistencies when registering new devices.

This patch fix this issue by saving the parsed PCI in device->name,
so when a new PCI device is registering the name displayed in the device
list will be the parsed version.

Fixes: 23eaa9059ec2 ("bus/pci: use given name as generic name")

Cc: stable@dpdk.org
Signed-off-by: Shani Peretz <shperetz@nvidia.com>
---
 drivers/bus/pci/pci_common.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 1173f0887c..70faae4e44 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -99,21 +99,11 @@ pci_common_set(struct rte_pci_device *dev)
 	/* Each device has its internal, canonical name set. */
 	rte_pci_device_name(&dev->addr,
 			dev->name, sizeof(dev->name));
+	dev->device.name = dev->name;
+
 	devargs = pci_devargs_lookup(&dev->addr);
 	dev->device.devargs = devargs;
 
-	/* When using a blocklist, only blocked devices will have
-	 * an rte_devargs. Allowed devices won't have one.
-	 */
-	if (devargs != NULL)
-		/* If an rte_devargs exists, the generic rte_device uses the
-		 * given name as its name.
-		 */
-		dev->device.name = dev->device.devargs->name;
-	else
-		/* Otherwise, it uses the internal, canonical form. */
-		dev->device.name = dev->name;
-
 	if (dev->bus_info != NULL ||
 			asprintf(&dev->bus_info, "vendor_id=%"PRIx16", device_id=%"PRIx16,
 				dev->id.vendor_id, dev->id.device_id) != -1)
-- 
2.34.1

Re: [PATCH v6 1/4] bus/pci: fix registration of PCI device

[Bruce Richardson]

On Thu, Feb 06, 2025 at 12:54:24PM +0200, Shani Peretz wrote:
> When registering a new PCI device, the device->name field stored
> the user-provided string from devargs (e.g., "08:00.0" or "0000:08:00.0").
> This approach led to inconsistencies when registering new devices.
> 
> This patch fix this issue by saving the parsed PCI in device->name,
> so when a new PCI device is registering the name displayed in the device
> list will be the parsed version.
> 
> Fixes: 23eaa9059ec2 ("bus/pci: use given name as generic name")
> 
> Cc: stable@dpdk.org
> Signed-off-by: Shani Peretz <shperetz@nvidia.com>
> ---
>  drivers/bus/pci/pci_common.c | 14 ++------------
>  1 file changed, 2 insertions(+), 12 deletions(-)
> 
Tested-by: Bruce Richardson <bruce.richardson@intel.com>

Tested with running testpmd using -a 27:00.0 -a 27:00.1 and then using
dpdk-proc-info with the same ports prefixed with 0000:. Without this patch,
that gave an error. With this patch, output was the same with and without
the extra prefix.
Also tested the reverse case, running testpmd with the domain prefix, and
proc-info without, that too worked in testing.

/Bruce

Re: [PATCH v6 1/4] bus/pci: fix registration of PCI device

[Stephen Hemminger]

On Thu, 6 Feb 2025 12:54:24 +0200
Shani Peretz <shperetz@nvidia.com> wrote:

> When registering a new PCI device, the device->name field stored
> the user-provided string from devargs (e.g., "08:00.0" or "0000:08:00.0").
> This approach led to inconsistencies when registering new devices.
> 
> This patch fix this issue by saving the parsed PCI in device->name,
> so when a new PCI device is registering the name displayed in the device
> list will be the parsed version.
> 
> Fixes: 23eaa9059ec2 ("bus/pci: use given name as generic name")
> 
> Cc: stable@dpdk.org
> Signed-off-by: Shani Peretz <shperetz@nvidia.com>

This part looks good, second part needs work.

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

[PATCH v7 1/4] bus/pci: fix registration of PCI device

[Shani Peretz]

When registering a new PCI device, the device->name field stored
the user-provided string from devargs (e.g., "08:00.0" or "0000:08:00.0").
This approach led to inconsistencies when registering new devices.

This patch fix this issue by saving the parsed PCI in device->name,
so when a new PCI device is registering the name displayed in the device
list will be the parsed version.

Fixes: 23eaa9059ec2 ("bus/pci: use given name as generic name")

Cc: stable@dpdk.org
Signed-off-by: Shani Peretz <shperetz@nvidia.com>
---
 drivers/bus/pci/pci_common.c | 14 ++------------
 1 file changed, 2 insertions(+), 12 deletions(-)

diff --git a/drivers/bus/pci/pci_common.c b/drivers/bus/pci/pci_common.c
index 1173f0887c..70faae4e44 100644
--- a/drivers/bus/pci/pci_common.c
+++ b/drivers/bus/pci/pci_common.c
@@ -99,21 +99,11 @@ pci_common_set(struct rte_pci_device *dev)
 	/* Each device has its internal, canonical name set. */
 	rte_pci_device_name(&dev->addr,
 			dev->name, sizeof(dev->name));
+	dev->device.name = dev->name;
+
 	devargs = pci_devargs_lookup(&dev->addr);
 	dev->device.devargs = devargs;
 
-	/* When using a blocklist, only blocked devices will have
-	 * an rte_devargs. Allowed devices won't have one.
-	 */
-	if (devargs != NULL)
-		/* If an rte_devargs exists, the generic rte_device uses the
-		 * given name as its name.
-		 */
-		dev->device.name = dev->device.devargs->name;
-	else
-		/* Otherwise, it uses the internal, canonical form. */
-		dev->device.name = dev->name;
-
 	if (dev->bus_info != NULL ||
 			asprintf(&dev->bus_info, "vendor_id=%"PRIx16", device_id=%"PRIx16,
 				dev->id.vendor_id, dev->id.device_id) != -1)
-- 
2.34.1

Re: [PATCH v7 1/4] bus/pci: fix registration of PCI device

[Stephen Hemminger]

On Wed, 12 Feb 2025 18:38:32 +0200
Shani Peretz <shperetz@nvidia.com> wrote:

> When registering a new PCI device, the device->name field stored
> the user-provided string from devargs (e.g., "08:00.0" or "0000:08:00.0").
> This approach led to inconsistencies when registering new devices.
> 
> This patch fix this issue by saving the parsed PCI in device->name,
> so when a new PCI device is registering the name displayed in the device
> list will be the parsed version.
> 
> Fixes: 23eaa9059ec2 ("bus/pci: use given name as generic name")
> 
> Cc: stable@dpdk.org
> Signed-off-by: Shani Peretz <shperetz@nvidia.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH v7 1/4] bus/pci: fix registration of PCI device

[Stephen Hemminger]

On Wed, 12 Feb 2025 18:38:32 +0200
Shani Peretz <shperetz@nvidia.com> wrote:

> When registering a new PCI device, the device->name field stored
> the user-provided string from devargs (e.g., "08:00.0" or "0000:08:00.0").
> This approach led to inconsistencies when registering new devices.
> 
> This patch fix this issue by saving the parsed PCI in device->name,
> so when a new PCI device is registering the name displayed in the device
> list will be the parsed version.
> 
> Fixes: 23eaa9059ec2 ("bus/pci: use given name as generic name")
> 
> Cc: stable@dpdk.org
> Signed-off-by: Shani Peretz <shperetz@nvidia.com>

Acked-by: Stephen Hemminger <stephen@networkplumber.org>

Re: [PATCH v7 1/4] bus/pci: fix registration of PCI device

[Stephen Hemminger]

On Wed, 12 Feb 2025 18:38:32 +0200
Shani Peretz <shperetz@nvidia.com> wrote:

> When registering a new PCI device, the device->name field stored
> the user-provided string from devargs (e.g., "08:00.0" or "0000:08:00.0").
> This approach led to inconsistencies when registering new devices.
> 
> This patch fix this issue by saving the parsed PCI in device->name,
> so when a new PCI device is registering the name displayed in the device
> list will be the parsed version.
> 
> Fixes: 23eaa9059ec2 ("bus/pci: use given name as generic name")
> 
> Cc: stable@dpdk.org
> Signed-off-by: Shani Peretz <shperetz@nvidia.com>

Is there a bugzilla entry for this?
Would like to be able to see what exactly was broken.

The PCI name thing goes back to 17.01 release where this was added.
Did something regress? If so why is there no test for this?

commit 23eaa9059ec24e95e32361f333ed0686f82bea74
Author: Gaetan Rivet <grive@u256.net>
Date:   Sat Jul 15 19:56:39 2017 +0200

    bus/pci: use given name as generic name
    
    When an application requests the use of a PCI device, it can currently
    interchangeably use either the longform DomBDF format (0000:00:00.0) or
    the shorter BDF format (00:00.0).
    
    When a device is inserted via the hotplug API, it must first be scanned
    and then will be identified by its name using `find_device`. The name of
    the device must match the name given by the user to be found and then
    probed.
    
    A new function sets the expected name for a scanned PCI device. It was
    previously generated from parsing the PCI address. This canonical name
    is superseded when an rte_devargs exists describing the device. In such
    case, the device takes the given name found within the rte_devargs.
    
    As the rte_devargs is linked to the rte_pci_device during scanning, it
    can be avoided during the probe. Additionally, this fixes the issue of
    the rte_devargs lookup not being done within rte_pci_probe_one.
    
    Fixes: beec692c5157 ("eal: add name field to generic device")
    Cc: stable@dpdk.org
    
    Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>