[bpf-next,1/2] bpf/selftests: coverage for calling kfuncs within tracepoint

Commit Message

JP Kobryn Aug. 14, 2024, 11:57 p.m. UTC

This test exposes the issue of being unable to call kfuncs within a
normal tracepoint program. The program will be rejected by the verifier
as not allowed.

Signed-off-by: JP Kobryn <[e-mail]>
---
 .../selftests/bpf/prog_tests/kfunc_in_tp.c    | 34 ++++++++++++++++++
 .../selftests/bpf/progs/test_kfunc_in_tp.c    | 35 +++++++++++++++++++
 2 files changed, 69 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/kfunc_in_tp.c
 create mode 100644 tools/testing/selftests/bpf/progs/test_kfunc_in_tp.c

Comments

Eduard Zingerman Aug. 15, 2024, 12:10 a.m. UTC | #1

On Wed, 2024-08-14 at 16:57 -0700, JP Kobryn wrote:
> This test exposes the issue of being unable to call kfuncs within a
> normal tracepoint program. The program will be rejected by the verifier
> as not allowed.
> 
> Signed-off-by: JP Kobryn <[e-mail]>
> ---

I have no opinion regarding the gist of this patch-set,
just a few technical nits.

Please swap places for patches 1 and 2, such that at any point in
commit history selftests are passing. This should help with any
potential bisect.

>  .../selftests/bpf/prog_tests/kfunc_in_tp.c    | 34 ++++++++++++++++++
>  .../selftests/bpf/progs/test_kfunc_in_tp.c    | 35 +++++++++++++++++++
>  2 files changed, 69 insertions(+)
>  create mode 100644 tools/testing/selftests/bpf/prog_tests/kfunc_in_tp.c
>  create mode 100644 tools/testing/selftests/bpf/progs/test_kfunc_in_tp.c
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_in_tp.c b/tools/testing/selftests/bpf/prog_tests/kfunc_in_tp.c
> new file mode 100644
> index 000000000000..bef1d192fc00
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/prog_tests/kfunc_in_tp.c
> @@ -0,0 +1,34 @@
> +// SPDX-License-Identifier: GPL-2.0
> +/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
> +
> +#include <errno.h>
> +#include <sys/syscall.h>
> +#include <unistd.h>
> +
> +#include "test_kfunc_in_tp.skel.h"
> +#include "test_progs.h"
> +
> +static void run_tp(void)
> +{
> +	(void)syscall(__NR_getpid);
> +}
> +
> +void test_kfunc_in_tp(void)

Could you please consider use of test_loader framework for these tests?
This would make 'kfunc_in_tp.c' unnecessary.
Example of tests using this framework:

    tools/testing/selftests/bpf/progs/cpumask_failure.c

(but please, integrate with tools/testing/selftests/bpf/prog_tests/verifier.c
 to avoid creation of unnecessary files).

> +{
> +	struct test_kfunc_in_tp *skel;
> +	int err;
> +
> +	skel = test_kfunc_in_tp__open();
> +	ASSERT_OK_PTR(skel, "test_kfunc_in_tp__open");
> +
> +	err = test_kfunc_in_tp__load(skel);
> +	ASSERT_OK(err, "test_kfunc_in_tp__load");
> +
> +	err = test_kfunc_in_tp__attach(skel);
> +	ASSERT_OK(err, "test_kfunc_in_tp__attach");
> +
> +	run_tp();
> +	ASSERT_OK(skel->data->result, "complete");
> +
> +	test_kfunc_in_tp__destroy(skel);
> +}

[...]

Patch

diff --git a/tools/testing/selftests/bpf/prog_tests/kfunc_in_tp.c b/tools/testing/selftests/bpf/prog_tests/kfunc_in_tp.c
new file mode 100644
index 000000000000..bef1d192fc00
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/kfunc_in_tp.c
@@ -0,0 +1,34 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+
+#include <errno.h>
+#include <sys/syscall.h>
+#include <unistd.h>
+
+#include "test_kfunc_in_tp.skel.h"
+#include "test_progs.h"
+
+static void run_tp(void)
+{
+	(void)syscall(__NR_getpid);
+}
+
+void test_kfunc_in_tp(void)
+{
+	struct test_kfunc_in_tp *skel;
+	int err;
+
+	skel = test_kfunc_in_tp__open();
+	ASSERT_OK_PTR(skel, "test_kfunc_in_tp__open");
+
+	err = test_kfunc_in_tp__load(skel);
+	ASSERT_OK(err, "test_kfunc_in_tp__load");
+
+	err = test_kfunc_in_tp__attach(skel);
+	ASSERT_OK(err, "test_kfunc_in_tp__attach");
+
+	run_tp();
+	ASSERT_OK(skel->data->result, "complete");
+
+	test_kfunc_in_tp__destroy(skel);
+}
diff --git a/tools/testing/selftests/bpf/progs/test_kfunc_in_tp.c b/tools/testing/selftests/bpf/progs/test_kfunc_in_tp.c
new file mode 100644
index 000000000000..5a3f1ff930af
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_kfunc_in_tp.c
@@ -0,0 +1,35 @@ 
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2024 Meta Platforms, Inc. and affiliates. */
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+
+char _license[] SEC("license") = "GPL";
+
+extern struct bpf_cpumask *bpf_cpumask_create(void) __ksym;
+extern void bpf_cpumask_set_cpu(u32 cpu, struct bpf_cpumask *cpumask) __ksym;
+extern bool bpf_cpumask_test_cpu(u32 cpu, const struct cpumask *cpumask) __ksym;
+extern void bpf_cpumask_release(struct bpf_cpumask *cpumask) __ksym;
+
+int result = -1;
+
+/* call arbitrary kfuncs within a tracepoint program */
+SEC("tp/syscalls/sys_enter_getpid")
+int handle_tp(struct trace_event_raw_ipi_send_cpumask *ctx)
+{
+	struct bpf_cpumask *cpumask;
+	const u32 cpu = bpf_get_smp_processor_id();
+
+	cpumask = bpf_cpumask_create();
+	if (!cpumask)
+		return 0;
+
+	bpf_cpumask_set_cpu(cpu, cpumask);
+	if (bpf_cpumask_test_cpu(cpu, (struct cpumask *)cpumask))
+		bpf_printk("match\n");
+
+	bpf_cpumask_release(cpumask);
+	result = 0;
+
+	return 0;
+}