On Tue, Mar 3, 2020 at 3:01 PM Daniel Borkmann <daniel@xxxxxxxxxxxxx> 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@xxxxxx>
>
> 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 */
> > +};
> [...]
