On Thu, Sep 5, 2024 at 12:56 AM Philo Lu <lulie@xxxxxxxxxxxxxxxxx> 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 <lulie@xxxxxxxxxxxxxxxxx>
> ---
> .../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
>
>
