Re: [dpdk-dev] [PATCH v2 1/4] mk: Remove combined library and related options

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

On 13/03/2015 13:16, Kavanagh, Mark B wrote:
>
>> -----Original Message-----
>> From: Gonzalez Monroy, Sergio
>> Sent: Friday, March 13, 2015 11:49 AM
>> To: Kavanagh, Mark B
>> Cc: dev@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v2 1/4] mk: Remove combined library and related options
>>
>> On 13/03/2015 11:34, Kavanagh, Mark B wrote:
>>>> On 13/03/2015 10:49, Kavanagh, Mark B wrote:
>>>>>> ---
>>>>>> config/common_bsdapp                        |   6 --
>>>>>> config/common_linuxapp                      |   6 --
>>>>>> config/defconfig_ppc_64-power8-linuxapp-gcc |   2 -
>>>>>> lib/Makefile                                |   1 -
>>>>>> mk/rte.app.mk                               |  12 ----
>>>>>> mk/rte.lib.mk                               |  35 ----------
>>>>>> mk/rte.sdkbuild.mk                          |   3 -
>>>>>> mk/rte.sharelib.mk                          | 101 ----------------------------
>>>>>> mk/rte.vars.mk                              |   9 ---
>>>>>> 9 files changed, 175 deletions(-)
>>>>>> delete mode 100644 mk/rte.sharelib.mk
>>>>>>
>>>>>> diff --git a/config/common_bsdapp b/config/common_bsdapp
>>>>>> index 8ff4dc2..7ee5ecf 100644
>>>>>> --- a/config/common_bsdapp
>>>>>> +++ b/config/common_bsdapp
>>>>>> @@ -79,12 +79,6 @@ CONFIG_RTE_FORCE_INTRINSICS=n
>>>>>> CONFIG_RTE_BUILD_SHARED_LIB=n
>>>>>>
>>>>>> #
>>>>>> -# Combine to one single library
>>>>>> -#
>>>>>> -CONFIG_RTE_BUILD_COMBINE_LIBS=n
>>>>>> -CONFIG_RTE_LIBNAME=intel_dpdk
>>>>> Hi Sergio,
>>>>>
>>>>> Removing these options breaks compatibility with OVS. While it may be feasible to link
>>>> to individual static libraries, in our experience, a single combined library provides a
>>>> much more convenient way of linking.
>>>>> Thanks,
>>>>> Mark
>>>>>
>>>>>> -
>>> (snip)
>>>
>>>
>>>>>> -endif
>>>>>> -
>>>>>> -RTE_LIBNAME := $(CONFIG_RTE_LIBNAME:"%"=%)
>>>>>> -ifeq ($(RTE_LIBNAME),)
>>>>>> -RTE_LIBNAME := intel_dpdk
>>>>>> endif
>>>>>>
>>>>>> # RTE_TARGET is deducted from config when we are building the SDK.
>>>>>> --
>>>>>> 1.9.3
>>>> Hi Mark,
>>>>
>>>> How does this patch break compatibility with OVS?
>>>>
>>>> Thanks,
>>>> Sergio
>>> Hey Sergio,
>>>
>>> We use the CONFIG_RTE_BUILD_COMBINE_LIBS and CONFIG_RTE_LINBNAME flags to build a single
>> static DPDK library, named 'libintel_dpdk.a', which OVS links against. Removing the
>> combined library option breaks compatibility with any application that links against the
>> combined DPDK library.
>>> Is there a strong technical motivation for removing these options?
>>>
>>> Thanks,
>>> Mark
>>  From a shared library point of view, it just does not make sense to
>> have applications linked against a 'combined' library that may have
>> different features built in it.
>>
> For OVS, we don't build DPDK as a set of shared libraries, but rather an individual static library, due to the performance penalties inherent in using shared libraries.
>
>> Removing these options, aside from the obvious 'less build config
>> option', it simplifies maintenance of makefiles as we currently have a
>> separated makefile with specific rules just for combined library.
>>
>> It is pretty straight forward to build a single combined archive out of
>> multiple archives, would it be acceptable to have a script to do this?
>>
> This seems a bit 'hacky' to me and I'm not sure that it would be amenable to the OVS maintainers. Unless I'm overlooking something here, I'd prefer to maintain the status quo.
This may be a case of personal opinions, but I don't think there is 
anything 'hacky' about it. Straight forward extract objects from 
archive, then archive them into a single library.

Currently to create a combined library we just copy all objects into one 
directory, then create the combined archive.
After the patch, the script just needs to extract all objects from 
individual libraries into a directory and archive them all.

In my opinion, one little exra step, same result plus we get less build 
config options, simplify lib building by removing the 'combined' lib path.

>> Thanks,
>> Sergio