[dpdk-dev] [PATCH] Restore support for virtio on FreeBSD

[dpdk-dev] [PATCH] Restore support for virtio on FreeBSD

[Raz Amir]

Signed-off-by: Raz Amir <razamir22@gmail.com>
---
 lib/librte_eal/bsdapp/eal/eal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 871d5f4..e20f915 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
 	fd = open("/dev/io", O_RDWR);
 	if (fd < 0)
 		return -1;
-	close(fd);
+	/* keep fd open for iopl */
 	return 0;
 }
 
-- 
2.1.2

[dpdk-dev] [PATCH v2] Restore support for virtio on FreeBSD

[Raz Amir]

Fixes: 8a312224bcde ("eal/bsd: fix fd leak")

Signed-off-by: Raz Amir <razamir22@gmail.com>
---
 lib/librte_eal/bsdapp/eal/eal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 871d5f4..e20f915 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
 	fd = open("/dev/io", O_RDWR);
 	if (fd < 0)
 		return -1;
-	close(fd);
+	/* keep fd open for iopl */
 	return 0;
 }
 
-- 
2.1.2

[dpdk-dev] [PATCH v3] Restore support for virtio on FreeBSD

[Raz Amir]

Fixes: 8a312224bcde ("eal/bsd: fix fd leak")

Signed-off-by: Raz Amir <razamir22@gmail.com>
---
 lib/librte_eal/bsdapp/eal/eal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 871d5f4..e20f915 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
 	fd = open("/dev/io", O_RDWR);
 	if (fd < 0)
 		return -1;
-	close(fd);
+	/* keep fd open for iopl */
 	return 0;
 }
 
-- 
2.1.2

Re: [dpdk-dev] [PATCH v3] Restore support for virtio on FreeBSD

[Thomas Monjalon]

Please provide more information in the commit message.
We need to know what was the problem (crash) in the git history.
Then when doing git blame, we'll have the full explanation.

2015-04-13 15:19, Raz Amir:
> Fixes: 8a312224bcde ("eal/bsd: fix fd leak")
> 
> Signed-off-by: Raz Amir <razamir22@gmail.com>
> ---
>  lib/librte_eal/bsdapp/eal/eal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 871d5f4..e20f915 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
>  	fd = open("/dev/io", O_RDWR);
>  	if (fd < 0)
>  		return -1;
> -	close(fd);
> +	/* keep fd open for iopl */

Yes we need a comment but "for iopl" is not descriptive and
not very accurate as iopl is a Linux mechanism.

>  	return 0;
>  }

Thanks

Re: [dpdk-dev] [PATCH v3] Restore support for virtio on FreeBSD

[Ouyang, Changchun]

Hi 

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Monday, April 13, 2015 8:55 PM
> To: Raz Amir
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] Restore support for virtio on FreeBSD
> 
> Please provide more information in the commit message.
> We need to know what was the problem (crash) in the git history.
> Then when doing git blame, we'll have the full explanation.
> 
> 2015-04-13 15:19, Raz Amir:
> > Fixes: 8a312224bcde ("eal/bsd: fix fd leak")
> >
> > Signed-off-by: Raz Amir <razamir22@gmail.com>
> > ---
> >  lib/librte_eal/bsdapp/eal/eal.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/eal.c
> > b/lib/librte_eal/bsdapp/eal/eal.c index 871d5f4..e20f915 100644
> > --- a/lib/librte_eal/bsdapp/eal/eal.c
> > +++ b/lib/librte_eal/bsdapp/eal/eal.c
> > @@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
> >  	fd = open("/dev/io", O_RDWR);
> >  	if (fd < 0)
> >  		return -1;
> > -	close(fd);
> > +	/* keep fd open for iopl */

Copy and paste my comment into this new patch:
Would you pls think about this solution?
Declare a static var to keep the fd which is opened for freebsd;
Then define a deinit function for virtio device, Inside the deinit function, close the fd which was opened in init stage.
Done.

thanks
Changchun

Re: [dpdk-dev] [PATCH v3] Restore support for virtio on FreeBSD

[Raz Amir]

Thomas, I will add more information to the commit message, but regarding
your feedback on the iopl comment, it is called also iopl on FreeBSD.
See this link to FreeBSD source code, for the io driver code - the flag name
is PSL_IOPL:
https://github.com/freebsd/freebsd/blob/master/sys/i386/i386/io.c#L38

Ouyang, I will implement your suggestion in the next patch version I submit.


-----Original Message-----
From: Ouyang, Changchun [mailto:changchun.ouyang@intel.com] 
Sent: 14 April 2015 05:33
To: Thomas Monjalon; Raz Amir
Cc: dev@dpdk.org; Ouyang, Changchun
Subject: RE: [dpdk-dev] [PATCH v3] Restore support for virtio on FreeBSD

