On Wed, May 27, 2020 at 4:34 AM Jesper Dangaard Brouer
<brouer@xxxxxxxxxx> wrote:
>
> When a BPF-map type doesn't support having a BTF info associated, the
> bpf_map_ops->map_check_btf is set to map_check_no_btf(). This function
> map_check_no_btf() currently returns -ENOTSUPP, which result in a very
> confusing error message in libbpf, see below.
>
> The errno ENOTSUPP is part of the kernels internal errno in file
> include/linux/errno.h. As is stated in the file, these "should never be
> seen by user programs". This is not a not a standard Unix error.
>
> This should likely have been EOPNOTSUPP instead. This seems to be a common
> mistake, that even checkpatch tries to catch see commit 6b9ea5ff5abd
> ("checkpatch: warn about uses of ENOTSUPP").
>
> Before this change end-users of libbpf will see:
> libbpf: Error in bpf_create_map_xattr(cpu_map):ERROR: strerror_r(-524)=22(-524). Retrying without BTF.
>
> After this change end-users of libbpf will see:
> libbpf: Error in bpf_create_map_xattr(cpu_map):Operation not supported(-95). Retrying without BTF.
>
> V2: Use EOPNOTSUPP instead of EUCLEAN.
>
> Fixes: e8d2bec04579 ("bpf: decouple btf from seq bpf fs dump and enable more maps")
> Signed-off-by: Jesper Dangaard Brouer <brouer@xxxxxxxxxx>
> ---
I don't mind this change, per se, mostly because I think it doesn't matter. But:
$ rg ENOTSUPP kernel/bpf | wc -l
42
$ rg EOPNOTSUPP kernel/bpf | wc -l
8
So 5 to 1 in favor of ENOTSUPP in purely BPF sources.
Globally across kernel sources the picture is different, though:
$ rg ENOTSUPP | wc -l
1597
$ rg EOPNOTSUPP | wc -l
6982
I didn't audit if those errors can get eventually propagated to
user-space, but I'd imagine that most would. So, despite what that
errno.h header says, EOPNOTSUPP is quite widely used still.
But regardless, can you please reply on v1 thread why adding support
for BTF to these special maps (that do not support BTF right now)
won't be a better solution and won't work (as you claimed)?
> kernel/bpf/syscall.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index d13b804ff045..e4e0a0c5192c 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -732,7 +732,7 @@ int map_check_no_btf(const struct bpf_map *map,
> const struct btf_type *key_type,
> const struct btf_type *value_type)
> {
> - return -ENOTSUPP;
> + return -EOPNOTSUPP;
> }
>
> static int map_check_btf(struct bpf_map *map, const struct btf *btf,
>
>
