> On 12/16/20 5:31 PM, Stokes, Ian wrote: > >> On 12/16/20 2:00 PM, Stokes, Ian wrote: > >>>> From: Ian Stokes <[email protected]> > >>>> > >>> > >>> Thanks for working and sending this Sunil. > >>> > >>> @Ilya, does this resolve your concerns WRT the pkg-config check and ARM > >> build in travis? > >> > >> It works fine. GHA passed and Travis for Arm works fine too. > >> Some small coments inline. > > > > Thanks for checking. > > > >> > >> <sinp> > >> > >>>> diff --git a/acinclude.m4 b/acinclude.m4 > >>>> index ddf4b71e1..eb0496c25 100644 > >>>> --- a/acinclude.m4 > >>>> +++ b/acinclude.m4 > >>>> @@ -334,8 +334,9 @@ dnl > >>>> dnl Configure DPDK source tree > >>>> AC_DEFUN([OVS_CHECK_DPDK], [ > >>>> AC_ARG_WITH([dpdk], > >>>> - [AC_HELP_STRING([--with-dpdk=/path/to/dpdk], > >>>> - [Specify the DPDK build directory])], > >>>> + [AC_HELP_STRING([--with-dpdk=static|shared|yes], > >>>> + [Specify "static" or "shared" depending > >>>> on the > >>>> + DPDK libraries to use])], > >>>> [have_dpdk=true]) > >>>> > >>>> AC_MSG_CHECKING([whether dpdk is enabled]) > >>>> @@ -345,35 +346,45 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > >>>> else > >>>> AC_MSG_RESULT([yes]) > >>>> case "$with_dpdk" in > >>>> - yes) > >>>> - DPDK_AUTO_DISCOVER="true" > >>>> - PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk], [ > >>>> - DPDK_INCLUDE="$DPDK_CFLAGS" > >>>> - DPDK_LIB="$DPDK_LIBS"], [ > >>>> - DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk" > >>>> - DPDK_LIB="-ldpdk"]) > >>>> - ;; > >>>> - *) > >>>> - DPDK_AUTO_DISCOVER="false" > >>>> - DPDK_INCLUDE_PATH="$with_dpdk/include" > >>>> - # If 'with_dpdk' is passed install directory, point to headers > >>>> - # installed in $DESTDIR/$prefix/include/dpdk > >>>> - if test -e "$DPDK_INCLUDE_PATH/rte_config.h"; then > >>>> - DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH" > >>>> - elif test -e "$DPDK_INCLUDE_PATH/dpdk/rte_config.h"; then > >>>> - DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH/dpdk" > >>>> - fi > >>>> - DPDK_LIB_DIR="$with_dpdk/lib" > >>>> - DPDK_LIB="-ldpdk" > >>>> - ;; > >>>> + "shared") > >>>> + PKG_CHECK_MODULES([DPDK], [libdpdk], [ > >>>> + DPDK_INCLUDE="$DPDK_CFLAGS" > >>>> + DPDK_LIB="$DPDK_LIBS"], [ > >>>> + DPDK_INCLUDE="-I/usr/local/include/dpdk > >>>> -I/usr/include/dpdk" > >>>> + DPDK_LIB="-ldpdk"]) > >>>> + ;; > >>>> + "static" | "yes") > >>>> + PKG_CHECK_MODULES_STATIC([DPDK], [libdpdk], [ > >>>> + DPDK_INCLUDE="$DPDK_CFLAGS" > >>>> + DPDK_LIB="$DPDK_LIBS"], [ > >>>> + DPDK_INCLUDE="-I/usr/local/include/dpdk > >>>> -I/usr/include/dpdk" > >>>> + DPDK_LIB="-ldpdk"]) > >>>> + > >>>> + dnl Statically linked private DPDK objects of form > >>>> + dnl -l:file.a must be positioned between > >>>> + dnl --whole-archive ... --no-whole-archive linker parameters. > >>>> + dnl Old pkg-config versions misplace --no-whole-archive > >>>> parameter > >>>> + dnl and put it next to --whole-archive. > >>>> + AC_MSG_CHECKING([for faulty pkg-config version]) > >>>> + echo "$DPDK_LIB" | grep -q 'whole-archive.*l:lib.*no-whole- > archive' > >>>> + status=$? > >>>> + case $status in > >>>> + 0) > >>>> + AC_MSG_RESULT([no]) > >>>> + ;; > >>>> + 1) > >>>> + AC_MSG_RESULT([yes]) > >>>> + AC_MSG_ERROR([Please upgrade.pkg-config]) > >> > >> Indentation of above lines is a bit off. And the text is a bit strange to > >> me. > >> Why is there a period in the middle of sentence? > >> > > > > Agreed, can remove this period and align correctly. > > > >>>> + ;; > >>>> + *) > >>>> + AC_MSG_ERROR([grep exited with status $status]) > >>>> + ;; > >>>> + esac > >>>> esac > >>>> > >>>> ovs_save_CFLAGS="$CFLAGS" > >>>> ovs_save_LDFLAGS="$LDFLAGS" > >>>> CFLAGS="$CFLAGS $DPDK_INCLUDE" > >>>> - if test "$DPDK_AUTO_DISCOVER" = "false"; then > >>>> - LDFLAGS="$LDFLAGS -L${DPDK_LIB_DIR}" > >>>> - fi > >>>> > >>>> AC_CHECK_HEADERS([rte_config.h], [], [ > >>>> AC_MSG_ERROR([unable to find rte_config.h in $with_dpdk]) > >>>> @@ -422,20 +433,18 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > >>>> [AC_MSG_RESULT([yes]) > >>>> DPDKLIB_FOUND=true], > >>>> [AC_MSG_RESULT([no]) > >>>> - if test "$DPDK_AUTO_DISCOVER" = "true"; then > >>>> - AC_MSG_ERROR(m4_normalize([ > >>>> - Could not find DPDK library in default search path, Use > >>>> --with-dpdk > >>>> - to specify the DPDK library installed in non-standard > >>>> location])) > >>>> - else > >>>> - AC_MSG_ERROR([Could not find DPDK libraries in $DPDK_LIB_DIR]) > >>>> - fi > >>>> + AC_MSG_ERROR(m4_normalize([ > >>>> + Could not find DPDK library in default search path, update > >>>> + PKG_CONFIG_PATH for pkg-config to find the .pc file in > >>>> + non-standard location])) > >>>> ]) > >>>> > >>>> CFLAGS="$ovs_save_CFLAGS" > >>>> LDFLAGS="$ovs_save_LDFLAGS" > >>>> - if test "$DPDK_AUTO_DISCOVER" = "false"; then > >>>> - OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" > >>>> - fi > >>>> + # Stripping out possible instruction set specific configuration > >>>> that DPDK > >>>> + # forces in pkg-config since this could override user-specified > >>>> options. > >>>> + # It's enough to have -mssse3 to build with DPDK headers. > >>>> + DPDK_INCLUDE=$(echo "$DPDK_INCLUDE" | sed 's/-march=[[^ ]]*//g') > >>>> OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE" > >>>> OVS_ENABLE_OPTION([-mssse3]) > >>>> > >>>> @@ -444,17 +453,16 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > >>>> # This happens because the rest of the DPDK code doesn't use any > symbol > >> in > >>>> # the pmd driver objects, and the drivers register themselves using > >>>> an > >>>> # __attribute__((constructor)) function. > >>>> - # > >>>> - # These options are specified inside a single -Wl directive to > >>>> prevent > >>>> - # autotools from reordering them. > >>>> - # > >>>> - # OTOH newer versions of dpdk pkg-config (generated with Meson) > >>>> - # will already have flagged just the right set of libs with > >>>> - # --whole-archive - in those cases do not wrap it once more. > >>>> - case "$DPDK_LIB" in > >>>> - *whole-archive*) DPDK_vswitchd_LDFLAGS=$DPDK_LIB;; > >>>> - *) DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no- > >> whole- > >>>> archive > >>>> - esac > >>>> + > >> > >> This empty line divides a comment into 2 separate comments. With that, > >> first > >> part of a comment looks like it's not related to the second one. > >> I think, it should be a single comment block, otherwise, it's unclear why > >> we're > >> talking about 'whole-archive' stuff there at all. > >> > > > > OK, we can move this back to one comment. > > > > With these changes are you happy for me to commit with your ack? > > Yes, I'm OK with that. >
Thanks Ilya, I've just sent a v5 with the changes. I can add it to that instead if you're ok with that. http://patchwork.ozlabs.org/project/openvswitch/list/?series=220908 Regards Ian > > > > Regards > > Ian > >>>> + # Wrap the DPDK libraries inside a single -Wl directive > >>>> + # after comma separation to prevent autotools from reordering them. > >>>> + DPDK_vswitchd_LDFLAGS=$(echo "$DPDK_LIB"| tr -s ' ' ',' | sed 's/- > >> Wl,//g') > >>>> + # Replace -pthread with -lpthread for LD and remove the last extra > >> comma. > >>>> + DPDK_vswitchd_LDFLAGS=$(echo "$DPDK_vswitchd_LDFLAGS"| sed > >> 's/,$//' | > >>>> \ > >>>> + sed 's/-pthread/-lpthread/g') > >>>> + # Prepend "-Wl,". > >>>> + DPDK_vswitchd_LDFLAGS="-Wl,$DPDK_vswitchd_LDFLAGS" > >>>> + > >>>> AC_SUBST([DPDK_vswitchd_LDFLAGS]) > >>>> AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.]) > >>>> fi _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
