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.bodire...@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.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Reply via email to