BPF List
 help / color / mirror / Atom feed
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

      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