Re: [PATCH bpf 5/5] selftests/bpf: extend multi-uprobe tests with USDTs

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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.

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).





[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux