* [PATCH bpf] bpf: Don't run arg-tracking analysis twice on main subprog
@ 2026-05-07 18:22 Paul Chaignon
2026-05-08 23:24 ` Alexei Starovoitov
0 siblings, 1 reply; 5+ messages in thread
From: Paul Chaignon @ 2026-05-07 18:22 UTC (permalink / raw)
To: bpf
Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi
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.
*/
for (k = env->subprog_cnt - 1; k >= 0; k--) {
int sub = env->subprog_topo_order[k];
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] bpf: Don't run arg-tracking analysis twice on main subprog
2026-05-07 18:22 [PATCH bpf] bpf: Don't run arg-tracking analysis twice on main subprog Paul Chaignon
@ 2026-05-08 23:24 ` Alexei Starovoitov
2026-05-09 11:50 ` Eduard Zingerman
0 siblings, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2026-05-08 23:24 UTC (permalink / raw)
To: Paul Chaignon
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Eduard Zingerman, Kumar Kartikeya Dwivedi
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?
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] bpf: Don't run arg-tracking analysis twice on main subprog
2026-05-08 23:24 ` Alexei Starovoitov
@ 2026-05-09 11:50 ` Eduard Zingerman
2026-05-09 23:16 ` Alexei Starovoitov
0 siblings, 1 reply; 5+ messages in thread
From: Eduard Zingerman @ 2026-05-09 11:50 UTC (permalink / raw)
To: Alexei Starovoitov, Paul Chaignon
Cc: bpf, Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
Kumar Kartikeya Dwivedi
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>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] bpf: Don't run arg-tracking analysis twice on main subprog
2026-05-09 11:50 ` Eduard Zingerman
@ 2026-05-09 23:16 ` Alexei Starovoitov
2026-05-11 10:19 ` Paul Chaignon
0 siblings, 1 reply; 5+ messages in thread
From: Alexei Starovoitov @ 2026-05-09 23:16 UTC (permalink / raw)
To: Eduard Zingerman
Cc: Paul Chaignon, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kumar Kartikeya Dwivedi
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.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH bpf] bpf: Don't run arg-tracking analysis twice on main subprog
2026-05-09 23:16 ` Alexei Starovoitov
@ 2026-05-11 10:19 ` Paul Chaignon
0 siblings, 0 replies; 5+ messages in thread
From: Paul Chaignon @ 2026-05-11 10:19 UTC (permalink / raw)
To: Alexei Starovoitov
Cc: Eduard Zingerman, bpf, Alexei Starovoitov, Daniel Borkmann,
Andrii Nakryiko, Kumar Kartikeya Dwivedi
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!
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-05-11 10:19 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-07 18:22 [PATCH bpf] bpf: Don't run arg-tracking analysis twice on main subprog Paul Chaignon
2026-05-08 23:24 ` Alexei Starovoitov
2026-05-09 11:50 ` Eduard Zingerman
2026-05-09 23:16 ` Alexei Starovoitov
2026-05-11 10:19 ` Paul Chaignon
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox