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

On Sun Aug 2, 2026 at 1:11 AM CEST, sashiko-bot wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issu=
e(s) to consider:
> - [Medium] The new stack depth verifier log format breaks stack metrics p=
arsing in the `veristat` tool.
> --
>
> --- Patch [1]: [PATCH bpf-next v1 1/3] bpf: Show more useful info in stac=
k 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 BP=
F
> verifier log. It replaces the old subprogram index-based format with a cl=
earer
> name and depth format, avoiding hard-to-read sequences.
>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 8d0635ee48c7e..51ac0a9ae3394 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -18703,10 +18703,18 @@ static void print_verification_stats(struct bp=
f_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 =3D 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 =3D 0; i < subprog_cnt; i++) {
>> +			const char *name =3D env->subprog_info[i].name;
>> +
>> +			if (name && name[0])
>> +				verbose(env, " %s=3D%d", name,
>> +					env->subprog_info[i].stack_depth);
>> +			else
>> +				verbose(env, " subprog[%d]=3D%d", i,
>> +					env->subprog_info[i].stack_depth);
>> +		}
>> +		verbose(env, " max=3D%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(), i=
t
> 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 =3D=3D sscanf(cur, "stack depth %511s max %ld", stack, &s->stat=
s[MAX_STACK]))
>     ...
>     while ((token =3D strtok_r(cnt++ ? NULL : stack, "+", &state))) {
>         if (sscanf(token, "%d", &sub_stack) =3D=3D 0)
>             break;
>     ...
> }
>
> Since the verifier now outputs max=3D%ld instead of max %ld, and the indi=
vidual
> depths are formatted as name=3Ddepth rather than numbers separated by a p=
lus
> sign, will veristat silently fail to parse STACK and MAX_STACK metrics wh=
en
> processing BPF objects?
>

Fixed in patch 2. It is an open question whether we care about format
compatibility, in that case we will need further adjustments to parse both =
kinds
of output (for older and newer kernels).

> [ ... ]