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 <[email protected]>
...
> - 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
[email protected]
http://openvswitch.org/mailman/listinfo/dev