DPDK patches and discussions
 help / color / mirror / Atom feed
From: Stephen Hemminger <stephen@networkplumber.org>
To: Neil Horman <nhorman@tuxdriver.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, 13 Oct 2015 17:05:32 -0700
Message-ID: <20151013170532.05e44007@xeon-e3> (raw)
In-Reply-To: <20150310105541.GA7873@hmsreliant.think-freely.org>

On Tue, 10 Mar 2015 06:55:41 -0400
Neil Horman <nhorman@tuxdriver.com> wrote:

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

Given all this, I recommend that Thomas not apply this patch.
Please resubmit if there is a real problem with drivers (something in tree).
There are enough other bugs to fix without chasing ghosts.

  reply	other threads:[~2015-10-14  0:05 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 [this message]
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=20151013170532.05e44007@xeon-e3 \
    --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

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