[dpdk-dev,2/2] virtio: change io privilege level as early as possible

  Playing with virtio link status triggers a segfault because of an incorrect io
privilege level.

To reproduce the problem, virtio device must be bound to igb_uio to have lsc
enabled.

$ lspci |grep Ethernet
00:03.0 Ethernet controller: Red Hat, Inc Virtio network device
$ modprobe uio
$ insmod ./x86_64-native-linuxapp-gcc/kmod/igb_uio.ko
$ echo 0000:00:03.0 > /sys/bus/pci/devices/0000\:00\:03.0/driver/unbind
$ echo 1af4 1000 > /sys/bus/pci/drivers/igb_uio/new_id
$ ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 3 -w 0000:00:03.0 -- -i --txqflags=0xf01
[snip]
EAL: PCI device 0000:00:03.0 on NUMA socket -1
EAL:   probe driver: 1af4:1000 rte_virtio_pmd
Interactive-mode selected
Configuring Port 0 (socket 0)
Port 0: DE:AD:DE:01:02:03
Checking link statuses...
Port 0 Link Up - speed 10000 Mbps - full-duplex
Done
testpmd>

Then, from qemu monitor:
(qemu) set_link virtio-net-pci.0 off

testpmd> Segmentation fault

A call to rte_eal_iopl_init() in a specific constructor has been added so that,
on Linux, iopl() is called asap:
- for statically linked virtio pmd, the constructor will be called at binary
  start,
- for shared virtio pmd (loaded using -d eal option), it will be called at
  dlopen() in rte_eal_init().

On linux, iopl() effects are inherited by children threads, so calling it in a
constructor will ensure that any thread / process created after rte_eal_init()
inherits the io privilege level needed by the pmd.

We keep call to rte_eal_iopl_init() in the pmd init macro so that the pmd will
not register if the io privilege level is insufficient.

Besides, I moved rte_virtio_pmd_init() and related structure near its calling
site, so that all init code can be looked at in a glance.
The only change here is a new comment.

Signed-off-by: David Marchand <david.marchand@6wind.com>
---
 lib/librte_pmd_virtio/virtio_ethdev.c |   71 ++++++++++++++++++++-------------
 1 file changed, 43 insertions(+), 28 deletions(-)
  

