[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzYzpyZL+hQogXp-BaWEu6CFvWyicCOnGUxJawMpErLWRQ@mail.gmail.com>
Date: Tue, 2 Jul 2024 15:10:55 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Jiri Olsa <jolsa@...nel.org>
Cc: Oleg Nesterov <oleg@...hat.com>, Peter Zijlstra <peterz@...radead.org>,
Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>, bpf@...r.kernel.org, Martin KaFai Lau <kafai@...com>,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...omium.org>,
Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>, Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org
Subject: Re: [PATCHv2 bpf-next 9/9] selftests/bpf: Add uprobe session
consumers test
On Mon, Jul 1, 2024 at 9:44 AM Jiri Olsa <jolsa@...nel.org> wrote:
>
> Adding test that attached/detaches multiple consumers on
> single uprobe and verifies all were hit as expected.
>
> Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> ---
> .../bpf/prog_tests/uprobe_multi_test.c | 203 ++++++++++++++++++
> .../progs/uprobe_multi_session_consumers.c | 53 +++++
> 2 files changed, 256 insertions(+)
> create mode 100644 tools/testing/selftests/bpf/progs/uprobe_multi_session_consumers.c
>
This is clever, though bit notation obscures the meaning of the code a
bit. But thanks for the long comment explaining the overall idea.
> 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 b521590fdbb9..83eac954cf00 100644
> --- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
> @@ -9,6 +9,7 @@
> #include "uprobe_multi_session.skel.h"
> #include "uprobe_multi_session_cookie.skel.h"
> #include "uprobe_multi_session_recursive.skel.h"
> +#include "uprobe_multi_session_consumers.skel.h"
> #include "bpf/libbpf_internal.h"
> #include "testing_helpers.h"
> #include "../sdt.h"
> @@ -739,6 +740,206 @@ static void test_session_recursive_skel_api(void)
> uprobe_multi_session_recursive__destroy(skel);
> }
>
> +static int uprobe_attach(struct uprobe_multi_session_consumers *skel, int bit)
> +{
> + struct bpf_program **prog = &skel->progs.uprobe_0 + bit;
> + struct bpf_link **link = &skel->links.uprobe_0 + bit;
> + LIBBPF_OPTS(bpf_uprobe_multi_opts, opts);
> +
> + /*
> + * bit: 0,1 uprobe session
> + * bit: 2,3 uprobe entry
> + * bit: 4,5 uprobe return
> + */
> + opts.session = bit < 2;
> + opts.retprobe = bit == 4 || bit == 5;
> +
> + *link = bpf_program__attach_uprobe_multi(*prog, 0, "/proc/self/exe",
> + "uprobe_session_consumer_test",
> + &opts);
> + if (!ASSERT_OK_PTR(*link, "bpf_program__attach_uprobe_multi"))
> + return -1;
> + return 0;
> +}
> +
> +static void uprobe_detach(struct uprobe_multi_session_consumers *skel, int bit)
> +{
> + struct bpf_link **link = &skel->links.uprobe_0 + bit;
ok, this is nasty, no one guarantees this should keep working,
explicit switch would be preferable
> +
> + bpf_link__destroy(*link);
> + *link = NULL;
> +}
> +
> +static bool test_bit(int bit, unsigned long val)
> +{
> + return val & (1 << bit);
> +}
> +
> +noinline int
> +uprobe_session_consumer_test(struct uprobe_multi_session_consumers *skel,
> + unsigned long before, unsigned long after)
> +{
> + int bit;
> +
> + /* detach uprobe for each unset bit in 'before' state ... */
> + for (bit = 0; bit < 6; bit++) {
Does "bit" correspond to the uprobe_X program? Maybe call it an uprobe
index or something, if that's the case? bits are just representations,
but semantically meaningful is identifier of an uprobe program, right?
> + if (test_bit(bit, before) && !test_bit(bit, after))
> + uprobe_detach(skel, bit);
> + }
> +
> + /* ... and attach all new bits in 'after' state */
> + for (bit = 0; bit < 6; bit++) {
> + if (!test_bit(bit, before) && test_bit(bit, after)) {
> + if (!ASSERT_OK(uprobe_attach(skel, bit), "uprobe_attach_after"))
> + return -1;
> + }
> + }
> + return 0;
> +}
> +
[...]
> +
> +static void test_session_consumers(void)
> +{
> + struct uprobe_multi_session_consumers *skel;
> + int before, after;
> +
> + skel = uprobe_multi_session_consumers__open_and_load();
> + if (!ASSERT_OK_PTR(skel, "uprobe_multi_session_consumers__open_and_load"))
> + return;
> +
> + /*
> + * The idea of this test is to try all possible combinations of
> + * uprobes consumers attached on single function.
> + *
> + * - 1 uprobe session with return handler called
> + * - 1 uprobe session without return handler called
> + * - 2 uprobe entry consumer
> + * - 2 uprobe exit consumers
> + *
> + * The test uses 6 uprobes attached on single function, but that
> + * translates into single uprobe with 6 consumers in kernel.
> + *
> + * The before/after values present the state of attached consumers
> + * before and after the probed function:
> + *
> + * bit 0 : uprobe session with return
> + * bit 1 : uprobe session with no return
> + * bit 2,3 : uprobe entry
> + * bit 4,5 : uprobe return
> + *
> + * For example for:
> + *
> + * before = 0b10101
> + * after = 0b00110
> + *
> + * it means that before we call 'uprobe_session_consumer_test' we
> + * attach uprobes defined in 'before' value:
> + *
> + * - bit 0: uprobe session with return
> + * - bit 2: uprobe entry
> + * - bit 4: uprobe return
> + *
> + * uprobe_session_consumer_test is called and inside it we attach
> + * and detach * uprobes based on 'after' value:
> + *
> + * - bit 0: uprobe session with return is detached
> + * - bit 1: uprobe session without return is attached
> + * - bit 2: stays untouched
> + * - bit 4: uprobe return is detached
> + *
> + * uprobe_session_consumer_test returs and we check counters values
> + * increased by bpf programs on each uprobe to match the expected
> + * count based on before/after bits.
> + */
> + for (before = 0; before < 64; before++) {
> + for (after = 0; after < 64; after++)
> + session_consumer_test(skel, before, after);
> + }
> +
> + uprobe_multi_session_consumers__destroy(skel);
> +}
> +
[...]
Powered by blists - more mailing lists