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

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. My preference
is if we just remove these functions completely if you compile it with
a kernel that does not have support for it. For the need_wakeup
feature, we cannot provide semantics that make sense in the function
above. If we return 0, Tx will break. If we return 1, Rx will become
really slow. And we cannot detect /without an ugly hack) if it is the
fill queue or the Tx queue that was provided to the function. So what
do you think of just removing these functions if the kernel does not
have the corresponding defines in its kernel header? The user should
not use them in that case.

We should also think about the case when someone takes the new libbpf
source, compiles it with an older kernel then runs the binary on the
newer kernel. It will for sure happen :-). We should add some code in
the xsk_socket__create call that checks so that users do not try to
use a bind flag that did not exist on the system libbpf was compiled
for. In that case, we should return an error. We need the same code
that we have in the kernel for checking against illegal flags to be
present in the xsk part of libbpf.

/Magnus