On Mon, Mar 09, 2015 at 03:56:39PM +0100, David Marchand wrote:
> Playing with virtio link status triggers a segfault because of an incorrect io
> privilege level.
> 
> To reproduce the problem, virtio device must be bound to igb_uio to have lsc
> enabled.
> 
> $ lspci |grep Ethernet
> 00:03.0 Ethernet controller: Red Hat, Inc Virtio network device
> $ modprobe uio
> $ insmod ./x86_64-native-linuxapp-gcc/kmod/igb_uio.ko
> $ echo 0000:00:03.0 > /sys/bus/pci/devices/0000\:00\:03.0/driver/unbind
> $ echo 1af4 1000 > /sys/bus/pci/drivers/igb_uio/new_id
> $ ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 3 -w 0000:00:03.0 -- -i --txqflags=0xf01
> [snip]
> EAL: PCI device 0000:00:03.0 on NUMA socket -1
> EAL:   probe driver: 1af4:1000 rte_virtio_pmd
> Interactive-mode selected
> Configuring Port 0 (socket 0)
> Port 0: DE:AD:DE:01:02:03
> Checking link statuses...
> Port 0 Link Up - speed 10000 Mbps - full-duplex
> Done
> testpmd>
> 
> Then, from qemu monitor:
> (qemu) set_link virtio-net-pci.0 off
> 
> testpmd> Segmentation fault
> 
> A call to rte_eal_iopl_init() in a specific constructor has been added so that,
> on Linux, iopl() is called asap:
> - for statically linked virtio pmd, the constructor will be called at binary
>   start,
> - for shared virtio pmd (loaded using -d eal option), it will be called at
>   dlopen() in rte_eal_init().
> 
> On linux, iopl() effects are inherited by children threads, so calling it in a
> constructor will ensure that any thread / process created after rte_eal_init()
> inherits the io privilege level needed by the pmd.
> 
> We keep call to rte_eal_iopl_init() in the pmd init macro so that the pmd will
> not register if the io privilege level is insufficient.
> 
> Besides, I moved rte_virtio_pmd_init() and related structure near its calling
> site, so that all init code can be looked at in a glance.
> The only change here is a new comment.
> 
> Signed-off-by: David Marchand <david.marchand@6wind.com>
> ---
>  lib/librte_pmd_virtio/virtio_ethdev.c |   71 ++++++++++++++++++++-------------
>  1 file changed, 43 insertions(+), 28 deletions(-)
> 
> diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
> index d239083..29332fa 100644
> --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> @@ -1228,34 +1228,6 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
>  	return 0;
>  }
>  
> -static struct eth_driver rte_virtio_pmd = {
> -	{
> -		.name = "rte_virtio_pmd",
> -		.id_table = pci_id_virtio_map,
> -	},
> -	.eth_dev_init = eth_virtio_dev_init,
> -	.dev_private_size = sizeof(struct virtio_hw),
> -};
> -
> -/*
> - * Driver initialization routine.
> - * Invoked once at EAL init time.
> - * Register itself as the [Poll Mode] Driver of PCI virtio devices.
> - * Returns 0 on success.
> - */
> -static int
> -rte_virtio_pmd_init(const char *name __rte_unused,
> -		    const char *param __rte_unused)
> -{
> -	if (rte_eal_iopl_init() != 0) {
> -		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
> -		return -1;
> -	}
> -
> -	rte_eth_driver_register(&rte_virtio_pmd);
> -	return 0;
> -}
> -
>  /*
>   * Only 1 queue is supported, no queue release related operation
>   */
> @@ -1484,6 +1456,49 @@ __rte_unused uint8_t is_rx)
>  	return 0;
>  }
>  
> +/*
> + * Early init function, this is needed so that iopl() on linux is done before
> + * rte_eal_init (since it creates other threads that must inherit from the one
> + * that calls rte_eal_init).
> + */
> +static void __attribute__((constructor, used))
> +rte_virtio_early_init(void)
> +{
> +	/* return value is checked at pmd init time, no need to check here, see
> +	 * below. */
> +	rte_eal_iopl_init();
> +}
> +

I don't see how this works for all cases.  The constructor is called once when
the library is first loaded.  What if you have multiple independent (i.e. not
forked children) processes that are using the dpdk in parallel?  Only the
process that triggered the library load will have io permissions set
appropriately.  I think what you need is to have every application that expects
to call through the transmit path or poll the receive path call iopl, which I
think speaks to having this requirement documented, so each application can call
iopl prior to calling fork/daemonize/etc.

Neil
  

On Tue, 10 Mar 2015 09:14:28 -0400
Neil Horman <nhorman@tuxdriver.com> wrote:

