[bpf] bpf: Make session kfuncs global

Commit Message

Jiri Olsa May 31, 2024, 10:15 a.m. UTC

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/[e-mail]/T/#m71c6d5ec71db2967288cb79acedc15cc5dbfeec5
Reported-by: Sebastian Andrzej Siewior <[e-mail]>
Suggested-by: Alexei Starovoitov <[e-mail]>
Fixes: 5c919acef8514 ("bpf: Add support for kprobe session cookie")
Signed-off-by: Jiri Olsa <[e-mail]>
---
 kernel/trace/bpf_trace.c | 2 --
 1 file changed, 2 deletions(-)

Comments

Sebastian Sewior May 31, 2024, 10:39 a.m. UTC | #1

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

Jiri Olsa May 31, 2024, 10:42 a.m. UTC | #2

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

Sebastian Sewior May 31, 2024, 10:49 a.m. UTC | #3

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

Jiri Olsa May 31, 2024, 11:25 a.m. UTC | #4

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

Sebastian Sewior May 31, 2024, 2:04 p.m. UTC | #5

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 <[e-mail]>

> jirka

Sebastian

Alexei Starovoitov May 31, 2024, 4:55 p.m. UTC | #6

On Fri, May 31, 2024 at 7:04 AM Sebastian Andrzej Siewior
<[e-mail]> 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.

Patch

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