[PATCH bpf-next v2 0/7] selftests/bpf: Fix selftests failures with LLVM 23

[PATCH bpf-next v2 0/7] selftests/bpf: Fix selftests failures with LLVM 23

[KaFai Wan]

Building with clang/LLVM 23 breaks several selftests. Patches #1 to #4 fix
those failures.

The following self-tests also fail under specific invocations of
test_progs:

- ./test_progs -t prog_tests_framework/test_fail -v
 test_prog_tests_framework:FAIL:skip_cnt_check
 unexpected skip_cnt_check: actual 0 != expected 2

- ./test_progs -t prog_tests_framework_expected_msgs -v
 Caught signal #11!

- ./test_progs -t prog_tests_framework_compare_text -v
 Caught signal #11!

Patches #5 to #7 fix these.

Changes in v2:
 - Collect the selftest fixes from the two patches in v1
   patches: #1, #5, #6 and #7.
 - Add three new patches: #2, #3 and #4.

Changes in v1:
 - https://lore.kernel.org/bpf/20261012135624.2749743-1-kafai.wan@linux.dev/T/
 - https://lore.kernel.org/bpf/20261012140202.2754552-1-kafai.wan@linux.dev/T/

---
KaFai Wan (7):
  bpf: Determine freplace staticness from BTF linkage
  selftests/bpf: Fix stack_arg precision backtracking log expectation
  selftests/bpf: Fix stack arg pruning test JOIN log expectation
  selftests/bpf: Use _exit() in uprobe_multi_test clone-ed child
  selftests/bpf: Fix prog_tests framework subtest count checks
  selftests/bpf: Skip expected_msgs subtests when stdout is not saved
  selftests/bpf: Skip compare_text subtest when stdout is not saved

 kernel/bpf/verifier.c                         |  3 +-
 .../selftests/bpf/prog_tests/exceptions.c     |  2 +-
 .../bpf/prog_tests/prog_tests_framework.c     | 34 ++++++++++++++-----
 .../bpf/prog_tests/uprobe_multi_test.c        |  4 +--
 .../selftests/bpf/progs/stack_arg_precision.c |  4 +--
 .../selftests/bpf/progs/verifier_stack_arg.c  |  4 +--
 6 files changed, 34 insertions(+), 17 deletions(-)

-- 
2.43.0

[PATCH bpf-next v2 1/7] bpf: Determine freplace staticness from BTF linkage

[KaFai Wan]

bpf_check_attach_target() rejects a freplace whose target is a static
function, using the subprog "unreliable" flag as a proxy for "static".
That flag is raised whenever a subprogram's BTF prototype disagrees with
the registers at its call site.

`147400e9c27c ("bpf: Compute parameters liveness in
bpf_compute_live_registers()")` made that work for static callees: a
static function with an unused argument no longer matches its BTF
prototype at the call site and is marked unreliable.

Since LLVM commit `29b36a16f111 ("[BPF] Emit ABI-accurate BTF
prototypes for DW_CC_nocall")` in 23.1 [1] that premise is gone:
BTFDebug may now drop arguments removed by the optimizer, so for

	__noinline static int subprog(struct __sk_buff *ctx)
	{
		return bpf_ktime_get_ns();
	}

compiled with clang 23 the BTF describes subprog() as

	[75] FUNC_PROTO '(anon)' ret_type_id=2 vlen=0
	[76] FUNC 'subprog' type_id=75 linkage=static

There is then no argument left to compare, the subprog stays reliable,
and the check in bpf_check_attach_target() is skipped. The freplace is
then rejected by the later linkage check instead, reporting "subprog()
is not a global function", and the selftest "exceptions" fails.

Determine staticness from the BTF linkage instead, and keep the
unreliable check for global functions whose BTF does not match their
call site.

There is no functional change: freplacing a static function was rejected
before and is still rejected with the same -EINVAL. Selftest "exceptions"
is updated to match.

