On Tue, Jun 18, 2024 at 12:08 PM Jiri Olsa olsajiri@gmail.com wrote:
On Tue, Jun 18, 2024 at 09:58:23AM -0700, Andrii Nakryiko wrote:
On Tue, Jun 18, 2024 at 5:43 AM Jiri Olsa olsajiri@gmail.com wrote:
On Mon, Jun 17, 2024 at 03:25:53PM -0700, Andrii Nakryiko wrote:
On Mon, Jun 10, 2024 at 6:04 AM Jiri Olsa olsajiri@gmail.com wrote:
On Sat, Jun 08, 2024 at 03:16:02PM -0600, Daniel Xu wrote:
The prototype defined in bpf_kfuncs.h was not in line with how the actual kfunc was defined. This causes compilation errors when kfunc prototypes are generated from BTF.
Fix by aligning with actual kfunc definition.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz
tools/testing/selftests/bpf/bpf_kfuncs.h | 2 +- tools/testing/selftests/bpf/progs/kprobe_multi_session_cookie.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/bpf_kfuncs.h b/tools/testing/selftests/bpf/bpf_kfuncs.h index be91a6919315..3b6675ab4086 100644 --- a/tools/testing/selftests/bpf/bpf_kfuncs.h +++ b/tools/testing/selftests/bpf/bpf_kfuncs.h @@ -77,5 +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 __weak; -extern long *bpf_session_cookie(void) __ksym __weak; +extern __u64 *bpf_session_cookie(void) __ksym __weak;
the original intent was to expose long instead of __u64 :-\
Cookies internally are always u64 (8 byte values). Marking them internally in the kernel as long could lead to problems on 32-bit architectures, potentially (it still needs to be 64-bit value according to BPF contract, but we'll allocate only 4 bytes for them).
It seems better and safer to be explicit with __u64/u64 for cookies everywhere.
hum, I based that on what we did for kprobe session, but I guess it makes sense just for bpf side:
yep, exactly, long is 64-bit only for BPF "architecture", but internally it will be 4 bytes for 32-bit architectures, which will potentially lead to problems. With recent kfunc vmlinux.h generation, it's probably better to stick to explicitly sized types.
hm, it already got in 2b8dd87332cd, revert needs more changes in selftests I'll send formal patch with fix below
Yeah, I was a bit late to the party. But I replied on the original thread as well, I think we should use __u64 (or unsigned long long if we worry about __u64 typedef, but I think at least for vmlinux.h it doesn't matter).
And thanks for working on a fix!
jirka
diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index 4b3fda456299..cd098846e251 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -3530,7 +3530,7 @@ __bpf_kfunc bool bpf_session_is_return(void) return session_ctx->is_return; }
-__bpf_kfunc long *bpf_session_cookie(void) +__bpf_kfunc __u64 *bpf_session_cookie(void) { struct bpf_session_run_ctx *session_ctx;
diff --git a/tools/testing/selftests/bpf/bpf_kfuncs.h b/tools/testing/selftests/bpf/bpf_kfuncs.h index be91a6919315..3b6675ab4086 100644 --- a/tools/testing/selftests/bpf/bpf_kfuncs.h +++ b/tools/testing/selftests/bpf/bpf_kfuncs.h @@ -77,5 +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 __weak; -extern long *bpf_session_cookie(void) __ksym __weak; +extern __u64 *bpf_session_cookie(void) __ksym __weak; #endif diff --git a/tools/testing/selftests/bpf/progs/kprobe_multi_session_cookie.c b/tools/testing/selftests/bpf/progs/kprobe_multi_session_cookie.c index d49070803e22..0835b5edf685 100644 --- a/tools/testing/selftests/bpf/progs/kprobe_multi_session_cookie.c +++ b/tools/testing/selftests/bpf/progs/kprobe_multi_session_cookie.c @@ -25,7 +25,7 @@ int BPF_PROG(trigger)
static int check_cookie(__u64 val, __u64 *result) {
long *cookie;
__u64 *cookie; if (bpf_get_current_pid_tgid() >> 32 != pid) return 1;