| Message ID | 1498888823-4065-4-git-send-email-odpbot@yandex.ru |
|---|---|
| State | New |
| Headers | show |
| Series | [v4,1/9] pkgconfig: provide minimal proper static linking flags | expand |
> diff --git a/test/Makefile.inc b/test/Makefile.inc > index 1ef2a92c..bf31b374 100644 > --- a/test/Makefile.inc > +++ b/test/Makefile.inc > @@ -4,7 +4,7 @@ LIB = $(top_builddir)/lib > #in the following line, the libs using the symbols should come before > #the libs containing them! The includer is given a chance to add things > #before libodp by setting PRE_LDADD before the inclusion. > -LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la > +LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la > $(DPDK_PMDS) Application using ODP should only need to add dependency to ODP and helper libs. It's not scalable if (all) applications need to know which (all) libs an ODP implementation may use internally. I guess this solves some DPDK linking issues, but is there a way to avoid explicit dependency to ODP lib internals ? -Petri
On 03.07.2017 13:34, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> diff --git a/test/Makefile.inc b/test/Makefile.inc >> index 1ef2a92c..bf31b374 100644 >> --- a/test/Makefile.inc >> +++ b/test/Makefile.inc >> @@ -4,7 +4,7 @@ LIB = $(top_builddir)/lib >> #in the following line, the libs using the symbols should come before >> #the libs containing them! The includer is given a chance to add things >> #before libodp by setting PRE_LDADD before the inclusion. >> -LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la >> +LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la >> $(DPDK_PMDS) > > Application using ODP should only need to add dependency to ODP and helper libs. It's not scalable if (all) applications need to know which (all) libs an ODP implementation may use internally. Applications using shared library don't need to know, what are ODP dependencies. Deps will be pulled in using .so DT_NEEDED. Static linking requires knowledge of all dependencies. Usually this will be handled by pkg-config (See Libs.private) or libtool (which also usually handles such configuration). Unfortunately DPDK PMDs do not fit into libtool scheme because of the way they are linked. Libtool doesn't understand whole -Wl,--whole-archive,... scheme, so it won't include it into dependencies list. Another possibility would be to create source file, which pulls in all PMDs detected by configure and link with just -ldpdk. > I guess this solves some DPDK linking issues, but is there a way to avoid explicit dependency to ODP lib internals ? Not with DPDK, unfortunately. -- With best wishes Dmitry
On Mon, Jul 3, 2017 at 7:04 AM, Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org> wrote: > On 03.07.2017 13:34, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>> diff --git a/test/Makefile.inc b/test/Makefile.inc >>> index 1ef2a92c..bf31b374 100644 >>> --- a/test/Makefile.inc >>> +++ b/test/Makefile.inc >>> @@ -4,7 +4,7 @@ LIB = $(top_builddir)/lib >>> #in the following line, the libs using the symbols should come before >>> #the libs containing them! The includer is given a chance to add things >>> #before libodp by setting PRE_LDADD before the inclusion. >>> -LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la >>> +LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la >>> $(DPDK_PMDS) >> >> Application using ODP should only need to add dependency to ODP and helper libs. It's not scalable if (all) applications need to know which (all) libs an ODP implementation may use internally. > > Applications using shared library don't need to know, what are ODP > dependencies. Deps will be pulled in using .so DT_NEEDED. Static linking > requires knowledge of all dependencies. Usually this will be handled by > pkg-config (See Libs.private) or libtool (which also usually handles > such configuration). Unfortunately DPDK PMDs do not fit into libtool > scheme because of the way they are linked. Libtool doesn't understand > whole -Wl,--whole-archive,... scheme, so it won't include it into > dependencies list. Another possibility would be to create source file, > which pulls in all PMDs detected by configure and link with just -ldpdk. Didn't Matias post some patches a while back to use --whole-archive for this purpose? See http://patches.opendataplane.org/patch/8237/ > >> I guess this solves some DPDK linking issues, but is there a way to avoid explicit dependency to ODP lib internals ? > > Not with DPDK, unfortunately. > > -- > With best wishes > Dmitry
> On 3 Jul 2017, at 15:24, Bill Fischofer <bill.fischofer@linaro.org> wrote: > > On Mon, Jul 3, 2017 at 7:04 AM, Dmitry Eremin-Solenikov > <dmitry.ereminsolenikov@linaro.org> wrote: >> On 03.07.2017 13:34, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>>> diff --git a/test/Makefile.inc b/test/Makefile.inc >>>> index 1ef2a92c..bf31b374 100644 >>>> --- a/test/Makefile.inc >>>> +++ b/test/Makefile.inc >>>> @@ -4,7 +4,7 @@ LIB = $(top_builddir)/lib >>>> #in the following line, the libs using the symbols should come before >>>> #the libs containing them! The includer is given a chance to add things >>>> #before libodp by setting PRE_LDADD before the inclusion. >>>> -LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la >>>> +LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la >>>> $(DPDK_PMDS) >>> >>> Application using ODP should only need to add dependency to ODP and helper libs. It's not scalable if (all) applications need to know which (all) libs an ODP implementation may use internally. >> >> Applications using shared library don't need to know, what are ODP >> dependencies. Deps will be pulled in using .so DT_NEEDED. Static linking >> requires knowledge of all dependencies. Usually this will be handled by >> pkg-config (See Libs.private) or libtool (which also usually handles >> such configuration). Unfortunately DPDK PMDs do not fit into libtool >> scheme because of the way they are linked. Libtool doesn't understand >> whole -Wl,--whole-archive,... scheme, so it won't include it into >> dependencies list. Another possibility would be to create source file, >> which pulls in all PMDs detected by configure and link with just -ldpdk. > > Didn't Matias post some patches a while back to use --whole-archive > for this purpose? See http://patches.opendataplane.org/patch/8237/ This patch only removed the need to reference to the drivers in source code. Instead, the build system automatically links all PMDs with '--whole-archive' flag. -Matias
On 03.07.2017 13:34, Savolainen, Petri (Nokia - FI/Espoo) wrote: >> diff --git a/test/Makefile.inc b/test/Makefile.inc >> index 1ef2a92c..bf31b374 100644 >> --- a/test/Makefile.inc >> +++ b/test/Makefile.inc >> @@ -4,7 +4,7 @@ LIB = $(top_builddir)/lib >> #in the following line, the libs using the symbols should come before >> #the libs containing them! The includer is given a chance to add things >> #before libodp by setting PRE_LDADD before the inclusion. >> -LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la >> +LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la >> $(DPDK_PMDS) > > Application using ODP should only need to add dependency to ODP and helper libs. It's not scalable if (all) applications need to know which (all) libs an ODP implementation may use internally. > > I guess this solves some DPDK linking issues, but is there a way to avoid explicit dependency to ODP lib internals ? Just to rephrase/emphasize my answer: user applications will use pkg-config file, which contains all necessary data to link with DPDK w/o mentioning it specifically. The only issue is internal linking inside ODP or linking using libtool. -- With best wishes Dmitry
> -----Original Message----- > From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsolenikov@linaro.org] > Sent: Tuesday, July 04, 2017 3:16 PM > To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; > Github ODP bot <odpbot@yandex.ru>; lng-odp@lists.linaro.org > Subject: Re: [lng-odp] [PATCH v4 3/9] linux-gen: stop poisoning > CPPFLAGS/LDFLAGS with DPDK flags > > On 03.07.2017 13:34, Savolainen, Petri (Nokia - FI/Espoo) wrote: > >> diff --git a/test/Makefile.inc b/test/Makefile.inc > >> index 1ef2a92c..bf31b374 100644 > >> --- a/test/Makefile.inc > >> +++ b/test/Makefile.inc > >> @@ -4,7 +4,7 @@ LIB = $(top_builddir)/lib > >> #in the following line, the libs using the symbols should come before > >> #the libs containing them! The includer is given a chance to add > things > >> #before libodp by setting PRE_LDADD before the inclusion. > >> -LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la > >> +LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la > >> $(DPDK_PMDS) > > > > Application using ODP should only need to add dependency to ODP and > helper libs. It's not scalable if (all) applications need to know which > (all) libs an ODP implementation may use internally. > > > > I guess this solves some DPDK linking issues, but is there a way to > avoid explicit dependency to ODP lib internals ? > > Just to rephrase/emphasize my answer: user applications will use > pkg-config file, which contains all necessary data to link with DPDK w/o > mentioning it specifically. The only issue is internal linking inside > ODP or linking using libtool. > In principle, our example/validation apps should see ODP the same way as any other app. That's why it seems odd that DPDK libs need to be exposed when linking those (or are those needed because of the test/linux-generic folder). My main concern is that external applications (like OFP) would need to add the same modification to their linking rules. That seems not to be the case, so I guess it's OK then. -Petri
On 04.07.2017 16:43, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > >> -----Original Message----- >> From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsolenikov@linaro.org] >> Sent: Tuesday, July 04, 2017 3:16 PM >> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; >> Github ODP bot <odpbot@yandex.ru>; lng-odp@lists.linaro.org >> Subject: Re: [lng-odp] [PATCH v4 3/9] linux-gen: stop poisoning >> CPPFLAGS/LDFLAGS with DPDK flags >> >> On 03.07.2017 13:34, Savolainen, Petri (Nokia - FI/Espoo) wrote: >>>> diff --git a/test/Makefile.inc b/test/Makefile.inc >>>> index 1ef2a92c..bf31b374 100644 >>>> --- a/test/Makefile.inc >>>> +++ b/test/Makefile.inc >>>> @@ -4,7 +4,7 @@ LIB = $(top_builddir)/lib >>>> #in the following line, the libs using the symbols should come before >>>> #the libs containing them! The includer is given a chance to add >> things >>>> #before libodp by setting PRE_LDADD before the inclusion. >>>> -LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la >>>> +LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la >>>> $(DPDK_PMDS) >>> >>> Application using ODP should only need to add dependency to ODP and >> helper libs. It's not scalable if (all) applications need to know which >> (all) libs an ODP implementation may use internally. >>> >>> I guess this solves some DPDK linking issues, but is there a way to >> avoid explicit dependency to ODP lib internals ? >> >> Just to rephrase/emphasize my answer: user applications will use >> pkg-config file, which contains all necessary data to link with DPDK w/o >> mentioning it specifically. The only issue is internal linking inside >> ODP or linking using libtool. >> > > In principle, our example/validation apps should see ODP the same way as any other app. That's why it seems odd that DPDK libs need to be exposed when linking those (or are those needed because of the test/linux-generic folder). > > My main concern is that external applications (like OFP) would need to add the same modification to their linking rules. That seems not to be the case, so I guess it's OK then. I understand that. In fact, my major concern when starting this PR was exactly linking of external apps. Check patch 9/9, which makes shure that one can use pkg-config to link ODP app instead of specifying all dependencies. -- With best wishes Dmitry
Petri, are you ok with that serries? Patch 9/9 is the major goal for odp packaging. p.s. as for me I don't have objections to merge it. Maxim. On 4 July 2017 at 23:07, Dmitry Eremin-Solenikov < dmitry.ereminsolenikov@linaro.org> wrote: > On 04.07.2017 16:43, Savolainen, Petri (Nokia - FI/Espoo) wrote: > > > > > >> -----Original Message----- > >> From: Dmitry Eremin-Solenikov [mailto:dmitry.ereminsolenikov@linaro.org > ] > >> Sent: Tuesday, July 04, 2017 3:16 PM > >> To: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; > >> Github ODP bot <odpbot@yandex.ru>; lng-odp@lists.linaro.org > >> Subject: Re: [lng-odp] [PATCH v4 3/9] linux-gen: stop poisoning > >> CPPFLAGS/LDFLAGS with DPDK flags > >> > >> On 03.07.2017 13:34, Savolainen, Petri (Nokia - FI/Espoo) wrote: > >>>> diff --git a/test/Makefile.inc b/test/Makefile.inc > >>>> index 1ef2a92c..bf31b374 100644 > >>>> --- a/test/Makefile.inc > >>>> +++ b/test/Makefile.inc > >>>> @@ -4,7 +4,7 @@ LIB = $(top_builddir)/lib > >>>> #in the following line, the libs using the symbols should come before > >>>> #the libs containing them! The includer is given a chance to add > >> things > >>>> #before libodp by setting PRE_LDADD before the inclusion. > >>>> -LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la > >>>> +LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la > >>>> $(DPDK_PMDS) > >>> > >>> Application using ODP should only need to add dependency to ODP and > >> helper libs. It's not scalable if (all) applications need to know which > >> (all) libs an ODP implementation may use internally. > >>> > >>> I guess this solves some DPDK linking issues, but is there a way to > >> avoid explicit dependency to ODP lib internals ? > >> > >> Just to rephrase/emphasize my answer: user applications will use > >> pkg-config file, which contains all necessary data to link with DPDK w/o > >> mentioning it specifically. The only issue is internal linking inside > >> ODP or linking using libtool. > >> > > > > In principle, our example/validation apps should see ODP the same way as > any other app. That's why it seems odd that DPDK libs need to be exposed > when linking those (or are those needed because of the test/linux-generic > folder). > > > > My main concern is that external applications (like OFP) would need to > add the same modification to their linking rules. That seems not to be the > case, so I guess it's OK then. > > I understand that. In fact, my major concern when starting this PR was > exactly linking of external apps. Check patch 9/9, which makes shure > that one can use pkg-config to link ODP app instead of specifying all > dependencies. > > -- > With best wishes > Dmitry >
From: Maxim Uvarov [mailto:maxim.uvarov@linaro.org]
Sent: Thursday, July 06, 2017 8:28 PM
To: Dmitry Eremin-Solenikov <dmitry.ereminsolenikov@linaro.org>
Cc: Savolainen, Petri (Nokia - FI/Espoo) <petri.savolainen@nokia.com>; Github ODP bot <odpbot@yandex.ru>; lng-odp@lists.linaro.org
Subject: Re: [lng-odp] [PATCH v4 3/9] linux-gen: stop poisoning CPPFLAGS/LDFLAGS with DPDK flags
Petri, are you ok with that serries? Patch 9/9 is the major goal for odp packaging.
p.s. as for me I don't have objections to merge it.
Maxim.
I'm OK with it.
-Petri
diff --git a/configure.ac b/configure.ac index 45812f66..3432b29a 100644 --- a/configure.ac +++ b/configure.ac @@ -210,7 +210,6 @@ AC_SUBST([testdir]) # Set conditionals as computed within platform specific files ########################################################################## AM_CONDITIONAL([netmap_support], [test x$netmap_support = xyes ]) -AM_CONDITIONAL([PKTIO_DPDK], [test x$pktio_dpdk_support = xyes ]) AM_CONDITIONAL([HAVE_PCAP], [test $have_pcap = yes]) AM_CONDITIONAL([SDK_INSTALL_PATH_], [test "x${SDK_INSTALL_PATH_}" = "x1"]) AM_CONDITIONAL([test_installdir], [test "$testdir" != ""]) diff --git a/pkgconfig/libodp-linux.pc.in b/pkgconfig/libodp-linux.pc.in index 0c5883b6..6dc06dc9 100644 --- a/pkgconfig/libodp-linux.pc.in +++ b/pkgconfig/libodp-linux.pc.in @@ -6,6 +6,6 @@ includedir=@includedir@ Name: libodp-linux Description: The ODP packet processing engine Version: @PKGCONFIG_VERSION@ -Libs: -L${libdir} -lodp-linux -Libs.private: @OPENSSL_STATIC_LIBS@ -lpcap @PTHREAD_LIBS@ -lrt -lpthread @ATOMIC_LIBS@ +Libs: -L${libdir} -lodp-linux @DPDK_LIBS@ +Libs.private: @OPENSSL_STATIC_LIBS@ @DPDK_PMDS@ @DPDK_LIBS@ -lpcap @PTHREAD_LIBS@ -lrt -lpthread @ATOMIC_LIBS@ Cflags: -I${includedir} diff --git a/platform/linux-generic/Makefile.am b/platform/linux-generic/Makefile.am index eb7b0422..45bee7ea 100644 --- a/platform/linux-generic/Makefile.am +++ b/platform/linux-generic/Makefile.am @@ -12,6 +12,7 @@ AM_CFLAGS += -Iinclude AM_CFLAGS += -D_ODP_PKTIO_IPC AM_CPPFLAGS += $(OPENSSL_CPPFLAGS) +AM_CPPFLAGS += $(DPDK_CPPFLAGS) include_HEADERS = \ $(top_srcdir)/include/odp.h \ @@ -257,6 +258,7 @@ endif __LIB__libodp_linux_la_LIBADD = $(ATOMIC_LIBS) __LIB__libodp_linux_la_LIBADD += $(OPENSSL_LIBS) +__LIB__libodp_linux_la_LIBADD += $(DPDK_LIBS) $(DPDK_PMDS) # Create symlink for ABI header files. Application does not need to use the arch # specific include path for installed files. diff --git a/platform/linux-generic/m4/odp_dpdk.m4 b/platform/linux-generic/m4/odp_dpdk.m4 index 58d14727..1995e0fe 100644 --- a/platform/linux-generic/m4/odp_dpdk.m4 +++ b/platform/linux-generic/m4/odp_dpdk.m4 @@ -4,15 +4,15 @@ pktio_dpdk_support=no AC_ARG_WITH([dpdk-path], AC_HELP_STRING([--with-dpdk-path=DIR path to dpdk build directory]), - [DPDK_PATH=$withval - AM_CPPFLAGS="$AM_CPPFLAGS -msse4.2 -isystem $DPDK_PATH/include" + [DPDK_PATH="$withval" + DPDK_CPPFLAGS="-msse4.2 -isystem $DPDK_PATH/include" pktio_dpdk_support=yes],[]) ########################################################################## # Save and set temporary compilation flags ########################################################################## -OLD_CPPFLAGS=$CPPFLAGS -CPPFLAGS="$AM_CPPFLAGS $CPPFLAGS" +OLD_CPPFLAGS="$CPPFLAGS" +CPPFLAGS="$DPDK_CPPFLAGS $CPPFLAGS" ########################################################################## # Check for DPDK availability @@ -25,22 +25,23 @@ then AC_CHECK_HEADERS([rte_config.h], [], [AC_MSG_FAILURE(["can't find DPDK header"])]) - DPDK_PMD=--whole-archive, - for filename in $with_dpdk_path/lib/*.a; do - cur_driver=`echo $(basename "$filename" .a) | \ - sed -n 's/^\(librte_pmd_\)/-lrte_pmd_/p' | sed -n 's/$/,/p'` + AS_VAR_SET([DPDK_PMDS], [-Wl,--whole-archive,]) + for filename in "$DPDK_PATH"/lib/librte_pmd_*.a; do + cur_driver=`basename "$filename" .a | sed -e 's/^lib//'` # rte_pmd_nfp has external dependencies which break linking - if test "$cur_driver" = "-lrte_pmd_nfp,"; then + if test "$cur_driver" = "rte_pmd_nfp"; then echo "skip linking rte_pmd_nfp" else - DPDK_PMD+=$cur_driver + AS_VAR_APPEND([DPDK_PMDS], [-l$cur_driver,]) fi done - DPDK_PMD+=--no-whole-archive + AS_VAR_APPEND([DPDK_PMDS], [--no-whole-archive]) ODP_CFLAGS="$ODP_CFLAGS -DODP_PKTIO_DPDK" - AM_LDFLAGS="$AM_LDFLAGS -L$DPDK_PATH/lib -Wl,$DPDK_PMD" - LIBS="$LIBS -ldpdk -ldl -lpcap" + DPDK_LIBS="-L$DPDK_PATH/lib -ldpdk -lpthread -ldl -lpcap" + AC_SUBST([DPDK_CPPFLAGS]) + AC_SUBST([DPDK_LIBS]) + AC_SUBST([DPDK_PMDS]) else pktio_dpdk_support=no fi @@ -49,3 +50,5 @@ fi # Restore old saved variables ########################################################################## CPPFLAGS=$OLD_CPPFLAGS + +AM_CONDITIONAL([PKTIO_DPDK], [test x$pktio_dpdk_support = xyes ]) diff --git a/test/Makefile.inc b/test/Makefile.inc index 1ef2a92c..bf31b374 100644 --- a/test/Makefile.inc +++ b/test/Makefile.inc @@ -4,7 +4,7 @@ LIB = $(top_builddir)/lib #in the following line, the libs using the symbols should come before #the libs containing them! The includer is given a chance to add things #before libodp by setting PRE_LDADD before the inclusion. -LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la +LDADD = $(PRE_LDADD) $(LIB)/libodphelper.la $(LIB)/libodp-linux.la $(DPDK_PMDS) INCFLAGS = \ -I$(top_builddir)/platform/@with_platform@/include \ diff --git a/test/common_plat/validation/api/Makefile.inc b/test/common_plat/validation/api/Makefile.inc index dda18adc..6069ee6b 100644 --- a/test/common_plat/validation/api/Makefile.inc +++ b/test/common_plat/validation/api/Makefile.inc @@ -9,6 +9,7 @@ AUTOMAKE_OPTIONS = nostdinc AM_CFLAGS += -I$(top_srcdir)/test/common_plat/common AM_LDFLAGS += -static +AM_LDFLAGS += $(DPDK_PMDS) LIBCUNIT_COMMON = $(COMMON_DIR)/libcunit_common.la LIBCPUMASK_COMMON = $(COMMON_DIR)/libcpumask_common.la diff --git a/test/linux-generic/Makefile.inc b/test/linux-generic/Makefile.inc index 198087f3..6e165d8d 100644 --- a/test/linux-generic/Makefile.inc +++ b/test/linux-generic/Makefile.inc @@ -6,7 +6,7 @@ AM_LDFLAGS += -static LIBCUNIT_COMMON = $(top_builddir)/test/common_plat/common/libcunit_common.la LIB = $(top_builddir)/lib -LIBODP = $(LIB)/libodphelper.la $(LIB)/libodp-linux.la +LIBODP = $(LIB)/libodphelper.la $(LIB)/libodp-linux.la $(DPDK_PMDS) INCCUNIT_COMMON = -I$(top_srcdir)/test/common_plat/common INCODP = \