[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

[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

Re: [bpf-next 0/4] selftest related fixes

[Yonghong Song]

On 8/3/26 10:02 AM, Vineet Gupta wrote:
> Hi,
>
> This is a set of fixes accumulated during BPF_GCC testing and an in-works
> verifier enhacement series for 32-bit tracking.

Similar to other patch sets, let us have the proper format for the subject
for example:
     [PATCH bpf-next 0/4] selftests/bpf: Fix a few selftest issues

For each individual patch, do
     [PATCH bpf-next 1/4] selftests/bpf: ...
     ...

>
> 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(-)
>