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. > > 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
