[ovs-dev] [PATCH v4] dpdk: Update to use DPDK v20.11.
Ilya Maximets
i.maximets at ovn.org
Wed Dec 16 17:47:57 UTC 2020
On 12/16/20 5:31 PM, Stokes, Ian wrote:
>> 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.
>
> 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
More information about the dev
mailing list