BPF List
 help / color / mirror / Atom feed
* [PATCH bpf-next v2 0/3] Improve stack depth verification stats output
@ 2026-08-02 22:52 Kumar Kartikeya Dwivedi
  2026-08-02 22:52 ` [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats Kumar Kartikeya Dwivedi
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-02 22:52 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, Emil Tsalapatis, kkd, kernel-team

Some improvements for more clarity in the stack depth verification
statistics output. See commit logs for details.

Changelog:
----------
v1 -> v2
v1: https://lore.kernel.org/bpf/20260801230400.850271-1-memxor@gmail.com

 * Use multi-line format. (Eduard)
 * Adjust veristat to work with old and new format.
 * Adjust selftest log_level without new option. (Eduard)

Kumar Kartikeya Dwivedi (3):
  bpf: Show more useful info in stack depth stats
  selftests/bpf: Adjust veristat stack depth parsing
  selftests/bpf: Test stack depth stats without BTF subprog names

 kernel/bpf/verifier.c                         | 13 +++++++----
 .../bpf/progs/verifier_basic_stack.c          |  4 ++--
 .../bpf/progs/verifier_bpf_fastcall.c         | 23 ++++++++++++-------
 .../bpf/progs/verifier_private_stack.c        | 15 +++++++++---
 .../selftests/bpf/progs/verifier_var_off.c    |  4 ++--
 tools/testing/selftests/bpf/test_verifier.c   |  2 +-
 tools/testing/selftests/bpf/verifier/calls.c  |  6 ++++-
 tools/testing/selftests/bpf/veristat.c        | 16 +++++++++----
 8 files changed, 58 insertions(+), 25 deletions(-)


base-commit: 8f876c773b79ed66dadc9507f50dbff39c6a9ec7
-- 
2.53.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats
  2026-08-02 22:52 [PATCH bpf-next v2 0/3] Improve stack depth verification stats output Kumar Kartikeya Dwivedi
@ 2026-08-02 22:52 ` Kumar Kartikeya Dwivedi
  2026-08-02 23:04   ` sashiko-bot
  2026-08-03  0:21   ` bot+bpf-ci
  2026-08-02 22:52 ` [PATCH bpf-next v2 2/3] selftests/bpf: Adjust veristat stack depth parsing Kumar Kartikeya Dwivedi
  2026-08-02 22:52 ` [PATCH bpf-next v2 3/3] selftests/bpf: Test stack depth stats without BTF subprog names Kumar Kartikeya Dwivedi
  2 siblings, 2 replies; 8+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-02 22:52 UTC (permalink / raw)
  To: bpf
  Cc: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann,
	Eduard Zingerman, Emil Tsalapatis, kkd, kernel-team

Currently, stack depth statistics are too crude, listing captured depths in
subprogram-number order. Since libbpf determines those numbers, it is hard to
associate each depth with its subprogram name.

Print the maximum stack depth and each subprogram on separate lines:

stack depth max <depth>
stack depth subprog <number> <name> <depth>

When no subprogram name is available, print <unknown>.

