[PATCH bpf] bpf: Make session kfuncs global

[PATCH bpf] bpf: Make session kfuncs global

[Jiri Olsa]

The bpf_session_cookie is unavailable for !CONFIG_FPROBE as reported
by Sebastian [1].

Instead of adding more ifdefs, making the session kfuncs globally
available as suggested by Alexei. It's still allowed only for
session programs, but it won't fail the build.

[1] https://lore.kernel.org/bpf/20240531071557.MvfIqkn7@linutronix.de/T/#m71c6d5ec71db2967288cb79acedc15cc5dbfeec5
Reported-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Suggested-by: Alexei Starovoitov <ast@kernel.org>
Fixes: 5c919acef8514 ("bpf: Add support for kprobe session cookie")
Signed-off-by: Jiri Olsa <jolsa@kernel.org>
---
 kernel/trace/bpf_trace.c | 2 --
 1 file changed, 2 deletions(-)

diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f5154c051d2c..cc90d56732eb 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -3519,7 +3519,6 @@ static u64 bpf_uprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
 }
 #endif /* CONFIG_UPROBES */
 
-#ifdef CONFIG_FPROBE
 __bpf_kfunc_start_defs();
 
 __bpf_kfunc bool bpf_session_is_return(void)
@@ -3568,4 +3567,3 @@ static int __init bpf_kprobe_multi_kfuncs_init(void)
 }
 
 late_initcall(bpf_kprobe_multi_kfuncs_init);
-#endif
-- 
2.45.1

Re: [PATCH bpf] bpf: Make session kfuncs global

[Sebastian Andrzej Siewior]

On 2024-05-31 12:15:50 [+0200], Jiri Olsa wrote:
> The bpf_session_cookie is unavailable for !CONFIG_FPROBE as reported
> by Sebastian [1].
> 
> Instead of adding more ifdefs, making the session kfuncs globally
> available as suggested by Alexei. It's still allowed only for
> session programs, but it won't fail the build.

but this relies on CONFIG_UPROBE_EVENTS=y
What about CONFIG_UPROBE_EVENTS=n?

Sebastian

Re: [PATCH bpf] bpf: Make session kfuncs global

[Jiri Olsa]

On Fri, May 31, 2024 at 12:39:31PM +0200, Sebastian Andrzej Siewior wrote:
> On 2024-05-31 12:15:50 [+0200], Jiri Olsa wrote:
> > The bpf_session_cookie is unavailable for !CONFIG_FPROBE as reported
> > by Sebastian [1].
> > 
> > Instead of adding more ifdefs, making the session kfuncs globally
> > available as suggested by Alexei. It's still allowed only for
> > session programs, but it won't fail the build.
> 
> but this relies on CONFIG_UPROBE_EVENTS=y
> What about CONFIG_UPROBE_EVENTS=n?

hum, I can't see that.. also I tested it with CONFIG_UPROBE_EVENTS=n,
the CONFIG_UPROBES ifdef is ended right above this code..

jirka

> 
> Sebastian

Re: [PATCH bpf] bpf: Make session kfuncs global

[Sebastian Andrzej Siewior]

On 2024-05-31 12:42:41 [+0200], Jiri Olsa wrote:
> On Fri, May 31, 2024 at 12:39:31PM +0200, Sebastian Andrzej Siewior wrote:
> > On 2024-05-31 12:15:50 [+0200], Jiri Olsa wrote:
> > > The bpf_session_cookie is unavailable for !CONFIG_FPROBE as reported
> > > by Sebastian [1].
> > > 
> > > Instead of adding more ifdefs, making the session kfuncs globally
> > > available as suggested by Alexei. It's still allowed only for
> > > session programs, but it won't fail the build.
> > 
> > but this relies on CONFIG_UPROBE_EVENTS=y
> > What about CONFIG_UPROBE_EVENTS=n?
> 
> hum, I can't see that.. also I tested it with CONFIG_UPROBE_EVENTS=n,
> the CONFIG_UPROBES ifdef is ended right above this code..

Your patch + v6.10-rc1 + https://breakpoint.cc/config-2024-03-31.xz

