diff mbox

[dpdk-dev,v2,5/8] pci: use given name as generic name

Message ID 846173ac225560e48d665fb1d6d847919694d3e8.1499728330.git.gaetan.rivet@6wind.com (mailing list archive)
State Superseded, archived
Headers

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK

Commit Message

Gaëtan Rivet July 10, 2017, 11:19 p.m. UTC 
  The PCI device is referenced by other DPDK systems by the name of its
parameter, not by the system name.

Moreover, this name should be set once and for all, as early as
possible. This means linking the rte_devargs associated with the PCI
device during its scan, making available other metadatas used afterward
upon probing and device search for plugging.

Fixes: beec692c5157 ("eal: add name field to generic device")
Cc: stable@dpdk.org

Signed-off-by: Gaetan Rivet <gaetan.rivet@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c    |  4 ++--
 lib/librte_eal/common/eal_common_pci.c | 21 ++++++++++++++++-----
 lib/librte_eal/common/eal_private.h    |  5 +++++
 lib/librte_eal/linuxapp/eal/eal_pci.c  |  4 ++--
 4 files changed, 25 insertions(+), 9 deletions(-)

Comments

Thomas Monjalon July 11, 2017, 10:05 p.m. UTC | #1 
11/07/2017 01:19, Gaetan Rivet:
> The PCI device is referenced by other DPDK systems by the name of its
> parameter, not by the system name.

I don't understand this sentence. Please give an example.

> Moreover, this name should be set once and for all, as early as
> possible. This means linking the rte_devargs associated with the PCI
> device during its scan, making available other metadatas used afterward
> upon probing and device search for plugging.

Which other metadatas? Please give an example.

[...]
> +void
> +pci_name_set(struct rte_pci_device *dev)
> +{
> +	struct rte_devargs *devargs;
> +
> +	rte_pci_device_name(&dev->addr,
> +			dev->name, sizeof(dev->name));
> +	devargs = pci_devargs_lookup(dev);
> +	dev->device.devargs = devargs;
> +	if (devargs != NULL)
> +		dev->device.name = dev->device.devargs->name;
> +	else
> +		dev->device.name = dev->name;
> +}

I think this function deserves some comments.
The relation between rte_pci_device_name and pci_devargs_lookup
is not obvious.
Moreover, this function is not just setting the name:
it is also setting the devargs in the rte_device.
How dev->device.devargs->name is different from dev->name?
diff mbox

Patch

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index e321461..97a88ec 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -282,8 +282,7 @@  pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 	/* FreeBSD has no NUMA support (yet) */
 	dev->device.numa_node = 0;
 
-	rte_pci_device_name(&dev->addr, dev->name, sizeof(dev->name));
-	dev->device.name = dev->name;
+	pci_name_set(dev);
 
 	/* FreeBSD has only one pass through driver */
 	dev->kdrv = RTE_KDRV_NIC_UIO;
@@ -334,6 +333,7 @@  pci_scan_one(int dev_pci_fd, struct pci_conf *conf)
 			} else { /* already registered */
 				dev2->kdrv = dev->kdrv;
 				dev2->max_vfs = dev->max_vfs;
+				pci_name_set(dev2);
 				memmove(dev2->mem_resource,
 					dev->mem_resource,
 					sizeof(dev->mem_resource));
diff --git a/lib/librte_eal/common/eal_common_pci.c b/lib/librte_eal/common/eal_common_pci.c
index 76bbcc8..b100525 100644
--- a/lib/librte_eal/common/eal_common_pci.c
+++ b/lib/librte_eal/common/eal_common_pci.c
@@ -88,6 +88,21 @@  static struct rte_devargs *pci_devargs_lookup(struct rte_pci_device *dev)
 	return NULL;
 }
 
+void
+pci_name_set(struct rte_pci_device *dev)
+{
+	struct rte_devargs *devargs;
+
+	rte_pci_device_name(&dev->addr,
+			dev->name, sizeof(dev->name));
+	devargs = pci_devargs_lookup(dev);
+	dev->device.devargs = devargs;
+	if (devargs != NULL)
+		dev->device.name = dev->device.devargs->name;
+	else
+		dev->device.name = dev->name;
+}
+
 /* map a particular resource from a file */
 void *
 pci_map_resource(void *requested_addr, int fd, off_t offset, size_t size,
@@ -396,11 +411,7 @@  rte_pci_probe(void)
 	FOREACH_DEVICE_ON_PCIBUS(dev) {
 		probed++;
 
-		/* set devargs in PCI structure */
-		devargs = pci_devargs_lookup(dev);
-		if (devargs != NULL)
-			dev->device.devargs = devargs;
-
+		devargs = dev->device.devargs;
 		/* probe all or only whitelisted devices */
 		if (probe_all)
 			ret = pci_probe_all_drivers(dev);
diff --git a/lib/librte_eal/common/eal_private.h b/lib/librte_eal/common/eal_private.h
index 0836339..597d82e 100644
--- a/lib/librte_eal/common/eal_private.h
+++ b/lib/librte_eal/common/eal_private.h
@@ -113,6 +113,11 @@  struct rte_pci_driver;
 struct rte_pci_device;
 
 /**
+ * Find the name of a PCI device.
+ */
+void pci_name_set(struct rte_pci_device *dev);
+
+/**
  * Add a PCI device to the PCI Bus (append to PCI Device list). This function
  * also updates the bus references of the PCI Device (and the generic device
  * object embedded within.
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 7d9e1a9..556ae2c 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -324,8 +324,7 @@  pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 		dev->device.numa_node = 0;
 	}
 
-	rte_pci_device_name(addr, dev->name, sizeof(dev->name));
-	dev->device.name = dev->name;
+	pci_name_set(dev);
 
 	/* parse resources */
 	snprintf(filename, sizeof(filename), "%s/resource", dirname);
@@ -373,6 +372,7 @@  pci_scan_one(const char *dirname, const struct rte_pci_addr *addr)
 			} else { /* already registered */
 				dev2->kdrv = dev->kdrv;
 				dev2->max_vfs = dev->max_vfs;
+				pci_name_set(dev2);
 				memmove(dev2->mem_resource, dev->mem_resource,
 					sizeof(dev->mem_resource));
 				free(dev);