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