On Tue, Nov 28, 2023 at 4:55 AM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > On Mon, 2023-11-27 at 22:15 -0500, Andrei Matei wrote: > > On Mon, Nov 27, 2023 at 8:23 PM Eduard Zingerman <eddyz87@xxxxxxxxx> wrote: > > > > > > On Sat, 2023-11-25 at 20:50 -0500, Andrei Matei wrote: > > > > This patch adds tests for the previous patch, checking the tracking of > > > > the maximum stack size and checking that accesses to uninit stack memory > > > > are allowed. > > > > > > > > They are a separate patch for review purposes; whoever merges them can > > > > consider squashing. > > > > > > > > Signed-off-by: Andrei Matei <andreimatei1@xxxxxxxxx> > > > > --- > > > > > > I think the strategy now is to add new tests using inline assembly, > > > e.g. as a part of verifier_* tests in test_progs. > > > > Thanks, I'll try that. I see in some of the verifier tests that you > > have converted to test_progs hints that they were converted > > "automatically". I'm curious what tool you've used, if any. > > I wrote a small tree-sitter based tool for this: > https://github.com/eddyz87/verifier-tests-migrator > > > Do you have any thoughts on how a test could assert the maximum stack > > depth? test_verifier has access to the verifier verifier log and > > parses it out of there; do you know how I could achieve the same in > > test_progs? > > Could be done like in the patch below (set log level so that stack > depth is printed, match stack depth message): > > diff --git a/tools/testing/selftests/bpf/progs/verifier_basic_stack.c b/tools/testing/selftests/bpf/progs/verifier_basic_stack.c > index 069c3f91705c..e85ac95c8dd3 100644 > --- a/tools/testing/selftests/bpf/progs/verifier_basic_stack.c > +++ b/tools/testing/selftests/bpf/progs/verifier_basic_stack.c > @@ -27,7 +27,7 @@ __naked void stack_out_of_bounds(void) > > SEC("socket") > __description("uninitialized stack1") > -__success > +__success __log_level(4) __msg("stack depth 8") > __failure_unpriv __msg_unpriv("invalid indirect read from stack") > __naked void uninitialized_stack1(void) > { > > > For the curiosity of someone who doesn't know much about this code > > base - how come we moved from test_verifier, which seems a bit more > > unit-test-y, do the higher level test_progs? Is it because of the > > nicer assembly syntax? > > Yes, ability to use assembly syntax is the main (sole?) driver. Not just that, test_progs by itself is a much nicer test runner for debugging tests. It's easier to get log_level 2 logs, for example. It's much easier to select a subset of tests to run. Plus by having these BPF programs compiled as stand-alone .bpf.o objects, we can use veristat to test them, bypassing test_progs altogether. > In fact, tests that use annotations from bpf_misc.h and are registered > in tools/testing/selftests/bpf/prog_tests/verifier.c provide almost > the same functionality as test_verifier: > - interface: > - select test to run using filter, e.g.: > ./test_progs -a 'verifier_basic_stack/uninitialized stack1' > ./test_progs -a 'verifier_basic_stack/uninitialized stack1 @unpriv' > - see verifier log for the test, e.g.: > ./test_progs -vvv -a 'verifier_basic_stack/uninitialized stack1' > - test expectations: > - use __success / __failure to mark expected test outcome; > - use __msg to search for expected messages in the log; > - use __log_level to specify log level when program is loaded; > - use __flags to enable additional knobs, e.g. BPF_F_TEST_STATE_FREQ; > - use __retval if test program has to be executed; > - use _unpriv suffix to specify any of the above for when test is > executed in unprivileged mode. > See comments in tools/testing/selftests/bpf/progs/bpf_misc.h for details. > - plus clang generates BTF and CO-RE relocations for us, > with test_verifier one has to write these things "by hand" > (which is only truly needed in tests for CO-RE/BTF). > > The only drawback is compilation time, because all test_progs/*.c > files depend on all *.bpf.o files. Locally I mitigate this by using > ccache: make CC='ccache clang' LLVM=1 -j14 test_progs . > I probably need to look at what could be done in the makefile one day. I think if we make a push to use BPF skeletons in all tests, we should be able to get proper dependencies between prog_tests/xxx.c and its corresponding progs/yyy.bpf.o file(s). And at which point we should be able to teach Makefile to only recompile relevant test files. > > Unfortunately neither test_verifier nor test_progs are true unit tests.