> On Mon, Mar 09, 2015 at 03:56:39PM +0100, David Marchand wrote:
> > Playing with virtio link status triggers a segfault because of an incorrect io
> > privilege level.
> > 
> > To reproduce the problem, virtio device must be bound to igb_uio to have lsc
> > enabled.
> > 
> > $ lspci |grep Ethernet
> > 00:03.0 Ethernet controller: Red Hat, Inc Virtio network device
> > $ modprobe uio
> > $ insmod ./x86_64-native-linuxapp-gcc/kmod/igb_uio.ko
> > $ echo 0000:00:03.0 > /sys/bus/pci/devices/0000\:00\:03.0/driver/unbind
> > $ echo 1af4 1000 > /sys/bus/pci/drivers/igb_uio/new_id
> > $ ./x86_64-native-linuxapp-gcc/app/testpmd -c 0x6 -n 3 -w 0000:00:03.0 -- -i --txqflags=0xf01
> > [snip]
> > EAL: PCI device 0000:00:03.0 on NUMA socket -1
> > EAL:   probe driver: 1af4:1000 rte_virtio_pmd
> > Interactive-mode selected
> > Configuring Port 0 (socket 0)
> > Port 0: DE:AD:DE:01:02:03
> > Checking link statuses...
> > Port 0 Link Up - speed 10000 Mbps - full-duplex
> > Done
> > testpmd>
> > 
> > Then, from qemu monitor:
> > (qemu) set_link virtio-net-pci.0 off
> > 
> > testpmd> Segmentation fault
> > 
> > A call to rte_eal_iopl_init() in a specific constructor has been added so that,
> > on Linux, iopl() is called asap:
> > - for statically linked virtio pmd, the constructor will be called at binary
> >   start,
> > - for shared virtio pmd (loaded using -d eal option), it will be called at
> >   dlopen() in rte_eal_init().
> > 
> > On linux, iopl() effects are inherited by children threads, so calling it in a
> > constructor will ensure that any thread / process created after rte_eal_init()
> > inherits the io privilege level needed by the pmd.
> > 
> > We keep call to rte_eal_iopl_init() in the pmd init macro so that the pmd will
> > not register if the io privilege level is insufficient.
> > 
> > Besides, I moved rte_virtio_pmd_init() and related structure near its calling
> > site, so that all init code can be looked at in a glance.
> > The only change here is a new comment.
> > 
> > Signed-off-by: David Marchand <david.marchand@6wind.com>
> > ---
> >  lib/librte_pmd_virtio/virtio_ethdev.c |   71 ++++++++++++++++++++-------------
> >  1 file changed, 43 insertions(+), 28 deletions(-)
> > 
> > diff --git a/lib/librte_pmd_virtio/virtio_ethdev.c b/lib/librte_pmd_virtio/virtio_ethdev.c
> > index d239083..29332fa 100644
> > --- a/lib/librte_pmd_virtio/virtio_ethdev.c
> > +++ b/lib/librte_pmd_virtio/virtio_ethdev.c
> > @@ -1228,34 +1228,6 @@ eth_virtio_dev_init(__rte_unused struct eth_driver *eth_drv,
> >  	return 0;
> >  }
> >  
> > -static struct eth_driver rte_virtio_pmd = {
> > -	{
> > -		.name = "rte_virtio_pmd",
> > -		.id_table = pci_id_virtio_map,
> > -	},
> > -	.eth_dev_init = eth_virtio_dev_init,
> > -	.dev_private_size = sizeof(struct virtio_hw),
> > -};
> > -
> > -/*
> > - * Driver initialization routine.
> > - * Invoked once at EAL init time.
> > - * Register itself as the [Poll Mode] Driver of PCI virtio devices.
> > - * Returns 0 on success.
> > - */
> > -static int
> > -rte_virtio_pmd_init(const char *name __rte_unused,
> > -		    const char *param __rte_unused)
> > -{
> > -	if (rte_eal_iopl_init() != 0) {
> > -		PMD_INIT_LOG(ERR, "IOPL call failed - cannot use virtio PMD");
> > -		return -1;
> > -	}
> > -
> > -	rte_eth_driver_register(&rte_virtio_pmd);
> > -	return 0;
> > -}
> > -
> >  /*
> >   * Only 1 queue is supported, no queue release related operation
> >   */
> > @@ -1484,6 +1456,49 @@ __rte_unused uint8_t is_rx)
> >  	return 0;
> >  }
> >  
> > +/*
> > + * Early init function, this is needed so that iopl() on linux is done before
> > + * rte_eal_init (since it creates other threads that must inherit from the one
> > + * that calls rte_eal_init).
> > + */
> > +static void __attribute__((constructor, used))
> > +rte_virtio_early_init(void)
> > +{
> > +	/* return value is checked at pmd init time, no need to check here, see
> > +	 * below. */
> > +	rte_eal_iopl_init();
> > +}
> > +
> 
> I don't see how this works for all cases.  The constructor is called once when
> the library is first loaded.  What if you have multiple independent (i.e. not
> forked children) processes that are using the dpdk in parallel?  Only the
> process that triggered the library load will have io permissions set
> appropriately.  I think what you need is to have every application that expects
> to call through the transmit path or poll the receive path call iopl, which I
> think speaks to having this requirement documented, so each application can call
> iopl prior to calling fork/daemonize/etc.
> 
> Neil

I am still seeing this problem with DPDK 2.0 and 2.1.
It seems to me that doing the iopl init in eal_init is the only safe way.
Other workaround is to have application calling iopl_init before eal_init
but that kind of violates the current method of all things being initialized
by eal_init
  

On Tue, Sep 29, 2015 at 9:25 PM, Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Tue, 10 Mar 2015 09:14:28 -0400
> Neil Horman <nhorman@tuxdriver.com> wrote:
>
> >
> > I don't see how this works for all cases.  The constructor is called
> once when
> > the library is first loaded.  What if you have multiple independent
> (i.e. not
> > forked children) processes that are using the dpdk in parallel?  Only the
> > process that triggered the library load will have io permissions set
> > appropriately.  I think what you need is to have every application that
> expects
> > to call through the transmit path or poll the receive path call iopl,
> which I
> > think speaks to having this requirement documented, so each application
> can call
> > iopl prior to calling fork/daemonize/etc.
> >
>
> I am still seeing this problem with DPDK 2.0 and 2.1.
> It seems to me that doing the iopl init in eal_init is the only safe way.
> Other workaround is to have application calling iopl_init before eal_init
> but that kind of violates the current method of all things being
> initialized
> by eal_init
>

Putting it in the virtio pmd constructor is my preferred solution and we
don't need to pollute the eal for virtio (specific to x86, btw).
  

On Wed, Sep 30, 2015 at 10:28:53AM +0200, David Marchand wrote:
> On Tue, Sep 29, 2015 at 9:25 PM, Stephen Hemminger <
> stephen@networkplumber.org> wrote:
> 
> > On Tue, 10 Mar 2015 09:14:28 -0400
> > Neil Horman <nhorman@tuxdriver.com> wrote:
> >
> > >
> > > I don't see how this works for all cases.  The constructor is called
> > once when
> > > the library is first loaded.  What if you have multiple independent
> > (i.e. not
> > > forked children) processes that are using the dpdk in parallel?  Only the
> > > process that triggered the library load will have io permissions set
> > > appropriately.  I think what you need is to have every application that
> > expects
> > > to call through the transmit path or poll the receive path call iopl,
> > which I
> > > think speaks to having this requirement documented, so each application
> > can call
> > > iopl prior to calling fork/daemonize/etc.
> > >
> >
> > I am still seeing this problem with DPDK 2.0 and 2.1.
> > It seems to me that doing the iopl init in eal_init is the only safe way.
> > Other workaround is to have application calling iopl_init before eal_init
> > but that kind of violates the current method of all things being
> > initialized
> > by eal_init
> >
> 
> Putting it in the virtio pmd constructor is my preferred solution and we
> don't need to pollute the eal for virtio (specific to x86, btw).
> 

Preferred solution or not, you can't just call iopl from the constructor,
because not all process will get appropriate permissions.  It needs to be called
by every process.  What Stephen is saying is that your solution has use cases
for which it doesn't work, and that needs to be solved.
Neil
 
> 
> -- 
> David Marchand
  

2015-09-30 10:52, Neil Horman:
> On Wed, Sep 30, 2015 at 10:28:53AM +0200, David Marchand wrote:
> > On Tue, Sep 29, 2015 at 9:25 PM, Stephen Hemminger <
> > stephen@networkplumber.org> wrote:
> > 
> > > On Tue, 10 Mar 2015 09:14:28 -0400
> > > Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > I don't see how this works for all cases.  The constructor is called
> > > once when
> > > > the library is first loaded.  What if you have multiple independent
> > > (i.e. not
> > > > forked children) processes that are using the dpdk in parallel?  Only the
> > > > process that triggered the library load will have io permissions set
> > > > appropriately.  I think what you need is to have every application that
> > > expects
> > > > to call through the transmit path or poll the receive path call iopl,
> > > which I
> > > > think speaks to having this requirement documented, so each application
> > > can call
> > > > iopl prior to calling fork/daemonize/etc.
> > > >
> > >
> > > I am still seeing this problem with DPDK 2.0 and 2.1.
> > > It seems to me that doing the iopl init in eal_init is the only safe way.
> > > Other workaround is to have application calling iopl_init before eal_init
> > > but that kind of violates the current method of all things being
> > > initialized by eal_init
> > 
> > Putting it in the virtio pmd constructor is my preferred solution and we
> > don't need to pollute the eal for virtio (specific to x86, btw).
> 
> Preferred solution or not, you can't just call iopl from the constructor,
> because not all process will get appropriate permissions.  It needs to be called
> by every process.  What Stephen is saying is that your solution has use cases
> for which it doesn't work, and that needs to be solved.

