Improve stack depth verification stats output

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The new stack depth verifier log format breaks stack metrics parsing in the veristat tool. --

--- Patch [1]: [PATCH bpf-next v1 1/3] bpf: Show more useful info in stack depth stats --- commit f7cb915ff02de7a02fe7b2353a1f2d77e1852496
Author: Kumar Kartikeya Dwivedi <[email protected]>

bpf: Show more useful info in stack depth stats

This commit changes the output format of stack depth statistics in the BPF verifier log. It replaces the old subprogram index-based format with a clearer name and depth format, avoiding hard-to-read sequences.

Quoted 14 lines

kernel/bpf/verifier.c +2 -4

index 8d0635ee48c7e..51ac0a9ae3394 100644
@@ -18703,10 +18703,18 @@ 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");
+		for (i = 0; i < subprog_cnt; i++) {

kernel/bpf/verifier.c static void print_verification_stats(struct bpf_verifier_env *env) · line 18717

Quoted 10 lines

kernel/bpf/verifier.c +10 -0

+			const char *name = env->subprog_info[i].name;
+
+			if (name && name[0])
+				verbose(env, " %s=%d", name,
+					env->subprog_info[i].stack_depth);
+			else
+				verbose(env, " subprog[%d]=%d", i,
+					env->subprog_info[i].stack_depth);
+		}
+		verbose(env, " max=%d\n", env->max_stack_depth);

[Severity: Medium]
Does this change break the veristat tool's parsing of stack depths?

Looking at tools/testing/selftests/bpf/veristat.c in parse_verif_log(), it appears to rely on the old format using a hardcoded string and splitting by the plus sign:

tools/testing/selftests/bpf/veristat.c:parse_verif_log() {
    ...
    if (2 == sscanf(cur, "stack depth %511s max %ld", stack, &s->stats[MAX_STACK]))
    ...
    while ((token = strtok_r(cnt++ ? NULL : stack, "+", &state))) {
        if (sscanf(token, "%d", &sub_stack) == 0)
            break;
    ...
}

Since the verifier now outputs max=%ld instead of max %ld, and the individual depths are formatted as name=depth rather than numbers separated by a plus sign, will veristat silently fail to parse STACK and MAX_STACK metrics when processing BPF objects?

[ ... ]

Currently, the output of stack depth statistics is two crude, with a list of captured stack depths ordered by the subprog numbers. The actual subprog numbers are determined by libbpf, hence it is hard to associate the stack depth statistic back to the subprog by name.

Change the format to:
stack depth <subprog>=<depth> ... max=<depth>

In case the subprog name is not specified, use subprog[N] as the string.

Suggested-by Andrii Nakryiko <[email protected]>

Signed-off-by Kumar Kartikeya Dwivedi <[email protected]>

---
 kernel/bpf/verifier.c                         | 16 ++++++++++++---- .../bpf/progs/verifier_basic_stack.c          |  4 ++-- .../bpf/progs/verifier_bpf_fastcall.c         | 19 +++++++++++-------- .../bpf/progs/verifier_private_stack.c        |  9 ++++++--- .../selftests/bpf/progs/verifier_var_off.c    |  4 ++--

 5 files changed, 33 insertions(+), 19 deletions(-)

kernel/bpf/verifier.c +0 -4

index 8d0635ee48c7..51ac0a9ae339 100644
@@ -18703,10 +18703,18 @@ 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);

Quoted 37 lines · Kumar Kartikeya Dwivedi

Currently, the output of stack depth statistics is two crude, with a list of captured stack depths ordered by the subprog numbers. The actual subprog numbers are determined by libbpf, hence it is hard to associate the stack depth statistic back to the subprog by name.

Change the format to:
stack depth <subprog>=<depth> ... max=<depth>

In case the subprog name is not specified, use subprog[N] as the string.

Suggested-by Andrii Nakryiko <[email protected]>

Signed-off-by Kumar Kartikeya Dwivedi <[email protected]>

---
 kernel/bpf/verifier.c                         | 16 ++++++++++++---- .../bpf/progs/verifier_basic_stack.c          |  4 ++-- .../bpf/progs/verifier_bpf_fastcall.c         | 19 +++++++++++-------- .../bpf/progs/verifier_private_stack.c        |  9 ++++++--- .../selftests/bpf/progs/verifier_var_off.c    |  4 ++--

 5 files changed, 33 insertions(+), 19 deletions(-)

kernel/bpf/verifier.c +5 -4

index 8d0635ee48c7..51ac0a9ae339 100644
@@ -18703,10 +18703,18 @@ 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");
+		for (i = 0; i < subprog_cnt; i++) {
+			const char *name = env->subprog_info[i].name;
+
+			if (name && name[0])

kernel/bpf/verifier.c static void print_verification_stats(struct bpf_verifier_env *env) · line 18715

Quoted 5 lines

kernel/bpf/verifier.c +5 -0

+				verbose(env, " %s=%d", name,
+					env->subprog_info[i].stack_depth);
+			else
+				verbose(env, " subprog[%d]=%d", i,
+					env->subprog_info[i].stack_depth);

That would be not very convenient to parse by veristat, but I'd print every subprogram on it's own line.

kernel/bpf/verifier.c static void print_verification_stats(struct bpf_verifier_env *env) · line 18717

Quoted 2 lines

kernel/bpf/verifier.c +2 -0

+		}
+		verbose(env, " max=%d\n", env->max_stack_depth);

