DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/virtio: avoid annoying IOPL call related errors
       [not found] <CGME20181123141748eucas1p132a221972a87ec82b50d4a6c83bd9646@eucas1p1.samsung.com>
@ 2018-11-23 14:17 ` Ilya Maximets
       [not found]   ` <CGME20181123143625eucas1p1def3421fa13b5aec7204549932c75bb7@eucas1p1.samsung.com>
  0 siblings, 1 reply; 17+ messages in thread
From: Ilya Maximets @ 2018-11-23 14:17 UTC (permalink / raw)
  To: dev, David Marchand
  Cc: Maxime Coquelin, Tiwei Bie, Zhihong Wang, Thomas Monjalon,
	Ferruh Yigit, Ian Stokes, Kevin Traynor, Ilya Maximets

In case of running with not enough capabilities, i.e. running as
non-root user any application linked with DPDK prints the message
about IOPL call failure even if it was just called like
'./testpmd --help'. For example, this beaks most of the OVS unit
tests if it built with DPDK support.

Let's register the virtio driver unconditionally and print error
message while probing the device. Silent iopl() call left in the
constructor to have privileges as early as possible as it was before.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---

We can avoid test failures in OVS by filtering the output
like this:
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=77706

But it still looks very inconvenient for me to have this
message in the output of every command for the DPDK linked app.

 drivers/net/virtio/virtio_ethdev.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e1fe36a23..2ba66d291 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1760,6 +1760,11 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
+	if (rte_eal_iopl_init() != 0) {
+		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
+		return 1;
+	}
+
 	/* virtio pmd skips probe if device needs to work in vdpa mode */
 	if (vdpa_mode_selected(pci_dev->device.devargs))
 		return 1;
