[PATCH bpf-next v2 0/4] selftest related fixes

[PATCH bpf-next v2 0/4] selftest related fixes

[Vineet Gupta]

Hi,

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

Changes since v1:

 - 1/4: reworked. Instead of forcing BPF_STX with barrier_var(), select
   the expected message with #ifdef __BPF_FEATURE_ST, per Yonghong. The
   changelog now spells out the two tradeoffs: on a BPF_ST toolchain the
   test no longer reaches map_kptr_match_type(), and __BPF_FEATURE_ST
   means "can emit BPF_ST", not "will" - clang -mcpu=v4 -O0 still emits
   BPF_STX, so the #ifdef is only correct because selftests build at -O2.

 - 2/4: reword the comment above the error-summary walk. With
   --no-error-summary and no --json-summary the loop is skipped
   entirely, not just the printing, which the old wording obscured.
   [bpf-ci]

 - 4/4: the RV64 recipe in README.rst wraps the denylist in \" and
   relies on the guest's second parse of the init script to remove
   them, so %q broke it - the quotes reached test_progs and
   parse_test_list() split them into junk filters. Drop the
   backslashes so the host consumes the quotes, and correct the
   changelog, which wrongly claimed README.rst shows the command
   unquoted. [bpf-ci]

 - 3/4: unchanged.

v1: https://lore.kernel.org/bpf/20260803170251.1898102-1-vineet.gupta@linux.dev

Thx,
-Vineet

Vineet Gupta (4):
  selftests/bpf: map_kptr: expect BPF_ST reject msg on cpuv4 toolchains
  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

 tools/testing/selftests/bpf/README.rst        |  4 +-
 .../selftests/bpf/progs/map_kptr_fail.c       |  9 +++
 tools/testing/selftests/bpf/test_progs.c      | 64 +++++++++++++------
 tools/testing/selftests/bpf/test_progs.h      |  3 +-
 tools/testing/selftests/bpf/vmtest.sh         | 13 +++-
 5 files changed, 67 insertions(+), 26 deletions(-)

-- 
2.53.0-Meta

[PATCH bpf-next v2 1/4] selftests/bpf: map_kptr: expect BPF_ST reject msg on cpuv4 toolchains

[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

These go through different rejection paths and output different
messages.
 - BPF_STX goes through map_kptr_match_type(), which prints
   "invalid kptr access, R...".
 - BPF_ST only gets the immediate check printing
   "BPF_ST imm must be 0 when storing to kptr"

The test only expects the BPF_STX message, so it fails on a toolchain
that folds the constant - bpf-gcc, and clang -mcpu=v4:

  7: (7a) *(u64 *)(r0 +8) = 195936478
  BPF_ST imm must be 0 when storing to kptr at off=8
  ...
  EXPECTED   SUBSTR: 'invalid kptr access, R'

Pick the expected message with __BPF_FEATURE_ST, which clang and bpf-gcc
both define exactly when BPF_ST codegen is available - cpuv4 for clang,
and by default for bpf-gcc, whose default cpu is v4.

  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

Two caveats worth noting:

- On a BPF_ST toolchain the test now only exercises the imm != 0 check
  and never reaches map_kptr_match_type(), so the scalar-vs-PTR_TO_BTF_ID
  rejection the test is named for is only covered by the non-ST builds.
  The imm path itself is already covered compiler-independently by
  verifier/map_kptr.c ("map_kptr: BPF_ST imm != 0").

- __BPF_FEATURE_ST says the compiler *can* emit BPF_ST, not that it will.
  The encoding also depends on the optimization level: clang -mcpu=v4 -O0
  still emits BPF_STX, which would send the #ifdef down the wrong branch
  and fail the test. Selftests always build BPF objects at -O2 so this
  does not bite today, but it is a latent failure mode if that changes.

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

diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
index f11848dfa78f..5e25ca806060 100644
--- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
+++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
@@ -386,7 +386,16 @@ int kptr_xchg_possibly_null(struct __sk_buff *ctx)
 }
 
 SEC("?tc")
+/*
+ * A compiler with BPF_ST folds the constant into a store-immediate, which the
+ * verifier rejects on a different path (and with a different message) than the
+ * BPF_STX form.
+ */
+#ifdef __BPF_FEATURE_ST
+__failure __msg("BPF_ST imm must be 0 when storing to kptr at off=8")
+#else
 __failure __msg("invalid kptr access, R")
