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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 4 Mar 2020 00:01:05 +0100
From:   Daniel Borkmann <daniel@...earbox.net>
To:     Andrii Nakryiko <andriin@...com>, bpf@...r.kernel.org,
        netdev@...r.kernel.org, ast@...com
Cc:     andrii.nakryiko@...il.com, kernel-team@...com, yhs@...com
Subject: Re: [PATCH v3 bpf-next 1/3] bpf: switch BPF UAPI #define constants
 used from BPF program side to enums

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@...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.

> ---
>   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 */
> +};
[...]