On Tue, Apr 23, 2024 at 05:27:22PM -0700, Andrii Nakryiko wrote:
> On Mon, Apr 22, 2024 at 5:14 AM Jiri Olsa <jolsa@xxxxxxxxxx> wrote:
> >
> > Adding kprobe multi session test that verifies the cookie
> > value get properly propagated from entry to return program.
> >
> > Signed-off-by: Jiri Olsa <jolsa@xxxxxxxxxx>
> > ---
>
> Forgot to update subject (still using "wrapper" naming)
ugh, thnx
>
> overall LGTM, see nits
>
> Acked-by: Andrii Nakryiko <andrii@xxxxxxxxxx>
>
> > tools/testing/selftests/bpf/bpf_kfuncs.h | 1 +
> > .../bpf/prog_tests/kprobe_multi_test.c | 35 ++++++++++++
> > .../bpf/progs/kprobe_multi_session_cookie.c | 56 +++++++++++++++++++
> > 3 files changed, 92 insertions(+)
> > create mode 100644 tools/testing/selftests/bpf/progs/kprobe_multi_session_cookie.c
> >
> > diff --git a/tools/testing/selftests/bpf/bpf_kfuncs.h b/tools/testing/selftests/bpf/bpf_kfuncs.h
> > index 180030b5d828..0281921cd654 100644
> > --- a/tools/testing/selftests/bpf/bpf_kfuncs.h
> > +++ b/tools/testing/selftests/bpf/bpf_kfuncs.h
> > @@ -77,4 +77,5 @@ extern int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_ptr,
> > struct bpf_key *trusted_keyring) __ksym;
> >
> > extern bool bpf_session_is_return(void) __ksym;
> > +extern __u64 *bpf_session_cookie(void) __ksym;
>
> btw, should we use `long *` as return type to avoid relying on having
> __u64 alias be available? Long is always an 8-byte value in the BPF
> world, it should be fine.
ok, there are some __u64 in kfuncs already, but let's stay on safe side
SNIP
> > +/*
> > + * No tests in here, just to trigger 'bpf_fentry_test*'
> > + * through tracing test_run
> > + */
> > +SEC("fentry/bpf_modify_return_test")
> > +int BPF_PROG(trigger)
> > +{
> > + return 0;
> > +}
> > +
> > +static int check_cookie(__u64 val, __u64 *result)
> > +{
> > + if (bpf_get_current_pid_tgid() >> 32 != pid)
> > + return 1;
> > +
> > + __u64 *cookie = bpf_session_cookie();
>
> we don't enforce this, but let's stick to C89 variable declaration
> style (or rather positioning in this case)?
ok, will change that
jirka
