Magnus Karlsson [off-list ref] writes:

On Thu, Oct 31, 2019 at 9:03 AM Björn Töpel [off-list ref] wrote:

quoted

On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson [off-list ref] wrote:

quoted

On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen [off-list ref] wrote:

quoted

Magnus Karlsson [off-list ref] writes:

quoted

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?

Thanks Toke for finding this. Of course it should be possible to
compile this on an older kernel, but without getting any of the newer
functionality that is not present in that older kernel.

Is the plan to support source compatibility for the headers only, or
the whole the libbpf itself? Is the usecase here, that you've built
libbpf.so with system headers X, and then would like to use the
library on a system with older system headers X~10? XDP sockets? BTF?

Good question. I let someone with more insight answer this. Providing
the support Toke wants does make the header files less pleasant to
look at for sure. But in any case, I think we should provide an error
when you try to enable a new kernel feature using an old libbpf that
has no support for it. Just in case someone mixes things up.

Yup, I agree. Removing the functions completely is fine with me. As for
the flags, I agree that having a check in libbpf would make sense; I can
see someone upgrading their kernel, but still using the distro-specified
libbpf and running into weird errors otherwise.

Maybe we should define XDP_FLAGS_ALL in if_xdp.h and use that for the
check in both libbpf and the kernel? We'd still need conditional defines
for backwards compatibility, but at least we wouldn't need to keep
updating that as new flags are added?

-Toke