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

["Gonzalez Monroy, Sergio" <sergio.gonzalez.monroy@intel.com>]

On 23/02/2015 13:52, Neil Horman wrote:
> 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.
>
I'm not sure I quite understand your suggestion.

Could you roughly describe steps for building eal, malloc and mempool libs ?
For example, something like this?
1) build eal, which creates librte_eal.so.1
2) write linker script for librte_eal.so
3) build malloc against eal (-lrte_eal )
etc

I suppose that another way to go about this, instead of creating the 
linker script that pulls
dependent libraries, is to always link (using --no-as-needed in case gcc 
adds it by default)
against these libraries (eal, mempool, malloc, and ring) with necessary 
doc about how to build apps.

Sergio
> 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)
>>>>
>>>>
>>
>>