Re: [PATCH bpf-next 3/3] libbpf: Use bpf_probe_read_kernel

[Andrii Nakryiko <andrii.nakryiko@gmail.com>]

On Tue, Jul 28, 2020 at 2:16 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 7/28/20 9:11 PM, Andrii Nakryiko wrote:
> > On Tue, Jul 28, 2020 at 5:15 AM Ilya Leoshkevich <iii@linux.ibm.com> wrote:
> >>
> >> Yet another adaptation to commit 0ebeea8ca8a4 ("bpf: Restrict
> >> bpf_probe_read{, str}() only to archs where they work") that makes more
> >> samples compile on s390.
> >>
> >> Signed-off-by: Ilya Leoshkevich <iii@linux.ibm.com>
> >
> > Sorry, we can't do this yet. This will break on older kernels that
> > don't yet have bpf_probe_read_kernel() implemented. Met and Yonghong
> > are working on extending a set of CO-RE relocations, that would allow
> > to do bpf_probe_read_kernel() detection on BPF side, transparently for
> > an application, and will pick either bpf_probe_read() or
> > bpf_probe_read_kernel(). It should be ready soon (this or next week,
> > most probably), though it will have dependency on the latest Clang.
> > But for now, please don't change this.
>
> Could you elaborate what this means wrt dependency on latest clang? Given clang
> releases have a rather long cadence, what about existing users with current clang
> releases?

So the overall idea is to use something like this to do kernel reads:

static __always_inline int bpf_probe_read_universal(void *dst, u32 sz,
const void *src)
{
    if (bpf_core_type_exists(btf_bpf_probe_read_kernel))
        return bpf_probe_read_kernel(dst, sz, src);
    else
        return bpf_probe_read(dst, sz, src);
}

And then use bpf_probe_read_universal() in BPF_CORE_READ and family.

This approach relies on few things:

1. each BPF helper has a corresponding btf_<helper-name> type defined for it
2. bpf_core_type_exists(some_type) returns 0 or 1, depending if
specified type is found in kernel BTF (so needs kernel BTF, of
course). This is the part me and Yonghong are working on at the
moment.
3. verifier's dead code elimination, which will leave only
bpf_probe_read() or bpf_probe_read_kernel() calls and will remove the
other one. So on older kernels, there will never be unsupoorted call
to bpf_probe_read_kernel().


The new type existence relocation requires the latest Clang. So the
way to deal with older Clangs would be to just fallback to
bpf_probe_read, if we detect that Clang is too old and can't emit
necessary relocation.

If that's not an acceptable plan, then one can "parameterize"
BPF_CORE_READ macro family by re-defining bpf_core_read() macro. Right
now it's defined as:

#define bpf_core_read(dst, sz, src) \
    bpf_probe_read(dst, sz, (const void *)__builtin_preserve_access_index(src))

Re-defining it in terms of bpf_probe_read_kernel is trivial, but I
can't do it for BPF_CORE_READ, because it will break all the users of
bpf_core_read.h that run on older kernels.


>
> >>   tools/lib/bpf/bpf_core_read.h | 51 ++++++++++++++++++-----------------
> >>   tools/lib/bpf/bpf_tracing.h   | 15 +++++++----
> >>   2 files changed, 37 insertions(+), 29 deletions(-)
> >>
> >
> > [...]
> >
>