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

[Alexei Starovoitov <alexei.starovoitov@gmail.com>]

On Thu, Oct 31, 2019 at 10:42 AM Jiri Olsa <jolsa@redhat.com> wrote:
>
> On Thu, Oct 31, 2019 at 08:17:43AM -0700, Alexei Starovoitov wrote:
> > On Thu, Oct 31, 2019 at 7:52 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > >
> > > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > >
> > > > On Thu, Oct 31, 2019 at 7:26 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > > >>
> > > >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > > >>
> > > >> > On Thu, Oct 31, 2019 at 7:13 AM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > > >> >>
> > > >> >> Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > > >> >>
> > > >> >> > On Thu, Oct 31, 2019 at 1:03 AM Björn Töpel <bjorn.topel@gmail.com> wrote:
> > > >> >> >>
> > > >> >> >> On Thu, 31 Oct 2019 at 08:17, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
> > > >> >> >> >
> > > >> >> >> > On Wed, Oct 30, 2019 at 2:36 PM Toke Høiland-Jørgensen <toke@redhat.com> wrote:
> > > >> >> >> > >
> > > >> >> >> > > Magnus Karlsson <magnus.karlsson@intel.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?
> > > >> >> >> >
> > > >> >> >> > 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?
> > > >> >> >
> > > >> >> > libbpf has to be backward and forward compatible.
> > > >> >> > Once compiled it has to run on older and newer kernels.
> > > >> >> > Conditional compilation is not an option obviously.
> > > >> >>
> > > >> >> So what do we do, then? Redefine the constants in libbpf/xsh.h if
> > > >> >> they're not in the kernel header file?
> > > >> >
> > > >> > why? How and whom it will help?
> > > >> > To libbpf.rpm creating person or to end user?
> > > >>
> > > >> Anyone who tries to compile a new libbpf against an older kernel. You're
> > > >> saying yourself that "libbpf has to be backward and forward compatible".
> > > >> Surely that extends to compile time as well as runtime?
> > > >
> > > > how old that older kernel?
> > > > Does it have up-to-date bpf.h in /usr/include ?
> > > > Also consider that running kernel is often not the same
> > > > thing as installed in /usr/include
> > > > vmlinux and /usr/include are different packages.
> > >
> > > In this case, it's a constant introduced in the kernel in the current
> > > (5.4) cycle; so currently, you can't compile libbpf with
> > > kernel-headers-5.3. And we're discussing how to handle this in a
> > > backwards compatible way in libbpf...
> >
> > you simply don't.
> > It's not a problem to begin with.
>
> hum, that's possible case for distro users.. older kernel, newer libbpf

yes. older vmlinux and newer installed libbpf.so
or any version of libbpf.a that is statically linked into apps
is something that libbpf code has to support.
The server can be rebooted into older than libbpf kernel and
into newer than libbpf kernel. libbpf has to recognize all these
combinations and work appropriately.
That's what backward and forward compatibility is.
That's what makes libbpf so difficult to test, develop and code review.
What that particular server has in /usr/include is irrelevant.