DPDK patches and discussions
 help / color / mirror / Atom feed
From: Neil Horman <nhorman@tuxdriver.com>
To: "Gonzalez Monroy, Sergio" <sergio.gonzalez.monroy@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH 0/8] Improve build process
Date: Mon, 23 Feb 2015 08:52:05 -0500	[thread overview]
Message-ID: <20150223135205.GA19230@hmsreliant.think-freely.org> (raw)
In-Reply-To: <54EAFFFD.5000200@intel.com>

On Mon, Feb 23, 2015 at 10:25:01AM +0000, Gonzalez Monroy, Sergio wrote:
> On 22/02/2015 23:37, Neil Horman wrote:
> >On Fri, Feb 20, 2015 at 02:31:36PM +0000, Gonzalez Monroy, Sergio wrote:
> >>On 13/02/2015 12:51, Neil Horman wrote:
> >>>On Fri, Feb 13, 2015 at 11:08:02AM +0000, Gonzalez Monroy, Sergio wrote:
> >>>>On 13/02/2015 10:14, Panu Matilainen wrote:
> >>>>>On 02/12/2015 05:52 PM, Neil Horman wrote:
> >>>>>>On Thu, Feb 12, 2015 at 04:07:50PM +0200, Panu Matilainen wrote:
> >>>>>>>On 02/12/2015 02:23 PM, Neil Horman wrote:
> >>>>>[...snip...]
> >>>>>>>>>>>So I just realized that I was not having into account a possible
> >>>>>>>>>>>scenario, where
> >>>>>>>>>>>we have an app built with static dpdk libs then loading a dso
> >>>>>>>>>>>with -d
> >>>>>>>>>>>option.
> >>>>>>>>>>>
> >>>>>>>>>>>In such case, because the pmd would have DT_NEEDED entries,
> >>>>>>>>>>>dlopen will
> >>>>>>>>>>>fail.
> >>>>>>>>>>>So to enable such scenario we would need to build PMDs without
> >>>>>>>>>>>DT_NEEDED
> >>>>>>>>>>>entries.
> >>>>>>>>>>Hmm, for that to be a problem you'd need to have the PMD built
> >>>>>>>>>>against
> >>>>>>>>>>shared dpdk libs and while the application is built against
> >>>>>>>>>>static dpdk
> >>>>>>>>>>libs. I dont think that's a supportable scenario in any case.
> >>>>>>>>>>
> >>>>>>>>>>Or is there some other scenario that I'm not seeing?
> >>>>>>>>>>
> >>>>>>>>>>    - Panu -
> >>>>>>>>>>
> >>>>>>>>>I agree with you. I suppose it comes down to, do we want to
> >>>>>>>>>support such
> >>>>>>>>>scenario?
> >>>>>>>>>
> >>>>>>>>> From what I can see, it seems that we do currently support such
> >>>>>>>>>scenario by
> >>>>>>>>>building dpdk apps against all static dpdk libs using
> >>>>>>>>>--whole-archive (all
> >>>>>>>>>libs and not only PMDs).
> >>>>>>>>>http://dpdk.org/browse/dpdk/commit/?id=20afd76a504155e947c770783ef5023e87136ad8
> >>>>>>>>>
> >>>>>>>>>
> >>>>>>>>>Am I misunderstanding this?
> >>>>>>>>>
> >>>>>>>>Shoot, you're right, I missed the static build aspect to this.  Yes,
> >>>>>>>>if we do the following:
> >>>>>>>>
> >>>>>>>>1) Build the DPDK as a static library
> >>>>>>>>2) Link an application against (1)
> >>>>>>>>3) Use the dlopen mechanism to load a PMD built as a DSO
> >>>>>>>>
> >>>>>>>>Then the DT_NEEDED entries in the DSO will go unsatisfied, because
> >>>>>>>>the shared
> >>>>>>>>objects on which it (the PMD) depends will not exist in the file
> >>>>>>>>system.
> >>>>>>>I think its even more twisty:
> >>>>>>>
> >>>>>>>1) Build the DPDK as a static library
> >>>>>>>2) Link an application against (1)
> >>>>>>>3) Do another build of DPDK as a shared library
> >>>>>>>4) In app 2), use the dlopen mechanism to load a PMD built as a part
> >>>>>>>of or
> >>>>>>>against 3)
> >>>>>>>
> >>>>>>>Somehow I doubt this would work very well.
> >>>>>>>
> >>>>>>Ideally it should, presuming the ABI is preserved between (1) and (3),
> >>>>>>though I
> >>>>>>agree, up until recently, that was an assumption that was unreliable.
> >>>>>Versioning is a big and important step towards reliability but there are
> >>>>>more issues to solve. This of course getting pretty far from the original
> >>>>>topic, but at least one such issue is that there are some cases where a
> >>>>>config value affects what are apparently public structs (rte_mbuf wrt
> >>>>>RTE_MBUF_REFCNT for example), which really is a no-go.
> >>>>>
> >>>>Agree, the RTE_MBUF_REFCNT is something that needs to be dealt with asap.
> >>>>I'll look into it.
> >>>>
> >>>>>>>>I think the problem is a little bit orthogonal to the libdpdk_core
> >>>>>>>>problem you
> >>>>>>>>were initially addressing.  That is to say, this problem of
> >>>>>>>>dlopen-ed PMD's
> >>>>>>>>exists regardless of weather you build the DPDK as part of a static
> >>>>>>>>or dynamic
> >>>>>>>>library.  The problems just happen to intersect in their
> >>>>>>>>manipulation of the
> >>>>>>>>DT_NEEDED entries.
> >>>>>>>>
> >>>>>>>>Ok, so, given the above, I would say your approach is likely
> >>>>>>>>correct, just
> >>>>>>>>prevent DT_NEEDED entries from getting added to PMD's. Doing so will
> >>>>>>>>sidestep
> >>>>>>>>loading issue for libraries that may not exist in the filesystem,
> >>>>>>>>but thats ok,
> >>>>>>>>because by all rights, the symbols codified in those needed
> >>>>>>>>libraries should
> >>>>>>>>already be present in the running application (either made available
> >>>>>>>>by the
> >>>>>>>>application having statically linked them, or having the linker load
> >>>>>>>>them from
> >>>>>>>>the proper libraries at run time).
> >>>>>>>My 5c is that I'd much rather see the common case (all static or all
> >>>>>>>shared)
> >>>>>>>be simple and reliable, which in case of DSOs includes no lying
> >>>>>>>(whether by
> >>>>>>>omission or otherwise) about DT_NEEDED, ever. That way the issue is
> >>>>>>>dealt
> >>>>>>>once where it belongs. If somebody wants to go down the rabbit hole of
> >>>>>>>mixed
> >>>>>>>shared + static linkage, let them dig the hole by themselves :)
> >>>>>>>
> >>>>>>This is a fair point.  Can DT_NEEDED sections be stripped via tools like
> >>>>>>objcopy
> >>>>>>after the build is complete?  If so, end users can hack this corner case
> >>>>>>to work
> >>>>>>as needed.
> >>>>>Patchelf (http://nixos.org/patchelf.html) appears to support that, but
> >>>>>given that source is available it'd be easier to just modify the makefiles
> >>>>>if that's really needed.
> >>>>>
> >>>>I think we agree on the issue.
> >>>>
> >>>>So I'll be sending a patch to add DT_NEEDED entries to all libraries and
> >>>>PMDs. The only exception would be librte_eal, which would not have proper
> >>>>NEEDED entries.
> >>>>Do we bother adding a linker script for librte_eal that would include
> >>>>dependent libraries?
> >>>>
> >>>I say yes to the linker script, but will happily bow to an alternate consensus
> >>>Neil
> >>>
> >>So the case we want to solve is the following circular dependencies:
> >>eal             -> mempool, malloc
> >>mempool -> eal , malloc, ring
> >>malloc      -> eal
> >>ring           -> eal, malloc
> >>
> >>We cannot write/create the proposed (below) linker script at least until we
> >>have built mempool and malloc.
> >>INPUT ( -lrte_eal.so -lrte_mempool -lrte_malloc )
> >>
> >Not sure I understand why you have a build time dependency on this.  Link time
> >perhaps, but not build time.  Or am I reading too much into your use of the term
> >'built' above?
> I meant 'built' as compiled + linked. Am I misusing the term?
No, you're not (though I misused the term link time above, I meant to say load
time).  So you're saying that when you build shared libraries, you get linker
errors indicating that, during the build, you're missing symbols, is that
correct?  I guess I'm confused because I don't see how thats not happening for
everyone, right now.  In other words, I'm not sure what about your changes is
giving rise to that problem.

