On Wed, May 1, 2024 at 2:05 AM Jiri Olsa <olsajiri@xxxxxxxxx> wrote:
>
> On Tue, Apr 30, 2024 at 10:29:05AM -0700, Andrii Nakryiko wrote:
> > On Tue, Apr 30, 2024 at 4:29 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> > >
> > > Adding kprobe session test and testing that the entry program
> > > return value controls execution of the return probe program.
> > >
> > > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > > ---
> > > tools/testing/selftests/bpf/bpf_kfuncs.h | 2 +
> > > .../bpf/prog_tests/kprobe_multi_test.c | 39 ++++++++++
> > > .../bpf/progs/kprobe_multi_session.c | 78 +++++++++++++++++++
> > > 3 files changed, 119 insertions(+)
> > > create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi_session.c
> > >
> >
> > Given the things I mentioned below were the only "problems" I found, I
> > applied the patch and fixed those issues up while applying. Thanks a
> > lot for working on this! Excited about this feature, it's been asked
> > by our internal customers for a while as well. Looking forward to
> > uprobe session program type!
>
> great, I'll send it soon
>
> >
> > > diff --git a/tools/testing/selftests/bpf/bpf_kfuncs.h b/tools/testing/selftests/bpf/bpf_kfuncs.h
> > > index 14ebe7d9e1a3..180030b5d828 100644
> > > --- a/tools/testing/selftests/bpf/bpf_kfuncs.h
> > > +++ b/tools/testing/selftests/bpf/bpf_kfuncs.h
> > > @@ -75,4 +75,6 @@ extern void bpf_key_put(struct bpf_key *key) __ksym;
> > > extern int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr,
> > > struct bpf_dynptr *sig_ptr,
> > > struct bpf_key *trusted_keyring) __ksym;
> > > +
> > > +extern bool bpf_session_is_return(void) __ksym;
> >
> > should be __weak, always make it __weak. vmlinux.h with kfuncs is coming
> >
> > same for another kfunc in next patch
>
> ok
>
> >
> > > #endif
> >
> > [...]
> >
> > > +static const void *kfuncs[8] = {
> > > + &bpf_fentry_test1,
> > > + &bpf_fentry_test2,
> > > + &bpf_fentry_test3,
> > > + &bpf_fentry_test4,
> > > + &bpf_fentry_test5,
> > > + &bpf_fentry_test6,
> > > + &bpf_fentry_test7,
> > > + &bpf_fentry_test8,
> > > +};
> > > +
> >
> > this is not supposed to work :) I don't think libbpf support this kind
> > of relocations in data section.
>
> aah, nice ;-) should we make it work (or make sure it works) ? seems useful
>
let's see if there are any real use cases beyond selftests, first
> >
> > The only reason it works in practice is because compiler completely
> > inlines access to this array and so code just has unrolled loop
> > (thanks to "static const" and -O2).
> >
> > This is a bit fragile, though. It might keep working, of course
> > (though I'm not sure if -O1 would still work), but I'd feel a bit more
> > comfortable if you define and initialize this array inside the
> > function (then it will be guaranteed to work with libbpf logic)
>
> thanks,
> jirka
