From: Jiri Olsa <olsajiri@gmail.com>
To: Viktor Malik <vmalik@redhat.com>
Cc: bpf@vger.kernel.org, Andrii Nakryiko <andrii@kernel.org>,
Eduard Zingerman <eddyz87@gmail.com>,
Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Martin KaFai Lau <martin.lau@linux.dev>,
Song Liu <song@kernel.org>,
Yonghong Song <yonghong.song@linux.dev>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@kernel.org>,
Stanislav Fomichev <sdf@fomichev.me>, Hao Luo <haoluo@google.com>,
Shuah Khan <shuah@kernel.org>, Jordan Rome <linux@jordanrome.com>,
Qais Yousef <qyousef@layalina.io>, Hou Tao <houtao1@huawei.com>
Subject: Re: [PATCH bpf-next 1/3] selftests/bpf: Split module_attach into subtests
Date: Tue, 10 Feb 2026 16:26:35 +0100 [thread overview]
Message-ID: <aYtOK8YerYM-801A@krava> (raw)
In-Reply-To: <699a95dfb6493cfd588524f388214db1250518f3.1770723724.git.vmalik@redhat.com>
On Tue, Feb 10, 2026 at 01:40:32PM +0100, Viktor Malik wrote:
SNIP
> -void test_module_attach(void)
> +static void test_module_attach_prog(const char *prog_name, int sz,
> + const char *attach_target, int ret)
> {
> - const int READ_SZ = 456;
> - const int WRITE_SZ = 457;
> - struct test_module_attach* skel;
> - struct test_module_attach__bss *bss;
> - struct bpf_link *link;
> + struct test_module_attach *skel;
> + struct bpf_program *prog;
> int err;
> - int writable_val = 0;
>
> skel = test_module_attach__open();
> if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
> return;
>
> - err = bpf_program__set_attach_target(skel->progs.handle_fentry_manual,
> - 0, "bpf_testmod_test_read");
> - ASSERT_OK(err, "set_attach_target");
> + prog = bpf_object__find_program_by_name(skel->obj, prog_name);
> + if (!ASSERT_OK_PTR(prog, "find program"))
> + goto cleanup;
> + bpf_program__set_autoload(prog, true);
>
> - err = bpf_program__set_attach_target(skel->progs.handle_fentry_explicit_manual,
> - 0, "bpf_testmod:bpf_testmod_test_read");
> - ASSERT_OK(err, "set_attach_target_explicit");
> + if (attach_target) {
> + err = bpf_program__set_attach_target(prog, 0, attach_target);
> + ASSERT_OK(err, attach_target);
> + }
> +
> + err = test_module_attach__load(skel);
> + if (CHECK(err, "skel_load", "failed to load skeleton\n"))
> + return;
goto cleanup?
> +
> + err = test_module_attach__attach(skel);
> + if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
should we use ASSERT in new code?
> + goto cleanup;
> +
> + if (sz) {
> + /* trigger both read and write though each test uses only one */
> + ASSERT_OK(trigger_module_test_read(sz), "trigger_read");
> + ASSERT_OK(trigger_module_test_write(sz), "trigger_write");
> +
> + ASSERT_EQ(skel->bss->sz, sz, prog_name);
> + }
> +
> + if (ret)
> + ASSERT_EQ(skel->bss->retval, ret, "ret");
> +cleanup:
> + test_module_attach__destroy(skel);
> +}
> +
> +static void test_module_attach_writable(void)
> +{
> + struct test_module_attach__bss *bss;
> + struct test_module_attach *skel;
> + struct bpf_program *prog;
> + int writable_val = 0;
> + int err;
> +
> + skel = test_module_attach__open();
> + if (CHECK(!skel, "skel_open", "failed to open skeleton\n"))
> + return;
> +
> + prog = bpf_object__find_program_by_name(skel->obj,
> + "handle_raw_tp_writable_bare");
> + if (!ASSERT_OK_PTR(prog, "find program"))
> + goto cleanup;
> + bpf_program__set_autoload(prog, true);
>
> err = test_module_attach__load(skel);
> if (CHECK(err, "skel_load", "failed to load skeleton\n"))
seems like there's missing goto cleanup in original code as well
> @@ -65,21 +121,6 @@ void test_module_attach(void)
> if (CHECK(err, "skel_attach", "skeleton attach failed: %d\n", err))
> goto cleanup;
>
> - /* trigger tracepoint */
> - ASSERT_OK(trigger_module_test_read(READ_SZ), "trigger_read");
> - ASSERT_OK(trigger_module_test_write(WRITE_SZ), "trigger_write");
> -
> - ASSERT_EQ(bss->raw_tp_read_sz, READ_SZ, "raw_tp");
> - ASSERT_EQ(bss->raw_tp_bare_write_sz, WRITE_SZ, "raw_tp_bare");
> - ASSERT_EQ(bss->tp_btf_read_sz, READ_SZ, "tp_btf");
> - ASSERT_EQ(bss->fentry_read_sz, READ_SZ, "fentry");
> - ASSERT_EQ(bss->fentry_manual_read_sz, READ_SZ, "fentry_manual");
> - ASSERT_EQ(bss->fentry_explicit_read_sz, READ_SZ, "fentry_explicit");
> - ASSERT_EQ(bss->fentry_explicit_manual_read_sz, READ_SZ, "fentry_explicit_manual");
> - ASSERT_EQ(bss->fexit_read_sz, READ_SZ, "fexit");
> - ASSERT_EQ(bss->fexit_ret, -EIO, "fexit_tet");
> - ASSERT_EQ(bss->fmod_ret_read_sz, READ_SZ, "fmod_ret");
SNIP
> +void test_module_attach(void)
> +{
> + int i;
> +
> + for (i = 0; i < ARRAY_SIZE(read_tests); i++) {
> + if (!test__start_subtest(read_tests[i]))
> + continue;
> + test_module_attach_prog(read_tests[i], READ_SZ, NULL, 0);
> + }
> + if (test__start_subtest("handle_raw_tp_bare")) {
> + test_module_attach_prog("handle_raw_tp_bare", WRITE_SZ, NULL,
> + 0);
nit, could be on the previous line?
jirka
next prev parent reply other threads:[~2026-02-10 15:26 UTC|newest]
Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-02-10 12:40 [PATCH bpf-next 0/3] Separate tests that need error injection Viktor Malik
2026-02-10 12:40 ` [PATCH bpf-next 1/3] selftests/bpf: Split module_attach into subtests Viktor Malik
2026-02-10 15:26 ` Jiri Olsa [this message]
2026-02-10 20:37 ` Viktor Malik
2026-02-10 12:40 ` [PATCH bpf-next 2/3] selftests/bpf: Split read_vsyscall " Viktor Malik
2026-02-10 12:40 ` [PATCH bpf-next 3/3] selftests/bpf: Split sleepable fentry from LSM test Viktor Malik
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aYtOK8YerYM-801A@krava \
--to=olsajiri@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=eddyz87@gmail.com \
--cc=haoluo@google.com \
--cc=houtao1@huawei.com \
--cc=john.fastabend@gmail.com \
--cc=kpsingh@kernel.org \
--cc=linux@jordanrome.com \
--cc=martin.lau@linux.dev \
--cc=qyousef@layalina.io \
--cc=sdf@fomichev.me \
--cc=shuah@kernel.org \
--cc=song@kernel.org \
--cc=vmalik@redhat.com \
--cc=yonghong.song@linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox