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!
> 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
> #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.
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)
[...]
