Re: [dpdk-dev] [PATCH 0/8] Improve build process

[Neil Horman <nhorman@tuxdriver.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)
> >>
> >>
> 
>