DPDK patches and discussions
 help / color / mirror / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: David Marchand <david.marchand@6wind.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 1/2] eal/linux: move plugin load to very start of eal init
Date: Tue, 10 Mar 2015 06:55:41 -0400
Message-ID: <20150310105541.GA7873@hmsreliant.think-freely.org> (raw)
In-Reply-To: <CALwxeUs4hPbYDPBUfz9u2AoiCoj_wwTsAyj=_1xxzuT6LLW6nw@mail.gmail.com>

On Tue, Mar 10, 2015 at 10:08:24AM +0100, David Marchand wrote:
> Hello Neil,
> 
> On Mon, Mar 9, 2015 at 4:21 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> 
> > On Mon, Mar 09, 2015 at 03:56:38PM +0100, David Marchand wrote:
> > > Loading shared libraries should be done at the very start of eal init so
> > that
> > > the code statically built in dpdk and the code loaded from shared
> > objects is
> > > handled (almost) the same way wrt to call to rte_eal_init().
> > > The only thing that must be done before is filling the solib_list which
> > is done
> > > by eal_parse_args().
> > >
> >
> >
> > I don't see anything explicitly wrong with this, but at the same time it
> > doesn't
> > seem to fix anything.  Is there a particular bug that you're fixing in
> > relation
> > to your cover letter here?  Or is there some expectation that PMD's loaded
> > in
> > this fashion expect the dpdk to be completely uninitalized?  That would
> > seem
> > like a strange operational requirement to me.
> >
> 
> Well, at first, I wanted to fix the virtio pmd init issue (iopl() not
> called at the right place wrt to other pthreads created in rte_eal_init()).
Ah, this is what you were addressing:
http://dpdk.org/ml/archives/dev/2015-March/014765.html

> With next patch, this issue is fixed for statically builtin virtio pmd, but
> for virtio pmd as a shared object, the dlopen comes too late.
> So, yes, I moved the dlopen() for this reason.
> 
But this doesn't do anything to help you.  The goal, according to the above
thread, is to initalize the pmd earlier so that you can call iopl prior to doing
any forks (so that io privlidges are inherited).  But both static and dynamic
pmd have constructors that just register their driver structures.  No
initalization happens until rte_eal_dev_init is called.  So this movement does
nothing to change the time any given drivers init routine is called.

> From a more general point of view, since we support both static and dso
> pmds, I would say that this is more logical to have dlopen comes very
> early, since static code is "loaded" even earlier : if the current pmds
> needed more than just register to the driver list, they would already have
> triggered segfaults and/or bugs.
> 
No, not really.  I suppose it doesn't hurt anything, but moving this earlier in
a function doesn't really buy you anything, as statically allocate pmds are
called by the gcc start code prior to an applications main routine running, so
we're never actually going to get close to parity there, nor do we need to,
because the actual init happens at rte_eal_dev_init, which is in parity for both
static and dynamic drivers.

> 
> I know this change comes really late for 2.0.
> I am open to other ideas but I don't want to see more #ifdef <some feature>
> in eal.c (especially for a pmd), this is a non sense.
> 
> I would say that at least the patch 2 is needed for 2.0 : it fixes the
> static case, but without patch 1 virtio pmd triggers a segfault on
> interrupt receipt when built as a dso.
> 
The static case suffers from problems as well I think, in that its possible to
architect multiple processes that are not started from fork that use the same
pmd, which would create the same issue.  I think a better course of action would
be to document the need for an application to call iopl before rte_eal_init.

Neil

> 
> -- 
> David Marchand

  parent reply	other threads:[~2015-03-10 10:55 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 [this message]
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
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=20150310105541.GA7873@hmsreliant.think-freely.org \
    --to=nhorman@tuxdriver.com \
    --cc=david.marchand@6wind.com \
    --cc=dev@dpdk.org \
    /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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git