[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <Zi9OwCwluxTo-Azd@krava>
Date: Mon, 29 Apr 2024 09:39:44 +0200
From: Jiri Olsa <olsajiri@...il.com>
To: Andrii Nakryiko <andrii.nakryiko@...il.com>
Cc: Steven Rostedt <rostedt@...dmis.org>,
Masami Hiramatsu <mhiramat@...nel.org>,
Oleg Nesterov <oleg@...hat.com>,
Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>, linux-kernel@...r.kernel.org,
linux-trace-kernel@...r.kernel.org, linux-api@...r.kernel.org,
x86@...nel.org, bpf@...r.kernel.org,
Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
John Fastabend <john.fastabend@...il.com>,
Peter Zijlstra <peterz@...radead.org>,
Thomas Gleixner <tglx@...utronix.de>,
"Borislav Petkov (AMD)" <bp@...en8.de>,
Ingo Molnar <mingo@...hat.com>, Andy Lutomirski <luto@...nel.org>
Subject: Re: [PATCHv3 bpf-next 6/7] selftests/bpf: Add uretprobe compat test
On Fri, Apr 26, 2024 at 11:06:53AM -0700, Andrii Nakryiko wrote:
> On Sun, Apr 21, 2024 at 12:43 PM Jiri Olsa <jolsa@...nel.org> wrote:
> >
> > Adding test that adds return uprobe inside 32 bit task
> > and verify the return uprobe and attached bpf programs
> > get properly executed.
> >
> > Signed-off-by: Jiri Olsa <jolsa@...nel.org>
> > ---
> > tools/testing/selftests/bpf/.gitignore | 1 +
> > tools/testing/selftests/bpf/Makefile | 6 ++-
> > .../selftests/bpf/prog_tests/uprobe_syscall.c | 40 +++++++++++++++++++
> > .../bpf/progs/uprobe_syscall_compat.c | 13 ++++++
> > 4 files changed, 59 insertions(+), 1 deletion(-)
> > create mode 100644 tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> >
> > diff --git a/tools/testing/selftests/bpf/.gitignore b/tools/testing/selftests/bpf/.gitignore
> > index f1aebabfb017..69d71223c0dd 100644
> > --- a/tools/testing/selftests/bpf/.gitignore
> > +++ b/tools/testing/selftests/bpf/.gitignore
> > @@ -45,6 +45,7 @@ test_cpp
> > /veristat
> > /sign-file
> > /uprobe_multi
> > +/uprobe_compat
> > *.ko
> > *.tmp
> > xskxceiver
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index edc73f8f5aef..d170b63eca62 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -134,7 +134,7 @@ TEST_GEN_PROGS_EXTENDED = test_sock_addr test_skb_cgroup_id_user \
> > xskxceiver xdp_redirect_multi xdp_synproxy veristat xdp_hw_metadata \
> > xdp_features bpf_test_no_cfi.ko
> >
> > -TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi
> > +TEST_GEN_FILES += liburandom_read.so urandom_read sign-file uprobe_multi uprobe_compat
>
> you need to add uprobe_compat to TRUNNER_EXTRA_FILES as well, no?
ah right
> > diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > index 9233210a4c33..3770254d893b 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/uprobe_syscall.c
> > @@ -11,6 +11,7 @@
> > #include <sys/wait.h>
> > #include "uprobe_syscall.skel.h"
> > #include "uprobe_syscall_call.skel.h"
> > +#include "uprobe_syscall_compat.skel.h"
> >
> > __naked unsigned long uretprobe_regs_trigger(void)
> > {
> > @@ -291,6 +292,35 @@ static void test_uretprobe_syscall_call(void)
> > "read_trace_pipe_iter");
> > ASSERT_EQ(found, 0, "found");
> > }
> > +
> > +static void trace_pipe_compat_cb(const char *str, void *data)
> > +{
> > + if (strstr(str, "uretprobe compat") != NULL)
> > + (*(int *)data)++;
> > +}
> > +
> > +static void test_uretprobe_compat(void)
> > +{
> > + struct uprobe_syscall_compat *skel = NULL;
> > + int err, found = 0;
> > +
> > + skel = uprobe_syscall_compat__open_and_load();
> > + if (!ASSERT_OK_PTR(skel, "uprobe_syscall_compat__open_and_load"))
> > + goto cleanup;
> > +
> > + err = uprobe_syscall_compat__attach(skel);
> > + if (!ASSERT_OK(err, "uprobe_syscall_compat__attach"))
> > + goto cleanup;
> > +
> > + system("./uprobe_compat");
> > +
> > + ASSERT_OK(read_trace_pipe_iter(trace_pipe_compat_cb, &found, 1000),
> > + "read_trace_pipe_iter");
>
> why so complicated? can't you just set global variable that it was called
hm, we execute separate uprobe_compat (32bit) process that triggers the bpf
program, so we can't use global variable.. using the trace_pipe was the only
thing that was easy to do
jirka
>
> > + ASSERT_EQ(found, 1, "found");
> > +
> > +cleanup:
> > + uprobe_syscall_compat__destroy(skel);
> > +}
> > #else
> > static void test_uretprobe_regs_equal(void)
> > {
> > @@ -306,6 +336,11 @@ static void test_uretprobe_syscall_call(void)
> > {
> > test__skip();
> > }
> > +
> > +static void test_uretprobe_compat(void)
> > +{
> > + test__skip();
> > +}
> > #endif
> >
> > void test_uprobe_syscall(void)
> > @@ -320,3 +355,8 @@ void serial_test_uprobe_syscall_call(void)
> > {
> > test_uretprobe_syscall_call();
> > }
> > +
> > +void serial_test_uprobe_syscall_compat(void)
>
> and then no need for serial_test?
>
> > +{
> > + test_uretprobe_compat();
> > +}
> > diff --git a/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c b/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> > new file mode 100644
> > index 000000000000..f8adde7f08e2
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/uprobe_syscall_compat.c
> > @@ -0,0 +1,13 @@
> > +// SPDX-License-Identifier: GPL-2.0
> > +#include <linux/bpf.h>
> > +#include <bpf/bpf_helpers.h>
> > +#include <bpf/bpf_tracing.h>
> > +
> > +char _license[] SEC("license") = "GPL";
> > +
> > +SEC("uretprobe.multi/./uprobe_compat:main")
> > +int uretprobe_compat(struct pt_regs *ctx)
> > +{
> > + bpf_printk("uretprobe compat\n");
> > + return 0;
> > +}
> > --
> > 2.44.0
> >
Powered by blists - more mailing lists