diff mbox series

[ovs-dev,v4,7/8] acinclude.m4: Build with AF_XDP support by default if possible.

Message ID 20221222000625.220692-8-i.maximets@ovn.org
State Accepted
Commit e44e80343189fcb7ec10d776f1b62747d7095c18
Headers show
Series AF_XDP build fixes and enhancements. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation success test: success

Commit Message

Ilya Maximets Dec. 22, 2022, 12:06 a.m. UTC 
With this change we will try to detect all the netdev-afxdp
dependencies and enable AF_XDP support by default if they are
present at the build time.

Configuration script behaves in a following way:

 - ./configure --enable-afxdp

   Will check for AF_XDP dependencies and fail if they are
   not available.

 - ./configure --disable-afxdp

   Disables checking for AF_XDP.  Build will not support
   AF_XDP even if all dependencies are installed.

 - Just ./configure or ./configure --enable-afxdp=auto

   Will check for AF_XDP dependencies.  Will print a warning
   if they are not available, but will continue without AF_XDP
   support.  If dependencies are available in a system, this
   option is equal to --enable-afxdp.

'--disable-afxdp' added to the debian and fedora package builds
to keep predictable behavior.

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---
 Documentation/intro/install/afxdp.rst |  6 ++-
 NEWS                                  |  3 ++
 acinclude.m4                          | 72 ++++++++++++++++++---------
 debian/rules                          | 25 ++++++----
 rhel/openvswitch-fedora.spec.in       |  2 +
 5 files changed, 72 insertions(+), 36 deletions(-)

Comments

Eelco Chaudron Dec. 22, 2022, 3:04 p.m. UTC | #1 
On 22 Dec 2022, at 1:06, Ilya Maximets wrote:

> With this change we will try to detect all the netdev-afxdp
> dependencies and enable AF_XDP support by default if they are
> present at the build time.
>
> Configuration script behaves in a following way:
>
>  - ./configure --enable-afxdp
>
>    Will check for AF_XDP dependencies and fail if they are
>    not available.
>
>  - ./configure --disable-afxdp
>
>    Disables checking for AF_XDP.  Build will not support
>    AF_XDP even if all dependencies are installed.
>
>  - Just ./configure or ./configure --enable-afxdp=auto
>
>    Will check for AF_XDP dependencies.  Will print a warning
>    if they are not available, but will continue without AF_XDP
>    support.  If dependencies are available in a system, this
>    option is equal to --enable-afxdp.
>
> '--disable-afxdp' added to the debian and fedora package builds
> to keep predictable behavior.
>
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> ---

Looks like the auto-enable is not working on my fedora35 machine (with or without libxdp installed) :)

[ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --enable-afxdp | grep -i -E "af_xdp|afxdp"
checking whether AF_XDP is enabled... yes
[ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --disable-afxdp | grep -i -E "af_xdp|afxdp"
checking whether AF_XDP is enabled... no
[ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --enable-afxdp=auto | grep -i -E "af_xdp|afxdp"
checking whether AF_XDP is enabled... no
[ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc | grep -i -E "af_xdp|afxdp"
checking whether AF_XDP is enabled... no

>  Documentation/intro/install/afxdp.rst |  6 ++-
>  NEWS                                  |  3 ++
>  acinclude.m4                          | 72 ++++++++++++++++++---------
>  debian/rules                          | 25 ++++++----
>  rhel/openvswitch-fedora.spec.in       |  2 +
>  5 files changed, 72 insertions(+), 36 deletions(-)
>
> diff --git a/Documentation/intro/install/afxdp.rst b/Documentation/intro/install/afxdp.rst
> index a4f0b87fe..51c24bf5b 100644
> --- a/Documentation/intro/install/afxdp.rst
> +++ b/Documentation/intro/install/afxdp.rst
> @@ -30,8 +30,7 @@ This document describes how to build and install Open vSwitch using
>  AF_XDP netdev.
>
>  .. warning::
> -  The AF_XDP support of Open vSwitch is considered 'experimental',
> -  and it is not compiled in by default.
> +  The AF_XDP support of Open vSwitch is considered 'experimental'.
>
>
>  Introduction
> @@ -137,6 +136,9 @@ bootstrap/configure the package::
>
>    ./boot.sh && ./configure --enable-afxdp
>
> +``--enable-afxdp`` here is optional, but it will ensure that all dependencies
> +are available at the build time.
> +
>  Finally, build and install OVS::
>
>    make && make install
> diff --git a/NEWS b/NEWS
> index ce5d11d73..2f6ededfe 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -4,6 +4,9 @@ Post-v3.0.0
>       of handler and revalidator threads if necessary.
>     - AF_XDP:
>       * Added support for building with libxdp and libbpf >= 0.7.
> +     * Support for AF_XDP is now enabled by default if all dependencies are
> +       available at the build time.  Use --disable-afxdp to disable.
> +       Use --enable-afxdp to fail the build if dependencies are not present.
>     - ovs-appctl:
>       * "ovs-appctl ofproto/trace" command can now display port names with the
>         "--names" option.
> diff --git a/acinclude.m4 b/acinclude.m4
> index e47e925b3..8aecfb63d 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -253,46 +253,70 @@ dnl OVS_CHECK_LINUX_AF_XDP
>  dnl
>  dnl Check both Linux kernel AF_XDP and libbpf/libxdp support
>  AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [
> -  AC_ARG_ENABLE([afxdp],
> -                [AS_HELP_STRING([--enable-afxdp], [Enable AF-XDP support])],
> -                [], [enable_afxdp=no])
> +  AC_ARG_ENABLE(
> +    [afxdp],
> +    [AS_HELP_STRING([--disable-afxdp], [Disable AF-XDP support])],
> +    [case "${enableval}" in
> +       (yes | no | auto) ;;
> +       (*) AC_MSG_ERROR([bad value ${enableval} for --enable-afxdp]) ;;
> +     esac],
> +    [enable_afxdp=auto])
> +
>    AC_MSG_CHECKING([whether AF_XDP is enabled])
> -  if test "$enable_afxdp" != yes; then
> +  if test "$enable_afxdp" == no; then
>      AC_MSG_RESULT([no])
>      AF_XDP_ENABLE=false
>    else
> -    AC_MSG_RESULT([yes])
> +    AC_MSG_RESULT([$enable_afxdp])
>      AF_XDP_ENABLE=true
> +    failed_dep=none
> +    dnl Saving libs to restore in case we will end up not building with AF_XDP.
> +    save_LIBS=$LIBS
>
> -    AC_CHECK_HEADER([bpf/libbpf.h], [],
> -      [AC_MSG_ERROR([unable to find bpf/libbpf.h for AF_XDP support])])
> +    AC_CHECK_HEADER([bpf/libbpf.h], [], [failed_dep="bpf/libbpf.h"])
>
> -    AC_CHECK_HEADER([linux/if_xdp.h], [],
> -      [AC_MSG_ERROR([unable to find linux/if_xdp.h for AF_XDP support])])
> +    if test "$failed_dep" = none; then
> +      AC_CHECK_HEADER([linux/if_xdp.h], [], [failed_dep="linux/if_xdp.h"])
> +    fi
>
> -    OVS_FIND_DEPENDENCY([libbpf_strerror], [bpf], [libbpf])
> -    AC_CHECK_FUNCS([bpf_xdp_query_id bpf_xdp_detach])
> +    if test "$failed_dep" = none; then
> +      AC_SEARCH_LIBS([libbpf_strerror], [bpf], [], [failed_dep="libbpf"])
> +      AC_CHECK_FUNCS([bpf_xdp_query_id bpf_xdp_detach])
> +    fi
>
> -    if test "x$ac_cv_func_bpf_xdp_detach" = xyes; then
> +    if test "$failed_dep" = none -a "x$ac_cv_func_bpf_xdp_detach" = xyes; then
>          dnl We have libbpf >= 0.7.  Look for libxdp as xsk functions
>          dnl were moved into this library.
> -        OVS_FIND_DEPENDENCY([libxdp_strerror], [xdp], [libxdp])
> -        AC_CHECK_HEADER([xdp/xsk.h],
> -          AC_DEFINE([HAVE_LIBXDP], [1], [xsk.h is supplied with libxdp]),
> -          AC_MSG_ERROR([unable to find xdp/xsk.h for AF_XDP support]))
> -    else
> +        AC_SEARCH_LIBS([libxdp_strerror], [xdp],
> +          AC_CHECK_HEADER([xdp/xsk.h],
> +            AC_DEFINE([HAVE_LIBXDP], [1], [xsk.h is supplied with libxdp]),
> +            [failed_dep="xdp/xsk.h"]),
> +          [failed_dep="libxdp"])
> +    elif test "$failed_dep" = none; then
>          dnl libbpf < 0.7 contains all the necessary functionality.
> -        AC_CHECK_HEADER([bpf/xsk.h], [],
> -          [AC_MSG_ERROR([unable to find bpf/xsk.h for AF_XDP support])])
> +        AC_CHECK_HEADER([bpf/xsk.h], [], [failed_dep="bpf/xsk.h"])
>      fi
>
> -    AC_CHECK_FUNCS([pthread_spin_lock], [],
> -      [AC_MSG_ERROR([unable to find pthread_spin_lock for AF_XDP support])])
> +    if test "$failed_dep" = none; then
> +      AC_CHECK_FUNCS([pthread_spin_lock], [], [failed_dep="pthread_spin_lock"])
> +    fi
>
> -    OVS_FIND_DEPENDENCY([numa_alloc_onnode], [numa], [libnuma])
> +    if test "$failed_dep" = none; then
> +      AC_SEARCH_LIBS([numa_alloc_onnode], [numa], [], [failed_dep="libnuma"])
> +    fi
>
> -    AC_DEFINE([HAVE_AF_XDP], [1],
> -              [Define to 1 if AF_XDP support is available and enabled.])
> +    if test "$failed_dep" = none; then
> +      AC_DEFINE([HAVE_AF_XDP], [1],
> +                [Define to 1 if AF_XDP support is available and enabled.])
> +    elif test "$enable_afxdp" = yes; then
> +      AC_MSG_ERROR([Missing $failed_dep dependency for AF_XDP support])
> +    else
> +      AC_MSG_WARN(m4_normalize(
> +          [Cannot find $failed_dep, netdev-afxdp will not be supported
> +           (use --disable-afxdp to suppress this warning).]))
> +      AF_XDP_ENABLE=false
> +      LIBS=$save_LIBS
> +    fi
>    fi
>    AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true)
>  ])
> diff --git a/debian/rules b/debian/rules
> index 971bc1775..ddbd4dc5c 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -23,21 +23,26 @@ override_dh_auto_configure:
>  	test -d _debian || mkdir _debian
>  	cd _debian && ( \
>  		test -e Makefile || \
> -		../configure --prefix=/usr --localstatedir=/var --enable-ssl \
> -					 --sysconfdir=/etc \
> -					 $(DATAPATH_CONFIGURE_OPTS) \
> -					 $(EXTRA_CONFIGURE_OPTS) \
> -					 )
> +		../configure --prefix=/usr --localstatedir=/var \
> +					--enable-ssl \
> +					--disable-afxdp \
> +					--sysconfdir=/etc \
> +					$(DATAPATH_CONFIGURE_OPTS) \
> +					$(EXTRA_CONFIGURE_OPTS) \
> +					)
>  ifneq (,$(filter i386 amd64 ppc64el arm64, $(DEB_HOST_ARCH)))
>  ifeq (,$(filter nodpdk, $(DEB_BUILD_OPTIONS)))
>  	test -d _dpdk || mkdir _dpdk
>  	cd _dpdk && ( \
>  		test -e Makefile || \
> -        ../configure --prefix=/usr --localstatedir=/var --enable-ssl \
> -                     --with-dpdk=shared --sysconfdir=/etc \
> -					 $(DATAPATH_CONFIGURE_OPTS) \
> -					 $(EXTRA_CONFIGURE_OPTS) \
> -					 )
> +		../configure --prefix=/usr --localstatedir=/var \
> +					--enable-ssl \
> +					--disable-afxdp \
> +					--with-dpdk=shared \
> +					--sysconfdir=/etc \
> +					$(DATAPATH_CONFIGURE_OPTS) \
> +					$(EXTRA_CONFIGURE_OPTS) \
> +					)
>  endif
>  endif
>
> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
> index eb5077a21..3091e204e 100644
> --- a/rhel/openvswitch-fedora.spec.in
> +++ b/rhel/openvswitch-fedora.spec.in
> @@ -171,6 +171,8 @@ This package provides IPsec tunneling support for OVS tunnels.
>  %endif
>  %if %{with afxdp}
>          --enable-afxdp \
> +%else
> +        --disable-afxdp \
>  %endif
>          --enable-ssl \
>          --disable-static \
> -- 
> 2.38.1
Eelco Chaudron Dec. 22, 2022, 3:09 p.m. UTC | #2 
On 22 Dec 2022, at 16:04, Eelco Chaudron wrote:

