[PATCH bpf-next v2 0/3] handle errno ENOTSUPP

[PATCH bpf-next v2 0/3] handle errno ENOTSUPP

[Geliang Tang]

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

-- 
2.43.0

[PATCH bpf-next v2 1/3] bpf: verifier: Fix return value of fixup_call_args

[Geliang Tang]

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

[PATCH bpf-next v2 2/3] libbpf: handle ENOTSUPP in libbpf_strerror_r

[Geliang Tang]

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 */
-- 
2.43.0

[PATCH bpf-next v2 3/3] selftests/bpf: Drop duplicate ENOTSUPP definitions

[Geliang Tang]

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
-- 
2.43.0