netdev - Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87tv7qpdbt.fsf@toke.dk>
Date:   Wed, 30 Oct 2019 14:33:58 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Magnus Karlsson <magnus.karlsson@...el.com>,
        magnus.karlsson@...el.com, bjorn.topel@...el.com, ast@...nel.org,
        daniel@...earbox.net, netdev@...r.kernel.org,
        jonathan.lemon@...il.com
Cc:     bpf@...r.kernel.org, degeneloy@...il.com, john.fastabend@...il.com
Subject: Re: [PATCH bpf-next v3] libbpf: fix compatibility for kernels without need_wakeup

Magnus Karlsson <magnus.karlsson@...el.com> writes:

> When the need_wakeup flag was added to AF_XDP, the format of the
> XDP_MMAP_OFFSETS getsockopt was extended. Code was added to the
> kernel to take care of compatibility issues arrising from running
> applications using any of the two formats. However, libbpf was
> not extended to take care of the case when the application/libbpf
> uses the new format but the kernel only supports the old
> format. This patch adds support in libbpf for parsing the old
> format, before the need_wakeup flag was added, and emulating a
> set of static need_wakeup flags that will always work for the
> application.

Hi Magnus

While you're looking at backwards compatibility issues with xsk: libbpf
currently fails to compile on a system that has old kernel headers
installed (this is with kernel-headers 5.3):

$ echo "#include <bpf/xsk.h>" | gcc -x c - 
In file included from <stdin>:1:
/usr/include/bpf/xsk.h: In function ‘xsk_ring_prod__needs_wakeup’:
/usr/include/bpf/xsk.h:82:21: error: ‘XDP_RING_NEED_WAKEUP’ undeclared (first use in this function)
   82 |  return *r->flags & XDP_RING_NEED_WAKEUP;
      |                     ^~~~~~~~~~~~~~~~~~~~
/usr/include/bpf/xsk.h:82:21: note: each undeclared identifier is reported only once for each function it appears in
/usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_addr’:
/usr/include/bpf/xsk.h:173:16: error: ‘XSK_UNALIGNED_BUF_ADDR_MASK’ undeclared (first use in this function)
  173 |  return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
      |                ^~~~~~~~~~~~~~~~~~~~~~~~~~~
/usr/include/bpf/xsk.h: In function ‘xsk_umem__extract_offset’:
/usr/include/bpf/xsk.h:178:17: error: ‘XSK_UNALIGNED_BUF_OFFSET_SHIFT’ undeclared (first use in this function)
  178 |  return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~



How would you prefer to handle this? A patch like the one below will fix
the compile errors, but I'm not sure it makes sense semantically?

-Toke

diff --git a/tools/lib/bpf/xsk.h b/tools/lib/bpf/xsk.h
index 584f6820a639..954d66e85208 100644
--- a/tools/lib/bpf/xsk.h
+++ b/tools/lib/bpf/xsk.h
@@ -79,7 +79,11 @@ xsk_ring_cons__rx_desc(const struct xsk_ring_cons *rx, __u32 idx)
 
 static inline int xsk_ring_prod__needs_wakeup(const struct xsk_ring_prod *r)
 {
+#ifdef XDP_RING_NEED_WAKEUP
        return *r->flags & XDP_RING_NEED_WAKEUP;
+#else
+       return 0;
+#endif
 }
 
 static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
@@ -170,12 +174,20 @@ static inline void *xsk_umem__get_data(void *umem_area, __u64 addr)
 
 static inline __u64 xsk_umem__extract_addr(__u64 addr)
 {
+#ifdef XSK_UNALIGNED_BUF_ADDR_MASK
        return addr & XSK_UNALIGNED_BUF_ADDR_MASK;
+#else
+       return addr;
+#endif
 }
 
 static inline __u64 xsk_umem__extract_offset(__u64 addr)
 {
+#ifdef XSK_UNALIGNED_BUF_OFFSET_SHIFT
        return addr >> XSK_UNALIGNED_BUF_OFFSET_SHIFT;
+#else
+       return 0;
+#endif
 }
 
 static inline __u64 xsk_umem__add_offset_to_addr(__u64 addr)