[bpf-next,v2,2/5] selftests/bpf: Add test for __nullable suffix in tp_btf

Commit Message

Philo Lu Sept. 5, 2024, 7:56 a.m. UTC

Add a tracepoint with __nullable suffix in bpf_testmod, and add a
failure load case:

$./test_progs -t "module_attach"
 #173/1   module_attach/handle_tp_btf_nullable_bare:OK
 #173     module_attach:OK
 Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED

Signed-off-by: Philo Lu <[e-mail]>
---
 .../bpf/bpf_testmod/bpf_testmod-events.h         |  6 ++++++
 .../selftests/bpf/bpf_testmod/bpf_testmod.c      |  2 ++
 .../selftests/bpf/prog_tests/module_attach.c     | 14 +++++++++++++-
 .../bpf/progs/test_module_attach_fail.c          | 16 ++++++++++++++++
 4 files changed, 37 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/bpf/progs/test_module_attach_fail.c

Comments

Andrii Nakryiko Sept. 6, 2024, 9:25 p.m. UTC | #1

On Thu, Sep 5, 2024 at 12:56 AM Philo Lu <[e-mail]> wrote:
>
> Add a tracepoint with __nullable suffix in bpf_testmod, and add a
> failure load case:
>
> $./test_progs -t "module_attach"
>  #173/1   module_attach/handle_tp_btf_nullable_bare:OK
>  #173     module_attach:OK
>  Summary: 1/1 PASSED, 0 SKIPPED, 0 FAILED
>
> Signed-off-by: Philo Lu <[e-mail]>
> ---
>  .../bpf/bpf_testmod/bpf_testmod-events.h         |  6 ++++++
>  .../selftests/bpf/bpf_testmod/bpf_testmod.c      |  2 ++
>  .../selftests/bpf/prog_tests/module_attach.c     | 14 +++++++++++++-
>  .../bpf/progs/test_module_attach_fail.c          | 16 ++++++++++++++++
>  4 files changed, 37 insertions(+), 1 deletion(-)
>  create mode 100644 tools/testing/selftests/bpf/progs/test_module_attach_fail.c
>

[...]

> +
> +static void module_attach_fail(void)
> +{
> +       RUN_TESTS(test_module_attach_fail);
> +}
> +
> +void test_module_attach(void)
> +{
> +       module_attach_succ();
> +       module_attach_fail();
> +}

this is not a good idea to combine existing non-RUN_TESTS test with
RUN_TESTS. The latter is a subtest, while the former is a full test.
Keep them separate, just add a new file for
RUN_TESTS(test_module_attach_fail), or add them to existing
RUN_TESTS(), whatever makes most sense.

> diff --git a/tools/testing/selftests/bpf/progs/test_module_attach_fail.c b/tools/testing/selftests/bpf/progs/test_module_attach_fail.c
> new file mode 100644
> index 0000000000000..0f848d8f2f5e8
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/test_module_attach_fail.c
> @@ -0,0 +1,16 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +#include "vmlinux.h"
> +#include <bpf/bpf_helpers.h>
> +#include <bpf/bpf_tracing.h>
> +#include "../bpf_testmod/bpf_testmod.h"
> +#include "bpf_misc.h"
> +
> +SEC("tp_btf/bpf_testmod_test_nullable_bare")
> +__failure __msg("invalid mem access")

would be nice to confirm that register corresponding to nullable_ctx
actually has "_or_null" suffix


> +int BPF_PROG(handle_tp_btf_nullable_bare, struct bpf_testmod_test_read_ctx *nullable_ctx)
> +{
> +       return nullable_ctx->len;
> +}
> +
> +char _license[] SEC("license") = "GPL";
> --
> 2.32.0.3.g01195cf9f
>
>

Patch

diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
index 11ee801e75e7e..6c3b4d4f173ac 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod-events.h
@@ -34,6 +34,12 @@  DECLARE_TRACE(bpf_testmod_test_write_bare,
 	TP_ARGS(task, ctx)
 );
 
+/* Used in bpf_testmod_test_read() to test __nullable suffix */
+DECLARE_TRACE(bpf_testmod_test_nullable_bare,
+	TP_PROTO(struct bpf_testmod_test_read_ctx *ctx__nullable),
+	TP_ARGS(ctx__nullable)
+);
+
 #undef BPF_TESTMOD_DECLARE_TRACE
 #ifdef DECLARE_TRACE_WRITABLE
 #define BPF_TESTMOD_DECLARE_TRACE(call, proto, args, size) \
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index c73d04bc9e9de..9649e7f09fc90 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -394,6 +394,8 @@  bpf_testmod_test_read(struct file *file, struct kobject *kobj,
 	if (bpf_testmod_loop_test(101) > 100)
 		trace_bpf_testmod_test_read(current, &ctx);
 
+	trace_bpf_testmod_test_nullable_bare(NULL);
+
 	/* Magic number to enable writable tp */
 	if (len == 64) {
 		struct bpf_testmod_test_writable_ctx writable = {
diff --git a/tools/testing/selftests/bpf/prog_tests/module_attach.c b/tools/testing/selftests/bpf/prog_tests/module_attach.c
index 6d391d95f96e0..961d8577d6fab 100644
--- a/tools/testing/selftests/bpf/prog_tests/module_attach.c
+++ b/tools/testing/selftests/bpf/prog_tests/module_attach.c
@@ -4,6 +4,7 @@ 
 #include <test_progs.h>
 #include <stdbool.h>
 #include "test_module_attach.skel.h"
+#include "test_module_attach_fail.skel.h"
 #include "testing_helpers.h"
 
 static int duration;
@@ -33,7 +34,7 @@  static int trigger_module_test_writable(int *val)
 	return 0;
 }
 
-void test_module_attach(void)
+static void module_attach_succ(void)
 {
 	const int READ_SZ = 456;
 	const int WRITE_SZ = 457;
@@ -115,3 +116,14 @@  void test_module_attach(void)
 cleanup:
 	test_module_attach__destroy(skel);
 }
+
+static void module_attach_fail(void)
+{
+	RUN_TESTS(test_module_attach_fail);
+}
+
+void test_module_attach(void)
+{
+	module_attach_succ();
+	module_attach_fail();
+}
diff --git a/tools/testing/selftests/bpf/progs/test_module_attach_fail.c b/tools/testing/selftests/bpf/progs/test_module_attach_fail.c
new file mode 100644
index 0000000000000..0f848d8f2f5e8
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/test_module_attach_fail.c
@@ -0,0 +1,16 @@ 
+// SPDX-License-Identifier: GPL-2.0
+
+#include "vmlinux.h"
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "../bpf_testmod/bpf_testmod.h"
+#include "bpf_misc.h"
+
+SEC("tp_btf/bpf_testmod_test_nullable_bare")
+__failure __msg("invalid mem access")
+int BPF_PROG(handle_tp_btf_nullable_bare, struct bpf_testmod_test_read_ctx *nullable_ctx)
+{
+	return nullable_ctx->len;
+}
+
+char _license[] SEC("license") = "GPL";