* [PATCH bpf-next v7 0/2] Skip callback tests if jit is disabled in test_verifier
@ 2024-01-23 9:03 Tiezhu Yang
2024-01-23 9:03 ` [PATCH bpf-next v7 1/2] selftests/bpf: Move is_jit_enabled() into testing_helpers Tiezhu Yang
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Tiezhu Yang @ 2024-01-23 9:03 UTC (permalink / raw)
To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko
Cc: Eduard Zingerman, John Fastabend, Jiri Olsa, Hou Tao, Song Liu,
bpf, linux-kernel
Thanks very much for the feedbacks from Eduard, John, Jiri, Daniel,
Hou Tao, Song Liu and Andrii.
v7:
-- Add an explicit flag F_NEEDS_JIT_ENABLED for checking,
thanks Andrii.
v6:
-- Copy insn_is_pseudo_func() into testing_helpers,
thanks Andrii.
v5:
-- Reuse is_ldimm64_insn() and insn_is_pseudo_func(),
thanks Song Liu.
v4:
-- Move the not-allowed-checking into "if (expected_ret ...)"
block, thanks Hou Tao.
-- Do some small changes to avoid checkpatch warning
about "line length exceeds 100 columns".
v3:
-- Rebase on the latest bpf-next tree.
-- Address the review comments by Hou Tao,
remove the second argument "0" of open(),
check only once whether jit is disabled,
check fd_prog, saved_errno and jit_disabled to skip.
Tiezhu Yang (2):
selftests/bpf: Move is_jit_enabled() into testing_helpers
selftests/bpf: Skip callback tests if jit is disabled in test_verifier
tools/testing/selftests/bpf/test_progs.c | 18 ------------------
tools/testing/selftests/bpf/test_verifier.c | 11 +++++++++++
tools/testing/selftests/bpf/testing_helpers.c | 18 ++++++++++++++++++
tools/testing/selftests/bpf/testing_helpers.h | 1 +
.../selftests/bpf/verifier/bpf_loop_inline.c | 6 ++++++
5 files changed, 36 insertions(+), 18 deletions(-)
--
2.42.0
^ permalink raw reply [flat|nested] 5+ messages in thread* [PATCH bpf-next v7 1/2] selftests/bpf: Move is_jit_enabled() into testing_helpers 2024-01-23 9:03 [PATCH bpf-next v7 0/2] Skip callback tests if jit is disabled in test_verifier Tiezhu Yang @ 2024-01-23 9:03 ` Tiezhu Yang 2024-01-23 9:03 ` [PATCH bpf-next v7 2/2] selftests/bpf: Skip callback tests if jit is disabled in test_verifier Tiezhu Yang 2024-01-24 4:50 ` [PATCH bpf-next v7 0/2] " patchwork-bot+netdevbpf 2 siblings, 0 replies; 5+ messages in thread From: Tiezhu Yang @ 2024-01-23 9:03 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Eduard Zingerman, John Fastabend, Jiri Olsa, Hou Tao, Song Liu, bpf, linux-kernel Currently, is_jit_enabled() is only used in test_progs, move it into testing_helpers so that it can be used in test_verifier. While at it, remove the second argument "0" of open() as Hou Tao suggested. Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> Acked-by: Hou Tao <houtao1@huawei.com> Acked-by: Song Liu <song@kernel.org> --- tools/testing/selftests/bpf/test_progs.c | 18 ------------------ tools/testing/selftests/bpf/testing_helpers.c | 18 ++++++++++++++++++ tools/testing/selftests/bpf/testing_helpers.h | 1 + 3 files changed, 19 insertions(+), 18 deletions(-) diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c index 1b9387890148..808550986f30 100644 --- a/tools/testing/selftests/bpf/test_progs.c +++ b/tools/testing/selftests/bpf/test_progs.c @@ -547,24 +547,6 @@ int bpf_find_map(const char *test, struct bpf_object *obj, const char *name) return bpf_map__fd(map); } -static bool is_jit_enabled(void) -{ - const char *jit_sysctl = "/proc/sys/net/core/bpf_jit_enable"; - bool enabled = false; - int sysctl_fd; - - sysctl_fd = open(jit_sysctl, 0, O_RDONLY); - if (sysctl_fd != -1) { - char tmpc; - - if (read(sysctl_fd, &tmpc, sizeof(tmpc)) == 1) - enabled = (tmpc != '0'); - close(sysctl_fd); - } - - return enabled; -} - int compare_map_keys(int map1_fd, int map2_fd) { __u32 key, next_key; diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c index 106ef05586b8..a59e56d804ee 100644 --- a/tools/testing/selftests/bpf/testing_helpers.c +++ b/tools/testing/selftests/bpf/testing_helpers.c @@ -457,3 +457,21 @@ int get_xlated_program(int fd_prog, struct bpf_insn **buf, __u32 *cnt) *buf = NULL; return -1; } + +bool is_jit_enabled(void) +{ + const char *jit_sysctl = "/proc/sys/net/core/bpf_jit_enable"; + bool enabled = false; + int sysctl_fd; + + sysctl_fd = open(jit_sysctl, O_RDONLY); + if (sysctl_fd != -1) { + char tmpc; + + if (read(sysctl_fd, &tmpc, sizeof(tmpc)) == 1) + enabled = (tmpc != '0'); + close(sysctl_fd); + } + + return enabled; +} diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h index e099aa4da611..d14de81727e6 100644 --- a/tools/testing/selftests/bpf/testing_helpers.h +++ b/tools/testing/selftests/bpf/testing_helpers.h @@ -52,5 +52,6 @@ struct bpf_insn; */ int get_xlated_program(int fd_prog, struct bpf_insn **buf, __u32 *cnt); int testing_prog_flags(void); +bool is_jit_enabled(void); #endif /* __TESTING_HELPERS_H */ -- 2.42.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH bpf-next v7 2/2] selftests/bpf: Skip callback tests if jit is disabled in test_verifier 2024-01-23 9:03 [PATCH bpf-next v7 0/2] Skip callback tests if jit is disabled in test_verifier Tiezhu Yang 2024-01-23 9:03 ` [PATCH bpf-next v7 1/2] selftests/bpf: Move is_jit_enabled() into testing_helpers Tiezhu Yang @ 2024-01-23 9:03 ` Tiezhu Yang 2024-01-23 19:51 ` Andrii Nakryiko 2024-01-24 4:50 ` [PATCH bpf-next v7 0/2] " patchwork-bot+netdevbpf 2 siblings, 1 reply; 5+ messages in thread From: Tiezhu Yang @ 2024-01-23 9:03 UTC (permalink / raw) To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko Cc: Eduard Zingerman, John Fastabend, Jiri Olsa, Hou Tao, Song Liu, bpf, linux-kernel If CONFIG_BPF_JIT_ALWAYS_ON is not set and bpf_jit_enable is 0, there exist 6 failed tests. [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable [root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled [root@linux bpf]# ./test_verifier | grep FAIL #106/p inline simple bpf_loop call FAIL #107/p don't inline bpf_loop call, flags non-zero FAIL #108/p don't inline bpf_loop call, callback non-constant FAIL #109/p bpf_loop_inline and a dead func FAIL #110/p bpf_loop_inline stack locations for loop vars FAIL #111/p inline bpf_loop call in a big program FAIL Summary: 768 PASSED, 15 SKIPPED, 6 FAILED The test log shows that callbacks are not allowed in non-JITed programs, interpreter doesn't support them yet, thus these tests should be skipped if jit is disabled. Add an explicit flag F_NEEDS_JIT_ENABLED to those tests to mark that they require JIT enabled in bpf_loop_inline.c, check the flag and jit_disabled at the beginning of do_test_single() to handle this case. With this patch: [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable [root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled [root@linux bpf]# ./test_verifier | grep FAIL Summary: 768 PASSED, 21 SKIPPED, 0 FAILED Suggested-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> --- tools/testing/selftests/bpf/test_verifier.c | 11 +++++++++++ .../testing/selftests/bpf/verifier/bpf_loop_inline.c | 6 ++++++ 2 files changed, 17 insertions(+) diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c index 1a09fc34d093..c65915188d7c 100644 --- a/tools/testing/selftests/bpf/test_verifier.c +++ b/tools/testing/selftests/bpf/test_verifier.c @@ -67,6 +67,7 @@ #define F_NEEDS_EFFICIENT_UNALIGNED_ACCESS (1 << 0) #define F_LOAD_WITH_STRICT_ALIGNMENT (1 << 1) +#define F_NEEDS_JIT_ENABLED (1 << 2) /* need CAP_BPF, CAP_NET_ADMIN, CAP_PERFMON to load progs */ #define ADMIN_CAPS (1ULL << CAP_NET_ADMIN | \ @@ -74,6 +75,7 @@ 1ULL << CAP_BPF) #define UNPRIV_SYSCTL "kernel/unprivileged_bpf_disabled" static bool unpriv_disabled = false; +static bool jit_disabled; static int skips; static bool verbose = false; static int verif_log_level = 0; @@ -1524,6 +1526,13 @@ static void do_test_single(struct bpf_test *test, bool unpriv, __u32 pflags; int i, err; + if ((test->flags & F_NEEDS_JIT_ENABLED) && jit_disabled) { + printf("SKIP (callbacks are not allowed in non-JITed programs)\n"); + skips++; + sched_yield(); + return; + } + fd_prog = -1; for (i = 0; i < MAX_NR_MAPS; i++) map_fds[i] = -1; @@ -1844,6 +1853,8 @@ int main(int argc, char **argv) return EXIT_FAILURE; } + jit_disabled = !is_jit_enabled(); + /* Use libbpf 1.0 API mode */ libbpf_set_strict_mode(LIBBPF_STRICT_ALL); diff --git a/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c b/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c index a535d41dc20d..59125b22ae39 100644 --- a/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c +++ b/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c @@ -57,6 +57,7 @@ .expected_insns = { PSEUDO_CALL_INSN() }, .unexpected_insns = { HELPER_CALL_INSN() }, .prog_type = BPF_PROG_TYPE_TRACEPOINT, + .flags = F_NEEDS_JIT_ENABLED, .result = ACCEPT, .runs = 0, .func_info = { { 0, MAIN_TYPE }, { 12, CALLBACK_TYPE } }, @@ -90,6 +91,7 @@ .expected_insns = { HELPER_CALL_INSN() }, .unexpected_insns = { PSEUDO_CALL_INSN() }, .prog_type = BPF_PROG_TYPE_TRACEPOINT, + .flags = F_NEEDS_JIT_ENABLED, .result = ACCEPT, .runs = 0, .func_info = { { 0, MAIN_TYPE }, { 16, CALLBACK_TYPE } }, @@ -127,6 +129,7 @@ .expected_insns = { HELPER_CALL_INSN() }, .unexpected_insns = { PSEUDO_CALL_INSN() }, .prog_type = BPF_PROG_TYPE_TRACEPOINT, + .flags = F_NEEDS_JIT_ENABLED, .result = ACCEPT, .runs = 0, .func_info = { @@ -165,6 +168,7 @@ .expected_insns = { PSEUDO_CALL_INSN() }, .unexpected_insns = { HELPER_CALL_INSN() }, .prog_type = BPF_PROG_TYPE_TRACEPOINT, + .flags = F_NEEDS_JIT_ENABLED, .result = ACCEPT, .runs = 0, .func_info = { @@ -235,6 +239,7 @@ }, .unexpected_insns = { HELPER_CALL_INSN() }, .prog_type = BPF_PROG_TYPE_TRACEPOINT, + .flags = F_NEEDS_JIT_ENABLED, .result = ACCEPT, .func_info = { { 0, MAIN_TYPE }, @@ -252,6 +257,7 @@ .unexpected_insns = { HELPER_CALL_INSN() }, .result = ACCEPT, .prog_type = BPF_PROG_TYPE_TRACEPOINT, + .flags = F_NEEDS_JIT_ENABLED, .func_info = { { 0, MAIN_TYPE }, { 16, CALLBACK_TYPE } }, .func_info_cnt = 2, BTF_TYPES -- 2.42.0 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v7 2/2] selftests/bpf: Skip callback tests if jit is disabled in test_verifier 2024-01-23 9:03 ` [PATCH bpf-next v7 2/2] selftests/bpf: Skip callback tests if jit is disabled in test_verifier Tiezhu Yang @ 2024-01-23 19:51 ` Andrii Nakryiko 0 siblings, 0 replies; 5+ messages in thread From: Andrii Nakryiko @ 2024-01-23 19:51 UTC (permalink / raw) To: Tiezhu Yang Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko, Eduard Zingerman, John Fastabend, Jiri Olsa, Hou Tao, Song Liu, bpf, linux-kernel On Tue, Jan 23, 2024 at 1:04 AM Tiezhu Yang <yangtiezhu@loongson.cn> wrote: > > If CONFIG_BPF_JIT_ALWAYS_ON is not set and bpf_jit_enable is 0, there > exist 6 failed tests. > > [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable > [root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled > [root@linux bpf]# ./test_verifier | grep FAIL > #106/p inline simple bpf_loop call FAIL > #107/p don't inline bpf_loop call, flags non-zero FAIL > #108/p don't inline bpf_loop call, callback non-constant FAIL > #109/p bpf_loop_inline and a dead func FAIL > #110/p bpf_loop_inline stack locations for loop vars FAIL > #111/p inline bpf_loop call in a big program FAIL > Summary: 768 PASSED, 15 SKIPPED, 6 FAILED > > The test log shows that callbacks are not allowed in non-JITed programs, > interpreter doesn't support them yet, thus these tests should be skipped > if jit is disabled. > > Add an explicit flag F_NEEDS_JIT_ENABLED to those tests to mark that they > require JIT enabled in bpf_loop_inline.c, check the flag and jit_disabled > at the beginning of do_test_single() to handle this case. > > With this patch: > > [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable > [root@linux bpf]# echo 0 > /proc/sys/kernel/unprivileged_bpf_disabled > [root@linux bpf]# ./test_verifier | grep FAIL > Summary: 768 PASSED, 21 SKIPPED, 0 FAILED > > Suggested-by: Andrii Nakryiko <andrii@kernel.org> > Signed-off-by: Tiezhu Yang <yangtiezhu@loongson.cn> > --- > tools/testing/selftests/bpf/test_verifier.c | 11 +++++++++++ > .../testing/selftests/bpf/verifier/bpf_loop_inline.c | 6 ++++++ > 2 files changed, 17 insertions(+) > > diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c > index 1a09fc34d093..c65915188d7c 100644 > --- a/tools/testing/selftests/bpf/test_verifier.c > +++ b/tools/testing/selftests/bpf/test_verifier.c > @@ -67,6 +67,7 @@ > > #define F_NEEDS_EFFICIENT_UNALIGNED_ACCESS (1 << 0) > #define F_LOAD_WITH_STRICT_ALIGNMENT (1 << 1) > +#define F_NEEDS_JIT_ENABLED (1 << 2) > > /* need CAP_BPF, CAP_NET_ADMIN, CAP_PERFMON to load progs */ > #define ADMIN_CAPS (1ULL << CAP_NET_ADMIN | \ > @@ -74,6 +75,7 @@ > 1ULL << CAP_BPF) > #define UNPRIV_SYSCTL "kernel/unprivileged_bpf_disabled" > static bool unpriv_disabled = false; > +static bool jit_disabled; > static int skips; > static bool verbose = false; > static int verif_log_level = 0; > @@ -1524,6 +1526,13 @@ static void do_test_single(struct bpf_test *test, bool unpriv, > __u32 pflags; > int i, err; > > + if ((test->flags & F_NEEDS_JIT_ENABLED) && jit_disabled) { > + printf("SKIP (callbacks are not allowed in non-JITed programs)\n"); This should be more generic "SKIP (test requires JIT)" or something. Whoever will apply this can fix it up, don't resend. > + skips++; > + sched_yield(); not sure why we need sched_yield(), tbh? It probably won't hurt, though. > + return; > + } > + > fd_prog = -1; > for (i = 0; i < MAX_NR_MAPS; i++) > map_fds[i] = -1; > @@ -1844,6 +1853,8 @@ int main(int argc, char **argv) > return EXIT_FAILURE; > } > > + jit_disabled = !is_jit_enabled(); > + > /* Use libbpf 1.0 API mode */ > libbpf_set_strict_mode(LIBBPF_STRICT_ALL); > > diff --git a/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c b/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c > index a535d41dc20d..59125b22ae39 100644 > --- a/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c > +++ b/tools/testing/selftests/bpf/verifier/bpf_loop_inline.c > @@ -57,6 +57,7 @@ > .expected_insns = { PSEUDO_CALL_INSN() }, > .unexpected_insns = { HELPER_CALL_INSN() }, > .prog_type = BPF_PROG_TYPE_TRACEPOINT, > + .flags = F_NEEDS_JIT_ENABLED, > .result = ACCEPT, > .runs = 0, > .func_info = { { 0, MAIN_TYPE }, { 12, CALLBACK_TYPE } }, > @@ -90,6 +91,7 @@ > .expected_insns = { HELPER_CALL_INSN() }, > .unexpected_insns = { PSEUDO_CALL_INSN() }, > .prog_type = BPF_PROG_TYPE_TRACEPOINT, > + .flags = F_NEEDS_JIT_ENABLED, > .result = ACCEPT, > .runs = 0, > .func_info = { { 0, MAIN_TYPE }, { 16, CALLBACK_TYPE } }, > @@ -127,6 +129,7 @@ > .expected_insns = { HELPER_CALL_INSN() }, > .unexpected_insns = { PSEUDO_CALL_INSN() }, > .prog_type = BPF_PROG_TYPE_TRACEPOINT, > + .flags = F_NEEDS_JIT_ENABLED, > .result = ACCEPT, > .runs = 0, > .func_info = { > @@ -165,6 +168,7 @@ > .expected_insns = { PSEUDO_CALL_INSN() }, > .unexpected_insns = { HELPER_CALL_INSN() }, > .prog_type = BPF_PROG_TYPE_TRACEPOINT, > + .flags = F_NEEDS_JIT_ENABLED, > .result = ACCEPT, > .runs = 0, > .func_info = { > @@ -235,6 +239,7 @@ > }, > .unexpected_insns = { HELPER_CALL_INSN() }, > .prog_type = BPF_PROG_TYPE_TRACEPOINT, > + .flags = F_NEEDS_JIT_ENABLED, > .result = ACCEPT, > .func_info = { > { 0, MAIN_TYPE }, > @@ -252,6 +257,7 @@ > .unexpected_insns = { HELPER_CALL_INSN() }, > .result = ACCEPT, > .prog_type = BPF_PROG_TYPE_TRACEPOINT, > + .flags = F_NEEDS_JIT_ENABLED, > .func_info = { { 0, MAIN_TYPE }, { 16, CALLBACK_TYPE } }, > .func_info_cnt = 2, > BTF_TYPES > -- > 2.42.0 > ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf-next v7 0/2] Skip callback tests if jit is disabled in test_verifier 2024-01-23 9:03 [PATCH bpf-next v7 0/2] Skip callback tests if jit is disabled in test_verifier Tiezhu Yang 2024-01-23 9:03 ` [PATCH bpf-next v7 1/2] selftests/bpf: Move is_jit_enabled() into testing_helpers Tiezhu Yang 2024-01-23 9:03 ` [PATCH bpf-next v7 2/2] selftests/bpf: Skip callback tests if jit is disabled in test_verifier Tiezhu Yang @ 2024-01-24 4:50 ` patchwork-bot+netdevbpf 2 siblings, 0 replies; 5+ messages in thread From: patchwork-bot+netdevbpf @ 2024-01-24 4:50 UTC (permalink / raw) To: Tiezhu Yang Cc: ast, daniel, andrii, eddyz87, john.fastabend, jolsa, houtao, song, bpf, linux-kernel Hello: This series was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <andrii@kernel.org>: On Tue, 23 Jan 2024 17:03:49 +0800 you wrote: > Thanks very much for the feedbacks from Eduard, John, Jiri, Daniel, > Hou Tao, Song Liu and Andrii. > > v7: > -- Add an explicit flag F_NEEDS_JIT_ENABLED for checking, > thanks Andrii. > > [...] Here is the summary with links: - [bpf-next,v7,1/2] selftests/bpf: Move is_jit_enabled() into testing_helpers https://git.kernel.org/bpf/bpf-next/c/15b4f88dcc0a - [bpf-next,v7,2/2] selftests/bpf: Skip callback tests if jit is disabled in test_verifier https://git.kernel.org/bpf/bpf-next/c/0b50478fd877 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-01-24 4:50 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-01-23 9:03 [PATCH bpf-next v7 0/2] Skip callback tests if jit is disabled in test_verifier Tiezhu Yang 2024-01-23 9:03 ` [PATCH bpf-next v7 1/2] selftests/bpf: Move is_jit_enabled() into testing_helpers Tiezhu Yang 2024-01-23 9:03 ` [PATCH bpf-next v7 2/2] selftests/bpf: Skip callback tests if jit is disabled in test_verifier Tiezhu Yang 2024-01-23 19:51 ` Andrii Nakryiko 2024-01-24 4:50 ` [PATCH bpf-next v7 0/2] " patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox