From: Stephen Hemminger <stephen@networkplumber.org>
To: Neil Horman <nhorman@tuxdriver.com>
Cc: dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH 2/2] virtio: change io privilege level as early as possible
Date: Tue, 29 Sep 2015 12:25:42 -0700 [thread overview]
Message-ID: <20150929122542.419781cc@urahara> (raw)
In-Reply-To: <20150310131428.GB7873@hmsreliant.think-freely.org>
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
next prev parent reply other threads:[~2015-09-29 19:25 UTC|newest]
Thread overview: 43+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-06 0:45 [dpdk-dev] [PATCH 0/2] virtio: bugfixes Stephen Hemminger
2015-03-06 0:45 ` [dpdk-dev] [PATCH 1/2] virtio: initialize iopl when device is initialized Stephen Hemminger
2015-03-06 3:41 ` Ouyang, Changchun
2015-03-06 16:20 ` Stephen Hemminger
2015-03-06 16:33 ` David Marchand
2015-03-06 16:55 ` Stephen Hemminger
2015-03-06 22:04 ` David Marchand
2015-03-06 23:43 ` Stephen Hemminger
2015-03-07 6:53 ` David Marchand
2015-03-09 11:05 ` David Marchand
2015-03-09 14:56 ` [dpdk-dev] [PATCH 0/2] fix virtio interrupt handling David Marchand
2015-03-09 14:56 ` [dpdk-dev] [PATCH 1/2] eal/linux: move plugin load to very start of eal init David Marchand
2015-03-09 15:21 ` Neil Horman
[not found] ` <CALwxeUs4hPbYDPBUfz9u2AoiCoj_wwTsAyj=_1xxzuT6LLW6nw@mail.gmail.com>
2015-03-10 10:55 ` Neil Horman
2015-10-14 0:05 ` Stephen Hemminger
2015-10-14 9:55 ` David Marchand
2015-03-09 14:56 ` [dpdk-dev] [PATCH 2/2] virtio: change io privilege level as early as possible David Marchand
2015-03-10 13:14 ` Neil Horman
2015-09-29 19:25 ` Stephen Hemminger [this message]
2015-09-30 8:28 ` David Marchand
2015-09-30 14:52 ` Neil Horman
2015-09-30 15:37 ` Thomas Monjalon
2015-09-30 17:26 ` Stephen Hemminger
2015-10-01 11:25 ` Neil Horman
2015-10-12 20:08 ` Stephen Hemminger
2015-10-14 0:07 ` Stephen Hemminger
2015-10-14 8:00 ` David Marchand
2015-10-14 9:49 ` David Marchand
2015-10-14 9:50 ` [dpdk-dev] [PATCH] eal: move interrupt init after device init David Marchand
2015-10-14 11:32 ` David Marchand
2015-10-20 21:22 ` Thomas Monjalon
2015-07-29 17:26 ` [dpdk-dev] [PATCH 1/2] virtio: initialize iopl when device is initialized Thomas Monjalon
2015-03-06 0:45 ` [dpdk-dev] [PATCH 2/2] virtio: allow running w/o vlan filtering Stephen Hemminger
2015-03-06 3:39 ` Ouyang, Changchun
2015-03-06 16:24 ` Stephen Hemminger
2015-07-29 12:56 ` Thomas Monjalon
2015-07-30 1:23 ` Ouyang, Changchun
2015-08-04 12:51 ` Vincent JARDIN
2015-08-05 1:01 ` Ouyang, Changchun
2015-08-05 1:22 ` Stephen Hemminger
2015-08-05 10:49 ` Vincent JARDIN
2015-10-21 13:58 ` Thomas Monjalon
2017-02-15 8:38 ` Thomas Monjalon
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150929122542.419781cc@urahara \
--to=stephen@networkplumber.org \
--cc=dev@dpdk.org \
--cc=nhorman@tuxdriver.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).