| Message ID | 20240520234720.1748918-6-andrii@kernel.org (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | BPF |
| Headers | show |
| Series | Fix BPF multi-uprobe PID filtering logic | expand |
On Mon, May 20, 2024 at 4:47 PM Andrii Nakryiko <andrii@kernel.org> 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@kernel.org> > --- > .../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. 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).
On Mon, May 20, 2024 at 9:54 PM Andrii Nakryiko <andrii.nakryiko@gmail.com> wrote: > > On Mon, May 20, 2024 at 4:47 PM Andrii Nakryiko <andrii@kernel.org> 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@kernel.org> > > --- > > .../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. pls wait for review from Jiri. > 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 whole thing through bpf tree makes it the easiest for everyone.
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@kernel.org> 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@kernel.org> > > --- > > .../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
On Tue, May 21, 2024 at 3:04 AM Jiri Olsa <olsajiri@gmail.com> wrote: > > 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@kernel.org> 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@kernel.org> > > > --- > > > .../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? no one should be building libbpf from kernel sources, Github is the authoritative source code for packaging. So I think it's not necessary to backport libbpf. Tests will keep passing with just patch #1. > > jirka
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; +}
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@kernel.org> --- .../bpf/prog_tests/uprobe_multi_test.c | 22 +++++++++++++ .../selftests/bpf/progs/uprobe_multi.c | 33 +++++++++++++++++-- 2 files changed, 53 insertions(+), 2 deletions(-)