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 18:40:12 -0800
From: Alexei Starovoitov <alexei.starovoitov@...il.com>
To: Andrii Nakryiko <andrii@...nel.org>
Cc: 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 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?

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

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 ;)

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.