| Message ID | 1465583550-21020-4-git-send-email-ferruh.yigit@intel.com (mailing list archive) |
|---|---|
| State | Rejected, archived |
| Delegated to: | Thomas Monjalon |
| Headers | show |
2016-06-10 19:32, Ferruh Yigit: > As stated in the comment: > Order is important: from higher level to lower level > > This is an attempt to make the layering order better respected. > > Limit scope of --whole-archive to pmd libraries Compared to the link order in the v2, you did two things: 1/ move PMDs to the first position 2/ restrict --whole-archive to PMDs only Having the PMDs first, helps the linker to get all the PMD dependencies in the static binary. Thus it seems we do not need --whole-archive for the PMD dependencies (ethdev, mbuf, etc). But, if an external PMD is loaded via dlopen, it is possible that it needs a symbol which was not used by the internal PMDs. So it will not be found in the statically linked binary. Let's take another example: if we disable the internal PMDs with their config options from the static build, and we decide to build them separately as DSOs. We won't be able to load them as plugins because they depend on symbols which won't be found in the static binary. To make it short, the PMDs must be considered as plugins. Therefore, we must not rely on their availability to link the required symbols in a static binary. To make sure the plugin loading will be always well achieved, we must link the static PMDs at the last position. If you agree, I vote for the v2 of this patchset.
On 6/13/2016 10:29 AM, Thomas Monjalon wrote: > 2016-06-10 19:32, Ferruh Yigit: >> As stated in the comment: >> Order is important: from higher level to lower level >> >> This is an attempt to make the layering order better respected. >> >> Limit scope of --whole-archive to pmd libraries > > Compared to the link order in the v2, you did two things: > 1/ move PMDs to the first position > 2/ restrict --whole-archive to PMDs only > > Having the PMDs first, helps the linker to get all the PMD dependencies > in the static binary. Thus it seems we do not need --whole-archive > for the PMD dependencies (ethdev, mbuf, etc). > But, if an external PMD is loaded via dlopen, it is possible that it > needs a symbol which was not used by the internal PMDs. So it will not > be found in the statically linked binary. > Let's take another example: if we disable the internal PMDs with their > config options from the static build, and we decide to build them > separately as DSOs. We won't be able to load them as plugins because > they depend on symbols which won't be found in the static binary. So you want to keep all objects in final binary (used or unused) because of the possibility that any plugin may use them. But what is the list to include here, whole dpdk?, -since we may not know what API will plugin call. What I am confused is --whole-archive only used when dpdk compiled as static, if dpdk compiled as shared, each PMD should have proper dependencies [1], and if external PMD compiled properly there shouldn't be a problem. So do we have a case that dpdk compiled statically into final binary but we still want to load some plugins dynamically? > > To make it short, the PMDs must be considered as plugins. Therefore, we > must not rely on their availability to link the required symbols in > a static binary. > To make sure the plugin loading will be always well achieved, we must > link the static PMDs at the last position. I think this is not the issue of linking PMD's first or last, but expanding --whole-archive to cover other libraries other than PMDs. > > If you agree, I vote for the v2 of this patchset. If this is breaking something and best way to fix is not in external PMD but in here, please feel free to go with v2. [1] Like all has mbuf.so dependency: # ldd build/lib/librte_pmd_ring.so linux-vdso.so.1 (0x00007ffe3d78a000) librte_eal.so.2.1 => not found librte_ring.so.1.1 => not found librte_mbuf.so.2.1 => not found libethdev.so.4.1 => not found librte_kvargs.so.1.1 => not found libc.so.6 => /lib64/libc.so.6 (0x00007f8ab5ae1000) /lib64/ld-linux-x86-64.so.2 (0x0000556616488000) # ldd build/lib/librte_pmd_af_packet.so linux-vdso.so.1 (0x00007ffcb0572000) librte_mbuf.so.2.1 => not found libethdev.so.4.1 => not found librte_kvargs.so.1.1 => not found libc.so.6 => /lib64/libc.so.6 (0x00007f7fbd4aa000) /lib64/ld-linux-x86-64.so.2 (0x00005604556d2000) # ldd build/lib/librte_pmd_null.so linux-vdso.so.1 (0x00007ffdf08cf000) librte_mbuf.so.2.1 => not found libethdev.so.4.1 => not found librte_kvargs.so.1.1 => not found libc.so.6 => /lib64/libc.so.6 (0x00007fca6fbfc000) /lib64/ld-linux-x86-64.so.2 (0x000055fe7dd4a000) Thanks, ferruh
2016-06-13 11:04, Ferruh Yigit: > On 6/13/2016 10:29 AM, Thomas Monjalon wrote: > > 2016-06-10 19:32, Ferruh Yigit: > >> As stated in the comment: > >> Order is important: from higher level to lower level > >> > >> This is an attempt to make the layering order better respected. > >> > >> Limit scope of --whole-archive to pmd libraries > > > > Compared to the link order in the v2, you did two things: > > 1/ move PMDs to the first position > > 2/ restrict --whole-archive to PMDs only > > > > Having the PMDs first, helps the linker to get all the PMD dependencies > > in the static binary. Thus it seems we do not need --whole-archive > > for the PMD dependencies (ethdev, mbuf, etc). > > But, if an external PMD is loaded via dlopen, it is possible that it > > needs a symbol which was not used by the internal PMDs. So it will not > > be found in the statically linked binary. > > Let's take another example: if we disable the internal PMDs with their > > config options from the static build, and we decide to build them > > separately as DSOs. We won't be able to load them as plugins because > > they depend on symbols which won't be found in the static binary. > > So you want to keep all objects in final binary (used or unused) because > of the possibility that any plugin may use them. Yes > But what is the list to include here, whole dpdk?, -since we may not > know what API will plugin call. The list of the libraries candidates to be called from a driver can be discussed. The top layers like lpm or distributor should not be in this list I think. > What I am confused is --whole-archive only used when dpdk compiled as > static, if dpdk compiled as shared, each PMD should have proper > dependencies [1], and if external PMD compiled properly there shouldn't > be a problem. So do we have a case that dpdk compiled statically into > final binary but we still want to load some plugins dynamically? Yes a plugin can be loaded from a static binary. Breaking this feature would need a separate discussion and notices. > > To make it short, the PMDs must be considered as plugins. Therefore, we > > must not rely on their availability to link the required symbols in > > a static binary. > > To make sure the plugin loading will be always well achieved, we must > > link the static PMDs at the last position. > > I think this is not the issue of linking PMD's first or last, but > expanding --whole-archive to cover other libraries other than PMDs. Yes, linking PMD at the end is a way to force us to keep some libraries in --whole-archive. > > If you agree, I vote for the v2 of this patchset. > > If this is breaking something and best way to fix is not in external PMD > but in here, please feel free to go with v2. I don't see any other solution. But I'm sure we could discuss it more and/or improve it in the future.
On 6/13/2016 11:21 AM, Thomas Monjalon wrote: > 2016-06-13 11:04, Ferruh Yigit: >> On 6/13/2016 10:29 AM, Thomas Monjalon wrote: >>> 2016-06-10 19:32, Ferruh Yigit: >>>> As stated in the comment: >>>> Order is important: from higher level to lower level >>>> >>>> This is an attempt to make the layering order better respected. >>>> >>>> Limit scope of --whole-archive to pmd libraries >>> >>> Compared to the link order in the v2, you did two things: >>> 1/ move PMDs to the first position >>> 2/ restrict --whole-archive to PMDs only >>> >>> Having the PMDs first, helps the linker to get all the PMD dependencies >>> in the static binary. Thus it seems we do not need --whole-archive >>> for the PMD dependencies (ethdev, mbuf, etc). >>> But, if an external PMD is loaded via dlopen, it is possible that it >>> needs a symbol which was not used by the internal PMDs. So it will not >>> be found in the statically linked binary. >>> Let's take another example: if we disable the internal PMDs with their >>> config options from the static build, and we decide to build them >>> separately as DSOs. We won't be able to load them as plugins because >>> they depend on symbols which won't be found in the static binary. >> >> So you want to keep all objects in final binary (used or unused) because >> of the possibility that any plugin may use them. > > Yes > >> But what is the list to include here, whole dpdk?, -since we may not >> know what API will plugin call. > > The list of the libraries candidates to be called from a driver can be > discussed. The top layers like lpm or distributor should not be in this > list I think. > >> What I am confused is --whole-archive only used when dpdk compiled as >> static, if dpdk compiled as shared, each PMD should have proper >> dependencies [1], and if external PMD compiled properly there shouldn't >> be a problem. So do we have a case that dpdk compiled statically into >> final binary but we still want to load some plugins dynamically? > > Yes a plugin can be loaded from a static binary. > Breaking this feature would need a separate discussion and notices. Thanks for clarifying, I wasn't aware of this. So I am leaving this patch, please just ping if anything required from me. > >>> To make it short, the PMDs must be considered as plugins. Therefore, we >>> must not rely on their availability to link the required symbols in >>> a static binary. >>> To make sure the plugin loading will be always well achieved, we must >>> link the static PMDs at the last position. >> >> I think this is not the issue of linking PMD's first or last, but >> expanding --whole-archive to cover other libraries other than PMDs. > > Yes, linking PMD at the end is a way to force us to keep some libraries > in --whole-archive. > >>> If you agree, I vote for the v2 of this patchset. >> >> If this is breaking something and best way to fix is not in external PMD >> but in here, please feel free to go with v2. > > I don't see any other solution. > But I'm sure we could discuss it more and/or improve it in the future. >
diff --git a/mk/rte.app.mk b/mk/rte.app.mk index 8068e66..b8761ae 100644 --- a/mk/rte.app.mk +++ b/mk/rte.app.mk @@ -59,6 +59,51 @@ _LDLIBS-y += -L$(RTE_SDK_BIN)/lib _LDLIBS-y += --whole-archive +ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) +# plugins (link only if static libraries) + +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += -lrte_pmd_af_packet +_LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD) += -lrte_pmd_bnx2x -lz +_LDLIBS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD) += -lrte_pmd_cxgbe +_LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += -lrte_pmd_e1000 +_LDLIBS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += -lrte_pmd_ena +_LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += -lrte_pmd_enic +_LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += -lrte_pmd_fm10k +_LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e +_LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -lrte_pmd_mlx4 -libverbs +_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += -lrte_pmd_mlx5 -libverbs +_LDLIBS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += -lrte_pmd_mpipe -lgxio +_LDLIBS-$(CONFIG_RTE_LIBRTE_NFP_PMD) += -lrte_pmd_nfp -lm +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += -lrte_pmd_null +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lrte_pmd_pcap -lpcap +_LDLIBS-$(CONFIG_RTE_LIBRTE_QEDE_PMD) += -lrte_pmd_qede -lz +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING) += -lrte_pmd_ring +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SZEDATA2) += -lrte_pmd_szedata2 -lsze2 +_LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += -lrte_pmd_virtio +ifeq ($(CONFIG_RTE_LIBRTE_VHOST),y) +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += -lrte_pmd_vhost +endif # $(CONFIG_RTE_LIBRTE_VHOST) +_LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += -lrte_pmd_vmxnet3_uio + +ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y) +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) += -lrte_pmd_aesni_mb +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) += -L$(AESNI_MULTI_BUFFER_LIB_PATH) -lIPSec_MB +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_GCM) += -lrte_pmd_aesni_gcm -lcrypto +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_GCM) += -L$(AESNI_MULTI_BUFFER_LIB_PATH) -lIPSec_MB +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += -lrte_pmd_null_crypto +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_QAT) += -lrte_pmd_qat -lcrypto +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SNOW3G) += -lrte_pmd_snow3g +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SNOW3G) += -L$(LIBSSO_PATH)/build -lsso +endif # CONFIG_RTE_LIBRTE_CRYPTODEV + +endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB) + +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond +_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt -lxenstore + +_LDLIBS-y += --no-whole-archive + _LDLIBS-$(CONFIG_RTE_LIBRTE_DISTRIBUTOR) += -lrte_distributor _LDLIBS-$(CONFIG_RTE_LIBRTE_REORDER) += -lrte_reorder @@ -107,52 +152,9 @@ _LDLIBS-$(CONFIG_RTE_LIBRTE_RING) += -lrte_ring _LDLIBS-$(CONFIG_RTE_LIBRTE_EAL) += -lrte_eal _LDLIBS-$(CONFIG_RTE_LIBRTE_CMDLINE) += -lrte_cmdline _LDLIBS-$(CONFIG_RTE_LIBRTE_CFGFILE) += -lrte_cfgfile -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_BOND) += -lrte_pmd_bond -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_XENVIRT) += -lrte_pmd_xenvirt -lxenstore - -ifeq ($(CONFIG_RTE_BUILD_SHARED_LIB),n) -# plugins (link only if static libraries) - -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AF_PACKET) += -lrte_pmd_af_packet -_LDLIBS-$(CONFIG_RTE_LIBRTE_BNX2X_PMD) += -lrte_pmd_bnx2x -lz -_LDLIBS-$(CONFIG_RTE_LIBRTE_CXGBE_PMD) += -lrte_pmd_cxgbe -_LDLIBS-$(CONFIG_RTE_LIBRTE_E1000_PMD) += -lrte_pmd_e1000 -_LDLIBS-$(CONFIG_RTE_LIBRTE_ENA_PMD) += -lrte_pmd_ena -_LDLIBS-$(CONFIG_RTE_LIBRTE_ENIC_PMD) += -lrte_pmd_enic -_LDLIBS-$(CONFIG_RTE_LIBRTE_FM10K_PMD) += -lrte_pmd_fm10k -_LDLIBS-$(CONFIG_RTE_LIBRTE_I40E_PMD) += -lrte_pmd_i40e -_LDLIBS-$(CONFIG_RTE_LIBRTE_IXGBE_PMD) += -lrte_pmd_ixgbe -_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX4_PMD) += -lrte_pmd_mlx4 -libverbs -_LDLIBS-$(CONFIG_RTE_LIBRTE_MLX5_PMD) += -lrte_pmd_mlx5 -libverbs -_LDLIBS-$(CONFIG_RTE_LIBRTE_MPIPE_PMD) += -lrte_pmd_mpipe -lgxio -_LDLIBS-$(CONFIG_RTE_LIBRTE_NFP_PMD) += -lrte_pmd_nfp -lm -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL) += -lrte_pmd_null -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_PCAP) += -lrte_pmd_pcap -lpcap -_LDLIBS-$(CONFIG_RTE_LIBRTE_QEDE_PMD) += -lrte_pmd_qede -lz -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_RING) += -lrte_pmd_ring -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SZEDATA2) += -lrte_pmd_szedata2 -lsze2 -_LDLIBS-$(CONFIG_RTE_LIBRTE_VIRTIO_PMD) += -lrte_pmd_virtio -ifeq ($(CONFIG_RTE_LIBRTE_VHOST),y) -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_VHOST) += -lrte_pmd_vhost -endif # $(CONFIG_RTE_LIBRTE_VHOST) -_LDLIBS-$(CONFIG_RTE_LIBRTE_VMXNET3_PMD) += -lrte_pmd_vmxnet3_uio - -ifeq ($(CONFIG_RTE_LIBRTE_CRYPTODEV),y) -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) += -lrte_pmd_aesni_mb -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_MB) += -L$(AESNI_MULTI_BUFFER_LIB_PATH) -lIPSec_MB -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_GCM) += -lrte_pmd_aesni_gcm -lcrypto -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_AESNI_GCM) += -L$(AESNI_MULTI_BUFFER_LIB_PATH) -lIPSec_MB -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_NULL_CRYPTO) += -lrte_pmd_null_crypto -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_QAT) += -lrte_pmd_qat -lcrypto -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SNOW3G) += -lrte_pmd_snow3g -_LDLIBS-$(CONFIG_RTE_LIBRTE_PMD_SNOW3G) += -L$(LIBSSO_PATH)/build -lsso -endif # CONFIG_RTE_LIBRTE_CRYPTODEV - -endif # ! $(CONFIG_RTE_BUILD_SHARED_LIB) _LDLIBS-y += $(EXECENV_LDLIBS) _LDLIBS-y += --end-group -_LDLIBS-y += --no-whole-archive LDLIBS += $(_LDLIBS-y) $(CPU_LDLIBS) $(EXTRA_LDLIBS)
As stated in the comment: Order is important: from higher level to lower level This is an attempt to make the layering order better respected. Limit scope of --whole-archive to pmd libraries Signed-off-by: Thomas Monjalon <thomas.monjalon@6wind.com> Signed-off-by: Ferruh Yigit <ferruh.yigit@intel.com> --- mk/rte.app.mk | 88 ++++++++++++++++++++++++++++++----------------------------- 1 file changed, 45 insertions(+), 43 deletions(-)