I think it may be solved by calling iopl in the constructor.
We just need an extra call in rte_virtio_pmd_init() to detect iopl failures.
We can also simply move rte_eal_intr_init() after rte_eal_dev_init().
Please read my previous post on this topic:
	http://thread.gmane.org/gmane.comp.networking.dpdk.devel/14761/focus=22341

About the multiprocess case, I don't see the problem as the RX/TX and interrupt
threads are forked in the rte_eal_init() context which should call iopl even in
secondary processes.
  

On Wed, 30 Sep 2015 17:37:05 +0200
Thomas Monjalon <thomas.monjalon@6wind.com> wrote:

> 2015-09-30 10:52, Neil Horman:
> > On Wed, Sep 30, 2015 at 10:28:53AM +0200, David Marchand wrote:
> > > On Tue, Sep 29, 2015 at 9:25 PM, Stephen Hemminger <
> > > stephen@networkplumber.org> wrote:
> > > 
> > > > On Tue, 10 Mar 2015 09:14:28 -0400
> > > > Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > > I don't see how this works for all cases.  The constructor is called
> > > > once when
> > > > > the library is first loaded.  What if you have multiple independent
> > > > (i.e. not
> > > > > forked children) processes that are using the dpdk in parallel?  Only the
> > > > > process that triggered the library load will have io permissions set
> > > > > appropriately.  I think what you need is to have every application that
> > > > expects
> > > > > to call through the transmit path or poll the receive path call iopl,
> > > > which I
> > > > > think speaks to having this requirement documented, so each application
> > > > can call
> > > > > iopl prior to calling fork/daemonize/etc.
> > > > >
> > > >
> > > > I am still seeing this problem with DPDK 2.0 and 2.1.
> > > > It seems to me that doing the iopl init in eal_init is the only safe way.
> > > > Other workaround is to have application calling iopl_init before eal_init
> > > > but that kind of violates the current method of all things being
> > > > initialized by eal_init
> > > 
> > > Putting it in the virtio pmd constructor is my preferred solution and we
> > > don't need to pollute the eal for virtio (specific to x86, btw).
> > 
> > Preferred solution or not, you can't just call iopl from the constructor,
> > because not all process will get appropriate permissions.  It needs to be called
> > by every process.  What Stephen is saying is that your solution has use cases
> > for which it doesn't work, and that needs to be solved.
> 
> I think it may be solved by calling iopl in the constructor.
> We just need an extra call in rte_virtio_pmd_init() to detect iopl failures.
> We can also simply move rte_eal_intr_init() after rte_eal_dev_init().
> Please read my previous post on this topic:
> 	http://thread.gmane.org/gmane.comp.networking.dpdk.devel/14761/focus=22341
> 
> About the multiprocess case, I don't see the problem as the RX/TX and interrupt
> threads are forked in the rte_eal_init() context which should call iopl even in
> secondary processes.

Another alternative is to use ioperm to give access to certain regions.
But ioperm does not inherit to other threads, therefore it required some code on
startup to execute the syscall on each EAL thread.
  

On Wed, Sep 30, 2015 at 05:37:05PM +0200, Thomas Monjalon wrote:
> 2015-09-30 10:52, Neil Horman:
> > On Wed, Sep 30, 2015 at 10:28:53AM +0200, David Marchand wrote:
> > > On Tue, Sep 29, 2015 at 9:25 PM, Stephen Hemminger <
> > > stephen@networkplumber.org> wrote:
> > > 
> > > > On Tue, 10 Mar 2015 09:14:28 -0400
> > > > Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > > I don't see how this works for all cases.  The constructor is called
> > > > once when
> > > > > the library is first loaded.  What if you have multiple independent
> > > > (i.e. not
> > > > > forked children) processes that are using the dpdk in parallel?  Only the
> > > > > process that triggered the library load will have io permissions set
> > > > > appropriately.  I think what you need is to have every application that
> > > > expects
> > > > > to call through the transmit path or poll the receive path call iopl,
> > > > which I
> > > > > think speaks to having this requirement documented, so each application
> > > > can call
> > > > > iopl prior to calling fork/daemonize/etc.
> > > > >
> > > >
> > > > I am still seeing this problem with DPDK 2.0 and 2.1.
> > > > It seems to me that doing the iopl init in eal_init is the only safe way.
> > > > Other workaround is to have application calling iopl_init before eal_init
> > > > but that kind of violates the current method of all things being
> > > > initialized by eal_init
> > > 
> > > Putting it in the virtio pmd constructor is my preferred solution and we
> > > don't need to pollute the eal for virtio (specific to x86, btw).
> > 
> > Preferred solution or not, you can't just call iopl from the constructor,
> > because not all process will get appropriate permissions.  It needs to be called
> > by every process.  What Stephen is saying is that your solution has use cases
> > for which it doesn't work, and that needs to be solved.
> 
> I think it may be solved by calling iopl in the constructor.
> We just need an extra call in rte_virtio_pmd_init() to detect iopl failures.
> We can also simply move rte_eal_intr_init() after rte_eal_dev_init().
> Please read my previous post on this topic:
> 	http://thread.gmane.org/gmane.comp.networking.dpdk.devel/14761/focus=22341
> 
> About the multiprocess case, I don't see the problem as the RX/TX and interrupt
> threads are forked in the rte_eal_init() context which should call iopl even in
> secondary processes.
> 

I'm not talking about secondary processes here (i.e. processes forked from a
parent that was the process which initialized the dpdk).  I'm referring to two
completely independent processes, both of which link to and use the dpdk.

Though I think we're saying the same thing.  When you say 'constructor' above,
you don't mean 'constructor' in the strict sense, but rather the pmd init
routine (the one called from rte_eal_vdev_init and rte_eal_dev_init).  If this
is the case, then yes, that works fine, since each process linking to the DPDK
will enter those routines and call iopl.  In fact, if thats the case, then no
call is needed in the constructor at all.

Neil
  

I think David's patch should be in 2.2 (and 2.1.X if that starts out).
It solves the problem for both link state and secondary processes.

It may need to be updated for the current drivers/net/virtio layout.
  

On Thu, 1 Oct 2015 07:25:45 -0400
Neil Horman <nhorman@tuxdriver.com> wrote:

> On Wed, Sep 30, 2015 at 05:37:05PM +0200, Thomas Monjalon wrote:
> > 2015-09-30 10:52, Neil Horman:
> > > On Wed, Sep 30, 2015 at 10:28:53AM +0200, David Marchand wrote:
> > > > On Tue, Sep 29, 2015 at 9:25 PM, Stephen Hemminger <
> > > > stephen@networkplumber.org> wrote:
> > > > 
> > > > > On Tue, 10 Mar 2015 09:14:28 -0400
> > > > > Neil Horman <nhorman@tuxdriver.com> wrote:
> > > > > > I don't see how this works for all cases.  The constructor is called
> > > > > once when
> > > > > > the library is first loaded.  What if you have multiple independent
> > > > > (i.e. not
> > > > > > forked children) processes that are using the dpdk in parallel?  Only the
> > > > > > process that triggered the library load will have io permissions set
> > > > > > appropriately.  I think what you need is to have every application that
> > > > > expects
> > > > > > to call through the transmit path or poll the receive path call iopl,
> > > > > which I
> > > > > > think speaks to having this requirement documented, so each application
> > > > > can call
> > > > > > iopl prior to calling fork/daemonize/etc.
> > > > > >
> > > > >
> > > > > I am still seeing this problem with DPDK 2.0 and 2.1.
> > > > > It seems to me that doing the iopl init in eal_init is the only safe way.
> > > > > Other workaround is to have application calling iopl_init before eal_init
> > > > > but that kind of violates the current method of all things being
> > > > > initialized by eal_init
> > > > 
> > > > Putting it in the virtio pmd constructor is my preferred solution and we
> > > > don't need to pollute the eal for virtio (specific to x86, btw).
> > > 
> > > Preferred solution or not, you can't just call iopl from the constructor,
> > > because not all process will get appropriate permissions.  It needs to be called
> > > by every process.  What Stephen is saying is that your solution has use cases
> > > for which it doesn't work, and that needs to be solved.
> > 
> > I think it may be solved by calling iopl in the constructor.
> > We just need an extra call in rte_virtio_pmd_init() to detect iopl failures.
> > We can also simply move rte_eal_intr_init() after rte_eal_dev_init().
> > Please read my previous post on this topic:
> > 	http://thread.gmane.org/gmane.comp.networking.dpdk.devel/14761/focus=22341
> > 
> > About the multiprocess case, I don't see the problem as the RX/TX and interrupt
> > threads are forked in the rte_eal_init() context which should call iopl even in
> > secondary processes.
> > 
> 
> I'm not talking about secondary processes here (i.e. processes forked from a
> parent that was the process which initialized the dpdk).  I'm referring to two
> completely independent processes, both of which link to and use the dpdk.
> 
> Though I think we're saying the same thing.  When you say 'constructor' above,
> you don't mean 'constructor' in the strict sense, but rather the pmd init
> routine (the one called from rte_eal_vdev_init and rte_eal_dev_init).  If this
> is the case, then yes, that works fine, since each process linking to the DPDK
> will enter those routines and call iopl.  In fact, if thats the case, then no
> call is needed in the constructor at all.

