Re: [PATCH 0/8] Improve build process

["Gonzalez Monroy, Sergio" <sergio.gonzalez.monroy-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>]

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 )

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

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)