diff mbox series

[bpf-next,5/7] selftests/bpf: add tp_btf CO-RE reloc test for modules

Message ID 20201121024616.1588175-6-andrii@kernel.org (mailing list archive)
State New, archived
Delegated to: BPF
Headers show
Series Add kernel modules support for tracing BPF program attachments | expand

Checks

Context Check Description
netdev/apply fail Patch does not apply to bpf-next
netdev/tree_selection success Clearly marked for bpf-next

Commit Message

Andrii Nakryiko Nov. 21, 2020, 2:46 a.m. UTC
Add another CO-RE relocation test for kernel module relocations. This time for
tp_btf with direct memory reads.

Signed-off-by: Andrii Nakryiko <andrii@kernel.org>
---
 .../selftests/bpf/prog_tests/core_reloc.c     |  3 +-
 .../bpf/progs/test_core_reloc_module.c        | 32 ++++++++++++++++++-
 2 files changed, 33 insertions(+), 2 deletions(-)

Comments

Alexei Starovoitov Nov. 29, 2020, 1:59 a.m. UTC | #1
On Fri, Nov 20, 2020 at 06:46:14PM -0800, Andrii Nakryiko wrote:
>  
>  SEC("raw_tp/bpf_sidecar_test_read")
> -int BPF_PROG(test_core_module,
> +int BPF_PROG(test_core_module_probed,
>  	     struct task_struct *task,
>  	     struct bpf_sidecar_test_read_ctx *read_ctx)
>  {
> @@ -64,3 +64,33 @@ int BPF_PROG(test_core_module,
>  
>  	return 0;
>  }
> +
> +SEC("tp_btf/bpf_sidecar_test_read")
> +int BPF_PROG(test_core_module_direct,
> +	     struct task_struct *task,
> +	     struct bpf_sidecar_test_read_ctx *read_ctx)

"sidecar" is such an overused name.
I didn't like it earlier, but seeing that it here again and again I couldn't help it.
Could you please pick a different name for kernel module?
It's just a kernel module for testing. Just call it so. No need for fancy name.
Andrii Nakryiko Nov. 30, 2020, 10:52 p.m. UTC | #2
On Sat, Nov 28, 2020 at 5:59 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Fri, Nov 20, 2020 at 06:46:14PM -0800, Andrii Nakryiko wrote:
> >
> >  SEC("raw_tp/bpf_sidecar_test_read")
> > -int BPF_PROG(test_core_module,
> > +int BPF_PROG(test_core_module_probed,
> >            struct task_struct *task,
> >            struct bpf_sidecar_test_read_ctx *read_ctx)
> >  {
> > @@ -64,3 +64,33 @@ int BPF_PROG(test_core_module,
> >
> >       return 0;
> >  }
> > +
> > +SEC("tp_btf/bpf_sidecar_test_read")
> > +int BPF_PROG(test_core_module_direct,
> > +          struct task_struct *task,
> > +          struct bpf_sidecar_test_read_ctx *read_ctx)
>
> "sidecar" is such an overused name.

How about "sidekick"? :) Its definition matches quite closely for what
we are doing with it ("person's assistant or close associate,
especially one who has less authority than that person.")?

But if you still hate it, I can call it just "bpf_selftest" or
"bpf_test" or "bpf_testmod", however boring that is... ;)


> I didn't like it earlier, but seeing that it here again and again I couldn't help it.
> Could you please pick a different name for kernel module?
> It's just a kernel module for testing. Just call it so. No need for fancy name.
Alexei Starovoitov Nov. 30, 2020, 10:55 p.m. UTC | #3
On Mon, Nov 30, 2020 at 2:52 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Sat, Nov 28, 2020 at 5:59 PM Alexei Starovoitov
> <alexei.starovoitov@gmail.com> wrote:
> >
> > On Fri, Nov 20, 2020 at 06:46:14PM -0800, Andrii Nakryiko wrote:
> > >
> > >  SEC("raw_tp/bpf_sidecar_test_read")
> > > -int BPF_PROG(test_core_module,
> > > +int BPF_PROG(test_core_module_probed,
> > >            struct task_struct *task,
> > >            struct bpf_sidecar_test_read_ctx *read_ctx)
> > >  {
> > > @@ -64,3 +64,33 @@ int BPF_PROG(test_core_module,
> > >
> > >       return 0;
> > >  }
> > > +
> > > +SEC("tp_btf/bpf_sidecar_test_read")
> > > +int BPF_PROG(test_core_module_direct,
> > > +          struct task_struct *task,
> > > +          struct bpf_sidecar_test_read_ctx *read_ctx)
> >
> > "sidecar" is such an overused name.
>
> How about "sidekick"? :) Its definition matches quite closely for what
> we are doing with it ("person's assistant or close associate,
> especially one who has less authority than that person.")?
>
> But if you still hate it, I can call it just "bpf_selftest" or
> "bpf_test" or "bpf_testmod", however boring that is... ;)

bpf_testmod sounds the best to me :)
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/core_reloc.c b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
index b3f14e3d9077..85c9cf8e3994 100644
--- a/tools/testing/selftests/bpf/prog_tests/core_reloc.c
+++ b/tools/testing/selftests/bpf/prog_tests/core_reloc.c
@@ -512,7 +512,8 @@  static struct core_reloc_test_case test_cases[] = {
 	},
 
 	/* validate we can find kernel module BTF types for relocs/attach */
-	MODULES_CASE("module", "raw_tp/bpf_sidecar_test_read", "bpf_sidecar_test_read"),
+	MODULES_CASE("module_probed", "raw_tp/bpf_sidecar_test_read", "bpf_sidecar_test_read"),
+	MODULES_CASE("module_direct", "tp_btf/bpf_sidecar_test_read", NULL),
 
 	/* validate BPF program can use multiple flavors to match against
 	 * single target BTF type
diff --git a/tools/testing/selftests/bpf/progs/test_core_reloc_module.c b/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
index 4630301de259..f336c0565d47 100644
--- a/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
+++ b/tools/testing/selftests/bpf/progs/test_core_reloc_module.c
@@ -36,7 +36,7 @@  struct core_reloc_module_output {
 };
 
 SEC("raw_tp/bpf_sidecar_test_read")
-int BPF_PROG(test_core_module,
+int BPF_PROG(test_core_module_probed,
 	     struct task_struct *task,
 	     struct bpf_sidecar_test_read_ctx *read_ctx)
 {
@@ -64,3 +64,33 @@  int BPF_PROG(test_core_module,
 
 	return 0;
 }
+
+SEC("tp_btf/bpf_sidecar_test_read")
+int BPF_PROG(test_core_module_direct,
+	     struct task_struct *task,
+	     struct bpf_sidecar_test_read_ctx *read_ctx)
+{
+	struct core_reloc_module_output *out = (void *)&data.out;
+	__u64 pid_tgid = bpf_get_current_pid_tgid();
+	__u32 real_tgid = (__u32)(pid_tgid >> 32);
+	__u32 real_pid = (__u32)pid_tgid;
+
+	if (data.my_pid_tgid != pid_tgid)
+		return 0;
+
+	if (task->pid != real_pid || task->tgid != real_tgid)
+		return 0;
+
+	out->len = read_ctx->len;
+	out->off = read_ctx->off;
+
+	out->read_ctx_sz = bpf_core_type_size(struct bpf_sidecar_test_read_ctx);
+	out->read_ctx_exists = bpf_core_type_exists(struct bpf_sidecar_test_read_ctx);
+	out->buf_exists = bpf_core_field_exists(read_ctx->buf);
+	out->off_exists = bpf_core_field_exists(read_ctx->off);
+	out->len_exists = bpf_core_field_exists(read_ctx->len);
+
+	out->comm_len = BPF_CORE_READ_STR_INTO(&out->comm, task, comm);
+
+	return 0;
+}