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

Message ID 1426177681-16931-2-git-send-email-sergio.gonzalez.monroy@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Sergio Gonzalez Monroy March 12, 2015, 4:27 p.m. UTC
  Remove CONFIG_RTE_BUILD_COMBINE_LIBS and CONFIG_RTE_LIBNAME.

Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
---
 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
  

Comments

Mark Kavanagh March 13, 2015, 10:49 a.m. UTC | #1
>---
> 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 

>-
>-#
> # Compile Environment Abstraction Layer
> #
> CONFIG_RTE_LIBRTE_EAL=y
>diff --git a/config/common_linuxapp b/config/common_linuxapp
>index 97f1c9e..ae13805 100644
>--- a/config/common_linuxapp
>+++ b/config/common_linuxapp
>@@ -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"
>-
>-#
> # Compile Environment Abstraction Layer
> #
> CONFIG_RTE_LIBRTE_EAL=y
>diff --git a/config/defconfig_ppc_64-power8-linuxapp-gcc b/config/defconfig_ppc_64-power8-
>linuxapp-gcc
>index d97a885..f1af518 100644
>--- a/config/defconfig_ppc_64-power8-linuxapp-gcc
>+++ b/config/defconfig_ppc_64-power8-linuxapp-gcc
>@@ -39,8 +39,6 @@ CONFIG_RTE_ARCH_64=y
> CONFIG_RTE_TOOLCHAIN="gcc"
> CONFIG_RTE_TOOLCHAIN_GCC=y
>
>-CONFIG_RTE_LIBNAME="powerpc_dpdk"
>-
> # Note: Power doesn't have this support
> CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
>
>diff --git a/lib/Makefile b/lib/Makefile
>index d94355d..c34cf2f 100644
>--- a/lib/Makefile
>+++ b/lib/Makefile
>@@ -77,5 +77,4 @@ DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
> DIRS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += librte_ivshmem
> endif
>
>-include $(RTE_SDK)/mk/rte.sharelib.mk
> include $(RTE_SDK)/mk/rte.subdir.mk
>diff --git a/mk/rte.app.mk b/mk/rte.app.mk
>index 63a41e2..e2baa49 100644
>--- a/mk/rte.app.mk
>+++ b/mk/rte.app.mk
>@@ -61,12 +61,6 @@ ifeq ($(NO_AUTOLIBS),)
>
> LDLIBS += --whole-archive
>
>-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
>-LDLIBS += -l$(RTE_LIBNAME)
>-endif
>-
>-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
>-
> ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y)
> LDLIBS += -lrte_distributor
> endif
>@@ -137,8 +131,6 @@ ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y)
> LDLIBS += -lrte_vhost
> endif
>
>-endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>-
> ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> LDLIBS += -lpcap
> endif
>@@ -153,8 +145,6 @@ endif
>
> LDLIBS += --start-group
>
>-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
>-
> ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
> LDLIBS += -lrte_kvargs
> endif
>@@ -253,8 +243,6 @@ endif
>
> endif # plugins
>
>-endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>-
> LDLIBS += $(EXECENV_LDLIBS)
>
> LDLIBS += --end-group
>diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
>index 0d7482d..d96101a 100644
>--- a/mk/rte.lib.mk
>+++ b/mk/rte.lib.mk
>@@ -87,24 +87,6 @@ O_TO_S_DO = @set -e; \
> 	$(O_TO_S) && \
> 	echo $(O_TO_S_CMD) > $(call exe2cmd,$(@))
>
>-ifeq ($(RTE_BUILD_SHARED_LIB),n)
>-O_TO_C = $(AR) crus $(LIB_ONE) $(OBJS-y)
>-O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
>-O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  AR_C $(@)")
>-O_TO_C_DO = @set -e; \
>-	$(lib_dir) \
>-	$(copy_obj)
>-else
>-O_TO_C = $(LD) -shared $(OBJS-y) -o $(LIB_ONE)
>-O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
>-O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  LD_C $(@)")
>-O_TO_C_DO = @set -e; \
>-	$(lib_dir) \
>-	$(copy_obj)
>-endif
>-
>-copy_obj = cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib;
>-lib_dir = [ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib;
> -include .$(LIB).cmd
>
> #
>@@ -129,15 +111,6 @@ endif
> 		$(depfile_missing),\
> 		$(depfile_newer)),\
> 		$(O_TO_S_DO))
>-
>-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
>-	$(if $(or \
>-        $(file_missing),\
>-        $(call cmdline_changed,$(O_TO_C_STR)),\
>-        $(depfile_missing),\
>-        $(depfile_newer)),\
>-        $(O_TO_C_DO))
>-endif
> else
> $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
> 	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
>@@ -153,14 +126,6 @@ $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
> 	    $(depfile_missing),\
> 	    $(depfile_newer)),\
> 	    $(O_TO_A_DO))
>-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
>-	$(if $(or \
>-        $(file_missing),\
>-        $(call cmdline_changed,$(O_TO_C_STR)),\
>-        $(depfile_missing),\
>-        $(depfile_newer)),\
>-        $(O_TO_C_DO))
>-endif
> endif
>
> #
>diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
>index 3154457..2b24e74 100644
>--- a/mk/rte.sdkbuild.mk
>+++ b/mk/rte.sdkbuild.mk
>@@ -93,9 +93,6 @@ $(ROOTDIRS-y):
> 	@[ -d $(BUILDDIR)/$@ ] || mkdir -p $(BUILDDIR)/$@
> 	@echo "== Build $@"
> 	$(Q)$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile -C $(BUILDDIR)/$@ all
>-	@if [ $@ = lib -a $(RTE_BUILD_COMBINE_LIBS) = y ]; then \
>-		$(MAKE) -f $(RTE_SDK)/lib/Makefile sharelib; \
>-	fi
>
> %_clean:
> 	@echo "== Clean $*"
>diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
>deleted file mode 100644
>index de53558..0000000
>--- a/mk/rte.sharelib.mk
>+++ /dev/null
>@@ -1,101 +0,0 @@
>-#   BSD LICENSE
>-#
>-#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
>-#   All rights reserved.
>-#
>-#   Redistribution and use in source and binary forms, with or without
>-#   modification, are permitted provided that the following conditions
>-#   are met:
>-#
>-#     * Redistributions of source code must retain the above copyright
>-#       notice, this list of conditions and the following disclaimer.
>-#     * Redistributions in binary form must reproduce the above copyright
>-#       notice, this list of conditions and the following disclaimer in
>-#       the documentation and/or other materials provided with the
>-#       distribution.
>-#     * Neither the name of Intel Corporation nor the names of its
>-#       contributors may be used to endorse or promote products derived
>-#       from this software without specific prior written permission.
>-#
>-#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>-#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>-#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>-#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>-#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>-#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>-#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>-#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>-#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>-#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>-#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>-
>-include $(RTE_SDK)/mk/internal/rte.build-pre.mk
>-
>-# VPATH contains at least SRCDIR
>-VPATH += $(SRCDIR)
>-
>-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
>-ifeq ($(RTE_BUILD_SHARED_LIB),y)
>-LIB_ONE := lib$(RTE_LIBNAME).so
>-else
>-LIB_ONE := lib$(RTE_LIBNAME).a
>-endif
>-endif
>-
>-.PHONY:sharelib
>-sharelib: $(LIB_ONE) FORCE
>-
>-OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
>-
>-ifeq ($(LINK_USING_CC),1)
>-# Override the definition of LD here, since we're linking with CC
>-LD := $(CC) $(CPU_CFLAGS)
>-O_TO_S = $(LD) $(call linkerprefix,$(CPU_LDFLAGS)) \
>-	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
>-else
>-O_TO_S = $(LD) $(CPU_LDFLAGS) \
>-	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
>-endif
>-
>-O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
>-O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
>-O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)"
>-O_TO_S_DO = @set -e; \
>-    echo $(O_TO_S_DISP); \
>-    $(O_TO_S)
>-
>-O_TO_A =  $(AR) crus $(RTE_OUTPUT)/lib/$(LIB_ONE) $(OBJS)
>-O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
>-O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  LD $(@)")
>-O_TO_A_CMD = "cmd_$@ = $(O_TO_A_STR)"
>-O_TO_A_DO = @set -e; \
>-    echo $(O_TO_A_DISP); \
>-    $(O_TO_A)
>-#
>-# Archive objects to share library
>-#
>-
>-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
>-ifeq ($(RTE_BUILD_SHARED_LIB),y)
>-$(LIB_ONE): FORCE
>-	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
>-	$(O_TO_S_DO)
>-else
>-$(LIB_ONE): FORCE
>-	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
>-	$(O_TO_A_DO)
>-endif
>-endif
>-
>-#
>-# Clean all generated files
>-#
>-.PHONY: clean
>-clean: _postclean
>-
>-.PHONY: doclean
>-doclean:
>-	$(Q)rm -rf $(LIB_ONE)
>-
>-.PHONY: FORCE
>-FORCE:
>diff --git a/mk/rte.vars.mk b/mk/rte.vars.mk
>index d2f01b6..7b6f53d 100644
>--- a/mk/rte.vars.mk
>+++ b/mk/rte.vars.mk
>@@ -67,15 +67,6 @@ ifneq ($(BUILDING_RTE_SDK),)
>   ifeq ($(RTE_BUILD_SHARED_LIB),)
>     RTE_BUILD_SHARED_LIB := n
>   endif
>-  RTE_BUILD_COMBINE_LIBS := $(CONFIG_RTE_BUILD_COMBINE_LIBS:"%"=%)
>-  ifeq ($(RTE_BUILD_COMBINE_LIBS),)
>-    RTE_BUILD_COMBINE_LIBS := n
>-  endif
>-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
  
Sergio Gonzalez Monroy March 13, 2015, 11:19 a.m. UTC | #2
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
>
>> -
>> -#
>> # Compile Environment Abstraction Layer
>> #
>> CONFIG_RTE_LIBRTE_EAL=y
>> diff --git a/config/common_linuxapp b/config/common_linuxapp
>> index 97f1c9e..ae13805 100644
>> --- a/config/common_linuxapp
>> +++ b/config/common_linuxapp
>> @@ -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"
>> -
>> -#
>> # Compile Environment Abstraction Layer
>> #
>> CONFIG_RTE_LIBRTE_EAL=y
>> diff --git a/config/defconfig_ppc_64-power8-linuxapp-gcc b/config/defconfig_ppc_64-power8-
>> linuxapp-gcc
>> index d97a885..f1af518 100644
>> --- a/config/defconfig_ppc_64-power8-linuxapp-gcc
>> +++ b/config/defconfig_ppc_64-power8-linuxapp-gcc
>> @@ -39,8 +39,6 @@ CONFIG_RTE_ARCH_64=y
>> CONFIG_RTE_TOOLCHAIN="gcc"
>> CONFIG_RTE_TOOLCHAIN_GCC=y
>>
>> -CONFIG_RTE_LIBNAME="powerpc_dpdk"
>> -
>> # Note: Power doesn't have this support
>> CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
>>
>> diff --git a/lib/Makefile b/lib/Makefile
>> index d94355d..c34cf2f 100644
>> --- a/lib/Makefile
>> +++ b/lib/Makefile
>> @@ -77,5 +77,4 @@ DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
>> DIRS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += librte_ivshmem
>> endif
>>
>> -include $(RTE_SDK)/mk/rte.sharelib.mk
>> include $(RTE_SDK)/mk/rte.subdir.mk
>> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
>> index 63a41e2..e2baa49 100644
>> --- a/mk/rte.app.mk
>> +++ b/mk/rte.app.mk
>> @@ -61,12 +61,6 @@ ifeq ($(NO_AUTOLIBS),)
>>
>> LDLIBS += --whole-archive
>>
>> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
>> -LDLIBS += -l$(RTE_LIBNAME)
>> -endif
>> -
>> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
>> -
>> ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y)
>> LDLIBS += -lrte_distributor
>> endif
>> @@ -137,8 +131,6 @@ ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y)
>> LDLIBS += -lrte_vhost
>> endif
>>
>> -endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>> -
>> ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
>> LDLIBS += -lpcap
>> endif
>> @@ -153,8 +145,6 @@ endif
>>
>> LDLIBS += --start-group
>>
>> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
>> -
>> ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
>> LDLIBS += -lrte_kvargs
>> endif
>> @@ -253,8 +243,6 @@ endif
>>
>> endif # plugins
>>
>> -endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
>> -
>> LDLIBS += $(EXECENV_LDLIBS)
>>
>> LDLIBS += --end-group
>> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
>> index 0d7482d..d96101a 100644
>> --- a/mk/rte.lib.mk
>> +++ b/mk/rte.lib.mk
>> @@ -87,24 +87,6 @@ O_TO_S_DO = @set -e; \
>> 	$(O_TO_S) && \
>> 	echo $(O_TO_S_CMD) > $(call exe2cmd,$(@))
>>
>> -ifeq ($(RTE_BUILD_SHARED_LIB),n)
>> -O_TO_C = $(AR) crus $(LIB_ONE) $(OBJS-y)
>> -O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
>> -O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  AR_C $(@)")
>> -O_TO_C_DO = @set -e; \
>> -	$(lib_dir) \
>> -	$(copy_obj)
>> -else
>> -O_TO_C = $(LD) -shared $(OBJS-y) -o $(LIB_ONE)
>> -O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
>> -O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  LD_C $(@)")
>> -O_TO_C_DO = @set -e; \
>> -	$(lib_dir) \
>> -	$(copy_obj)
>> -endif
>> -
>> -copy_obj = cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib;
>> -lib_dir = [ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib;
>> -include .$(LIB).cmd
>>
>> #
>> @@ -129,15 +111,6 @@ endif
>> 		$(depfile_missing),\
>> 		$(depfile_newer)),\
>> 		$(O_TO_S_DO))
>> -
>> -ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
>> -	$(if $(or \
>> -        $(file_missing),\
>> -        $(call cmdline_changed,$(O_TO_C_STR)),\
>> -        $(depfile_missing),\
>> -        $(depfile_newer)),\
>> -        $(O_TO_C_DO))
>> -endif
>> else
>> $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
>> 	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
>> @@ -153,14 +126,6 @@ $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
>> 	    $(depfile_missing),\
>> 	    $(depfile_newer)),\
>> 	    $(O_TO_A_DO))
>> -ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
>> -	$(if $(or \
>> -        $(file_missing),\
>> -        $(call cmdline_changed,$(O_TO_C_STR)),\
>> -        $(depfile_missing),\
>> -        $(depfile_newer)),\
>> -        $(O_TO_C_DO))
>> -endif
>> endif
>>
>> #
>> diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
>> index 3154457..2b24e74 100644
>> --- a/mk/rte.sdkbuild.mk
>> +++ b/mk/rte.sdkbuild.mk
>> @@ -93,9 +93,6 @@ $(ROOTDIRS-y):
>> 	@[ -d $(BUILDDIR)/$@ ] || mkdir -p $(BUILDDIR)/$@
>> 	@echo "== Build $@"
>> 	$(Q)$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile -C $(BUILDDIR)/$@ all
>> -	@if [ $@ = lib -a $(RTE_BUILD_COMBINE_LIBS) = y ]; then \
>> -		$(MAKE) -f $(RTE_SDK)/lib/Makefile sharelib; \
>> -	fi
>>
>> %_clean:
>> 	@echo "== Clean $*"
>> diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
>> deleted file mode 100644
>> index de53558..0000000
>> --- a/mk/rte.sharelib.mk
>> +++ /dev/null
>> @@ -1,101 +0,0 @@
>> -#   BSD LICENSE
>> -#
>> -#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
>> -#   All rights reserved.
>> -#
>> -#   Redistribution and use in source and binary forms, with or without
>> -#   modification, are permitted provided that the following conditions
>> -#   are met:
>> -#
>> -#     * Redistributions of source code must retain the above copyright
>> -#       notice, this list of conditions and the following disclaimer.
>> -#     * Redistributions in binary form must reproduce the above copyright
>> -#       notice, this list of conditions and the following disclaimer in
>> -#       the documentation and/or other materials provided with the
>> -#       distribution.
>> -#     * Neither the name of Intel Corporation nor the names of its
>> -#       contributors may be used to endorse or promote products derived
>> -#       from this software without specific prior written permission.
>> -#
>> -#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
>> -#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
>> -#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
>> -#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
>> -#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
>> -#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
>> -#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
>> -#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
>> -#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
>> -#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
>> -#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> -
>> -include $(RTE_SDK)/mk/internal/rte.build-pre.mk
>> -
>> -# VPATH contains at least SRCDIR
>> -VPATH += $(SRCDIR)
>> -
>> -ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
>> -ifeq ($(RTE_BUILD_SHARED_LIB),y)
>> -LIB_ONE := lib$(RTE_LIBNAME).so
>> -else
>> -LIB_ONE := lib$(RTE_LIBNAME).a
>> -endif
>> -endif
>> -
>> -.PHONY:sharelib
>> -sharelib: $(LIB_ONE) FORCE
>> -
>> -OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
>> -
>> -ifeq ($(LINK_USING_CC),1)
>> -# Override the definition of LD here, since we're linking with CC
>> -LD := $(CC) $(CPU_CFLAGS)
>> -O_TO_S = $(LD) $(call linkerprefix,$(CPU_LDFLAGS)) \
>> -	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
>> -else
>> -O_TO_S = $(LD) $(CPU_LDFLAGS) \
>> -	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
>> -endif
>> -
>> -O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
>> -O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
>> -O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)"
>> -O_TO_S_DO = @set -e; \
>> -    echo $(O_TO_S_DISP); \
>> -    $(O_TO_S)
>> -
>> -O_TO_A =  $(AR) crus $(RTE_OUTPUT)/lib/$(LIB_ONE) $(OBJS)
>> -O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
>> -O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  LD $(@)")
>> -O_TO_A_CMD = "cmd_$@ = $(O_TO_A_STR)"
>> -O_TO_A_DO = @set -e; \
>> -    echo $(O_TO_A_DISP); \
>> -    $(O_TO_A)
>> -#
>> -# Archive objects to share library
>> -#
>> -
>> -ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
>> -ifeq ($(RTE_BUILD_SHARED_LIB),y)
>> -$(LIB_ONE): FORCE
>> -	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
>> -	$(O_TO_S_DO)
>> -else
>> -$(LIB_ONE): FORCE
>> -	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
>> -	$(O_TO_A_DO)
>> -endif
>> -endif
>> -
>> -#
>> -# Clean all generated files
>> -#
>> -.PHONY: clean
>> -clean: _postclean
>> -
>> -.PHONY: doclean
>> -doclean:
>> -	$(Q)rm -rf $(LIB_ONE)
>> -
>> -.PHONY: FORCE
>> -FORCE:
>> diff --git a/mk/rte.vars.mk b/mk/rte.vars.mk
>> index d2f01b6..7b6f53d 100644
>> --- a/mk/rte.vars.mk
>> +++ b/mk/rte.vars.mk
>> @@ -67,15 +67,6 @@ ifneq ($(BUILDING_RTE_SDK),)
>>    ifeq ($(RTE_BUILD_SHARED_LIB),)
>>      RTE_BUILD_SHARED_LIB := n
>>    endif
>> -  RTE_BUILD_COMBINE_LIBS := $(CONFIG_RTE_BUILD_COMBINE_LIBS:"%"=%)
>> -  ifeq ($(RTE_BUILD_COMBINE_LIBS),)
>> -    RTE_BUILD_COMBINE_LIBS := n
>> -  endif
>> -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
  
Mark Kavanagh March 13, 2015, 11:34 a.m. UTC | #3
>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
  
Sergio Gonzalez Monroy March 13, 2015, 11:48 a.m. UTC | #4
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.

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?

Thanks,
Sergio
  
Mark Kavanagh March 13, 2015, 1:16 p.m. UTC | #5
>-----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.

>Thanks,

>Sergio
  
Neil Horman March 13, 2015, 1:17 p.m. UTC | #6
On Fri, Mar 13, 2015 at 11:48:59AM +0000, Gonzalez Monroy, Sergio wrote:
> 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.
> 
> 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?
> 
> Thanks,
> Sergio
> 
+1

For the static case, its easy to do a post build combination of archives.  For
the shared library case, its equally easy to simply create a linker scripts call
<CONFIG_RTE_LIBNAME>.so that pulls in all the individual libraries.

Neil
  
Sergio Gonzalez Monroy March 13, 2015, 2:11 p.m. UTC | #7
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
  
Stefan Puiu March 13, 2015, 2:12 p.m. UTC | #8
Hi,

2 cents from a DPDK library user - I make 2 changes to the default
linux+gcc configuration: combine libraries and build shared libraries
(since I want 2 instances of the app, it didn't make sense to me to
link statically). I tried working with the individual libs, but adding
all of them with --start-group/-end-group just seemed so much more
painful than simply linking against one lib. I know there are some
Makefile variables to help with this, but I use scons for building my
app, so that doesn't help much.

Of course, if that can be achieved easily after building all the
libraries, that's fine. But I think combining the libs makes a lot of
sense in many cases.

Thanks,
Stefan.

On Fri, Mar 13, 2015 at 3:17 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> On Fri, Mar 13, 2015 at 11:48:59AM +0000, Gonzalez Monroy, Sergio wrote:
>> 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.
>>
>> 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?
>>
>> Thanks,
>> Sergio
>>
> +1
>
> For the static case, its easy to do a post build combination of archives.  For
> the shared library case, its equally easy to simply create a linker scripts call
> <CONFIG_RTE_LIBNAME>.so that pulls in all the individual libraries.
>
> Neil
>
  
Neil Horman March 13, 2015, 3:18 p.m. UTC | #9
On Fri, Mar 13, 2015 at 04:12:35PM +0200, Stefan Puiu wrote:
> Hi,
> 
> 2 cents from a DPDK library user - I make 2 changes to the default
> linux+gcc configuration: combine libraries and build shared libraries
> (since I want 2 instances of the app, it didn't make sense to me to
> link statically). I tried working with the individual libs, but adding
> all of them with --start-group/-end-group just seemed so much more
> painful than simply linking against one lib. I know there are some
> Makefile variables to help with this, but I use scons for building my
> app, so that doesn't help much.
> 
> Of course, if that can be achieved easily after building all the
> libraries, that's fine. But I think combining the libs makes a lot of
> sense in many cases.
> 
So do it, create a linker script that internally contains one line:
INPUT(-lrte_eal -lrte_alarm -lrte_mempool ... etc)

Name the file libdpdk.so

then when you build your app, just link -ldpdk

Done.

Neil

> Thanks,
> Stefan.
> 
> On Fri, Mar 13, 2015 at 3:17 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> > On Fri, Mar 13, 2015 at 11:48:59AM +0000, Gonzalez Monroy, Sergio wrote:
> >> 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.
> >>
> >> 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?
> >>
> >> Thanks,
> >> Sergio
> >>
> > +1
> >
> > For the static case, its easy to do a post build combination of archives.  For
> > the shared library case, its equally easy to simply create a linker scripts call
> > <CONFIG_RTE_LIBNAME>.so that pulls in all the individual libraries.
> >
> > Neil
> >
>
  
Sergio Gonzalez Monroy March 13, 2015, 3:28 p.m. UTC | #10
On 13/03/2015 15:18, Neil Horman wrote:
> On Fri, Mar 13, 2015 at 04:12:35PM +0200, Stefan Puiu wrote:
>> Hi,
>>
>> 2 cents from a DPDK library user - I make 2 changes to the default
>> linux+gcc configuration: combine libraries and build shared libraries
>> (since I want 2 instances of the app, it didn't make sense to me to
>> link statically). I tried working with the individual libs, but adding
>> all of them with --start-group/-end-group just seemed so much more
>> painful than simply linking against one lib. I know there are some
>> Makefile variables to help with this, but I use scons for building my
>> app, so that doesn't help much.
>>
>> Of course, if that can be achieved easily after building all the
>> libraries, that's fine. But I think combining the libs makes a lot of
>> sense in many cases.
>>
> So do it, create a linker script that internally contains one line:
> INPUT(-lrte_eal -lrte_alarm -lrte_mempool ... etc)
>
> Name the file libdpdk.so
>
> then when you build your app, just link -ldpdk
>
> Done.
>
> Neil

Plus I believe that as it currently stands, building combined shared 
libraries will be broken
the moment we have different versions of any API because the linking for 
the combined lib
does not use a version map.

Sergio
>> Thanks,
>> Stefan.
>>
>> On Fri, Mar 13, 2015 at 3:17 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
>>> On Fri, Mar 13, 2015 at 11:48:59AM +0000, Gonzalez Monroy, Sergio wrote:
>>>> 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.
>>>>
>>>> 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?
>>>>
>>>> Thanks,
>>>> Sergio
>>>>
>>> +1
>>>
>>> For the static case, its easy to do a post build combination of archives.  For
>>> the shared library case, its equally easy to simply create a linker scripts call
>>> <CONFIG_RTE_LIBNAME>.so that pulls in all the individual libraries.
>>>
>>> Neil
>>>
  
Stephen Hemminger March 13, 2015, 4:07 p.m. UTC | #11
On Thu, 12 Mar 2015 16:27:58 +0000
Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> wrote:

> Remove CONFIG_RTE_BUILD_COMBINE_LIBS and CONFIG_RTE_LIBNAME.
> 
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> ---

NAK. The combined library is good and useful for those who want simplicity
and build with static library.

It is not clear what you are trying to solve.
  
Neil Horman March 13, 2015, 4:16 p.m. UTC | #12
On Fri, Mar 13, 2015 at 03:28:00PM +0000, Gonzalez Monroy, Sergio wrote:
> On 13/03/2015 15:18, Neil Horman wrote:
> >On Fri, Mar 13, 2015 at 04:12:35PM +0200, Stefan Puiu wrote:
> >>Hi,
> >>
> >>2 cents from a DPDK library user - I make 2 changes to the default
> >>linux+gcc configuration: combine libraries and build shared libraries
> >>(since I want 2 instances of the app, it didn't make sense to me to
> >>link statically). I tried working with the individual libs, but adding
> >>all of them with --start-group/-end-group just seemed so much more
> >>painful than simply linking against one lib. I know there are some
> >>Makefile variables to help with this, but I use scons for building my
> >>app, so that doesn't help much.
> >>
> >>Of course, if that can be achieved easily after building all the
> >>libraries, that's fine. But I think combining the libs makes a lot of
> >>sense in many cases.
> >>
> >So do it, create a linker script that internally contains one line:
> >INPUT(-lrte_eal -lrte_alarm -lrte_mempool ... etc)
> >
> >Name the file libdpdk.so
> >
> >then when you build your app, just link -ldpdk
> >
> >Done.
> >
> >Neil
> 
> Plus I believe that as it currently stands, building combined shared
> libraries will be broken
> the moment we have different versions of any API because the linking for the
> combined lib
> does not use a version map.
> 
Correct, the above is the only way to create a single library that is properly
versioned, short of _only_ building a single library and exporting the version
map for that (which is non-sensical, as it defeats the purpose of DSO's).

> Sergio
> >>Thanks,
> >>Stefan.
> >>
> >>On Fri, Mar 13, 2015 at 3:17 PM, Neil Horman <nhorman@tuxdriver.com> wrote:
> >>>On Fri, Mar 13, 2015 at 11:48:59AM +0000, Gonzalez Monroy, Sergio wrote:
> >>>>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.
> >>>>
> >>>>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?
> >>>>
> >>>>Thanks,
> >>>>Sergio
> >>>>
> >>>+1
> >>>
> >>>For the static case, its easy to do a post build combination of archives.  For
> >>>the shared library case, its equally easy to simply create a linker scripts call
> >>><CONFIG_RTE_LIBNAME>.so that pulls in all the individual libraries.
> >>>
> >>>Neil
> >>>
> 
>
  
Neil Horman March 13, 2015, 4:32 p.m. UTC | #13
On Fri, Mar 13, 2015 at 09:07:59AM -0700, Stephen Hemminger wrote:
> On Thu, 12 Mar 2015 16:27:58 +0000
> Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> wrote:
> 
> > Remove CONFIG_RTE_BUILD_COMBINE_LIBS and CONFIG_RTE_LIBNAME.
> > 
> > Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> > ---
> 
> NAK. The combined library is good and useful for those who want simplicity
> and build with static library.
> 
No one is removing the ability to link against a single library, its simple to
do by hand.  The only thing this does is the codification of doing so as part of
the DPDK link process.

> It is not clear what you are trying to solve.
> 
In my mind, the problem is that using combined libs breaks the versioning
infrastrucure.  Because .o files can only be linked into .so files once, the
combined lib in the dpdk tree links all .o files at the same time.  But because
version maps are specified per individual library, and we can only specify one
at link time, we are left with a decision as to how to bring both together:

1) We can write a script to merge all the versioning files into one, and apply
that to the master link

2) Drop the combined libraries all together, and use a linker script to simulate
the merging of the libraries

Option two seems a bit rickety, and prone to failure, whereas option two seems
both easy and clean (at least in my view).  Just create a file libdpdk.so, that
is actually a text file with the contents:
INPUT(-lrte_malloc -lrte_mempool ....etc)

That will allow you to link applications to a single library -ldpdk, and things
will just work.  The only additional requriement that puts on your head is that
the individual libraries need to be installed locally on the target system, but
thats a run time requirement, not build time, and is easily managed with
whatever run time software management tools you care to use

Neil
  
Sergio Gonzalez Monroy March 13, 2015, 4:38 p.m. UTC | #14
On 13/03/2015 16:07, Stephen Hemminger wrote:
> On Thu, 12 Mar 2015 16:27:58 +0000
> Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com> wrote:
>
>> Remove CONFIG_RTE_BUILD_COMBINE_LIBS and CONFIG_RTE_LIBNAME.
>>
>> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
>> ---
> NAK. The combined library is good and useful for those who want simplicity
> and build with static library.
>
> It is not clear what you are trying to solve.
As I mentioned in other post, you can merge multiple archives into a 
single/combined one pretty easily.

I think we can agree that it is not useful for shared libraries, and 
with versioning in place, combined shared
library will be broken the moment we version an API.

I don't think we need build config  options just for the combined static 
library when we can achieve the
same result with a simple script post build.

Sergio
  
Sergio Gonzalez Monroy March 18, 2015, 12:11 p.m. UTC | #15
On 12/03/2015 16:27, Sergio Gonzalez Monroy wrote:
> Remove CONFIG_RTE_BUILD_COMBINE_LIBS and CONFIG_RTE_LIBNAME.
>
> Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> ---
>   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
> -
> -#
>   # Compile Environment Abstraction Layer
>   #
>   CONFIG_RTE_LIBRTE_EAL=y
> diff --git a/config/common_linuxapp b/config/common_linuxapp
> index 97f1c9e..ae13805 100644
> --- a/config/common_linuxapp
> +++ b/config/common_linuxapp
> @@ -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"
> -
> -#
>   # Compile Environment Abstraction Layer
>   #
>   CONFIG_RTE_LIBRTE_EAL=y
> diff --git a/config/defconfig_ppc_64-power8-linuxapp-gcc b/config/defconfig_ppc_64-power8-linuxapp-gcc
> index d97a885..f1af518 100644
> --- a/config/defconfig_ppc_64-power8-linuxapp-gcc
> +++ b/config/defconfig_ppc_64-power8-linuxapp-gcc
> @@ -39,8 +39,6 @@ CONFIG_RTE_ARCH_64=y
>   CONFIG_RTE_TOOLCHAIN="gcc"
>   CONFIG_RTE_TOOLCHAIN_GCC=y
>   
> -CONFIG_RTE_LIBNAME="powerpc_dpdk"
> -
>   # Note: Power doesn't have this support
>   CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
>   
> diff --git a/lib/Makefile b/lib/Makefile
> index d94355d..c34cf2f 100644
> --- a/lib/Makefile
> +++ b/lib/Makefile
> @@ -77,5 +77,4 @@ DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
>   DIRS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += librte_ivshmem
>   endif
>   
> -include $(RTE_SDK)/mk/rte.sharelib.mk
>   include $(RTE_SDK)/mk/rte.subdir.mk
> diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> index 63a41e2..e2baa49 100644
> --- a/mk/rte.app.mk
> +++ b/mk/rte.app.mk
> @@ -61,12 +61,6 @@ ifeq ($(NO_AUTOLIBS),)
>   
>   LDLIBS += --whole-archive
>   
> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
> -LDLIBS += -l$(RTE_LIBNAME)
> -endif
> -
> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
> -
>   ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y)
>   LDLIBS += -lrte_distributor
>   endif
> @@ -137,8 +131,6 @@ ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y)
>   LDLIBS += -lrte_vhost
>   endif
>   
> -endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
> -
>   ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
>   LDLIBS += -lpcap
>   endif
> @@ -153,8 +145,6 @@ endif
>   
>   LDLIBS += --start-group
>   
> -ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
> -
>   ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
>   LDLIBS += -lrte_kvargs
>   endif
> @@ -253,8 +243,6 @@ endif
>   
>   endif # plugins
>   
> -endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
> -
>   LDLIBS += $(EXECENV_LDLIBS)
>   
>   LDLIBS += --end-group
> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
> index 0d7482d..d96101a 100644
> --- a/mk/rte.lib.mk
> +++ b/mk/rte.lib.mk
> @@ -87,24 +87,6 @@ O_TO_S_DO = @set -e; \
>   	$(O_TO_S) && \
>   	echo $(O_TO_S_CMD) > $(call exe2cmd,$(@))
>   
> -ifeq ($(RTE_BUILD_SHARED_LIB),n)
> -O_TO_C = $(AR) crus $(LIB_ONE) $(OBJS-y)
> -O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
> -O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  AR_C $(@)")
> -O_TO_C_DO = @set -e; \
> -	$(lib_dir) \
> -	$(copy_obj)
> -else
> -O_TO_C = $(LD) -shared $(OBJS-y) -o $(LIB_ONE)
> -O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
> -O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  LD_C $(@)")
> -O_TO_C_DO = @set -e; \
> -	$(lib_dir) \
> -	$(copy_obj)
> -endif
> -
> -copy_obj = cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib;
> -lib_dir = [ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib;
>   -include .$(LIB).cmd
>   
>   #
> @@ -129,15 +111,6 @@ endif
>   		$(depfile_missing),\
>   		$(depfile_newer)),\
>   		$(O_TO_S_DO))
> -
> -ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> -	$(if $(or \
> -        $(file_missing),\
> -        $(call cmdline_changed,$(O_TO_C_STR)),\
> -        $(depfile_missing),\
> -        $(depfile_newer)),\
> -        $(O_TO_C_DO))
> -endif
>   else
>   $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
>   	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
> @@ -153,14 +126,6 @@ $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
>   	    $(depfile_missing),\
>   	    $(depfile_newer)),\
>   	    $(O_TO_A_DO))
> -ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> -	$(if $(or \
> -        $(file_missing),\
> -        $(call cmdline_changed,$(O_TO_C_STR)),\
> -        $(depfile_missing),\
> -        $(depfile_newer)),\
> -        $(O_TO_C_DO))
> -endif
>   endif
>   
>   #
> diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
> index 3154457..2b24e74 100644
> --- a/mk/rte.sdkbuild.mk
> +++ b/mk/rte.sdkbuild.mk
> @@ -93,9 +93,6 @@ $(ROOTDIRS-y):
>   	@[ -d $(BUILDDIR)/$@ ] || mkdir -p $(BUILDDIR)/$@
>   	@echo "== Build $@"
>   	$(Q)$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile -C $(BUILDDIR)/$@ all
> -	@if [ $@ = lib -a $(RTE_BUILD_COMBINE_LIBS) = y ]; then \
> -		$(MAKE) -f $(RTE_SDK)/lib/Makefile sharelib; \
> -	fi
>   
>   %_clean:
>   	@echo "== Clean $*"
> diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
> deleted file mode 100644
> index de53558..0000000
> --- a/mk/rte.sharelib.mk
> +++ /dev/null
> @@ -1,101 +0,0 @@
> -#   BSD LICENSE
> -#
> -#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> -#   All rights reserved.
> -#
> -#   Redistribution and use in source and binary forms, with or without
> -#   modification, are permitted provided that the following conditions
> -#   are met:
> -#
> -#     * Redistributions of source code must retain the above copyright
> -#       notice, this list of conditions and the following disclaimer.
> -#     * Redistributions in binary form must reproduce the above copyright
> -#       notice, this list of conditions and the following disclaimer in
> -#       the documentation and/or other materials provided with the
> -#       distribution.
> -#     * Neither the name of Intel Corporation nor the names of its
> -#       contributors may be used to endorse or promote products derived
> -#       from this software without specific prior written permission.
> -#
> -#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> -#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> -#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> -#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> -#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> -#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> -#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> -#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> -#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> -#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> -#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> -
> -include $(RTE_SDK)/mk/internal/rte.build-pre.mk
> -
> -# VPATH contains at least SRCDIR
> -VPATH += $(SRCDIR)
> -
> -ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> -ifeq ($(RTE_BUILD_SHARED_LIB),y)
> -LIB_ONE := lib$(RTE_LIBNAME).so
> -else
> -LIB_ONE := lib$(RTE_LIBNAME).a
> -endif
> -endif
> -
> -.PHONY:sharelib
> -sharelib: $(LIB_ONE) FORCE
> -
> -OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
> -
> -ifeq ($(LINK_USING_CC),1)
> -# Override the definition of LD here, since we're linking with CC
> -LD := $(CC) $(CPU_CFLAGS)
> -O_TO_S = $(LD) $(call linkerprefix,$(CPU_LDFLAGS)) \
> -	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
> -else
> -O_TO_S = $(LD) $(CPU_LDFLAGS) \
> -	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
> -endif
> -
> -O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
> -O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
> -O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)"
> -O_TO_S_DO = @set -e; \
> -    echo $(O_TO_S_DISP); \
> -    $(O_TO_S)
> -
> -O_TO_A =  $(AR) crus $(RTE_OUTPUT)/lib/$(LIB_ONE) $(OBJS)
> -O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
> -O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  LD $(@)")
> -O_TO_A_CMD = "cmd_$@ = $(O_TO_A_STR)"
> -O_TO_A_DO = @set -e; \
> -    echo $(O_TO_A_DISP); \
> -    $(O_TO_A)
> -#
> -# Archive objects to share library
> -#
> -
> -ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> -ifeq ($(RTE_BUILD_SHARED_LIB),y)
> -$(LIB_ONE): FORCE
> -	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
> -	$(O_TO_S_DO)
> -else
> -$(LIB_ONE): FORCE
> -	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
> -	$(O_TO_A_DO)
> -endif
> -endif
> -
> -#
> -# Clean all generated files
> -#
> -.PHONY: clean
> -clean: _postclean
> -
> -.PHONY: doclean
> -doclean:
> -	$(Q)rm -rf $(LIB_ONE)
> -
> -.PHONY: FORCE
> -FORCE:
> diff --git a/mk/rte.vars.mk b/mk/rte.vars.mk
> index d2f01b6..7b6f53d 100644
> --- a/mk/rte.vars.mk
> +++ b/mk/rte.vars.mk
> @@ -67,15 +67,6 @@ ifneq ($(BUILDING_RTE_SDK),)
>     ifeq ($(RTE_BUILD_SHARED_LIB),)
>       RTE_BUILD_SHARED_LIB := n
>     endif
> -  RTE_BUILD_COMBINE_LIBS := $(CONFIG_RTE_BUILD_COMBINE_LIBS:"%"=%)
> -  ifeq ($(RTE_BUILD_COMBINE_LIBS),)
> -    RTE_BUILD_COMBINE_LIBS := n
> -  endif
> -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.
Given that the patch to remove combined libraries is not welcome, I'll 
try to explain the current situation so we can agree on the way forward.

Currently we have build config option for shared libraries and combined 
libraries. Thus, this results in four possible combinations when 
building dpdk:
- not combined static
- not combined shared
- combined static
- combined shared

The makefile rules/targets for combined are different than for not 
combined. Thus, we currently have two different files for 
archive/linking (rte.lib.mk and rte.sharelib.mk).

Since having versioning, combined shared libraries build will be broken 
the moment we add a versioned API, as we do not have a global version 
map that we use when linking such library.
Also in my opinion, we would want to prevent users linking against a 
combined libdpdk.so that may have different features built-in, with the 
corresponding debugging difficulties when users
report different problems/errors. I think this would defeat many of the 
advantages of using shared libraries.

By removing the combined library build option, we would simplify the 
build system with only two possible choices:
- static
- shared

This would allow us to remove one file (rte.sharelib.mk) and have a 
single file with archive/linking rules.

For the convenience of linking against a single library instead of the 
multiple dpdk libraries, there are a few ways to go around it:
  - for combined static lib, we can either have a script to re-archive 
all libraries into a single/combined library (ie. extract all archives 
into one directory, the re-archive all objects into a combined library),
   or use a linker script (ie. GROUP ( -lrte_eal -lrte_malloc ... ) ).
- for combined shared lib, we can use a linker script (ie. INPUT ( 
-lrte_eal -lrte_malloc ... AS_NEEDED -lrte_hash ...) ) or we could use a 
global version map (either somehow merging all independent version maps
or maintaining a global version map).

My preference would be to remove the combined libs as a build config 
option, then either add scripts to create those linker scripts or 
document it so users know how to create their own linker scripts.
This would simplify the build process and still be able to provide the 
convenience of the combined library by using a linker script.

Comments?

Sergio
  
Thomas Monjalon March 18, 2015, 12:59 p.m. UTC | #16
Hi Sergio,

Thank you for explaining the situation.

2015-03-18 12:11, Gonzalez Monroy, Sergio:
> Given that the patch to remove combined libraries is not welcome, I'll 
> try to explain the current situation so we can agree on the way forward.
> 
> Currently we have build config option for shared libraries and combined 
> libraries. Thus, this results in four possible combinations when 
> building dpdk:
> - not combined static
> - not combined shared
> - combined static
> - combined shared
> 
> The makefile rules/targets for combined are different than for not 
> combined. Thus, we currently have two different files for 
> archive/linking (rte.lib.mk and rte.sharelib.mk).
> 
> Since having versioning, combined shared libraries build will be broken 
> the moment we add a versioned API, as we do not have a global version 
> map that we use when linking such library.
> Also in my opinion, we would want to prevent users linking against a 
> combined libdpdk.so that may have different features built-in, with the 
> corresponding debugging difficulties when users
> report different problems/errors. I think this would defeat many of the 
> advantages of using shared libraries.
> 
> By removing the combined library build option, we would simplify the 
> build system with only two possible choices:
> - static
> - shared

+1
I believe that simplification is the way go.

> This would allow us to remove one file (rte.sharelib.mk) and have a 
> single file with archive/linking rules.
> 
> For the convenience of linking against a single library instead of the 
> multiple dpdk libraries, there are a few ways to go around it:
>   - for combined static lib, we can either have a script to re-archive 
> all libraries into a single/combined library (ie. extract all archives 
> into one directory, the re-archive all objects into a combined library),
>    or use a linker script (ie. GROUP ( -lrte_eal -lrte_malloc ... ) ).
> - for combined shared lib, we can use a linker script (ie. INPUT ( 
> -lrte_eal -lrte_malloc ... AS_NEEDED -lrte_hash ...) ) or we could use a 
> global version map (either somehow merging all independent version maps
> or maintaining a global version map).
> 
> My preference would be to remove the combined libs as a build config 
> option, then either add scripts to create those linker scripts or 
> document it so users know how to create their own linker scripts.
> This would simplify the build process and still be able to provide the 
> convenience of the combined library by using a linker script.
> 
> Comments?

You're right about the word convenience.
There are many ways to provide such convenience.
The first one is to simply use the DPDK makefiles which abstract linking problems.
If using DPDK framework is not an option, we can add new conveniences like
scripts or pkgconfig support.
  
Stefan Puiu March 18, 2015, 3:30 p.m. UTC | #17
Hi,

On Wed, Mar 18, 2015 at 2:59 PM, Thomas Monjalon
<thomas.monjalon@6wind.com> wrote:
> Hi Sergio,
>
> Thank you for explaining the situation.
>
> 2015-03-18 12:11, Gonzalez Monroy, Sergio:
>> Given that the patch to remove combined libraries is not welcome, I'll
>> try to explain the current situation so we can agree on the way forward.
>>
>> Currently we have build config option for shared libraries and combined
>> libraries. Thus, this results in four possible combinations when
>> building dpdk:
>> - not combined static
>> - not combined shared
>> - combined static
>> - combined shared
>>
>> The makefile rules/targets for combined are different than for not
>> combined. Thus, we currently have two different files for
>> archive/linking (rte.lib.mk and rte.sharelib.mk).
>>
>> Since having versioning, combined shared libraries build will be broken
>> the moment we add a versioned API, as we do not have a global version
>> map that we use when linking such library.
>> Also in my opinion, we would want to prevent users linking against a
>> combined libdpdk.so that may have different features built-in, with the
>> corresponding debugging difficulties when users
>> report different problems/errors. I think this would defeat many of the
>> advantages of using shared libraries.
>>
>> By removing the combined library build option, we would simplify the
>> build system with only two possible choices:
>> - static
>> - shared
>
> +1
> I believe that simplification is the way go.
>
>> This would allow us to remove one file (rte.sharelib.mk) and have a
>> single file with archive/linking rules.
>>
>> For the convenience of linking against a single library instead of the
>> multiple dpdk libraries, there are a few ways to go around it:
>>   - for combined static lib, we can either have a script to re-archive
>> all libraries into a single/combined library (ie. extract all archives
>> into one directory, the re-archive all objects into a combined library),
>>    or use a linker script (ie. GROUP ( -lrte_eal -lrte_malloc ... ) ).

Would the linker script be provided in the repository or would it be
the responsibility of people building against the DPDK? If I'd need to
make a linker script with the list of libraries to link against, might
as well put that list in my SConstruct / Makefile and be done with it.
So the "write your own linker script" and "just deal with separate
libraries" options don't seem that different to me.

Let me ask you something - I understand your concerns about
simplifying Makefiles and the concerns about versioning. How
significant is the "separate libs" use case? And especially the
"separate libs" in the current division of the code / libraries? I
counted about ~30 libs in 1.8.0 under build/lib. Are there people
using librte_eal without rte_malloc? Or rte_malloc without
rte_mempool?

I noticed that some examples I built ended up using --whole-archive
-lrte_eal -lrte_etc.... To me, --whole-archive is one way of saying
"we have lots of libraries with obscure dependencies", maybe reducing
the number of libs might also be a way to make the combined lib
unnecessary? I wouldn't bother with the combined lib if I had 3-4 libs
to link against instead of the number of libs needed now.

Just asking - obviously you guys are maintaining the code and know
best, but I want to better understand the context from your side, as
opposed to my (selfish) user perspective :).

Thanks,
Stefan.


>> - for combined shared lib, we can use a linker script (ie. INPUT (
>> -lrte_eal -lrte_malloc ... AS_NEEDED -lrte_hash ...) ) or we could use a
>> global version map (either somehow merging all independent version maps
>> or maintaining a global version map).
>>
>> My preference would be to remove the combined libs as a build config
>> option, then either add scripts to create those linker scripts or
>> document it so users know how to create their own linker scripts.
>> This would simplify the build process and still be able to provide the
>> convenience of the combined library by using a linker script.
>>
>> Comments?
>
> You're right about the word convenience.
> There are many ways to provide such convenience.
> The first one is to simply use the DPDK makefiles which abstract linking problems.
> If using DPDK framework is not an option, we can add new conveniences like
> scripts or pkgconfig support.
>
  
Sergio Gonzalez Monroy March 18, 2015, 3:52 p.m. UTC | #18
On 18/03/2015 15:30, Stefan Puiu wrote:
> Hi,
>
> On Wed, Mar 18, 2015 at 2:59 PM, Thomas Monjalon
> <thomas.monjalon@6wind.com> wrote:
>> Hi Sergio,
>>
>> Thank you for explaining the situation.
>>
>> 2015-03-18 12:11, Gonzalez Monroy, Sergio:
>>> Given that the patch to remove combined libraries is not welcome, I'll
>>> try to explain the current situation so we can agree on the way forward.
>>>
>>> Currently we have build config option for shared libraries and combined
>>> libraries. Thus, this results in four possible combinations when
>>> building dpdk:
>>> - not combined static
>>> - not combined shared
>>> - combined static
>>> - combined shared
>>>
>>> The makefile rules/targets for combined are different than for not
>>> combined. Thus, we currently have two different files for
>>> archive/linking (rte.lib.mk and rte.sharelib.mk).
>>>
>>> Since having versioning, combined shared libraries build will be broken
>>> the moment we add a versioned API, as we do not have a global version
>>> map that we use when linking such library.
>>> Also in my opinion, we would want to prevent users linking against a
>>> combined libdpdk.so that may have different features built-in, with the
>>> corresponding debugging difficulties when users
>>> report different problems/errors. I think this would defeat many of the
>>> advantages of using shared libraries.
>>>
>>> By removing the combined library build option, we would simplify the
>>> build system with only two possible choices:
>>> - static
>>> - shared
>> +1
>> I believe that simplification is the way go.
>>
>>> This would allow us to remove one file (rte.sharelib.mk) and have a
>>> single file with archive/linking rules.
>>>
>>> For the convenience of linking against a single library instead of the
>>> multiple dpdk libraries, there are a few ways to go around it:
>>>    - for combined static lib, we can either have a script to re-archive
>>> all libraries into a single/combined library (ie. extract all archives
>>> into one directory, the re-archive all objects into a combined library),
>>>     or use a linker script (ie. GROUP ( -lrte_eal -lrte_malloc ... ) ).
> Would the linker script be provided in the repository or would it be
> the responsibility of people building against the DPDK? If I'd need to
> make a linker script with the list of libraries to link against, might
> as well put that list in my SConstruct / Makefile and be done with it.
> So the "write your own linker script" and "just deal with separate
> libraries" options don't seem that different to me.
>
> Let me ask you something - I understand your concerns about
> simplifying Makefiles and the concerns about versioning. How
> significant is the "separate libs" use case? And especially the
> "separate libs" in the current division of the code / libraries? I
> counted about ~30 libs in 1.8.0 under build/lib. Are there people
> using librte_eal without rte_malloc? Or rte_malloc without
> rte_mempool?
>
> I noticed that some examples I built ended up using --whole-archive
> -lrte_eal -lrte_etc.... To me, --whole-archive is one way of saying
> "we have lots of libraries with obscure dependencies", maybe reducing
> the number of libs might also be a way to make the combined lib
> unnecessary? I wouldn't bother with the combined lib if I had 3-4 libs
> to link against instead of the number of libs needed now.
>
> Just asking - obviously you guys are maintaining the code and know
> best, but I want to better understand the context from your side, as
> opposed to my (selfish) user perspective :).
>
> Thanks,
> Stefan.
>
Some of this questions have been discussed previously:

http://dpdk.org/ml/archives/dev/2014-October/007389.html
http://dpdk.org/ml/archives/dev/2015-January/010917.html
http://dpdk.org/ml/archives/dev/2015-January/011912.html

I think those threads will provide enough context but as a very general 
summary, only eal, malloc, mempool and ring would have circular 
dependencies and you could consider them as 'core' libraries in the 
sense that almost (if not all) dpdk apps are going to be linked against 
them.
Most of dpdk libraries are optional features that your application may 
or may not be using.

Sergio
>>> - for combined shared lib, we can use a linker script (ie. INPUT (
>>> -lrte_eal -lrte_malloc ... AS_NEEDED -lrte_hash ...) ) or we could use a
>>> global version map (either somehow merging all independent version maps
>>> or maintaining a global version map).
>>>
>>> My preference would be to remove the combined libs as a build config
>>> option, then either add scripts to create those linker scripts or
>>> document it so users know how to create their own linker scripts.
>>> This would simplify the build process and still be able to provide the
>>> convenience of the combined library by using a linker script.
>>>
>>> Comments?
>> You're right about the word convenience.
>> There are many ways to provide such convenience.
>> The first one is to simply use the DPDK makefiles which abstract linking problems.
>> If using DPDK framework is not an option, we can add new conveniences like
>> scripts or pkgconfig support.
>>
  
Neil Horman March 18, 2015, 4:41 p.m. UTC | #19
On Wed, Mar 18, 2015 at 12:11:50PM +0000, Gonzalez Monroy, Sergio wrote:
> On 12/03/2015 16:27, Sergio Gonzalez Monroy wrote:
> >Remove CONFIG_RTE_BUILD_COMBINE_LIBS and CONFIG_RTE_LIBNAME.
> >
> >Signed-off-by: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
> >---
> >  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
> >-
> >-#
> >  # Compile Environment Abstraction Layer
> >  #
> >  CONFIG_RTE_LIBRTE_EAL=y
> >diff --git a/config/common_linuxapp b/config/common_linuxapp
> >index 97f1c9e..ae13805 100644
> >--- a/config/common_linuxapp
> >+++ b/config/common_linuxapp
> >@@ -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"
> >-
> >-#
> >  # Compile Environment Abstraction Layer
> >  #
> >  CONFIG_RTE_LIBRTE_EAL=y
> >diff --git a/config/defconfig_ppc_64-power8-linuxapp-gcc b/config/defconfig_ppc_64-power8-linuxapp-gcc
> >index d97a885..f1af518 100644
> >--- a/config/defconfig_ppc_64-power8-linuxapp-gcc
> >+++ b/config/defconfig_ppc_64-power8-linuxapp-gcc
> >@@ -39,8 +39,6 @@ CONFIG_RTE_ARCH_64=y
> >  CONFIG_RTE_TOOLCHAIN="gcc"
> >  CONFIG_RTE_TOOLCHAIN_GCC=y
> >-CONFIG_RTE_LIBNAME="powerpc_dpdk"
> >-
> >  # Note: Power doesn't have this support
> >  CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
> >diff --git a/lib/Makefile b/lib/Makefile
> >index d94355d..c34cf2f 100644
> >--- a/lib/Makefile
> >+++ b/lib/Makefile
> >@@ -77,5 +77,4 @@ DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
> >  DIRS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += librte_ivshmem
> >  endif
> >-include $(RTE_SDK)/mk/rte.sharelib.mk
> >  include $(RTE_SDK)/mk/rte.subdir.mk
> >diff --git a/mk/rte.app.mk b/mk/rte.app.mk
> >index 63a41e2..e2baa49 100644
> >--- a/mk/rte.app.mk
> >+++ b/mk/rte.app.mk
> >@@ -61,12 +61,6 @@ ifeq ($(NO_AUTOLIBS),)
> >  LDLIBS += --whole-archive
> >-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
> >-LDLIBS += -l$(RTE_LIBNAME)
> >-endif
> >-
> >-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
> >-
> >  ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y)
> >  LDLIBS += -lrte_distributor
> >  endif
> >@@ -137,8 +131,6 @@ ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y)
> >  LDLIBS += -lrte_vhost
> >  endif
> >-endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
> >-
> >  ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
> >  LDLIBS += -lpcap
> >  endif
> >@@ -153,8 +145,6 @@ endif
> >  LDLIBS += --start-group
> >-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
> >-
> >  ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
> >  LDLIBS += -lrte_kvargs
> >  endif
> >@@ -253,8 +243,6 @@ endif
> >  endif # plugins
> >-endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
> >-
> >  LDLIBS += $(EXECENV_LDLIBS)
> >  LDLIBS += --end-group
> >diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
> >index 0d7482d..d96101a 100644
> >--- a/mk/rte.lib.mk
> >+++ b/mk/rte.lib.mk
> >@@ -87,24 +87,6 @@ O_TO_S_DO = @set -e; \
> >  	$(O_TO_S) && \
> >  	echo $(O_TO_S_CMD) > $(call exe2cmd,$(@))
> >-ifeq ($(RTE_BUILD_SHARED_LIB),n)
> >-O_TO_C = $(AR) crus $(LIB_ONE) $(OBJS-y)
> >-O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
> >-O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  AR_C $(@)")
> >-O_TO_C_DO = @set -e; \
> >-	$(lib_dir) \
> >-	$(copy_obj)
> >-else
> >-O_TO_C = $(LD) -shared $(OBJS-y) -o $(LIB_ONE)
> >-O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
> >-O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  LD_C $(@)")
> >-O_TO_C_DO = @set -e; \
> >-	$(lib_dir) \
> >-	$(copy_obj)
> >-endif
> >-
> >-copy_obj = cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib;
> >-lib_dir = [ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib;
> >  -include .$(LIB).cmd
> >  #
> >@@ -129,15 +111,6 @@ endif
> >  		$(depfile_missing),\
> >  		$(depfile_newer)),\
> >  		$(O_TO_S_DO))
> >-
> >-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> >-	$(if $(or \
> >-        $(file_missing),\
> >-        $(call cmdline_changed,$(O_TO_C_STR)),\
> >-        $(depfile_missing),\
> >-        $(depfile_newer)),\
> >-        $(O_TO_C_DO))
> >-endif
> >  else
> >  $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
> >  	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
> >@@ -153,14 +126,6 @@ $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
> >  	    $(depfile_missing),\
> >  	    $(depfile_newer)),\
> >  	    $(O_TO_A_DO))
> >-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> >-	$(if $(or \
> >-        $(file_missing),\
> >-        $(call cmdline_changed,$(O_TO_C_STR)),\
> >-        $(depfile_missing),\
> >-        $(depfile_newer)),\
> >-        $(O_TO_C_DO))
> >-endif
> >  endif
> >  #
> >diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
> >index 3154457..2b24e74 100644
> >--- a/mk/rte.sdkbuild.mk
> >+++ b/mk/rte.sdkbuild.mk
> >@@ -93,9 +93,6 @@ $(ROOTDIRS-y):
> >  	@[ -d $(BUILDDIR)/$@ ] || mkdir -p $(BUILDDIR)/$@
> >  	@echo "== Build $@"
> >  	$(Q)$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile -C $(BUILDDIR)/$@ all
> >-	@if [ $@ = lib -a $(RTE_BUILD_COMBINE_LIBS) = y ]; then \
> >-		$(MAKE) -f $(RTE_SDK)/lib/Makefile sharelib; \
> >-	fi
> >  %_clean:
> >  	@echo "== Clean $*"
> >diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
> >deleted file mode 100644
> >index de53558..0000000
> >--- a/mk/rte.sharelib.mk
> >+++ /dev/null
> >@@ -1,101 +0,0 @@
> >-#   BSD LICENSE
> >-#
> >-#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
> >-#   All rights reserved.
> >-#
> >-#   Redistribution and use in source and binary forms, with or without
> >-#   modification, are permitted provided that the following conditions
> >-#   are met:
> >-#
> >-#     * Redistributions of source code must retain the above copyright
> >-#       notice, this list of conditions and the following disclaimer.
> >-#     * Redistributions in binary form must reproduce the above copyright
> >-#       notice, this list of conditions and the following disclaimer in
> >-#       the documentation and/or other materials provided with the
> >-#       distribution.
> >-#     * Neither the name of Intel Corporation nor the names of its
> >-#       contributors may be used to endorse or promote products derived
> >-#       from this software without specific prior written permission.
> >-#
> >-#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
> >-#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
> >-#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
> >-#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
> >-#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
> >-#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
> >-#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
> >-#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
> >-#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
> >-#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
> >-#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
> >-
> >-include $(RTE_SDK)/mk/internal/rte.build-pre.mk
> >-
> >-# VPATH contains at least SRCDIR
> >-VPATH += $(SRCDIR)
> >-
> >-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> >-ifeq ($(RTE_BUILD_SHARED_LIB),y)
> >-LIB_ONE := lib$(RTE_LIBNAME).so
> >-else
> >-LIB_ONE := lib$(RTE_LIBNAME).a
> >-endif
> >-endif
> >-
> >-.PHONY:sharelib
> >-sharelib: $(LIB_ONE) FORCE
> >-
> >-OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
> >-
> >-ifeq ($(LINK_USING_CC),1)
> >-# Override the definition of LD here, since we're linking with CC
> >-LD := $(CC) $(CPU_CFLAGS)
> >-O_TO_S = $(LD) $(call linkerprefix,$(CPU_LDFLAGS)) \
> >-	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
> >-else
> >-O_TO_S = $(LD) $(CPU_LDFLAGS) \
> >-	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
> >-endif
> >-
> >-O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
> >-O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
> >-O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)"
> >-O_TO_S_DO = @set -e; \
> >-    echo $(O_TO_S_DISP); \
> >-    $(O_TO_S)
> >-
> >-O_TO_A =  $(AR) crus $(RTE_OUTPUT)/lib/$(LIB_ONE) $(OBJS)
> >-O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
> >-O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  LD $(@)")
> >-O_TO_A_CMD = "cmd_$@ = $(O_TO_A_STR)"
> >-O_TO_A_DO = @set -e; \
> >-    echo $(O_TO_A_DISP); \
> >-    $(O_TO_A)
> >-#
> >-# Archive objects to share library
> >-#
> >-
> >-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
> >-ifeq ($(RTE_BUILD_SHARED_LIB),y)
> >-$(LIB_ONE): FORCE
> >-	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
> >-	$(O_TO_S_DO)
> >-else
> >-$(LIB_ONE): FORCE
> >-	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
> >-	$(O_TO_A_DO)
> >-endif
> >-endif
> >-
> >-#
> >-# Clean all generated files
> >-#
> >-.PHONY: clean
> >-clean: _postclean
> >-
> >-.PHONY: doclean
> >-doclean:
> >-	$(Q)rm -rf $(LIB_ONE)
> >-
> >-.PHONY: FORCE
> >-FORCE:
> >diff --git a/mk/rte.vars.mk b/mk/rte.vars.mk
> >index d2f01b6..7b6f53d 100644
> >--- a/mk/rte.vars.mk
> >+++ b/mk/rte.vars.mk
> >@@ -67,15 +67,6 @@ ifneq ($(BUILDING_RTE_SDK),)
> >    ifeq ($(RTE_BUILD_SHARED_LIB),)
> >      RTE_BUILD_SHARED_LIB := n
> >    endif
> >-  RTE_BUILD_COMBINE_LIBS := $(CONFIG_RTE_BUILD_COMBINE_LIBS:"%"=%)
> >-  ifeq ($(RTE_BUILD_COMBINE_LIBS),)
> >-    RTE_BUILD_COMBINE_LIBS := n
> >-  endif
> >-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.
> Given that the patch to remove combined libraries is not welcome, I'll try
> to explain the current situation so we can agree on the way forward.
> 
> Currently we have build config option for shared libraries and combined
> libraries. Thus, this results in four possible combinations when building
> dpdk:
> - not combined static
> - not combined shared
> - combined static
> - combined shared
> 
> The makefile rules/targets for combined are different than for not combined.
> Thus, we currently have two different files for archive/linking (rte.lib.mk
> and rte.sharelib.mk).
> 
> Since having versioning, combined shared libraries build will be broken the
> moment we add a versioned API, as we do not have a global version map that
> we use when linking such library.
> Also in my opinion, we would want to prevent users linking against a
> combined libdpdk.so that may have different features built-in, with the
> corresponding debugging difficulties when users
> report different problems/errors. I think this would defeat many of the
> advantages of using shared libraries.
> 
> By removing the combined library build option, we would simplify the build
> system with only two possible choices:
> - static
> - shared
> 
> This would allow us to remove one file (rte.sharelib.mk) and have a single
> file with archive/linking rules.
> 
> For the convenience of linking against a single library instead of the
> multiple dpdk libraries, there are a few ways to go around it:
>  - for combined static lib, we can either have a script to re-archive all
> libraries into a single/combined library (ie. extract all archives into one
> directory, the re-archive all objects into a combined library),
>   or use a linker script (ie. GROUP ( -lrte_eal -lrte_malloc ... ) ).
> - for combined shared lib, we can use a linker script (ie. INPUT ( -lrte_eal
> -lrte_malloc ... AS_NEEDED -lrte_hash ...) ) or we could use a global
> version map (either somehow merging all independent version maps
> or maintaining a global version map).
> 
> My preference would be to remove the combined libs as a build config option,
> then either add scripts to create those linker scripts or document it so
> users know how to create their own linker scripts.
> This would simplify the build process and still be able to provide the
> convenience of the combined library by using a linker script.
> 
> Comments?
> 
I agree, the combined libs config option creates significant build environment
complications that don't need to be there (given that in both the static and
shared cases, post processing gives us the same convieniences).

