Re: [PATCH v4 bpf-next 6/7] libbpf: add BPF_CORE_READ/BPF_CORE_READ_INTO helpers

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

On Mon, Oct 7, 2019 at 9:56 PM Alexei Starovoitov <ast@fb.com> wrote:
>
> On 10/7/19 3:47 PM, Andrii Nakryiko wrote:
> > Add few macros simplifying BCC-like multi-level probe reads, while also
> > emitting CO-RE relocations for each read.
> >
> > Acked-by: John Fastabend <john.fastabend@gmail.com>
> > Acked-by: Song Liu <songliubraving@fb.com>
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
> ...
> > +/*
> > + * BPF_CORE_READ() is used to simplify BPF CO-RE relocatable read, especially
> > + * when there are few pointer chasing steps.
> > + * E.g., what in non-BPF world (or in BPF w/ BCC) would be something like:
> > + *   int x = s->a.b.c->d.e->f->g;
> > + * can be succinctly achieved using BPF_CORE_READ as:
> > + *   int x = BPF_CORE_READ(s, a.b.c, d.e, f, g);
> > + *
> > + * BPF_CORE_READ will decompose above statement into 4 bpf_core_read (BPF
> > + * CO-RE relocatable bpf_probe_read() wrapper) calls, logically equivalent to:
> > + * 1. const void *__t = s->a.b.c;
> > + * 2. __t = __t->d.e;
> > + * 3. __t = __t->f;
> > + * 4. return __t->g;
> > + *
> > + * Equivalence is logical, because there is a heavy type casting/preservation
> > + * involved, as well as all the reads are happening through bpf_probe_read()
> > + * calls using __builtin_preserve_access_index() to emit CO-RE relocations.
> > + *
> > + * N.B. Only up to 9 "field accessors" are supported, which should be more
> > + * than enough for any practical purpose.
> > + */
> > +#define BPF_CORE_READ(src, a, ...)                                       \
> > +     ({                                                                  \
> > +             ___type(src, a, ##__VA_ARGS__) __r;                         \
> > +             BPF_CORE_READ_INTO(&__r, src, a, ##__VA_ARGS__);            \
> > +             __r;                                                        \
> > +     })
> > +
>
> Since we're splitting things into
> bpf_{helpers,helper_defs,endian,tracing}.h
> how about adding all core macros into bpf_core_read.h ?

ok, but maybe just bpf_core.h then?

> #define___concat, ___empty are very generic names.
> I'd rather contain the risk of conflicts to progs that are going
> to use co-re instead of forcing it on all progs that use bpf_helpers.h.
> With my btf vmlinux stuff all these bpf_probe_read*() wrappers
> hopefully will be obsolete eventually. So keeping them separate in
> bpf_core_read.h would help the transition too.