[bpf-next 0/4] selftest related fixes

[bpf-next 0/4] selftest related fixes

[Vineet Gupta]

Hi,

This is a set of fixes accumulated during BPF_GCC testing and an in-works
verifier enhacement series for 32-bit tracking.

Thx,
-Vineet

Vineet Gupta (4):
  selftests/bpf: map_kptr: force BPF_STX for the scalar store to kptr
  selftests/bpf: add --no-error-summary to skip end-of-run error log
    dump
  selftests/bpf: report failed subtest count in test_progs summary
  selftests/bpf: vmtest.sh: preserve command quoting when running in the
    VM

 .../selftests/bpf/progs/map_kptr_fail.c       | 11 +++-
 tools/testing/selftests/bpf/test_progs.c      | 62 ++++++++++++-------
 tools/testing/selftests/bpf/test_progs.h      |  3 +-
 tools/testing/selftests/bpf/vmtest.sh         | 13 +++-
 4 files changed, 64 insertions(+), 25 deletions(-)

-- 
2.55.0

[bpf-next 1/4] selftests/bpf: map_kptr: force BPF_STX for the scalar store to kptr

[Vineet Gupta]

reject_scalar_store_to_kptr stores a scalar constant to a kptr field:

        *(volatile u64 *)&v->unref_ptr = 0xBADC0DE;

Compilers generate one of two encodings for that:

 1. Materialize the constant into a register and emit BPF_STX:

        r1 = 0xbadc0de
        *(u64 *)(r0 + 0x8) = r1

 2. Or fold it into a single BPF_ST (store immediate):

        *(u64 *)(r0 + 0x8) = 0xbadc0de

check_map_kptr_access() rejects both, but through very different checks.
BPF_STX goes through map_kptr_match_type(), whose first test is
base_type(reg->type) != PTR_TO_BTF_ID - the scalar rejection this test is
named for - and which prints "invalid kptr access, R...". BPF_ST only gets
the trivial "BPF_ST imm must be 0 when storing to kptr" immediate check and
never reaches map_kptr_match_type() at all.

So on a compiler that folds the constant - bpf-gcc, and clang from
-mcpu=v4, which enabled BPF_ST around v4 support due to historical
verifier limitations - the test fails against its expected message.

Widening the __msg to accept either message would make it pass again, but
on those toolchains it would then only re-test the imm != 0 path, which
verifier/map_kptr.c ("map_kptr: BPF_ST imm != 0") already covers, and the
scalar-vs-PTR_TO_BTF_ID check would lose its only test in the tree.

Route the value through barrier_var() instead, so the store stays a
BPF_STX everywhere and the test keeps asserting what it was written to
assert. clang -mcpu=v1..v4 and bpf-gcc 16.1 all emit the register form
afterwards.

  bpf-gcc, before: #229/20 map_kptr/reject_scalar_store_to_kptr:FAIL
  bpf-gcc, after : #229/20 map_kptr/reject_scalar_store_to_kptr:OK

Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
---
 tools/testing/selftests/bpf/progs/map_kptr_fail.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
index f11848dfa78f..cb84e23b83c0 100644
--- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
@@ -390,13 +390,22 @@ __failure __msg("invalid kptr access, R")
 int reject_scalar_store_to_kptr(struct __sk_buff *ctx)
 {
 	struct map_value *v;
+	u64 val = 0xBADC0DE;
 	int key = 0;
 
 	v = bpf_map_lookup_elem(&array_map, &key);
 	if (!v)
 		return 0;
 
-	*(volatile u64 *)&v->unref_ptr = 0xBADC0DE;
+	/*
+	 * Keep the value in a register so this stays a BPF_STX and keeps
+	 * exercising map_kptr_match_type(). Compilers that fold the constant
+	 * into a BPF_ST (store immediate) instead - bpf-gcc, and clang from
+	 * -mcpu=v4 - would be rejected by the far weaker "BPF_ST imm must be
+	 * 0" check, which verifier/map_kptr.c already covers.
+	 */
+	barrier_var(val);
+	*(volatile u64 *)&v->unref_ptr = val;
 	return 0;
 }
 
-- 
2.55.0

[bpf-next 2/4] selftests/bpf: add --no-error-summary to skip end-of-run error log dump

[Vineet Gupta]

By default test_progs re-prints the aggregated error logs of all failed
tests at the end of the run (when not in verbose mode), starting with
"All error logs:".

