| Message ID | YVnfFTL/3T6jOwHI@krava (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | fba27f590fd3bc98834a813ae087a8ea35115b2b |
| Delegated to: | BPF |
| Headers | show |
| Series | [bpf-next] selftest/bpf: Switch recursion test to use htab_map_delete_elem | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for bpf-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | warning | 4 maintainers not CCed: shuah@kernel.org linux-kselftest@vger.kernel.org kpsingh@kernel.org andrii@kernel.org |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | warning | WARNING: From:/Signed-off-by: email address mismatch: 'From: Jiri Olsa <jolsa@redhat.com>' != 'Signed-off-by: Jiri Olsa <jolsa@kernel.org>' |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
| bpf/vmtest-bpf-next-PR | success | PR summary |
| bpf/vmtest-bpf-next | success | VM_Test |
On Sun, Oct 3, 2021 at 9:58 AM Jiri Olsa <jolsa@redhat.com> wrote: > > Currently the recursion test is hooking __htab_map_lookup_elem > function, which is invoked both from bpf_prog and bpf syscall. > > But in our kernel build, the __htab_map_lookup_elem gets inlined > within the htab_map_lookup_elem, so it's not trigered and the > test fails. > > Fixing this by using htab_map_delete_elem, which is not inlined > for bpf_prog calls (like htab_map_lookup_elem is) and is used > directly as pointer for map_delete_elem, so it won't disappear > by inlining. > > Signed-off-by: Jiri Olsa <jolsa@kernel.org> Acked-by: Song Liu <songliubraving@fb.com> > --- > tools/testing/selftests/bpf/prog_tests/recursion.c | 10 +++++----- > tools/testing/selftests/bpf/progs/recursion.c | 9 +++------ > 2 files changed, 8 insertions(+), 11 deletions(-) > > diff --git a/tools/testing/selftests/bpf/prog_tests/recursion.c b/tools/testing/selftests/bpf/prog_tests/recursion.c > index 0e378d63fe18..f3af2627b599 100644 > --- a/tools/testing/selftests/bpf/prog_tests/recursion.c > +++ b/tools/testing/selftests/bpf/prog_tests/recursion.c > @@ -20,18 +20,18 @@ void test_recursion(void) > goto out; > > ASSERT_EQ(skel->bss->pass1, 0, "pass1 == 0"); > - bpf_map_lookup_elem(bpf_map__fd(skel->maps.hash1), &key, 0); > + bpf_map_delete_elem(bpf_map__fd(skel->maps.hash1), &key); > ASSERT_EQ(skel->bss->pass1, 1, "pass1 == 1"); > - bpf_map_lookup_elem(bpf_map__fd(skel->maps.hash1), &key, 0); > + bpf_map_delete_elem(bpf_map__fd(skel->maps.hash1), &key); > ASSERT_EQ(skel->bss->pass1, 2, "pass1 == 2"); > > ASSERT_EQ(skel->bss->pass2, 0, "pass2 == 0"); > - bpf_map_lookup_elem(bpf_map__fd(skel->maps.hash2), &key, 0); > + bpf_map_delete_elem(bpf_map__fd(skel->maps.hash2), &key); > ASSERT_EQ(skel->bss->pass2, 1, "pass2 == 1"); > - bpf_map_lookup_elem(bpf_map__fd(skel->maps.hash2), &key, 0); > + bpf_map_delete_elem(bpf_map__fd(skel->maps.hash2), &key); > ASSERT_EQ(skel->bss->pass2, 2, "pass2 == 2"); > > - err = bpf_obj_get_info_by_fd(bpf_program__fd(skel->progs.on_lookup), > + err = bpf_obj_get_info_by_fd(bpf_program__fd(skel->progs.on_delete), > &prog_info, &prog_info_len); > if (!ASSERT_OK(err, "get_prog_info")) > goto out; > diff --git a/tools/testing/selftests/bpf/progs/recursion.c b/tools/testing/selftests/bpf/progs/recursion.c > index 49f679375b9d..3c2423bb19e2 100644 > --- a/tools/testing/selftests/bpf/progs/recursion.c > +++ b/tools/testing/selftests/bpf/progs/recursion.c > @@ -24,8 +24,8 @@ struct { > int pass1 = 0; > int pass2 = 0; > > -SEC("fentry/__htab_map_lookup_elem") > -int BPF_PROG(on_lookup, struct bpf_map *map) > +SEC("fentry/htab_map_delete_elem") > +int BPF_PROG(on_delete, struct bpf_map *map) > { > int key = 0; > > @@ -35,10 +35,7 @@ int BPF_PROG(on_lookup, struct bpf_map *map) > } > if (map == (void *)&hash2) { > pass2++; > - /* htab_map_gen_lookup() will inline below call > - * into direct call to __htab_map_lookup_elem() > - */ > - bpf_map_lookup_elem(&hash2, &key); > + bpf_map_delete_elem(&hash2, &key); > return 0; > } > > -- > 2.31.1 >
Hello: This patch was applied to bpf/bpf-next.git (refs/heads/master): On Sun, 3 Oct 2021 18:49:25 +0200 you wrote: > Currently the recursion test is hooking __htab_map_lookup_elem > function, which is invoked both from bpf_prog and bpf syscall. > > But in our kernel build, the __htab_map_lookup_elem gets inlined > within the htab_map_lookup_elem, so it's not trigered and the > test fails. > > [...] Here is the summary with links: - [bpf-next] selftest/bpf: Switch recursion test to use htab_map_delete_elem https://git.kernel.org/bpf/bpf-next/c/fba27f590fd3 You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
diff --git a/tools/testing/selftests/bpf/prog_tests/recursion.c b/tools/testing/selftests/bpf/prog_tests/recursion.c index 0e378d63fe18..f3af2627b599 100644 --- a/tools/testing/selftests/bpf/prog_tests/recursion.c +++ b/tools/testing/selftests/bpf/prog_tests/recursion.c @@ -20,18 +20,18 @@ void test_recursion(void) goto out; ASSERT_EQ(skel->bss->pass1, 0, "pass1 == 0"); - bpf_map_lookup_elem(bpf_map__fd(skel->maps.hash1), &key, 0); + bpf_map_delete_elem(bpf_map__fd(skel->maps.hash1), &key); ASSERT_EQ(skel->bss->pass1, 1, "pass1 == 1"); - bpf_map_lookup_elem(bpf_map__fd(skel->maps.hash1), &key, 0); + bpf_map_delete_elem(bpf_map__fd(skel->maps.hash1), &key); ASSERT_EQ(skel->bss->pass1, 2, "pass1 == 2"); ASSERT_EQ(skel->bss->pass2, 0, "pass2 == 0"); - bpf_map_lookup_elem(bpf_map__fd(skel->maps.hash2), &key, 0); + bpf_map_delete_elem(bpf_map__fd(skel->maps.hash2), &key); ASSERT_EQ(skel->bss->pass2, 1, "pass2 == 1"); - bpf_map_lookup_elem(bpf_map__fd(skel->maps.hash2), &key, 0); + bpf_map_delete_elem(bpf_map__fd(skel->maps.hash2), &key); ASSERT_EQ(skel->bss->pass2, 2, "pass2 == 2"); - err = bpf_obj_get_info_by_fd(bpf_program__fd(skel->progs.on_lookup), + err = bpf_obj_get_info_by_fd(bpf_program__fd(skel->progs.on_delete), &prog_info, &prog_info_len); if (!ASSERT_OK(err, "get_prog_info")) goto out; diff --git a/tools/testing/selftests/bpf/progs/recursion.c b/tools/testing/selftests/bpf/progs/recursion.c index 49f679375b9d..3c2423bb19e2 100644 --- a/tools/testing/selftests/bpf/progs/recursion.c +++ b/tools/testing/selftests/bpf/progs/recursion.c @@ -24,8 +24,8 @@ struct { int pass1 = 0; int pass2 = 0; -SEC("fentry/__htab_map_lookup_elem") -int BPF_PROG(on_lookup, struct bpf_map *map) +SEC("fentry/htab_map_delete_elem") +int BPF_PROG(on_delete, struct bpf_map *map) { int key = 0; @@ -35,10 +35,7 @@ int BPF_PROG(on_lookup, struct bpf_map *map) } if (map == (void *)&hash2) { pass2++; - /* htab_map_gen_lookup() will inline below call - * into direct call to __htab_map_lookup_elem() - */ - bpf_map_lookup_elem(&hash2, &key); + bpf_map_delete_elem(&hash2, &key); return 0; }
Currently the recursion test is hooking __htab_map_lookup_elem function, which is invoked both from bpf_prog and bpf syscall. But in our kernel build, the __htab_map_lookup_elem gets inlined within the htab_map_lookup_elem, so it's not trigered and the test fails. Fixing this by using htab_map_delete_elem, which is not inlined for bpf_prog calls (like htab_map_lookup_elem is) and is used directly as pointer for map_delete_elem, so it won't disappear by inlining. Signed-off-by: Jiri Olsa <jolsa@kernel.org> --- tools/testing/selftests/bpf/prog_tests/recursion.c | 10 +++++----- tools/testing/selftests/bpf/progs/recursion.c | 9 +++------ 2 files changed, 8 insertions(+), 11 deletions(-)