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
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 <[email protected]>
Acked-by: Hou Tao <[email protected]>
Acked-by: Song Liu <[email protected]>
---
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
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 <[email protected]>
Signed-off-by: Tiezhu Yang <[email protected]>
---
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
On Tue, Jan 23, 2024 at 1:04 AM Tiezhu Yang <[email protected]> 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 <[email protected]>
> Signed-off-by: Tiezhu Yang <[email protected]>
> ---
> 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
>
Hello:
This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <[email protected]>:
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