Hi 

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Thomas Monjalon
> Sent: Monday, April 13, 2015 8:55 PM
> To: Raz Amir
> Cc: dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v3] Restore support for virtio on 
> FreeBSD
> 
> Please provide more information in the commit message.
> We need to know what was the problem (crash) in the git history.
> Then when doing git blame, we'll have the full explanation.
> 
> 2015-04-13 15:19, Raz Amir:
> > Fixes: 8a312224bcde ("eal/bsd: fix fd leak")
> >
> > Signed-off-by: Raz Amir <razamir22@gmail.com>
> > ---
> >  lib/librte_eal/bsdapp/eal/eal.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/eal.c 
> > b/lib/librte_eal/bsdapp/eal/eal.c index 871d5f4..e20f915 100644
> > --- a/lib/librte_eal/bsdapp/eal/eal.c
> > +++ b/lib/librte_eal/bsdapp/eal/eal.c
> > @@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
> >  	fd = open("/dev/io", O_RDWR);
> >  	if (fd < 0)
> >  		return -1;
> > -	close(fd);
> > +	/* keep fd open for iopl */

Copy and paste my comment into this new patch:
Would you pls think about this solution?
Declare a static var to keep the fd which is opened for freebsd; Then define
a deinit function for virtio device, Inside the deinit function, close the
fd which was opened in init stage.
Done.

thanks
Changchun

[dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD

[Raz Amir]

Fixes: 8a312224bcde ("eal/bsd: fix fd leak")

Closing /dev/io fd causes SIGBUS in inb/outb instructions
as the process loses the IOPL privileges once the fd is closed:
(gdb) bt
0  0x0000000000492f2c in outb (port=49170, data=0 '\000')
    at /usr/include/machine/cpufunc.h:244
1  0x0000000000492f7a in outb_p (data=0 '\000', port=49170)
    at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211
2  0x000000000049328d in vtpci_set_status (hw=0x80331f380, status=0 '\000')
    at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130
3  0x00000000004931fe in vtpci_reset (hw=0x80331f380)
    at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108
4  0x00000000004a175e in eth_virtio_dev_init (eth_dev=0x831b80 <rte_eth_devices>)
    at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150
5  0x0000000000462c09 in rte_eth_dev_init (pci_drv=0x79d1a0 <rte_virtio_pmd>,
    pci_dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326
6  0x000000000046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0 <rte_virtio_pmd>,
    dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487
7  0x0000000000475b06 in pci_probe_all_drivers (dev=0x802417560)
    at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116
8  0x0000000000475bb9 in rte_eal_pci_probe ()
    at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246
9  0x000000000046cd63 in rte_eal_init (argc=5, argv=0x7fffffffeaf0)
    at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554
10 0x0000000000404544 in main ()

Signed-off-by: Raz Amir <razamir22@gmail.com>
---
 lib/librte_eal/bsdapp/eal/eal.c         | 19 ++++++++++++++-----
 lib/librte_eal/common/include/rte_eal.h | 10 ++++++++++
 lib/librte_eal/linuxapp/eal/eal.c       |  5 +++++
 lib/librte_pmd_virtio/virtio_ethdev.c   |  9 +++++++++
 4 files changed, 38 insertions(+), 5 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 871d5f4..687dd83 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -112,6 +112,9 @@ struct internal_config internal_config;
 /* used by rte_rdtsc() */
 int rte_cycles_vmware_tsc_map;
 
+/* fd to keep open for iopl */
+static int iopl_fd = -1;
+
 /* Return a pointer to the configuration structure */
 struct rte_config *
 rte_eal_get_configuration(void)
@@ -421,15 +424,21 @@ int rte_eal_has_hugepages(void)
 int
 rte_eal_iopl_init(void)
 {
-	int fd;
-
-	fd = open("/dev/io", O_RDWR);
-	if (fd < 0)
+	iopl_fd = open("/dev/io", O_RDWR);
+	if (iopl_fd < 0)
 		return -1;
-	close(fd);
+	/* keep fd open for iopl */
 	return 0;
 }
 
+void
+rte_eal_iopl_uninit(void)
+{
+	if (iopl_fd != -1)
+		close(iopl_fd);
+	iopl_fd = -1;
+}
+
 /* Launch threads, called at application init(). */
 int
 rte_eal_init(int argc, char **argv)
diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
index 1385a73..9151e08 100644
--- a/lib/librte_eal/common/include/rte_eal.h
+++ b/lib/librte_eal/common/include/rte_eal.h
@@ -127,6 +127,16 @@ enum rte_proc_type_t rte_eal_process_type(void);
 int rte_eal_iopl_init(void);
 
 /**
+ * Release iopl priviledge - currently relevant only for FreeBSD.
+ *
+ * This function should be called by pmds which need access to ioports.
+
+ * @return
+ *   void
+ */
+void rte_eal_iopl_uninit(void);
+
+/**
  * Initialize the Environment Abstraction Layer (EAL).
  *
  * This function is to be executed on the MASTER lcore only, as soon
diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
index bd770cf..687cebf 100644
--- a/lib/librte_eal/linuxapp/eal/eal.c
+++ b/lib/librte_eal/linuxapp/eal/eal.c
@@ -695,6 +695,11 @@ rte_eal_iopl_init(void)
 #endif
 }
 
+void
+rte_eal_iopl_uninit(void)
+{
+}
+
 /* Launch threads, called at application init(). */
 int
 rte_eal_init(int argc, char **argv)
diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
index 7b83d9b..5be5c27 100644
--- a/lib/librte_pmd_virtio/virtio_ethdev.c
+++ b/lib/librte_pmd_virtio/virtio_ethdev.c
@@ -1265,6 +1265,14 @@ rte_virtio_pmd_init(const char *name __rte_unused,
 	return 0;
 }
 
+static int
+rte_virtio_pmd_uninit(const char *name)
+{
+	(void)name;
+	rte_eal_iopl_uninit();
+	return 0;
+}
+
 /*
  * Only 1 queue is supported, no queue release related operation
  */
@@ -1499,6 +1507,7 @@ __rte_unused uint8_t is_rx)
 static struct rte_driver rte_virtio_driver = {
 	.type = PMD_PDEV,
 	.init = rte_virtio_pmd_init,
+	.uninit = rte_virtio_pmd_uninit,
 };
 
 PMD_REGISTER_DRIVER(rte_virtio_driver);
-- 
2.1.2

Re: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD

[Ananyev, Konstantin]

Hi,

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Raz Amir
> Sent: Tuesday, April 14, 2015 5:23 PM
> To: dev@dpdk.org
> Cc: Raz Amir
> Subject: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD
> 
> Fixes: 8a312224bcde ("eal/bsd: fix fd leak")
> 
> Closing /dev/io fd causes SIGBUS in inb/outb instructions
> as the process loses the IOPL privileges once the fd is closed:
> (gdb) bt
> 0  0x0000000000492f2c in outb (port=49170, data=0 '\000')
>     at /usr/include/machine/cpufunc.h:244
> 1  0x0000000000492f7a in outb_p (data=0 '\000', port=49170)
>     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211
> 2  0x000000000049328d in vtpci_set_status (hw=0x80331f380, status=0 '\000')
>     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130
> 3  0x00000000004931fe in vtpci_reset (hw=0x80331f380)
>     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108
> 4  0x00000000004a175e in eth_virtio_dev_init (eth_dev=0x831b80 <rte_eth_devices>)
>     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150
> 5  0x0000000000462c09 in rte_eth_dev_init (pci_drv=0x79d1a0 <rte_virtio_pmd>,
>     pci_dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326
> 6  0x000000000046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0 <rte_virtio_pmd>,
>     dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487
> 7  0x0000000000475b06 in pci_probe_all_drivers (dev=0x802417560)
>     at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116
> 8  0x0000000000475bb9 in rte_eal_pci_probe ()
>     at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246
> 9  0x000000000046cd63 in rte_eal_init (argc=5, argv=0x7fffffffeaf0)
>     at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554
> 10 0x0000000000404544 in main ()
> 
> Signed-off-by: Raz Amir <razamir22@gmail.com>
> ---
>  lib/librte_eal/bsdapp/eal/eal.c         | 19 ++++++++++++++-----
>  lib/librte_eal/common/include/rte_eal.h | 10 ++++++++++
>  lib/librte_eal/linuxapp/eal/eal.c       |  5 +++++
>  lib/librte_pmd_virtio/virtio_ethdev.c   |  9 +++++++++
>  4 files changed, 38 insertions(+), 5 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 871d5f4..687dd83 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -112,6 +112,9 @@ struct internal_config internal_config;
>  /* used by rte_rdtsc() */
>  int rte_cycles_vmware_tsc_map;
> 
> +/* fd to keep open for iopl */
> +static int iopl_fd = -1;
> +
>  /* Return a pointer to the configuration structure */
>  struct rte_config *
>  rte_eal_get_configuration(void)
> @@ -421,15 +424,21 @@ int rte_eal_has_hugepages(void)
>  int
>  rte_eal_iopl_init(void)
>  {
> -	int fd;
> -
> -	fd = open("/dev/io", O_RDWR);
> -	if (fd < 0)
> +	iopl_fd = open("/dev/io", O_RDWR);
> +	if (iopl_fd < 0)
>  		return -1;
> -	close(fd);
> +	/* keep fd open for iopl */
>  	return 0;
>  }
> 
> +void
> +rte_eal_iopl_uninit(void)
> +{
> +	if (iopl_fd != -1)
> +		close(iopl_fd);
> +	iopl_fd = -1;
> +}

Did I get it right: that function would be invoked for at dev_detach()?
And after we invoked it, we still we can have other multiple virtio devices attached and active?
If so, then I suppose you'll hit the same problem again.
Konstantin  

> +
>  /* Launch threads, called at application init(). */
>  int
>  rte_eal_init(int argc, char **argv)
> diff --git a/lib/librte_eal/common/include/rte_eal.h b/lib/librte_eal/common/include/rte_eal.h
> index 1385a73..9151e08 100644
> --- a/lib/librte_eal/common/include/rte_eal.h
> +++ b/lib/librte_eal/common/include/rte_eal.h
> @@ -127,6 +127,16 @@ enum rte_proc_type_t rte_eal_process_type(void);
>  int rte_eal_iopl_init(void);
> 
>  /**
> + * Release iopl priviledge - currently relevant only for FreeBSD.
> + *
> + * This function should be called by pmds which need access to ioports.
> +
> + * @return
> + *   void
> + */
> +void rte_eal_iopl_uninit(void);
> +
> +/**
>   * Initialize the Environment Abstraction Layer (EAL).
>   *
>   * This function is to be executed on the MASTER lcore only, as soon
> diff --git a/lib/librte_eal/linuxapp/eal/eal.c b/lib/librte_eal/linuxapp/eal/eal.c
> index bd770cf..687cebf 100644
> --- a/lib/librte_eal/linuxapp/eal/eal.c
> +++ b/lib/librte_eal/linuxapp/eal/eal.c
> @@ -695,6 +695,11 @@ rte_eal_iopl_init(void)
>  #endif
>  }
> 
> +void
> +rte_eal_iopl_uninit(void)
> +{
> +}
> +
>  /* Launch threads, called at application init(). */
>  int
>  rte_eal_init(int argc, char **argv)
> diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
> index 7b83d9b..5be5c27 100644
> --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> @@ -1265,6 +1265,14 @@ rte_virtio_pmd_init(const char *name __rte_unused,
>  	return 0;
>  }
> 
> +static int
> +rte_virtio_pmd_uninit(const char *name)
> +{
> +	(void)name;
> +	rte_eal_iopl_uninit();
> +	return 0;
> +}
> +
>  /*
>   * Only 1 queue is supported, no queue release related operation
>   */
> @@ -1499,6 +1507,7 @@ __rte_unused uint8_t is_rx)
>  static struct rte_driver rte_virtio_driver = {
>  	.type = PMD_PDEV,
>  	.init = rte_virtio_pmd_init,
> +	.uninit = rte_virtio_pmd_uninit,
>  };
> 
>  PMD_REGISTER_DRIVER(rte_virtio_driver);
> --
> 2.1.2

Re: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD

[Ouyang, Changchun]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev,
> Konstantin
> Sent: Wednesday, April 15, 2015 6:22 AM
> To: Raz Amir; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD
> 
> Hi,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Raz Amir
> > Sent: Tuesday, April 14, 2015 5:23 PM
> > To: dev@dpdk.org
> > Cc: Raz Amir
> > Subject: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD
> >
> > Fixes: 8a312224bcde ("eal/bsd: fix fd leak")
> >
> > Closing /dev/io fd causes SIGBUS in inb/outb instructions as the
> > process loses the IOPL privileges once the fd is closed:
> > (gdb) bt
> > 0  0x0000000000492f2c in outb (port=49170, data=0 '\000')
> >     at /usr/include/machine/cpufunc.h:244
> > 1  0x0000000000492f7a in outb_p (data=0 '\000', port=49170)
> >     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211
> > 2  0x000000000049328d in vtpci_set_status (hw=0x80331f380, status=0
> '\000')
> >     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130
> > 3  0x00000000004931fe in vtpci_reset (hw=0x80331f380)
> >     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108
> > 4  0x00000000004a175e in eth_virtio_dev_init (eth_dev=0x831b80
> <rte_eth_devices>)
> >     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150
> > 5  0x0000000000462c09 in rte_eth_dev_init (pci_drv=0x79d1a0
> <rte_virtio_pmd>,
> >     pci_dev=0x802417560) at
> > /dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326
> > 6  0x000000000046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0
> <rte_virtio_pmd>,
> >     dev=0x802417560) at
> > /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487
> > 7  0x0000000000475b06 in pci_probe_all_drivers (dev=0x802417560)
> >     at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116
> > 8  0x0000000000475bb9 in rte_eal_pci_probe ()
> >     at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246
> > 9  0x000000000046cd63 in rte_eal_init (argc=5, argv=0x7fffffffeaf0)
> >     at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554
> > 10 0x0000000000404544 in main ()
> >
> > Signed-off-by: Raz Amir <razamir22@gmail.com>
> > ---
> >  lib/librte_eal/bsdapp/eal/eal.c         | 19 ++++++++++++++-----
> >  lib/librte_eal/common/include/rte_eal.h | 10 ++++++++++
> >  lib/librte_eal/linuxapp/eal/eal.c       |  5 +++++
> >  lib/librte_pmd_virtio/virtio_ethdev.c   |  9 +++++++++
> >  4 files changed, 38 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/eal.c
> > b/lib/librte_eal/bsdapp/eal/eal.c index 871d5f4..687dd83 100644
> > --- a/lib/librte_eal/bsdapp/eal/eal.c
> > +++ b/lib/librte_eal/bsdapp/eal/eal.c
> > @@ -112,6 +112,9 @@ struct internal_config internal_config;
> >  /* used by rte_rdtsc() */
> >  int rte_cycles_vmware_tsc_map;
> >
> > +/* fd to keep open for iopl */
> > +static int iopl_fd = -1;
> > +
> >  /* Return a pointer to the configuration structure */  struct
> > rte_config *
> >  rte_eal_get_configuration(void)
> > @@ -421,15 +424,21 @@ int rte_eal_has_hugepages(void)  int
> >  rte_eal_iopl_init(void)
> >  {
> > -	int fd;
> > -
> > -	fd = open("/dev/io", O_RDWR);
> > -	if (fd < 0)
> > +	iopl_fd = open("/dev/io", O_RDWR);
> > +	if (iopl_fd < 0)
> >  		return -1;
> > -	close(fd);
> > +	/* keep fd open for iopl */
> >  	return 0;
> >  }
> >
> > +void
> > +rte_eal_iopl_uninit(void)
> > +{
> > +	if (iopl_fd != -1)
> > +		close(iopl_fd);
> > +	iopl_fd = -1;
> > +}
> 
> Did I get it right: that function would be invoked for at dev_detach()?
> And after we invoked it, we still we can have other multiple virtio devices
> attached and active?
> If so, then I suppose you'll hit the same problem again.
> Konstantin
> 