@@ -1785,11 +1790,7 @@ static struct rte_pci_driver rte_virtio_pmd = {
 
 RTE_INIT(rte_virtio_pmd_init)
 {
-	if (rte_eal_iopl_init() != 0) {
-		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
-		return;
-	}
-
+	rte_eal_iopl_init();
 	rte_pci_register(&rte_virtio_pmd);
 }
 
-- 
2.17.1

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

* [dpdk-dev] [PATCH v2] net/virtio: avoid annoying IOPL call related errors
       [not found]   ` <CGME20181123143625eucas1p1def3421fa13b5aec7204549932c75bb7@eucas1p1.samsung.com>
@ 2018-11-23 14:36     ` Ilya Maximets
  2018-11-23 15:02       ` David Marchand
                         ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Ilya Maximets @ 2018-11-23 14:36 UTC (permalink / raw)
  To: dev, David Marchand
  Cc: Maxime Coquelin, Tiwei Bie, Zhihong Wang, Thomas Monjalon,
	Ferruh Yigit, Ian Stokes, Kevin Traynor, Ilya Maximets

In case of running with not enough capabilities, i.e. running as
non-root user any application linked with DPDK prints the message
about IOPL call failure even if it was just called like
'./testpmd --help'. For example, this beaks most of the OVS unit
tests if it built with DPDK support.

Let's register the virtio driver unconditionally and print error
message while probing the device. Silent iopl() call left in the
constructor to have privileges as early as possible as it was before.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---

Version 2:
	* Fixed possible fd leak on BSD.

We can avoid test failures in OVS by filtering the output
like this:
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=77706

But it still looks very inconvenient for me to have this
message in the output of every command for the DPDK linked app.

 drivers/net/virtio/virtio_ethdev.c | 11 ++++++-----
 lib/librte_eal/bsdapp/eal/eal.c    |  6 ++++--
 2 files changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e1fe36a23..2ba66d291 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1760,6 +1760,11 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
+	if (rte_eal_iopl_init() != 0) {
+		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
+		return 1;
+	}
+
 	/* virtio pmd skips probe if device needs to work in vdpa mode */
 	if (vdpa_mode_selected(pci_dev->device.devargs))
 		return 1;
@@ -1785,11 +1790,7 @@ static struct rte_pci_driver rte_virtio_pmd = {
 
 RTE_INIT(rte_virtio_pmd_init)
 {
-	if (rte_eal_iopl_init() != 0) {
-		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
-		return;
-	}
-
+	rte_eal_iopl_init();
 	rte_pci_register(&rte_virtio_pmd);
 }
 
diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 508cbc46f..b8152a75c 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -556,9 +556,11 @@ int rte_eal_has_hugepages(void)
 int
 rte_eal_iopl_init(void)
 {
-	static int fd;
+	static int fd = -1;
+
+	if (fd < 0)
+		fd = open("/dev/io", O_RDWR);
 
-	fd = open("/dev/io", O_RDWR);
 	if (fd < 0)
 		return -1;
 	/* keep fd open for iopl */
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH v2] net/virtio: avoid annoying IOPL call related errors
  2018-11-23 14:36     ` [dpdk-dev] [PATCH v2] " Ilya Maximets
@ 2018-11-23 15:02       ` David Marchand
       [not found]       ` <CGME20181123153936eucas1p13bfbe13fcda92de7760ac768fcab43c5@eucas1p1.samsung.com>
  2018-11-23 17:15       ` [dpdk-dev] [PATCH v2] net/virtio: avoid annoying IOPL call related errors Timothy Redaelli
  2 siblings, 0 replies; 17+ messages in thread
From: David Marchand @ 2018-11-23 15:02 UTC (permalink / raw)
  To: i.maximets
  Cc: dev, maxime.coquelin, tiwei.bie, zhihong.wang, thomas,
	ferruh.yigit, ian.stokes, Kevin Traynor

On Fri, Nov 23, 2018 at 3:36 PM Ilya Maximets <i.maximets@samsung.com>
wrote:

> In case of running with not enough capabilities, i.e. running as
> non-root user any application linked with DPDK prints the message
> about IOPL call failure even if it was just called like
> './testpmd --help'. For example, this beaks most of the OVS unit
> tests if it built with DPDK support.
>

breaks


>
> Let's register the virtio driver unconditionally and print error
> message while probing the device. Silent iopl() call left in the
> constructor to have privileges as early as possible as it was before.
>

Yes, that's the important part to avoid issues with the interrupt thread
which is spawned later and inherits the capa from the thread running
rte_eal_init, iirc.


> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>

Reviewed-by: David Marchand <david.marchand@redhat.com>

I wonder if we could move the "new" iopl check even later when probing a
device since it only makes sense in legacy mode when using uio.
But this patch keeps previous behaviour.

diff --git a/lib/librte_eal/bsdapp/eal/eal.c
> b/lib/librte_eal/bsdapp/eal/eal.c
> index 508cbc46f..b8152a75c 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -556,9 +556,11 @@ int rte_eal_has_hugepages(void)
>  int
>  rte_eal_iopl_init(void)
>  {
> -       static int fd;
> +       static int fd = -1;
> +
> +       if (fd < 0)
> +               fd = open("/dev/io", O_RDWR);
>
> -       fd = open("/dev/io", O_RDWR);
>         if (fd < 0)
>                 return -1;
>         /* keep fd open for iopl */
>

Good catch.
Should be a separate fix.


-- 
David Marchand

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

* [dpdk-dev] [PATCH v3 0/2] IOPL related fixes
       [not found]       ` <CGME20181123153936eucas1p13bfbe13fcda92de7760ac768fcab43c5@eucas1p1.samsung.com>
@ 2018-11-23 15:39         ` Ilya Maximets
       [not found]           ` <CGME20181123153947eucas1p169a2b7cec02b2edd7258aef11b1c1e2e@eucas1p1.samsung.com>
                             ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Ilya Maximets @ 2018-11-23 15:39 UTC (permalink / raw)
  To: dev, David Marchand
  Cc: Maxime Coquelin, Tiwei Bie, Zhihong Wang, Thomas Monjalon,
	Ferruh Yigit, Ian Stokes, Kevin Traynor, Bruce Richardson,
	Ilya Maximets

Patches primary targeted to fix OVS unit test failures with
DPDK 18.11 due to following error:
    'IOPL call failed - cannot use virtio PMD'.

We can avoid test failures in OVS by filtering the output like this:
  https://patchwork.ozlabs.org/project/openvswitch/list/?series=77706
but it still looks very inconvenient for me to have this
message in the output of every command for the DPDK linked app.

Version 3:
    * Splitted in two patches.      [David Marchand]
    * Fixed typo in commit message. [David Marchand]

Version 2:
    * Fixed possible fd leak on BSD.

Ilya Maximets (2):
  eal/bsd: fix possible IOPL fd leak
  net/virtio: avoid annoying IOPL call related errors

 drivers/net/virtio/virtio_ethdev.c | 11 ++++++-----
 lib/librte_eal/bsdapp/eal/eal.c    |  6 ++++--
 2 files changed, 10 insertions(+), 7 deletions(-)

-- 
2.17.1

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

* [dpdk-dev] [PATCH v3 1/2] eal/bsd: fix possible IOPL fd leak
       [not found]           ` <CGME20181123153947eucas1p169a2b7cec02b2edd7258aef11b1c1e2e@eucas1p1.samsung.com>
@ 2018-11-23 15:39             ` Ilya Maximets
  2018-11-23 17:32               ` Maxime Coquelin
  0 siblings, 1 reply; 17+ messages in thread
From: Ilya Maximets @ 2018-11-23 15:39 UTC (permalink / raw)
  To: dev, David Marchand
  Cc: Maxime Coquelin, Tiwei Bie, Zhihong Wang, Thomas Monjalon,
	Ferruh Yigit, Ian Stokes, Kevin Traynor, Bruce Richardson,
	Ilya Maximets, stable

If rte_eal_iopl_init() will be called more than once we'll leak
the file descriptor.

Fixes: b46fe31862ec ("eal/bsd: fix virtio on FreeBSD")
Cc: stable@dpdk.org

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 lib/librte_eal/bsdapp/eal/eal.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
index 508cbc46f..b8152a75c 100644
--- a/lib/librte_eal/bsdapp/eal/eal.c
+++ b/lib/librte_eal/bsdapp/eal/eal.c
@@ -556,9 +556,11 @@ int rte_eal_has_hugepages(void)
 int
 rte_eal_iopl_init(void)
 {
-	static int fd;
+	static int fd = -1;
+
+	if (fd < 0)
+		fd = open("/dev/io", O_RDWR);
 
-	fd = open("/dev/io", O_RDWR);
 	if (fd < 0)
 		return -1;
 	/* keep fd open for iopl */
-- 
2.17.1

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

* [dpdk-dev] [PATCH v3 2/2] net/virtio: avoid annoying IOPL call related errors
       [not found]           ` <CGME20181123153951eucas1p1218331b7e51db8eae3073fa6a14f3bcd@eucas1p1.samsung.com>
@ 2018-11-23 15:39             ` Ilya Maximets
  2018-11-23 17:34               ` Maxime Coquelin
  0 siblings, 1 reply; 17+ messages in thread
From: Ilya Maximets @ 2018-11-23 15:39 UTC (permalink / raw)
  To: dev, David Marchand
  Cc: Maxime Coquelin, Tiwei Bie, Zhihong Wang, Thomas Monjalon,
	Ferruh Yigit, Ian Stokes, Kevin Traynor, Bruce Richardson,
	Ilya Maximets

In case of running with not enough capabilities, i.e. running as
non-root user any application linked with DPDK prints the message
about IOPL call failure even if it was just called like
'./testpmd --help'. For example, this breaks most of the OVS unit
tests if it built with DPDK support.

Let's register the virtio driver unconditionally and print error
message while probing the device. Silent iopl() call left in the
constructor to have privileges as early as possible as it was before.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
Reviewed-by: David Marchand <david.marchand@redhat.com>
---
 drivers/net/virtio/virtio_ethdev.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
index e1fe36a23..2ba66d291 100644
--- a/drivers/net/virtio/virtio_ethdev.c
+++ b/drivers/net/virtio/virtio_ethdev.c
@@ -1760,6 +1760,11 @@ vdpa_mode_selected(struct rte_devargs *devargs)
 static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
 	struct rte_pci_device *pci_dev)
 {
+	if (rte_eal_iopl_init() != 0) {
+		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
+		return 1;
+	}
+
 	/* virtio pmd skips probe if device needs to work in vdpa mode */
 	if (vdpa_mode_selected(pci_dev->device.devargs))
 		return 1;
@@ -1785,11 +1790,7 @@ static struct rte_pci_driver rte_virtio_pmd = {
 
 RTE_INIT(rte_virtio_pmd_init)
 {
-	if (rte_eal_iopl_init() != 0) {
-		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
-		return;
-	}
-
+	rte_eal_iopl_init();
 	rte_pci_register(&rte_virtio_pmd);
 }
 
-- 
2.17.1

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

* Re: [dpdk-dev] [PATCH v3 0/2] IOPL related fixes
  2018-11-23 15:39         ` [dpdk-dev] [PATCH v3 0/2] IOPL related fixes Ilya Maximets
       [not found]           ` <CGME20181123153947eucas1p169a2b7cec02b2edd7258aef11b1c1e2e@eucas1p1.samsung.com>
       [not found]           ` <CGME20181123153951eucas1p1218331b7e51db8eae3073fa6a14f3bcd@eucas1p1.samsung.com>
@ 2018-11-23 15:41           ` Maxime Coquelin
  2018-11-23 15:53             ` Ilya Maximets
  2 siblings, 1 reply; 17+ messages in thread
From: Maxime Coquelin @ 2018-11-23 15:41 UTC (permalink / raw)
  To: Ilya Maximets, dev, David Marchand
  Cc: Tiwei Bie, Zhihong Wang, Thomas Monjalon, Ferruh Yigit,
	Ian Stokes, Kevin Traynor, Bruce Richardson

Hi,

On 11/23/18 4:39 PM, Ilya Maximets wrote:
> Patches primary targeted to fix OVS unit test failures with
> DPDK 18.11 due to following error:
>      'IOPL call failed - cannot use virtio PMD'.

You mention v18.11, do you mean this is a regression?

> 
> We can avoid test failures in OVS by filtering the output like this:
>    https://patchwork.ozlabs.org/project/openvswitch/list/?series=77706
> but it still looks very inconvenient for me to have this
> message in the output of every command for the DPDK linked app.
> 
> Version 3:
>      * Splitted in two patches.      [David Marchand]
>      * Fixed typo in commit message. [David Marchand]
> 
> Version 2:
>      * Fixed possible fd leak on BSD.
> 
> Ilya Maximets (2):
>    eal/bsd: fix possible IOPL fd leak
>    net/virtio: avoid annoying IOPL call related errors
> 
>   drivers/net/virtio/virtio_ethdev.c | 11 ++++++-----
>   lib/librte_eal/bsdapp/eal/eal.c    |  6 ++++--
>   2 files changed, 10 insertions(+), 7 deletions(-)
> 

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

* Re: [dpdk-dev] [PATCH v3 0/2] IOPL related fixes
  2018-11-23 15:41           ` [dpdk-dev] [PATCH v3 0/2] IOPL related fixes Maxime Coquelin
@ 2018-11-23 15:53             ` Ilya Maximets
  2018-11-23 16:25               ` Maxime Coquelin
  0 siblings, 1 reply; 17+ messages in thread
From: Ilya Maximets @ 2018-11-23 15:53 UTC (permalink / raw)
  To: Maxime Coquelin, dev, David Marchand
  Cc: Tiwei Bie, Zhihong Wang, Thomas Monjalon, Ferruh Yigit,
	Ian Stokes, Kevin Traynor, Bruce Richardson

On 23.11.2018 18:41, Maxime Coquelin wrote:
> Hi,
> 
> On 11/23/18 4:39 PM, Ilya Maximets wrote:
>> Patches primary targeted to fix OVS unit test failures with
>> DPDK 18.11 due to following error:
>>      'IOPL call failed - cannot use virtio PMD'.
> 
> You mention v18.11, do you mean this is a regression?

Kind of. But not really a bug. It's just a message that shows up
every time you starting the app as a non-root user.

The message itself was introduced long time ago, but it wasn't
printed for unclear reasons. It's probably some change in logging
subsystem uncovered it.

I just checked and found that message appears starting from v18.02.
But OVS stuck with 17.11 LTS where this message exists, but not
printed for some reason. That's why this wasn't an issue previously.

P.S. OVS tests just checks the output for error messages.
     Tests themselves works fine. They are not actually using DPDK.

Best regards, Ilya Maximets.

> 
>>
>> We can avoid test failures in OVS by filtering the output like this:
>>    https://patchwork.ozlabs.org/project/openvswitch/list/?series=77706
>> but it still looks very inconvenient for me to have this
>> message in the output of every command for the DPDK linked app.
>>
>> Version 3:
>>      * Splitted in two patches.      [David Marchand]
>>      * Fixed typo in commit message. [David Marchand]
>>
>> Version 2:
>>      * Fixed possible fd leak on BSD.
>>
>> Ilya Maximets (2):
>>    eal/bsd: fix possible IOPL fd leak
>>    net/virtio: avoid annoying IOPL call related errors
>>
>>   drivers/net/virtio/virtio_ethdev.c | 11 ++++++-----
>>   lib/librte_eal/bsdapp/eal/eal.c    |  6 ++++--
>>   2 files changed, 10 insertions(+), 7 deletions(-)
>>
> 
> 

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

* Re: [dpdk-dev] [PATCH v3 0/2] IOPL related fixes
  2018-11-23 15:53             ` Ilya Maximets
@ 2018-11-23 16:25               ` Maxime Coquelin
  2018-11-23 20:07                 ` Kevin Traynor
  0 siblings, 1 reply; 17+ messages in thread
From: Maxime Coquelin @ 2018-11-23 16:25 UTC (permalink / raw)
  To: Ilya Maximets, dev, David Marchand
  Cc: Tiwei Bie, Zhihong Wang, Thomas Monjalon, Ferruh Yigit,
	Ian Stokes, Kevin Traynor, Bruce Richardson



On 11/23/18 4:53 PM, Ilya Maximets wrote:
> On 23.11.2018 18:41, Maxime Coquelin wrote:
>> Hi,
>>
>> On 11/23/18 4:39 PM, Ilya Maximets wrote:
>>> Patches primary targeted to fix OVS unit test failures with
>>> DPDK 18.11 due to following error:
>>>       'IOPL call failed - cannot use virtio PMD'.
>>
>> You mention v18.11, do you mean this is a regression?
> 
> Kind of. But not really a bug. It's just a message that shows up
> every time you starting the app as a non-root user.
> 
> The message itself was introduced long time ago, but it wasn't
> printed for unclear reasons. It's probably some change in logging
> subsystem uncovered it.
> 
> I just checked and found that message appears starting from v18.02.
> But OVS stuck with 17.11 LTS where this message exists, but not
> printed for some reason. That's why this wasn't an issue previously.

Thanks for the clarification.
In that case, we may want it in v18.11 LTS, but I think it can wait or
v18.11.1.

Once in the next-virtio tree, I'll send it to stable@dpdk.org.

Regards,
Maxime

> P.S. OVS tests just checks the output for error messages.
>       Tests themselves works fine. They are not actually using DPDK.
> 
> Best regards, Ilya Maximets.
> 
>>
>>>
>>> We can avoid test failures in OVS by filtering the output like this:
>>>     https://patchwork.ozlabs.org/project/openvswitch/list/?series=77706
>>> but it still looks very inconvenient for me to have this
>>> message in the output of every command for the DPDK linked app.
>>>
>>> Version 3:
>>>       * Splitted in two patches.      [David Marchand]
>>>       * Fixed typo in commit message. [David Marchand]
>>>
>>> Version 2:
>>>       * Fixed possible fd leak on BSD.
>>>
>>> Ilya Maximets (2):
>>>     eal/bsd: fix possible IOPL fd leak
>>>     net/virtio: avoid annoying IOPL call related errors
>>>
>>>    drivers/net/virtio/virtio_ethdev.c | 11 ++++++-----
>>>    lib/librte_eal/bsdapp/eal/eal.c    |  6 ++++--
>>>    2 files changed, 10 insertions(+), 7 deletions(-)
>>>
>>
>>

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

* Re: [dpdk-dev] [PATCH v2] net/virtio: avoid annoying IOPL call related errors
  2018-11-23 14:36     ` [dpdk-dev] [PATCH v2] " Ilya Maximets
  2018-11-23 15:02       ` David Marchand
       [not found]       ` <CGME20181123153936eucas1p13bfbe13fcda92de7760ac768fcab43c5@eucas1p1.samsung.com>
@ 2018-11-23 17:15       ` Timothy Redaelli
  2 siblings, 0 replies; 17+ messages in thread
From: Timothy Redaelli @ 2018-11-23 17:15 UTC (permalink / raw)
  To: Ilya Maximets
  Cc: dev, David Marchand, Maxime Coquelin, Tiwei Bie, Zhihong Wang,
	Thomas Monjalon, Ferruh Yigit, Ian Stokes, Kevin Traynor

On Fri, 23 Nov 2018 17:36:20 +0300
Ilya Maximets <i.maximets@samsung.com> wrote:

> In case of running with not enough capabilities, i.e. running as
> non-root user any application linked with DPDK prints the message
> about IOPL call failure even if it was just called like
> './testpmd --help'. For example, this beaks most of the OVS unit
> tests if it built with DPDK support.
> 
> Let's register the virtio driver unconditionally and print error
> message while probing the device. Silent iopl() call left in the
> constructor to have privileges as early as possible as it was before.
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
> 
> Version 2:
> 	* Fixed possible fd leak on BSD.
> 
> We can avoid test failures in OVS by filtering the output
> like this:
>   https://patchwork.ozlabs.org/project/openvswitch/list/?series=77706
> 
> But it still looks very inconvenient for me to have this
> message in the output of every command for the DPDK linked app.
> 
>  drivers/net/virtio/virtio_ethdev.c | 11 ++++++-----
>  lib/librte_eal/bsdapp/eal/eal.c    |  6 ++++--
>  2 files changed, 10 insertions(+), 7 deletions(-)

Without this commit, if you link OVS as shared library
(--enable-shared), you'll also have this annoying message every time
you open a new (bash) shell, as user, due to OVS bash-completion:

[tredaell@aldebaran ~]$ bash
rte_virtio_pmd_init(): IOPL call failed - cannot use virtio PMD
rte_virtio_pmd_init(): IOPL call failed - cannot use virtio PMD
[tredaell@aldebaran ~]$

Acked-By: Timothy Redaelli <tredaelli@redhat.com>

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

* Re: [dpdk-dev] [PATCH v3 1/2] eal/bsd: fix possible IOPL fd leak
  2018-11-23 15:39             ` [dpdk-dev] [PATCH v3 1/2] eal/bsd: fix possible IOPL fd leak Ilya Maximets
@ 2018-11-23 17:32               ` Maxime Coquelin
  0 siblings, 0 replies; 17+ messages in thread
From: Maxime Coquelin @ 2018-11-23 17:32 UTC (permalink / raw)
  To: Ilya Maximets, dev, David Marchand
  Cc: Tiwei Bie, Zhihong Wang, Thomas Monjalon, Ferruh Yigit,
	Ian Stokes, Kevin Traynor, Bruce Richardson, stable



On 11/23/18 4:39 PM, Ilya Maximets wrote:
> If rte_eal_iopl_init() will be called more than once we'll leak
> the file descriptor.
> 
> Fixes: b46fe31862ec ("eal/bsd: fix virtio on FreeBSD")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>   lib/librte_eal/bsdapp/eal/eal.c | 6 ++++--
>   1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_eal/bsdapp/eal/eal.c b/lib/librte_eal/bsdapp/eal/eal.c
> index 508cbc46f..b8152a75c 100644
> --- a/lib/librte_eal/bsdapp/eal/eal.c
> +++ b/lib/librte_eal/bsdapp/eal/eal.c
> @@ -556,9 +556,11 @@ int rte_eal_has_hugepages(void)
>   int
>   rte_eal_iopl_init(void)
>   {
> -	static int fd;
> +	static int fd = -1;
> +
> +	if (fd < 0)
> +		fd = open("/dev/io", O_RDWR);
>   
> -	fd = open("/dev/io", O_RDWR);
>   	if (fd < 0)
>   		return -1;
>   	/* keep fd open for iopl */
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

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

* Re: [dpdk-dev] [PATCH v3 2/2] net/virtio: avoid annoying IOPL call related errors
  2018-11-23 15:39             ` [dpdk-dev] [PATCH v3 2/2] net/virtio: avoid annoying IOPL call related errors Ilya Maximets
@ 2018-11-23 17:34               ` Maxime Coquelin
  0 siblings, 0 replies; 17+ messages in thread
From: Maxime Coquelin @ 2018-11-23 17:34 UTC (permalink / raw)
  To: Ilya Maximets, dev, David Marchand
  Cc: Tiwei Bie, Zhihong Wang, Thomas Monjalon, Ferruh Yigit,
	Ian Stokes, Kevin Traynor, Bruce Richardson



On 11/23/18 4:39 PM, Ilya Maximets wrote:
> In case of running with not enough capabilities, i.e. running as
> non-root user any application linked with DPDK prints the message
> about IOPL call failure even if it was just called like
> './testpmd --help'. For example, this breaks most of the OVS unit
> tests if it built with DPDK support.
> 
> Let's register the virtio driver unconditionally and print error
> message while probing the device. Silent iopl() call left in the
> constructor to have privileges as early as possible as it was before.
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> ---
>   drivers/net/virtio/virtio_ethdev.c | 11 ++++++-----
>   1 file changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/virtio/virtio_ethdev.c b/drivers/net/virtio/virtio_ethdev.c
> index e1fe36a23..2ba66d291 100644
> --- a/drivers/net/virtio/virtio_ethdev.c
> +++ b/drivers/net/virtio/virtio_ethdev.c
> @@ -1760,6 +1760,11 @@ vdpa_mode_selected(struct rte_devargs *devargs)
>   static int eth_virtio_pci_probe(struct rte_pci_driver *pci_drv __rte_unused,
>   	struct rte_pci_device *pci_dev)
>   {
> +	if (rte_eal_iopl_init() != 0) {
> +		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
> +		return 1;
> +	}
> +
>   	/* virtio pmd skips probe if device needs to work in vdpa mode */
>   	if (vdpa_mode_selected(pci_dev->device.devargs))
>   		return 1;
> @@ -1785,11 +1790,7 @@ static struct rte_pci_driver rte_virtio_pmd = {
>   
>   RTE_INIT(rte_virtio_pmd_init)
>   {
> -	if (rte_eal_iopl_init() != 0) {
> -		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
> -		return;
> -	}
> -
> +	rte_eal_iopl_init();
>   	rte_pci_register(&rte_virtio_pmd);
>   }
>   
> 

Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>

Thanks,
Maxime

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

* Re: [dpdk-dev] [PATCH v3 0/2] IOPL related fixes
  2018-11-23 16:25               ` Maxime Coquelin
@ 2018-11-23 20:07                 ` Kevin Traynor
  2018-11-23 22:29                   ` Thomas Monjalon
  0 siblings, 1 reply; 17+ messages in thread
From: Kevin Traynor @ 2018-11-23 20:07 UTC (permalink / raw)
  To: Maxime Coquelin, Ilya Maximets, dev, David Marchand
  Cc: Tiwei Bie, Zhihong Wang, Thomas Monjalon, Ferruh Yigit,
	Ian Stokes, Bruce Richardson

On 11/23/2018 04:25 PM, Maxime Coquelin wrote:
> 
> 
> On 11/23/18 4:53 PM, Ilya Maximets wrote:
>> On 23.11.2018 18:41, Maxime Coquelin wrote:
>>> Hi,
>>>
>>> On 11/23/18 4:39 PM, Ilya Maximets wrote:
>>>> Patches primary targeted to fix OVS unit test failures with
>>>> DPDK 18.11 due to following error:
>>>>       'IOPL call failed - cannot use virtio PMD'.
>>>
>>> You mention v18.11, do you mean this is a regression?
>>
>> Kind of. But not really a bug. It's just a message that shows up
>> every time you starting the app as a non-root user.
>>
>> The message itself was introduced long time ago, but it wasn't
>> printed for unclear reasons. It's probably some change in logging
>> subsystem uncovered it.
>>
>> I just checked and found that message appears starting from v18.02.
>> But OVS stuck with 17.11 LTS where this message exists, but not
>> printed for some reason. That's why this wasn't an issue previously.
> 
> Thanks for the clarification.
> In that case, we may want it in v18.11 LTS, but I think it can wait or
> v18.11.1.
> 

It would be better if it can be in 18.11. Upstream OVS will point at a
release tarball, and at present an 18.11.1 release is not due until
mid-January. If it's too late for 18.11, alternatively we could do an
earlier 18.11.1 release.

> Once in the next-virtio tree, I'll send it to stable@dpdk.org.
> 
> Regards,
> Maxime
> 
>> P.S. OVS tests just checks the output for error messages.
>>       Tests themselves works fine. They are not actually using DPDK.
>>
>> Best regards, Ilya Maximets.
>>
>>>
>>>>
>>>> We can avoid test failures in OVS by filtering the output like this:
>>>>     https://patchwork.ozlabs.org/project/openvswitch/list/?series=77706
>>>> but it still looks very inconvenient for me to have this
>>>> message in the output of every command for the DPDK linked app.
>>>>
>>>> Version 3:
>>>>       * Splitted in two patches.      [David Marchand]
>>>>       * Fixed typo in commit message. [David Marchand]
>>>>
>>>> Version 2:
>>>>       * Fixed possible fd leak on BSD.
>>>>
>>>> Ilya Maximets (2):
>>>>     eal/bsd: fix possible IOPL fd leak
>>>>     net/virtio: avoid annoying IOPL call related errors
>>>>
>>>>    drivers/net/virtio/virtio_ethdev.c | 11 ++++++-----
>>>>    lib/librte_eal/bsdapp/eal/eal.c    |  6 ++++--
>>>>    2 files changed, 10 insertions(+), 7 deletions(-)
>>>>
>>>
>>>

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

* Re: [dpdk-dev] [PATCH v3 0/2] IOPL related fixes
  2018-11-23 20:07                 ` Kevin Traynor
@ 2018-11-23 22:29                   ` Thomas Monjalon
  2018-11-25 10:42                     ` Thomas Monjalon
  0 siblings, 1 reply; 17+ messages in thread
From: Thomas Monjalon @ 2018-11-23 22:29 UTC (permalink / raw)
  To: Ilya Maximets
  Cc: Kevin Traynor, Maxime Coquelin, dev, David Marchand, Tiwei Bie,
	Zhihong Wang, Ferruh Yigit, Ian Stokes, Bruce Richardson

23/11/2018 21:07, Kevin Traynor:
> On 11/23/2018 04:25 PM, Maxime Coquelin wrote:
> > 
> > 
> > On 11/23/18 4:53 PM, Ilya Maximets wrote:
> >> On 23.11.2018 18:41, Maxime Coquelin wrote:
> >>> Hi,
> >>>
> >>> On 11/23/18 4:39 PM, Ilya Maximets wrote:
> >>>> Patches primary targeted to fix OVS unit test failures with
> >>>> DPDK 18.11 due to following error:
> >>>>       'IOPL call failed - cannot use virtio PMD'.
> >>>
> >>> You mention v18.11, do you mean this is a regression?
> >>
> >> Kind of. But not really a bug. It's just a message that shows up
> >> every time you starting the app as a non-root user.
> >>
> >> The message itself was introduced long time ago, but it wasn't
> >> printed for unclear reasons. It's probably some change in logging
> >> subsystem uncovered it.
> >>
> >> I just checked and found that message appears starting from v18.02.
> >> But OVS stuck with 17.11 LTS where this message exists, but not
> >> printed for some reason. That's why this wasn't an issue previously.
> > 
> > Thanks for the clarification.
> > In that case, we may want it in v18.11 LTS, but I think it can wait or
> > v18.11.1.
> 
> It would be better if it can be in 18.11. Upstream OVS will point at a
> release tarball, and at present an 18.11.1 release is not due until
> mid-January. If it's too late for 18.11, alternatively we could do an
> earlier 18.11.1 release.

I won't take any risk at this stage.
The memory of the headaches we had in the past with iopl is too much strong.
The real change of this patch is to register the PMD even without successful iopl.
I would like to be sure it won't cause any regression.

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

* Re: [dpdk-dev] [PATCH v3 0/2] IOPL related fixes
  2018-11-23 22:29                   ` Thomas Monjalon
@ 2018-11-25 10:42                     ` Thomas Monjalon
  2018-11-26  9:49                       ` Kevin Traynor
  2018-11-27 14:05                       ` David Marchand
  0 siblings, 2 replies; 17+ messages in thread
From: Thomas Monjalon @ 2018-11-25 10:42 UTC (permalink / raw)
  To: Ilya Maximets, Kevin Traynor
  Cc: dev, Maxime Coquelin, David Marchand, Tiwei Bie, Zhihong Wang,
	Ferruh Yigit, Ian Stokes, Bruce Richardson

23/11/2018 23:29, Thomas Monjalon:
> 23/11/2018 21:07, Kevin Traynor:
> > On 11/23/2018 04:25 PM, Maxime Coquelin wrote:
> > > On 11/23/18 4:53 PM, Ilya Maximets wrote:
> > >> On 23.11.2018 18:41, Maxime Coquelin wrote:
> > >>> On 11/23/18 4:39 PM, Ilya Maximets wrote:
> > >>>> Patches primary targeted to fix OVS unit test failures with
> > >>>> DPDK 18.11 due to following error:
> > >>>>       'IOPL call failed - cannot use virtio PMD'.
> > >>>
> > >>> You mention v18.11, do you mean this is a regression?
> > >>
> > >> Kind of. But not really a bug. It's just a message that shows up
> > >> every time you starting the app as a non-root user.
> > >>
> > >> The message itself was introduced long time ago, but it wasn't
> > >> printed for unclear reasons. It's probably some change in logging
> > >> subsystem uncovered it.
> > >>
> > >> I just checked and found that message appears starting from v18.02.
> > >> But OVS stuck with 17.11 LTS where this message exists, but not
> > >> printed for some reason. That's why this wasn't an issue previously.
> > > 
> > > Thanks for the clarification.
> > > In that case, we may want it in v18.11 LTS, but I think it can wait or
> > > v18.11.1.
> > 
> > It would be better if it can be in 18.11. Upstream OVS will point at a
> > release tarball, and at present an 18.11.1 release is not due until
> > mid-January. If it's too late for 18.11, alternatively we could do an
> > earlier 18.11.1 release.
> 
> I won't take any risk at this stage.
> The memory of the headaches we had in the past with iopl is too much strong.
> The real change of this patch is to register the PMD even without successful iopl.
> I would like to be sure it won't cause any regression.

After more thinking, I decide to take the risk.
Take it as a gift to OVS :-)

Series applied

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

* Re: [dpdk-dev] [PATCH v3 0/2] IOPL related fixes
  2018-11-25 10:42                     ` Thomas Monjalon
@ 2018-11-26  9:49                       ` Kevin Traynor
  2018-11-27 14:05                       ` David Marchand
  1 sibling, 0 replies; 17+ messages in thread
From: Kevin Traynor @ 2018-11-26  9:49 UTC (permalink / raw)
  To: Thomas Monjalon, Ilya Maximets
  Cc: dev, Maxime Coquelin, David Marchand, Tiwei Bie, Zhihong Wang,
	Ferruh Yigit, Ian Stokes, Bruce Richardson

On 11/25/2018 10:42 AM, Thomas Monjalon wrote:
> 23/11/2018 23:29, Thomas Monjalon:
>> 23/11/2018 21:07, Kevin Traynor:
>>> On 11/23/2018 04:25 PM, Maxime Coquelin wrote:
>>>> On 11/23/18 4:53 PM, Ilya Maximets wrote:
>>>>> On 23.11.2018 18:41, Maxime Coquelin wrote:
>>>>>> On 11/23/18 4:39 PM, Ilya Maximets wrote:
>>>>>>> Patches primary targeted to fix OVS unit test failures with
>>>>>>> DPDK 18.11 due to following error:
>>>>>>>       'IOPL call failed - cannot use virtio PMD'.
>>>>>>
>>>>>> You mention v18.11, do you mean this is a regression?
>>>>>
>>>>> Kind of. But not really a bug. It's just a message that shows up
>>>>> every time you starting the app as a non-root user.
>>>>>
>>>>> The message itself was introduced long time ago, but it wasn't
>>>>> printed for unclear reasons. It's probably some change in logging
>>>>> subsystem uncovered it.
>>>>>
>>>>> I just checked and found that message appears starting from v18.02.
>>>>> But OVS stuck with 17.11 LTS where this message exists, but not
>>>>> printed for some reason. That's why this wasn't an issue previously.
>>>>
>>>> Thanks for the clarification.
>>>> In that case, we may want it in v18.11 LTS, but I think it can wait or
>>>> v18.11.1.
>>>
>>> It would be better if it can be in 18.11. Upstream OVS will point at a
>>> release tarball, and at present an 18.11.1 release is not due until
>>> mid-January. If it's too late for 18.11, alternatively we could do an
>>> earlier 18.11.1 release.
>>
>> I won't take any risk at this stage.
>> The memory of the headaches we had in the past with iopl is too much strong.
>> The real change of this patch is to register the PMD even without successful iopl.
>> I would like to be sure it won't cause any regression.
> 
> After more thinking, I decide to take the risk.
> Take it as a gift to OVS :-)
> 

ok, thanks :-) If there's any subsequent changes needed, I will get them
on an 18.11 stable branch straight away.