The comments on this thread have suggested that the combined libraries are
mostly seen as adventageous for their convienience, so if we can provide that
convienience in another way that solves other problems we have (versioning of a
combined library), and reduce build complexity, we should do it.

Neil

> Sergio
>
  
Neil Horman March 18, 2015, 4:48 p.m. UTC | #20
On Wed, Mar 18, 2015 at 05:30:12PM +0200, Stefan Puiu wrote:
> Hi,
> 
> On Wed, Mar 18, 2015 at 2:59 PM, Thomas Monjalon
> <thomas.monjalon@6wind.com> wrote:
> > Hi Sergio,
> >
> > Thank you for explaining the situation.
> >
> > 2015-03-18 12:11, Gonzalez Monroy, Sergio:
> >> Given that the patch to remove combined libraries is not welcome, I'll
> >> try to explain the current situation so we can agree on the way forward.
> >>
> >> Currently we have build config option for shared libraries and combined
> >> libraries. Thus, this results in four possible combinations when
> >> building dpdk:
> >> - not combined static
> >> - not combined shared
> >> - combined static
> >> - combined shared
> >>
> >> The makefile rules/targets for combined are different than for not
> >> combined. Thus, we currently have two different files for
> >> archive/linking (rte.lib.mk and rte.sharelib.mk).
> >>
> >> Since having versioning, combined shared libraries build will be broken
> >> the moment we add a versioned API, as we do not have a global version
> >> map that we use when linking such library.
> >> Also in my opinion, we would want to prevent users linking against a
> >> combined libdpdk.so that may have different features built-in, with the
> >> corresponding debugging difficulties when users
> >> report different problems/errors. I think this would defeat many of the
> >> advantages of using shared libraries.
> >>
> >> By removing the combined library build option, we would simplify the
> >> build system with only two possible choices:
> >> - static
> >> - shared
> >
> > +1
> > I believe that simplification is the way go.
> >
> >> This would allow us to remove one file (rte.sharelib.mk) and have a
> >> single file with archive/linking rules.
> >>
> >> For the convenience of linking against a single library instead of the
> >> multiple dpdk libraries, there are a few ways to go around it:
> >>   - for combined static lib, we can either have a script to re-archive
> >> all libraries into a single/combined library (ie. extract all archives
> >> into one directory, the re-archive all objects into a combined library),
> >>    or use a linker script (ie. GROUP ( -lrte_eal -lrte_malloc ... ) ).
> 
> Would the linker script be provided in the repository or would it be
> the responsibility of people building against the DPDK? If I'd need to
> make a linker script with the list of libraries to link against, might
> as well put that list in my SConstruct / Makefile and be done with it.
> So the "write your own linker script" and "just deal with separate
> libraries" options don't seem that different to me.
> 
just to level set, I think you're thinking of a linker script in too grand a
scale.  Technically what we're proposing is a linker script, but its literally a
single line.  If you want an example take a look at /usr/lib64/libc.so.

that said, I think it makes more sense for the linker script in question to be
part of the dpdk distribution so that the combined library name picks up new
libraries as they are created.


> Let me ask you something - I understand your concerns about
> simplifying Makefiles and the concerns about versioning. How
> significant is the "separate libs" use case? And especially the
> "separate libs" in the current division of the code / libraries? I
> counted about ~30 libs in 1.8.0 under build/lib. Are there people
> using librte_eal without rte_malloc? Or rte_malloc without
> rte_mempool?
> 

Highly doubtful/impossible since they are explicitly dependent on one another.

> I noticed that some examples I built ended up using --whole-archive
> -lrte_eal -lrte_etc.... To me, --whole-archive is one way of saying
> "we have lots of libraries with obscure dependencies", maybe reducing
> the number of libs might also be a way to make the combined lib
> unnecessary? I wouldn't bother with the combined lib if I had 3-4 libs
> to link against instead of the number of libs needed now.
> 
This isn't a bad suggestion.  combining the low level malloc/mempool/eal
libraries into a libdpdk_core probably makes sense.  Not sure if doing it right
now makes sense (this close to the release).  But as a next release goal that
seems reasonable.

Neil
  
Sergio Gonzalez Monroy March 26, 2015, 8:52 a.m. UTC | #21
On 18/03/2015 12:59, Thomas Monjalon wrote:
> Hi Sergio,
>
> Thank you for explaining the situation.
>
> 2015-03-18 12:11, Gonzalez Monroy, Sergio:
>> Given that the patch to remove combined libraries is not welcome, I'll
>> try to explain the current situation so we can agree on the way forward.
>>
>> Currently we have build config option for shared libraries and combined
>> libraries. Thus, this results in four possible combinations when
>> building dpdk:
>> - not combined static
>> - not combined shared
>> - combined static
>> - combined shared
>>
>> The makefile rules/targets for combined are different than for not
>> combined. Thus, we currently have two different files for
>> archive/linking (rte.lib.mk and rte.sharelib.mk).
>>
>> Since having versioning, combined shared libraries build will be broken
>> the moment we add a versioned API, as we do not have a global version
>> map that we use when linking such library.
>> Also in my opinion, we would want to prevent users linking against a
>> combined libdpdk.so that may have different features built-in, with the
>> corresponding debugging difficulties when users
>> report different problems/errors. I think this would defeat many of the
>> advantages of using shared libraries.
>>
>> By removing the combined library build option, we would simplify the
>> build system with only two possible choices:
>> - static
>> - shared
> +1
> I believe that simplification is the way go.
>
>> This would allow us to remove one file (rte.sharelib.mk) and have a
>> single file with archive/linking rules.
>>
>> For the convenience of linking against a single library instead of the
>> multiple dpdk libraries, there are a few ways to go around it:
>>    - for combined static lib, we can either have a script to re-archive
>> all libraries into a single/combined library (ie. extract all archives
>> into one directory, the re-archive all objects into a combined library),
>>     or use a linker script (ie. GROUP ( -lrte_eal -lrte_malloc ... ) ).
>> - for combined shared lib, we can use a linker script (ie. INPUT (
>> -lrte_eal -lrte_malloc ... AS_NEEDED -lrte_hash ...) ) or we could use a
>> global version map (either somehow merging all independent version maps
>> or maintaining a global version map).
>>
>> My preference would be to remove the combined libs as a build config
>> option, then either add scripts to create those linker scripts or
>> document it so users know how to create their own linker scripts.
>> This would simplify the build process and still be able to provide the
>> convenience of the combined library by using a linker script.
>>
>> Comments?
> You're right about the word convenience.
> There are many ways to provide such convenience.
> The first one is to simply use the DPDK makefiles which abstract linking problems.
> If using DPDK framework is not an option, we can add new conveniences like
> scripts or pkgconfig support.
>
So for v3, do we provide such script/pkgconfig or just documentation on 
how to create it?

Sergio
  
Neil Horman March 26, 2015, 10:30 a.m. UTC | #22
On Thu, Mar 26, 2015 at 08:52:29AM +0000, Gonzalez Monroy, Sergio wrote:
> On 18/03/2015 12:59, Thomas Monjalon wrote:
> >Hi Sergio,
> >
> >Thank you for explaining the situation.
> >
> >2015-03-18 12:11, Gonzalez Monroy, Sergio:
> >>Given that the patch to remove combined libraries is not welcome, I'll
> >>try to explain the current situation so we can agree on the way forward.
> >>
> >>Currently we have build config option for shared libraries and combined
> >>libraries. Thus, this results in four possible combinations when
> >>building dpdk:
> >>- not combined static
> >>- not combined shared
> >>- combined static
> >>- combined shared
> >>
> >>The makefile rules/targets for combined are different than for not
> >>combined. Thus, we currently have two different files for
> >>archive/linking (rte.lib.mk and rte.sharelib.mk).
> >>
> >>Since having versioning, combined shared libraries build will be broken
> >>the moment we add a versioned API, as we do not have a global version
> >>map that we use when linking such library.
> >>Also in my opinion, we would want to prevent users linking against a
> >>combined libdpdk.so that may have different features built-in, with the
> >>corresponding debugging difficulties when users
> >>report different problems/errors. I think this would defeat many of the
> >>advantages of using shared libraries.
> >>
> >>By removing the combined library build option, we would simplify the
> >>build system with only two possible choices:
> >>- static
> >>- shared
> >+1
> >I believe that simplification is the way go.
> >
> >>This would allow us to remove one file (rte.sharelib.mk) and have a
> >>single file with archive/linking rules.
> >>
> >>For the convenience of linking against a single library instead of the
> >>multiple dpdk libraries, there are a few ways to go around it:
> >>   - for combined static lib, we can either have a script to re-archive
> >>all libraries into a single/combined library (ie. extract all archives
> >>into one directory, the re-archive all objects into a combined library),
> >>    or use a linker script (ie. GROUP ( -lrte_eal -lrte_malloc ... ) ).
> >>- for combined shared lib, we can use a linker script (ie. INPUT (
> >>-lrte_eal -lrte_malloc ... AS_NEEDED -lrte_hash ...) ) or we could use a
> >>global version map (either somehow merging all independent version maps
> >>or maintaining a global version map).
> >>
> >>My preference would be to remove the combined libs as a build config
> >>option, then either add scripts to create those linker scripts or
> >>document it so users know how to create their own linker scripts.
> >>This would simplify the build process and still be able to provide the
> >>convenience of the combined library by using a linker script.
> >>
> >>Comments?
> >You're right about the word convenience.
> >There are many ways to provide such convenience.
> >The first one is to simply use the DPDK makefiles which abstract linking problems.
> >If using DPDK framework is not an option, we can add new conveniences like
> >scripts or pkgconfig support.
> >
> So for v3, do we provide such script/pkgconfig or just documentation on how
> to create it?
> 
> Sergio
> 
I think the former makes the most sense, personally.  Provide a script or
automatically build the combined lib script during the build.
Neil
  