Suggested-by: Andrii Nakryiko <andrii@kernel.org>
Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 kernel/bpf/verifier.c                         | 13 +++++++----
 .../bpf/progs/verifier_basic_stack.c          |  4 ++--
 .../bpf/progs/verifier_bpf_fastcall.c         | 23 ++++++++++++-------
 .../bpf/progs/verifier_private_stack.c        | 15 +++++++++---
 .../selftests/bpf/progs/verifier_var_off.c    |  4 ++--
 5 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index b274004fccfd..3b61897ed0d2 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -18777,10 +18777,15 @@ static void print_verification_stats(struct bpf_verifier_env *env)
 	if (env->log.level & BPF_LOG_STATS) {
 		verbose(env, "verification time %lld usec\n",
 			div_u64(env->verification_time, 1000));
-		verbose(env, "stack depth %d", env->subprog_info[0].stack_depth);
-		for (i = 1; i < subprog_cnt; i++)
-			verbose(env, "+%d", env->subprog_info[i].stack_depth);
-		verbose(env, " max %d\n", env->max_stack_depth);
+		verbose(env, "stack depth max %d\n", env->max_stack_depth);
+		for (i = 0; i < subprog_cnt; i++) {
+			const char *name = env->subprog_info[i].name;
+
+			if (!name || !name[0])
+				name = "<unknown>";
+			verbose(env, "stack depth subprog %d %s %d\n", i, name,
+				env->subprog_info[i].stack_depth);
+		}
 		verbose(env, "insns processed %d", env->subprog_info[0].insn_processed);
 		for (i = 1; i < subprog_cnt; i++)
 			if (bpf_subprog_is_global(env, i))
diff --git a/tools/testing/selftests/bpf/progs/verifier_basic_stack.c b/tools/testing/selftests/bpf/progs/verifier_basic_stack.c
index fb62e09f2114..61ec8790fd21 100644
--- a/tools/testing/selftests/bpf/progs/verifier_basic_stack.c
+++ b/tools/testing/selftests/bpf/progs/verifier_basic_stack.c
@@ -27,7 +27,7 @@ __naked void stack_out_of_bounds(void)
 
 SEC("socket")
 __description("uninitialized stack1")
-__success __log_level(4) __msg("stack depth 8")
+__success __log_level(4) __msg("stack depth subprog 0 uninitialized_stack1 8")
 __failure_unpriv __msg_unpriv("invalid read from stack")
 __naked void uninitialized_stack1(void)
 {
@@ -45,7 +45,7 @@ __naked void uninitialized_stack1(void)
 
 SEC("socket")
 __description("uninitialized stack2")
-__success __log_level(4) __msg("stack depth 8")
+__success __log_level(4) __msg("stack depth subprog 0 uninitialized_stack2 8")
 __failure_unpriv __msg_unpriv("invalid read from stack")
 __naked void uninitialized_stack2(void)
 {
diff --git a/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c b/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c
index 83707faea049..cb36866bfed3 100644
--- a/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c
+++ b/tools/testing/selftests/bpf/progs/verifier_bpf_fastcall.c
@@ -10,7 +10,7 @@
 
 SEC("raw_tp")
 __arch_x86_64
-__log_level(4) __msg("stack depth 8")
+__log_level(4) __msg("stack depth subprog 0 simple 8")
 __xlated("4: r5 = 5")
 __xlated("5: r0 = ")
 __xlated("6: r0 = &(void __percpu *)(r0)")
@@ -96,7 +96,7 @@ __naked void canary_zero_spills(void)
 
 SEC("raw_tp")
 __arch_x86_64
-__log_level(4) __msg("stack depth 16")
+__log_level(4) __msg("stack depth subprog 0 wrong_reg_in_pattern1 16")
 __xlated("1: *(u64 *)(r10 -16) = r1")
 __xlated("...")
 __xlated("3: r0 = &(void __percpu *)(r0)")
@@ -598,7 +598,7 @@ __naked static void subprogs_use_independent_offsets_aux(void)
 
 SEC("raw_tp")
 __arch_x86_64
-__log_level(4) __msg("stack depth 8")
+__log_level(4) __msg("stack depth subprog 0 helper_call_does_not_prevent_bpf_fastcall 8")
 __xlated("2: r0 = &(void __percpu *)(r0)")
 __success
 __naked void helper_call_does_not_prevent_bpf_fastcall(void)
@@ -620,7 +620,7 @@ __naked void helper_call_does_not_prevent_bpf_fastcall(void)
 
 SEC("raw_tp")
 __arch_x86_64
-__log_level(4) __msg("stack depth 24")
+__log_level(4) __msg("stack depth subprog 0 may_goto_interaction_x86_64 24")
 /* may_goto counter at -24 */
 __xlated("0: *(u64 *)(r10 -24) =")
 /* may_goto timestamp at -16 */
@@ -661,7 +661,7 @@ __naked void may_goto_interaction_x86_64(void)
 SEC("raw_tp")
 __arch_arm64
 __arch_riscv64
-__log_level(4) __msg("stack depth 24")
+__log_level(4) __msg("stack depth subprog 0 may_goto_interaction 24")
 /* may_goto counter at -24 */
 __xlated("0: *(u64 *)(r10 -24) =")
 /* may_goto timestamp at -16 */
@@ -708,7 +708,9 @@ __naked static void dummy_loop_callback(void)
 
 SEC("raw_tp")
 __arch_x86_64
-__log_level(4) __msg("stack depth 32+0")
+__log_level(4)
+__msg("stack depth subprog 0 bpf_loop_interaction1 32")
+__msg("stack depth subprog 1 dummy_loop_callback 0")
 __xlated("2: r1 = 1")
 __xlated("3: r0 =")
 __xlated("4: r0 = &(void __percpu *)(r0)")
@@ -756,7 +758,9 @@ __naked int bpf_loop_interaction1(void)
 
 SEC("raw_tp")
 __arch_x86_64
-__log_level(4) __msg("stack depth 40+0")
+__log_level(4)
+__msg("stack depth subprog 0 bpf_loop_interaction2 40")
+__msg("stack depth subprog 1 dummy_loop_callback 0")
 /* call bpf_get_smp_processor_id */
 __xlated("2: r1 = 42")
 __xlated("3: r0 =")
@@ -800,7 +804,10 @@ __naked int bpf_loop_interaction2(void)
 
 SEC("raw_tp")
 __arch_x86_64
-__log_level(4) __msg("stack depth 512+0 max 512")
+__log_level(4)
+__msg("stack depth max 512")
+__msg("stack depth subprog 0 cumulative_stack_depth 512")
+__msg("stack depth subprog 1 cumulative_stack_depth_subprog 0")
 /* just to print xlated version when debugging */
 __xlated("r0 = &(void __percpu *)(r0)")
 __success
diff --git a/tools/testing/selftests/bpf/progs/verifier_private_stack.c b/tools/testing/selftests/bpf/progs/verifier_private_stack.c
index bb8206e10880..c818898836c1 100644
--- a/tools/testing/selftests/bpf/progs/verifier_private_stack.c
+++ b/tools/testing/selftests/bpf/progs/verifier_private_stack.c
@@ -86,7 +86,9 @@ __naked static void cumulative_stack_depth_subprog(void)
 SEC("kprobe")
 __description("Private stack, subtree > MAX_BPF_STACK")
 __success
-__log_level(4) __msg("stack depth 512+32 max 512")
+__log_level(4) __msg("stack depth max 512")
+__msg("stack depth subprog 0 private_stack_nested_1 512")
+__msg("stack depth subprog 1 cumulative_stack_depth_subprog 32")
 __arch_x86_64
 /* private stack fp for the main prog */
 __jited("	movabsq	$0x{{.*}}, %r9")
@@ -331,7 +333,11 @@ SEC("fentry/bpf_fentry_test9")
 __description("Private stack, async callback, potential nesting")
 __success __retval(0)
 __load_if_JITed()
-__log_level(4) __msg("stack depth 8+0+256+0 max 272")
+__log_level(4) __msg("stack depth max 272")
+__msg("stack depth subprog 0 private_stack_async_callback_2 8")
+__msg("stack depth subprog 1 timer_cb1 0")
+__msg("stack depth subprog 2 subprog1 256")
+__msg("stack depth subprog 3 subprog2 0")
 __arch_x86_64
 __jited("	subq	$0x100, %rsp")
 __arch_arm64
@@ -355,7 +361,10 @@ int private_stack_async_callback_2(void)
 SEC("fentry/bpf_fentry_test9")
 __description("private stack, max stack depth is private stack")
 __success
-__log_level(4) __msg("stack depth 8+256+0 max 256")
+__log_level(4) __msg("stack depth max 256")
+__msg("stack depth subprog 0 private_stack_max_depth 8")
+__msg("stack depth subprog 1 subprog1 256")
+__msg("stack depth subprog 2 subprog2 0")
 int private_stack_max_depth(void)
 {
 	int x = 0;
diff --git a/tools/testing/selftests/bpf/progs/verifier_var_off.c b/tools/testing/selftests/bpf/progs/verifier_var_off.c
index 24cd0a763673..df436bbc9ecf 100644
--- a/tools/testing/selftests/bpf/progs/verifier_var_off.c
+++ b/tools/testing/selftests/bpf/progs/verifier_var_off.c
@@ -198,7 +198,7 @@ __success
 /* Check that the maximum stack depth is correctly maintained according to the
  * maximum possible variable offset.
  */
-__log_level(4) __msg("stack depth 16")
+__log_level(4) __msg("stack depth subprog 0 stack_write_priv_vs_unpriv 16")
 __failure_unpriv
 /* Variable stack access is rejected for unprivileged.
  */
@@ -238,7 +238,7 @@ __success
 /* Check that the maximum stack depth is correctly maintained according to the
  * maximum possible variable offset.
  */
-__log_level(4) __msg("stack depth 16")
+__log_level(4) __msg("stack depth subprog 0 stack_write_followed_by_read 16")
 __failure_unpriv
 __msg_unpriv("R2 variable stack access prohibited for !root")
 __retval(0)
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH bpf-next v2 2/3] selftests/bpf: Adjust veristat stack depth parsing
  2026-08-02 22:52 [PATCH bpf-next v2 0/3] Improve stack depth verification stats output Kumar Kartikeya Dwivedi
  2026-08-02 22:52 ` [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats Kumar Kartikeya Dwivedi
@ 2026-08-02 22:52 ` Kumar Kartikeya Dwivedi
  2026-08-02 22:52 ` [PATCH bpf-next v2 3/3] selftests/bpf: Test stack depth stats without BTF subprog names Kumar Kartikeya Dwivedi
  2 siblings, 0 replies; 8+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-02 22:52 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, Emil Tsalapatis, kkd, kernel-team

The verifier now reports stack depth statistics using one record per line.
Teach veristat to parse the new maximum and per-subprogram records while
retaining support for the legacy one-line format used by older kernels.

Increase the bounded backward scan so it can include all 256 per-subprogram
records.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 tools/testing/selftests/bpf/veristat.c | 16 ++++++++++++----
 1 file changed, 12 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/veristat.c b/tools/testing/selftests/bpf/veristat.c
index c9c257784ee3..97397b97745d 100644
--- a/tools/testing/selftests/bpf/veristat.c
+++ b/tools/testing/selftests/bpf/veristat.c
@@ -993,13 +993,15 @@ static void free_verif_stats(struct verif_stats *stats, size_t stat_cnt)
 
 static char verif_log_buf[64 * 1024];
 
-#define MAX_PARSED_LOG_LINES 100
+/* Keep room for all 256 subprogram records and trailing statistics. */
+#define MAX_PARSED_LOG_LINES 300
 
 static int parse_verif_log(char * const buf, size_t buf_sz, struct verif_stats *s)
 {
 	const char *cur;
-	int pos, lines, sub_stack, cnt = 0;
-	char *state = NULL, *token, stack[512];
+	long sub_stack;
+	int pos, lines, cnt = 0;
+	char *state = NULL, *token, stack[512] = {};
 
 	buf[buf_sz - 1] = '\0';
 
@@ -1025,11 +1027,17 @@ static int parse_verif_log(char * const buf, size_t buf_sz, struct verif_stats *
 				&s->stats[MARK_READ_MAX_LEN]))
 			continue;
 
+		if (1 == sscanf(cur, "stack depth max %ld", &s->stats[MAX_STACK]))
+			continue;
+		if (1 == sscanf(cur, "stack depth subprog %*d %*s %ld", &sub_stack)) {
+			s->stats[STACK] += sub_stack;
+			continue;
+		}
 		if (2 == sscanf(cur, "stack depth %511s max %ld", stack, &s->stats[MAX_STACK]))
 			continue;
 	}
 	while ((token = strtok_r(cnt++ ? NULL : stack, "+", &state))) {
-		if (sscanf(token, "%d", &sub_stack) == 0)
+		if (sscanf(token, "%ld", &sub_stack) == 0)
 			break;
 		s->stats[STACK] += sub_stack;
 	}
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH bpf-next v2 3/3] selftests/bpf: Test stack depth stats without BTF subprog names
  2026-08-02 22:52 [PATCH bpf-next v2 0/3] Improve stack depth verification stats output Kumar Kartikeya Dwivedi
  2026-08-02 22:52 ` [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats Kumar Kartikeya Dwivedi
  2026-08-02 22:52 ` [PATCH bpf-next v2 2/3] selftests/bpf: Adjust veristat stack depth parsing Kumar Kartikeya Dwivedi
@ 2026-08-02 22:52 ` Kumar Kartikeya Dwivedi
  2 siblings, 0 replies; 8+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-02 22:52 UTC (permalink / raw)
  To: bpf
  Cc: Alexei Starovoitov, Andrii Nakryiko, Daniel Borkmann,
	Eduard Zingerman, Emil Tsalapatis, kkd, kernel-team

Test the stack depth statistics emitted when BTF function info does not
provide subprogram names.

Make VERBOSE_ACCEPT request verifier statistics so the raw-insn test can
validate the <unknown> output without a test-specific log level.

Signed-off-by: Kumar Kartikeya Dwivedi <memxor@gmail.com>
---
 tools/testing/selftests/bpf/test_verifier.c  | 2 +-
 tools/testing/selftests/bpf/verifier/calls.c | 6 +++++-
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c
index a8ae03c57bba..bffb7360434c 100644
--- a/tools/testing/selftests/bpf/test_verifier.c
+++ b/tools/testing/selftests/bpf/test_verifier.c
@@ -1560,7 +1560,7 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
 
 	opts.expected_attach_type = test->expected_attach_type;
 	if (expected_ret == VERBOSE_ACCEPT)
-		opts.log_level = 2;
+		opts.log_level = 2 | 4;
 	else if (verbose)
 		opts.log_level = verif_log_level | 4; /* force stats */
 	else
diff --git a/tools/testing/selftests/bpf/verifier/calls.c b/tools/testing/selftests/bpf/verifier/calls.c
index 8cd626e04551..14f6f260b88b 100644
--- a/tools/testing/selftests/bpf/verifier/calls.c
+++ b/tools/testing/selftests/bpf/verifier/calls.c
@@ -1091,7 +1091,11 @@
 	/* stack_main=32, stack_A=256, stack_B=64
 	 * and max(main+A, main+A+B) < 512
 	 */
-	.result = ACCEPT,
+	.result = VERBOSE_ACCEPT,
+	.errstr = "stack depth max 352\t"
+		  "stack depth subprog 0 <unknown> 32\t"
+		  "stack depth subprog 1 <unknown> 256\t"
+		  "stack depth subprog 2 <unknown> 64",
 },
 {
 	"calls: stack depth check using three frames. test2",
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats
  2026-08-02 22:52 ` [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats Kumar Kartikeya Dwivedi
@ 2026-08-02 23:04   ` sashiko-bot
  2026-08-02 23:09     ` Kumar Kartikeya Dwivedi
  2026-08-03  0:21   ` bot+bpf-ci
  1 sibling, 1 reply; 8+ messages in thread
From: sashiko-bot @ 2026-08-02 23:04 UTC (permalink / raw)
  To: Kumar Kartikeya Dwivedi; +Cc: bpf

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The commit changes the verifier's 'stack depth' log format from a single-line format (`stack depth %d+%d...+%d max %d`) to a multi-line format (`stack depth max %d` followed by per-subprogram lines). However, it fails to update the `veristat` tool in the BPF selftests, which relies on parsing the old format to collect stack depth statistics. When `veristat` attempts to parse the new verifier log, the hardcoded `sscanf` string (`"stack depth %511s max %ld"`) matches only the first word (`"max"`) but fails on the rest, leading to broken and unpopulated `STACK` and `MAX_STACK` metrics.
--

--- Patch [1]: [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats ---
commit f4a761d5dad941fea12558fab4093ec6ef3e52da
Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>

bpf: Show more useful info in stack depth stats

This commit updates the BPF verifier's stack depth statistics logging to print
the maximum stack depth and each subprogram's stack depth on separate lines.
It also updates various selftests to match the new output format.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index e6f35f4e715b6..74a92036d84e7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18703,10 +18703,15 @@ static void print_verification_stats(struct bpf_verifier_env *env)
>  	if (env->log.level & BPF_LOG_STATS) {
>  		verbose(env, "verification time %lld usec\n",
>  			div_u64(env->verification_time, 1000));
> -		verbose(env, "stack depth %d", env->subprog_info[0].stack_depth);
> -		for (i = 1; i < subprog_cnt; i++)
> -			verbose(env, "+%d", env->subprog_info[i].stack_depth);
> -		verbose(env, " max %d\n", env->max_stack_depth);
> +		verbose(env, "stack depth max %d\n", env->max_stack_depth);
> +		for (i = 0; i < subprog_cnt; i++) {
> +			const char *name = env->subprog_info[i].name;
> +
> +			if (!name || !name[0])
> +				name = "<unknown>";
> +			verbose(env, "stack depth subprog %d %s %d\n", i, name,
> +				env->subprog_info[i].stack_depth);
> +		}

[Severity: Medium]
Will this break the veristat tool's ability to collect stack depth statistics?

Looking at parse_verif_log() in tools/testing/selftests/bpf/veristat.c, the 
parser still expects the old single-line format:

    if (2 == sscanf(cur, "stack depth %511s max %ld", stack, &s->stats[MAX_STACK]))
        continue;
    
    while ((token = strtok_r(cnt++ ? NULL : stack, "+", &state))) {
        if (sscanf(token, "%d", &sub_stack) == 0)
            break;
        s->stats[STACK] += sub_stack;
    }

With the new multi-line format emitted here, it appears this parsing logic 
will silently fail to extract the stack depth numbers, leaving the STACK and 
MAX_STACK metrics unpopulated when comparing regressions. Does veristat.c 
need to be updated to match the new verifier output format?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260802225209.2511758-1-memxor@gmail.com?part=1

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats
  2026-08-02 23:04   ` sashiko-bot
@ 2026-08-02 23:09     ` Kumar Kartikeya Dwivedi
  0 siblings, 0 replies; 8+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-02 23:09 UTC (permalink / raw)
  To: sashiko-reviews, Roman Gushchin; +Cc: bpf

On Mon Aug 3, 2026 at 1:04 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
> - [Medium] The commit changes the verifier's 'stack depth' log format from a single-line format (`stack depth %d+%d...+%d max %d`) to a multi-line format (`stack depth max %d` followed by per-subprogram lines). However, it fails to update the `veristat` tool in the BPF selftests, which relies on parsing the old format to collect stack depth statistics. When `veristat` attempts to parse the new verifier log, the hardcoded `sscanf` string (`"stack depth %511s max %ld"`) matches only the first word (`"max"`) but fails on the rest, leading to broken and unpopulated `STACK` and `MAX_STACK` metrics.
> --
>
> --- Patch [1]: [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats ---
> commit f4a761d5dad941fea12558fab4093ec6ef3e52da
> Author: Kumar Kartikeya Dwivedi <memxor@gmail.com>
>
> bpf: Show more useful info in stack depth stats
>
> This commit updates the BPF verifier's stack depth statistics logging to print
> the maximum stack depth and each subprogram's stack depth on separate lines.
> It also updates various selftests to match the new output format.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index e6f35f4e715b6..74a92036d84e7 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -18703,10 +18703,15 @@ static void print_verification_stats(struct bpf_verifier_env *env)
>>  	if (env->log.level & BPF_LOG_STATS) {
>>  		verbose(env, "verification time %lld usec\n",
>>  			div_u64(env->verification_time, 1000));
>> -		verbose(env, "stack depth %d", env->subprog_info[0].stack_depth);
>> -		for (i = 1; i < subprog_cnt; i++)
>> -			verbose(env, "+%d", env->subprog_info[i].stack_depth);
>> -		verbose(env, " max %d\n", env->max_stack_depth);
>> +		verbose(env, "stack depth max %d\n", env->max_stack_depth);
>> +		for (i = 0; i < subprog_cnt; i++) {
>> +			const char *name = env->subprog_info[i].name;
>> +
>> +			if (!name || !name[0])
>> +				name = "<unknown>";
>> +			verbose(env, "stack depth subprog %d %s %d\n", i, name,
>> +				env->subprog_info[i].stack_depth);
>> +		}
>
> [Severity: Medium]
> Will this break the veristat tool's ability to collect stack depth statistics?
>
> Looking at parse_verif_log() in tools/testing/selftests/bpf/veristat.c, the
> parser still expects the old single-line format:
>
>     if (2 == sscanf(cur, "stack depth %511s max %ld", stack, &s->stats[MAX_STACK]))
>         continue;
>
>     while ((token = strtok_r(cnt++ ? NULL : stack, "+", &state))) {
>         if (sscanf(token, "%d", &sub_stack) == 0)
>             break;
>         s->stats[STACK] += sub_stack;
>     }
>
> With the new multi-line format emitted here, it appears this parsing logic
> will silently fail to extract the stack depth numbers, leaving the STACK and
> MAX_STACK metrics unpopulated when comparing regressions. Does veristat.c
> need to be updated to match the new verifier output format?

Hi Roman,

I think Sashiko should look at patches later in the stack to see whether the
issue it is pointing out will already be addressed later. Bisection related
concerns are different, but this (and many others) do not fall in that category.
This is a recurring source of various reports now. It isn't a big deal, but
would be good to improve upon.

Thanks

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats
  2026-08-02 22:52 ` [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats Kumar Kartikeya Dwivedi
  2026-08-02 23:04   ` sashiko-bot
@ 2026-08-03  0:21   ` bot+bpf-ci
  2026-08-03  0:40     ` Kumar Kartikeya Dwivedi
  1 sibling, 1 reply; 8+ messages in thread
From: bot+bpf-ci @ 2026-08-03  0:21 UTC (permalink / raw)
  To: memxor, bpf
  Cc: andrii, ast, daniel, eddyz87, emil, kkd, kernel-team, ast, andrii,
	daniel, martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

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

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index b274004fccfd9..3b61897ed0d2a 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18777,10 +18777,15 @@ static void print_verification_stats(struct bpf_verifier_env *env)
>  	if (env->log.level & BPF_LOG_STATS) {
>  		verbose(env, "verification time %lld usec\n",
>  			div_u64(env->verification_time, 1000));
> -		verbose(env, "stack depth %d", env->subprog_info[0].stack_depth);
> -		for (i = 1; i < subprog_cnt; i++)
> -			verbose(env, "+%d", env->subprog_info[i].stack_depth);
> -		verbose(env, " max %d\n", env->max_stack_depth);
> +		verbose(env, "stack depth max %d\n", env->max_stack_depth);
> +		for (i = 0; i < subprog_cnt; i++) {
> +			const char *name = env->subprog_info[i].name;
> +
> +			if (!name || !name[0])
> +				name = "<unknown>";
> +			verbose(env, "stack depth subprog %d %s %d\n", i, name,
> +				env->subprog_info[i].stack_depth);
> +		}

This isn't a bug, but could the name lookup here reuse subprog_name()?
It is already in this file and several other verbose() sites use it,
for example in do_check_common()'s caller:

kernel/bpf/verifier.c
	...
		} else if (env->log.level & BPF_LOG_LEVEL) {
			verbose(env, "Func#%d ('%s') is safe for any args that match its prototype\n",
				i, subprog_name(env, i));
		}
	...

subprog_name() returns "" when there is no func_info, so the
!name[0] fallback would still catch it:

kernel/bpf/verifier.c:subprog_name() {
	if (!env->prog->aux->func_info)
		return "";

	info = &env->prog->aux->func_info[subprog];
	return btf_type_name(env->prog->aux->btf, info->type_id);
}

There is also fmt_subprog() in kernel/bpf/liveness.c doing the same
name-or-fallback formatting, so this is a third variant of the same
lookup.

>  		verbose(env, "insns processed %d", env->subprog_info[0].insn_processed);
>  		for (i = 1; i < subprog_cnt; i++)
>  			if (bpf_subprog_is_global(env, i))

[ ... ]


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

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats
  2026-08-03  0:21   ` bot+bpf-ci
@ 2026-08-03  0:40     ` Kumar Kartikeya Dwivedi
  0 siblings, 0 replies; 8+ messages in thread
From: Kumar Kartikeya Dwivedi @ 2026-08-03  0:40 UTC (permalink / raw)
  To: bot+bpf-ci, bpf
  Cc: andrii, ast, daniel, eddyz87, emil, kkd, kernel-team, martin.lau,
	yonghong.song, clm, ihor.solodrai

On Mon Aug 3, 2026 at 2:21 AM CEST, bot+bpf-ci wrote:
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index b274004fccfd9..3b61897ed0d2a 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -18777,10 +18777,15 @@ static void print_verification_stats(struct bpf_verifier_env *env)
>>  	if (env->log.level & BPF_LOG_STATS) {
>>  		verbose(env, "verification time %lld usec\n",
>>  			div_u64(env->verification_time, 1000));
>> -		verbose(env, "stack depth %d", env->subprog_info[0].stack_depth);
>> -		for (i = 1; i < subprog_cnt; i++)
>> -			verbose(env, "+%d", env->subprog_info[i].stack_depth);
>> -		verbose(env, " max %d\n", env->max_stack_depth);
>> +		verbose(env, "stack depth max %d\n", env->max_stack_depth);
>> +		for (i = 0; i < subprog_cnt; i++) {
>> +			const char *name = env->subprog_info[i].name;
>> +
>> +			if (!name || !name[0])
>> +				name = "<unknown>";
>> +			verbose(env, "stack depth subprog %d %s %d\n", i, name,
>> +				env->subprog_info[i].stack_depth);
>> +		}
>
> This isn't a bug, but could the name lookup here reuse subprog_name()?
> It is already in this file and several other verbose() sites use it,
> for example in do_check_common()'s caller:
>
> kernel/bpf/verifier.c
> 	...
> 		} else if (env->log.level & BPF_LOG_LEVEL) {
> 			verbose(env, "Func#%d ('%s') is safe for any args that match its prototype\n",
> 				i, subprog_name(env, i));
> 		}
> 	...
>
> subprog_name() returns "" when there is no func_info, so the
> !name[0] fallback would still catch it:
>

This nit is fine.

> kernel/bpf/verifier.c:subprog_name() {
> 	if (!env->prog->aux->func_info)
> 		return "";
>
> 	info = &env->prog->aux->func_info[subprog];
> 	return btf_type_name(env->prog->aux->btf, info->type_id);
> }
>
> There is also fmt_subprog() in kernel/bpf/liveness.c doing the same
> name-or-fallback formatting, so this is a third variant of the same
> lookup.
>

That's too far fetched, and changes the format, I'll go with using subprog_name().

>>  		verbose(env, "insns processed %d", env->subprog_info[0].insn_processed);
>>  		for (i = 1; i < subprog_cnt; i++)
>>  			if (bpf_subprog_is_global(env, i))
>
> [ ... ]
>
>
> ---
> 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/30771845943


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-08-03  0:40 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-02 22:52 [PATCH bpf-next v2 0/3] Improve stack depth verification stats output Kumar Kartikeya Dwivedi
2026-08-02 22:52 ` [PATCH bpf-next v2 1/3] bpf: Show more useful info in stack depth stats Kumar Kartikeya Dwivedi
2026-08-02 23:04   ` sashiko-bot
2026-08-02 23:09     ` Kumar Kartikeya Dwivedi
2026-08-03  0:21   ` bot+bpf-ci
2026-08-03  0:40     ` Kumar Kartikeya Dwivedi
2026-08-02 22:52 ` [PATCH bpf-next v2 2/3] selftests/bpf: Adjust veristat stack depth parsing Kumar Kartikeya Dwivedi
2026-08-02 22:52 ` [PATCH bpf-next v2 3/3] selftests/bpf: Test stack depth stats without BTF subprog names Kumar Kartikeya Dwivedi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox