On Mon, Dec 11, 2023 at 6:40 PM Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Thu, Dec 7, 2023 at 2:28 PM Andrii Nakryiko <andrii@xxxxxxxxxx> 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.