[1] https://github.com/llvm/llvm-project/pull/198426
Fixes: 147400e9c27c ("bpf: Compute parameters liveness in bpf_compute_live_registers()")
Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
---
 kernel/bpf/verifier.c                               | 3 ++-
 tools/testing/selftests/bpf/prog_tests/exceptions.c | 2 +-
 2 files changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 2f190abe34489..e9814e3ac370e 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -22079,7 +22079,8 @@ int bpf_check_attach_target(struct bpf_verifier_log *log,
 		}
 		conservative = aux->func_info_aux[subprog].unreliable;
 		if (prog_extension) {
-			if (conservative) {
+			if (aux->func_info_aux[subprog].linkage != BTF_FUNC_GLOBAL ||
+			    conservative) {
 				bpf_log(log,
 					"Cannot replace static functions\n");
 				return -EINVAL;
diff --git a/tools/testing/selftests/bpf/prog_tests/exceptions.c b/tools/testing/selftests/bpf/prog_tests/exceptions.c
index cea01d638f6fc..1bd15c4c125ba 100644
--- a/tools/testing/selftests/bpf/prog_tests/exceptions.c
+++ b/tools/testing/selftests/bpf/prog_tests/exceptions.c
@@ -333,7 +333,7 @@ static void test_exceptions_success(void)
 				       bpf_program__fd(skel->progs.exception_throw_subprog),
 				       "throwing_subprog"), "set_attach_target"))
 				goto done;
-		}), "throwing_subprog() is not a global function", 0);
+		}), "Cannot replace static functions", 0);
 
 	if (test__start_subtest("non-throwing extension -> non-throwing subprog"))
 		/* non-throwing extension -> non-throwing global subprog : OK */
-- 
2.43.0

[PATCH bpf-next v2 2/7] selftests/bpf: Fix stack_arg precision backtracking log expectation

[KaFai Wan]

