On 20 Dec 2022, at 14:24, Ilya Maximets wrote:
> 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 <[email protected]> >> >> 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. Nice :) Then the series looks good to me… Acked-by: Eelco Chaudron <[email protected]> >> >>> --enable-ssl \ >>> --disable-static \ >>> -- >>> 2.38.1 >> _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