> On 22 Dec 2022, at 1:06, Ilya Maximets wrote:
>
>> With this change we will try to detect all the netdev-afxdp
>> dependencies and enable AF_XDP support by default if they are
>> present at the build time.
>>
>> Configuration script behaves in a following way:
>>
>>  - ./configure --enable-afxdp
>>
>>    Will check for AF_XDP dependencies and fail if they are
>>    not available.
>>
>>  - ./configure --disable-afxdp
>>
>>    Disables checking for AF_XDP.  Build will not support
>>    AF_XDP even if all dependencies are installed.
>>
>>  - Just ./configure or ./configure --enable-afxdp=auto
>>
>>    Will check for AF_XDP dependencies.  Will print a warning
>>    if they are not available, but will continue without AF_XDP
>>    support.  If dependencies are available in a system, this
>>    option is equal to --enable-afxdp.
>>
>> '--disable-afxdp' added to the debian and fedora package builds
>> to keep predictable behavior.
>>
>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>> ---
>
> Looks like the auto-enable is not working on my fedora35 machine (with or without libxdp installed) :)
>
> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --enable-afxdp | grep -i -E "af_xdp|afxdp"
> checking whether AF_XDP is enabled... yes
> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --disable-afxdp | grep -i -E "af_xdp|afxdp"
> checking whether AF_XDP is enabled... no
> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --enable-afxdp=auto | grep -i -E "af_xdp|afxdp"
> checking whether AF_XDP is enabled... no
> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc | grep -i -E "af_xdp|afxdp"
> checking whether AF_XDP is enabled... no