With bpf-gcc the current failures and a couple runaway 1M fails cause a
huge print overhead/delay at the end.

Add a subtractive --no-error-summary flag, gated on a new
env.error_summary field which defaults to true, so the default behavior
is unchanged. Passing --no-error-summary suppresses the final
"All error logs:" dump.

Only the human readable output is elided. dump_test_log() also emits the
per-test and per-subtest entries of the --json-summary "results" array,
so it keeps being called (via a new @quiet argument) and the JSON report
is bit for bit what it was before.

Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
---
 tools/testing/selftests/bpf/test_progs.c | 41 ++++++++++++++++--------
 tools/testing/selftests/bpf/test_progs.h |  1 +
 2 files changed, 29 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index aa06bab30966..8ee46745e2e8 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -424,10 +424,12 @@ static void jsonw_write_log_message(json_writer_t *w, char *log_buf, size_t log_
 	}
 }
 
+/* @quiet elides the human readable output, the JSON report is unaffected */
 static void dump_test_log(const struct prog_test_def *test,
 			  const struct test_state *test_state,
 			  bool skip_ok_subtests,
 			  bool par_exec_result,
+			  bool quiet,
 			  json_writer_t *w)
 {
 	bool test_failed = test_state->error_cnt > 0;
@@ -449,7 +451,7 @@ static void dump_test_log(const struct prog_test_def *test,
 	if (verbose() && !par_exec_result)
 		return;
 
-	if (test_state->log_cnt && print_test)
+	if (test_state->log_cnt && print_test && !quiet)
 		print_test_log(test_state->log_buf, test_state->log_cnt);
 
 	if (w && print_test) {
@@ -471,15 +473,16 @@ static void dump_test_log(const struct prog_test_def *test,
 		if ((skip_ok_subtests && !subtest_failed) || subtest_filtered)
 			continue;
 
-		if (subtest_state->log_cnt && print_subtest) {
+		if (subtest_state->log_cnt && print_subtest && !quiet) {
 			print_test_log(subtest_state->log_buf,
 				       subtest_state->log_cnt);
 		}
 
-		print_subtest_name(test->test_num, i + 1,
-				   test->test_name, subtest_state->name,
-				   test_result(subtest_state->error_cnt,
-					       subtest_state->skipped));
+		if (!quiet)
+			print_subtest_name(test->test_num, i + 1,
+					   test->test_name, subtest_state->name,
+					   test_result(subtest_state->error_cnt,
+						       subtest_state->skipped));
 
 		if (w && print_subtest) {
 			jsonw_start_object(w);
@@ -496,7 +499,8 @@ static void dump_test_log(const struct prog_test_def *test,
 		jsonw_end_object(w);
 	}
 
-	print_test_result(test, test_state);
+	if (!quiet)
+		print_test_result(test, test_state);
 }
 
 /* A bunch of tests set custom affinity per-thread and/or per-process. Reset
@@ -899,6 +903,7 @@ enum ARG_KEYS {
 	ARG_JSON_SUMMARY = 'J',
 	ARG_TRAFFIC_MONITOR = 'm',
 	ARG_WATCHDOG_TIMEOUT = 'w',
+	ARG_NO_ERROR_SUMMARY = -2,
 };
 
 static const struct argp_option opts[] = {
@@ -931,6 +936,8 @@ static const struct argp_option opts[] = {
 #endif
 	{ "watchdog-timeout", ARG_WATCHDOG_TIMEOUT, "SECONDS", 0,
 	  "Kill the process if tests are not making progress for specified number of seconds." },
+	{ "no-error-summary", ARG_NO_ERROR_SUMMARY, NULL, 0,
+	  "Do not re-print the aggregated error logs of failed tests at the end of the run." },
 	{},
 };
 
@@ -1132,6 +1139,9 @@ static error_t parse_arg(int key, char *arg, struct argp_state *state)
 	case ARG_DEBUG:
 		env->debug = true;
 		break;
+	case ARG_NO_ERROR_SUMMARY:
+		env->error_summary = false;
+		break;
 	case ARG_JSON_SUMMARY:
 		env->json = fopen(arg, "w");
 		if (env->json == NULL) {
@@ -1304,7 +1314,7 @@ static void dump_crash_log(void)
 
 	if (env.test) {
 		env.test_state->error_cnt++;
-		dump_test_log(env.test, env.test_state, true, false, NULL);
+		dump_test_log(env.test, env.test_state, true, false, false, NULL);
 	}
 }
 
@@ -1462,7 +1472,7 @@ static void run_one_test(int test_num)
 
 	free(stop_libbpf_log_capture());
 
-	dump_test_log(test, state, false, false, NULL);
+	dump_test_log(test, state, false, false, false, NULL);
 }
 
 struct dispatch_data {
@@ -1623,7 +1633,7 @@ static void *dispatch_thread(void *ctx)
 		} while (false);
 
 		pthread_mutex_lock(&stdout_output_lock);
-		dump_test_log(test, state, false, true, NULL);
+		dump_test_log(test, state, false, true, false, NULL);
 		pthread_mutex_unlock(&stdout_output_lock);
 	} /* while (true) */
 error:
@@ -1686,9 +1696,12 @@ static void calculate_summary_and_print_errors(struct test_env *env)
 	 * We only print error logs summary when there are failed tests and
 	 * verbose mode is not enabled. Otherwise, results may be inconsistent.
 	 *
+	 * --no-error-summary only elides the human readable dump: the walk
+	 * still happens so the JSON report keeps its per-test results.
 	 */
-	if (!verbose() && fail_cnt) {
-		printf("\nAll error logs:\n");
+	if (!verbose() && fail_cnt && (env->error_summary || w)) {
+		if (env->error_summary)
+			printf("\nAll error logs:\n");
 
 		/* print error logs again */
 		for (i = 0; i < prog_test_cnt; i++) {
@@ -1698,7 +1711,8 @@ static void calculate_summary_and_print_errors(struct test_env *env)
 			if (!state->tested || !state->error_cnt)
 				continue;
 
-			dump_test_log(test, state, true, true, w);
+			dump_test_log(test, state, true, true,
+				      !env->error_summary, w);
 		}
 	}
 
@@ -2028,6 +2042,7 @@ int main(int argc, char **argv)
 
 	env.secs_till_notify = 10;
 	env.secs_till_kill = 120;
+	env.error_summary = true;
 	err = argp_parse(&argp, argc, argv, 0, NULL, &env);
 	if (err)
 		return err;
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index 2cf950afcd85..e66d5c457901 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -105,6 +105,7 @@ struct test_env {
 	struct test_selector tmon_selector;
 	bool verifier_stats;
 	bool debug;
+	bool error_summary;
 	enum verbosity verbosity;
 
 	bool jit_enabled;
-- 
2.55.0

[bpf-next 3/4] selftests/bpf: report failed subtest count in test_progs summary

[Vineet Gupta]

The final summary line is asymmetric: the PASSED field reports both the
number of top-level tests and the number of subtests within them, while
the FAILED field reports only top-level tests:

  Summary: 640/5750 PASSED, 7760 SKIPPED, 100 FAILED

There is no way to tell whether those 100 failing tests amount to 100
broken subtests or 1000. So count subtests with a non-zero error_cnt
into a new sub_fail_cnt and print it alongside fail_cnt:

  Summary: 640/5750 PASSED, 7760 SKIPPED, 100/342 FAILED
                                             ^^^^^

This is correct for -j runs, as subtest_states[] is populated both in
sequential and parallel modes.

A test that fails without declaring any subtests contributes 0 to
sub_fail_cnt. That mirrors the existing behaviour of sub_succ_cnt for
tests that pass without subtests, keeping the two numerators
comparable.

Also emit the new count as a "failed_subtest" field in the JSON output,
for parity with the existing "success_subtest".

Note that this changes the trailing field of the summary line from a bare
integer to "A/B", so anything scraping "N FAILED" out of it needs updating.
While here, fix the fail_cnt comment in struct test_env, which claims it
counts "total failed tests + sub-tests".

Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
---
 tools/testing/selftests/bpf/test_progs.c | 21 +++++++++++++--------
 tools/testing/selftests/bpf/test_progs.h |  2 +-
 2 files changed, 14 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index 8ee46745e2e8..5e2f6ec2e212 100644
--- a/tools/testing/selftests/bpf/test_progs.c
+++ b/tools/testing/selftests/bpf/test_progs.c
@@ -1656,8 +1656,8 @@ static void *dispatch_thread(void *ctx)
 
 static void calculate_summary_and_print_errors(struct test_env *env)
 {
-	int i;
-	int succ_cnt = 0, fail_cnt = 0, sub_succ_cnt = 0, skip_cnt = 0;
+	int i, j;
+	int succ_cnt = 0, fail_cnt = 0, sub_succ_cnt = 0, sub_fail_cnt = 0, skip_cnt = 0;
 	json_writer_t *w = NULL;
 
 	for (i = 0; i < prog_test_cnt; i++) {
@@ -1670,10 +1670,14 @@ static void calculate_summary_and_print_errors(struct test_env *env)
 		sub_succ_cnt += state->sub_succ_cnt;
 		skip_cnt += state->skip_cnt;
 
-		if (state->error_cnt)
+		if (state->error_cnt) {
 			fail_cnt++;
-		else if (!test->not_built)
+			for (j = 0; j < state->subtest_num; j++)
+				if (state->subtest_states[j].error_cnt)
+					sub_fail_cnt++;
+		} else if (!test->not_built) {
 			succ_cnt++;
+		}
 	}
 
 	if (env->json) {
@@ -1688,6 +1692,7 @@ static void calculate_summary_and_print_errors(struct test_env *env)
 		jsonw_uint_field(w, "success_subtest", sub_succ_cnt);
 		jsonw_uint_field(w, "skipped", skip_cnt);
 		jsonw_uint_field(w, "failed", fail_cnt);
+		jsonw_uint_field(w, "failed_subtest", sub_fail_cnt);
 		jsonw_name(w, "results");
 		jsonw_start_array(w);
 	}
@@ -1726,12 +1731,12 @@ static void calculate_summary_and_print_errors(struct test_env *env)
 		fclose(env->json);
 
 	if (env->not_built_cnt)
-		printf("Summary: %d/%d PASSED, %d SKIPPED (%d not built), %d FAILED\n",
+		printf("Summary: %d/%d PASSED, %d SKIPPED (%d not built), %d/%d FAILED\n",
 		       succ_cnt, sub_succ_cnt, skip_cnt, env->not_built_cnt,
-		       fail_cnt);
+		       fail_cnt, sub_fail_cnt);
 	else
-		printf("Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
-		       succ_cnt, sub_succ_cnt, skip_cnt, fail_cnt);
+		printf("Summary: %d/%d PASSED, %d SKIPPED, %d/%d FAILED\n",
+		       succ_cnt, sub_succ_cnt, skip_cnt, fail_cnt, sub_fail_cnt);
 
 	env->succ_cnt = succ_cnt;
 	env->sub_succ_cnt = sub_succ_cnt;
diff --git a/tools/testing/selftests/bpf/test_progs.h b/tools/testing/selftests/bpf/test_progs.h
index e66d5c457901..ea493c477fbd 100644
--- a/tools/testing/selftests/bpf/test_progs.h
+++ b/tools/testing/selftests/bpf/test_progs.h
@@ -124,7 +124,7 @@ struct test_env {
 
 	int succ_cnt; /* successful tests */
 	int sub_succ_cnt; /* successful sub-tests */
-	int fail_cnt; /* total failed tests + sub-tests */
+	int fail_cnt; /* failed tests */
 	int skip_cnt; /* skipped tests */
 	int not_built_cnt; /* tests not built */
 
-- 
2.55.0

[bpf-next 4/4] selftests/bpf: vmtest.sh: preserve command quoting when running in the VM

[Vineet Gupta]

vmtest.sh captures the trailing command with command="$@", which flattens
the arguments into a single space-separated string, and then pastes it
into the generated guest init script:

        cd /root/bpf
        echo ${command}
        stdbuf -oL -eL ${command}

That here-doc is unquoted, so the host expands ${command} and the
flattened text lands in the script verbatim. The guest bash then parses
those lines as shell source, re-splitting the text on whitespace and
glob-expanding it against /root/bpf. As a result any command with a glob
or an argument containing spaces is corrupted before it reaches the test
binary. For example:

        vmtest.sh -- ./test_progs -a 'verifier_*'

has 'verifier_*' expanded in the guest into the matching object/skeleton
files (verifier_align.bpf.o verifier_align.skel.h ...), so test_progs is
handed a list of filenames instead of the intended name filter and runs no
matching tests.

Quote each argument with printf '%q ' so the command is reproduced
verbatim inside the VM: the escaped text goes through exactly one round
of quote removal when the guest parses the init script, yielding the
original argv with globs and special characters intact. The common case
(e.g. -t <name>) is unaffected.

Only do this when there is a command to quote. printf '%q ' with no
arguments still applies the format once and emits '', which the -s
(debug shell) path would take for a real command and try to run.

Note this makes the trailing command strictly an argv rather than a shell
snippet: passing it pre-quoted as one word, e.g.

        vmtest.sh -- "./test_progs -t foo"

no longer works, and neither does embedding guest-side shell syntax such
as ';' or a redirection. Neither form is documented - usage() and
README.rst both show the command unquoted - and 'sh -c ...' still works.

Fixes: c9709f52386d ("bpf: Helper script for running BPF presubmit tests")
Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>
---
 tools/testing/selftests/bpf/vmtest.sh | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh
index 9ca802285393..6a3d026d76bd 100755
--- a/tools/testing/selftests/bpf/vmtest.sh
+++ b/tools/testing/selftests/bpf/vmtest.sh
@@ -428,8 +428,17 @@ main()
 
 	if [[ $# -eq 0  && "${debug_shell}" == "no" ]]; then
 		echo "No command specified, will run ${DEFAULT_COMMAND} in the vm"
-	else
-		command="$@"
+	elif [[ $# -gt 0 ]]; then
+		# Quote each argument so the command survives into the guest: the
+		# host expands ${command} into the generated init script, which
+		# the guest bash then parses as shell source. Without the %q
+		# escapes an argument with a space or a glob (e.g. -a 'verifier_*')
+		# is re-split and expanded against /root/bpf there.
+		#
+		# Skip this when there is no command: printf '%q ' would still
+		# apply the format once and emit '', which is not the empty
+		# command that -s (debug shell) expects.
+		command=$(printf '%q ' "$@")
 	fi
 
 	local kconfig_file="${OUTPUT_DIR}/latest.config"
-- 
2.55.0

Re: [bpf-next 2/4] selftests/bpf: add --no-error-summary to skip end-of-run error log dump

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index aa06bab309665..8ee46745e2e85 100644
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c

[ ... ]

> @@ -1686,9 +1696,12 @@ static void calculate_summary_and_print_errors(struct test_env *env)
>  	 * We only print error logs summary when there are failed tests and
>  	 * verbose mode is not enabled. Otherwise, results may be inconsistent.
>  	 *
> +	 * --no-error-summary only elides the human readable dump: the walk
> +	 * still happens so the JSON report keeps its per-test results.
>  	 */
> -	if (!verbose() && fail_cnt) {
> -		printf("\nAll error logs:\n");
> +	if (!verbose() && fail_cnt && (env->error_summary || w)) {
> +		if (env->error_summary)
> +			printf("\nAll error logs:\n");

Does the comment match what the code does?  The comment says "the walk
still happens", but the guard now requires either env->error_summary or w
to be non-NULL.

When --no-error-summary is passed without --json-summary, both
env->error_summary and w are false/NULL, so the entire for loop at the
bottom of this function gets skipped, not just the human readable output.

The walk does happen with --no-error-summary --json-summary FILE (where
w != NULL), but not in the no-JSON case. Could the comment be more
specific, maybe "the walk still happens when a JSON report was requested,
so the JSON report keeps its per-test results"?


---
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/30836252778

Re: [bpf-next 3/4] selftests/bpf: report failed subtest count in test_progs summary

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> --- a/tools/testing/selftests/bpf/test_progs.c
> +++ b/tools/testing/selftests/bpf/test_progs.c

[ ... ]

> @@ -1726,12 +1731,12 @@
>  	fclose(env->json);
>
>  if (env->not_built_cnt)
> -	printf("Summary: %d/%d PASSED, %d SKIPPED (%d not built), %d FAILED\n",
> +	printf("Summary: %d/%d PASSED, %d SKIPPED (%d not built), %d/%d FAILED\n",
>  	       succ_cnt, sub_succ_cnt, skip_cnt, env->not_built_cnt,
> -	       fail_cnt);
> +	       fail_cnt, sub_fail_cnt);
>  else
> -	printf("Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
> -	       succ_cnt, sub_succ_cnt, skip_cnt, fail_cnt);
> +	printf("Summary: %d/%d PASSED, %d SKIPPED, %d/%d FAILED\n",
> +	       succ_cnt, sub_succ_cnt, skip_cnt, fail_cnt, sub_fail_cnt);

A subsystem pattern flags this as potentially concerning: the BPF selftests
guide asks that test binaries emit TAP-compliant output via kselftest.h
(ksft_set_plan()/ksft_finished()) rather than hand-formatting their own
summary lines with printf(), so that kselftest runners and CI can parse the
results uniformly.

test_progs.c has always had its own reporting framework and does not use
kselftest.h; this predates the patch by many years and converting it is far
outside the scope of this change. The patch only widens one existing field on
an already ad hoc line.

The changelog acknowledges that the field-format change means "anything
scraping 'N FAILED' out of it needs updating." A search over .github/workflows,
.github/scripts, ci/, tools/testing/selftests/bpf/*.sh and Documentation/
turned up no in-tree parser of the test_progs summary line.

Does this output format remain compatible with the consumers you're aware of?


---
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/30836252778

Re: [bpf-next 4/4] selftests/bpf: vmtest.sh: preserve command quoting when running in the VM

[bot+bpf-ci]

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

> diff --git a/tools/testing/selftests/bpf/vmtest.sh b/tools/testing/selftests/bpf/vmtest.sh
> index 9ca8022853933..6a3d026d76bd6 100755
> --- a/tools/testing/selftests/bpf/vmtest.sh
> +++ b/tools/testing/selftests/bpf/vmtest.sh
> @@ -428,8 +428,17 @@ main()
>
>  	if [[ $# -eq 0  && "${debug_shell}" == "no" ]]; then
>  		echo "No command specified, will run ${DEFAULT_COMMAND} in the vm"
> -	else
> -		command="$@"
> +	elif [[ $# -gt 0 ]]; then
> +		# Quote each argument so the command survives into the guest: the
> +		# host expands ${command} into the generated init script, which
> +		# the guest bash then parses as shell source. Without the %q
> +		# escapes an argument with a space or a glob (e.g. -a 'verifier_*')
> +		# is re-split and expanded against /root/bpf there.
> +		#
> +		# Skip this when there is no command: printf '%q ' would still
> +		# apply the format once and emit '', which is not the empty
> +		# command that -s (debug shell) expects.
> +		command=$(printf '%q ' "$@")
>  	fi
>
>  	local kconfig_file="${OUTPUT_DIR}/latest.config"

The commit message states:

>     Note this makes the trailing command strictly an argv rather than a shell
>     snippet: passing it pre-quoted as one word, e.g.
>
>             vmtest.sh -- "./test_progs -t foo"
>
>     no longer works, and neither does embedding guest-side shell syntax such
>     as ';' or a redirection. Neither form is documented - usage() and
>     README.rst both show the command unquoted - and 'sh -c ...' still works.

Is the claim about README.rst accurate? Looking at
tools/testing/selftests/bpf/README.rst lines 106-115, there is a
documented vmtest.sh invocation whose trailing command is explicitly
quoted:

  $ PLATFORM=riscv64 CROSS_COMPILE=riscv64-linux-gnu- \
    tools/testing/selftests/bpf/vmtest.sh \
    -l <path of local rootfs image> -- \
    ./test_progs -d \
        \"$(cat tools/testing/selftests/bpf/DENYLIST.riscv64 | ... | tr -s '\n' ',' )\"

The `\"` are escaped in the user's host shell, so vmtest.sh's main()
receives argv[3] as the literal 10-character string `",exceptions,"`
including the double quotes.

Under the old `command="$@"`, the guest bash's second parse of the init
script performed quote removal on those literal quotes, so test_progs
received `-d ,exceptions,`. That is exactly what the `\"` in the recipe
are for: they survive the host and are consumed by the guest.

With `command=$(printf '%q ' "$@")` the quotes are escaped
(`\",exceptions,\"`) and reach the binary verbatim, so test_progs now
receives `-d ",exceptions,"`.

Downstream that argument goes to parse_arg() ARG_TEST_NAME_GLOB_DENYLIST
-> parse_test_list() (tools/testing/selftests/bpf/testing_helpers.c),
which strtok_r()s on ',' and hands each field to insert_test() with
is_glob_pattern=true. So the filter list becomes three globs instead of
one: ["] [exceptions] ["], two of which can never match a test name in
glob_match() (test_progs.c).

For DENYLIST.riscv64 as it stands today the damage is confined to those
two junk filters, because the file's first line is a comment and
therefore reduces to an empty leading field.

The same recipe applied to a denylist file whose first line is a real
entry silently drops that entry. With DENYLIST.asan-shaped input:

  OLD guest -> test_progs -d [*arena*,task_local_data,uprobe_multi_test,]
    filters: [*arena*] [task_local_data] [uprobe_multi_test]
  NEW guest -> test_progs -d ["*arena*,task_local_data,uprobe_multi_test,"]
    filters: ["*arena*] [task_local_data] [uprobe_multi_test] ["]

`"*arena*` matches nothing, so the *arena* tests are no longer denied
and run anyway, a silent loss of denylist coverage rather than an error.

Should the commit message be corrected to acknowledge that README.rst
does document a quoted invocation that breaks with this change, and
should README.rst lines 109-115 be updated in the same patch to remove
the now-harmful `\"`?


---
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/30836252778

Re: [bpf-next 3/4] selftests/bpf: report failed subtest count in test_progs summary

[Vineet Gupta]

On 8/3/26 11:31 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
>> --- a/tools/testing/selftests/bpf/test_progs.c
>> +++ b/tools/testing/selftests/bpf/test_progs.c
> [ ... ]
>
>> @@ -1726,12 +1731,12 @@
>>   	fclose(env->json);
>>
>>   if (env->not_built_cnt)
>> -	printf("Summary: %d/%d PASSED, %d SKIPPED (%d not built), %d FAILED\n",
>> +	printf("Summary: %d/%d PASSED, %d SKIPPED (%d not built), %d/%d FAILED\n",
>>   	       succ_cnt, sub_succ_cnt, skip_cnt, env->not_built_cnt,
>> -	       fail_cnt);
>> +	       fail_cnt, sub_fail_cnt);
>>   else
>> -	printf("Summary: %d/%d PASSED, %d SKIPPED, %d FAILED\n",
>> -	       succ_cnt, sub_succ_cnt, skip_cnt, fail_cnt);
>> +	printf("Summary: %d/%d PASSED, %d SKIPPED, %d/%d FAILED\n",
>> +	       succ_cnt, sub_succ_cnt, skip_cnt, fail_cnt, sub_fail_cnt);
> A subsystem pattern flags this as potentially concerning: the BPF selftests
> guide asks that test binaries emit TAP-compliant output via kselftest.h
> (ksft_set_plan()/ksft_finished()) rather than hand-formatting their own
> summary lines with printf(), so that kselftest runners and CI can parse the
> results uniformly.
>
> test_progs.c has always had its own reporting framework and does not use
> kselftest.h; this predates the patch by many years and converting it is far
> outside the scope of this change. The patch only widens one existing field on
> an already ad hoc line.
>
> The changelog acknowledges that the field-format change means "anything
> scraping 'N FAILED' out of it needs updating." A search over .github/workflows,
> .github/scripts, ci/, tools/testing/selftests/bpf/*.sh and Documentation/
> turned up no in-tree parser of the test_progs summary line.
>
> Does this output format remain compatible with the consumers you're aware of?

As the Changelog mentions, I don't know of any existing in-tree 
consumers/parser of the existing info.

Thx,
-Vineet

>
>
> ---
> 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/30836252778

Re: [bpf-next 2/4] selftests/bpf: add --no-error-summary to skip end-of-run error log dump

[Vineet Gupta]

On 8/3/26 11:15 AM, bot+bpf-ci@kernel.org wrote:
>> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
>> index aa06bab309665..8ee46745e2e85 100644
>> --- a/tools/testing/selftests/bpf/test_progs.c
>> +++ b/tools/testing/selftests/bpf/test_progs.c
> [ ... ]
>
>> @@ -1686,9 +1696,12 @@ static void calculate_summary_and_print_errors(struct test_env *env)
>>   	 * We only print error logs summary when there are failed tests and
>>   	 * verbose mode is not enabled. Otherwise, results may be inconsistent.
>>   	 *
>> +	 * --no-error-summary only elides the human readable dump: the walk
>> +	 * still happens so the JSON report keeps its per-test results.
>>   	 */
>> -	if (!verbose() && fail_cnt) {
>> -		printf("\nAll error logs:\n");
>> +	if (!verbose() && fail_cnt && (env->error_summary || w)) {
>> +		if (env->error_summary)
>> +			printf("\nAll error logs:\n");
> Does the comment match what the code does?  The comment says "the walk
> still happens", but the guard now requires either env->error_summary or w
> to be non-NULL.
>
> When --no-error-summary is passed without --json-summary, both
> env->error_summary and w are false/NULL, so the entire for loop at the
> bottom of this function gets skipped, not just the human readable output.
>
> The walk does happen with --no-error-summary --json-summary FILE (where
> w != NULL), but not in the no-JSON case. Could the comment be more
> specific, maybe "the walk still happens when a JSON report was requested,
> so the JSON report keeps its per-test results"?

Good catch indeed the comment overstate it. Will reword in v2.

Thx,
-Vineet

> ---
> 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/30836252778

Re: [bpf-next 4/4] selftests/bpf: vmtest.sh: preserve command quoting when running in the VM

[Vineet Gupta]

On 8/3/26 11:31 AM, bot+bpf-ci@kernel.org wrote:
>>      Note this makes the trailing command strictly an argv rather than a shell
>>      snippet: passing it pre-quoted as one word, e.g.
>>
>>              vmtest.sh -- "./test_progs -t foo"
>>
>>      no longer works, and neither does embedding guest-side shell syntax such
>>      as ';' or a redirection. Neither form is documented - usage() and
>>      README.rst both show the command unquoted - and 'sh -c ...' still works.
> Is the claim about README.rst accurate?

You are right, it is not.

>   Looking at
> tools/testing/selftests/bpf/README.rst lines 106-115, there is a
> documented vmtest.sh invocation whose trailing command is explicitly
> quoted:
>
>    $ PLATFORM=riscv64 CROSS_COMPILE=riscv64-linux-gnu- \
>      tools/testing/selftests/bpf/vmtest.sh \
>      -l <path of local rootfs image> -- \
>      ./test_progs -d \
>          \"$(cat tools/testing/selftests/bpf/DENYLIST.riscv64 | ... | tr -s '\n' ',' )\"
>
> The `\"` are escaped in the user's host shell, so vmtest.sh's main()
> receives argv[3] as the literal 10-character string `",exceptions,"`
> including the double quotes.
>
> Under the old `command="$@"`, the guest bash's second parse of the init
> script performed quote removal on those literal quotes, so test_progs
> received `-d ,exceptions,`. That is exactly what the `\"` in the recipe
> are for: they survive the host and are consumed by the guest.
>
> With `command=$(printf '%q ' "$@")` the quotes are escaped
> (`\",exceptions,\"`) and reach the binary verbatim, so test_progs now
> receives `-d ",exceptions,"`.
>
> Downstream that argument goes to parse_arg() ARG_TEST_NAME_GLOB_DENYLIST
> -> parse_test_list() (tools/testing/selftests/bpf/testing_helpers.c),
> which strtok_r()s on ',' and hands each field to insert_test() with
> is_glob_pattern=true. So the filter list becomes three globs instead of
> one: ["] [exceptions] ["], two of which can never match a test name in
> glob_match() (test_progs.c).
>
> For DENYLIST.riscv64 as it stands today the damage is confined to those
> two junk filters, because the file's first line is a comment and
> therefore reduces to an empty leading field.
>
> The same recipe applied to a denylist file whose first line is a real
> entry silently drops that entry. With DENYLIST.asan-shaped input:
>
>    OLD guest -> test_progs -d [*arena*,task_local_data,uprobe_multi_test,]
>      filters: [*arena*] [task_local_data] [uprobe_multi_test]
>    NEW guest -> test_progs -d ["*arena*,task_local_data,uprobe_multi_test,"]
>      filters: ["*arena*] [task_local_data] [uprobe_multi_test] ["]
>
> `"*arena*` matches nothing, so the *arena* tests are no longer denied
> and run anyway, a silent loss of denylist coverage rather than an error.

This is a really good catch. Indeed arena never matches.

> Should the commit message be corrected to acknowledge that README.rst
> does document a quoted invocation that breaks with this change, and
> should README.rst lines 109-115 be updated in the same patch to remove
> the now-harmful `\"`?

v2 drops the backslash, fixes README.rst and also updates the changelog 
accordingly.

Thx,
-Vineet

> ---
> 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/30836252778