Eduard Zingerman wrote: > On Mon, 2023-12-25 at 17:18 +0800, Tiezhu Yang 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]# ./test_verifier | grep FAIL > > #107/p inline simple bpf_loop call FAIL > > #108/p don't inline bpf_loop call, flags non-zero FAIL > > #109/p don't inline bpf_loop call, callback non-constant FAIL > > #110/p bpf_loop_inline and a dead func FAIL > > #111/p bpf_loop_inline stack locations for loop vars FAIL > > #112/p inline bpf_loop call in a big program FAIL > > Summary: 505 PASSED, 266 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, just return -ENOTSUPP instead of -EINVAL for pseudo > > calls in fixup_call_args(). > > > > With this patch: > > > > [root@linux bpf]# echo 0 > /proc/sys/net/core/bpf_jit_enable > > [root@linux bpf]# ./test_verifier | grep FAIL > > Summary: 505 PASSED, 272 SKIPPED, 0 FAILED > > > > Signed-off-by: Tiezhu Yang <yangtiezhu@xxxxxxxxxxx> > > --- > > kernel/bpf/verifier.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c > > index a376eb609c41..1c780a893284 100644 > > --- a/kernel/bpf/verifier.c > > +++ b/kernel/bpf/verifier.c > > @@ -19069,7 +19069,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 -ENOTSUPP; > > } > > > > if (!bpf_pseudo_call(insn)) > > I agree with this change, however I think that it should be consistent. > Quick and non-exhaustive grepping shows that there are 4 places where > "non-JITed" is used in error messages: in check_map_func_compatibility() > and in fixup_call_args(). > All these places currently use -EINVAL and should be updated to -ENOTSUPP, > if this change gets a green light. My preference is to just leave them as is unless its a serious problem. In this case any userspace has likely already figured out how to handle these errors so I don't think there is much value in changing things. > > If the goal is to merely make test_verifier pass when JIT is disabled > there is a different way: > - test_progs has a global variable 'env.jit_enabled' which is used by > several tests to decide whether to skip or not; > - loop inlining tests could use similar feature, but unfortunately > test_verifier does not provide similar functionality; > - test_verifier is sort-of in legacy mode, so instead of porting the > jit_enabled to test_verifier, I think that loop inlining tests > should be migrated to test_progs. I can do that some time after [1] > would be landed. > > [1] https://lore.kernel.org/bpf/20231223104042.1432300-3-houtao@xxxxxxxxxxxxxxx/ >