> jirka

Sebastian

Re: [PATCH bpf] bpf: Make session kfuncs global

[Jiri Olsa]

On Fri, May 31, 2024 at 12:49:22PM +0200, Sebastian Andrzej Siewior wrote:
> On 2024-05-31 12:42:41 [+0200], Jiri Olsa wrote:
> > On Fri, May 31, 2024 at 12:39:31PM +0200, Sebastian Andrzej Siewior wrote:
> > > On 2024-05-31 12:15:50 [+0200], Jiri Olsa wrote:
> > > > The bpf_session_cookie is unavailable for !CONFIG_FPROBE as reported
> > > > by Sebastian [1].
> > > > 
> > > > Instead of adding more ifdefs, making the session kfuncs globally
> > > > available as suggested by Alexei. It's still allowed only for
> > > > session programs, but it won't fail the build.
> > > 
> > > but this relies on CONFIG_UPROBE_EVENTS=y
> > > What about CONFIG_UPROBE_EVENTS=n?
> > 
> > hum, I can't see that.. also I tested it with CONFIG_UPROBE_EVENTS=n,
> > the CONFIG_UPROBES ifdef is ended right above this code..
> 
> Your patch + v6.10-rc1 + https://breakpoint.cc/config-2024-03-31.xz

ah there's also CONFIG_KPROBE=n

kernel/trace/bpf_trace.c is enabled with CONFIG_BPF_EVENTS,
which has:

        depends on BPF_SYSCALL
        depends on (KPROBE_EVENTS || UPROBE_EVENTS) && PERF_EVENTS

so I think we chould combine both like below

jirka


---
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 77da1f438bec..cb202a289cf6 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -11124,7 +11124,11 @@ BTF_ID(func, bpf_iter_css_task_new)
 #else
 BTF_ID_UNUSED
 #endif
+#ifdef CONFIG_BPF_EVENTS
 BTF_ID(func, bpf_session_cookie)
+#else
+BTF_ID_UNUSED
+#endif
 
 static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
 {
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c
index f5154c051d2c..cc90d56732eb 100644
--- a/kernel/trace/bpf_trace.c
+++ b/kernel/trace/bpf_trace.c
@@ -3519,7 +3519,6 @@ static u64 bpf_uprobe_multi_entry_ip(struct bpf_run_ctx *ctx)
 }
 #endif /* CONFIG_UPROBES */
 
-#ifdef CONFIG_FPROBE
 __bpf_kfunc_start_defs();
 
 __bpf_kfunc bool bpf_session_is_return(void)
@@ -3568,4 +3567,3 @@ static int __init bpf_kprobe_multi_kfuncs_init(void)
 }
 
 late_initcall(bpf_kprobe_multi_kfuncs_init);
-#endif

Re: [PATCH bpf] bpf: Make session kfuncs global

[Sebastian Andrzej Siewior]

On 2024-05-31 13:25:14 [+0200], Jiri Olsa wrote:
> ah there's also CONFIG_KPROBE=n
> 
> kernel/trace/bpf_trace.c is enabled with CONFIG_BPF_EVENTS,
> which has:
> 
>         depends on BPF_SYSCALL
>         depends on (KPROBE_EVENTS || UPROBE_EVENTS) && PERF_EVENTS
> 
> so I think we chould combine both like below

Yes, this would work.

Tested-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>

> jirka

Sebastian

Re: [PATCH bpf] bpf: Make session kfuncs global

[Alexei Starovoitov]

On Fri, May 31, 2024 at 7:04 AM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2024-05-31 13:25:14 [+0200], Jiri Olsa wrote:
> > ah there's also CONFIG_KPROBE=n
> >
> > kernel/trace/bpf_trace.c is enabled with CONFIG_BPF_EVENTS,
> > which has:
> >
> >         depends on BPF_SYSCALL
> >         depends on (KPROBE_EVENTS || UPROBE_EVENTS) && PERF_EVENTS
> >
> > so I think we chould combine both like below
>
> Yes, this would work.

Makes sense to me as well. Pls respin.