diff mbox

[ovs-dev,v3] acinclude: Autodetect DPDK when configuring OVS

Message ID 1458919871-67578-1-git-send-email-bhanuprakash.bodireddy@intel.com
State Changes Requested
Headers show

Commit Message

Bodireddy, Bhanuprakash March 25, 2016, 3:31 p.m. UTC
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 | 61 ++++++++++++++++++++++++++++++++++++------------------------
 1 file changed, 37 insertions(+), 24 deletions(-)

Comments

Panu Matilainen March 30, 2016, 12:06 p.m. UTC | #1
On 03/25/2016 05:31 PM, 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>
> ---
>   acinclude.m4 | 61 ++++++++++++++++++++++++++++++++++++------------------------
>   1 file changed, 37 insertions(+), 24 deletions(-)
>
> diff --git a/acinclude.m4 b/acinclude.m4
> index f345c31..edb9563 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -163,22 +163,32 @@ 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"
> +      DPDK_LIB_DIR="/usr/local/lib -L/usr/lib64 -L/usr/lib"

This raises questions like why /usr/lib64 but not /usr/local/lib64? 
However, the bigger issue there is that lib and lib64 contents cannot be 
mixed because on a system where lib64 paths are valid, lib paths are 
32bit libraries. The linker already knows all that, so instead of trying 
to guess paths in vain, just try to link to -ldpdk. If that fails then 
its either outside standard library paths or does not exist on the system.

The include path does need to be determined since it needs to be added 
to -I, those two locations should be enough though..

> +    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`
> -
> -    AC_COMPILE_IFELSE(
> -      [AC_LANG_PROGRAM([#include <$RTE_SDK_FULL/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"])

This is silently removing vhost-cuse/vhost-user detection. I personally 
wouldn't mind support for vhost-cuse dropped but this is not the way to 
do it, it belongs to a separate patch along with appropriate explanation.


	- Panu -
Bodireddy, Bhanuprakash March 30, 2016, 6:10 p.m. UTC | #2
> -----Original Message-----

> From: Panu Matilainen [mailto:pmatilai@redhat.com]

> Sent: Wednesday, March 30, 2016 1:06 PM

> To: Bodireddy, Bhanuprakash <bhanuprakash.bodireddy@intel.com>;

> dev@openvswitch.org

> Cc: mchandras@suse.de

> Subject: Re: [PATCH v3] acinclude: Autodetect DPDK when configuring OVS

> 

> On 03/25/2016 05:31 PM, 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>

> > ---

> >   acinclude.m4 | 61 ++++++++++++++++++++++++++++++++++++-----------

> -------------

> >   1 file changed, 37 insertions(+), 24 deletions(-)

> >

> > diff --git a/acinclude.m4 b/acinclude.m4 index f345c31..edb9563 100644

> > --- a/acinclude.m4

> > +++ b/acinclude.m4

> > @@ -163,22 +163,32 @@ 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"

> > +      DPDK_LIB_DIR="/usr/local/lib -L/usr/lib64 -L/usr/lib"

> 

> This raises questions like why /usr/lib64 but not /usr/local/lib64?

> However, the bigger issue there is that lib and lib64 contents cannot be

> mixed because on a system where lib64 paths are valid, lib paths are 32bit

> libraries. The linker already knows all that, so instead of trying to guess paths

> in vain, just try to link to -ldpdk. If that fails then its either outside standard

> library paths or does not exist on the system.


Ok, In case of autodiscovery mechanism I will let the linker do the job and will have the DPDK_LIB_DIR set only when user explicitly pass the build location on "--with-dpdk".

> 

> The include path does need to be determined since it needs to be added to -

> I, those two locations should be enough though..


Fine. 
 
> > +    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_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`

> > -

> > -    AC_COMPILE_IFELSE(

> > -      [AC_LANG_PROGRAM([#include

> <$RTE_SDK_FULL/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"])

> 

> This is silently removing vhost-cuse/vhost-user detection. I personally

> wouldn't mind support for vhost-cuse dropped but this is not the way to do

> it, it belongs to a separate patch along with appropriate explanation.


Point taken, I will handle this separately in a different patch with updates to INSTALL.DPDK.md.

- Bhanu Prakash.
> 

> 

> 	- Panu -
diff mbox

Patch

diff --git a/acinclude.m4 b/acinclude.m4
index f345c31..edb9563 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -163,22 +163,32 @@  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"
+      DPDK_LIB_DIR="/usr/local/lib -L/usr/lib64 -L/usr/lib"
+    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`
-
-    AC_COMPILE_IFELSE(
-      [AC_LANG_PROGRAM([#include <$RTE_SDK_FULL/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"
@@ -192,22 +202,28 @@  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"
+        LIBS="$DPDK_LIB $extras $save_LIBS"
         AC_LINK_IFELSE(
            [AC_LANG_PROGRAM([#include <rte_config.h>
                              #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"
@@ -226,12 +242,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])