So we need libxdp-devel to be installed even with libbpf 0.6. Though you changes it it would not need it for < 0.7?

>>  Documentation/intro/install/afxdp.rst |  6 ++-
>>  NEWS                                  |  3 ++
>>  acinclude.m4                          | 72 ++++++++++++++++++---------
>>  debian/rules                          | 25 ++++++----
>>  rhel/openvswitch-fedora.spec.in       |  2 +
>>  5 files changed, 72 insertions(+), 36 deletions(-)
>>
>> diff --git a/Documentation/intro/install/afxdp.rst b/Documentation/intro/install/afxdp.rst
>> index a4f0b87fe..51c24bf5b 100644
>> --- a/Documentation/intro/install/afxdp.rst
>> +++ b/Documentation/intro/install/afxdp.rst
>> @@ -30,8 +30,7 @@ This document describes how to build and install Open vSwitch using
>>  AF_XDP netdev.
>>
>>  .. warning::
>> -  The AF_XDP support of Open vSwitch is considered 'experimental',
>> -  and it is not compiled in by default.
>> +  The AF_XDP support of Open vSwitch is considered 'experimental'.
>>
>>
>>  Introduction
>> @@ -137,6 +136,9 @@ bootstrap/configure the package::
>>
>>    ./boot.sh && ./configure --enable-afxdp
>>
>> +``--enable-afxdp`` here is optional, but it will ensure that all dependencies
>> +are available at the build time.
>> +
>>  Finally, build and install OVS::
>>
>>    make && make install
>> diff --git a/NEWS b/NEWS
>> index ce5d11d73..2f6ededfe 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -4,6 +4,9 @@ Post-v3.0.0
>>       of handler and revalidator threads if necessary.
>>     - AF_XDP:
>>       * Added support for building with libxdp and libbpf >= 0.7.
>> +     * Support for AF_XDP is now enabled by default if all dependencies are
>> +       available at the build time.  Use --disable-afxdp to disable.
>> +       Use --enable-afxdp to fail the build if dependencies are not present.
>>     - ovs-appctl:
>>       * "ovs-appctl ofproto/trace" command can now display port names with the
>>         "--names" option.
>> diff --git a/acinclude.m4 b/acinclude.m4
>> index e47e925b3..8aecfb63d 100644
>> --- a/acinclude.m4
>> +++ b/acinclude.m4
>> @@ -253,46 +253,70 @@ dnl OVS_CHECK_LINUX_AF_XDP
>>  dnl
>>  dnl Check both Linux kernel AF_XDP and libbpf/libxdp support
>>  AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [
>> -  AC_ARG_ENABLE([afxdp],
>> -                [AS_HELP_STRING([--enable-afxdp], [Enable AF-XDP support])],
>> -                [], [enable_afxdp=no])
>> +  AC_ARG_ENABLE(
>> +    [afxdp],
>> +    [AS_HELP_STRING([--disable-afxdp], [Disable AF-XDP support])],
>> +    [case "${enableval}" in
>> +       (yes | no | auto) ;;
>> +       (*) AC_MSG_ERROR([bad value ${enableval} for --enable-afxdp]) ;;
>> +     esac],
>> +    [enable_afxdp=auto])
>> +
>>    AC_MSG_CHECKING([whether AF_XDP is enabled])
>> -  if test "$enable_afxdp" != yes; then
>> +  if test "$enable_afxdp" == no; then
>>      AC_MSG_RESULT([no])
>>      AF_XDP_ENABLE=false
>>    else
>> -    AC_MSG_RESULT([yes])
>> +    AC_MSG_RESULT([$enable_afxdp])
>>      AF_XDP_ENABLE=true
>> +    failed_dep=none
>> +    dnl Saving libs to restore in case we will end up not building with AF_XDP.
>> +    save_LIBS=$LIBS
>>
>> -    AC_CHECK_HEADER([bpf/libbpf.h], [],
>> -      [AC_MSG_ERROR([unable to find bpf/libbpf.h for AF_XDP support])])
>> +    AC_CHECK_HEADER([bpf/libbpf.h], [], [failed_dep="bpf/libbpf.h"])
>>
>> -    AC_CHECK_HEADER([linux/if_xdp.h], [],
>> -      [AC_MSG_ERROR([unable to find linux/if_xdp.h for AF_XDP support])])
>> +    if test "$failed_dep" = none; then
>> +      AC_CHECK_HEADER([linux/if_xdp.h], [], [failed_dep="linux/if_xdp.h"])
>> +    fi
>>
>> -    OVS_FIND_DEPENDENCY([libbpf_strerror], [bpf], [libbpf])
>> -    AC_CHECK_FUNCS([bpf_xdp_query_id bpf_xdp_detach])
>> +    if test "$failed_dep" = none; then
>> +      AC_SEARCH_LIBS([libbpf_strerror], [bpf], [], [failed_dep="libbpf"])
>> +      AC_CHECK_FUNCS([bpf_xdp_query_id bpf_xdp_detach])
>> +    fi
>>
>> -    if test "x$ac_cv_func_bpf_xdp_detach" = xyes; then
>> +    if test "$failed_dep" = none -a "x$ac_cv_func_bpf_xdp_detach" = xyes; then
>>          dnl We have libbpf >= 0.7.  Look for libxdp as xsk functions
>>          dnl were moved into this library.
>> -        OVS_FIND_DEPENDENCY([libxdp_strerror], [xdp], [libxdp])
>> -        AC_CHECK_HEADER([xdp/xsk.h],
>> -          AC_DEFINE([HAVE_LIBXDP], [1], [xsk.h is supplied with libxdp]),
>> -          AC_MSG_ERROR([unable to find xdp/xsk.h for AF_XDP support]))
>> -    else
>> +        AC_SEARCH_LIBS([libxdp_strerror], [xdp],
>> +          AC_CHECK_HEADER([xdp/xsk.h],
>> +            AC_DEFINE([HAVE_LIBXDP], [1], [xsk.h is supplied with libxdp]),
>> +            [failed_dep="xdp/xsk.h"]),
>> +          [failed_dep="libxdp"])
>> +    elif test "$failed_dep" = none; then
>>          dnl libbpf < 0.7 contains all the necessary functionality.
>> -        AC_CHECK_HEADER([bpf/xsk.h], [],
>> -          [AC_MSG_ERROR([unable to find bpf/xsk.h for AF_XDP support])])
>> +        AC_CHECK_HEADER([bpf/xsk.h], [], [failed_dep="bpf/xsk.h"])
>>      fi
>>
>> -    AC_CHECK_FUNCS([pthread_spin_lock], [],
>> -      [AC_MSG_ERROR([unable to find pthread_spin_lock for AF_XDP support])])
>> +    if test "$failed_dep" = none; then
>> +      AC_CHECK_FUNCS([pthread_spin_lock], [], [failed_dep="pthread_spin_lock"])
>> +    fi
>>
>> -    OVS_FIND_DEPENDENCY([numa_alloc_onnode], [numa], [libnuma])
>> +    if test "$failed_dep" = none; then
>> +      AC_SEARCH_LIBS([numa_alloc_onnode], [numa], [], [failed_dep="libnuma"])
>> +    fi
>>
>> -    AC_DEFINE([HAVE_AF_XDP], [1],
>> -              [Define to 1 if AF_XDP support is available and enabled.])
>> +    if test "$failed_dep" = none; then
>> +      AC_DEFINE([HAVE_AF_XDP], [1],
>> +                [Define to 1 if AF_XDP support is available and enabled.])
>> +    elif test "$enable_afxdp" = yes; then
>> +      AC_MSG_ERROR([Missing $failed_dep dependency for AF_XDP support])
>> +    else
>> +      AC_MSG_WARN(m4_normalize(
>> +          [Cannot find $failed_dep, netdev-afxdp will not be supported
>> +           (use --disable-afxdp to suppress this warning).]))
>> +      AF_XDP_ENABLE=false
>> +      LIBS=$save_LIBS
>> +    fi
>>    fi
>>    AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true)
>>  ])
>> diff --git a/debian/rules b/debian/rules
>> index 971bc1775..ddbd4dc5c 100755
>> --- a/debian/rules
>> +++ b/debian/rules
>> @@ -23,21 +23,26 @@ override_dh_auto_configure:
>>  	test -d _debian || mkdir _debian
>>  	cd _debian && ( \
>>  		test -e Makefile || \
>> -		../configure --prefix=/usr --localstatedir=/var --enable-ssl \
>> -					 --sysconfdir=/etc \
>> -					 $(DATAPATH_CONFIGURE_OPTS) \
>> -					 $(EXTRA_CONFIGURE_OPTS) \
>> -					 )
>> +		../configure --prefix=/usr --localstatedir=/var \
>> +					--enable-ssl \
>> +					--disable-afxdp \
>> +					--sysconfdir=/etc \
>> +					$(DATAPATH_CONFIGURE_OPTS) \
>> +					$(EXTRA_CONFIGURE_OPTS) \
>> +					)
>>  ifneq (,$(filter i386 amd64 ppc64el arm64, $(DEB_HOST_ARCH)))
>>  ifeq (,$(filter nodpdk, $(DEB_BUILD_OPTIONS)))
>>  	test -d _dpdk || mkdir _dpdk
>>  	cd _dpdk && ( \
>>  		test -e Makefile || \
>> -        ../configure --prefix=/usr --localstatedir=/var --enable-ssl \
>> -                     --with-dpdk=shared --sysconfdir=/etc \
>> -					 $(DATAPATH_CONFIGURE_OPTS) \
>> -					 $(EXTRA_CONFIGURE_OPTS) \
>> -					 )
>> +		../configure --prefix=/usr --localstatedir=/var \
>> +					--enable-ssl \
>> +					--disable-afxdp \
>> +					--with-dpdk=shared \
>> +					--sysconfdir=/etc \
>> +					$(DATAPATH_CONFIGURE_OPTS) \
>> +					$(EXTRA_CONFIGURE_OPTS) \
>> +					)
>>  endif
>>  endif
>>
>> diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
>> index eb5077a21..3091e204e 100644
>> --- a/rhel/openvswitch-fedora.spec.in
>> +++ b/rhel/openvswitch-fedora.spec.in
>> @@ -171,6 +171,8 @@ This package provides IPsec tunneling support for OVS tunnels.
>>  %endif
>>  %if %{with afxdp}
>>          --enable-afxdp \
>> +%else
>> +        --disable-afxdp \
>>  %endif
>>          --enable-ssl \
>>          --disable-static \
>> -- 
>> 2.38.1
Eelco Chaudron Dec. 22, 2022, 3:21 p.m. UTC | #3 
On 22 Dec 2022, at 16:09, Eelco Chaudron wrote:

