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

"Kumar Kartikeya Dwivedi" <[email protected]> Sun, 02 Aug 2026 01:15:16 +0200
Newsgroups org.kernel.vger.bpf,dev.linux.lists.sashiko-reviews
Message-ID <[email protected]>
On Sun Aug 2, 2026 at 1:11 AM CEST, sashiko-bot wrote:
> 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.
>
>> 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 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])
>> +				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?
>

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).

> [ ... ]