DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v2 0/7] pci cleanup
@ 2014-05-09 13:15 David Marchand
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 1/7] pci: fix potential mem leaks David Marchand
                   ` (7 more replies)
  0 siblings, 8 replies; 9+ messages in thread
From: David Marchand @ 2014-05-09 13:15 UTC (permalink / raw)
  To: dev

Hello all, 

Here is an attempt at having an equal implementation in bsd and linux eal_pci.c.
It results in following changes :
- checks on driver flag in bsd which were missing
- remove virtio-uio workaround in linux eal_pci.c
- remove deprecated RTE_EAL_UNBIND_PORTS option

Along the way, I discovered two small bugs: a mem leak in linux eal_pci.c and a
fd leak in both bsd and linux eal_pci.c.

Changes included in v2:
- fix another mem leak noticed by Anatoly Burakov

-- 
David Marchand

David Marchand (7):
  pci: fix potential mem leaks
  pci: align bsd implementation on linux
  pci: remove virtio-uio workaround
  pci: rework interrupt fd init and fix fd leak
  pci: pci_switch_module cleanup
  pci: move RTE_PCI_DRV_FORCE_UNBIND handling out of #ifdef
  pci: remove deprecated RTE_EAL_UNBIND_PORTS option

 lib/librte_eal/bsdapp/eal/eal_pci.c   |  105 ++++++------
 lib/librte_eal/linuxapp/eal/eal_pci.c |  282 +++++----------------------------
 lib/librte_pmd_virtio/virtio_ethdev.c |  133 +++++++++++++++-
 3 files changed, 218 insertions(+), 302 deletions(-)

-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 1/7] pci: fix potential mem leaks
  2014-05-09 13:15 [dpdk-dev] [PATCH v2 0/7] pci cleanup David Marchand
@ 2014-05-09 13:15 ` David Marchand
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 2/7] pci: align bsd implementation on linux David Marchand
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2014-05-09 13:15 UTC (permalink / raw)
  To: dev

Looking at bsd implementation, we can see that there are some potential mem
leaks in linux implementation. Fix them.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c |   11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 9538efe..99e07d2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -649,11 +649,13 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr));
 
 	/* collect info about device mappings */
-	if ((nb_maps = pci_uio_get_mappings(dirname, uio_res->maps,
-			sizeof (uio_res->maps) / sizeof (uio_res->maps[0])))
-			< 0)
+	nb_maps = pci_uio_get_mappings(dirname, uio_res->maps,
+				       RTE_DIM(uio_res->maps));
+	if (nb_maps < 0) {
+		rte_free(uio_res);
 		return (nb_maps);
- 
+	}
+
 	uio_res->nb_maps = nb_maps;
 
 	/* Map all BARs */
@@ -678,6 +680,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 					(mapaddr = pci_map_resource(dev,
 					NULL, devname, (off_t)offset,
 					(size_t)maps[j].size)) == NULL) {
+				rte_free(uio_res);
 				return (-1);
 			}
  
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 2/7] pci: align bsd implementation on linux
  2014-05-09 13:15 [dpdk-dev] [PATCH v2 0/7] pci cleanup David Marchand
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 1/7] pci: fix potential mem leaks David Marchand
@ 2014-05-09 13:15 ` David Marchand
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 3/7] pci: remove virtio-uio workaround David Marchand
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2014-05-09 13:15 UTC (permalink / raw)
  To: dev

bsd implementation lacks check on driver flags, fix this.
Besides, check on BAR0 is not needed and could cause trouble for devices that
have no BAR0.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c |   42 +++++++++++++++++++----------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 987b446..5d8bcbd 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -108,8 +108,14 @@ TAILQ_HEAD(uio_res_list, uio_resource);
 
 static struct uio_res_list *uio_res_list = NULL;
 
-/* forward prototype of function called in pci_switch_module below */
-static int pci_uio_map_resource(struct rte_pci_device *dev);
+/* unbind kernel driver for this device */
+static int
+pci_unbind_kernel_driver(struct rte_pci_device *dev)
+{
+	RTE_LOG(ERR, EAL, "RTE_PCI_DRV_FORCE_UNBIND flag is not implemented "
+		"for BSD\n");
+	return -ENOTSUP;
+}
 
 /* map a particular resource from a file */
 static void *
@@ -214,6 +220,11 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 
 	dev->intr_handle.fd = -1;
 
+	/* secondary processes - use already recorded details */
+	if ((rte_eal_process_type() != RTE_PROC_PRIMARY) &&
+		(dev->id.vendor_id != PCI_VENDOR_ID_QUMRANET))
+		return (pci_uio_map_secondary(dev));
+
 	rte_snprintf(devname, sizeof(devname), "/dev/uio@pci:%u:%u:%u",
 			dev->addr.bus, dev->addr.devid, dev->addr.function);
 
@@ -223,11 +234,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return -1;
 	}
 
-	/* secondary processes - use already recorded details */
-	if ((rte_eal_process_type() != RTE_PROC_PRIMARY) &&
-		(dev->id.vendor_id != PCI_VENDOR_ID_QUMRANET))
-		return (pci_uio_map_secondary(dev));
-
 	if(dev->id.vendor_id == PCI_VENDOR_ID_QUMRANET) {
 		/* I/O port address already assigned */
 		/* rte_virtio_pmd does not need any other bar even if available */
@@ -479,19 +485,17 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 			return 0;
 		}
 
-		/* just map the NIC resources */
-		if (pci_uio_map_resource(dev) < 0)
-			return -1;
-
-		/* We always should have BAR0 mapped */
-		if (rte_eal_process_type() == RTE_PROC_PRIMARY && 
-			dev->mem_resource[0].addr == NULL) {
-			RTE_LOG(ERR, EAL,
-				"%s(): BAR0 is not mapped\n",
-				__func__);
-			return (-1);
+		if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO) {
+			/* map resources for devices that use igb_uio */
+			if (pci_uio_map_resource(dev) < 0)
+				return -1;
+		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
+		           rte_eal_process_type() == RTE_PROC_PRIMARY) {
+			/* unbind current driver */
+			if (pci_unbind_kernel_driver(dev) < 0)
+				return -1;
 		}
- 
+
 		/* reference driver structure */
 		dev->driver = dr;
 
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 3/7] pci: remove virtio-uio workaround
  2014-05-09 13:15 [dpdk-dev] [PATCH v2 0/7] pci cleanup David Marchand
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 1/7] pci: fix potential mem leaks David Marchand
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 2/7] pci: align bsd implementation on linux David Marchand
@ 2014-05-09 13:15 ` David Marchand
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 4/7] pci: rework interrupt fd init and fix fd leak David Marchand
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2014-05-09 13:15 UTC (permalink / raw)
  To: dev