> On 22 Dec 2022, at 16:04, Eelco Chaudron wrote:
>
>> On 22 Dec 2022, at 1:06, Ilya Maximets wrote:
>>
>>> With this change we will try to detect all the netdev-afxdp
>>> dependencies and enable AF_XDP support by default if they are
>>> present at the build time.
>>>
>>> Configuration script behaves in a following way:
>>>
>>>  - ./configure --enable-afxdp
>>>
>>>    Will check for AF_XDP dependencies and fail if they are
>>>    not available.
>>>
>>>  - ./configure --disable-afxdp
>>>
>>>    Disables checking for AF_XDP.  Build will not support
>>>    AF_XDP even if all dependencies are installed.
>>>
>>>  - Just ./configure or ./configure --enable-afxdp=auto
>>>
>>>    Will check for AF_XDP dependencies.  Will print a warning
>>>    if they are not available, but will continue without AF_XDP
>>>    support.  If dependencies are available in a system, this
>>>    option is equal to --enable-afxdp.
>>>
>>> '--disable-afxdp' added to the debian and fedora package builds
>>> to keep predictable behavior.
>>>
>>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>>> ---
>>
>> Looks like the auto-enable is not working on my fedora35 machine (with or without libxdp installed) :)
>>
>> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --enable-afxdp | grep -i -E "af_xdp|afxdp"
>> checking whether AF_XDP is enabled... yes
>> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --disable-afxdp | grep -i -E "af_xdp|afxdp"
>> checking whether AF_XDP is enabled... no
>> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --enable-afxdp=auto | grep -i -E "af_xdp|afxdp"
>> checking whether AF_XDP is enabled... no
>> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc | grep -i -E "af_xdp|afxdp"
>> checking whether AF_XDP is enabled... no
>
> So we need libxdp-devel to be installed even with libbpf 0.6. Though you changes it it would not need it for < 0.7?
>

Ignore all of this :) I was on the v2 branch when testing this patch :(

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Ilya Maximets Jan. 3, 2023, 3:09 p.m. UTC | #4 
On 12/22/22 16:21, Eelco Chaudron wrote:
> 
> 
> On 22 Dec 2022, at 16:09, Eelco Chaudron wrote:
> 
>> On 22 Dec 2022, at 16:04, Eelco Chaudron wrote:
>>
>>> On 22 Dec 2022, at 1:06, Ilya Maximets wrote:
>>>
>>>> With this change we will try to detect all the netdev-afxdp
>>>> dependencies and enable AF_XDP support by default if they are
>>>> present at the build time.
>>>>
>>>> Configuration script behaves in a following way:
>>>>
>>>>  - ./configure --enable-afxdp
>>>>
>>>>    Will check for AF_XDP dependencies and fail if they are
>>>>    not available.
>>>>
>>>>  - ./configure --disable-afxdp
>>>>
>>>>    Disables checking for AF_XDP.  Build will not support
>>>>    AF_XDP even if all dependencies are installed.
>>>>
>>>>  - Just ./configure or ./configure --enable-afxdp=auto
>>>>
>>>>    Will check for AF_XDP dependencies.  Will print a warning
>>>>    if they are not available, but will continue without AF_XDP
>>>>    support.  If dependencies are available in a system, this
>>>>    option is equal to --enable-afxdp.
>>>>
>>>> '--disable-afxdp' added to the debian and fedora package builds
>>>> to keep predictable behavior.
>>>>
>>>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>>>> ---
>>>
>>> Looks like the auto-enable is not working on my fedora35 machine (with or without libxdp installed) :)
>>>
>>> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --enable-afxdp | grep -i -E "af_xdp|afxdp"
>>> checking whether AF_XDP is enabled... yes
>>> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --disable-afxdp | grep -i -E "af_xdp|afxdp"
>>> checking whether AF_XDP is enabled... no
>>> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc --enable-afxdp=auto | grep -i -E "af_xdp|afxdp"
>>> checking whether AF_XDP is enabled... no
>>> [ebuild:~/...eview/ovs_ilya_afxdp]$ ./configure --enable-Werror --sysconfdir=/etc | grep -i -E "af_xdp|afxdp"
>>> checking whether AF_XDP is enabled... no
>>
>> So we need libxdp-devel to be installed even with libbpf 0.6. Though you changes it it would not need it for < 0.7?
>>
> 
> Ignore all of this :) I was on the v2 branch when testing this patch :(
> 
> Acked-by: Eelco Chaudron <echaudro@redhat.com>
> 


Thanks!  I applied the set now.
The first CI fix additionally backported down to 2.13.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/Documentation/intro/install/afxdp.rst b/Documentation/intro/install/afxdp.rst
index a4f0b87fe..51c24bf5b 100644
--- a/Documentation/intro/install/afxdp.rst
+++ b/Documentation/intro/install/afxdp.rst
@@ -30,8 +30,7 @@  This document describes how to build and install Open vSwitch using
 AF_XDP netdev.
 
 .. warning::
-  The AF_XDP support of Open vSwitch is considered 'experimental',
-  and it is not compiled in by default.
+  The AF_XDP support of Open vSwitch is considered 'experimental'.
 
 
 Introduction
@@ -137,6 +136,9 @@  bootstrap/configure the package::
 
   ./boot.sh && ./configure --enable-afxdp
 
+``--enable-afxdp`` here is optional, but it will ensure that all dependencies
+are available at the build time.
+
 Finally, build and install OVS::
 
   make && make install
diff --git a/NEWS b/NEWS
index ce5d11d73..2f6ededfe 100644
--- a/NEWS
+++ b/NEWS
@@ -4,6 +4,9 @@  Post-v3.0.0
      of handler and revalidator threads if necessary.
    - AF_XDP:
      * Added support for building with libxdp and libbpf >= 0.7.
+     * Support for AF_XDP is now enabled by default if all dependencies are
+       available at the build time.  Use --disable-afxdp to disable.
+       Use --enable-afxdp to fail the build if dependencies are not present.
    - ovs-appctl:
      * "ovs-appctl ofproto/trace" command can now display port names with the
        "--names" option.
diff --git a/acinclude.m4 b/acinclude.m4
index e47e925b3..8aecfb63d 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -253,46 +253,70 @@  dnl OVS_CHECK_LINUX_AF_XDP
 dnl
 dnl Check both Linux kernel AF_XDP and libbpf/libxdp support
 AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [
-  AC_ARG_ENABLE([afxdp],
-                [AS_HELP_STRING([--enable-afxdp], [Enable AF-XDP support])],
-                [], [enable_afxdp=no])
+  AC_ARG_ENABLE(
+    [afxdp],
+    [AS_HELP_STRING([--disable-afxdp], [Disable AF-XDP support])],
+    [case "${enableval}" in
+       (yes | no | auto) ;;
+       (*) AC_MSG_ERROR([bad value ${enableval} for --enable-afxdp]) ;;
+     esac],
+    [enable_afxdp=auto])
+
   AC_MSG_CHECKING([whether AF_XDP is enabled])
-  if test "$enable_afxdp" != yes; then
+  if test "$enable_afxdp" == no; then
     AC_MSG_RESULT([no])
     AF_XDP_ENABLE=false
   else
-    AC_MSG_RESULT([yes])
+    AC_MSG_RESULT([$enable_afxdp])
     AF_XDP_ENABLE=true
+    failed_dep=none
+    dnl Saving libs to restore in case we will end up not building with AF_XDP.
+    save_LIBS=$LIBS
 
-    AC_CHECK_HEADER([bpf/libbpf.h], [],
-      [AC_MSG_ERROR([unable to find bpf/libbpf.h for AF_XDP support])])
+    AC_CHECK_HEADER([bpf/libbpf.h], [], [failed_dep="bpf/libbpf.h"])
 
-    AC_CHECK_HEADER([linux/if_xdp.h], [],
-      [AC_MSG_ERROR([unable to find linux/if_xdp.h for AF_XDP support])])
+    if test "$failed_dep" = none; then
+      AC_CHECK_HEADER([linux/if_xdp.h], [], [failed_dep="linux/if_xdp.h"])
+    fi
 
-    OVS_FIND_DEPENDENCY([libbpf_strerror], [bpf], [libbpf])
-    AC_CHECK_FUNCS([bpf_xdp_query_id bpf_xdp_detach])
+    if test "$failed_dep" = none; then
+      AC_SEARCH_LIBS([libbpf_strerror], [bpf], [], [failed_dep="libbpf"])
+      AC_CHECK_FUNCS([bpf_xdp_query_id bpf_xdp_detach])
+    fi
 
-    if test "x$ac_cv_func_bpf_xdp_detach" = xyes; then
+    if test "$failed_dep" = none -a "x$ac_cv_func_bpf_xdp_detach" = xyes; then
         dnl We have libbpf >= 0.7.  Look for libxdp as xsk functions
         dnl were moved into this library.
-        OVS_FIND_DEPENDENCY([libxdp_strerror], [xdp], [libxdp])
-        AC_CHECK_HEADER([xdp/xsk.h],
-          AC_DEFINE([HAVE_LIBXDP], [1], [xsk.h is supplied with libxdp]),
-          AC_MSG_ERROR([unable to find xdp/xsk.h for AF_XDP support]))
-    else
+        AC_SEARCH_LIBS([libxdp_strerror], [xdp],
+          AC_CHECK_HEADER([xdp/xsk.h],
+            AC_DEFINE([HAVE_LIBXDP], [1], [xsk.h is supplied with libxdp]),
+            [failed_dep="xdp/xsk.h"]),
+          [failed_dep="libxdp"])
+    elif test "$failed_dep" = none; then
         dnl libbpf < 0.7 contains all the necessary functionality.
-        AC_CHECK_HEADER([bpf/xsk.h], [],
-          [AC_MSG_ERROR([unable to find bpf/xsk.h for AF_XDP support])])
+        AC_CHECK_HEADER([bpf/xsk.h], [], [failed_dep="bpf/xsk.h"])
     fi
 
-    AC_CHECK_FUNCS([pthread_spin_lock], [],
-      [AC_MSG_ERROR([unable to find pthread_spin_lock for AF_XDP support])])
+    if test "$failed_dep" = none; then
+      AC_CHECK_FUNCS([pthread_spin_lock], [], [failed_dep="pthread_spin_lock"])
+    fi
 
-    OVS_FIND_DEPENDENCY([numa_alloc_onnode], [numa], [libnuma])
+    if test "$failed_dep" = none; then
+      AC_SEARCH_LIBS([numa_alloc_onnode], [numa], [], [failed_dep="libnuma"])
+    fi
 
-    AC_DEFINE([HAVE_AF_XDP], [1],
-              [Define to 1 if AF_XDP support is available and enabled.])
+    if test "$failed_dep" = none; then
+      AC_DEFINE([HAVE_AF_XDP], [1],
+                [Define to 1 if AF_XDP support is available and enabled.])
+    elif test "$enable_afxdp" = yes; then
+      AC_MSG_ERROR([Missing $failed_dep dependency for AF_XDP support])
+    else
+      AC_MSG_WARN(m4_normalize(
+          [Cannot find $failed_dep, netdev-afxdp will not be supported
+           (use --disable-afxdp to suppress this warning).]))
+      AF_XDP_ENABLE=false
+      LIBS=$save_LIBS
+    fi
   fi
   AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true)
 ])
