| Message ID | 1459450985-2695-1-git-send-email-bhanuprakash.bodireddy@intel.com |
|---|---|
| State | Changes Requested |
| Headers | show |
On Thu, Mar 31, 2016 at 08:03:05PM +0100, Bhanuprakash Bodireddy wrote: > When using DPDK datapath, the OVS configure script requires the DPDK > build directory passed on --with-dpdk. This can be avoided if DPDK > library, headers are in standard compiler search paths. > > This patch fixes the problem by searching for DPDK libraries in standard > locations and configures OVS sources for dpdk datapath. > > If the install location is manually specified in "--with-dpdk" > autodiscovery shall be skipped. > > Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> ... > - if test X"$with_dpdk" != X; then > - RTE_SDK=$with_dpdk > + AC_MSG_CHECKING([whether dpdk datapath is enabled]) == is not portable for "test", use = instead: > + if test -z "$with_dpdk" || test "$with_dpdk" == no; then > + AC_MSG_RESULT([no]) > + DPDKLIB_FOUND=false Isn't the following "elif" test always true? That is, can't it just be an "else"? > + elif test -n "$with_dpdk"; then > + AC_MSG_RESULT([yes]) > + case "$with_dpdk" in > + yes) > + DPDK_AUTO_DISCOVER="true" > + ;; > + *) > + DPDK_AUTO_DISCOVER="false" > + ;; > + esac > > - DPDK_INCLUDE=$RTE_SDK/include > - DPDK_LIB_DIR=$RTE_SDK/lib Isn't the following "if" a little redundant given the previous "case" command, that is, why not integrate these into the cases? > + if $DPDK_AUTO_DISCOVER; then > + DPDK_INCLUDE="/usr/local/include/dpdk -I/usr/include/dpdk" > + else > + DPDK_INCLUDE="$with_dpdk/include" > + # If 'with_dpdk' is passed install directory, point to headers > + # installed in $DESTDIR/$prefix/include/dpdk This is really crunched together, why not add some whitespace to match the rest of the style: > + AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h],,[AC_CHECK_FILE([$DPDK_INCLUDE/dpdk/rte_config.h],[DPDK_INCLUDE=$DPDK_INCLUDE/dpdk],[])]) > + DPDK_LIB_DIR="$with_dpdk/lib" > + fi > DPDK_LIB="-ldpdk" > DPDK_EXTRA_LIB="" > - RTE_SDK_FULL=`readlink -f $RTE_SDK` > + > + ovs_save_CFLAGS="$CFLAGS" > + ovs_save_LDFLAGS="$LDFLAGS" > + CFLAGS="$CFLAGS -I$DPDK_INCLUDE" > + if test "$DPDK_AUTO_DISCOVER" = "false"; then > + LDFLAGS="$LDFLAGS -L${DPDK_LIB_DIR}" > + fi > > AC_COMPILE_IFELSE( > - [AC_LANG_PROGRAM([#include <$RTE_SDK_FULL/include/rte_config.h> > + [AC_LANG_PROGRAM([#include <rte_config.h> > #if !RTE_LIBRTE_VHOST_USER > #error > #endif], [])], The following line looks pretty randomly indented, I'd suggest that it should be two spaces farther in than the AC_LANG_PROGRAM keyword (and probably should start the [#include... on a separate line also indented the same amount): > [], [AC_DEFINE([VHOST_CUSE], [1], [DPDK vhost-cuse support enabled, vhost-user disabled.]) > DPDK_EXTRA_LIB="-lfuse"]) > > - ovs_save_CFLAGS="$CFLAGS" > - ovs_save_LDFLAGS="$LDFLAGS" > - LDFLAGS="$LDFLAGS -L$DPDK_LIB_DIR" > - CFLAGS="$CFLAGS -I$DPDK_INCLUDE" > - > # On some systems we have to add -ldl to link with dpdk > # > # This code, at first, tries to link without -ldl (""), Thanks, Ben.
Thanks for the review Ben. I have sent out Patch V5 based on your comments. - Bhanu Prakash. > -----Original Message----- > From: Ben Pfaff [mailto:blp@ovn.org] > Sent: Tuesday, April 12, 2016 3:58 AM > To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com> > Cc: dev@openvswitch.org > Subject: Re: [ovs-dev] [PATCH v4] acinclude: Autodetect DPDK location when > configuring OVS > > On Thu, Mar 31, 2016 at 08:03:05PM +0100, Bhanuprakash Bodireddy wrote: > > When using DPDK datapath, the OVS configure script requires the DPDK > > build directory passed on --with-dpdk. This can be avoided if DPDK > > library, headers are in standard compiler search paths. > > > > This patch fixes the problem by searching for DPDK libraries in > > standard locations and configures OVS sources for dpdk datapath. > > > > If the install location is manually specified in "--with-dpdk" > > autodiscovery shall be skipped. > > > > Signed-off-by: Bhanuprakash Bodireddy > > <bhanuprakash.bodireddy@intel.com> > > ... > > > - if test X"$with_dpdk" != X; then > > - RTE_SDK=$with_dpdk > > + AC_MSG_CHECKING([whether dpdk datapath is enabled]) > > == is not portable for "test", use = instead: > > + if test -z "$with_dpdk" || test "$with_dpdk" == no; then > > + AC_MSG_RESULT([no]) > > + DPDKLIB_FOUND=false > > Isn't the following "elif" test always true? That is, can't it just be an "else"? > > > + elif test -n "$with_dpdk"; then > > + AC_MSG_RESULT([yes]) > > + case "$with_dpdk" in > > + yes) > > + DPDK_AUTO_DISCOVER="true" > > + ;; > > + *) > > + DPDK_AUTO_DISCOVER="false" > > + ;; > > + esac > > > > - DPDK_INCLUDE=$RTE_SDK/include > > - DPDK_LIB_DIR=$RTE_SDK/lib > > Isn't the following "if" a little redundant given the previous "case" > command, that is, why not integrate these into the cases? > > > + if $DPDK_AUTO_DISCOVER; then > > + DPDK_INCLUDE="/usr/local/include/dpdk -I/usr/include/dpdk" > > + else > > + DPDK_INCLUDE="$with_dpdk/include" > > + # If 'with_dpdk' is passed install directory, point to headers > > + # installed in $DESTDIR/$prefix/include/dpdk > > This is really crunched together, why not add some whitespace to match the > rest of the style: > > + > AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h],,[AC_CHECK_FILE([$DPDK_I > NCLUDE/dpdk/rte_config.h],[DPDK_INCLUDE=$DPDK_INCLUDE/dpdk],[])]) > > + DPDK_LIB_DIR="$with_dpdk/lib" > > + fi > > DPDK_LIB="-ldpdk" > > DPDK_EXTRA_LIB="" > > - RTE_SDK_FULL=`readlink -f $RTE_SDK` > > + > > + ovs_save_CFLAGS="$CFLAGS" > > + ovs_save_LDFLAGS="$LDFLAGS" > > + CFLAGS="$CFLAGS -I$DPDK_INCLUDE" > > + if test "$DPDK_AUTO_DISCOVER" = "false"; then > > + LDFLAGS="$LDFLAGS -L${DPDK_LIB_DIR}" > > + fi > > > > AC_COMPILE_IFELSE( > > - [AC_LANG_PROGRAM([#include > <$RTE_SDK_FULL/include/rte_config.h> > > + [AC_LANG_PROGRAM([#include <rte_config.h> > > #if !RTE_LIBRTE_VHOST_USER > > #error > > #endif], [])], > > The following line looks pretty randomly indented, I'd suggest that it should > be two spaces farther in than the AC_LANG_PROGRAM keyword (and > probably should start the [#include... on a separate line also indented the > same amount): > > > [], [AC_DEFINE([VHOST_CUSE], [1], [DPDK vhost-cuse support > enabled, vhost-user disabled.]) > > DPDK_EXTRA_LIB="-lfuse"]) > > > > - ovs_save_CFLAGS="$CFLAGS" > > - ovs_save_LDFLAGS="$LDFLAGS" > > - LDFLAGS="$LDFLAGS -L$DPDK_LIB_DIR" > > - CFLAGS="$CFLAGS -I$DPDK_INCLUDE" > > - > > # On some systems we have to add -ldl to link with dpdk > > # > > # This code, at first, tries to link without -ldl (""), > > Thanks, > > Ben.
diff --git a/acinclude.m4 b/acinclude.m4 index f345c31..0d54ebe 100644 --- a/acinclude.m4 +++ b/acinclude.m4 @@ -163,28 +163,48 @@ AC_DEFUN([OVS_CHECK_DPDK], [ [AC_HELP_STRING([--with-dpdk=/path/to/dpdk], [Specify the DPDK build directory])]) - if test X"$with_dpdk" != X; then - RTE_SDK=$with_dpdk + AC_MSG_CHECKING([whether dpdk datapath is enabled]) + if test -z "$with_dpdk" || test "$with_dpdk" == no; then + AC_MSG_RESULT([no]) + DPDKLIB_FOUND=false + elif test -n "$with_dpdk"; then + AC_MSG_RESULT([yes]) + case "$with_dpdk" in + yes) + DPDK_AUTO_DISCOVER="true" + ;; + *) + DPDK_AUTO_DISCOVER="false" + ;; + esac - DPDK_INCLUDE=$RTE_SDK/include - DPDK_LIB_DIR=$RTE_SDK/lib + if $DPDK_AUTO_DISCOVER; then + DPDK_INCLUDE="/usr/local/include/dpdk -I/usr/include/dpdk" + else + DPDK_INCLUDE="$with_dpdk/include" + # If 'with_dpdk' is passed install directory, point to headers + # installed in $DESTDIR/$prefix/include/dpdk + AC_CHECK_FILE([$DPDK_INCLUDE/rte_config.h],,[AC_CHECK_FILE([$DPDK_INCLUDE/dpdk/rte_config.h],[DPDK_INCLUDE=$DPDK_INCLUDE/dpdk],[])]) + DPDK_LIB_DIR="$with_dpdk/lib" + fi DPDK_LIB="-ldpdk" DPDK_EXTRA_LIB="" - RTE_SDK_FULL=`readlink -f $RTE_SDK` + + ovs_save_CFLAGS="$CFLAGS" + ovs_save_LDFLAGS="$LDFLAGS" + CFLAGS="$CFLAGS -I$DPDK_INCLUDE" + if test "$DPDK_AUTO_DISCOVER" = "false"; then + LDFLAGS="$LDFLAGS -L${DPDK_LIB_DIR}" + fi AC_COMPILE_IFELSE( - [AC_LANG_PROGRAM([#include <$RTE_SDK_FULL/include/rte_config.h> + [AC_LANG_PROGRAM([#include <rte_config.h> #if !RTE_LIBRTE_VHOST_USER #error #endif], [])], [], [AC_DEFINE([VHOST_CUSE], [1], [DPDK vhost-cuse support enabled, vhost-user disabled.]) DPDK_EXTRA_LIB="-lfuse"]) - ovs_save_CFLAGS="$CFLAGS" - ovs_save_LDFLAGS="$LDFLAGS" - LDFLAGS="$LDFLAGS -L$DPDK_LIB_DIR" - CFLAGS="$CFLAGS -I$DPDK_INCLUDE" - # On some systems we have to add -ldl to link with dpdk # # This code, at first, tries to link without -ldl (""), @@ -192,7 +212,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [ # Before each attempt the search cache must be unset, # otherwise autoconf will stick with the old result - found=false + DPDKLIB_FOUND=false save_LIBS=$LIBS for extras in "" "-ldl"; do LIBS="$DPDK_LIB $extras $save_LIBS $DPDK_EXTRA_LIB" @@ -201,17 +221,25 @@ AC_DEFUN([OVS_CHECK_DPDK], [ #include <rte_eal.h>], [int rte_argc; char ** rte_argv; rte_eal_init(rte_argc, rte_argv);])], - [found=true]) - if $found; then + [DPDKLIB_FOUND=true]) + if $DPDKLIB_FOUND; then break fi done - if $found; then :; else - AC_MSG_ERROR([cannot link with dpdk]) + + # If linking unsuccessful + if test "$DPDKLIB_FOUND" = "false" ; then + if $DPDK_AUTO_DISCOVER; then + AC_MSG_ERROR([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 fi CFLAGS="$ovs_save_CFLAGS" LDFLAGS="$ovs_save_LDFLAGS" - OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" + if test "$DPDK_AUTO_DISCOVER" = "false"; then + OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR" + fi OVS_CFLAGS="$OVS_CFLAGS -I$DPDK_INCLUDE" OVS_ENABLE_OPTION([-mssse3]) @@ -226,12 +254,9 @@ AC_DEFUN([OVS_CHECK_DPDK], [ DPDK_vswitchd_LDFLAGS=-Wl,--whole-archive,$DPDK_LIB,--no-whole-archive AC_SUBST([DPDK_vswitchd_LDFLAGS]) AC_DEFINE([DPDK_NETDEV], [1], [System uses the DPDK module.]) - - else - RTE_SDK= fi - AM_CONDITIONAL([DPDK_NETDEV], test -n "$RTE_SDK") + AM_CONDITIONAL([DPDK_NETDEV], test "$DPDKLIB_FOUND" = true) ]) dnl OVS_GREP_IFELSE(FILE, REGEX, [IF-MATCH], [IF-NO-MATCH])
When using DPDK datapath, the OVS configure script requires the DPDK build directory passed on --with-dpdk. This can be avoided if DPDK library, headers are in standard compiler search paths. This patch fixes the problem by searching for DPDK libraries in standard locations and configures OVS sources for dpdk datapath. If the install location is manually specified in "--with-dpdk" autodiscovery shall be skipped. Signed-off-by: Bhanuprakash Bodireddy <bhanuprakash.bodireddy@intel.com> --- acinclude.m4 | 67 +++++++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 46 insertions(+), 21 deletions(-)