From: sashiko-bot@kernel.org
To: "Qiliang Yuan" <odys.yuan@gmail.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v2] selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN
Date: Fri, 09 Oct 2026 02:13:40 +0000 [thread overview]
Message-ID: <sashiko-outbox-164721@kernel.org> (raw)
In-Reply-To: <20261009-selftests-bpf-enable-stats-inflight-v2-1-691ee379e511@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Low] bpf: selftests: use of deprecated CHECK macro instead of ASSERT_*
--
commit 2195ab7af2e709db01f39a1419b8eb0dd3b3c741
Author: Qiliang Yuan <odys.yuan@gmail.com>
selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN
This commit changes the test_enable_stats test to run its program with
BPF_PROG_TEST_RUN rather than attaching it to sys_enter. This ensures that
counts are final and avoids race conditions with parallel tests.
Link: https://lore.kernel.org/r/20261008-selftests-bpf-enable-stats-inflight-v1-1-0cb92ee2c6d3@gmail.com
> diff --git a/tools/testing/selftests/bpf/prog_tests/enable_stats.c b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> index 75f85d0fe74a4..691a82a4d6d20 100644
> --- a/tools/testing/selftests/bpf/prog_tests/enable_stats.c
> +++ b/tools/testing/selftests/bpf/prog_tests/enable_stats.c
[ ... ]
> @@ -20,18 +21,23 @@ void test_enable_stats(void)
[ ... ]
> + do {
> + err = bpf_prog_test_run_opts(prog_fd, &topts);
> + if (err)
> + break;
> +
> + memset(&info, 0, info_len);
> + err = bpf_prog_get_info_by_fd(prog_fd, &info, &info_len);
> + } while (!err && info.run_cnt && !info.run_time_ns);
> + if (CHECK(err, "run_and_get_prog_info", "err %d\n", err))
[Severity: Low]
Does this newly added error path use the deprecated CHECK() macro? The BPF
subsystem selftest guidelines prefer using modern assertions like ASSERT_OK()
for new code instead of CHECK().
> goto cleanup;
[Severity: Low]
This problem wasn't introduced by this patch, but while updating the
test_enable_stats() logic, would it be worth migrating the existing
CHECK(info.run_cnt != skel->bss->count, ...) to a modern assertion like
ASSERT_NEQ() to help reduce technical debt?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261009-selftests-bpf-enable-stats-inflight-v2-1-691ee379e511@gmail.com?part=1
prev parent reply other threads:[~2026-10-09 2:13 UTC|newest]
Thread overview: 2+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-10-09 2:04 [PATCH bpf-next v2] selftests/bpf: Run the enable_stats program with BPF_PROG_TEST_RUN Qiliang Yuan
2026-10-09 2:13 ` sashiko-bot [this message]
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=sashiko-outbox-164721@kernel.org \
--to=sashiko-bot@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=odys.yuan@gmail.com \
--cc=sashiko-reviews@lists.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