Patch

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
-
-#
 # Compile Environment Abstraction Layer
 #
 CONFIG_RTE_LIBRTE_EAL=y
diff --git a/config/common_linuxapp b/config/common_linuxapp
index 97f1c9e..ae13805 100644
--- a/config/common_linuxapp
+++ b/config/common_linuxapp
@@ -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"
-
-#
 # Compile Environment Abstraction Layer
 #
 CONFIG_RTE_LIBRTE_EAL=y
diff --git a/config/defconfig_ppc_64-power8-linuxapp-gcc b/config/defconfig_ppc_64-power8-linuxapp-gcc
index d97a885..f1af518 100644
--- a/config/defconfig_ppc_64-power8-linuxapp-gcc
+++ b/config/defconfig_ppc_64-power8-linuxapp-gcc
@@ -39,8 +39,6 @@  CONFIG_RTE_ARCH_64=y
 CONFIG_RTE_TOOLCHAIN="gcc"
 CONFIG_RTE_TOOLCHAIN_GCC=y
 
-CONFIG_RTE_LIBNAME="powerpc_dpdk"
-
 # Note: Power doesn't have this support
 CONFIG_RTE_LIBRTE_EAL_VMWARE_TSC_MAP_SUPPORT=n
 
diff --git a/lib/Makefile b/lib/Makefile
index d94355d..c34cf2f 100644
--- a/lib/Makefile
+++ b/lib/Makefile
@@ -77,5 +77,4 @@  DIRS-$(CONFIG_RTE_LIBRTE_KNI) += librte_kni
 DIRS-$(CONFIG_RTE_LIBRTE_IVSHMEM) += librte_ivshmem
 endif
 
-include $(RTE_SDK)/mk/rte.sharelib.mk
 include $(RTE_SDK)/mk/rte.subdir.mk
diff --git a/mk/rte.app.mk b/mk/rte.app.mk
index 63a41e2..e2baa49 100644
--- a/mk/rte.app.mk
+++ b/mk/rte.app.mk
@@ -61,12 +61,6 @@  ifeq ($(NO_AUTOLIBS),)
 
 LDLIBS += --whole-archive
 
-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),y)
-LDLIBS += -l$(RTE_LIBNAME)
-endif
-
-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
-
 ifeq ($(CONFIG_RTE_LIBRTE_DISTRIBUTOR),y)
 LDLIBS += -lrte_distributor
 endif
@@ -137,8 +131,6 @@  ifeq ($(CONFIG_RTE_LIBRTE_VHOST), y)
 LDLIBS += -lrte_vhost
 endif
 
-endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
-
 ifeq ($(CONFIG_RTE_LIBRTE_PMD_PCAP),y)
 LDLIBS += -lpcap
 endif
@@ -153,8 +145,6 @@  endif
 
 LDLIBS += --start-group
 
-ifeq ($(CONFIG_RTE_BUILD_COMBINE_LIBS),n)
-
 ifeq ($(CONFIG_RTE_LIBRTE_KVARGS),y)
 LDLIBS += -lrte_kvargs
 endif
@@ -253,8 +243,6 @@  endif
 
 endif # plugins
 
-endif # ! CONFIG_RTE_BUILD_COMBINE_LIBS
-
 LDLIBS += $(EXECENV_LDLIBS)
 
 LDLIBS += --end-group
diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk
index 0d7482d..d96101a 100644
--- a/mk/rte.lib.mk
+++ b/mk/rte.lib.mk
@@ -87,24 +87,6 @@  O_TO_S_DO = @set -e; \
 	$(O_TO_S) && \
 	echo $(O_TO_S_CMD) > $(call exe2cmd,$(@))
 
-ifeq ($(RTE_BUILD_SHARED_LIB),n)
-O_TO_C = $(AR) crus $(LIB_ONE) $(OBJS-y)
-O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
-O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  AR_C $(@)")
-O_TO_C_DO = @set -e; \
-	$(lib_dir) \
-	$(copy_obj)
-else
-O_TO_C = $(LD) -shared $(OBJS-y) -o $(LIB_ONE)
-O_TO_C_STR = $(subst ','\'',$(O_TO_C)) #'# fix syntax highlight
-O_TO_C_DISP = $(if $(V),"$(O_TO_C_STR)","  LD_C $(@)")
-O_TO_C_DO = @set -e; \
-	$(lib_dir) \
-	$(copy_obj)
-endif
-
-copy_obj = cp -f $(OBJS-y) $(RTE_OUTPUT)/build/lib;
-lib_dir = [ -d $(RTE_OUTPUT)/lib ] || mkdir -p $(RTE_OUTPUT)/lib;
 -include .$(LIB).cmd
 
 #
@@ -129,15 +111,6 @@  endif
 		$(depfile_missing),\
 		$(depfile_newer)),\
 		$(O_TO_S_DO))
-
-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
-	$(if $(or \
-        $(file_missing),\
-        $(call cmdline_changed,$(O_TO_C_STR)),\
-        $(depfile_missing),\
-        $(depfile_newer)),\
-        $(O_TO_C_DO))
-endif
 else
 $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
 	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
@@ -153,14 +126,6 @@  $(LIB): $(OBJS-y) $(DEP_$(LIB)) FORCE
 	    $(depfile_missing),\
 	    $(depfile_newer)),\
 	    $(O_TO_A_DO))
-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
-	$(if $(or \
-        $(file_missing),\
-        $(call cmdline_changed,$(O_TO_C_STR)),\
-        $(depfile_missing),\
-        $(depfile_newer)),\
-        $(O_TO_C_DO))
-endif
 endif
 
 #
diff --git a/mk/rte.sdkbuild.mk b/mk/rte.sdkbuild.mk
index 3154457..2b24e74 100644
--- a/mk/rte.sdkbuild.mk
+++ b/mk/rte.sdkbuild.mk
@@ -93,9 +93,6 @@  $(ROOTDIRS-y):
 	@[ -d $(BUILDDIR)/$@ ] || mkdir -p $(BUILDDIR)/$@
 	@echo "== Build $@"
 	$(Q)$(MAKE) S=$@ -f $(RTE_SRCDIR)/$@/Makefile -C $(BUILDDIR)/$@ all
-	@if [ $@ = lib -a $(RTE_BUILD_COMBINE_LIBS) = y ]; then \
-		$(MAKE) -f $(RTE_SDK)/lib/Makefile sharelib; \
-	fi
 
 %_clean:
 	@echo "== Clean $*"
diff --git a/mk/rte.sharelib.mk b/mk/rte.sharelib.mk
deleted file mode 100644
index de53558..0000000
--- a/mk/rte.sharelib.mk
+++ /dev/null
@@ -1,101 +0,0 @@ 
-#   BSD LICENSE
-#
-#   Copyright(c) 2010-2014 Intel Corporation. All rights reserved.
-#   All rights reserved.
-#
-#   Redistribution and use in source and binary forms, with or without
-#   modification, are permitted provided that the following conditions
-#   are met:
-#
-#     * Redistributions of source code must retain the above copyright
-#       notice, this list of conditions and the following disclaimer.
-#     * Redistributions in binary form must reproduce the above copyright
-#       notice, this list of conditions and the following disclaimer in
-#       the documentation and/or other materials provided with the
-#       distribution.
-#     * Neither the name of Intel Corporation nor the names of its
-#       contributors may be used to endorse or promote products derived
-#       from this software without specific prior written permission.
-#
-#   THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
-#   "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
-#   LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
-#   A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
-#   OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
-#   SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
-#   LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
-#   DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
-#   THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
-#   (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
-#   OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
-
-include $(RTE_SDK)/mk/internal/rte.build-pre.mk
-
-# VPATH contains at least SRCDIR
-VPATH += $(SRCDIR)
-
-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
-LIB_ONE := lib$(RTE_LIBNAME).so
-else
-LIB_ONE := lib$(RTE_LIBNAME).a
-endif
-endif
-
-.PHONY:sharelib
-sharelib: $(LIB_ONE) FORCE
-
-OBJS = $(wildcard $(RTE_OUTPUT)/build/lib/*.o)
-
-ifeq ($(LINK_USING_CC),1)
-# Override the definition of LD here, since we're linking with CC
-LD := $(CC) $(CPU_CFLAGS)
-O_TO_S = $(LD) $(call linkerprefix,$(CPU_LDFLAGS)) \
-	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
-else
-O_TO_S = $(LD) $(CPU_LDFLAGS) \
-	-shared $(OBJS) -o $(RTE_OUTPUT)/lib/$(LIB_ONE)
-endif
-
-O_TO_S_STR = $(subst ','\'',$(O_TO_S)) #'# fix syntax highlight
-O_TO_S_DISP = $(if $(V),"$(O_TO_S_STR)","  LD $(@)")
-O_TO_S_CMD = "cmd_$@ = $(O_TO_S_STR)"
-O_TO_S_DO = @set -e; \
-    echo $(O_TO_S_DISP); \
-    $(O_TO_S)
-
-O_TO_A =  $(AR) crus $(RTE_OUTPUT)/lib/$(LIB_ONE) $(OBJS)
-O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight
-O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)","  LD $(@)")
-O_TO_A_CMD = "cmd_$@ = $(O_TO_A_STR)"
-O_TO_A_DO = @set -e; \
-    echo $(O_TO_A_DISP); \
-    $(O_TO_A)
-#
-# Archive objects to share library
-#
-
-ifeq ($(RTE_BUILD_COMBINE_LIBS),y)
-ifeq ($(RTE_BUILD_SHARED_LIB),y)
-$(LIB_ONE): FORCE
-	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
-	$(O_TO_S_DO)
-else
-$(LIB_ONE): FORCE
-	@[ -d $(dir $@) ] || mkdir -p $(dir $@)
-	$(O_TO_A_DO)
-endif
-endif
-
-#
-# Clean all generated files
-#
-.PHONY: clean
-clean: _postclean
-
-.PHONY: doclean
-doclean:
-	$(Q)rm -rf $(LIB_ONE)
-
-.PHONY: FORCE
-FORCE:
diff --git a/mk/rte.vars.mk b/mk/rte.vars.mk
index d2f01b6..7b6f53d 100644
--- a/mk/rte.vars.mk
+++ b/mk/rte.vars.mk
@@ -67,15 +67,6 @@  ifneq ($(BUILDING_RTE_SDK),)
   ifeq ($(RTE_BUILD_SHARED_LIB),)
     RTE_BUILD_SHARED_LIB := n
   endif
-  RTE_BUILD_COMBINE_LIBS := $(CONFIG_RTE_BUILD_COMBINE_LIBS:"%"=%)
-  ifeq ($(RTE_BUILD_COMBINE_LIBS),)
-    RTE_BUILD_COMBINE_LIBS := n
-  endif
-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.