On 05/26/2016 04:13 PM, Ferruh Yigit wrote: > On 5/24/2016 11:11 AM, Christian Ehrhardt wrote: >> Hi Panu, >> I already agreed to Thomas on IRC but won't have time until next week. >> Thanks for making a patch that does that already - I'll give it a look and >> some test on my end next week then. >> >> >> >> Christian Ehrhardt >> Software Engineer, Ubuntu Server >> Canonical Ltd >> >> On Tue, May 24, 2016 at 11:56 AM, Panu Matilainen <pmatilai at redhat.com> >> wrote: >> >>> On 05/20/2016 08:08 PM, Thomas Monjalon wrote: >>> >>>> 2016-05-20 18:50, Christian Ehrhardt: >>>> >>>>> The individual libraries have various cross dependencies. >>>>> This is already refelcted in the DEPDIR dependency, but not yet in >>>>> proper DT_NEEDED flags in the .so's. >>>>> This adds the -l flags so that is properly stored in the .so's ELF >>>>> headers. >>>>> >>>> >>>> Why not filling LDLIBS by parsing DEPDIRS-y in rte.lib.mk? >>>> >>>> >>> A fair question :) The thought has passed my mind too, but I thought it'd >>> be too messy with differing library vs directory names and other >>> exceptions. But in reality manually maintained separate LDLIBS is only >>> going to get out of sync sooner or later, and turns out its not that bad >>> anyway: >>> >>> diff --git a/mk/rte.lib.mk b/mk/rte.lib.mk >>> index b420280..88b4e98 100644 >>> --- a/mk/rte.lib.mk >>> +++ b/mk/rte.lib.mk >>> @@ -77,6 +77,12 @@ else >>> _CPU_LDFLAGS := $(CPU_LDFLAGS) >>> endif >>> >>> +# Translate DEPDIRS-y into LDLIBS >>> +IGNORE_DEPS = -lrte_eal/% -lrte_net -lrte_compat >>> +_LDDIRS = $(subst librte_ether,libethdev,$(DEPDIRS-y)) >>> +_LDDEPS = $(subst lib/lib,-l,$(_LDDIRS)) >>> +LDLIBS += $(filter-out $(IGNORE_DEPS), $(_LDDEPS)) >>> + >>> O_TO_A = $(AR) crDs $(LIB) $(OBJS-y) >>> O_TO_A_STR = $(subst ','\'',$(O_TO_A)) #'# fix syntax highlight >>> O_TO_A_DISP = $(if $(V),"$(O_TO_A_STR)"," AR $(@)") >>> >>> I'm also sure somebody more familiar with gmake could improve it, but it >>> seems to work quite fine here. I'll wait a bit to see if there are other >>> suggestions before sending it as a proper patch. >>> >>> - Panu - >>> > > I did test the patch with minor diff in rte.vars.mk [1], otherwise all > libraries kept needed in final library. > > Patch itself looks good, overlinking problem in final binary seems fixed > [2] [3]. > > But this cause a compilation error for cmdline_test [4]. This is because > of cyclic dependency between librte_eal <-> librte_mempool. > > Other applications don't have this compilation error because they use > librte_mempool, and since it is linked against application, this error > not observed. cmdline_test itself doesn't have any direct call to > librte_mempool. > > It is possible to add workaround to cmdline_test [5], (introduce > overlinking back), but should we attack on cyclic dependency instead?
Cyclic dependencies are nasty, eliminating it would be good. But in the meanwhile it can be worked around in a generic manner by handling it in the libdpdk.so linker script, see http://dpdk.org/ml/archives/dev/2016-May/039413.html - Panu - > > Thanks, > ferruh > > --- > > [1] > diff --git a/mk/exec-env/linuxapp/rte.vars.mk > b/mk/exec-env/linuxapp/rte.vars.mk > index d51bd17..10d37d5 100644 > --- a/mk/exec-env/linuxapp/rte.vars.mk > +++ b/mk/exec-env/linuxapp/rte.vars.mk > @@ -46,7 +46,7 @@ EXECENV_CFLAGS = -pthread > endif > > # Workaround lack of DT_NEEDED entry > -EXECENV_LDFLAGS = --no-as-needed > +EXECENV_LDFLAGS = --as-needed > > EXECENV_LDLIBS = > EXECENV_ASFLAGS = > > > [2] > # ldd build/app/testpmd > linux-vdso.so.1 (0x00007ffcff92e000) > librte_mbuf.so.2.1 => .../build/lib/librte_mbuf.so.2.1 > libethdev.so.4.1 => .../build/lib/libethdev.so.4.1 > librte_mempool.so.2.1 => .../build/lib/librte_mempool.so.2.1 > librte_ring.so.1.1 => .../build/lib/librte_ring.so.1.1 > librte_eal.so.2.1 => .../build/lib/librte_eal.so.2.1 > librte_cmdline.so.2.1 => .../build/lib/librte_cmdline.so.2.1 > librte_pmd_bond.so.1.1 => .../build/lib/librte_pmd_bond.so.1.1 > libpthread.so.0 => /lib64/libpthread.so.0 (0x00007f6879ff1000) > libc.so.6 => /lib64/libc.so.6 (0x00007f6879c30000) > /lib64/ld-linux-x86-64.so.2 (0x0000564998dbd000) > libdl.so.2 => /lib64/libdl.so.2 (0x00007f6879a2c000) > libgcc_s.so.1 => /lib64/libgcc_s.so.1 (0x00007f6879814000) > librt.so.1 => /lib64/librt.so.1 (0x00007f687960c000) > librte_kvargs.so.1.1 => .../build/lib/librte_kvargs.so.1.1 > > [3] > # ldd -u -r build/app/testpmd > # > > > [4] > == Build app/cmdline_test > CC cmdline_test.o > CC commands.o > LD cmdline_test > .../build/lib/librte_eal.so: undefined reference to `rte_mempool_lookup' > .../build/lib/librte_eal.so: undefined reference to `rte_mempool_create' > collect2: error: ld returned 1 exit status > .../mk/rte.app.mk:243: recipe for target 'cmdline_test' failed > make[3]: *** [cmdline_test] Error 1 > > > [5] > diff --git a/app/cmdline_test/Makefile b/app/cmdline_test/Makefile > index c6169f5..69ebfb2 100644 > --- a/app/cmdline_test/Makefile > +++ b/app/cmdline_test/Makefile > @@ -46,6 +46,7 @@ SRCS-y += commands.c > > CFLAGS += -O3 > CFLAGS += $(WERROR_FLAGS) > +LDFLAGS += --no-as-needed > > # this application needs libraries first > DEPDIRS-y += lib drivers >
