[ovs-dev] acinclude: netdev-dpdk: use pkg-config of libdpdk
diff mbox

Message ID 1499246365-9633-1-git-send-email-christian.ehrhardt@canonical.com
State Not Applicable
Headers show

Commit Message

Christian Ehrhardt July 5, 2017, 9:19 a.m. UTC
If available use dpdk pkg-config info of libdpdk to set the right
include paths.
That for example, allows packagers to provide non default include
paths in a common way (pkg-config).

Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
Suggested-by: Luca Boccassi <luca.boccassi@gmail.com>
---
 Documentation/intro/install/dpdk.rst |  6 +++++-
 acinclude.m4                         | 18 +++++++++++-------
 configure.ac                         |  1 +
 3 files changed, 17 insertions(+), 8 deletions(-)

Comments

Luca Boccassi July 5, 2017, 10:43 a.m. UTC | #1
On Wed, 2017-07-05 at 11:19 +0200, Christian Ehrhardt wrote:
> If available use dpdk pkg-config info of libdpdk to set the right
> include paths.
> That for example, allows packagers to provide non default include
> paths in a common way (pkg-config).
> 
> Signed-off-by: Christian Ehrhardt <christian.ehrhardt@canonical.com>
> Suggested-by: Luca Boccassi <luca.boccassi@gmail.com>
> ---
>  Documentation/intro/install/dpdk.rst |  6 +++++-
>  acinclude.m4                         | 18 +++++++++++-------
>  configure.ac                         |  1 +
>  3 files changed, 17 insertions(+), 8 deletions(-)
> 
> diff --git a/Documentation/intro/install/dpdk.rst
> b/Documentation/intro/install/dpdk.rst
> index e83f852..097cd56 100644
> --- a/Documentation/intro/install/dpdk.rst
> +++ b/Documentation/intro/install/dpdk.rst
> @@ -124,7 +124,11 @@ has to be configured with DPDK support (``
> --with-dpdk``).
>         $ ./configure --with-dpdk=$DPDK_BUILD
>  
>     where ``DPDK_BUILD`` is the path to the built DPDK library. This
> can be
> -   skipped if DPDK library is installed in its default location
> +   skipped if DPDK library is installed in its default location.
> +
> +   If no path is provided to ``--with-dpdk``, but a pkg-config
> configuration
> +   for libdpdk is available the include paths will be generated via
> an
> +   equivalent ``pkg-config --cflags libdpdk``.
>  
>     .. note::
>       While ``--with-dpdk`` is required, you can pass any other
> configuration
> diff --git a/acinclude.m4 b/acinclude.m4
> index 7d7b683..3a388e2 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -199,16 +199,20 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>      case "$with_dpdk" in
>        yes)
>          DPDK_AUTO_DISCOVER="true"
> -        DPDK_INCLUDE="/usr/local/include/dpdk -I/usr/include/dpdk"
> +        PKG_CHECK_MODULES([DPDK], [libdpdk],
> +                          [DPDK_INCLUDE="$DPDK_CFLAGS"],
> +                          [DPDK_INCLUDE="-I/usr/local/include/dpdk
> -I/usr/include/dpdk"])
>          ;;
>        *)
>          DPDK_AUTO_DISCOVER="false"
> -        DPDK_INCLUDE="$with_dpdk/include"
> +        DPDK_INCLUDE_PATH="$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/dpd
> k], [])])
> +        AC_CHECK_FILE([$DPDK_INCLUDE_PATH/rte_config.h],
> +                      [DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH"],
> +                      [AC_CHECK_FILE([$DPDK_INCLUDE_PATH/dpdk/rte_co
> nfig.h],
> +                                     [DPDK_INCLUDE="-
> I$DPDK_INCLUDE_PATH/dpdk"],
> +                                     [])])
>          DPDK_LIB_DIR="$with_dpdk/lib"
>          ;;
>      esac
> @@ -218,7 +222,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>  
>      ovs_save_CFLAGS="$CFLAGS"
>      ovs_save_LDFLAGS="$LDFLAGS"
> -    CFLAGS="$CFLAGS -I$DPDK_INCLUDE"
> +    CFLAGS="$CFLAGS $DPDK_INCLUDE"
>      if test "$DPDK_AUTO_DISCOVER" = "false"; then
>        LDFLAGS="$LDFLAGS -L${DPDK_LIB_DIR}"
>      fi
> @@ -294,7 +298,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [
>      if test "$DPDK_AUTO_DISCOVER" = "false"; then
>        OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR"
>      fi
> -    OVS_CFLAGS="$OVS_CFLAGS -I$DPDK_INCLUDE"
> +    OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE"
>      OVS_ENABLE_OPTION([-mssse3])
>  
>      # DPDK pmd drivers are not linked unless --whole-archive is
> used.
> diff --git a/configure.ac b/configure.ac
> index 6404b5f..b364f28 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -27,6 +27,7 @@ AC_PROG_CPP
>  AC_PROG_MKDIR_P
>  AC_PROG_FGREP
>  AC_PROG_EGREP
> +PKG_PROG_PKG_CONFIG
>  
>  AC_ARG_VAR([PERL], [path to Perl interpreter])
>  AC_PATH_PROG([PERL], perl, no)

Acked-by: Luca Boccassi <luca.boccassi@gmail.com>
Christian Ehrhardt July 11, 2017, 2:34 p.m. UTC | #2
Ping - any update on this thread - anybody looked into considering the
config change?
Timothy Redaelli July 11, 2017, 3:53 p.m. UTC | #3
On 07/05/2017 11:19 AM, Christian Ehrhardt wrote:
> If available use dpdk pkg-config info of libdpdk to set the right
> include paths.
> That for example, allows packagers to provide non default include
> paths in a common way (pkg-config).

I like the idea, but I think it's better to wait for upstream (DPDK) to
add pkg-config support before pushing something that actually can only
be used in debian-based distributions (libdpdk-dev package).
Christian Ehrhardt July 11, 2017, 4:41 p.m. UTC | #4
On Tue, Jul 11, 2017 at 5:53 PM, Timothy M. Redaelli <tredaelli@redhat.com>
wrote:

> I like the idea, but I think it's better to wait for upstream (DPDK) to
> add pkg-config support before pushing something that actually can only
> be used in debian-based distributions (libdpdk-dev package).
>

Thanks Timothy for your feedback.

DPDK will unlikely grow that feature before rebuilding the build system
which is work in progress but still needs quite some time.
OTOH on said Debian based Distributions it would fix a real issue which is
to allow to properly split header packaging for multiarch support.

And that is exactly why we made it:
  if pkg-config available
    use pkg-config
  else
    as it always was

If unfortunately "wait" is the overall consensus we can live with that, yet
I'd clearly prefer to see the change accepted to resolve our issues we have
"now".

cu
Christian
Ben Pfaff July 14, 2017, 12:07 a.m. UTC | #5
On Tue, Jul 11, 2017 at 04:34:04PM +0200, Christian Ehrhardt wrote:
> Ping - any update on this thread - anybody looked into considering the
> config change?

I don't object to this myself.  I'm not a big fan of pkg-config but it's
a lot better than most home-grown ways to find libraries and headers.
Timothy Redaelli has a good point that it's not in upstream yet, but I
don't think that should rule out adding support via pkg-config given
that real distros do support it and since there's a fallback.

Can you rebase and repost?  OVS master has changed in this area, so
there's a patch reject.

Thanks,

Ben.

Patch
diff mbox

diff --git a/Documentation/intro/install/dpdk.rst b/Documentation/intro/install/dpdk.rst
index e83f852..097cd56 100644
--- a/Documentation/intro/install/dpdk.rst
+++ b/Documentation/intro/install/dpdk.rst
@@ -124,7 +124,11 @@  has to be configured with DPDK support (``--with-dpdk``).
        $ ./configure --with-dpdk=$DPDK_BUILD
 
    where ``DPDK_BUILD`` is the path to the built DPDK library. This can be
-   skipped if DPDK library is installed in its default location
+   skipped if DPDK library is installed in its default location.
+
+   If no path is provided to ``--with-dpdk``, but a pkg-config configuration
+   for libdpdk is available the include paths will be generated via an
+   equivalent ``pkg-config --cflags libdpdk``.
 
    .. note::
      While ``--with-dpdk`` is required, you can pass any other configuration
diff --git a/acinclude.m4 b/acinclude.m4
index 7d7b683..3a388e2 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -199,16 +199,20 @@  AC_DEFUN([OVS_CHECK_DPDK], [
     case "$with_dpdk" in
       yes)
         DPDK_AUTO_DISCOVER="true"
-        DPDK_INCLUDE="/usr/local/include/dpdk -I/usr/include/dpdk"
+        PKG_CHECK_MODULES([DPDK], [libdpdk],
+                          [DPDK_INCLUDE="$DPDK_CFLAGS"],
+                          [DPDK_INCLUDE="-I/usr/local/include/dpdk -I/usr/include/dpdk"])
         ;;
       *)
         DPDK_AUTO_DISCOVER="false"
-        DPDK_INCLUDE="$with_dpdk/include"
+        DPDK_INCLUDE_PATH="$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], [])])
+        AC_CHECK_FILE([$DPDK_INCLUDE_PATH/rte_config.h],
+                      [DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH"],
+                      [AC_CHECK_FILE([$DPDK_INCLUDE_PATH/dpdk/rte_config.h],
+                                     [DPDK_INCLUDE="-I$DPDK_INCLUDE_PATH/dpdk"],
+                                     [])])
         DPDK_LIB_DIR="$with_dpdk/lib"
         ;;
     esac
@@ -218,7 +222,7 @@  AC_DEFUN([OVS_CHECK_DPDK], [
 
     ovs_save_CFLAGS="$CFLAGS"
     ovs_save_LDFLAGS="$LDFLAGS"
-    CFLAGS="$CFLAGS -I$DPDK_INCLUDE"
+    CFLAGS="$CFLAGS $DPDK_INCLUDE"
     if test "$DPDK_AUTO_DISCOVER" = "false"; then
       LDFLAGS="$LDFLAGS -L${DPDK_LIB_DIR}"
     fi
@@ -294,7 +298,7 @@  AC_DEFUN([OVS_CHECK_DPDK], [
     if test "$DPDK_AUTO_DISCOVER" = "false"; then
       OVS_LDFLAGS="$OVS_LDFLAGS -L$DPDK_LIB_DIR"
     fi
-    OVS_CFLAGS="$OVS_CFLAGS -I$DPDK_INCLUDE"
+    OVS_CFLAGS="$OVS_CFLAGS $DPDK_INCLUDE"
     OVS_ENABLE_OPTION([-mssse3])
 
     # DPDK pmd drivers are not linked unless --whole-archive is used.
diff --git a/configure.ac b/configure.ac
index 6404b5f..b364f28 100644
--- a/configure.ac
+++ b/configure.ac
@@ -27,6 +27,7 @@  AC_PROG_CPP
 AC_PROG_MKDIR_P
 AC_PROG_FGREP
 AC_PROG_EGREP
+PKG_PROG_PKG_CONFIG
 
 AC_ARG_VAR([PERL], [path to Perl interpreter])
 AC_PATH_PROG([PERL], perl, no)