Re: [PATCH bpf-next v6 3/6] bpf: Show more useful info in stack depth stats

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The verifier log format change for stack depth breaks `veristat`'s parsing of stack depth statistics.
--

--- Patch [3]: [PATCH bpf-next v6 3/6] bpf: Show more useful info in stack depth stats ---
commit 48703e00e6b412bacd50ea22b6dd5fcca0a7acb6
Author: Kumar Kartikeya Dwivedi <[email protected]>

bpf: Show more useful info in stack depth stats

This commit changes the verifier stack depth statistics format to keep
the combined maximum stack depth on its own line and print one uniform
record for each subprogram. This helps associate the statistics with
subprogram names and clarifies where verifier work is spent.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 47f3791530ded..a4ab7ee334c6b 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -18833,15 +18833,20 @@ 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, "insns processed %d", env->subprog_info[0].insns_total);
> -		for (i = 1; i < subprog_cnt; i++)
> -			if (bpf_subprog_is_global(env, i))
> -				verbose(env, "+%d", env->subprog_info[i].insns_total);
> -		verbose(env, "\n");
> +		verbose(env, "stack depth max %d\n", env->max_stack_depth);

[Severity: Medium]
Does this format change break veristat's log parsing?

Looking at parse_verif_log() in tools/testing/selftests/bpf/veristat.c,
the tool relies on the older format to parse these stats:

    if (2 == sscanf(cur, "stack depth %511s max %ld", stack, &s->stats[MAX_STACK]))
        continue;

Because the new format is "stack depth max %d\n", the sscanf parser will
consume "max" into the %511s string buffer and fail to match the rest of
the string.

This causes the sscanf to return 1 instead of 2, and the subsequent
fallback tokenization loop fails as well. Will this cause veristat to
silently report 0 for both STACK and MAX_STACK metrics for all processed
BPF programs?

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=3