Re: [PATCH bpf-next v3 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 BPF verifier log format for stack depth but fails to update the parser in `veristat.c`, breaking its ability to parse and report stack depth metrics.
--

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

bpf: Show more useful info in stack depth stats

This commit updates the verifier log output format for stack depth statistics.
It prints the maximum stack depth and each subprogram's stack depth on
separate lines instead of a single concatenated string.

> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index b274004fccfd9..cc2b4a8619ac7 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);

[Severity: Medium]
Will this format change break the log parsing in veristat?

Looking at tools/testing/selftests/bpf/veristat.c:parse_verifier_log(), it
appears to still expect the old format to extract stack depth statistics:

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

Since the new log outputs "stack depth max %d\n", does veristat.c need to
be updated in this patch to prevent it from silently failing to extract the
metrics?

> +		for (i = 0; i < subprog_cnt; i++) {
> +			const char *name = subprog_name(env, i);
> +
> +			if (!name[0])
> +				name = "<unknown>";
> +			verbose(env, "stack depth subprog %d %s %d\n", i, name,
> +				env->subprog_info[i].stack_depth);
> +		}

[ ... ]

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