Yes, need verify this issue,
If it is true, may use reference counter to resolve it?
Thanks
Changchun
 
> > +
> >  /* Launch threads, called at application init(). */  int
> > rte_eal_init(int argc, char **argv) diff --git
> > a/lib/librte_eal/common/include/rte_eal.h
> > b/lib/librte_eal/common/include/rte_eal.h
> > index 1385a73..9151e08 100644
> > --- a/lib/librte_eal/common/include/rte_eal.h
> > +++ b/lib/librte_eal/common/include/rte_eal.h
> > @@ -127,6 +127,16 @@ enum rte_proc_type_t
> rte_eal_process_type(void);
> > int rte_eal_iopl_init(void);
> >
> >  /**
> > + * Release iopl priviledge - currently relevant only for FreeBSD.
> > + *
> > + * This function should be called by pmds which need access to ioports.
> > +
> > + * @return
> > + *   void
> > + */
> > +void rte_eal_iopl_uninit(void);
> > +
> > +/**
> >   * Initialize the Environment Abstraction Layer (EAL).
> >   *
> >   * This function is to be executed on the MASTER lcore only, as soon
> > diff --git a/lib/librte_eal/linuxapp/eal/eal.c
> > b/lib/librte_eal/linuxapp/eal/eal.c
> > index bd770cf..687cebf 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal.c
> > @@ -695,6 +695,11 @@ rte_eal_iopl_init(void)  #endif  }
> >
> > +void
> > +rte_eal_iopl_uninit(void)
> > +{
> > +}
> > +
> >  /* Launch threads, called at application init(). */  int
> > rte_eal_init(int argc, char **argv) diff --git
> > a/lib/librte_pmd_virtio/virtio_ethdev.c
> > b/lib/librte_pmd_virtio/virtio_ethdev.c
> > index 7b83d9b..5be5c27 100644
> > --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> > @@ -1265,6 +1265,14 @@ rte_virtio_pmd_init(const char *name
> __rte_unused,
> >  	return 0;
> >  }
> >
> > +static int
> > +rte_virtio_pmd_uninit(const char *name) {
> > +	(void)name;
> > +	rte_eal_iopl_uninit();
> > +	return 0;
> > +}
> > +
> >  /*
> >   * Only 1 queue is supported, no queue release related operation
> >   */
> > @@ -1499,6 +1507,7 @@ __rte_unused uint8_t is_rx)  static struct
> > rte_driver rte_virtio_driver = {
> >  	.type = PMD_PDEV,
> >  	.init = rte_virtio_pmd_init,
> > +	.uninit = rte_virtio_pmd_uninit,
> >  };
> >
> >  PMD_REGISTER_DRIVER(rte_virtio_driver);
> > --
> > 2.1.2

Re: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD

[Raz Amir]

Hi,

>From both running and reading the code, the rte_virtio_pmd_init is called
only once from: rte_eal_init -> rte_eal_dev_init.
But, the uninit won't be called, since uninit it called only for PMD_VDEV
driver types, while virtio is PMD_PDEV.
Based on that, I am going to submit the original patch again, that fd won't
be closed, and without handling the close at uninit as it isn't called and
since fd will be closed anyway when the process exits.

Thanks,
Raz.

-----Original Message-----
From: Ouyang, Changchun [mailto:changchun.ouyang@intel.com] 
Sent: 16 April 2015 06:31
To: Ananyev, Konstantin; Raz Amir; dev@dpdk.org
Cc: Ouyang, Changchun
Subject: RE: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD



> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev, 
> Konstantin
> Sent: Wednesday, April 15, 2015 6:22 AM
> To: Raz Amir; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v5] Restore support for virtio on 
> FreeBSD
> 
> Hi,
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Raz Amir
> > Sent: Tuesday, April 14, 2015 5:23 PM
> > To: dev@dpdk.org
> > Cc: Raz Amir
> > Subject: [dpdk-dev] [PATCH v5] Restore support for virtio on FreeBSD
> >
> > Fixes: 8a312224bcde ("eal/bsd: fix fd leak")
> >
> > Closing /dev/io fd causes SIGBUS in inb/outb instructions as the 
> > process loses the IOPL privileges once the fd is closed:
> > (gdb) bt
> > 0  0x0000000000492f2c in outb (port=49170, data=0 '\000')
> >     at /usr/include/machine/cpufunc.h:244
> > 1  0x0000000000492f7a in outb_p (data=0 '\000', port=49170)
> >     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211
> > 2  0x000000000049328d in vtpci_set_status (hw=0x80331f380, status=0
> '\000')
> >     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130
> > 3  0x00000000004931fe in vtpci_reset (hw=0x80331f380)
> >     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108
> > 4  0x00000000004a175e in eth_virtio_dev_init (eth_dev=0x831b80
> <rte_eth_devices>)
> >     at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150
> > 5  0x0000000000462c09 in rte_eth_dev_init (pci_drv=0x79d1a0
> <rte_virtio_pmd>,
> >     pci_dev=0x802417560) at
> > /dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326
> > 6  0x000000000046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0
> <rte_virtio_pmd>,
> >     dev=0x802417560) at
> > /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487
> > 7  0x0000000000475b06 in pci_probe_all_drivers (dev=0x802417560)
> >     at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116
> > 8  0x0000000000475bb9 in rte_eal_pci_probe ()
> >     at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246
> > 9  0x000000000046cd63 in rte_eal_init (argc=5, argv=0x7fffffffeaf0)
> >     at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554
> > 10 0x0000000000404544 in main ()
> >
> > Signed-off-by: Raz Amir <razamir22@gmail.com>
> > ---
> >  lib/librte_eal/bsdapp/eal/eal.c         | 19 ++++++++++++++-----
> >  lib/librte_eal/common/include/rte_eal.h | 10 ++++++++++
> >  lib/librte_eal/linuxapp/eal/eal.c       |  5 +++++
> >  lib/librte_pmd_virtio/virtio_ethdev.c   |  9 +++++++++
> >  4 files changed, 38 insertions(+), 5 deletions(-)
> >
> > diff --git a/lib/librte_eal/bsdapp/eal/eal.c 
> > b/lib/librte_eal/bsdapp/eal/eal.c index 871d5f4..687dd83 100644
> > --- a/lib/librte_eal/bsdapp/eal/eal.c
> > +++ b/lib/librte_eal/bsdapp/eal/eal.c
> > @@ -112,6 +112,9 @@ struct internal_config internal_config;
> >  /* used by rte_rdtsc() */
> >  int rte_cycles_vmware_tsc_map;
> >
> > +/* fd to keep open for iopl */
> > +static int iopl_fd = -1;
> > +
> >  /* Return a pointer to the configuration structure */  struct 
> > rte_config *
> >  rte_eal_get_configuration(void)
> > @@ -421,15 +424,21 @@ int rte_eal_has_hugepages(void)  int
> >  rte_eal_iopl_init(void)
> >  {
> > -	int fd;
> > -
> > -	fd = open("/dev/io", O_RDWR);
> > -	if (fd < 0)
> > +	iopl_fd = open("/dev/io", O_RDWR);
> > +	if (iopl_fd < 0)
> >  		return -1;
> > -	close(fd);
> > +	/* keep fd open for iopl */
> >  	return 0;
> >  }
> >
> > +void
> > +rte_eal_iopl_uninit(void)
> > +{
> > +	if (iopl_fd != -1)
> > +		close(iopl_fd);
> > +	iopl_fd = -1;
> > +}
> 
> Did I get it right: that function would be invoked for at dev_detach()?
> And after we invoked it, we still we can have other multiple virtio 
> devices attached and active?
> If so, then I suppose you'll hit the same problem again.
> Konstantin
> 

Yes, need verify this issue,
If it is true, may use reference counter to resolve it?
Thanks
Changchun
 
> > +
> >  /* Launch threads, called at application init(). */  int 
> > rte_eal_init(int argc, char **argv) diff --git 
> > a/lib/librte_eal/common/include/rte_eal.h
> > b/lib/librte_eal/common/include/rte_eal.h
> > index 1385a73..9151e08 100644
> > --- a/lib/librte_eal/common/include/rte_eal.h
> > +++ b/lib/librte_eal/common/include/rte_eal.h
> > @@ -127,6 +127,16 @@ enum rte_proc_type_t
> rte_eal_process_type(void);
> > int rte_eal_iopl_init(void);
> >
> >  /**
> > + * Release iopl priviledge - currently relevant only for FreeBSD.
> > + *
> > + * This function should be called by pmds which need access to ioports.
> > +
> > + * @return
> > + *   void
> > + */
> > +void rte_eal_iopl_uninit(void);
> > +
> > +/**
> >   * Initialize the Environment Abstraction Layer (EAL).
> >   *
> >   * This function is to be executed on the MASTER lcore only, as 
> > soon diff --git a/lib/librte_eal/linuxapp/eal/eal.c
> > b/lib/librte_eal/linuxapp/eal/eal.c
> > index bd770cf..687cebf 100644
> > --- a/lib/librte_eal/linuxapp/eal/eal.c
> > +++ b/lib/librte_eal/linuxapp/eal/eal.c
> > @@ -695,6 +695,11 @@ rte_eal_iopl_init(void)  #endif  }
> >
> > +void
> > +rte_eal_iopl_uninit(void)
> > +{
> > +}
> > +
> >  /* Launch threads, called at application init(). */  int 
> > rte_eal_init(int argc, char **argv) diff --git 
> > a/lib/librte_pmd_virtio/virtio_ethdev.c
> > b/lib/librte_pmd_virtio/virtio_ethdev.c
> > index 7b83d9b..5be5c27 100644
> > --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> > @@ -1265,6 +1265,14 @@ rte_virtio_pmd_init(const char *name
> __rte_unused,
> >  	return 0;
> >  }
> >
> > +static int
> > +rte_virtio_pmd_uninit(const char *name) {
> > +	(void)name;
> > +	rte_eal_iopl_uninit();
> > +	return 0;
> > +}
> > +
> >  /*
> >   * Only 1 queue is supported, no queue release related operation
> >   */
> > @@ -1499,6 +1507,7 @@ __rte_unused uint8_t is_rx)  static struct 
> > rte_driver rte_virtio_driver = {
> >  	.type = PMD_PDEV,
> >  	.init = rte_virtio_pmd_init,
> > +	.uninit = rte_virtio_pmd_uninit,
> >  };
> >
> >  PMD_REGISTER_DRIVER(rte_virtio_driver);
> > --
> > 2.1.2

[dpdk-dev] [PATCH v6] Restore support for virtio on FreeBSD

[Raz Amir]

Fixes: 8a312224bcde ("eal/bsd: fix fd leak")

Closing /dev/io fd causes SIGBUS in inb/outb instructions
as the process loses the IOPL privileges once the fd is closed:
(gdb) bt
0  0x0000000000492f2c in outb (port=49170, data=0 '\000')
at /usr/include/machine/cpufunc.h:244
1  0x0000000000492f7a in outb_p (data=0 '\000', port=49170)
at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211
2  0x000000000049328d in vtpci_set_status (hw=0x80331f380, status=0 '\000')
at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130
3  0x00000000004931fe in vtpci_reset (hw=0x80331f380)
at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108
4  0x00000000004a175e in eth_virtio_dev_init (eth_dev=0x831b80 <rte_eth_devices>)
at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150
5  0x0000000000462c09 in rte_eth_dev_init (pci_drv=0x79d1a0 <rte_virtio_pmd>,
pci_dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326
6  0x000000000046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0 <rte_virtio_pmd>,
dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487
7  0x0000000000475b06 in pci_probe_all_drivers (dev=0x802417560)
at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116
8  0x0000000000475bb9 in rte_eal_pci_probe ()
at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246
9  0x000000000046cd63 in rte_eal_init (argc=5, argv=0x7fffffffeaf0)
at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554
10 0x0000000000404544 in main ()

Signed-off-by: Raz Amir <razamir22@gmail.com>
---
 lib/librte_eal/bsdapp/eal/eal.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 871d5f4..e20f915 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
 	fd = open("/dev/io", O_RDWR);
 	if (fd < 0)
 		return -1;
-	close(fd);
+	/* keep fd open for iopl */
 	return 0;
 }
 
-- 
2.1.2

Re: [dpdk-dev] [PATCH v6] Restore support for virtio on FreeBSD

[Bruce Richardson]

On Thu, Apr 16, 2015 at 11:02:03AM +0300, Raz Amir wrote:
> Fixes: 8a312224bcde ("eal/bsd: fix fd leak")
> 
> Closing /dev/io fd causes SIGBUS in inb/outb instructions
> as the process loses the IOPL privileges once the fd is closed:
> (gdb) bt
> 0  0x0000000000492f2c in outb (port=49170, data=0 '\000')
> at /usr/include/machine/cpufunc.h:244
> 1  0x0000000000492f7a in outb_p (data=0 '\000', port=49170)
> at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211
> 2  0x000000000049328d in vtpci_set_status (hw=0x80331f380, status=0 '\000')
> at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130
> 3  0x00000000004931fe in vtpci_reset (hw=0x80331f380)
> at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108
> 4  0x00000000004a175e in eth_virtio_dev_init (eth_dev=0x831b80 <rte_eth_devices>)
> at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150
> 5  0x0000000000462c09 in rte_eth_dev_init (pci_drv=0x79d1a0 <rte_virtio_pmd>,
> pci_dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326
> 6  0x000000000046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0 <rte_virtio_pmd>,
> dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487
> 7  0x0000000000475b06 in pci_probe_all_drivers (dev=0x802417560)
> at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116
> 8  0x0000000000475bb9 in rte_eal_pci_probe ()
> at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246
> 9  0x000000000046cd63 in rte_eal_init (argc=5, argv=0x7fffffffeaf0)
> at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554
> 10 0x0000000000404544 in main ()
> 
> Signed-off-by: Raz Amir <razamir22@gmail.com>

It does look the cleanest solution, though I don't like the idea of leaking 
the file handle. Can you perhaps just change things a little so that the fd
variable is static - even locally in the function will do, as it helps indicate
that the fd is persistent.

/Bruce

> ---
>  lib/librte_eal/bsdapp/eal/eal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 871d5f4..e20f915 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
>  	fd = open("/dev/io", O_RDWR);
>  	if (fd < 0)
>  		return -1;
> -	close(fd);
> +	/* keep fd open for iopl */
>  	return 0;
>  }
>  
> -- 
> 2.1.2
> 

Re: [dpdk-dev] [PATCH v6] Restore support for virtio on FreeBSD

[Raz Amir]

Will do

-----Original Message-----
From: Bruce Richardson [mailto:bruce.richardson@intel.com] 
Sent: 16 April 2015 12:39
To: Raz Amir
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH v6] Restore support for virtio on FreeBSD

On Thu, Apr 16, 2015 at 11:02:03AM +0300, Raz Amir wrote:
> Fixes: 8a312224bcde ("eal/bsd: fix fd leak")
> 
> Closing /dev/io fd causes SIGBUS in inb/outb instructions as the 
> process loses the IOPL privileges once the fd is closed:
> (gdb) bt
> 0  0x0000000000492f2c in outb (port=49170, data=0 '\000') at 
> /usr/include/machine/cpufunc.h:244
> 1  0x0000000000492f7a in outb_p (data=0 '\000', port=49170) at 
> /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211
> 2  0x000000000049328d in vtpci_set_status (hw=0x80331f380, status=0 
> '\000') at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130
> 3  0x00000000004931fe in vtpci_reset (hw=0x80331f380) at 
> /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108
> 4  0x00000000004a175e in eth_virtio_dev_init (eth_dev=0x831b80 
> <rte_eth_devices>) at 
> /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150
> 5  0x0000000000462c09 in rte_eth_dev_init (pci_drv=0x79d1a0 
> <rte_virtio_pmd>,
> pci_dev=0x802417560) at 
> /dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326
> 6  0x000000000046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0 
> <rte_virtio_pmd>,
> dev=0x802417560) at 
> /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487
> 7  0x0000000000475b06 in pci_probe_all_drivers (dev=0x802417560) at 
> /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116
> 8  0x0000000000475bb9 in rte_eal_pci_probe () at 
> /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246
> 9  0x000000000046cd63 in rte_eal_init (argc=5, argv=0x7fffffffeaf0) at 
> /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554
> 10 0x0000000000404544 in main ()
> 
> Signed-off-by: Raz Amir <razamir22@gmail.com>