virtio-uio does not need eal to map bars from uio device, so remove flag
RTE_PCI_DRV_NEED_IGB_UIO.
Then, move virtio-uio workaround out of generic eal_pci.c for linux
implementation.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c   |    9 +--
 lib/librte_eal/linuxapp/eal/eal_pci.c |   30 +-------
 lib/librte_pmd_virtio/virtio_ethdev.c |  133 ++++++++++++++++++++++++++++++++-
 3 files changed, 134 insertions(+), 38 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index 5d8bcbd..a8945e4 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -221,8 +221,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	dev->intr_handle.fd = -1;
 
 	/* secondary processes - use already recorded details */
-	if ((rte_eal_process_type() != RTE_PROC_PRIMARY) &&
-		(dev->id.vendor_id != PCI_VENDOR_ID_QUMRANET))
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return (pci_uio_map_secondary(dev));
 
 	rte_snprintf(devname, sizeof(devname), "/dev/uio@pci:%u:%u:%u",
@@ -234,12 +233,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return -1;
 	}
 
-	if(dev->id.vendor_id == PCI_VENDOR_ID_QUMRANET) {
-		/* I/O port address already assigned */
-		/* rte_virtio_pmd does not need any other bar even if available */
-		return (0);
-	}
-	
 	/* allocate the mapping details for secondary processes*/
 	if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) {
 		RTE_LOG(ERR, EAL,
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 99e07d2..c006cf5 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -584,11 +584,9 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 {
 	int i, j;
 	char dirname[PATH_MAX];
-	char filename[PATH_MAX];
 	char devname[PATH_MAX]; /* contains the /dev/uioX */
 	void *mapaddr;
 	int uio_num;
-	unsigned long start,size;
 	uint64_t phaddr;
 	uint64_t offset;
 	uint64_t pagesz;
@@ -600,8 +598,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	dev->intr_handle.fd = -1;
 
 	/* secondary processes - use already recorded details */
-	if ((rte_eal_process_type() != RTE_PROC_PRIMARY) &&
-	    (dev->id.vendor_id != PCI_VENDOR_ID_QUMRANET))
+	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return (pci_uio_map_secondary(dev));
 
 	/* find uio resource */
@@ -612,31 +609,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return -1;
 	}
 
-	if(dev->id.vendor_id == PCI_VENDOR_ID_QUMRANET) {
-		/* get portio size */
-		rte_snprintf(filename, sizeof(filename),
-			 "%s/portio/port0/size", dirname);
-		if (eal_parse_sysfs_value(filename, &size) < 0) {
-			RTE_LOG(ERR, EAL, "%s(): cannot parse size\n",
-				__func__);
-			return -1;
-		}
-
-		/* get portio start */
-		rte_snprintf(filename, sizeof(filename),
-			 "%s/portio/port0/start", dirname);
-		if (eal_parse_sysfs_value(filename, &start) < 0) {
-			RTE_LOG(ERR, EAL, "%s(): cannot parse portio start\n",
-				__func__);
-			return -1;
-		}
-		dev->mem_resource[0].addr = (void *)(uintptr_t)start;
-		dev->mem_resource[0].len =  (uint64_t)size;
-		RTE_LOG(DEBUG, EAL, "PCI Port IO found start=0x%lx with size=0x%lx\n", start, size);
-		/* rte_virtio_pmd does not need any other bar even if available */
-		return (0);
-	}
-	
 	/* allocate the mapping details for secondary processes*/
 	if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) {
 		RTE_LOG(ERR, EAL,
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index f107161..c6a1df5 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -36,6 +36,9 @@
 #include <stdio.h>
 #include <errno.h>
 #include <unistd.h>
+#ifdef RTE_EXEC_ENV_LINUXAPP
+#include <dirent.h>
+#endif
 
 #include <rte_ethdev.h>
 #include <rte_memcpy.h>
@@ -392,6 +395,103 @@ virtio_negotiate_features(struct virtio_hw *hw)
 	hw->guest_features = vtpci_negotiate_features(hw, guest_features);
 }
 
+#ifdef RTE_EXEC_ENV_LINUXAPP
+static int
+parse_sysfs_value(const char *filename, unsigned long *val)
+{
+	FILE *f;
+	char buf[BUFSIZ];
+	char *end = NULL;
+
+	if ((f = fopen(filename, "r")) == NULL) {
+		PMD_INIT_LOG(ERR, "%s(): cannot open sysfs value %s\n",
+			     __func__, filename);
+		return -1;
+	}
+
+	if (fgets(buf, sizeof(buf), f) == NULL) {
+		PMD_INIT_LOG(ERR, "%s(): cannot read sysfs value %s\n",
+			     __func__, filename);
+		fclose(f);
+		return -1;
+	}
+	*val = strtoul(buf, &end, 0);
+	if ((buf[0] == '\0') || (end == NULL) || (*end != '\n')) {
+		PMD_INIT_LOG(ERR, "%s(): cannot parse sysfs value %s\n",
+			     __func__, filename);
+		fclose(f);
+		return -1;
+	}
+	fclose(f);
+	return 0;
+}
+
+static int get_uio_dev(struct rte_pci_addr *loc, char *buf, unsigned int buflen)
+{
+	unsigned int uio_num;
+	struct dirent *e;
+	DIR *dir;
+	char dirname[PATH_MAX];
+
+	/* depending on kernel version, uio can be located in uio/uioX
+	 * or uio:uioX */
+	rte_snprintf(dirname, sizeof(dirname),
+	         SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/uio",
+	         loc->domain, loc->bus, loc->devid, loc->function);
+	dir = opendir(dirname);
+	if (dir == NULL) {
+		/* retry with the parent directory */
+		rte_snprintf(dirname, sizeof(dirname),
+		         SYSFS_PCI_DEVICES "/" PCI_PRI_FMT,
+		         loc->domain, loc->bus, loc->devid, loc->function);
+		dir = opendir(dirname);
+
+		if (dir == NULL) {
+			PMD_INIT_LOG(ERR, "Cannot opendir %s\n", dirname);
+			return -1;
+		}
+	}
+
+	/* take the first file starting with "uio" */
+	while ((e = readdir(dir)) != NULL) {
+		/* format could be uio%d ...*/
+		int shortprefix_len = sizeof("uio") - 1;
+		/* ... or uio:uio%d */
+		int longprefix_len = sizeof("uio:uio") - 1;
+		char *endptr;
+
+		if (strncmp(e->d_name, "uio", 3) != 0)
+			continue;
+
+		/* first try uio%d */
+		errno = 0;
+		uio_num = strtoull(e->d_name + shortprefix_len, &endptr, 10);
+		if (errno == 0 && endptr != (e->d_name + shortprefix_len)) {
+			rte_snprintf(buf, buflen, "%s/uio%u", dirname, uio_num);
+			break;
+		}
+
+		/* then try uio:uio%d */
+		errno = 0;
+		uio_num = strtoull(e->d_name + longprefix_len, &endptr, 10);
+		if (errno == 0 && endptr != (e->d_name + longprefix_len)) {
+			rte_snprintf(buf, buflen, "%s/uio:uio%u", dirname,
+				     uio_num);
+			break;
+		}
+	}
+	closedir(dir);
+
+	/* No uio resource found */
+	if (e == NULL) {
+		PMD_INIT_LOG(ERR, "Could not find uio resource\n");
+		return -1;
+	}
+
+	return 0;
+}
+#endif
+
 /*
  * This function is based on probe() function in virtio_pci.c
  * It returns 0 on success.
@@ -426,6 +526,38 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
 
 	hw->device_id = pci_dev->id.device_id;
 	hw->vendor_id = pci_dev->id.vendor_id;
+#ifdef RTE_EXEC_ENV_LINUXAPP
+	{
+		char dirname[PATH_MAX];
+		char filename[PATH_MAX];
+		unsigned long start,size;
+
+		if (get_uio_dev(&pci_dev->addr, dirname, sizeof(dirname)) < 0)
+			return -1;
+
+		/* get portio size */
+		rte_snprintf(filename, sizeof(filename),
+			     "%s/portio/port0/size", dirname);
+		if (parse_sysfs_value(filename, &size) < 0) {
+			PMD_INIT_LOG(ERR, "%s(): cannot parse size\n",
+				     __func__);
+			return -1;
+		}
+
+		/* get portio start */
+		rte_snprintf(filename, sizeof(filename),
+			     "%s/portio/port0/start", dirname);
+		if (parse_sysfs_value(filename, &start) < 0) {
+			PMD_INIT_LOG(ERR, "%s(): cannot parse portio start\n",
+				     __func__);
+			return -1;
+		}
+		pci_dev->mem_resource[0].addr = (void *)(uintptr_t)start;
+		pci_dev->mem_resource[0].len =  (uint64_t)size;
+		PMD_INIT_LOG(DEBUG, "PCI Port IO found start=0x%lx with "
+			     "size=0x%lx\n", start, size);
+	}
+#endif
 	hw->io_base = (uint32_t)(uintptr_t)pci_dev->mem_resource[0].addr;
 
 	hw->max_rx_queues = VIRTIO_MAX_RX_QUEUES;
@@ -474,7 +606,6 @@ static struct eth_driver rte_virtio_pmd = {
 	{
 		.name = "rte_virtio_pmd",
 		.id_table = pci_id_virtio_map,
-		.drv_flags = RTE_PCI_DRV_NEED_IGB_UIO,
 	},
 	.eth_dev_init = eth_virtio_dev_init,
 	.dev_private_size = sizeof(struct virtio_adapter),
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 4/7] pci: rework interrupt fd init and fix fd leak
  2014-05-09 13:15 [dpdk-dev] [PATCH v2 0/7] pci cleanup David Marchand
                   ` (2 preceding siblings ...)
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 3/7] pci: remove virtio-uio workaround David Marchand
@ 2014-05-09 13:15 ` David Marchand
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 5/7] pci: pci_switch_module cleanup David Marchand
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2014-05-09 13:15 UTC (permalink / raw)
  To: dev