> Series applied
> 
> 

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

* Re: [dpdk-dev] [PATCH v3 0/2] IOPL related fixes
  2018-11-25 10:42                     ` Thomas Monjalon
  2018-11-26  9:49                       ` Kevin Traynor
@ 2018-11-27 14:05                       ` David Marchand
  1 sibling, 0 replies; 17+ messages in thread
From: David Marchand @ 2018-11-27 14:05 UTC (permalink / raw)
  To: thomas, Kevin Traynor
  Cc: i.maximets, dev, maxime.coquelin, tiwei.bie, zhihong.wang,
	ferruh.yigit, ian.stokes, bruce.richardson

On Sun, Nov 25, 2018 at 11:42 AM Thomas Monjalon <thomas@monjalon.net>
wrote:

> 23/11/2018 23:29, Thomas Monjalon:
> > 23/11/2018 21:07, Kevin Traynor:
> > > On 11/23/2018 04:25 PM, Maxime Coquelin wrote:
> > > > On 11/23/18 4:53 PM, Ilya Maximets wrote:
> > > >> On 23.11.2018 18:41, Maxime Coquelin wrote:
> > > >>> On 11/23/18 4:39 PM, Ilya Maximets wrote:
> > > >>>> Patches primary targeted to fix OVS unit test failures with
> > > >>>> DPDK 18.11 due to following error:
> > > >>>>       'IOPL call failed - cannot use virtio PMD'.
> > > >>>
> > > >>> You mention v18.11, do you mean this is a regression?
> > > >>
> > > >> Kind of. But not really a bug. It's just a message that shows up
> > > >> every time you starting the app as a non-root user.
> > > >>
> > > >> The message itself was introduced long time ago, but it wasn't
> > > >> printed for unclear reasons. It's probably some change in logging
> > > >> subsystem uncovered it.
> > > >>
> > > >> I just checked and found that message appears starting from v18.02.
> > > >> But OVS stuck with 17.11 LTS where this message exists, but not
> > > >> printed for some reason. That's why this wasn't an issue previously.
> > > >
> > > > Thanks for the clarification.
> > > > In that case, we may want it in v18.11 LTS, but I think it can wait
> or
> > > > v18.11.1.
> > >
> > > It would be better if it can be in 18.11. Upstream OVS will point at a
> > > release tarball, and at present an 18.11.1 release is not due until
> > > mid-January. If it's too late for 18.11, alternatively we could do an
> > > earlier 18.11.1 release.
> >
> > I won't take any risk at this stage.
> > The memory of the headaches we had in the past with iopl is too much
> strong.
> > The real change of this patch is to register the PMD even without
> successful iopl.
> > I would like to be sure it won't cause any regression.
>
> After more thinking, I decide to take the risk.
> Take it as a gift to OVS :-)
>

Just for the record, this log is visible by default since 18.02 with commit:
0062818856c3 ("net/virtio: implement dynamic logging")

So probably worth backporting in 18.08 at least.

-- 
David Marchand

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

end of thread, other threads:[~2018-11-27 14:05 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20181123141748eucas1p132a221972a87ec82b50d4a6c83bd9646@eucas1p1.samsung.com>
2018-11-23 14:17 ` [dpdk-dev] [PATCH] net/virtio: avoid annoying IOPL call related errors Ilya Maximets
     [not found]   ` <CGME20181123143625eucas1p1def3421fa13b5aec7204549932c75bb7@eucas1p1.samsung.com>
2018-11-23 14:36     ` [dpdk-dev] [PATCH v2] " Ilya Maximets
2018-11-23 15:02       ` David Marchand
     [not found]       ` <CGME20181123153936eucas1p13bfbe13fcda92de7760ac768fcab43c5@eucas1p1.samsung.com>
2018-11-23 15:39         ` [dpdk-dev] [PATCH v3 0/2] IOPL related fixes Ilya Maximets
     [not found]           ` <CGME20181123153947eucas1p169a2b7cec02b2edd7258aef11b1c1e2e@eucas1p1.samsung.com>
2018-11-23 15:39             ` [dpdk-dev] [PATCH v3 1/2] eal/bsd: fix possible IOPL fd leak Ilya Maximets
2018-11-23 17:32               ` Maxime Coquelin
     [not found]           ` <CGME20181123153951eucas1p1218331b7e51db8eae3073fa6a14f3bcd@eucas1p1.samsung.com>
2018-11-23 15:39             ` [dpdk-dev] [PATCH v3 2/2] net/virtio: avoid annoying IOPL call related errors Ilya Maximets
2018-11-23 17:34               ` Maxime Coquelin
2018-11-23 15:41           ` [dpdk-dev] [PATCH v3 0/2] IOPL related fixes Maxime Coquelin
2018-11-23 15:53             ` Ilya Maximets
2018-11-23 16:25               ` Maxime Coquelin
2018-11-23 20:07                 ` Kevin Traynor
2018-11-23 22:29                   ` Thomas Monjalon
2018-11-25 10:42                     ` Thomas Monjalon
2018-11-26  9:49                       ` Kevin Traynor
2018-11-27 14:05                       ` David Marchand
2018-11-23 17:15       ` [dpdk-dev] [PATCH v2] net/virtio: avoid annoying IOPL call related errors Timothy Redaelli

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