+#endif
 int reject_scalar_store_to_kptr(struct __sk_buff *ctx)
 {
 	struct map_value *v;
-- 
2.53.0-Meta

Re: [PATCH bpf-next v2 1/4] selftests/bpf: map_kptr: expect BPF_ST reject msg on cpuv4 toolchains

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The patch conditionally alters the expected verifier error message for BPF_ST toolchains, which eliminates the test's intended coverage (verifying that `map_kptr_match_type()` rejects scalar registers) on modern compilers and introduces brittleness tied to compiler optimization levels. As acknowledged in the commit message, the test now bypasses the original validation path, and changes in optimization (e.g., compiling at -O0) will break the `#ifdef` logic because the compiler may still emit `BPF_STX`. A more robust approach is to force the compiler to emit `BPF_STX` (e.g., by using a 64-bit constant that cannot be encoded as a 32-bit immediate, or by using `barrier_var()`), which preserves the intended verifier coverage without relying on fragile macro checks.
--

--- Patch [1]: [PATCH bpf-next v2 1/4] selftests/bpf: map_kptr: expect BPF_ST reject msg on cpuv4 toolchains ---
commit 302e7847f20b43d2cf543bac72a29e4a57633dfa
Author: Vineet Gupta <vineet.gupta@linux.dev>

selftests/bpf: map_kptr: expect BPF_ST reject msg on cpuv4 toolchains

This commit modifies the reject_scalar_store_to_kptr test to expect a
different verifier error message on toolchains supporting BPF_ST. This
avoids a test failure when the compiler folds the constant into a
BPF_ST store immediate instruction instead of using BPF_STX.

> diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> index f11848dfa78f9..5e25ca806060c 100644
> --- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> +++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> @@ -386,7 +386,16 @@ int kptr_xchg_possibly_null(struct __sk_buff *ctx)
>  }
>  
>  SEC("?tc")
> +/*
> + * A compiler with BPF_ST folds the constant into a store-immediate, which the
> + * verifier rejects on a different path (and with a different message) than the
> + * BPF_STX form.
> + */
> +#ifdef __BPF_FEATURE_ST
> +__failure __msg("BPF_ST imm must be 0 when storing to kptr at off=8")
> +#else
>  __failure __msg("invalid kptr access, R")
> +#endif

[Severity: Medium]
Does this conditionally altered expectation eliminate the test's intended
coverage on modern toolchains?

As noted in the commit message, the test now bypasses the original validation
path on BPF_ST builds, meaning it no longer verifies that
map_kptr_match_type() rejects scalar registers.

This also appears to introduce brittleness. If the optimization level changes
(such as compiling at -O0), the compiler might still emit BPF_STX, which would
evaluate the wrong macro branch and fail the test.

Would it be more robust to force the compiler to emit BPF_STX here, for
example by using a 64-bit constant that cannot be encoded as a 32-bit
immediate, or by using barrier_var()? This could preserve the intended
verifier coverage across different toolchains and optimization levels.

