Re: [PATCH bpf] bpf: Don't run arg-tracking analysis twice on main subprog

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

On Sat, May 09, 2026 at 04:16:10PM -0700, Alexei Starovoitov wrote:
> On Sat, May 9, 2026 at 4:50 AM Eduard Zingerman <eddyz87@gmail.com> wrote:
> >
> > On Fri, 2026-05-08 at 16:24 -0700, Alexei Starovoitov wrote:
> > > On Thu, May 7, 2026 at 11:22 AM Paul Chaignon <paul.chaignon@gmail.com> wrote:
> > > >
> > > > Because subprog 0, the main subprog, is considered a global function,
> > > > we end up running the arg-tracking dataflow analysis twice on it. That
> > > > results in slightly longer verification but mostly in more verbose
> > > > verifier logs. This patch fixes it by keeping only the iteration over
> > > > global subprogs.
> > > >
> > > > When running over all of Cilium's programs with BPF_LOG_LEVEL2, this
> > > > reduces verbosity by ~20% on average.
> > > >
> > > > Fixes: bf0c571f7feb6 ("bpf: introduce forward arg-tracking dataflow analysis")
> > > > Signed-off-by: Paul Chaignon <paul.chaignon@gmail.com>
> > > > ---
> > > >  kernel/bpf/liveness.c | 11 ++---------
> > > >  1 file changed, 2 insertions(+), 9 deletions(-)
> > > >
> > > > diff --git a/kernel/bpf/liveness.c b/kernel/bpf/liveness.c
> > > > index 332e6e003f27..505250998f36 100644
> > > > --- a/kernel/bpf/liveness.c
> > > > +++ b/kernel/bpf/liveness.c
> > > > @@ -1914,15 +1914,6 @@ int bpf_compute_subprog_arg_access(struct bpf_verifier_env *env)
> > > >                 return -ENOMEM;
> > > >         }
> > > >
> > > > -       instance = call_instance(env, NULL, 0, 0);
> > > > -       if (IS_ERR(instance)) {
> > > > -               err = PTR_ERR(instance);
> > > > -               goto out;
> > > > -       }
> > > > -       err = analyze_subprog(env, NULL, info, instance, callsites);
> > > > -       if (err)
> > > > -               goto out;
> > > > -
> > > >         /*
> > > >          * Subprogs and callbacks that don't receive FP-derived arguments
> > > >          * cannot access ancestor stack frames, so they were skipped during
> > > > @@ -1934,6 +1925,8 @@ int bpf_compute_subprog_arg_access(struct bpf_verifier_env *env)
> > > >          * each subprog is analyzed before its callees, allowing the
> > > >          * recursive walk inside analyze_subprog() to naturally
> > > >          * reach nested callees that also lack FP-derived args.
> > > > +        *
> > > > +        * Note the main subprog is also analyzed as part of this loop.
> > > >          */
> > >
> > > Thanks for the report.
> > > I guess the fix is correct.
> > > This part of the comment needs to be adjusted:
> > > "they were skipped during
> > >          * the recursive walk above."
> > >
> > > I wonder whether something like this is cleaner?
> > > -               if (info[sub].at_in && !bpf_subprog_is_global(env, sub))
> > > +               if (info[sub].at_in && (!bpf_subprog_is_global(env,
> > > sub) || sub == 0))
> > >
> > > This part of the comment:
> > > "Async callbacks (timer, workqueue) are
> > > * also not reachable from the main program's call graph."
> > >
> > > is also not quite correct.
> > > In here:
> > >                 } else if (bpf_calls_callback(env, idx)) {
> > >                         callee = find_callback_subprog(env, insn, idx,
> > > &caller_reg, &cb_callee_reg);
> > >
> > > they could have been reached, but find_callback_subprog()
> > > will ignore timers because FP-derived args won't be passed
> > > into them.
> > >
> > > Eduard,
> > > wdyt?
> >
> > This is a nice catch. Idk if 'sub == 0' part is necessary, tbh.
> > Paul, please add my acked-by when you respin with fixed comments.
> >
> > Acked-by: Eduard Zingerman <eddyz87@gmail.com>
> 
> Reworded myself and applied.

Thanks!