On 19 Dec 2022, at 13:20, Ilya Maximets wrote:
> AF_XDP functions was deprecated in libbpf 0.7 and moved to libxdp. > Functions bpf_get/set_link_xdp_id() was deprecated in libbpf 0.8 > and replaced with bpf_xdp_query_id() and bpf_xdp_attach/detach(). > > Updating configuration and source code to accommodate above changes > and allow building OVS with AF_XDP support on newer systems: > > - Checking availability of the libxdp in a system by looking > for a library providing libxdp_strerror(). > > - Checking for xsk.h header provided by libxdp-dev[el] first, > fall back to xsk.h from libbpf if not found. > > - Check for the NEED_WAKEUP feature replaced with direct checking > in the source code if XDP_USE_NEED_WAKEUP is defined. > > - Checking availability of bpf_xdp_query_id and bpf_xdp_detach > and using them instead of deprecated APIs. Fall back to old > functions if not found. So I guess this requires our build environment to match our runtime environment, as these functions are from dynamic libraries, not statically linked? I guess this is find, as long as people understand it. > > - Dropped LIBBPF_LDADD variable as it makes library and function > detection much harder without providing any actual benefits. > AC_SEARCH_LIBS is used instead and it allows use of AC_CHECK_FUNCS. > > - Header includes moved around to files where they are actually used. > > - Removed libelf dependency as it is not really used. > > With these changes it should be possible to build OVS with either: > > - libbpf built from the kernel sources (5.19 or older). > - libbpf < 0.7 provided in distributions. > - libxdp and libbpf >= 0.7 provided in newer distributions. > > libxdp added as a build dependency for Fedora build since all > supported versions of Fedora are packaging this library. > > Signed-off-by: Ilya Maximets <[email protected]> I have problems building this on my fedora35 system with gcc-11.3.1-3.fc35.x86_64: libtool: link: ( cd "include/openvswitch/.libs" && rm -f "libcxxtest.la" && ln -s "../libcxxtest.la" "libcxxtest.la" ) In file included from lib/netdev-linux-private.h:30, from lib/netdev-afxdp.c:19: In function ‘dp_packet_delete’, inlined from ‘dp_packet_delete’ at lib/dp-packet.h:246:1, inlined from ‘dp_packet_batch_add__’ at lib/dp-packet.h:775:9, inlined from ‘dp_packet_batch_add’ at lib/dp-packet.h:783:5, inlined from ‘netdev_afxdp_rxq_recv’ at lib/netdev-afxdp.c:894:9: lib/dp-packet.h:260:9: error: ‘free’ called on pointer ‘*umem.xpool.array’ with nonzero offset [8, 2558044588346441168] [-Werror=free-nonheap-object] 260 | free(b); | ^~~~~~~ Guess it does not recognise the (b->source == DPBUF_AFXDP) statement… This is my build config: ./configure --enable-Werror --enable-usdt-probes --localstatedir=/var --prefix=/usr --sysconfdir=/etc --enable-afxdp Guess this should be fixed before we enable afxdp by default? Also when I build it without the Werror option I’m not able to start a sandbox: make[1]: Leaving directory '/home/echaudron/Documents/review/ovs_ilya_afxdp' ovsdb-tool create conf.db /home/echaudron/Documents/review/ovs_ilya_afxdp/vswitchd/vswitch.ovsschema ovsdb-tool: symbol lookup error: /lib64/libxdp.so.1: undefined symbol: silence_libbpf_logging cat: '/home/echaudron/Documents/review/ovs_ilya_afxdp/tutorial/sandbox/*.pid': No such file or directory But this might be something specific to libxdp on my system, and libbpf :( > --- > NEWS | 2 ++ > acinclude.m4 | 21 +++++++++--------- > lib/automake.mk | 1 - > lib/libopenvswitch.pc.in | 2 +- > lib/netdev-afxdp-pool.c | 2 ++ > lib/netdev-afxdp-pool.h | 5 ----- > lib/netdev-afxdp.c | 38 ++++++++++++++++++++++++++------- > rhel/openvswitch-fedora.spec.in | 2 +- > 8 files changed, 46 insertions(+), 27 deletions(-) > > diff --git a/NEWS b/NEWS > index 265375e1c..5d39c7d27 100644 > --- a/NEWS > +++ b/NEWS > @@ -1,5 +1,7 @@ > Post-v3.0.0 > -------------------- > + - AF_XDP: > + * Added support for building with libxdp and libbpf >= 0.7. > - 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 aa9af5506..aed01c967 100644 > --- a/acinclude.m4 > +++ b/acinclude.m4 > @@ -251,7 +251,7 @@ AC_DEFUN([OVS_FIND_DEPENDENCY], [ > > dnl OVS_CHECK_LINUX_AF_XDP > dnl > -dnl Check both Linux kernel AF_XDP and libbpf support > +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])], > @@ -270,23 +270,22 @@ AC_DEFUN([OVS_CHECK_LINUX_AF_XDP], [ > AC_CHECK_HEADER([linux/if_xdp.h], [], > [AC_MSG_ERROR([unable to find linux/if_xdp.h for AF_XDP support])]) > > - AC_CHECK_HEADER([bpf/xsk.h], [], > - [AC_MSG_ERROR([unable to find bpf/xsk.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.]) > - LIBBPF_LDADD=" -lbpf -lelf" > - AC_SUBST([LIBBPF_LDADD]) > - > - AC_CHECK_DECL([xsk_ring_prod__needs_wakeup], [ > - AC_DEFINE([HAVE_XDP_NEED_WAKEUP], [1], > - [XDP need wakeup support detected in xsk.h.]) > - ], [], [[#include <bpf/xsk.h>]]) > fi > AM_CONDITIONAL([HAVE_AF_XDP], test "$AF_XDP_ENABLE" = true) > ]) > @@ -357,7 +356,7 @@ AC_DEFUN([OVS_CHECK_DPDK], [ > ], [], [[#include <rte_config.h>]]) > > AC_CHECK_DECL([RTE_NET_AF_XDP], [ > - LIBBPF_LDADD="-lbpf" > + OVS_FIND_DEPENDENCY([libbpf_strerror], [bpf], [libbpf]) > ], [], [[#include <rte_config.h>]]) > > AC_CHECK_DECL([RTE_LIBRTE_VHOST_NUMA], [ > diff --git a/lib/automake.mk b/lib/automake.mk > index a0fabe38f..61bdc308f 100644 > --- a/lib/automake.mk > +++ b/lib/automake.mk > @@ -9,7 +9,6 @@ lib_LTLIBRARIES += lib/libopenvswitch.la > > lib_libopenvswitch_la_LIBADD = $(SSL_LIBS) > lib_libopenvswitch_la_LIBADD += $(CAPNG_LDADD) > -lib_libopenvswitch_la_LIBADD += $(LIBBPF_LDADD) > > > if WIN32 > diff --git a/lib/libopenvswitch.pc.in b/lib/libopenvswitch.pc.in > index 44fbb1f9f..a5f4d3947 100644 > --- a/lib/libopenvswitch.pc.in > +++ b/lib/libopenvswitch.pc.in > @@ -7,5 +7,5 @@ Name: libopenvswitch > Description: Open vSwitch library > Version: @VERSION@ > Libs: -L${libdir} -lopenvswitch > -Libs.private: @LIBS@ @SSL_LIBS@ @CAPNG_LDADD@ @LIBBPF_LDADD@ > +Libs.private: @LIBS@ @SSL_LIBS@ @CAPNG_LDADD@ > Cflags: -I${includedir} > diff --git a/lib/netdev-afxdp-pool.c b/lib/netdev-afxdp-pool.c > index 3386d2dcf..f56a7b29e 100644 > --- a/lib/netdev-afxdp-pool.c > +++ b/lib/netdev-afxdp-pool.c > @@ -15,6 +15,8 @@ > */ > #include <config.h> > Remove new line here? > +#include <errno.h> > + > #include "dp-packet.h" > #include "netdev-afxdp-pool.h" > #include "openvswitch/util.h" > diff --git a/lib/netdev-afxdp-pool.h b/lib/netdev-afxdp-pool.h > index f929b9489..6681cf539 100644 > --- a/lib/netdev-afxdp-pool.h > +++ b/lib/netdev-afxdp-pool.h > @@ -19,12 +19,7 @@ > > #ifdef HAVE_AF_XDP > > -#include <bpf/xsk.h> > -#include <errno.h> > -#include <stdbool.h> > - > #include "openvswitch/thread.h" > -#include "ovs-atomic.h" > > /* LIFO ptr_array. */ > struct umem_pool { > diff --git a/lib/netdev-afxdp.c b/lib/netdev-afxdp.c > index ca3f2431e..6ced8a2b6 100644 > --- a/lib/netdev-afxdp.c > +++ b/lib/netdev-afxdp.c > @@ -21,6 +21,11 @@ > #include "netdev-afxdp.h" > #include "netdev-afxdp-pool.h" > > +#ifdef HAVE_LIBXDP > +#include <xdp/xsk.h> > +#else > +#include <bpf/xsk.h> > +#endif > #include <errno.h> > #include <inttypes.h> > #include <linux/rtnetlink.h> > @@ -29,6 +34,7 @@ > #include <numa.h> > #include <numaif.h> > #include <poll.h> > +#include <stdbool.h> > #include <stdlib.h> > #include <sys/resource.h> > #include <sys/socket.h> > @@ -44,6 +50,7 @@ > #include "openvswitch/list.h" > #include "openvswitch/thread.h" > #include "openvswitch/vlog.h" > +#include "ovs-atomic.h" > #include "ovs-numa.h" > #include "packets.h" > #include "socket-util.h" > @@ -72,7 +79,7 @@ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, > 20); > #define PROD_NUM_DESCS XSK_RING_PROD__DEFAULT_NUM_DESCS > #define CONS_NUM_DESCS XSK_RING_CONS__DEFAULT_NUM_DESCS > > -#ifdef HAVE_XDP_NEED_WAKEUP > +#ifdef XDP_USE_NEED_WAKEUP > #define NEED_WAKEUP_DEFAULT true > #else > #define NEED_WAKEUP_DEFAULT false > @@ -169,7 +176,7 @@ struct netdev_afxdp_tx_lock { > ); > }; > > -#ifdef HAVE_XDP_NEED_WAKEUP > +#ifdef XDP_USE_NEED_WAKEUP > static inline void > xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem, > struct netdev *netdev, int fd) > @@ -201,7 +208,7 @@ xsk_tx_need_wakeup(struct xsk_socket_info *xsk_info) > return xsk_ring_prod__needs_wakeup(&xsk_info->tx); > } > > -#else /* !HAVE_XDP_NEED_WAKEUP */ > +#else /* !XDP_USE_NEED_WAKEUP */ > static inline void > xsk_rx_wakeup_if_needed(struct xsk_umem_info *umem OVS_UNUSED, > struct netdev *netdev OVS_UNUSED, > @@ -215,7 +222,7 @@ xsk_tx_need_wakeup(struct xsk_socket_info *xsk_info > OVS_UNUSED) > { > return true; > } > -#endif /* HAVE_XDP_NEED_WAKEUP */ > +#endif /* XDP_USE_NEED_WAKEUP */ > > static void > netdev_afxdp_cleanup_unused_pool(struct unused_pool *pool) > @@ -351,7 +358,7 @@ xsk_configure_socket(struct xsk_umem_info *umem, uint32_t > ifindex, > cfg.bind_flags = xdp_modes[mode].bind_flags; > cfg.xdp_flags = xdp_modes[mode].xdp_flags | XDP_FLAGS_UPDATE_IF_NOEXIST; > > -#ifdef HAVE_XDP_NEED_WAKEUP > +#ifdef XDP_USE_NEED_WAKEUP > if (use_need_wakeup) { > cfg.bind_flags |= XDP_USE_NEED_WAKEUP; > } > @@ -377,7 +384,11 @@ xsk_configure_socket(struct xsk_umem_info *umem, > uint32_t ifindex, > } > > /* Make sure the built-in AF_XDP program is loaded. */ > +#ifdef HAVE_BPF_XDP_QUERY_ID > + ret = bpf_xdp_query_id(ifindex, cfg.xdp_flags, &prog_id); > +#else > ret = bpf_get_link_xdp_id(ifindex, &prog_id, cfg.xdp_flags); > +#endif > if (ret || !prog_id) { > if (ret) { > VLOG_ERR("Get XDP prog ID failed (%s)", ovs_strerror(errno)); > @@ -630,9 +641,9 @@ netdev_afxdp_set_config(struct netdev *netdev, const > struct smap *args, > } > > need_wakeup = smap_get_bool(args, "use-need-wakeup", > NEED_WAKEUP_DEFAULT); > -#ifndef HAVE_XDP_NEED_WAKEUP > +#ifndef XDP_USE_NEED_WAKEUP > if (need_wakeup) { > - VLOG_WARN("XDP need_wakeup is not supported in libbpf."); > + VLOG_WARN("XDP need_wakeup is not supported in libbpf/libxdp."); > need_wakeup = false; > } > #endif > @@ -742,7 +753,11 @@ xsk_remove_xdp_program(uint32_t ifindex, enum afxdp_mode > mode) > uint32_t ret, prog_id = 0; > > /* Check whether XDP program is loaded. */ > +#ifdef HAVE_BPF_XDP_QUERY_ID > + ret = bpf_xdp_query_id(ifindex, flags, &prog_id); > +#else > ret = bpf_get_link_xdp_id(ifindex, &prog_id, flags); > +#endif > if (ret) { > VLOG_ERR("Failed to get XDP prog id (%s)", ovs_strerror(errno)); > return; > @@ -753,7 +768,14 @@ xsk_remove_xdp_program(uint32_t ifindex, enum afxdp_mode > mode) > return; > } > > - bpf_set_link_xdp_fd(ifindex, -1, flags); > +#ifdef HAVE_BPF_XDP_DETACH > + if (bpf_xdp_detach(ifindex, flags, NULL) != 0) { > +#else > + if (bpf_set_link_xdp_fd(ifindex, -1, flags) != 0) { > +#endif > + VLOG_ERR("Failed to detach XDP program (%s) at ifindex %d", > + ovs_strerror(errno), ifindex); > + } > } > > void > diff --git a/rhel/openvswitch-fedora.spec.in b/rhel/openvswitch-fedora.spec.in > index 4a3e6294b..6564d5252 100644 > --- a/rhel/openvswitch-fedora.spec.in > +++ b/rhel/openvswitch-fedora.spec.in > @@ -75,7 +75,7 @@ BuildRequires: dpdk-devel >= 22.11 > Provides: %{name}-dpdk = %{version}-%{release} > %endif > %if %{with afxdp} > -BuildRequires: libbpf-devel numactl-devel > +BuildRequires: libxdp-devel libbpf-devel numactl-devel > %endif > BuildRequires: unbound unbound-devel > > -- > 2.38.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