>  int reject_scalar_store_to_kptr(struct __sk_buff *ctx)
>  {
>  	struct map_value *v;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260807204434.1036279-1-vineet.gupta@linux.dev?part=1

Re: [PATCH bpf-next v2 1/4] selftests/bpf: map_kptr: expect BPF_ST reject msg on cpuv4 toolchains

[Vineet Gupta]

On 8/7/26 1:53 PM, sashiko-bot@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The patch conditionally alters the expected verifier error message for BPF_ST toolchains, which eliminates the test's intended coverage (verifying that `map_kptr_match_type()` rejects scalar registers) on modern compilers and introduces brittleness tied to compiler optimization levels. As acknowledged in the commit message, the test now bypasses the original validation path, and changes in optimization (e.g., compiling at -O0) will break the `#ifdef` logic because the compiler may still emit `BPF_STX`. A more robust approach is to force the compiler to emit `BPF_STX` (e.g., by using a 64-bit constant that cannot be encoded as a 32-bit immediate, or by using `barrier_var()`), which preserves the intended verifier coverage without relying on fragile macro checks.
> --
>
> --- Patch [1]: [PATCH bpf-next v2 1/4] selftests/bpf: map_kptr: expect BPF_ST reject msg on cpuv4 toolchains ---
> commit 302e7847f20b43d2cf543bac72a29e4a57633dfa
> Author: Vineet Gupta <vineet.gupta@linux.dev>
>
> selftests/bpf: map_kptr: expect BPF_ST reject msg on cpuv4 toolchains
>
> This commit modifies the reject_scalar_store_to_kptr test to expect a
> different verifier error message on toolchains supporting BPF_ST. This
> avoids a test failure when the compiler folds the constant into a
> BPF_ST store immediate instruction instead of using BPF_STX.
>
>> diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
>> index f11848dfa78f9..5e25ca806060c 100644
>> --- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
>> +++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
>> @@ -386,7 +386,16 @@ int kptr_xchg_possibly_null(struct __sk_buff *ctx)
>>   }
>>   
>>   SEC("?tc")
>> +/*
>> + * A compiler with BPF_ST folds the constant into a store-immediate, which the
>> + * verifier rejects on a different path (and with a different message) than the
>> + * BPF_STX form.
>> + */
>> +#ifdef __BPF_FEATURE_ST
>> +__failure __msg("BPF_ST imm must be 0 when storing to kptr at off=8")
>> +#else
>>   __failure __msg("invalid kptr access, R")
>> +#endif
> [Severity: Medium]
> Does this conditionally altered expectation eliminate the test's intended
> coverage on modern toolchains?
>
> As noted in the commit message, the test now bypasses the original validation
> path on BPF_ST builds, meaning it no longer verifies that
> map_kptr_match_type() rejects scalar registers.
>
> This also appears to introduce brittleness. If the optimization level changes
> (such as compiling at -O0), the compiler might still emit BPF_STX, which would
> evaluate the wrong macro branch and fail the test.
>
> Would it be more robust to force the compiler to emit BPF_STX here, for
> example by using a 64-bit constant that cannot be encoded as a 32-bit
> immediate, or by using barrier_var()? This could preserve the intended
> verifier coverage across different toolchains and optimization levels.

That was exactly in v1 and reviewers suggested this approach !

Thx,
-Vineet

Re: [PATCH bpf-next v2 1/4] selftests/bpf: map_kptr: expect BPF_ST reject msg on cpuv4 toolchains

[Yonghong Song]

On 8/7/26 1:44 PM, Vineet Gupta wrote:
> 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
>
> These go through different rejection paths and output different
> messages.
>   - BPF_STX goes through map_kptr_match_type(), which prints
>     "invalid kptr access, R...".
>   - BPF_ST only gets the immediate check printing
>     "BPF_ST imm must be 0 when storing to kptr"
>
> The test only expects the BPF_STX message, so it fails on a toolchain
> that folds the constant - bpf-gcc, and clang -mcpu=v4:
>
>    7: (7a) *(u64 *)(r0 +8) = 195936478
>    BPF_ST imm must be 0 when storing to kptr at off=8
>    ...
>    EXPECTED   SUBSTR: 'invalid kptr access, R'
>
> Pick the expected message with __BPF_FEATURE_ST, which clang and bpf-gcc
> both define exactly when BPF_ST codegen is available - cpuv4 for clang,
> and by default for bpf-gcc, whose default cpu is v4.
>
>    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
>
> Two caveats worth noting:
>
> - On a BPF_ST toolchain the test now only exercises the imm != 0 check
>    and never reaches map_kptr_match_type(), so the scalar-vs-PTR_TO_BTF_ID
>    rejection the test is named for is only covered by the non-ST builds.
>    The imm path itself is already covered compiler-independently by
>    verifier/map_kptr.c ("map_kptr: BPF_ST imm != 0").
>
> - __BPF_FEATURE_ST says the compiler *can* emit BPF_ST, not that it will.
>    The encoding also depends on the optimization level: clang -mcpu=v4 -O0
>    still emits BPF_STX, which would send the #ifdef down the wrong branch
>    and fail the test. Selftests always build BPF objects at -O2 so this
>    does not bite today, but it is a latent failure mode if that changes.
>
> Signed-off-by: Vineet Gupta <vineet.gupta@linux.dev>

Acked-by: Yonghong Song <yonghong.song@linux.dev>

