linux-kernel - Re: [PATCH bpf-next v4 2/2] selftests/bpf: Add stacktrace map lookup_and_delete

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAEf4BzbwkmeiRb5v3TRLxNEywvtn7tynYu850E-sh8Z--hM-dg@mail.gmail.com>
Date: Mon, 22 Sep 2025 13:58:22 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Tao Chen <chen.dylane@...ux.dev>
Cc: ast@...nel.org, daniel@...earbox.net, andrii@...nel.org, 
	martin.lau@...ux.dev, eddyz87@...il.com, song@...nel.org, 
	yonghong.song@...ux.dev, john.fastabend@...il.com, kpsingh@...nel.org, 
	sdf@...ichev.me, haoluo@...gle.com, jolsa@...nel.org, bpf@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH bpf-next v4 2/2] selftests/bpf: Add stacktrace map
 lookup_and_delete_elem test case

On Mon, Sep 22, 2025 at 7:03 AM Tao Chen <chen.dylane@...ux.dev> wrote:
>
> Add tests for stacktrace map lookup and delete:
> 1. use bpf_map_lookup_and_delete_elem to lookup and delete the target
>    stack_id,
> 2. lookup the deleted stack_id again to double check.
>
> Signed-off-by: Tao Chen <chen.dylane@...ux.dev>
> ---
>  .../selftests/bpf/prog_tests/stacktrace_map.c | 21 ++++++++++++++++++-
>  .../selftests/bpf/progs/test_stacktrace_map.c |  8 +++++++
>  2 files changed, 28 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
> index 84a7e405e91..d50659fc25e 100644
> --- a/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
> +++ b/tools/testing/selftests/bpf/prog_tests/stacktrace_map.c
> @@ -3,7 +3,7 @@
>
>  void test_stacktrace_map(void)
>  {
> -       int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd;
> +       int control_map_fd, stackid_hmap_fd, stackmap_fd, stack_amap_fd, stack_key_map_fd;
>         const char *prog_name = "oncpu";
>         int err, prog_fd, stack_trace_len;
>         const char *file = "./test_stacktrace_map.bpf.o";
> @@ -11,6 +11,8 @@ void test_stacktrace_map(void)
>         struct bpf_program *prog;
>         struct bpf_object *obj;
>         struct bpf_link *link;
> +       __u32 stack_id;
> +       char val_buf[PERF_MAX_STACK_DEPTH * sizeof(struct bpf_stack_build_id)];
>
>         err = bpf_prog_test_load(file, BPF_PROG_TYPE_TRACEPOINT, &obj, &prog_fd);
>         if (CHECK(err, "prog_load", "err %d errno %d\n", err, errno))
> @@ -41,6 +43,10 @@ void test_stacktrace_map(void)
>         if (CHECK_FAIL(stack_amap_fd < 0))
>                 goto disable_pmu;
>
> +       stack_key_map_fd = bpf_find_map(__func__, obj, "stack_key_map");
> +       if (CHECK_FAIL(stack_key_map_fd < 0))

please don't use CHECK*() macros, they are superseded by more targeted
ASSERT_xxx() ones

pw-bot: cr


> +               goto disable_pmu;
> +
>         /* give some time for bpf program run */
>         sleep(1);
>
> @@ -68,6 +74,19 @@ void test_stacktrace_map(void)
>                   "err %d errno %d\n", err, errno))
>                 goto disable_pmu;
>
> +       err = bpf_map_lookup_elem(stack_key_map_fd, &key, &stack_id);
> +       if (CHECK(err, "stack_key_map lookup", "err %d errno %d\n", err, errno))
> +               goto disable_pmu;
> +
> +       err = bpf_map_lookup_and_delete_elem(stackmap_fd, &stack_id, &val_buf);
> +       if (CHECK(err, "stackmap lookup and delete",
> +                 "err %d errno %d\n", err, errno))
> +               goto disable_pmu;
> +
> +       err = bpf_map_lookup_elem(stackmap_fd, &stack_id, &val_buf);
> +       CHECK((!err || errno != ENOENT), "stackmap lookup deleted stack_id",
> +             "err %d errno %d\n", err, errno);

bpf_map_lookup_elem() returns error code directly, no need to use
errno, just check that err == -ENOENT

> +
>  disable_pmu:
>         bpf_link__destroy(link);
>  close_prog:
> diff --git a/tools/testing/selftests/bpf/progs/test_stacktrace_map.c b/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
> index 47568007b66..3bede76c151 100644
> --- a/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
> +++ b/tools/testing/selftests/bpf/progs/test_stacktrace_map.c
> @@ -38,6 +38,13 @@ struct {
>         __type(value, stack_trace_t);
>  } stack_amap SEC(".maps");
>
> +struct {
> +       __uint(type, BPF_MAP_TYPE_ARRAY);
> +       __uint(max_entries, 1);
> +       __type(key, __u32);
> +       __type(value, __u32);
> +} stack_key_map SEC(".maps");
> +
>  /* taken from /sys/kernel/tracing/events/sched/sched_switch/format */
>  struct sched_switch_args {
>         unsigned long long pad;
> @@ -64,6 +71,7 @@ int oncpu(struct sched_switch_args *ctx)
>         /* The size of stackmap and stackid_hmap should be the same */
>         key = bpf_get_stackid(ctx, &stackmap, 0);
>         if ((int)key >= 0) {
> +               bpf_map_update_elem(&stack_key_map, &val, &key, 0);

ugh... you'd just use a global variable if this test was used through
skeleton... maybe convert the test to skeleton and get rid of all
those unnecessary bpf_find_map() calls as well?

>                 bpf_map_update_elem(&stackid_hmap, &key, &val, 0);
>                 stack_p = bpf_map_lookup_elem(&stack_amap, &key);
>                 if (stack_p)
> --
> 2.48.1
>