[ovs-dev] [PATCH v4] dpdk: Update to use DPDK v20.11.

[Ilya Maximets]

On 12/16/20 2:00 PM, Stokes, Ian wrote:
>> From: Ian Stokes <ian.stokes at intel.com>
>>
> 
> 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.

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

>> +              ;;
>> +            *)
>> +              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.

>> +    # 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