| Message ID | 20231020144839.2734006-1-memxor@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | da1055b673f3baac2249571c9882ce767a0aa746 |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] selftests/bpf: Make linked_list failure test more robust | expand |
| Context | Check | Description |
|---|---|---|
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next-VM_Test-0 | success | Logs for ShellCheck |
| bpf/vmtest-bpf-next-VM_Test-4 | success | Logs for build for x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-19 | success | Logs for test_progs_no_alu32_parallel on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-10 | success | Logs for test_progs on aarch64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-8 | success | Logs for test_maps on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-9 | success | Logs for test_maps on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-16 | success | Logs for test_progs_no_alu32 on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-22 | success | Logs for test_progs_parallel on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-25 | success | Logs for test_verifier on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-6 | success | Logs for test_maps on aarch64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-24 | success | Logs for test_verifier on aarch64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-3 | success | Logs for build for x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-21 | success | Logs for test_progs_parallel on aarch64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-7 | success | Logs for test_maps on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-11 | success | Logs for test_progs on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-23 | success | Logs for test_progs_parallel on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-12 | success | Logs for test_progs on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-1 | success | Logs for build for aarch64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-2 | success | Logs for build for s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-20 | success | Logs for test_progs_no_alu32_parallel on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-18 | success | Logs for test_progs_no_alu32_parallel on aarch64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-27 | success | Logs for test_verifier on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-15 | success | Logs for test_progs_no_alu32 on s390x with gcc |
| bpf/vmtest-bpf-next-VM_Test-13 | success | Logs for test_progs on x86_64 with llvm-16 |
| bpf/vmtest-bpf-next-VM_Test-28 | success | Logs for veristat |
| bpf/vmtest-bpf-next-VM_Test-5 | success | Logs for set-matrix |
| bpf/vmtest-bpf-next-VM_Test-14 | success | Logs for test_progs_no_alu32 on aarch64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-26 | success | Logs for test_verifier on x86_64 with gcc |
| bpf/vmtest-bpf-next-VM_Test-17 | success | Logs for test_progs_no_alu32 on x86_64 with llvm-16 |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/apply | success | Patch already applied to bpf-next |
On Fri, Oct 20, 2023 at 7:48 AM Kumar Kartikeya Dwivedi <memxor@gmail.com> wrote: > > The linked list failure test 'pop_front_off' and 'pop_back_off' > currently rely on matching exact instruction and register values. The > purpose of the test is to ensure the offset is correctly incremented for > the returned pointers from list pop helpers, which can then be used with > container_of to obtain the real object. Hence, somehow obtaining the > information that the offset is 48 will work for us. Make the test more > robust by relying on verifier error string of bpf_spin_lock and remove > dependence on fragile instruction index or register number, which can be > affected by different clang versions used to build the selftests. > > Fixes: 300f19dcdb99 ("selftests/bpf: Add BPF linked list API tests") > Reported-by: Andrii Nakryiko <andrii@kernel.org> > Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> > --- > tools/testing/selftests/bpf/prog_tests/linked_list.c | 10 ++-------- > tools/testing/selftests/bpf/progs/linked_list_fail.c | 4 +++- > 2 files changed, 5 insertions(+), 9 deletions(-) > Thanks for the fix! Applied to bpf-next. > diff --git a/tools/testing/selftests/bpf/prog_tests/linked_list.c b/tools/testing/selftests/bpf/prog_tests/linked_list.c > index 69dc31383b78..2fb89de63bd2 100644 > --- a/tools/testing/selftests/bpf/prog_tests/linked_list.c > +++ b/tools/testing/selftests/bpf/prog_tests/linked_list.c > @@ -94,14 +94,8 @@ static struct { > { "incorrect_head_var_off2", "variable ptr_ access var_off=(0x0; 0xffffffff) disallowed" }, > { "incorrect_head_off1", "bpf_list_head not found at offset=25" }, > { "incorrect_head_off2", "bpf_list_head not found at offset=1" }, > - { "pop_front_off", > - "15: (bf) r1 = r6 ; R1_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) " > - "R6_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) refs=2,4\n" > - "16: (85) call bpf_this_cpu_ptr#154\nR1 type=ptr_or_null_ expected=percpu_ptr_" }, > - { "pop_back_off", > - "15: (bf) r1 = r6 ; R1_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) " > - "R6_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) refs=2,4\n" > - "16: (85) call bpf_this_cpu_ptr#154\nR1 type=ptr_or_null_ expected=percpu_ptr_" }, > + { "pop_front_off", "off 48 doesn't point to 'struct bpf_spin_lock' that is at 40" }, > + { "pop_back_off", "off 48 doesn't point to 'struct bpf_spin_lock' that is at 40" }, > }; > > static void test_linked_list_fail_prog(const char *prog_name, const char *err_msg) > diff --git a/tools/testing/selftests/bpf/progs/linked_list_fail.c b/tools/testing/selftests/bpf/progs/linked_list_fail.c > index f4c63daba229..6438982b928b 100644 > --- a/tools/testing/selftests/bpf/progs/linked_list_fail.c > +++ b/tools/testing/selftests/bpf/progs/linked_list_fail.c > @@ -591,7 +591,9 @@ int pop_ptr_off(void *(*op)(void *head)) > n = op(&p->head); > bpf_spin_unlock(&p->lock); > > - bpf_this_cpu_ptr(n); > + if (!n) > + return 0; > + bpf_spin_lock((void *)n); > return 0; > } > > -- > 2.40.1 >
Hello: This patch was applied to bpf/bpf-next.git (master) by Andrii Nakryiko <andrii@kernel.org>: On Fri, 20 Oct 2023 14:48:39 +0000 you wrote: > The linked list failure test 'pop_front_off' and 'pop_back_off' > currently rely on matching exact instruction and register values. The > purpose of the test is to ensure the offset is correctly incremented for > the returned pointers from list pop helpers, which can then be used with > container_of to obtain the real object. Hence, somehow obtaining the > information that the offset is 48 will work for us. Make the test more > robust by relying on verifier error string of bpf_spin_lock and remove > dependence on fragile instruction index or register number, which can be > affected by different clang versions used to build the selftests. > > [...] Here is the summary with links: - [bpf-next] selftests/bpf: Make linked_list failure test more robust https://git.kernel.org/bpf/bpf-next/c/da1055b673f3 You are awesome, thank you!
diff --git a/tools/testing/selftests/bpf/prog_tests/linked_list.c b/tools/testing/selftests/bpf/prog_tests/linked_list.c index 69dc31383b78..2fb89de63bd2 100644 --- a/tools/testing/selftests/bpf/prog_tests/linked_list.c +++ b/tools/testing/selftests/bpf/prog_tests/linked_list.c @@ -94,14 +94,8 @@ static struct { { "incorrect_head_var_off2", "variable ptr_ access var_off=(0x0; 0xffffffff) disallowed" }, { "incorrect_head_off1", "bpf_list_head not found at offset=25" }, { "incorrect_head_off2", "bpf_list_head not found at offset=1" }, - { "pop_front_off", - "15: (bf) r1 = r6 ; R1_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) " - "R6_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) refs=2,4\n" - "16: (85) call bpf_this_cpu_ptr#154\nR1 type=ptr_or_null_ expected=percpu_ptr_" }, - { "pop_back_off", - "15: (bf) r1 = r6 ; R1_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) " - "R6_w=ptr_or_null_foo(id=4,ref_obj_id=4,off=48,imm=0) refs=2,4\n" - "16: (85) call bpf_this_cpu_ptr#154\nR1 type=ptr_or_null_ expected=percpu_ptr_" }, + { "pop_front_off", "off 48 doesn't point to 'struct bpf_spin_lock' that is at 40" }, + { "pop_back_off", "off 48 doesn't point to 'struct bpf_spin_lock' that is at 40" }, }; static void test_linked_list_fail_prog(const char *prog_name, const char *err_msg) diff --git a/tools/testing/selftests/bpf/progs/linked_list_fail.c b/tools/testing/selftests/bpf/progs/linked_list_fail.c index f4c63daba229..6438982b928b 100644 --- a/tools/testing/selftests/bpf/progs/linked_list_fail.c +++ b/tools/testing/selftests/bpf/progs/linked_list_fail.c @@ -591,7 +591,9 @@ int pop_ptr_off(void *(*op)(void *head)) n = op(&p->head); bpf_spin_unlock(&p->lock); - bpf_this_cpu_ptr(n); + if (!n) + return 0; + bpf_spin_lock((void *)n); return 0; }
The linked list failure test 'pop_front_off' and 'pop_back_off' currently rely on matching exact instruction and register values. The purpose of the test is to ensure the offset is correctly incremented for the returned pointers from list pop helpers, which can then be used with container_of to obtain the real object. Hence, somehow obtaining the information that the offset is 48 will work for us. Make the test more robust by relying on verifier error string of bpf_spin_lock and remove dependence on fragile instruction index or register number, which can be affected by different clang versions used to build the selftests. Fixes: 300f19dcdb99 ("selftests/bpf: Add BPF linked list API tests") Reported-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com> --- tools/testing/selftests/bpf/prog_tests/linked_list.c | 10 ++-------- tools/testing/selftests/bpf/progs/linked_list_fail.c | 4 +++- 2 files changed, 5 insertions(+), 9 deletions(-) -- 2.40.1