Thread (9 messages) 9 messages, 4 authors, 2023-12-13

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

From: Andrii Nakryiko <hidden>
Date: 2023-12-12 04:01:19
Also in: bpf, linux-fsdevel, linux-security-module

On Mon, Dec 11, 2023 at 6:40 PM Alexei Starovoitov
[off-list ref] wrote:
On Thu, Dec 7, 2023 at 2:28 PM Andrii Nakryiko [off-list ref] wrote:
quoted
+#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.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help