It does look the cleanest solution, though I don't like the idea of leaking
the file handle. Can you perhaps just change things a little so that the fd
variable is static - even locally in the function will do, as it helps
indicate that the fd is persistent.

/Bruce

> ---
>  lib/librte_eal/bsdapp/eal/eal.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c 
> b/lib/librte_eal/bsdapp/eal/eal.c index 871d5f4..e20f915 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -426,7 +426,7 @@ rte_eal_iopl_init(void)
>  	fd = open("/dev/io", O_RDWR);
>  	if (fd < 0)
>  		return -1;
> -	close(fd);
> +	/* keep fd open for iopl */
>  	return 0;
>  }
>  
> --
> 2.1.2
> 

[dpdk-dev] [PATCH v7] Restore support for virtio on FreeBSD

[Raz Amir]

Fixes: 8a312224bcde ("eal/bsd: fix fd leak")

Closing /dev/io fd causes SIGBUS in inb/outb instructions
as the process loses the IOPL privileges once the fd is closed:
(gdb) bt
0  0x0000000000492f2c in outb (port=49170, data=0 '\000')
at /usr/include/machine/cpufunc.h:244
1  0x0000000000492f7a in outb_p (data=0 '\000', port=49170)
at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211
2  0x000000000049328d in vtpci_set_status (hw=0x80331f380, status=0 '\000')
at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130
3  0x00000000004931fe in vtpci_reset (hw=0x80331f380)
at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108
4  0x00000000004a175e in eth_virtio_dev_init (eth_dev=0x831b80 <rte_eth_devices>)
at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150
5  0x0000000000462c09 in rte_eth_dev_init (pci_drv=0x79d1a0 <rte_virtio_pmd>,
pci_dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326
6  0x000000000046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0 <rte_virtio_pmd>,
dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487
7  0x0000000000475b06 in pci_probe_all_drivers (dev=0x802417560)
at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116
8  0x0000000000475bb9 in rte_eal_pci_probe ()
at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246
9  0x000000000046cd63 in rte_eal_init (argc=5, argv=0x7fffffffeaf0)
at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554
10 0x0000000000404544 in main ()

Signed-off-by: Raz Amir <razamir22@gmail.com>
---
 lib/librte_eal/bsdapp/eal/eal.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 871d5f4..43e8a47 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -421,12 +421,12 @@ int rte_eal_has_hugepages(void)
 int
 rte_eal_iopl_init(void)
 {
-	int fd;
+	static int fd;
 
 	fd = open("/dev/io", O_RDWR);
 	if (fd < 0)
 		return -1;
-	close(fd);
+	/* keep fd open for iopl */
 	return 0;
 }
 
-- 
2.1.2

Re: [dpdk-dev] [PATCH v7] Restore support for virtio on FreeBSD

[Thomas Monjalon]

> Fixes: 8a312224bcde ("eal/bsd: fix fd leak")
> 
> Closing /dev/io fd causes SIGBUS in inb/outb instructions
> as the process loses the IOPL privileges once the fd is closed:
> (gdb) bt
> 0  0x0000000000492f2c in outb (port=49170, data=0 '\000')
> at /usr/include/machine/cpufunc.h:244
> 1  0x0000000000492f7a in outb_p (data=0 '\000', port=49170)
> at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.h:211
> 2  0x000000000049328d in vtpci_set_status (hw=0x80331f380, status=0 '\000')
> at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:130
> 3  0x00000000004931fe in vtpci_reset (hw=0x80331f380)
> at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_pci.c:108
> 4  0x00000000004a175e in eth_virtio_dev_init (eth_dev=0x831b80 <rte_eth_devices>)
> at /dpdk/dpdk-2.0.0/lib/librte_pmd_virtio/virtio_ethdev.c:1150
> 5  0x0000000000462c09 in rte_eth_dev_init (pci_drv=0x79d1a0 <rte_virtio_pmd>,
> pci_dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_ether/rte_ethdev.c:326
> 6  0x000000000046f03f in rte_eal_pci_probe_one_driver (dr=0x79d1a0 <rte_virtio_pmd>,
> dev=0x802417560) at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal_pci.c:487
> 7  0x0000000000475b06 in pci_probe_all_drivers (dev=0x802417560)
> at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:116
> 8  0x0000000000475bb9 in rte_eal_pci_probe ()
> at /dpdk/dpdk-2.0.0/lib/librte_eal/common/eal_common_pci.c:246
> 9  0x000000000046cd63 in rte_eal_init (argc=5, argv=0x7fffffffeaf0)
> at /dpdk/dpdk-2.0.0/lib/librte_eal/bsdapp/eal/eal.c:554
> 10 0x0000000000404544 in main ()
> 
> Signed-off-by: Raz Amir <razamir22@gmail.com>

Applied, thanks