netdev - Re: [PATCH RFC bpf-next 1/3] bpf: add mapper macro for bpf

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: Mon, 11 Dec 2023 20:01:06 -0800
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc: Andrii Nakryiko <andrii@...nel.org>, bpf <bpf@...r.kernel.org>, 
	Network Development <netdev@...r.kernel.org>, Paul Moore <paul@...l-moore.com>, 
	Christian Brauner <brauner@...nel.org>, Linux-Fsdevel <linux-fsdevel@...r.kernel.org>, 
	LSM List <linux-security-module@...r.kernel.org>, Kees Cook <keescook@...omium.org>, 
	Kernel Team <kernel-team@...a.com>, Sargun Dhillon <sargun@...gun.me>
Subject: Re: [PATCH RFC bpf-next 1/3] bpf: add mapper macro for bpf_cmd enum

On Mon, Dec 11, 2023 at 6:40 PM Alexei Starovoitov
<alexei.starovoitov@...il.com> wrote:
>
> On Thu, Dec 7, 2023 at 2:28 PM Andrii Nakryiko <andrii@...nel.org> wrote:
> >
> > +#define __BPF_CMD_MAPPER(FN, ctx...)                                   \
> > +       FN(BPF_MAP_CREATE, 0)                                           \
> > +       FN(BPF_MAP_LOOKUP_ELEM, 1)                                      \
> > +       FN(BPF_MAP_UPDATE_ELEM, 2)                                      \
> > +       FN(BPF_MAP_DELETE_ELEM, 3)                                      \
> > +       FN(BPF_MAP_GET_NEXT_KEY, 4)                                     \
>
> So macro conversion across 4 main enums in uapi/bpf.h
> is just to do:
> +static const struct constant_table cmd_kvs[] = {
> +       __BPF_CMD_MAPPER(__BPF_KV_FN)
> +       {}
> +};
>
> on the kernel side,
> right?

Right.

>
> While in libbpf we already hard code name to value in arrays:
> prog_type_name[], map_type_name[]
>
> which probably will remain as-is, since libbpf needs to be
> built independently from the kernel.
> (unless we will say that tools/uapi/bpf.h is part of libbpf,
> which probably not a good way).

No, we can easily use this on the libbpf side as well. Libbpf syncs
the latest UAPI headers ([0]) and uses them during build time.

  [0] https://github.com/libbpf/libbpf/tree/master/include/uapi/linux

>
> There are more pros than cons in this enum uglification,
> but cons are definitely staring in the face.
>
> Have you considered other options?
> Like using vmlinix BTF for parsing bpffs delegation?
> [14083] ENUM 'bpf_cmd' encoding=UNSIGNED size=4 vlen=39
>         'BPF_MAP_CREATE' val=0
>         'BPF_MAP_LOOKUP_ELEM' val=1
>         'BPF_MAP_UPDATE_ELEM' val=2
>         'BPF_MAP_DELETE_ELEM' val=3
>         'BPF_MAP_GET_NEXT_KEY' val=4
>         'BPF_PROG_LOAD' val=5
>
> Names and values are available.
> btf_find_by_name_kind(vmlinux_btf, "bpd_cmd", BTF_KIND_ENUM);
> is fast enough.
>
> I suspect you'll argue that you don't want to tie in
> bpffs delegation parsing with BTF ;)
>

Yep, that's the reason I didn't go for it in the initial version, of
course. But it's fine.

> While I can preemptively answer that in the case vmlinux BTF
> is not available it's fine not to parse names and rely on hex.

It's fine, I can do optional BTF-based parsing, if that's what you prefer.