Commit 147400e9c27c ("bpf: Compute parameters liveness in
bpf_compute_live_registers()") stopped assuming that a call to a static
subprogram reads r1..r5 and now uses the callee's entry live-in. R1-R5 are
not live at these call insns, so __clean_func_state() clears them from the
caller state and the backtracked parent state prints as:

  mark_precise: frame0: parent state regs= stack=:  R10=fp0

Update both expectations accordingly.

Fixes: 147400e9c27c ("bpf: Compute parameters liveness in bpf_compute_live_registers()")
Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
---
 tools/testing/selftests/bpf/progs/stack_arg_precision.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/stack_arg_precision.c b/tools/testing/selftests/bpf/progs/stack_arg_precision.c
index 7baa9c1c8e31e..c9564776d7333 100644
--- a/tools/testing/selftests/bpf/progs/stack_arg_precision.c
+++ b/tools/testing/selftests/bpf/progs/stack_arg_precision.c
@@ -74,7 +74,7 @@ __msg("mark_precise: frame1: parent state regs= stack=:  frame1: R10=fp0")
 __msg("mark_precise: frame0: parent state regs= stack=:  R10=fp0")
 __msg("mark_precise: frame1: last_idx 11 first_idx 11 subseq_idx 13")
 __msg("mark_precise: frame1: regs= stack= before 11: (85) call pc+1")
-__msg("mark_precise: frame0: parent state regs= stack=:  R1=1 R2=2 R3=3 R4=4 R5=5 R10=fp0")
+__msg("mark_precise: frame0: parent state regs= stack=:  R10=fp0")
 __msg("mark_precise: frame0: last_idx 9 first_idx 7 subseq_idx 11")
 __msg("mark_precise: frame0: regs= stack= before 9: (05) goto pc+1")
 __msg("mark_precise: frame0: regs= stack= before 8: (7a) *(u64 *)(r11 -8) = 4")
@@ -95,7 +95,7 @@ __msg("mark_precise: frame1: parent state regs= stack=:  frame1: R10=fp0")
 __msg("mark_precise: frame0: parent state regs= stack=:  R10=fp0")
 __msg("mark_precise: frame1: last_idx 11 first_idx 11 subseq_idx 13 ")
 __msg("mark_precise: frame1: regs= stack= before 11: (85) call pc+1")
-__msg("mark_precise: frame0: parent state regs= stack=:  R1=1 R2=2 R3=3 R4=4 R5=5 R10=fp0")
+__msg("mark_precise: frame0: parent state regs= stack=:  R10=fp0")
 __msg("mark_precise: frame0: last_idx 10 first_idx 10 subseq_idx 11 ")
 __msg("mark_precise: frame0: regs= stack= before 10: (7a) *(u64 *)(r11 -8) = 6")
 __naked void stack_arg_precision_bpf2bpf(void)
-- 
2.43.0

[PATCH bpf-next v2 3/7] selftests/bpf: Fix stack arg pruning test JOIN log expectation

[KaFai Wan]

Commit f414d20879ae ("bpf: Save DFS postorder numbers for program
instructions") changed bpf_compute_postorder() to use a real DFS, which
changes the postorder of this test's instructions and thus the insn edge
the arg track join at insn 10 is reported on:

  arg JOIN insn 8 -> 10 r1: _ + fp0-8 => fp0-8|fp0+0
  arg JOIN insn 8 -> 10 sa0: _ + fp0-8 => fp0-8|fp0+0

Update the r1 and sa0 expectations to the new output.

Fixes: f414d20879ae ("bpf: Save DFS postorder numbers for program instructions")
Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
---
 tools/testing/selftests/bpf/progs/verifier_stack_arg.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
index f6a4a6aa0e617..b9f799f6e4051 100644
--- a/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
+++ b/tools/testing/selftests/bpf/progs/verifier_stack_arg.c
@@ -117,8 +117,8 @@ SEC("tc")
 __description("stack_arg: pruning with different stack arg types")
 __failure __log_level(2)
 __flag(BPF_F_TEST_STATE_FREQ)
-__msg("arg JOIN insn 9 -> 10 r1: fp0-8 + _ => fp0-8|fp0+0")
-__msg("arg JOIN insn 9 -> 10 sa0: fp0-8 + _ => fp0-8|fp0+0")
+__msg("arg JOIN insn 8 -> 10 r1: _ + fp0-8 => fp0-8|fp0+0")
+__msg("arg JOIN insn 8 -> 10 sa0: _ + fp0-8 => fp0-8|fp0+0")
 __msg("R{{[0-9]}} invalid mem access 'scalar'")
 __naked void stack_arg_pruning_type_mismatch(void)
 {
-- 
2.43.0

[PATCH bpf-next v2 4/7] selftests/bpf: Use _exit() in uprobe_multi_test clone-ed child

[KaFai Wan]

child_func() terminates with exit(), but spawn_child_flag() also starts
it via clone(CLONE_VM), which shares the parent's address space. That
exit() then runs atexit handlers and static destructors against globals
the parent is still using.

LLVM commit `3c3ed59c6a2a ("[llvm] Add format check for
MCSubtargetFeatures")` [1], included in the LLVM 23.1 releases, added
Target::isValidFeatureListFormat() with a function-local static holding
the compiled regex:

	static const llvm::Regex pattern("^([+-][^,]+)(,[+-][^,]+)*,?$");
	return pattern.match(Features);

The child exit destroys that static while its guard variable still marks
it as initialized, so the regex is never rebuilt in the parent. From that
point on, get_jited_program_text() gets NULL from
LLVMCreateDisasmCPUFeatures(), so every jited program text check in the
tests running after filter_clone_vm fails, e.g. "verifier_jit_convergence",
"verifier_private_stack", "verifier_tailcall_jit" and "verifier_jit_inline".

Use _exit(), which runs neither atexit handlers nor static destructors, as
uprobe_syscall.c already does for its own CLONE_VM child.

[1] https://github.com/llvm/llvm-project/pull/180943
Fixes: d2520bdb1932f ("selftests/bpf: Add uprobe multi pid filter test for clone-ed processes")
Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
---
 tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
index f0baf5738b751..7fbcfddc3cc36 100644
--- a/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
+++ b/tools/testing/selftests/bpf/prog_tests/uprobe_multi_test.c
@@ -94,14 +94,14 @@ static int child_func(void *arg)
 	/* wait for parent's kick */
 	err = read(child->go[0], &c, 1);
 	if (err != 1)
-		exit(err);
+		_exit(err);
 
 	uprobe_multi_func_1();
 	uprobe_multi_func_2();
 	uprobe_multi_func_3();
 	usdt_trigger();
 
-	exit(errno);
+	_exit(errno);
 }
 
 static int spawn_child_flag(struct child *child, bool clone_vm)
-- 
2.43.0

[PATCH bpf-next v2 5/7] selftests/bpf: Fix prog_tests framework subtest count checks

[KaFai Wan]

The hardcoded expected skip_cnt/error_cnt in test_prog_tests_framework()
mismatch the real count when the binary is run with -t or -n filters,
since some subtests are not selected. Count the executed subtests and
compare against that value instead.

Fixes: 2324257dbd68 ("selftests/bpf: Refactor prog_tests logging and test execution")
Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
---
 .../selftests/bpf/prog_tests/prog_tests_framework.c    | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
index d111fe1054476..53347f3799245 100644
--- a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
+++ b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
@@ -13,6 +13,7 @@ static void clear_test_state(struct test_state *state)
 void test_prog_tests_framework(void)
 {
 	struct test_state *state = env.test_state;
+	int subtest_cnt;
 
 	/* in all the ASSERT calls below we need to return on the first
 	 * error due to the fact that we are cleaning the test state after
@@ -30,25 +31,30 @@ void test_prog_tests_framework(void)
 		return;
 	clear_test_state(state);
 
+	subtest_cnt = 0;
 	if (test__start_subtest("test_skip_subtest")) {
 		test__skip();
 		test__end_subtest();
+		subtest_cnt++;
 	}
 	if (test__start_subtest("test_skip_subtest")) {
 		test__skip();
 		test__end_subtest();
+		subtest_cnt++;
 	}
-	if (!ASSERT_EQ(state->skip_cnt, 2, "skip_cnt_check"))
+	if (!ASSERT_EQ(state->skip_cnt, subtest_cnt, "skip_cnt_check"))
 		return;
 	if (!ASSERT_EQ(state->subtest_num, 3, "subtest_num_check"))
 		return;
 	clear_test_state(state);
 
+	subtest_cnt = 0;
 	if (test__start_subtest("test_fail_subtest")) {
 		test__fail();
 		test__end_subtest();
+		subtest_cnt++;
 	}
-	if (!ASSERT_EQ(state->error_cnt, 1, "error_cnt_check"))
+	if (!ASSERT_EQ(state->error_cnt, subtest_cnt, "error_cnt_check"))
 		return;
 	if (!ASSERT_EQ(state->subtest_num, 4, "subtest_num_check"))
 		return;
-- 
2.43.0

[PATCH bpf-next v2 6/7] selftests/bpf: Skip expected_msgs subtests when stdout is not saved

[KaFai Wan]

stdio_hijack_init() does not set subtest_state->stdout_saved when
running with -v in non-parallel mode, since output goes straight to
stdout. In that case subtest_state->log_buf is left NULL, so the
expected-messages subtests crash when inspecting it. Skip them when
stdout was not saved.

Fixes: 34c513be3dad ("selftests/bpf: __not_msg() tag for test_loader framework")
Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
---
 .../testing/selftests/bpf/prog_tests/prog_tests_framework.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
index 53347f3799245..b4cd7c7ff4391 100644
--- a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
+++ b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
@@ -169,6 +169,12 @@ void test_prog_tests_framework_expected_msgs(void)
 
 	for (i = 0; i < ARRAY_SIZE(cases); i++) {
 		if (test__start_subtest(cases[i].name)) {
+			if (!env.subtest_state->stdout_saved) {
+				test__skip();
+				test__end_subtest();
+				continue;
+			}
+
 			error_cnt = env.subtest_state->error_cnt;
 			msgs.patterns = cases[i].pats;
 			msgs.cnt = 0;
-- 
2.43.0

[PATCH bpf-next v2 7/7] selftests/bpf: Skip compare_text subtest when stdout is not saved

[KaFai Wan]

stdio_hijack_init() does not set subtest_state->stdout_saved when
running with -v in non-parallel mode, since output goes straight to
stdout. In that case subtest_state->log_buf is left NULL, so the
compare_text_mismatch subtest crashes when inspecting it. Skip it when
stdout was not saved.

Fixes: bd71904697c6 ("selftests/bpf: Add compare_text_to_expected() helper")
Signed-off-by: KaFai Wan <kafai.wan@linux.dev>
---
 .../bpf/prog_tests/prog_tests_framework.c      | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
index b4cd7c7ff4391..ddb6cdc1579f1 100644
--- a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
+++ b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
@@ -203,14 +203,18 @@ void test_prog_tests_framework_compare_text(void)
 	}
 
 	if (test__start_subtest("compare_text_mismatch")) {
-		err = compare_text_to_expected("line two\n", "line one\n");
-		fflush(stdout);
+		if (!env.subtest_state->stdout_saved) {
+			test__skip();
+		} else {
+			err = compare_text_to_expected("line two\n", "line one\n");
+			fflush(stdout);
 
-		ASSERT_EQ(err, -1, "mismatch_rc");
-		ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "-line one",
-				  "diff_has_expected");
-		ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "+line two",
-				  "diff_has_actual");
+			ASSERT_EQ(err, -1, "mismatch_rc");
+			ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "-line one",
+					  "diff_has_expected");
+			ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "+line two",
+					  "diff_has_actual");
+		}
 		test__end_subtest();
 	}
 }
-- 
2.43.0

Re: [PATCH bpf-next v2 6/7] selftests/bpf: Skip expected_msgs subtests when stdout is not saved

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2115 bytes --]

> selftests/bpf: Skip expected_msgs subtests when stdout is not saved

TL;DR: Nit only: the new test__skip() in the expected_msgs selftest is silent,
so -v output doesn't say why subtests were skipped; the review asks whether to
print a short reason. The skip logic itself looks correct.

> diff --git a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> index 53347f379924..b4cd7c7ff439 100644
> --- a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> +++ b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c

[ ... ]

> @@ -169,6 +169,12 @@ void test_prog_tests_framework_expected_msgs(void)
>
>  	for (i = 0; i < ARRAY_SIZE(cases); i++) {
>  		if (test__start_subtest(cases[i].name)) {
> +			if (!env.subtest_state->stdout_saved) {
> +				test__skip();
> +				test__end_subtest();
> +				continue;
> +			}
> +

This isn't a bug, but a subsystem guideline flags a test__skip() that is
issued silently, with no message.

test__skip() here is called without any explanation, so a -v run only shows
the generic "#N/M <subtest>:SKIP" line from test__end_subtest().  Someone
reading that output has to look at the code or the commit message to learn
the subtests were skipped because stdout is not captured in non-parallel
verbose mode.

The skip itself looks correct, since stdio_hijack_init() returns early when
verbose() && env.worker_id == -1 and leaves subtest_state->stdout_saved and
log_buf unset.  test__skip() takes no reason argument, and many other
test__skip() callers in prog_tests are also bare.

Would it be worth printing a short reason before test__skip(), for example:

    fprintf(stderr, "SKIP: stdout is not saved in non-parallel verbose mode\n");

>  			error_cnt = env.subtest_state->error_cnt;
>  			msgs.patterns = cases[i].pats;
>  			msgs.cnt = 0;


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38559567568

Re: [PATCH bpf-next v2 7/7] selftests/bpf: Skip compare_text subtest when stdout is not saved

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2710 bytes --]

> selftests/bpf: Skip compare_text subtest when stdout is not saved

TL;DR: Nit only: the new test__skip() in the compare_text_mismatch subtest
gives no reason for skipping; a short comment or stderr message might help.
The skip logic itself looks correct.

> diff --git a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> index b4cd7c7ff439..ddb6cdc1579f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> +++ b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c

[ ... ]

> @@ -203,14 +203,18 @@ void test_prog_tests_framework_compare_text(void)
>  	}
>
>  	if (test__start_subtest("compare_text_mismatch")) {
> -		err = compare_text_to_expected("line two\n", "line one\n");
> -		fflush(stdout);
> +		if (!env.subtest_state->stdout_saved) {
> +			test__skip();
> +		} else {
> +			err = compare_text_to_expected("line two\n", "line one\n");
> +			fflush(stdout);
>
> -		ASSERT_EQ(err, -1, "mismatch_rc");
> -		ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "-line one",
> -				  "diff_has_expected");
> -		ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "+line two",
> -				  "diff_has_actual");
> +			ASSERT_EQ(err, -1, "mismatch_rc");
> +			ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "-line one",
> +					  "diff_has_expected");
> +			ASSERT_HAS_SUBSTR(env.subtest_state->log_buf, "+line two",
> +					  "diff_has_actual");
> +		}
>  		test__end_subtest();
>  	}
>  }

This isn't a bug, but could the skip in the compare_text_mismatch subtest
carry an explanation?

	if (!env.subtest_state->stdout_saved) {
		test__skip();
	}

A subsystem guideline flags selftests that skip silently with no message,
and test__skip() here prints nothing about why the subtest was skipped.
The same condition covers -v in non-parallel mode, a libc without
open_memstream() support, and an open_memstream() failure, so the reason is
not obvious from the test output alone.

For context, the skip itself looks correct: env.subtest_state is non-NULL
inside the subtest, test__skip() sets subtest_state->skipped, and
test__end_subtest() handles a NULL stdout_saved.

The other skips in this file are also bare test__skip() calls, and
test__skip() takes no reason argument, so the guideline may not map cleanly
onto test_progs.  If a reason is wanted, would a short comment above the
test__skip() call, or a separate message printed to env.stderr_saved, be
appropriate?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38559567568

Re: [PATCH bpf-next v2 6/7] selftests/bpf: Skip expected_msgs subtests when stdout is not saved

[KaFai Wan]

On Wed, 2026-10-14 at 12:08 +0000, bot+bpf-ci@kernel.org wrote:
> > selftests/bpf: Skip expected_msgs subtests when stdout is not saved
> 
> TL;DR: Nit only: the new test__skip() in the expected_msgs selftest is silent,
> so -v output doesn't say why subtests were skipped; the review asks whether to
> print a short reason. The skip logic itself looks correct.
> 
> > diff --git a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> > b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> > index 53347f379924..b4cd7c7ff439 100644
> > --- a/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> > +++ b/tools/testing/selftests/bpf/prog_tests/prog_tests_framework.c
> 
> [ ... ]
> 
> > @@ -169,6 +169,12 @@ void test_prog_tests_framework_expected_msgs(void)
> > 
> >  	for (i = 0; i < ARRAY_SIZE(cases); i++) {
> >  		if (test__start_subtest(cases[i].name)) {
> > +			if (!env.subtest_state->stdout_saved) {
> > +				test__skip();
> > +				test__end_subtest();
> > +				continue;
> > +			}
> > +
> 
> This isn't a bug, but a subsystem guideline flags a test__skip() that is
> issued silently, with no message.
> 
> test__skip() here is called without any explanation, so a -v run only shows
> the generic "#N/M <subtest>:SKIP" line from test__end_subtest().  Someone
> reading that output has to look at the code or the commit message to learn
> the subtests were skipped because stdout is not captured in non-parallel
> verbose mode.
> 
> The skip itself looks correct, since stdio_hijack_init() returns early when
> verbose() && env.worker_id == -1 and leaves subtest_state->stdout_saved and
> log_buf unset.  test__skip() takes no reason argument, and many other
> test__skip() callers in prog_tests are also bare.
> 
> Would it be worth printing a short reason before test__skip(), for example:
> 
>     fprintf(stderr, "SKIP: stdout is not saved in non-parallel verbose mode\n");

I don't think we need this. for two reasons:
a) -v is mainly used for debugging, showing details; this is a test for the framework, not the real
bpf test, all tests should pass, no need to debug;
b) this is a loop. if we keep adding this message, it could repeat for 9 lines.
> 
> >  			error_cnt = env.subtest_state->error_cnt;
> >  			msgs.patterns = cases[i].pats;
> >  			msgs.cnt = 0;
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/38559567568

-- 
Thanks,
KaFai