On Mon, May 20, 2024 at 09:54:40PM -0700, Andrii Nakryiko wrote: > On Mon, May 20, 2024 at 4:47 PM Andrii Nakryiko <andrii@xxxxxxxxxx> wrote: > > > > Validate libbpf's USDT-over-multi-uprobe logic by adding USDTs to > > existing multi-uprobe tests. This checks correct libbpf fallback to > > singular uprobes (when run on older kernels with buggy PID filtering). > > We reuse already established child process and child thread testing > > infrastructure, so additions are minimal. These test fail on either > > older kernels or older version of libbpf that doesn't detect PID > > filtering problems. > > > > Signed-off-by: Andrii Nakryiko <andrii@xxxxxxxxxx> > > --- > > .../bpf/prog_tests/uprobe_multi_test.c | 22 +++++++++++++ > > .../selftests/bpf/progs/uprobe_multi.c | 33 +++++++++++++++++-- > > 2 files changed, 53 insertions(+), 2 deletions(-) > > > > diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c > > index 677232d31432..85d46e568e90 100644 > > --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c > > +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c > > @@ -8,6 +8,7 @@ > > #include "uprobe_multi_usdt.skel.h" > > #include "bpf/libbpf_internal.h" > > #include "testing_helpers.h" > > +#include "../sdt.h" > > > > static char test_data[] = "test_data"; > > > > @@ -26,6 +27,11 @@ noinline void uprobe_multi_func_3(void) > > asm volatile (""); > > } > > > > +noinline void usdt_trigger(void) > > +{ > > + STAP_PROBE(test, pid_filter_usdt); > > +} > > + > > struct child { > > int go[2]; > > int c2p[2]; /* child -> parent channel */ > > @@ -269,8 +275,24 @@ __test_attach_api(const char *binary, const char *pattern, struct bpf_uprobe_mul > > if (!ASSERT_OK_PTR(skel->links.uprobe_extra, "bpf_program__attach_uprobe_multi")) > > goto cleanup; > > > > + /* Attach (uprobe-backed) USDTs */ > > + skel->links.usdt_pid = bpf_program__attach_usdt(skel->progs.usdt_pid, pid, binary, > > + "test", "pid_filter_usdt", NULL); > > + if (!ASSERT_OK_PTR(skel->links.usdt_pid, "attach_usdt_pid")) > > + goto cleanup; > > + > > + skel->links.usdt_extra = bpf_program__attach_usdt(skel->progs.usdt_extra, -1, binary, > > + "test", "pid_filter_usdt", NULL); > > + if (!ASSERT_OK_PTR(skel->links.usdt_extra, "attach_usdt_extra")) > > + goto cleanup; > > + > > uprobe_multi_test_run(skel, child); > > > > + ASSERT_FALSE(skel->bss->bad_pid_seen_usdt, "bad_pid_seen_usdt"); > > + if (child) { > > + ASSERT_EQ(skel->bss->child_pid_usdt, child->pid, "usdt_multi_child_pid"); > > + ASSERT_EQ(skel->bss->child_tid_usdt, child->tid, "usdt_multi_child_tid"); > > + } > > cleanup: > > uprobe_multi__destroy(skel); > > } > > diff --git a/tools/testing/selftests/bpf/progs/uprobe_multi.c b/tools/testing/selftests/bpf/progs/uprobe_multi.c > > index 86a7ff5d3726..44190efcdba2 100644 > > --- a/tools/testing/selftests/bpf/progs/uprobe_multi.c > > +++ b/tools/testing/selftests/bpf/progs/uprobe_multi.c > > @@ -1,8 +1,8 @@ > > // SPDX-License-Identifier: GPL-2.0 > > -#include <linux/bpf.h> > > +#include "vmlinux.h" > > #include <bpf/bpf_helpers.h> > > #include <bpf/bpf_tracing.h> > > -#include <stdbool.h> > > +#include <bpf/usdt.bpf.h> > > > > char _license[] SEC("license") = "GPL"; > > > > @@ -23,9 +23,12 @@ __u64 uprobe_multi_sleep_result = 0; > > int pid = 0; > > int child_pid = 0; > > int child_tid = 0; > > +int child_pid_usdt = 0; > > +int child_tid_usdt = 0; > > > > int expect_pid = 0; > > bool bad_pid_seen = false; > > +bool bad_pid_seen_usdt = false; > > > > bool test_cookie = false; > > void *user_ptr = 0; > > @@ -112,3 +115,29 @@ int uprobe_extra(struct pt_regs *ctx) > > /* we need this one just to mix PID-filtered and global uprobes */ > > return 0; > > } > > + > > +SEC("usdt") > > +int usdt_pid(struct pt_regs *ctx) > > +{ > > + __u64 cur_pid_tgid = bpf_get_current_pid_tgid(); > > + __u32 cur_pid; > > + > > + cur_pid = cur_pid_tgid >> 32; > > + if (pid && cur_pid != pid) > > + return 0; > > + > > + if (expect_pid && cur_pid != expect_pid) > > + bad_pid_seen_usdt = true; > > + > > + child_pid_usdt = cur_pid_tgid >> 32; > > + child_tid_usdt = (__u32)cur_pid_tgid; > > + > > + return 0; > > +} > > + > > +SEC("usdt") > > +int usdt_extra(struct pt_regs *ctx) > > +{ > > + /* we need this one just to mix PID-filtered and global USDT probes */ > > + return 0; > > +} > > -- > > 2.43.0 > > > > I lost the following during the final rebase before submitting, > sigh... With the piece below tests are passing again: > > diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c > b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c > index 85d46e568e90..bf6ca8e3eb13 100644 > --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c > +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c > @@ -96,6 +96,7 @@ static struct child *spawn_child(void) > uprobe_multi_func_1(); > uprobe_multi_func_2(); > uprobe_multi_func_3(); > + usdt_trigger(); > > exit(errno); > } > @@ -123,6 +124,7 @@ static void *child_thread(void *ctx) > uprobe_multi_func_1(); > uprobe_multi_func_2(); > uprobe_multi_func_3(); > + usdt_trigger(); > > err = 0; > pthread_exit(&err); > @@ -188,6 +190,7 @@ static void uprobe_multi_test_run(struct > uprobe_multi *skel, struct child *child > uprobe_multi_func_1(); > uprobe_multi_func_2(); > uprobe_multi_func_3(); > + usdt_trigger(); > } > > if (child) > > > I'll wait till tomorrow for any feedback and will post v2. tests are passing for me with the changes above > > I'm also curious about logistics? Do we want to get everything through > the bpf tree? Or bpf-next? Or split somehow? Thoughts? > > I think the fix in patch #1 is important enough to backport to stable > kernels (multi-uprobes went into upstream v6.6 kernel, FYI). agreed, perhaps also the patch #3 for libbpf detection? jirka