A fd leak happens in pci_map_resource when multiple bars are mapped.
Fix this by closing fd unconditionnally in this function and open the
intr_handle fd in pci_uio_map_resource instead.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/bsdapp/eal/eal_pci.c   |   60 +++++++++++++---------------
 lib/librte_eal/linuxapp/eal/eal_pci.c |   71 ++++++++++++++++-----------------
 2 files changed, 62 insertions(+), 69 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal_pci.c b/lib/librte_eal/bsdapp/eal/eal_pci.c
index a8945e4..94ae461 100644
--- a/lib/librte_eal/bsdapp/eal/eal_pci.c
+++ b/lib/librte_eal/bsdapp/eal/eal_pci.c
@@ -119,8 +119,8 @@ pci_unbind_kernel_driver(struct rte_pci_device *dev)
 
 /* map a particular resource from a file */
 static void *
-pci_map_resource(struct rte_pci_device *dev, void *requested_addr, 
-		const char *devname, off_t offset, size_t size)
+pci_map_resource(void *requested_addr, const char *devname, off_t offset,
+		 size_t size)
 {
 	int fd;
 	void *mapaddr;
@@ -130,7 +130,7 @@ pci_map_resource(struct rte_pci_device *dev, void *requested_addr,
 	 */
 	fd = open(devname, O_RDWR);
 	if (fd < 0) {
-		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", 
+		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
 			devname, strerror(errno));
 		goto fail;
 	}
@@ -138,35 +138,21 @@ pci_map_resource(struct rte_pci_device *dev, void *requested_addr,
 	/* Map the PCI memory resource of device */
 	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
 			MAP_SHARED, fd, offset);
+	close(fd);
 	if (mapaddr == MAP_FAILED ||
 			(requested_addr != NULL && mapaddr != requested_addr)) {
 		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
-			" %s (%p)\n", __func__, devname, fd, requested_addr, 
+			" %s (%p)\n", __func__, devname, fd, requested_addr,
 			(unsigned long)size, (unsigned long)offset,
 			strerror(errno), mapaddr);
-		close(fd);
 		goto fail;
 	}
 
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-		/* save fd if in primary process */
-		dev->intr_handle.fd = fd;
-		dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
-	} else {
-		/* fd is not needed in slave process, close it */
-		dev->intr_handle.fd = -1;
-		dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
-		close(fd);
-	}
-
 	RTE_LOG(DEBUG, EAL, "  PCI memory mapped at %p\n", mapaddr);
 
 	return mapaddr;
 
 fail:
-	dev->intr_handle.fd = -1;
-	dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
-
 	return NULL;
 }
 
@@ -179,19 +165,19 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 {
         size_t i;
         struct uio_resource *uio_res;
- 
+
 	TAILQ_FOREACH(uio_res, uio_res_list, next) {
- 
+
 		/* skip this element if it doesn't match our PCI address */
 		if (memcmp(&uio_res->pci_addr, &dev->addr, sizeof(dev->addr)))
 			continue;
-                
+
 		for (i = 0; i != uio_res->nb_maps; i++) {
-			if (pci_map_resource(dev, uio_res->maps[i].addr,
-					uio_res->path,
-					(off_t)uio_res->maps[i].offset,
-					(size_t)uio_res->maps[i].size) != 
-					uio_res->maps[i].addr) {
+			if (pci_map_resource(uio_res->maps[i].addr,
+					     uio_res->path,
+					     (off_t)uio_res->maps[i].offset,
+					     (size_t)uio_res->maps[i].size)
+			    != uio_res->maps[i].addr) {
 				RTE_LOG(ERR, EAL,
 					"Cannot mmap device resource\n");
 				return (-1);
@@ -219,6 +205,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	struct uio_map *maps;
 
 	dev->intr_handle.fd = -1;
+	dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
 
 	/* secondary processes - use already recorded details */
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
@@ -233,6 +220,15 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return -1;
 	}
 
+	/* save fd if in primary process */
+	dev->intr_handle.fd = open(devname, O_RDWR);
+	if (dev->intr_handle.fd < 0) {
+		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
+			devname, strerror(errno));
+		return -1;
+	}
+	dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
+
 	/* allocate the mapping details for secondary processes*/
 	if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) {
 		RTE_LOG(ERR, EAL,
@@ -246,7 +242,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 
 	/* Map all BARs */
 	pagesz = sysconf(_SC_PAGESIZE);
- 
+
 	maps = uio_res->maps;
 	for (i = uio_res->nb_maps = 0; i != PCI_MAX_RESOURCE; i++) {
 
@@ -254,16 +250,16 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		/* skip empty BAR */
 		if ((phaddr = dev->mem_resource[i].phys_addr) == 0)
 			continue;
- 
+
 		/* if matching map is found, then use it */
 		offset = i * pagesz;
 		maps[j].offset = offset;
 		maps[j].phaddr = dev->mem_resource[i].phys_addr;
 		maps[j].size = dev->mem_resource[i].len;
 		if (maps[j].addr != NULL ||
-				(mapaddr = pci_map_resource(dev,
-				NULL, devname, (off_t)offset,
-				(size_t)maps[j].size)) == NULL) {
+		    (mapaddr = pci_map_resource(NULL, devname, (off_t)offset,
+						(size_t)maps[j].size)
+		    ) == NULL) {
 			rte_free(uio_res);
 			return (-1);
 		}
diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index c006cf5..451fbd2 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -305,8 +305,8 @@ pci_switch_module(struct rte_pci_driver *dr, struct rte_pci_device *dev,
 
 /* map a particular resource from a file */
 static void *
-pci_map_resource(struct rte_pci_device *dev, void *requested_addr, 
-		const char *devname, off_t offset, size_t size)
+pci_map_resource(void *requested_addr, const char *devname, off_t offset,
+		 size_t size)
 {
 	int fd;
 	void *mapaddr;
@@ -317,8 +317,7 @@ pci_map_resource(struct rte_pci_device *dev, void *requested_addr,
 	 * appear, so we wait some time before returning an error
 	 */
 	unsigned n;
-	fd = dev->intr_handle.fd;
-	for (n = 0; n < UIO_DEV_WAIT_TIMEOUT*10 && fd < 0; n++) {
+	for (n = 0; n < UIO_DEV_WAIT_TIMEOUT*10; n++) {
 		errno = 0;
 		if ((fd = open(devname, O_RDWR)) < 0 && errno != ENOENT)
 			break;
@@ -331,7 +330,7 @@ pci_map_resource(struct rte_pci_device *dev, void *requested_addr,
 	fd = open(devname, O_RDWR);
 #endif
 	if (fd < 0) {
-		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n", 
+		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
 			devname, strerror(errno));
 		goto fail;
 	}
@@ -339,34 +338,21 @@ pci_map_resource(struct rte_pci_device *dev, void *requested_addr,
 	/* Map the PCI memory resource of device */
 	mapaddr = mmap(requested_addr, size, PROT_READ | PROT_WRITE,
 			MAP_SHARED, fd, offset);
+	close(fd);
 	if (mapaddr == MAP_FAILED ||
 			(requested_addr != NULL && mapaddr != requested_addr)) {
 		RTE_LOG(ERR, EAL, "%s(): cannot mmap(%s(%d), %p, 0x%lx, 0x%lx):"
-			" %s (%p)\n", __func__, devname, fd, requested_addr, 
+			" %s (%p)\n", __func__, devname, fd, requested_addr,
 			(unsigned long)size, (unsigned long)offset,
 			strerror(errno), mapaddr);
-		close(fd);
 		goto fail;
 	}
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-		/* save fd if in primary process */
-		dev->intr_handle.fd = fd;
-		dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
-	} else {
-		/* fd is not needed in slave process, close it */
-		dev->intr_handle.fd = -1;
-		dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
-		close(fd);
-	}
 
 	RTE_LOG(DEBUG, EAL, "  PCI memory mapped at %p\n", mapaddr);
 
 	return mapaddr;
 
 fail:
-	dev->intr_handle.fd = -1;
-	dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
-
 	return NULL;
 }
 
@@ -436,19 +422,19 @@ pci_uio_map_secondary(struct rte_pci_device *dev)
 {
         size_t i;
         struct uio_resource *uio_res;
- 
+
 	TAILQ_FOREACH(uio_res, uio_res_list, next) {
- 
+
 		/* skip this element if it doesn't match our PCI address */
 		if (memcmp(&uio_res->pci_addr, &dev->addr, sizeof(dev->addr)))
 			continue;
-                
+
 		for (i = 0; i != uio_res->nb_maps; i++) {
-			if (pci_map_resource(dev, uio_res->maps[i].addr,
-					uio_res->path,
-					(off_t)uio_res->maps[i].offset,
-					(size_t)uio_res->maps[i].size) != 
-					uio_res->maps[i].addr) {
+			if (pci_map_resource(uio_res->maps[i].addr,
+					     uio_res->path,
+					     (off_t)uio_res->maps[i].offset,
+					     (size_t)uio_res->maps[i].size)
+			    != uio_res->maps[i].addr) {
 				RTE_LOG(ERR, EAL,
 					"Cannot mmap device resource\n");
 				return (-1);
@@ -596,6 +582,7 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 	struct uio_map *maps;
 
 	dev->intr_handle.fd = -1;
+	dev->intr_handle.type = RTE_INTR_HANDLE_UNKNOWN;
 
 	/* secondary processes - use already recorded details */
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
@@ -608,6 +595,16 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 				"skipping\n", loc->domain, loc->bus, loc->devid, loc->function);
 		return -1;
 	}
+	rte_snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
+
+	/* save fd if in primary process */
+	dev->intr_handle.fd = open(devname, O_RDWR);
+	if (dev->intr_handle.fd < 0) {
+		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
+			devname, strerror(errno));
+		return -1;
+	}
+	dev->intr_handle.type = RTE_INTR_HANDLE_UIO;
 
 	/* allocate the mapping details for secondary processes*/
 	if ((uio_res = rte_zmalloc("UIO_RES", sizeof (*uio_res), 0)) == NULL) {
@@ -616,7 +613,6 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 		return (-1);
 	}
 
-	rte_snprintf(devname, sizeof(devname), "/dev/uio%u", uio_num);
 	rte_snprintf(uio_res->path, sizeof(uio_res->path), "%s", devname);
 	memcpy(&uio_res->pci_addr, &dev->addr, sizeof(uio_res->pci_addr));
 
@@ -632,30 +628,31 @@ pci_uio_map_resource(struct rte_pci_device *dev)
 
 	/* Map all BARs */
 	pagesz = sysconf(_SC_PAGESIZE);
- 
+
 	maps = uio_res->maps;
 	for (i = 0; i != PCI_MAX_RESOURCE; i++) {
-    
+
 		/* skip empty BAR */
 		if ((phaddr = dev->mem_resource[i].phys_addr) == 0)
 			continue;
- 
+
 		for (j = 0; j != nb_maps && (phaddr != maps[j].phaddr ||
 				dev->mem_resource[i].len != maps[j].size);
 				j++)
 			;
- 
+
 		/* if matching map is found, then use it */
 		if (j != nb_maps) {
 			offset = j * pagesz;
 			if (maps[j].addr != NULL ||
-					(mapaddr = pci_map_resource(dev,
-					NULL, devname, (off_t)offset,
-					(size_t)maps[j].size)) == NULL) {
+			    (mapaddr = pci_map_resource(NULL, devname,
+							(off_t)offset,
+							(size_t)maps[j].size)
+			    ) == NULL) {
 				rte_free(uio_res);
 				return (-1);
 			}
- 
+
 			maps[j].addr = mapaddr;
 			maps[j].offset = offset;
 			dev->mem_resource[i].addr = mapaddr;
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 5/7] pci: pci_switch_module cleanup
  2014-05-09 13:15 [dpdk-dev] [PATCH v2 0/7] pci cleanup David Marchand
                   ` (3 preceding siblings ...)
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 4/7] pci: rework interrupt fd init and fix fd leak David Marchand
@ 2014-05-09 13:15 ` David Marchand
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 6/7] pci: move RTE_PCI_DRV_FORCE_UNBIND handling out of #ifdef David Marchand
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2014-05-09 13:15 UTC (permalink / raw)
  To: dev

The pci_switch_module() function should only do what its name tells: unbind pci
devices and rebind them on the specified kernel driver.
Hence, it can not call pci_uio_map_resource().

Call to pci_uio_map_resource() should be moved to rte_eal_pci_probe_one_driver()
so that we can factorize code.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c |   24 +++++++++---------------
 1 file changed, 9 insertions(+), 15 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index 451fbd2..dadb198 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -107,9 +107,6 @@ TAILQ_HEAD(uio_res_list, uio_resource);
 static struct uio_res_list *uio_res_list = NULL;
 static int pci_parse_sysfs_value(const char *filename, uint64_t *val);
 
-/* forward prototype of function called in pci_switch_module below */
-static int pci_uio_map_resource(struct rte_pci_device *dev);
-
 #ifdef RTE_EAL_UNBIND_PORTS
 #define PROC_MODULES "/proc/modules"
 
@@ -279,12 +276,11 @@ error:
 
 static int
 pci_switch_module(struct rte_pci_driver *dr, struct rte_pci_device *dev,
-		int uio_status, const char *module_name)
+		  const char *module_name)
 {
 	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
 		/* check that our driver is loaded */
-		if (uio_status != 0 &&
-				(uio_status = pci_uio_check_module(module_name)) != 0)
+		if (pci_uio_check_module(module_name) != 0)
 			rte_exit(EXIT_FAILURE, "The %s module is required by the "
 					"%s driver\n", module_name, dr->name);
 
@@ -294,9 +290,6 @@ pci_switch_module(struct rte_pci_driver *dr, struct rte_pci_device *dev,
 		if (pci_uio_bind_device(dev, module_name) < 0)
 			return -1;
 	}
-	/* map the NIC resources */
-	if (pci_uio_map_resource(dev) < 0)
-		return -1;
 
 	return 0;
 }
@@ -1012,8 +1005,8 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 
 #ifdef RTE_EAL_UNBIND_PORTS
 		if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO) {
-			/* unbind driver and load uio resources for Intel NICs */
-			if (pci_switch_module(dr, dev, 1, IGB_UIO_NAME) < 0)
+			/* unbind current driver and bind on igb_uio */
+			if (pci_switch_module(dr, dev, IGB_UIO_NAME) < 0)
 				return -1;
 		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
 		           rte_eal_process_type() == RTE_PROC_PRIMARY) {
@@ -1021,12 +1014,13 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 			if (pci_unbind_kernel_driver(dev) < 0)
 				return -1;
 		}
-#else
-		if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO)
-			/* just map resources for Intel NICs */
+#endif
+
+		if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO) {
+			/* map resources for devices that use igb_uio */
 			if (pci_uio_map_resource(dev) < 0)
 				return -1;
-#endif
+		}
 
 		/* reference driver structure */
 		dev->driver = dr;
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 6/7] pci: move RTE_PCI_DRV_FORCE_UNBIND handling out of #ifdef
  2014-05-09 13:15 [dpdk-dev] [PATCH v2 0/7] pci cleanup David Marchand
                   ` (4 preceding siblings ...)
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 5/7] pci: pci_switch_module cleanup David Marchand
@ 2014-05-09 13:15 ` David Marchand
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 7/7] pci: remove deprecated RTE_EAL_UNBIND_PORTS option David Marchand
  2014-05-13 14:29 ` [dpdk-dev] [PATCH v2 0/7] pci cleanup Thomas Monjalon
  7 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2014-05-09 13:15 UTC (permalink / raw)
  To: dev

Move RTE_PCI_DRV_FORCE_UNBIND flag handling out of RTE_EAL_UNBIND_PORTS section.
This had nothing to do with RTE_EAL_UNBIND_PORTS anyway.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c |   89 ++++++++++++++++-----------------
 1 file changed, 44 insertions(+), 45 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index dadb198..d529ced 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -107,6 +107,45 @@ TAILQ_HEAD(uio_res_list, uio_resource);
 static struct uio_res_list *uio_res_list = NULL;
 static int pci_parse_sysfs_value(const char *filename, uint64_t *val);
 
+/* unbind kernel driver for this device */
+static int
+pci_unbind_kernel_driver(struct rte_pci_device *dev)
+{
+	int n;
+	FILE *f;
+	char filename[PATH_MAX];
+	char buf[BUFSIZ];
+	struct rte_pci_addr *loc = &dev->addr;
+
+	/* open /sys/bus/pci/devices/AAAA:BB:CC.D/driver */
+	rte_snprintf(filename, sizeof(filename),
+	         SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/driver/unbind",
+	         loc->domain, loc->bus, loc->devid, loc->function);
+
+	f = fopen(filename, "w");
+	if (f == NULL) /* device was not bound */
+		return 0;
+
+	n = rte_snprintf(buf, sizeof(buf), PCI_PRI_FMT "\n",
+	             loc->domain, loc->bus, loc->devid, loc->function);
+	if ((n < 0) || (n >= (int)sizeof(buf))) {
+		RTE_LOG(ERR, EAL, "%s(): rte_snprintf failed\n", __func__);
+		goto error;
+	}
+	if (fwrite(buf, n, 1, f) == 0) {
+		RTE_LOG(ERR, EAL, "%s(): could not write to %s\n", __func__,
+				filename);
+		goto error;
+	}
+
+	fclose(f);
+	return 0;
+
+error:
+	fclose(f);
+	return -1;
+}
+
 #ifdef RTE_EAL_UNBIND_PORTS
 #define PROC_MODULES "/proc/modules"
 
@@ -234,46 +273,6 @@ pci_uio_bind_device(struct rte_pci_device *dev, const char *module_name)
 	return 0;
 }
 
-/* unbind kernel driver for this device */
-static int
-pci_unbind_kernel_driver(struct rte_pci_device *dev)
-{
-	int n;
-	FILE *f;
-	char filename[PATH_MAX];
-	char buf[BUFSIZ];
-	struct rte_pci_addr *loc = &dev->addr;
-
-	/* open /sys/bus/pci/devices/AAAA:BB:CC.D/driver */
-	rte_snprintf(filename, sizeof(filename),
-	         SYSFS_PCI_DEVICES "/" PCI_PRI_FMT "/driver/unbind",
-	         loc->domain, loc->bus, loc->devid, loc->function);
-
-	f = fopen(filename, "w");
-	if (f == NULL) /* device was not bound */
-		return 0;
-
-	n = rte_snprintf(buf, sizeof(buf), PCI_PRI_FMT "\n",
-	             loc->domain, loc->bus, loc->devid, loc->function);
-	if ((n < 0) || (n >= (int)sizeof(buf))) {
-		RTE_LOG(ERR, EAL, "%s(): rte_snprintf failed\n", __func__);
-		goto error;
-	}
-	if (fwrite(buf, n, 1, f) == 0) {
-		RTE_LOG(ERR, EAL, "%s(): could not write to %s\n", __func__,
-				filename);
-		goto error;
-	}
-
-	fclose(f);
-	return 0;
-
-error:
-	fclose(f);
-	return -1;
-}
-
-
 static int
 pci_switch_module(struct rte_pci_driver *dr, struct rte_pci_device *dev,
 		  const char *module_name)
@@ -1008,11 +1007,6 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 			/* unbind current driver and bind on igb_uio */
 			if (pci_switch_module(dr, dev, IGB_UIO_NAME) < 0)
 				return -1;
-		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
-		           rte_eal_process_type() == RTE_PROC_PRIMARY) {
-			/* unbind current driver */
-			if (pci_unbind_kernel_driver(dev) < 0)
-				return -1;
 		}
 #endif
 
@@ -1020,6 +1014,11 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 			/* map resources for devices that use igb_uio */
 			if (pci_uio_map_resource(dev) < 0)
 				return -1;
+		} else if (dr->drv_flags & RTE_PCI_DRV_FORCE_UNBIND &&
+		           rte_eal_process_type() == RTE_PROC_PRIMARY) {
+			/* unbind current driver */
+			if (pci_unbind_kernel_driver(dev) < 0)
+				return -1;
 		}
 
 		/* reference driver structure */
-- 
1.7.10.4

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

* [dpdk-dev] [PATCH v2 7/7] pci: remove deprecated RTE_EAL_UNBIND_PORTS option
  2014-05-09 13:15 [dpdk-dev] [PATCH v2 0/7] pci cleanup David Marchand
                   ` (5 preceding siblings ...)
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 6/7] pci: move RTE_PCI_DRV_FORCE_UNBIND handling out of #ifdef David Marchand
@ 2014-05-09 13:15 ` David Marchand
  2014-05-13 14:29 ` [dpdk-dev] [PATCH v2 0/7] pci cleanup Thomas Monjalon
  7 siblings, 0 replies; 9+ messages in thread
From: David Marchand @ 2014-05-09 13:15 UTC (permalink / raw)
  To: dev

RTE_EAL_UNBIND_PORTS was deprecated in DPDK 1.4.0 and removed in 1.6.0, but the
code was not removed.

The bind/unbind operations should not be handled by the eal.
These operations should be either done outside of dpdk or inside the PMDs
themselves as these are their problems.

Signed-off-by: Anatoly Burakov <anatoly.burakov@intel.com>
Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_eal/linuxapp/eal/eal_pci.c |  171 ---------------------------------
 1 file changed, 171 deletions(-)

diff --git a/lib/librte_eal/linuxapp/eal/eal_pci.c b/lib/librte_eal/linuxapp/eal/eal_pci.c
index d529ced..ac2c1fe 100644
--- a/lib/librte_eal/linuxapp/eal/eal_pci.c
+++ b/lib/librte_eal/linuxapp/eal/eal_pci.c
@@ -146,155 +146,6 @@ error:
 	return -1;
 }
 
-#ifdef RTE_EAL_UNBIND_PORTS
-#define PROC_MODULES "/proc/modules"
-
-#define IGB_UIO_NAME "igb_uio"
-
-#define UIO_DRV_PATH  "/sys/bus/pci/drivers/%s"
-
-/* maximum time to wait that /dev/uioX appears */
-#define UIO_DEV_WAIT_TIMEOUT 3 /* seconds */
-
-/*
- * Check that a kernel module is loaded. Returns 0 on success, or if the
- * parameter is NULL, or -1 if the module is not loaded.
- */
-static int
-pci_uio_check_module(const char *module_name)
-{
-	FILE *f;
-	unsigned i;
-	char buf[BUFSIZ];
-
-	if (module_name == NULL)
-		return 0;
-
-	f = fopen(PROC_MODULES, "r");
-	if (f == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot open "PROC_MODULES": %s\n", 
-				strerror(errno));
-		return -1;
-	}
-
-	while(fgets(buf, sizeof(buf), f) != NULL) {
-
-		for (i = 0; i < sizeof(buf) && buf[i] != '\0'; i++) {
-			if (isspace(buf[i]))
-			    buf[i] = '\0';
-		}
-
-		if (strncmp(buf, module_name, sizeof(buf)) == 0) {
-			fclose(f);
-			return 0;
-		}
-	}
-	fclose(f);
-	return -1;
-}
-
-/* bind a PCI to the kernel module driver */
-static int
-pci_bind_device(struct rte_pci_device *dev, char dr_path[])
-{
-	FILE *f;
-	int n;
-	char buf[BUFSIZ];
-	char dev_bind[PATH_MAX];
-	struct rte_pci_addr *loc = &dev->addr;
-
-	n = rte_snprintf(dev_bind, sizeof(dev_bind), "%s/bind", dr_path);
-	if ((n < 0) || (n >= (int)sizeof(buf))) {
-		RTE_LOG(ERR, EAL, "Cannot rte_snprintf device bind path\n");
-		return -1;
-	}
-
-	f = fopen(dev_bind, "w");
-	if (f == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot open %s\n", dev_bind);
-		return -1;
-	}
-	n = rte_snprintf(buf, sizeof(buf), PCI_PRI_FMT "\n",
-	                 loc->domain, loc->bus, loc->devid, loc->function);
-	if ((n < 0) || (n >= (int)sizeof(buf))) {
-		RTE_LOG(ERR, EAL, "Cannot rte_snprintf PCI infos\n");
-		fclose(f);
-		return -1;
-	}
-	if (fwrite(buf, n, 1, f) == 0) {
-		fclose(f);
-		return -1;
-	}
-
-	fclose(f);
-	return 0;
-}
-
-static int
-pci_uio_bind_device(struct rte_pci_device *dev, const char *module_name)
-{
-	FILE *f;
-	int n;
-	char buf[BUFSIZ];
-	char uio_newid[PATH_MAX];
-	char uio_bind[PATH_MAX];
-
-	n = rte_snprintf(uio_newid, sizeof(uio_newid), UIO_DRV_PATH "/new_id", module_name);
-	if ((n < 0) || (n >= (int)sizeof(uio_newid))) {
-		RTE_LOG(ERR, EAL, "Cannot rte_snprintf uio_newid name\n");
-		return -1;
-	}
-
-	n = rte_snprintf(uio_bind, sizeof(uio_bind), UIO_DRV_PATH, module_name);
-	if ((n < 0) || (n >= (int)sizeof(uio_bind))) {
-		RTE_LOG(ERR, EAL, "Cannot rte_snprintf uio_bind name\n");
-		return -1;
-	}
-
-	n = rte_snprintf(buf, sizeof(buf), "%x %x\n",
-			dev->id.vendor_id, dev->id.device_id);
-	if ((n < 0) || (n >= (int)sizeof(buf))) {
-		RTE_LOG(ERR, EAL, "Cannot rte_snprintf vendor_id/device_id\n");
-		return -1;
-	}
-
-	f = fopen(uio_newid, "w");
-	if (f == NULL) {
-		RTE_LOG(ERR, EAL, "Cannot open %s\n", uio_newid);
-		return -1;
-	}
-	if (fwrite(buf, n, 1, f) == 0) {
-		fclose(f);
-		return -1;
-	}
-	fclose(f);
-
-	pci_bind_device(dev, uio_bind);
-	return 0;
-}
-
-static int
-pci_switch_module(struct rte_pci_driver *dr, struct rte_pci_device *dev,
-		  const char *module_name)
-{
-	if (rte_eal_process_type() == RTE_PROC_PRIMARY) {
-		/* check that our driver is loaded */
-		if (pci_uio_check_module(module_name) != 0)
-			rte_exit(EXIT_FAILURE, "The %s module is required by the "
-					"%s driver\n", module_name, dr->name);
-
-		/* unbind current driver, bind ours */
-		if (pci_unbind_kernel_driver(dev) < 0)
-			return -1;
-		if (pci_uio_bind_device(dev, module_name) < 0)
-			return -1;
-	}
-
-	return 0;
-}
-
-#endif /* ifdef EAL_UNBIND_PORTS */
-
 /* map a particular resource from a file */
 static void *
 pci_map_resource(void *requested_addr, const char *devname, off_t offset,
@@ -303,24 +154,10 @@ pci_map_resource(void *requested_addr, const char *devname, off_t offset,
 	int fd;
 	void *mapaddr;
 
-#ifdef RTE_EAL_UNBIND_PORTS
-	/*
-	 * open devname, and mmap it: it can take some time to
-	 * appear, so we wait some time before returning an error
-	 */
-	unsigned n;
-	for (n = 0; n < UIO_DEV_WAIT_TIMEOUT*10; n++) {
-		errno = 0;
-		if ((fd = open(devname, O_RDWR)) < 0 && errno != ENOENT)
-			break;
-		usleep(100000);
-	}
-#else
 	/*
 	 * open devname, to mmap it
 	 */
 	fd = open(devname, O_RDWR);
-#endif
 	if (fd < 0) {
 		RTE_LOG(ERR, EAL, "Cannot open %s: %s\n",
 			devname, strerror(errno));
@@ -1002,14 +839,6 @@ rte_eal_pci_probe_one_driver(struct rte_pci_driver *dr, struct rte_pci_device *d
 			return 0;
 		}
 
-#ifdef RTE_EAL_UNBIND_PORTS
-		if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO) {
-			/* unbind current driver and bind on igb_uio */
-			if (pci_switch_module(dr, dev, IGB_UIO_NAME) < 0)
-				return -1;
-		}
-#endif
-
 		if (dr->drv_flags & RTE_PCI_DRV_NEED_IGB_UIO) {
 			/* map resources for devices that use igb_uio */
 			if (pci_uio_map_resource(dev) < 0)
-- 
1.7.10.4

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

* Re: [dpdk-dev] [PATCH v2 0/7] pci cleanup
  2014-05-09 13:15 [dpdk-dev] [PATCH v2 0/7] pci cleanup David Marchand
                   ` (6 preceding siblings ...)
  2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 7/7] pci: remove deprecated RTE_EAL_UNBIND_PORTS option David Marchand
@ 2014-05-13 14:29 ` Thomas Monjalon
  7 siblings, 0 replies; 9+ messages in thread
From: Thomas Monjalon @ 2014-05-13 14:29 UTC (permalink / raw)
  To: David Marchand; +Cc: dev

2014-05-09 15:15, David Marchand:
> Hello all,
> 
> Here is an attempt at having an equal implementation in bsd and linux
> eal_pci.c. It results in following changes :
> - checks on driver flag in bsd which were missing
> - remove virtio-uio workaround in linux eal_pci.c
> - remove deprecated RTE_EAL_UNBIND_PORTS option
> 
> Along the way, I discovered two small bugs: a mem leak in linux eal_pci.c
> and a fd leak in both bsd and linux eal_pci.c.
> 
> Changes included in v2:
> - fix another mem leak noticed by Anatoly Burakov

First version was acked:
Acked-by: Anatoly Burakov <anatoly.burakov@intel.com>
Acked-by: Neil Horman <nhorman@tuxdriver.com>

Applied for version 1.7.0

-- 
Thomas

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

end of thread, other threads:[~2014-05-13 14:29 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-05-09 13:15 [dpdk-dev] [PATCH v2 0/7] pci cleanup David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 1/7] pci: fix potential mem leaks David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 2/7] pci: align bsd implementation on linux David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 3/7] pci: remove virtio-uio workaround David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 4/7] pci: rework interrupt fd init and fix fd leak David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 5/7] pci: pci_switch_module cleanup David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 6/7] pci: move RTE_PCI_DRV_FORCE_UNBIND handling out of #ifdef David Marchand
2014-05-09 13:15 ` [dpdk-dev] [PATCH v2 7/7] pci: remove deprecated RTE_EAL_UNBIND_PORTS option David Marchand
2014-05-13 14:29 ` [dpdk-dev] [PATCH v2 0/7] pci cleanup Thomas Monjalon

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).