From: Geliang Tang tanggeliang@kylinos.cn
v2: - patch 1, only commit log updated - update patch 2 - add an unsigned variable - use "switch-case" - only "Operation not supported", no (-524) in string - patch 3, a now one
This patchset contains three fixes for handling errno ENOTSUPP.
Patch 1 fixes the return value of fixup_call_args() to make sure ENOTSUPP is returned to user space correctly. Patch 2 handles ENOTSUPP in libbpf_strerror_r() in libbpf. Patch 3 includes str_error.h in BPF selftests, and drop duplicate ENOTSUPP definitions.
Geliang Tang (3): bpf: verifier: Fix return value of fixup_call_args libbpf: handle ENOTSUPP in libbpf_strerror_r selftests/bpf: Drop duplicate ENOTSUPP definitions
kernel/bpf/verifier.c | 6 +++--- tools/lib/bpf/str_error.c | 18 +++++++++++++----- tools/lib/bpf/str_error.h | 4 ++++ .../selftests/bpf/prog_tests/bpf_tcp_ca.c | 4 ---- .../selftests/bpf/prog_tests/lsm_cgroup.c | 4 ---- .../selftests/bpf/prog_tests/sock_addr.c | 4 ---- tools/testing/selftests/bpf/test_maps.c | 4 ---- tools/testing/selftests/bpf/test_verifier.c | 4 ---- tools/testing/selftests/bpf/testing_helpers.h | 1 + 9 files changed, 21 insertions(+), 28 deletions(-)
From: Geliang Tang tanggeliang@kylinos.cn
Run bloom_filter_map selftests (./test_progs -t bloom_filter_map) on a Loongarch platform, an error message "JIT doesn't support bpf-to-bpf calls" is got in user space, together with an unexpected errno EINVAL (22), not ENOTSUPP (524):
libbpf: prog 'inner_map': BPF program load failed: Invalid argument libbpf: prog 'inner_map': -- BEGIN PROG LOAD LOG -- JIT doesn't support bpf-to-bpf calls callbacks are not allowed in non-JITed programs processed 37 insns (limit 1000000) max_states_per_insn 1 total_states -- END PROG LOAD LOG -- libbpf: prog 'inner_map': failed to load: -22 libbpf: failed to load object 'bloom_filter_map' libbpf: failed to load BPF skeleton 'bloom_filter_map': -22 setup_progs:FAIL:bloom_filter_map__open_and_load unexpected error: -22 #16 bloom_filter_map:FAIL
Although the return value of jit_subprogs() does be set as "ENOTSUPP":
verbose(env, "JIT doesn't support bpf-to-bpf calls\n"); err = -ENOTSUPP; goto out_free;
But afterwards in fixup_call_args(), the return value of jit_subprogs() is ignored, and overwritten as "-EINVAL":
verbose(env, "callbacks are not allowed in non-JITed programs\n"); return -EINVAL;
This patch fixes this by changing return values of fixup_call_args() from "-EINVAL" to "err ?: -EINVAL". With this change, errno 524 is got in user space now:
libbpf: prog 'inner_map': BPF program load failed: unknown error (-524) libbpf: prog 'inner_map': -- BEGIN PROG LOAD LOG -- JIT doesn't support bpf-to-bpf calls processed 37 insns (limit 1000000) max_states_per_insn 1 total_states -- END PROG LOAD LOG -- libbpf: prog 'inner_map': failed to load: -524 libbpf: failed to load object 'bloom_filter_map' libbpf: failed to load BPF skeleton 'bloom_filter_map': -524 setup_progs:FAIL:bloom_filter_map__open_and_load unexpected error: -524
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- kernel/bpf/verifier.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index c0263fb5ca4b..aa589fedd036 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -19717,14 +19717,14 @@ static int fixup_call_args(struct bpf_verifier_env *env) #ifndef CONFIG_BPF_JIT_ALWAYS_ON if (has_kfunc_call) { verbose(env, "calling kernel functions are not allowed in non-JITed programs\n"); - return -EINVAL; + return err ?: -EINVAL; } if (env->subprog_cnt > 1 && env->prog->aux->tail_call_reachable) { /* When JIT fails the progs with bpf2bpf calls and tail_calls * have to be rejected, since interpreter doesn't support them yet. */ verbose(env, "tail_calls are not allowed in non-JITed programs with bpf-to-bpf calls\n"); - return -EINVAL; + return err ?: -EINVAL; } for (i = 0; i < prog->len; i++, insn++) { if (bpf_pseudo_func(insn)) { @@ -19732,7 +19732,7 @@ static int fixup_call_args(struct bpf_verifier_env *env) * have to be rejected, since interpreter doesn't support them yet. */ verbose(env, "callbacks are not allowed in non-JITed programs\n"); - return -EINVAL; + return err ?: -EINVAL; }
if (!bpf_pseudo_call(insn))
From: Geliang Tang tanggeliang@kylinos.cn
Errno 95 (ENOTSUP or EOPNOTSUPP) can be recognized by libbpf_strerror_r(), but 524 (ENOTSUPP) can't:
prog 'basic_alloc3': BPF program load failed: Operation not supported prog 'basic_alloc3': failed to load: -95 failed to load object 'verifier_arena' FAIL:unexpected_load_failure unexpected error: -95 (errno 95)
prog 'inner_map': BPF program load failed: unknown error (-524) prog 'inner_map': failed to load: -524 failed to load object 'bloom_filter_map' failed to load BPF skeleton 'bloom_filter_map': -524 FAIL:bloom_filter_map__open_and_load unexpected error: -524
This patch fixes this by handling ENOTSUPP in libbpf_strerror_r(). With this change, the new error string looks like:
prog 'inner_map': BPF program load failed: Operation not supported
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/lib/bpf/str_error.c | 18 +++++++++++++----- tools/lib/bpf/str_error.h | 4 ++++ 2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/tools/lib/bpf/str_error.c b/tools/lib/bpf/str_error.c index 5e6a1e27ddf9..5eef2bc7fac5 100644 --- a/tools/lib/bpf/str_error.c +++ b/tools/lib/bpf/str_error.c @@ -15,7 +15,8 @@ */ char *libbpf_strerror_r(int err, char *dst, int len) { - int ret = strerror_r(err < 0 ? -err : err, dst, len); + unsigned int no = err < 0 ? -err : err; + int ret = strerror_r(no, dst, len); /* on glibc <2.13, ret == -1 and errno is set, if strerror_r() can't * handle the error, on glibc >=2.13 *positive* (errno-like) error * code is returned directly @@ -23,11 +24,18 @@ char *libbpf_strerror_r(int err, char *dst, int len) if (ret == -1) ret = errno; if (ret) { - if (ret == EINVAL) - /* strerror_r() doesn't recognize this specific error */ - snprintf(dst, len, "unknown error (%d)", err < 0 ? err : -err); - else + if (ret == EINVAL) { + switch (no) { + case ENOTSUPP: + snprintf(dst, len, "Operation not supported"); + break; + default: + /* strerror_r() doesn't recognize this specific error */ + snprintf(dst, len, "unknown error (-%u)", no); + } + } else { snprintf(dst, len, "ERROR: strerror_r(%d)=%d", err, ret); + } } return dst; } diff --git a/tools/lib/bpf/str_error.h b/tools/lib/bpf/str_error.h index 626d7ffb03d6..c41f6ba133cf 100644 --- a/tools/lib/bpf/str_error.h +++ b/tools/lib/bpf/str_error.h @@ -4,6 +4,10 @@
#define STRERR_BUFSIZE 128
+#ifndef ENOTSUPP +#define ENOTSUPP 524 +#endif + char *libbpf_strerror_r(int err, char *dst, int len);
#endif /* __LIBBPF_STR_ERROR_H */
From: Geliang Tang tanggeliang@kylinos.cn
ENOTSUPP is defined in bpf/str_error.h now, so no need to redefine it in so many places in bpf selftests.
This patch includes <bpf/str_error.h> in testing_helpers.h, which is almost included by each tests. And drop all duplicate definitions.
Signed-off-by: Geliang Tang tanggeliang@kylinos.cn --- tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c | 4 ---- tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c | 4 ---- tools/testing/selftests/bpf/prog_tests/sock_addr.c | 4 ---- tools/testing/selftests/bpf/test_maps.c | 4 ---- tools/testing/selftests/bpf/test_verifier.c | 4 ---- tools/testing/selftests/bpf/testing_helpers.h | 1 + 6 files changed, 1 insertion(+), 20 deletions(-)
diff --git a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c index 1d494b4453f4..676eb7cd7fb0 100644 --- a/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c +++ b/tools/testing/selftests/bpf/prog_tests/bpf_tcp_ca.c @@ -16,10 +16,6 @@ #include "tcp_ca_kfunc.skel.h" #include "bpf_cc_cubic.skel.h"
-#ifndef ENOTSUPP -#define ENOTSUPP 524 -#endif - static const unsigned int total_bytes = 10 * 1024 * 1024; static int expected_stg = 0xeB9F;
diff --git a/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c b/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c index 130a3b21e467..6df25de8f080 100644 --- a/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c +++ b/tools/testing/selftests/bpf/prog_tests/lsm_cgroup.c @@ -10,10 +10,6 @@ #include "cgroup_helpers.h" #include "network_helpers.h"
-#ifndef ENOTSUPP -#define ENOTSUPP 524 -#endif - static struct btf *btf;
static __u32 query_prog_cnt(int cgroup_fd, const char *attach_func) diff --git a/tools/testing/selftests/bpf/prog_tests/sock_addr.c b/tools/testing/selftests/bpf/prog_tests/sock_addr.c index b880c564a204..68d9255d2bb7 100644 --- a/tools/testing/selftests/bpf/prog_tests/sock_addr.c +++ b/tools/testing/selftests/bpf/prog_tests/sock_addr.c @@ -23,10 +23,6 @@ #include "getpeername_unix_prog.skel.h" #include "network_helpers.h"
-#ifndef ENOTSUPP -# define ENOTSUPP 524 -#endif - #define TEST_NS "sock_addr" #define TEST_IF_PREFIX "test_sock_addr" #define TEST_IPV4 "127.0.0.4" diff --git a/tools/testing/selftests/bpf/test_maps.c b/tools/testing/selftests/bpf/test_maps.c index dfbab214f4d1..227d7d6eaf8e 100644 --- a/tools/testing/selftests/bpf/test_maps.c +++ b/tools/testing/selftests/bpf/test_maps.c @@ -26,10 +26,6 @@ #include "test_maps.h" #include "testing_helpers.h"
-#ifndef ENOTSUPP -#define ENOTSUPP 524 -#endif - int skips;
static struct bpf_map_create_opts map_opts = { .sz = sizeof(map_opts) }; diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c index 610392dfc4fb..447b68509d76 100644 --- a/tools/testing/selftests/bpf/test_verifier.c +++ b/tools/testing/selftests/bpf/test_verifier.c @@ -42,10 +42,6 @@ #include "../../../include/linux/filter.h" #include "testing_helpers.h"
-#ifndef ENOTSUPP -#define ENOTSUPP 524 -#endif - #define MAX_INSNS BPF_MAXINSNS #define MAX_EXPECTED_INSNS 32 #define MAX_UNEXPECTED_INSNS 32 diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h index d55f6ab12433..1e882f408596 100644 --- a/tools/testing/selftests/bpf/testing_helpers.h +++ b/tools/testing/selftests/bpf/testing_helpers.h @@ -7,6 +7,7 @@ #include <stdbool.h> #include <bpf/bpf.h> #include <bpf/libbpf.h> +#include <bpf/str_error.h> #include <time.h>
#define __TO_STR(x) #x
linux-kselftest-mirror@lists.linaro.org
-
Geliang Tang