> ---
>   tools/testing/selftests/bpf/progs/map_kptr_fail.c | 9 +++++++++
>   1 file changed, 9 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/progs/map_kptr_fail.c b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> index f11848dfa78f..5e25ca806060 100644
> --- a/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> +++ b/tools/testing/selftests/bpf/progs/map_kptr_fail.c
> @@ -386,7 +386,16 @@ int kptr_xchg_possibly_null(struct __sk_buff *ctx)
>   }
>   
>   SEC("?tc")
> +/*
> + * A compiler with BPF_ST folds the constant into a store-immediate, which the
> + * verifier rejects on a different path (and with a different message) than the
> + * BPF_STX form.
> + */
> +#ifdef __BPF_FEATURE_ST
> +__failure __msg("BPF_ST imm must be 0 when storing to kptr at off=8")
> +#else
>   __failure __msg("invalid kptr access, R")
> +#endif
>   int reject_scalar_store_to_kptr(struct __sk_buff *ctx)
>   {
>   	struct map_value *v;

[PATCH bpf-next v2 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 | 43 +++++++++++++++++-------
 tools/testing/selftests/bpf/test_progs.h |  1 +
 2 files changed, 31 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
index aa06bab30966..b274d98faac4 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,14 @@ 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 elides the human readable dump. The walk still
+	 * happens when a JSON report was requested, so the JSON output keeps
+	 * its per-test results; with no JSON report there is nothing left to
+	 * do and the whole loop is skipped.
 	 */
-	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 +1713,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 +2044,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.53.0-Meta

Re: [PATCH bpf-next v2 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: 1813 bytes --]

> diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> index aa06bab30966..b274d98faac4 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)
>  {

[ ... ]

> @@ -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);
>  }

This isn't a bug, but with the new @quiet parameter, dump_test_log()
now takes three consecutive bool parameters (skip_ok_subtests,
par_exec_result, quiet), so the call sites read as a row of anonymous
literals: false, false, false. The other call sites have similar
patterns:

In dump_crash_log():
    dump_test_log(env.test, env.test_state, true, false, false, NULL);

In dispatch_thread():
    dump_test_log(test, state, false, true, false, NULL);

In calculate_summary_and_print_errors():
    dump_test_log(test, state, true, true, !env->error_summary, w);

Would a small flags parameter (or naming the arguments at the call
sites) make these invocations more readable?


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

[PATCH bpf-next v2 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 b274d98faac4..46eb201b96a3 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);
 	}
@@ -1728,12 +1733,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.53.0-Meta

[PATCH bpf-next v2 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. 'sh -c ...' still works.

The RV64 recipe in README.rst does depend on the old double parse: it
wraps the denylist in \" so the literal quotes reach the guest, whose
second parse of the init script removes them. Under %q those quotes now
survive into argv, and parse_test_list() strtok_r()s on ',' turns them
into junk filters:

        -d ",exceptions,"  ->  ["] [exceptions] ["]

That is harmless for DENYLIST.riscv64 only because its first line is a
comment, so the leading field is empty. A denylist starting with a real
entry would silently lose it - ["*arena*] never matches - so drop the
backslashes and let the host consume the quotes instead.

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

diff --git a/tools/testing/selftests/bpf/README.rst b/tools/testing/selftests/bpf/README.rst
index 37164322a102..07c834433b38 100644
--- a/tools/testing/selftests/bpf/README.rst
+++ b/tools/testing/selftests/bpf/README.rst
@@ -107,12 +107,12 @@ Docker container and local rootfs image. The overall steps are as follows:
     tools/testing/selftests/bpf/vmtest.sh \
     -l <path of local rootfs image> -- \
     ./test_progs -d \
-        \"$(cat tools/testing/selftests/bpf/DENYLIST.riscv64 \
+        "$(cat tools/testing/selftests/bpf/DENYLIST.riscv64 \
             | cut -d'#' -f1 \
             | sed -e 's/^[[:space:]]*//' \
                   -e 's/[[:space:]]*$//' \
             | tr -s '\n' ',' \
-        )\"
+        )"
 
 Link: https://github.com/pulehui/riscv-bpf-vmtest.git [0]
 Link: https://github.com/libbpf/ci/blob/main/rootfs/mkrootfs_debian.sh [1]
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.53.0-Meta