> >>Few ways I have thought about implementing this (not particularly fond of
> >>any of them) :
> >>  - Have the linker script file in the repo (scripts/ ?) in a fixed location
> >>and just copy it to $(RTE_OUTPUT)/lib/ once all libs have finished building.
> >>  - Generate the file on build time from a defined make variable once all
> >>libs have finished
> >>
> >I'm still not sure I understand.  Why does this dependency exist at build time?
> >The dependency between malloc and eal shouldn't be a problem during the build,
> >as symbols from each other should just remain undefined, and get resolved at
> >load time.
> Is that not the way it is currently implemented?
> I get the impression that we are talking about different goals (correct me
> if it is not the case)
> 
We may well be, I'm not sure yet.

> I thought that the agreed solution was to:
> 1) NOT to create/generate a 'core' library
> 2) Add DT_NEEDED entries for all libraries (except eal which is the first
> library we link)
> 3) Use linker script for eal
> 
Ok, we're definately on the same page, as thats what I thought the goal was as
well.

> Given the previously mentioned circular dependencies between eal, mempool,
> malloc and ring:
> - eal would not be linked against other libraries (no NEEDED entries)
> - malloc is linked against eal (previously built), so malloc would have a
> NEEDED entry for eal.
> 
> In that scenario, if the linker script is setup/created after we build eal,
> then when we try to link malloc
> against eal, the linker will pull mempool and malloc too (because we
> included them in the linker script).
> Therefore, the link fails as none of those libraries (malloc and mempool)
> have been built yet.
> 
Ah, I see now, I wasn't thinking about the extra requirements that DT_NEEDED
entries placed on the build conditions.