I think check_max_stack_depth_subprog() needs a similar update, at the moment it reports a not very helpful error like this:

combined stack size of 34 calls is 528. Too large

Printing the offending spine would be helpful.

...

Quoted 38 lines · Eduard Zingerman

Quoted 37 lines · Kumar Kartikeya Dwivedi

Currently, the output of stack depth statistics is two crude, with a list of captured stack depths ordered by the subprog numbers. The actual subprog numbers are determined by libbpf, hence it is hard to associate the stack depth statistic back to the subprog by name.

Change the format to:
stack depth <subprog>=<depth> ... max=<depth>

In case the subprog name is not specified, use subprog[N] as the string.

Suggested-by Andrii Nakryiko <[email protected]>

Signed-off-by Kumar Kartikeya Dwivedi <[email protected]>

---
 kernel/bpf/verifier.c                         | 16 ++++++++++++---- .../bpf/progs/verifier_basic_stack.c          |  4 ++-- .../bpf/progs/verifier_bpf_fastcall.c         | 19 +++++++++++-------- .../bpf/progs/verifier_private_stack.c        |  9 ++++++--- .../selftests/bpf/progs/verifier_var_off.c    |  4 ++--

 5 files changed, 33 insertions(+), 19 deletions(-)

kernel/bpf/verifier.c +5 -4

index 8d0635ee48c7..51ac0a9ae339 100644
@@ -18703,10 +18703,18 @@ 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");
+		for (i = 0; i < subprog_cnt; i++) {
+			const char *name = env->subprog_info[i].name;
+
+			if (name && name[0])

Quoted 9 lines

kernel/bpf/verifier.c static void print_verification_stats(struct bpf_verifier_env *env) · line 18715

Quoted 5 lines

kernel/bpf/verifier.c +5 -0

+				verbose(env, " %s=%d", name,
+					env->subprog_info[i].stack_depth);
+			else
+				verbose(env, " subprog[%d]=%d", i,
+					env->subprog_info[i].stack_depth);

That would be not very convenient to parse by veristat, but I'd print every subprogram on it's own line.

I can do that, but let's agree on the format before I get to it. How should it look like? Prefix with stack depth every line, and print stack depth max ... as the very first line? Like so:

stack depth max N
stack depth subprog 0 <name> N
...
stack depth subprog N <name> N

?

Quoted 6 lines

kernel/bpf/verifier.c static void print_verification_stats(struct bpf_verifier_env *env) · line 18717

Quoted 2 lines

kernel/bpf/verifier.c +2 -0

+		}
+		verbose(env, " max=%d\n", env->max_stack_depth);

I think check_max_stack_depth_subprog() needs a similar update, at the moment it reports a not very helpful error like this:

Quoted 4 lines

  combined stack size of 34 calls is 528. Too large

Printing the offending spine would be helpful.

I'll do that in verifier errors patch set. This is just for the verification stats, that are outside its scope.

The verifier changed the output format for stack depth verification statistics. Adjust the parsing code.

Signed-off-by Kumar Kartikeya Dwivedi <[email protected]>

---

Quoted 3 lines · Kumar Kartikeya Dwivedi

The verifier changed the output format for stack depth verification statistics. Adjust the parsing code.

Quoted 2 lines

Signed-off-by Kumar Kartikeya Dwivedi <[email protected]>

---

Do we care about backwards compatibility in veristat? E.g. I see it packaged in my debian system.
This won't be able to parse the output of an old kernel. On the other hand, I personally do not use stack metrics, so the change does not really affect me.

...

tools/testing/selftests/bpf/test_verifier.c +0 -0

@@ -1559,7 +1560,9 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
  		       test->errstr_unpriv : test->errstr;

  opts.expected_attach_type = test->expected_attach_type;

tools/testing/selftests/bpf/test_verifier.c +3 -1

-	if (expected_ret == VERBOSE_ACCEPT)
+	if (test->log_level)
+		opts.log_level = test->log_level;
+	else if (expected_ret == VERBOSE_ACCEPT)

  opts.log_level = 2;

  else if (verbose)
  opts.log_level = verif_log_level | 4; /* force stats */

On Sun, 2026-08-02 at 01:03 +0200, Kumar Kartikeya Dwivedi wrote:

...

Quoted 9 lines

tools/testing/selftests/bpf/test_verifier.c +0 -0

@@ -1559,7 +1560,9 @@ static void do_test_single(struct bpf_test *test, bool unpriv,
  		       test->errstr_unpriv : test->errstr;

  opts.expected_attach_type = test->expected_attach_type;

tools/testing/selftests/bpf/test_verifier.c +3 -1

-	if (expected_ret == VERBOSE_ACCEPT)
+	if (test->log_level)
+		opts.log_level = test->log_level;
+	else if (expected_ret == VERBOSE_ACCEPT)

  opts.log_level = 2;

I did this:

          else if (expected_ret == VERBOSE_ACCEPT)

- opts.log_level = 2;
+ opts.log_level = 2 | 4;

And all test_verifier tests passed.
There is no need to a new option.

Quoted 2 lines

  else if (verbose)
  opts.log_level = verif_log_level | 4; /* force stats */

...