Re: [PATCH bpf-next 1/2] bpf: Report maximum combined stack depth

[Paul Chaignon <paul.chaignon@gmail.com>]

On Tue, May 12, 2026 at 02:53:33PM -0700, Eduard Zingerman wrote:
> On Tue, 2026-05-12 at 19:19 +0200, Paul Chaignon wrote:

[...]

> > diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> > index 11054ad89c14..896dbb4515d7 100644
> > --- a/kernel/bpf/verifier.c
> > +++ b/kernel/bpf/verifier.c
> > @@ -5045,6 +5045,8 @@ static int check_max_stack_depth_subprog(struct bpf_verifier_env *env, int idx,
> >  		}
> >  	} else {
> >  		depth += subprog_depth;
> > +		if (depth > env->max_stack_depth)
> > +			env->max_stack_depth = depth;
> >  		if (depth > MAX_BPF_STACK) {
> >  			total = 0;
> >  			for (tmp = idx; tmp >= 0; tmp = dinfo[tmp].caller)
> > @@ -5185,6 +5187,8 @@ static int check_max_stack_depth(struct bpf_verifier_env *env)
> >  	if (priv_stack_mode == PRIV_STACK_UNKNOWN)
> >  		priv_stack_mode = bpf_enable_priv_stack(env->prog);
> >  
> > +	env->max_stack_depth = env->subprog_info[0].stack_depth;
> > +
> 
> I think this line is redundant, the loop below would call
> check_max_stack_depth_subprog() for the main subprogram anyway.
> Additionally it does not round the value same way
> check_max_stack_depth_subprog() does. Also note that if main
> subprogram uses private stack it's depth is omitted in cumulative
> depth computation.

Yep, you're right. I had misread the loop below. I also need to update
env->max_stack_depth in the private-stack case in
check_max_stack_depth_subprog(). I'll add a selftest to cover that in
the v2.

> 
> >  	/* All async_cb subprogs use normal kernel stack. If a particular
> >  	 * subprog appears in both main prog and async_cb subtree, that
> >  	 * subprog will use normal kernel stack to avoid potential nesting.
> > @@ -18289,7 +18293,7 @@ static void print_verification_stats(struct bpf_verifier_env *env)
> >  		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, "\n");
> > +		verbose(env, " max %d\n", env->max_stack_depth);
> >  		verbose(env, "insns processed %d", env->subprog_info[0].insn_processed);
> >  		for (i = 1; i < subprog_cnt; i++)
> >  			if (bpf_subprog_is_global(env, i))
> 
> Maybe also add a veristat metric for this value?

Ack, makes sense.