Re: [PATCH v3 bpf-next 1/3] bpf: switch BPF UAPI #define constants used from BPF program side to enums

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

On Tue, Mar 3, 2020 at 3:01 PM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 3/3/20 1:32 AM, Andrii Nakryiko wrote:
> > Switch BPF UAPI constants, previously defined as #define macro, to anonymous
> > enum values. This preserves constants values and behavior in expressions, but
> > has added advantaged of being captured as part of DWARF and, subsequently, BTF
> > type info. Which, in turn, greatly improves usefulness of generated vmlinux.h
> > for BPF applications, as it will not require BPF users to copy/paste various
> > flags and constants, which are frequently used with BPF helpers. Only those
> > constants that are used/useful from BPF program side are converted.
> >
> > Signed-off-by: Andrii Nakryiko <andriin@fb.com>
>
> Just thinking out loud, is there some way this could be resolved generically
> either from compiler side or via additional tooling where this ends up as BTF
> data and thus inside vmlinux.h as anon enum eventually? bpf.h is one single
> header and worst case libbpf could also ship a copy of it (?), but what about
> all the other things one would need to redefine e.g. for tracing? Small example
> that comes to mind are all these TASK_* defines in sched.h etc, and there's
> probably dozens of other similar stuff needed too depending on the particular
> case; would be nice to have some generic catch-all, hmm.

Enum convertion seems to be the simplest and cleanest way,
unfortunately (as far as I know). DWARF has some extensions capturing
#defines, but values are strings (and need to be parsed, which is pain
already for "1 << 1ULL"), and it's some obscure extension, not a
standard thing. I agree would be nice not to have and change all UAPI
headers for this, but I'm not aware of the solution like that.

>
> > ---
> >   include/uapi/linux/bpf.h       | 175 ++++++++++++++++++++------------
> >   tools/include/uapi/linux/bpf.h | 177 ++++++++++++++++++++-------------
> >   2 files changed, 219 insertions(+), 133 deletions(-)
> >
> > diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
> > index 8e98ced0963b..3ce4e8759661 100644
> > --- a/include/uapi/linux/bpf.h
> > +++ b/include/uapi/linux/bpf.h
> > @@ -325,44 +325,46 @@ enum bpf_attach_type {
> >   #define BPF_PSEUDO_CALL             1
> >
> >   /* flags for BPF_MAP_UPDATE_ELEM command */
> > -#define BPF_ANY              0 /* create new element or update existing */
> > -#define BPF_NOEXIST  1 /* create new element if it didn't exist */
> > -#define BPF_EXIST    2 /* update existing element */
> > -#define BPF_F_LOCK   4 /* spin_lock-ed map_lookup/map_update */
> > +enum {
> > +     BPF_ANY         = 0, /* create new element or update existing */
> > +     BPF_NOEXIST     = 1, /* create new element if it didn't exist */
> > +     BPF_EXIST       = 2, /* update existing element */
> > +     BPF_F_LOCK      = 4, /* spin_lock-ed map_lookup/map_update */
> > +};
> [...]