diff mbox series

[v2,bpf-next,07/14] selftests/bpf: add selftest for multi-prog sections and bpf-to-bpf calls

Message ID 20200901015003.2871861-8-andriin@fb.com
State Changes Requested
Delegated to: BPF Maintainers
Headers show
Series Add libbpf full support for BPF-to-BPF calls | expand

Commit Message

Andrii Nakryiko Sept. 1, 2020, 1:49 a.m. UTC 
Add a selftest excercising bpf-to-bpf subprogram calls, as well as multiple
entry-point BPF programs per section. Also make sure that BPF CO-RE works for
such set ups both for sub-programs and for multi-entry sections.

Signed-off-by: Andrii Nakryiko <andriin@fb.com>
---
 .../selftests/bpf/prog_tests/subprogs.c       |  31 ++++++
 .../selftests/bpf/progs/test_subprogs.c       | 103 ++++++++++++++++++
 2 files changed, 134 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/subprogs.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_subprogs.c

Comments

Alexei Starovoitov Sept. 2, 2020, 5:41 a.m. UTC | #1 
On Mon, Aug 31, 2020 at 06:49:56PM -0700, Andrii Nakryiko wrote:
> +
> +__noinline int sub1(int x)
> +{
> +	return x + 1;
> +}
> +
> +static __noinline int sub5(int v);
> +
> +__noinline int sub2(int y)
> +{
> +	return sub5(y + 2);
> +}
> +
> +static __noinline int sub3(int z)
> +{
> +	return z + 3 + sub1(4);
> +}
> +
> +static __noinline int sub4(int w)
> +{
> +	return w + sub3(5) + sub1(6);

Did you check that asm has these calls?
Since sub3 is static the compiler doesn't have to do the call.
'static noinline' doesn't mean that compiler have to do the call.
It can compute the value and replace a call with a constant.
It only has to keep the body of the function if the address of it
was taken.
Andrii Nakryiko Sept. 2, 2020, 7:57 p.m. UTC | #2 
On Tue, Sep 1, 2020 at 10:41 PM Alexei Starovoitov
<alexei.starovoitov@gmail.com> wrote:
>
> On Mon, Aug 31, 2020 at 06:49:56PM -0700, Andrii Nakryiko wrote:
> > +
> > +__noinline int sub1(int x)
> > +{
> > +     return x + 1;
> > +}
> > +
> > +static __noinline int sub5(int v);
> > +
> > +__noinline int sub2(int y)
> > +{
> > +     return sub5(y + 2);
> > +}
> > +
> > +static __noinline int sub3(int z)
> > +{
> > +     return z + 3 + sub1(4);
> > +}
> > +
> > +static __noinline int sub4(int w)
> > +{
> > +     return w + sub3(5) + sub1(6);
>
> Did you check that asm has these calls?

Yeah, I actually did check. All calls are there.

> Since sub3 is static the compiler doesn't have to do the call.
> 'static noinline' doesn't mean that compiler have to do the call.
> It can compute the value and replace a call with a constant.
> It only has to keep the body of the function if the address of it
> was taken.

All these subX() functions are either global or call global function
(sub1() is global), which seems to keep Clang from optimizing all
this. Clang has to assume the worst case for global functions,
probably due to LD_PRELOAD, right?
diff mbox series

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/subprogs.c b/tools/testing/selftests/bpf/prog_tests/subprogs.c
new file mode 100644
index 000000000000..a00abf58c037
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/subprogs.c
@@ -0,0 +1,31 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2020 Facebook */
+#include <test_progs.h>
+#include <time.h>
+#include "test_subprogs.skel.h"
+
+static int duration;
+
+void test_subprogs(void)
+{
+	struct test_subprogs *skel;
+	int err;
+
+	skel = test_subprogs__open_and_load();
+	if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
+		return;
+
+	err = test_subprogs__attach(skel);
+	if (CHECK(err, "skel_attach", "failed to attach skeleton: %d\n", err))
+		goto cleanup;
+
+	usleep(1);
+
+	CHECK(skel->bss->res1 != 12, "res1", "got %d, exp %d\n", skel->bss->res1, 12);
+	CHECK(skel->bss->res2 != 17, "res2", "got %d, exp %d\n", skel->bss->res2, 17);
+	CHECK(skel->bss->res3 != 19, "res3", "got %d, exp %d\n", skel->bss->res3, 19);
+	CHECK(skel->bss->res4 != 36, "res4", "got %d, exp %d\n", skel->bss->res4, 36);
+
+cleanup:
+	test_subprogs__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_subprogs.c b/tools/testing/selftests/bpf/progs/test_subprogs.c
new file mode 100644
index 000000000000..d3c5673c0218
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_subprogs.c
@@ -0,0 +1,103 @@ 
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_core_read.h>
+
+const char LICENSE[] SEC("license") = "GPL";
+
+__noinline int sub1(int x)
+{
+	return x + 1;
+}
+
+static __noinline int sub5(int v);
+
+__noinline int sub2(int y)
+{
+	return sub5(y + 2);
+}
+
+static __noinline int sub3(int z)
+{
+	return z + 3 + sub1(4);
+}
+
+static __noinline int sub4(int w)
+{
+	return w + sub3(5) + sub1(6);
+}
+
+/* sub5() is an identitify function, just to test weirder functions layout and
+ * call patterns
+ */
+static __noinline int sub5(int v)
+{
+	return sub1(v) - 1; /* compensates sub1()'s + 1 */
+}
+
+/* unfortunately verifier rejects `struct task_struct *t` as an unkown pointer
+ * type, so we need to accept pointer as integer and then cast it inside the
+ * function
+ */
+__noinline int get_task_tgid(uintptr_t t)
+{
+	/* this ensures that CO-RE relocs work in multi-subprogs .text */
+	return BPF_CORE_READ((struct task_struct *)(void *)t, tgid);
+}
+
+int res1 = 0;
+int res2 = 0;
+int res3 = 0;
+int res4 = 0;
+
+SEC("raw_tp/sys_enter")
+int prog1(void *ctx)
+{
+	/* perform some CO-RE relocations to ensure they work with multi-prog
+	 * sections correctly
+	 */
+	struct task_struct *t = (void *)bpf_get_current_task();
+
+	if (!BPF_CORE_READ(t, pid) || !get_task_tgid((uintptr_t)t))
+		return 1;
+
+	res1 = sub1(1) + sub3(2); /* (1 + 1) + (2 + 3 + (4 + 1)) = 12 */
+	return 0;
+}
+
+SEC("raw_tp/sys_exit")
+int prog2(void *ctx)
+{
+	struct task_struct *t = (void *)bpf_get_current_task();
+
+	if (!BPF_CORE_READ(t, pid) || !get_task_tgid((uintptr_t)t))
+		return 1;
+
+	res2 = sub2(3) + sub3(4); /* (3 + 2) + (4 + 3 + (4 + 1)) = 17 */
+	return 0;
+}
+
+/* prog3 has the same section name as prog1 */
+SEC("raw_tp/sys_enter")
+int prog3(void *ctx)
+{
+	struct task_struct *t = (void *)bpf_get_current_task();
+
+	if (!BPF_CORE_READ(t, pid) || !get_task_tgid((uintptr_t)t))
+		return 1;
+
+	res3 = sub3(5) + 6; /* (5 + 3 + (4 + 1)) + 6 = 19 */
+	return 0;
+}
+
+/* prog4 has the same section name as prog2 */
+SEC("raw_tp/sys_exit")
+int prog4(void *ctx)
+{
+	struct task_struct *t = (void *)bpf_get_current_task();
+
+	if (!BPF_CORE_READ(t, pid) || !get_task_tgid((uintptr_t)t))
+		return 1;
+
+	res4 = sub4(7) + sub1(8); /* (7 + (5 + 3 + (4 + 1)) + (6 + 1)) + (8 + 1) = 36 */
+	return 0;
+}