On 12/20/22 14:19, Eelco Chaudron wrote:
> 
> 
> On 19 Dec 2022, at 13:20, 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, except that AF_XDP will
>>    not be enabled for libbpf >= 0.7 if libxdp is not available,
>>    to avoid deprecation warnings during the build.
>>
>> '--disable-afxdp' added to the debian and fedora package builds
>> to keep predictable behavior.
>>
>> Signed-off-by: Ilya Maximets <i.maxim...@ovn.org>
> 
> I still don’t like building AF_XDP automatically, but looks like I’m the only 
> one ;)
> 
>> ---
>>  Documentation/intro/install/afxdp.rst |  6 +-
>>  NEWS                                  |  3 +
>>  acinclude.m4                          | 89 ++++++++++++++++++---------
>>  debian/rules                          | 25 +++++---
>>  rhel/openvswitch-fedora.spec.in       |  2 +
>>  5 files changed, 85 insertions(+), 40 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 5d39c7d27..d2bbae591 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -2,6 +2,9 @@ Post-v3.0.0
>>  --------------------
>>     - 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 aed01c967..8411c0e6c 100644
>> --- a/acinclude.m4
>> +++ b/acinclude.m4
>> @@ -253,39 +253,72 @@ 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
>> -
>> -    AC_CHECK_HEADER([bpf/libbpf.h], [],
>> -      [AC_MSG_ERROR([unable to find bpf/libbpf.h for AF_XDP support])])
>> -
>> -    AC_CHECK_HEADER([linux/if_xdp.h], [],
>> -      [AC_MSG_ERROR([unable to find linux/if_xdp.h for AF_XDP support])])
>> -
>> -    AC_CHECK_HEADER([xdp/xsk.h],
>> -      AC_DEFINE([HAVE_LIBXDP], [1], [xsk.h is supplied with libxdp]),
>> -      AC_CHECK_HEADER([bpf/xsk.h], [],
>> -        [AC_MSG_ERROR([unable to find xsk.h for AF_XDP support])]))
>> -
>> -    AC_CHECK_FUNCS([pthread_spin_lock], [],
>> -      [AC_MSG_ERROR([unable to find pthread_spin_lock for AF_XDP support])])
>> -
>> -    OVS_FIND_DEPENDENCY([numa_alloc_onnode], [numa], [libnuma])
>> -    OVS_FIND_DEPENDENCY([libbpf_strerror], [bpf], [libbpf])
>> -    AC_SEARCH_LIBS([libxdp_strerror], [xdp])
>> -
>> -    AC_CHECK_FUNCS([bpf_xdp_query_id bpf_xdp_detach])
>> -
>> -    AC_DEFINE([HAVE_AF_XDP], [1],
>> -              [Define to 1 if AF_XDP support is available and enabled.])
>> +    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], [], [failed_dep="bpf/libbpf.h"])
>> +
>> +    if test "$failed_dep" = none; then
>> +      AC_CHECK_HEADER([linux/if_xdp.h], [], [failed_dep="linux/if_xdp.h"])
>> +    fi
>> +
>> +    if test "$failed_dep" = none; then
>> +      AC_CHECK_HEADER([xdp/xsk.h],
>> +        AC_DEFINE([HAVE_LIBXDP], [1], [xsk.h is supplied with libxdp]),
>> +        AC_CHECK_HEADER([bpf/xsk.h], [], [failed_dep="xsk.h"]))
>> +    fi
>> +
>> +    if test "$failed_dep" = none; then
>> +      AC_CHECK_FUNCS([pthread_spin_lock], [], 
>> [failed_dep="pthread_spin_lock"])
>> +    fi
>> +
>> +    if test "$failed_dep" = none; then
>> +      AC_SEARCH_LIBS([numa_alloc_onnode], [numa], [], 
>> [failed_dep="libnuma"])
>> +    fi
>> +    if test "$failed_dep" = none; then
>> +      AC_SEARCH_LIBS([libbpf_strerror], [bpf], [], [failed_dep="libbpf"])
>> +    fi
>> +
>> +    if test "$failed_dep" = none; then
>> +      AC_CHECK_FUNCS([bpf_xdp_query_id bpf_xdp_detach])
>> +      AC_SEARCH_LIBS([libxdp_strerror], [xdp], [],
>> +        [dnl Fail the auto discovery if we have libbpf >= 0.7 but not libxdp
>> +         dnl to avoid deprecation warnings during the build.
>> +         if test "$enable_afxdp" = auto -a "x$ac_cv_func_bpf_xdp_detach" = 
>> xyes; then
>> +            failed_dep="libxdp"
>> +         fi
>> +        ])
>> +    fi
>> +
>> +    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
>> +           (you may 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 6564d5252..fbfcdcf63 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
> 
> Now that we build with libxdp, should we also not add some requirements for 
> packages to be installed?
> 
> I do not think we link statically to libxdp/libbpf?

I thought so as well, but it looks like rpm is smart to figure
out on its own which libraries the binary is linked with.  So,
it adds all required runtime dependencies automatically.
Kind of magic.

Otherwise, we already have a lot of missing runtime dependencies
in this spec.

> 
>>          --enable-ssl \
>>          --disable-static \
>> -- 
>> 2.38.1
> 

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to