I see now, apologies for being dense previously.  Given what you indicate I
would say that the solution here is to link the libraries against individual
other specific libraries, not the core library that you generate as a linker
script.  That way you avoid the circular dependency, and the core library just
becomes a convienience for application developers looking to link to a single
library.

Neil

> Was your suggestion to leave all of these libraries (eal, mempool, malloc,
> ring) without NEEDED entries?
> 
No, you can add NEEDED entries there, they will just be for the individual
libraries, not the core linker script library.

Best
Neil

> Regards,
> Sergio
> >What is the error you are getting?
> >
> >Best
> >Neil
> >
> >>Thoughts? any other approached is more than welcome!
> >>
> >>Sergio
> >>
> >>PS: Thinking again on the core library and the issue of having multiple
> >>version.map files, we could have a core_version.map instead instead of
> >>multiple files per core library (eal, mempool, etc)
> >>
> >>
> 
> 

  reply	other threads:[~2015-02-23 13:52 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-01-29 15:20 Sergio Gonzalez Monroy
2015-01-29 15:20 ` [dpdk-dev] [PATCH 1/8] mk: remove combined library and related options Sergio Gonzalez Monroy
2015-01-29 15:20 ` [dpdk-dev] [PATCH 2/8] core: create new librte_core Sergio Gonzalez Monroy
2015-01-29 15:20 ` [dpdk-dev] [PATCH 3/8] mk: new corelib makefile Sergio Gonzalez Monroy
2015-01-29 15:20 ` [dpdk-dev] [PATCH 4/8] lib: update DEPDIRS variable Sergio Gonzalez Monroy
2015-01-29 15:20 ` [dpdk-dev] [PATCH 5/8] lib: set LDLIBS for each library Sergio Gonzalez Monroy
2015-01-29 15:20 ` [dpdk-dev] [PATCH 6/8] mk: use LDLIBS when linking shared libraries Sergio Gonzalez Monroy
2015-01-29 15:20 ` [dpdk-dev] [PATCH 7/8] mk: update LDLIBS for app building Sergio Gonzalez Monroy
2015-01-29 15:20 ` [dpdk-dev] [PATCH 8/8] mk: add -lpthread to linuxapp EXECENV_LDLIBS Sergio Gonzalez Monroy
2015-01-29 16:38 ` [dpdk-dev] [PATCH 0/8] Improve build process Neil Horman
2015-01-29 17:02   ` Thomas Monjalon
2015-01-29 17:04   ` Gonzalez Monroy, Sergio
2015-01-29 19:45     ` Neil Horman
2015-01-30 13:39       ` Gonzalez Monroy, Sergio
2015-01-30 14:05         ` Neil Horman
2015-01-30 17:38           ` Gonzalez Monroy, Sergio
2015-01-30 18:12             ` Neil Horman
2015-02-11 11:11               ` Gonzalez Monroy, Sergio
2015-02-12  5:41                 ` Neil Horman
2015-02-12  9:17                   ` Gonzalez Monroy, Sergio
2015-02-12 12:16                     ` Neil Horman
2015-02-12  9:22                 ` Panu Matilainen
2015-02-12 10:03                   ` Gonzalez Monroy, Sergio
2015-02-12 12:23                     ` Neil Horman
2015-02-12 14:07                       ` Panu Matilainen
2015-02-12 15:52                         ` Neil Horman
2015-02-13 10:14                           ` Panu Matilainen
2015-02-13 11:08                             ` Gonzalez Monroy, Sergio
2015-02-13 12:51                               ` Neil Horman
2015-02-20 14:31                                 ` Gonzalez Monroy, Sergio
2015-02-22 23:37                                   ` Neil Horman
2015-02-23 10:25                                     ` Gonzalez Monroy, Sergio
2015-02-23 13:52                                       ` Neil Horman [this message]
2015-02-23 14:58                                         ` Gonzalez Monroy, Sergio
2015-02-23 18:23                                           ` Neil Horman
2015-02-24 13:24                                             ` Gonzalez Monroy, Sergio
2015-03-12 16:27 ` [dpdk-dev] [PATCH v2 0/4] " Sergio Gonzalez Monroy
2015-03-12 16:27   ` [dpdk-dev] [PATCH v2 1/4] mk: Remove combined library and related options Sergio Gonzalez Monroy
2015-03-13 10:49     ` Kavanagh, Mark B
2015-03-13 11:19       ` Gonzalez Monroy, Sergio
2015-03-13 11:34         ` Kavanagh, Mark B
2015-03-13 11:48           ` Gonzalez Monroy, Sergio
2015-03-13 13:16             ` Kavanagh, Mark B
2015-03-13 14:11               ` Gonzalez Monroy, Sergio
2015-03-13 13:17             ` Neil Horman
2015-03-13 14:12               ` Stefan Puiu
2015-03-13 15:18                 ` Neil Horman
2015-03-13 15:28                   ` Gonzalez Monroy, Sergio
2015-03-13 16:16                     ` Neil Horman
2015-03-13 16:07     ` Stephen Hemminger
2015-03-13 16:32       ` Neil Horman
2015-03-13 16:38       ` Gonzalez Monroy, Sergio
2015-03-18 12:11     ` Gonzalez Monroy, Sergio
2015-03-18 12:59       ` Thomas Monjalon
2015-03-18 15:30         ` Stefan Puiu
2015-03-18 15:52           ` Gonzalez Monroy, Sergio
2015-03-18 16:48           ` Neil Horman
2015-03-26  8:52         ` Gonzalez Monroy, Sergio
2015-03-26 10:30           ` Neil Horman
2015-03-18 16:41       ` Neil Horman
2015-03-12 16:27   ` [dpdk-dev] [PATCH v2 2/4] lib: Set LDLIBS for each library Sergio Gonzalez Monroy
2015-03-12 16:28   ` [dpdk-dev] [PATCH v2 3/4] mk: Use LDLIBS when linking shared libraries Sergio Gonzalez Monroy
2015-03-12 16:28   ` [dpdk-dev] [PATCH v2 4/4] mk: update LDLIBS for app building Sergio Gonzalez Monroy

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=20150223135205.GA19230@hmsreliant.think-freely.org \
    --to=nhorman@tuxdriver.com \
    --cc=dev@dpdk.org \
    --cc=sergio.gonzalez.monroy@intel.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).