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

[Panu Matilainen <pmatilai@redhat.com>]

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.

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

	- Panu -