diff --git a/debian/rules b/debian/rules
index 971bc1775..ddbd4dc5c 100755
--- a/debian/rules
+++ b/debian/rules
@@ -23,21 +23,26 @@  override_dh_auto_configure:
 	test -d _debian || mkdir _debian
 	cd _debian && ( \
 		test -e Makefile || \
-		../configure --prefix=/usr --localstatedir=/var --enable-ssl \
-					 --sysconfdir=/etc \
-					 $(DATAPATH_CONFIGURE_OPTS) \
-					 $(EXTRA_CONFIGURE_OPTS) \
-					 )
+		../configure --prefix=/usr --localstatedir=/var \
+					--enable-ssl \
+					--disable-afxdp \
+					--sysconfdir=/etc \
+					$(DATAPATH_CONFIGURE_OPTS) \
+					$(EXTRA_CONFIGURE_OPTS) \
+					)
 ifneq (,$(filter i386 amd64 ppc64el arm64, $(DEB_HOST_ARCH)))
 ifeq (,$(filter nodpdk, $(DEB_BUILD_OPTIONS)))
 	test -d _dpdk || mkdir _dpdk
 	cd _dpdk && ( \
 		test -e Makefile || \
-        ../configure --prefix=/usr --localstatedir=/var --enable-ssl \
-                     --with-dpdk=shared --sysconfdir=/etc \
-					 $(DATAPATH_CONFIGURE_OPTS) \
-					 $(EXTRA_CONFIGURE_OPTS) \
-					 )
+		../configure --prefix=/usr --localstatedir=/var \
+					--enable-ssl \
+					--disable-afxdp \
+					--with-dpdk=shared \
+					--sysconfdir=/etc \
+					$(DATAPATH_CONFIGURE_OPTS) \
+					$(EXTRA_CONFIGURE_OPTS) \
+					)
 endif
 endif
 
diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in
index eb5077a21..3091e204e 100644
--- a/rhel/openvswitch-fedora.spec.in
+++ b/rhel/openvswitch-fedora.spec.in
@@ -171,6 +171,8 @@  This package provides IPsec tunneling support for OVS tunnels.
 %endif
 %if %{with afxdp}
         --enable-afxdp \
+%else
+        --disable-afxdp \
 %endif
         --enable-ssl \
         --disable-static \