I think this patch should be rebased and resubmitted for 2.2.

It fixes a real problem (virtio link state). The driver changed directory
and the the patch could be redone to minimize changes.
  

On Wed, Oct 14, 2015 at 2:07 AM, Stephen Hemminger <
stephen@networkplumber.org> wrote:

> On Thu, 1 Oct 2015 07:25:45 -0400
> Neil Horman <nhorman@tuxdriver.com> wrote:
>
> > On Wed, Sep 30, 2015 at 05:37:05PM +0200, Thomas Monjalon wrote:
> > > 2015-09-30 10:52, Neil Horman:
>
> > > I think it may be solved by calling iopl in the constructor.
> > > We just need an extra call in rte_virtio_pmd_init() to detect iopl
> failures.
> > > We can also simply move rte_eal_intr_init() after rte_eal_dev_init().
> > > Please read my previous post on this topic:
> > >
> http://thread.gmane.org/gmane.comp.networking.dpdk.devel/14761/focus=22341
> > >
> > > About the multiprocess case, I don't see the problem as the RX/TX and
> interrupt
> > > threads are forked in the rte_eal_init() context which should call
> iopl even in
> > > secondary processes.
> > >
> >
> > I'm not talking about secondary processes here (i.e. processes forked
> from a
> > parent that was the process which initialized the dpdk).  I'm referring
> to two
> > completely independent processes, both of which link to and use the dpdk.
> >
> > Though I think we're saying the same thing.  When you say 'constructor'
> above,
> > you don't mean 'constructor' in the strict sense, but rather the pmd init
> > routine (the one called from rte_eal_vdev_init and rte_eal_dev_init).
> If this
> > is the case, then yes, that works fine, since each process linking to
> the DPDK
> > will enter those routines and call iopl.  In fact, if thats the case,
> then no
> > call is needed in the constructor at all.
>
> I think this patch should be rebased and resubmitted for 2.2.
>
> It fixes a real problem (virtio link state). The driver changed directory
> and the the patch could be redone to minimize changes.
>

I am testing Thomas proposal (just moving the intr_init() call), else I
will rebase this patch.
  

On Wed, Oct 14, 2015 at 10:00 AM, David Marchand <david.marchand@6wind.com>
wrote:

>
> I am testing Thomas proposal (just moving the intr_init() call), else I
> will rebase this patch.
>

Thomas approach works and is minimal.
I ran some autotest, did not see anything wrong (let aside the tests that
already fail on current head).
Looking at eal and the drivers, this looks fine as well.

So my patches can be dropped.
Let's go with this new patch.