Re: [PATCH bpf-next v2 1/3] 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 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 <[email protected]>

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/[email protected]?part=1