This patchset enables both detecting as well as dumping compilable prototypes for kfuncs.
The first commit instructs pahole to DECL_TAG kfuncs when available. This requires v1.27 or newer. v1.27 is nearing release at time of writing. Following this, users will be able to look at BTF inside vmlinux (or modules) and check if the kfunc they want is available.
The final commit teaches bpftool how to dump kfunc prototypes. This is done for developer convenience.
The rest of the commits are fixups to enable selftests to use the newly dumped kfunc prototypes. With these, selftests will regularly exercise the newly added codepaths.
=== Changelog === From v3: * Teach selftests to use dumped prototypes
From v2: * Update Makefile.btf with pahole flag * More error checking * Output formatting changes * Drop already-merged commit
From v1: * Add __weak annotation * Use btf_dump for kfunc prototypes * Update kernel bpf_rdonly_cast() signature
Daniel Xu (12): kbuild: bpf: Tell pahole to DECL_TAG kfuncs bpf: selftests: Fix bpf_iter_task_vma_new() prototype bpf: selftests: Fix fentry test kfunc prototypes bpf: selftests: Fix bpf_cpumask_first_zero() kfunc prototype bpf: selftests: Fix bpf_map_sum_elem_count() kfunc prototype bpf: selftests: Fix bpf_session_cookie() kfunc prototype bpf: selftests: Namespace struct_opt callbacks in bpf_dctcp bpf: verifier: Relax caller requirements for kfunc projection type args bpf: treewide: Align kfunc signatures to prog point-of-view bpf: selftests: nf: Opt out of using generated kfunc prototypes bpf: selftests: xfrm: Opt out of using generated kfunc prototypes bpftool: Support dumping kfunc prototypes from BTF
fs/verity/measure.c | 3 +- include/linux/bpf.h | 8 +-- kernel/bpf/crypto.c | 24 +++++--- kernel/bpf/helpers.c | 39 +++++++++---- kernel/bpf/verifier.c | 12 +++- kernel/trace/bpf_trace.c | 9 ++- net/core/filter.c | 32 +++++++---- scripts/Makefile.btf | 2 +- tools/bpf/bpftool/btf.c | 55 +++++++++++++++++++ .../testing/selftests/bpf/bpf_experimental.h | 2 +- tools/testing/selftests/bpf/bpf_kfuncs.h | 2 +- tools/testing/selftests/bpf/progs/bpf_dctcp.c | 36 ++++++------ .../selftests/bpf/progs/get_func_ip_test.c | 14 ++--- .../selftests/bpf/progs/ip_check_defrag.c | 10 ++-- .../bpf/progs/kprobe_multi_session_cookie.c | 2 +- .../selftests/bpf/progs/map_percpu_stats.c | 2 +- .../selftests/bpf/progs/nested_trust_common.h | 2 +- .../testing/selftests/bpf/progs/test_bpf_nf.c | 1 + .../selftests/bpf/progs/test_bpf_nf_fail.c | 1 + .../bpf/progs/verifier_netfilter_ctx.c | 6 +- .../selftests/bpf/progs/xdp_synproxy_kern.c | 1 + tools/testing/selftests/bpf/progs/xfrm_info.c | 1 + 22 files changed, 183 insertions(+), 81 deletions(-)
bpf_iter_task_vma_new() is defined as taking a u64 as its 3rd argument. u64 is a unsigned long long. bpf_experimental.h was defining the prototype as unsigned long.
Fix by using __u64.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz --- tools/testing/selftests/bpf/bpf_experimental.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/bpf_experimental.h b/tools/testing/selftests/bpf/bpf_experimental.h index 3d9e4b8c6b81..8ee7a00b7c82 100644 --- a/tools/testing/selftests/bpf/bpf_experimental.h +++ b/tools/testing/selftests/bpf/bpf_experimental.h @@ -163,7 +163,7 @@ struct bpf_iter_task_vma;
extern int bpf_iter_task_vma_new(struct bpf_iter_task_vma *it, struct task_struct *task, - unsigned long addr) __ksym; + __u64 addr) __ksym; extern struct vm_area_struct *bpf_iter_task_vma_next(struct bpf_iter_task_vma *it) __ksym; extern void bpf_iter_task_vma_destroy(struct bpf_iter_task_vma *it) __ksym;
The prototypes in progs/get_func_ip_test.c were not in line with how the actual kfuncs are defined in net/bpf/test_run.c. This causes compilation errors when kfunc prototypes are generated from BTF.
Fix by aligning with actual kfunc definitions.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz --- .../testing/selftests/bpf/progs/get_func_ip_test.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/get_func_ip_test.c b/tools/testing/selftests/bpf/progs/get_func_ip_test.c index 8956eb78a226..a89596f7585d 100644 --- a/tools/testing/selftests/bpf/progs/get_func_ip_test.c +++ b/tools/testing/selftests/bpf/progs/get_func_ip_test.c @@ -5,13 +5,13 @@
char _license[] SEC("license") = "GPL";
-extern const void bpf_fentry_test1 __ksym; -extern const void bpf_fentry_test2 __ksym; -extern const void bpf_fentry_test3 __ksym; -extern const void bpf_fentry_test4 __ksym; -extern const void bpf_modify_return_test __ksym; -extern const void bpf_fentry_test6 __ksym; -extern const void bpf_fentry_test7 __ksym; +extern int bpf_fentry_test1(int a) __ksym; +extern int bpf_fentry_test2(int a, __u64 b) __ksym; +extern int bpf_fentry_test3(char a, int b, __u64 c) __ksym; +extern int bpf_fentry_test4(void *a, char b, int c, __u64 d) __ksym; +extern int bpf_modify_return_test(int a, int *b) __ksym; +extern int bpf_fentry_test6(__u64 a, void *b, short c, int d, void *e, __u64 f) __ksym; +extern int bpf_fentry_test7(struct bpf_fentry_test_t *arg) __ksym;
extern bool CONFIG_X86_KERNEL_IBT __kconfig __weak;
On Sat, Jun 08, 2024 at 03:15:59PM -0600, Daniel Xu wrote:
The prototypes in progs/get_func_ip_test.c were not in line with how the actual kfuncs are defined in net/bpf/test_run.c. This causes compilation errors when kfunc prototypes are generated from BTF.
Fix by aligning with actual kfunc definitions.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz
.../testing/selftests/bpf/progs/get_func_ip_test.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/get_func_ip_test.c b/tools/testing/selftests/bpf/progs/get_func_ip_test.c index 8956eb78a226..a89596f7585d 100644 --- a/tools/testing/selftests/bpf/progs/get_func_ip_test.c +++ b/tools/testing/selftests/bpf/progs/get_func_ip_test.c @@ -5,13 +5,13 @@ char _license[] SEC("license") = "GPL"; -extern const void bpf_fentry_test1 __ksym; -extern const void bpf_fentry_test2 __ksym; -extern const void bpf_fentry_test3 __ksym; -extern const void bpf_fentry_test4 __ksym; -extern const void bpf_modify_return_test __ksym; -extern const void bpf_fentry_test6 __ksym; -extern const void bpf_fentry_test7 __ksym; +extern int bpf_fentry_test1(int a) __ksym;
hum, the only registered one as kfunc is bpf_fentry_test1, to allow fmodret also there's bpf_fentry_test9 as kfunc, which AFAICS is not really needed
jirka
[1] 5b481acab4ce bpf: do not rely on ALLOW_ERROR_INJECTION for fmod_ret
+extern int bpf_fentry_test2(int a, __u64 b) __ksym; +extern int bpf_fentry_test3(char a, int b, __u64 c) __ksym; +extern int bpf_fentry_test4(void *a, char b, int c, __u64 d) __ksym; +extern int bpf_modify_return_test(int a, int *b) __ksym; +extern int bpf_fentry_test6(__u64 a, void *b, short c, int d, void *e, __u64 f) __ksym; +extern int bpf_fentry_test7(struct bpf_fentry_test_t *arg) __ksym; extern bool CONFIG_X86_KERNEL_IBT __kconfig __weak; -- 2.44.0
On Mon, Jun 10, 2024 at 03:28:36PM GMT, Jiri Olsa wrote:
On Sat, Jun 08, 2024 at 03:15:59PM -0600, Daniel Xu wrote:
The prototypes in progs/get_func_ip_test.c were not in line with how the actual kfuncs are defined in net/bpf/test_run.c. This causes compilation errors when kfunc prototypes are generated from BTF.
Fix by aligning with actual kfunc definitions.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz
.../testing/selftests/bpf/progs/get_func_ip_test.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/get_func_ip_test.c b/tools/testing/selftests/bpf/progs/get_func_ip_test.c index 8956eb78a226..a89596f7585d 100644 --- a/tools/testing/selftests/bpf/progs/get_func_ip_test.c +++ b/tools/testing/selftests/bpf/progs/get_func_ip_test.c @@ -5,13 +5,13 @@ char _license[] SEC("license") = "GPL"; -extern const void bpf_fentry_test1 __ksym; -extern const void bpf_fentry_test2 __ksym; -extern const void bpf_fentry_test3 __ksym; -extern const void bpf_fentry_test4 __ksym; -extern const void bpf_modify_return_test __ksym; -extern const void bpf_fentry_test6 __ksym; -extern const void bpf_fentry_test7 __ksym; +extern int bpf_fentry_test1(int a) __ksym;
hum, the only registered one as kfunc is bpf_fentry_test1, to allow fmodret also there's bpf_fentry_test9 as kfunc, which AFAICS is not really needed
I think bpf_modify_return_test() is also registered. But otherwise yeah, I think I was overaggressive here. Are you thinking something like this?
diff --git a/tools/testing/selftests/bpf/progs/get_func_ip_test.c b/tools/testing/selftests/bpf/progs/get_func_ip_test.c index a89596f7585d..2011cacdeb18 100644 --- a/tools/testing/selftests/bpf/progs/get_func_ip_test.c +++ b/tools/testing/selftests/bpf/progs/get_func_ip_test.c @@ -6,12 +6,11 @@ char _license[] SEC("license") = "GPL";
extern int bpf_fentry_test1(int a) __ksym; -extern int bpf_fentry_test2(int a, __u64 b) __ksym; -extern int bpf_fentry_test3(char a, int b, __u64 c) __ksym; -extern int bpf_fentry_test4(void *a, char b, int c, __u64 d) __ksym; extern int bpf_modify_return_test(int a, int *b) __ksym; -extern int bpf_fentry_test6(__u64 a, void *b, short c, int d, void *e, __u64 f) __ksym; -extern int bpf_fentry_test7(struct bpf_fentry_test_t *arg) __ksym; + +extern const void bpf_fentry_test2 __ksym; +extern const void bpf_fentry_test3 __ksym; +extern const void bpf_fentry_test4 __ksym;
extern bool CONFIG_X86_KERNEL_IBT __kconfig __weak;
On Tue, Jun 11, 2024 at 10:58:26AM -0600, Daniel Xu wrote:
On Mon, Jun 10, 2024 at 03:28:36PM GMT, Jiri Olsa wrote:
On Sat, Jun 08, 2024 at 03:15:59PM -0600, Daniel Xu wrote:
The prototypes in progs/get_func_ip_test.c were not in line with how the actual kfuncs are defined in net/bpf/test_run.c. This causes compilation errors when kfunc prototypes are generated from BTF.
Fix by aligning with actual kfunc definitions.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz
.../testing/selftests/bpf/progs/get_func_ip_test.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/get_func_ip_test.c b/tools/testing/selftests/bpf/progs/get_func_ip_test.c index 8956eb78a226..a89596f7585d 100644 --- a/tools/testing/selftests/bpf/progs/get_func_ip_test.c +++ b/tools/testing/selftests/bpf/progs/get_func_ip_test.c @@ -5,13 +5,13 @@ char _license[] SEC("license") = "GPL"; -extern const void bpf_fentry_test1 __ksym; -extern const void bpf_fentry_test2 __ksym; -extern const void bpf_fentry_test3 __ksym; -extern const void bpf_fentry_test4 __ksym; -extern const void bpf_modify_return_test __ksym; -extern const void bpf_fentry_test6 __ksym; -extern const void bpf_fentry_test7 __ksym; +extern int bpf_fentry_test1(int a) __ksym;
hum, the only registered one as kfunc is bpf_fentry_test1, to allow fmodret also there's bpf_fentry_test9 as kfunc, which AFAICS is not really needed
I think bpf_modify_return_test() is also registered. But otherwise yeah, I think I was overaggressive here. Are you thinking something like this?
yes, looks good
diff --git a/tools/testing/selftests/bpf/progs/get_func_ip_test.c b/tools/testing/selftests/bpf/progs/get_func_ip_test.c index a89596f7585d..2011cacdeb18 100644 --- a/tools/testing/selftests/bpf/progs/get_func_ip_test.c +++ b/tools/testing/selftests/bpf/progs/get_func_ip_test.c @@ -6,12 +6,11 @@ char _license[] SEC("license") = "GPL";
extern int bpf_fentry_test1(int a) __ksym; -extern int bpf_fentry_test2(int a, __u64 b) __ksym; -extern int bpf_fentry_test3(char a, int b, __u64 c) __ksym; -extern int bpf_fentry_test4(void *a, char b, int c, __u64 d) __ksym; extern int bpf_modify_return_test(int a, int *b) __ksym; -extern int bpf_fentry_test6(__u64 a, void *b, short c, int d, void *e, __u64 f) __ksym; -extern int bpf_fentry_test7(struct bpf_fentry_test_t *arg) __ksym;
I did not realize bpf_fentry_test6/7 are not used.. ok
thanks, jirka
+extern const void bpf_fentry_test2 __ksym; +extern const void bpf_fentry_test3 __ksym; +extern const void bpf_fentry_test4 __ksym;
extern bool CONFIG_X86_KERNEL_IBT __kconfig __weak;
The prototype in progs/nested_trust_common.h is not in line with how the actual kfuncs are defined in kernel/bpf/cpumask.c. This causes compilation errors when kfunc prototypes are generated from BTF.
Fix by aligning with actual kfunc definitions.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz --- tools/testing/selftests/bpf/progs/nested_trust_common.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/progs/nested_trust_common.h b/tools/testing/selftests/bpf/progs/nested_trust_common.h index 83d33931136e..1784b496be2e 100644 --- a/tools/testing/selftests/bpf/progs/nested_trust_common.h +++ b/tools/testing/selftests/bpf/progs/nested_trust_common.h @@ -7,6 +7,6 @@ #include <stdbool.h>
bool bpf_cpumask_test_cpu(unsigned int cpu, const struct cpumask *cpumask) __ksym; -bool bpf_cpumask_first_zero(const struct cpumask *cpumask) __ksym; +__u32 bpf_cpumask_first_zero(const struct cpumask *cpumask) __ksym;
#endif /* _NESTED_TRUST_COMMON_H */
The prototype in progs/map_percpu_stats.c is not in line with how the actual kfuncs are defined in kernel/bpf/map_iter.c. This causes compilation errors when kfunc prototypes are generated from BTF.
Fix by aligning with actual kfunc definitions.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz --- tools/testing/selftests/bpf/progs/map_percpu_stats.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/progs/map_percpu_stats.c b/tools/testing/selftests/bpf/progs/map_percpu_stats.c index 10b2325c1720..63245785eb69 100644 --- a/tools/testing/selftests/bpf/progs/map_percpu_stats.c +++ b/tools/testing/selftests/bpf/progs/map_percpu_stats.c @@ -7,7 +7,7 @@
__u32 target_id;
-__s64 bpf_map_sum_elem_count(struct bpf_map *map) __ksym; +__s64 bpf_map_sum_elem_count(const struct bpf_map *map) __ksym;
SEC("iter/bpf_map") int dump_bpf_map(struct bpf_iter__bpf_map *ctx)
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; #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;
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 :-\
could we rather change the bpf_session_cookie function to return long? should be just return value type change
thanks, jirka
#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; -- 2.44.0
Hi Jiri,
On Mon, Jun 10, 2024 at 03:04:25PM GMT, Jiri Olsa 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 :-\
could we rather change the bpf_session_cookie function to return long? should be just return value type change
Sounds reasonable to me. I don't think the kfunc has made it to a release yet, so perhaps if we extract this commit out as a fix to bpf tree it can still make it into 6.10. That way we won't have to worry about any ABI changes.
Thanks, Daniel
On Tue, Jun 11, 2024 at 8:54 AM Daniel Xu dxu@dxuuu.xyz wrote:
Hi Jiri,
On Mon, Jun 10, 2024 at 03:04:25PM GMT, Jiri Olsa 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 :-\
could we rather change the bpf_session_cookie function to return long? should be just return value type change
Sounds reasonable to me. I don't think the kfunc has made it to a release yet, so perhaps if we extract this commit out as a fix to bpf tree it can still make it into 6.10. That way we won't have to worry about any ABI changes.
kfunc-s can be changed at any time. Keep targeting bpf-next for everything.
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.
What am I missing?
could we rather change the bpf_session_cookie function to return long? should be just return value type change
thanks, jirka
#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;-- 2.44.0
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:
https://lore.kernel.org/bpf/CAEf4BzbyQpKvZS-mUECLRq3gyBJbsqQghOKyAbutoB76mJM...
jirka
What am I missing?
could we rather change the bpf_session_cookie function to return long? should be just return value type change
thanks, jirka
#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;-- 2.44.0
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.
https://lore.kernel.org/bpf/CAEf4BzbyQpKvZS-mUECLRq3gyBJbsqQghOKyAbutoB76mJM...
jirka
What am I missing?
could we rather change the bpf_session_cookie function to return long? should be just return value type change
thanks, jirka
#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;-- 2.44.0
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
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;
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;
With generated kfunc prototypes, the existing callback names will conflict. Fix by namespacing with a bpf_ prefix.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz --- tools/testing/selftests/bpf/progs/bpf_dctcp.c | 36 +++++++++---------- 1 file changed, 18 insertions(+), 18 deletions(-)
diff --git a/tools/testing/selftests/bpf/progs/bpf_dctcp.c b/tools/testing/selftests/bpf/progs/bpf_dctcp.c index 3c9ffe340312..02f552e7fd4d 100644 --- a/tools/testing/selftests/bpf/progs/bpf_dctcp.c +++ b/tools/testing/selftests/bpf/progs/bpf_dctcp.c @@ -65,7 +65,7 @@ static void dctcp_reset(const struct tcp_sock *tp, struct bpf_dctcp *ca) }
SEC("struct_ops") -void BPF_PROG(dctcp_init, struct sock *sk) +void BPF_PROG(bpf_dctcp_init, struct sock *sk) { const struct tcp_sock *tp = tcp_sk(sk); struct bpf_dctcp *ca = inet_csk_ca(sk); @@ -77,7 +77,7 @@ void BPF_PROG(dctcp_init, struct sock *sk) (void *)fallback, sizeof(fallback)) == -EBUSY) ebusy_cnt++;
- /* Switch back to myself and the recurred dctcp_init() + /* Switch back to myself and the recurred bpf_dctcp_init() * will get -EBUSY for all bpf_setsockopt(TCP_CONGESTION), * except the last "cdg" one. */ @@ -112,7 +112,7 @@ void BPF_PROG(dctcp_init, struct sock *sk) }
SEC("struct_ops") -__u32 BPF_PROG(dctcp_ssthresh, struct sock *sk) +__u32 BPF_PROG(bpf_dctcp_ssthresh, struct sock *sk) { struct bpf_dctcp *ca = inet_csk_ca(sk); struct tcp_sock *tp = tcp_sk(sk); @@ -122,7 +122,7 @@ __u32 BPF_PROG(dctcp_ssthresh, struct sock *sk) }
SEC("struct_ops") -void BPF_PROG(dctcp_update_alpha, struct sock *sk, __u32 flags) +void BPF_PROG(bpf_dctcp_update_alpha, struct sock *sk, __u32 flags) { const struct tcp_sock *tp = tcp_sk(sk); struct bpf_dctcp *ca = inet_csk_ca(sk); @@ -161,12 +161,12 @@ static void dctcp_react_to_loss(struct sock *sk) }
SEC("struct_ops") -void BPF_PROG(dctcp_state, struct sock *sk, __u8 new_state) +void BPF_PROG(bpf_dctcp_state, struct sock *sk, __u8 new_state) { if (new_state == TCP_CA_Recovery && new_state != BPF_CORE_READ_BITFIELD(inet_csk(sk), icsk_ca_state)) dctcp_react_to_loss(sk); - /* We handle RTO in dctcp_cwnd_event to ensure that we perform only + /* We handle RTO in bpf_dctcp_cwnd_event to ensure that we perform only * one loss-adjustment per RTT. */ } @@ -208,7 +208,7 @@ static void dctcp_ece_ack_update(struct sock *sk, enum tcp_ca_event evt, }
SEC("struct_ops") -void BPF_PROG(dctcp_cwnd_event, struct sock *sk, enum tcp_ca_event ev) +void BPF_PROG(bpf_dctcp_cwnd_event, struct sock *sk, enum tcp_ca_event ev) { struct bpf_dctcp *ca = inet_csk_ca(sk);
@@ -227,7 +227,7 @@ void BPF_PROG(dctcp_cwnd_event, struct sock *sk, enum tcp_ca_event ev) }
SEC("struct_ops") -__u32 BPF_PROG(dctcp_cwnd_undo, struct sock *sk) +__u32 BPF_PROG(bpf_dctcp_cwnd_undo, struct sock *sk) { const struct bpf_dctcp *ca = inet_csk_ca(sk);
@@ -237,28 +237,28 @@ __u32 BPF_PROG(dctcp_cwnd_undo, struct sock *sk) extern void tcp_reno_cong_avoid(struct sock *sk, __u32 ack, __u32 acked) __ksym;
SEC("struct_ops") -void BPF_PROG(dctcp_cong_avoid, struct sock *sk, __u32 ack, __u32 acked) +void BPF_PROG(bpf_dctcp_cong_avoid, struct sock *sk, __u32 ack, __u32 acked) { tcp_reno_cong_avoid(sk, ack, acked); }
SEC(".struct_ops") struct tcp_congestion_ops dctcp_nouse = { - .init = (void *)dctcp_init, - .set_state = (void *)dctcp_state, + .init = (void *)bpf_dctcp_init, + .set_state = (void *)bpf_dctcp_state, .flags = TCP_CONG_NEEDS_ECN, .name = "bpf_dctcp_nouse", };
SEC(".struct_ops") struct tcp_congestion_ops dctcp = { - .init = (void *)dctcp_init, - .in_ack_event = (void *)dctcp_update_alpha, - .cwnd_event = (void *)dctcp_cwnd_event, - .ssthresh = (void *)dctcp_ssthresh, - .cong_avoid = (void *)dctcp_cong_avoid, - .undo_cwnd = (void *)dctcp_cwnd_undo, - .set_state = (void *)dctcp_state, + .init = (void *)bpf_dctcp_init, + .in_ack_event = (void *)bpf_dctcp_update_alpha, + .cwnd_event = (void *)bpf_dctcp_cwnd_event, + .ssthresh = (void *)bpf_dctcp_ssthresh, + .cong_avoid = (void *)bpf_dctcp_cong_avoid, + .undo_cwnd = (void *)bpf_dctcp_cwnd_undo, + .set_state = (void *)bpf_dctcp_state, .flags = TCP_CONG_NEEDS_ECN, .name = "bpf_dctcp", };
Previously, kfunc declarations in bpf_kfuncs.h (and others) used "user facing" types for kfuncs prototypes while the actual kfunc definitions used "kernel facing" types. More specifically: bpf_dynptr vs bpf_dynptr_kern, __sk_buff vs sk_buff, and xdp_md vs xdp_buff.
It wasn't an issue before, as the verifier allows aliased types. However, since we are now generating kfunc prototypes in vmlinux.h (in addition to keeping bpf_kfuncs.h around), this conflict creates compilation errors.
Fix this conflict by using "user facing" types in kfunc definitions. This results in more casts, but otherwise has no additional runtime cost.
Note, similar to 5b268d1ebcdc ("bpf: Have bpf_rdonly_cast() take a const pointer"), we also make kfuncs take const arguments where appropriate in order to make the kfunc more permissive.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz --- fs/verity/measure.c | 3 +- include/linux/bpf.h | 8 ++-- kernel/bpf/crypto.c | 24 ++++++++---- kernel/bpf/helpers.c | 39 +++++++++++++------ kernel/bpf/verifier.c | 2 +- kernel/trace/bpf_trace.c | 9 +++-- net/core/filter.c | 32 +++++++++------ .../selftests/bpf/progs/ip_check_defrag.c | 10 ++--- .../bpf/progs/verifier_netfilter_ctx.c | 6 +-- 9 files changed, 84 insertions(+), 49 deletions(-)
diff --git a/fs/verity/measure.c b/fs/verity/measure.c index 3969d54158d1..7af0f7f8a6f3 100644 --- a/fs/verity/measure.c +++ b/fs/verity/measure.c @@ -117,8 +117,9 @@ __bpf_kfunc_start_defs(); * * Return: 0 on success, a negative value on error. */ -__bpf_kfunc int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr_kern *digest_ptr) +__bpf_kfunc int bpf_get_fsverity_digest(struct file *file, struct bpf_dynptr *digest_p) { + struct bpf_dynptr_kern *digest_ptr = (struct bpf_dynptr_kern *)digest_p; const struct inode *inode = file_inode(file); u32 dynptr_sz = __bpf_dynptr_size(digest_ptr); struct fsverity_digest *arg; diff --git a/include/linux/bpf.h b/include/linux/bpf.h index a834f4b761bc..f636b4998bf7 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -3265,8 +3265,8 @@ u32 bpf_sock_convert_ctx_access(enum bpf_access_type type, struct bpf_insn *insn_buf, struct bpf_prog *prog, u32 *target_size); -int bpf_dynptr_from_skb_rdonly(struct sk_buff *skb, u64 flags, - struct bpf_dynptr_kern *ptr); +int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags, + struct bpf_dynptr *ptr); #else static inline bool bpf_sock_common_is_valid_access(int off, int size, enum bpf_access_type type, @@ -3288,8 +3288,8 @@ static inline u32 bpf_sock_convert_ctx_access(enum bpf_access_type type, { return 0; } -static inline int bpf_dynptr_from_skb_rdonly(struct sk_buff *skb, u64 flags, - struct bpf_dynptr_kern *ptr) +static inline int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags, + struct bpf_dynptr *ptr) { return -EOPNOTSUPP; } diff --git a/kernel/bpf/crypto.c b/kernel/bpf/crypto.c index 2bee4af91e38..3c1de0e5c0bd 100644 --- a/kernel/bpf/crypto.c +++ b/kernel/bpf/crypto.c @@ -311,11 +311,15 @@ static int bpf_crypto_crypt(const struct bpf_crypto_ctx *ctx, * Decrypts provided buffer using IV data and the crypto context. Crypto context must be configured. */ __bpf_kfunc int bpf_crypto_decrypt(struct bpf_crypto_ctx *ctx, - const struct bpf_dynptr_kern *src, - const struct bpf_dynptr_kern *dst, - const struct bpf_dynptr_kern *siv) + const struct bpf_dynptr *src, + const struct bpf_dynptr *dst, + const struct bpf_dynptr *siv) { - return bpf_crypto_crypt(ctx, src, dst, siv, true); + const struct bpf_dynptr_kern *src_kern = (struct bpf_dynptr_kern *)src; + const struct bpf_dynptr_kern *dst_kern = (struct bpf_dynptr_kern *)dst; + const struct bpf_dynptr_kern *siv_kern = (struct bpf_dynptr_kern *)siv; + + return bpf_crypto_crypt(ctx, src_kern, dst_kern, siv_kern, true); }
/** @@ -328,11 +332,15 @@ __bpf_kfunc int bpf_crypto_decrypt(struct bpf_crypto_ctx *ctx, * Encrypts provided buffer using IV data and the crypto context. Crypto context must be configured. */ __bpf_kfunc int bpf_crypto_encrypt(struct bpf_crypto_ctx *ctx, - const struct bpf_dynptr_kern *src, - const struct bpf_dynptr_kern *dst, - const struct bpf_dynptr_kern *siv) + const struct bpf_dynptr *src, + const struct bpf_dynptr *dst, + const struct bpf_dynptr *siv) { - return bpf_crypto_crypt(ctx, src, dst, siv, false); + const struct bpf_dynptr_kern *src_kern = (struct bpf_dynptr_kern *)src; + const struct bpf_dynptr_kern *dst_kern = (struct bpf_dynptr_kern *)dst; + const struct bpf_dynptr_kern *siv_kern = (struct bpf_dynptr_kern *)siv; + + return bpf_crypto_crypt(ctx, src_kern, dst_kern, siv_kern, false); }
__bpf_kfunc_end_defs(); diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 6f1abcb4b084..3ac521c48bba 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -2459,9 +2459,10 @@ __bpf_kfunc struct task_struct *bpf_task_from_pid(s32 pid) * provided buffer, with its contents containing the data, if unable to obtain * direct pointer) */ -__bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset, +__bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr *p, u32 offset, void *buffer__opt, u32 buffer__szk) { + const struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)p; enum bpf_dynptr_type type; u32 len = buffer__szk; int err; @@ -2543,9 +2544,11 @@ __bpf_kfunc void *bpf_dynptr_slice(const struct bpf_dynptr_kern *ptr, u32 offset * provided buffer, with its contents containing the data, if unable to obtain * direct pointer) */ -__bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 offset, +__bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr *p, u32 offset, void *buffer__opt, u32 buffer__szk) { + const struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)p; + if (!ptr->data || __bpf_dynptr_is_rdonly(ptr)) return NULL;
@@ -2571,11 +2574,12 @@ __bpf_kfunc void *bpf_dynptr_slice_rdwr(const struct bpf_dynptr_kern *ptr, u32 o * will be copied out into the buffer and the user will need to call * bpf_dynptr_write() to commit changes. */ - return bpf_dynptr_slice(ptr, offset, buffer__opt, buffer__szk); + return bpf_dynptr_slice(p, offset, buffer__opt, buffer__szk); }
-__bpf_kfunc int bpf_dynptr_adjust(struct bpf_dynptr_kern *ptr, u32 start, u32 end) +__bpf_kfunc int bpf_dynptr_adjust(const struct bpf_dynptr *p, u32 start, u32 end) { + struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)p; u32 size;
if (!ptr->data || start > end) @@ -2592,36 +2596,45 @@ __bpf_kfunc int bpf_dynptr_adjust(struct bpf_dynptr_kern *ptr, u32 start, u32 en return 0; }
-__bpf_kfunc bool bpf_dynptr_is_null(struct bpf_dynptr_kern *ptr) +__bpf_kfunc bool bpf_dynptr_is_null(const struct bpf_dynptr *p) { + struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)p; + return !ptr->data; }
-__bpf_kfunc bool bpf_dynptr_is_rdonly(struct bpf_dynptr_kern *ptr) +__bpf_kfunc bool bpf_dynptr_is_rdonly(const struct bpf_dynptr *p) { + struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)p; + if (!ptr->data) return false;
return __bpf_dynptr_is_rdonly(ptr); }
-__bpf_kfunc __u32 bpf_dynptr_size(const struct bpf_dynptr_kern *ptr) +__bpf_kfunc __u32 bpf_dynptr_size(const struct bpf_dynptr *p) { + struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)p; + if (!ptr->data) return -EINVAL;
return __bpf_dynptr_size(ptr); }
-__bpf_kfunc int bpf_dynptr_clone(struct bpf_dynptr_kern *ptr, - struct bpf_dynptr_kern *clone__uninit) +__bpf_kfunc int bpf_dynptr_clone(const struct bpf_dynptr *p, + struct bpf_dynptr *clone__uninit) { + struct bpf_dynptr_kern *clone = (struct bpf_dynptr_kern *)clone__uninit; + struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)p; + if (!ptr->data) { - bpf_dynptr_set_null(clone__uninit); + bpf_dynptr_set_null(clone); return -EINVAL; }
- *clone__uninit = *ptr; + *clone = *ptr;
return 0; } @@ -2986,7 +2999,9 @@ late_initcall(kfunc_init); */ const void *__bpf_dynptr_data(const struct bpf_dynptr_kern *ptr, u32 len) { - return bpf_dynptr_slice(ptr, 0, NULL, len); + const struct bpf_dynptr *p = (struct bpf_dynptr *)ptr; + + return bpf_dynptr_slice(p, 0, NULL, len); }
/* Get a pointer to dynptr data up to len bytes for read write access. If diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 0808beca3837..05491b6de213 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -10910,7 +10910,7 @@ enum { };
BTF_ID_LIST(kf_arg_btf_ids) -BTF_ID(struct, bpf_dynptr_kern) +BTF_ID(struct, bpf_dynptr) BTF_ID(struct, bpf_list_head) BTF_ID(struct, bpf_list_node) BTF_ID(struct, bpf_rb_root) diff --git a/kernel/trace/bpf_trace.c b/kernel/trace/bpf_trace.c index f5154c051d2c..58ae34b730df 100644 --- a/kernel/trace/bpf_trace.c +++ b/kernel/trace/bpf_trace.c @@ -1378,10 +1378,12 @@ __bpf_kfunc void bpf_key_put(struct bpf_key *bkey) * * Return: 0 on success, a negative value on error. */ -__bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr_kern *data_ptr, - struct bpf_dynptr_kern *sig_ptr, +__bpf_kfunc int bpf_verify_pkcs7_signature(struct bpf_dynptr *data_p, + struct bpf_dynptr *sig_p, struct bpf_key *trusted_keyring) { + struct bpf_dynptr_kern *data_ptr = (struct bpf_dynptr_kern *)data_p; + struct bpf_dynptr_kern *sig_ptr = (struct bpf_dynptr_kern *)sig_p; const void *data, *sig; u32 data_len, sig_len; int ret; @@ -1453,8 +1455,9 @@ __bpf_kfunc_start_defs(); * Return: 0 on success, a negative value on error. */ __bpf_kfunc int bpf_get_file_xattr(struct file *file, const char *name__str, - struct bpf_dynptr_kern *value_ptr) + struct bpf_dynptr *value_p) { + struct bpf_dynptr_kern *value_ptr = (struct bpf_dynptr_kern *)value_p; struct dentry *dentry; u32 value_len; void *value; diff --git a/net/core/filter.c b/net/core/filter.c index 7c46ecba3b01..73722790cee3 100644 --- a/net/core/filter.c +++ b/net/core/filter.c @@ -11859,28 +11859,34 @@ bpf_sk_base_func_proto(enum bpf_func_id func_id, const struct bpf_prog *prog) }
__bpf_kfunc_start_defs(); -__bpf_kfunc int bpf_dynptr_from_skb(struct sk_buff *skb, u64 flags, - struct bpf_dynptr_kern *ptr__uninit) +__bpf_kfunc int bpf_dynptr_from_skb(struct __sk_buff *s, u64 flags, + struct bpf_dynptr *ptr__uninit) { + struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)ptr__uninit; + struct sk_buff *skb = (struct sk_buff *)s; + if (flags) { - bpf_dynptr_set_null(ptr__uninit); + bpf_dynptr_set_null(ptr); return -EINVAL; }
- bpf_dynptr_init(ptr__uninit, skb, BPF_DYNPTR_TYPE_SKB, 0, skb->len); + bpf_dynptr_init(ptr, skb, BPF_DYNPTR_TYPE_SKB, 0, skb->len);
return 0; }
-__bpf_kfunc int bpf_dynptr_from_xdp(struct xdp_buff *xdp, u64 flags, - struct bpf_dynptr_kern *ptr__uninit) +__bpf_kfunc int bpf_dynptr_from_xdp(struct xdp_md *x, u64 flags, + struct bpf_dynptr *ptr__uninit) { + struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)ptr__uninit; + struct xdp_buff *xdp = (struct xdp_buff *)x; + if (flags) { - bpf_dynptr_set_null(ptr__uninit); + bpf_dynptr_set_null(ptr); return -EINVAL; }
- bpf_dynptr_init(ptr__uninit, xdp, BPF_DYNPTR_TYPE_XDP, 0, xdp_get_buff_len(xdp)); + bpf_dynptr_init(ptr, xdp, BPF_DYNPTR_TYPE_XDP, 0, xdp_get_buff_len(xdp));
return 0; } @@ -11906,10 +11912,11 @@ __bpf_kfunc int bpf_sock_addr_set_sun_path(struct bpf_sock_addr_kern *sa_kern, return 0; }
-__bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct sk_buff *skb, struct sock *sk, +__bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct __sk_buff *s, struct sock *sk, struct bpf_tcp_req_attrs *attrs, int attrs__sz) { #if IS_ENABLED(CONFIG_SYN_COOKIES) + struct sk_buff *skb = (struct sk_buff *)s; const struct request_sock_ops *ops; struct inet_request_sock *ireq; struct tcp_request_sock *treq; @@ -12004,16 +12011,17 @@ __bpf_kfunc int bpf_sk_assign_tcp_reqsk(struct sk_buff *skb, struct sock *sk,
__bpf_kfunc_end_defs();
-int bpf_dynptr_from_skb_rdonly(struct sk_buff *skb, u64 flags, - struct bpf_dynptr_kern *ptr__uninit) +int bpf_dynptr_from_skb_rdonly(struct __sk_buff *skb, u64 flags, + struct bpf_dynptr *ptr__uninit) { + struct bpf_dynptr_kern *ptr = (struct bpf_dynptr_kern *)ptr__uninit; int err;
err = bpf_dynptr_from_skb(skb, flags, ptr__uninit); if (err) return err;
- bpf_dynptr_set_rdonly(ptr__uninit); + bpf_dynptr_set_rdonly(ptr);
return 0; } diff --git a/tools/testing/selftests/bpf/progs/ip_check_defrag.c b/tools/testing/selftests/bpf/progs/ip_check_defrag.c index 1c2b6c1616b0..645b2c9f7867 100644 --- a/tools/testing/selftests/bpf/progs/ip_check_defrag.c +++ b/tools/testing/selftests/bpf/progs/ip_check_defrag.c @@ -12,7 +12,7 @@ #define IP_OFFSET 0x1FFF #define NEXTHDR_FRAGMENT 44
-extern int bpf_dynptr_from_skb(struct sk_buff *skb, __u64 flags, +extern int bpf_dynptr_from_skb(struct __sk_buff *skb, __u64 flags, struct bpf_dynptr *ptr__uninit) __ksym; extern void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, uint32_t offset, void *buffer, uint32_t buffer__sz) __ksym; @@ -42,7 +42,7 @@ static bool is_frag_v6(struct ipv6hdr *ip6h) return ip6h->nexthdr == NEXTHDR_FRAGMENT; }
-static int handle_v4(struct sk_buff *skb) +static int handle_v4(struct __sk_buff *skb) { struct bpf_dynptr ptr; u8 iph_buf[20] = {}; @@ -64,7 +64,7 @@ static int handle_v4(struct sk_buff *skb) return NF_ACCEPT; }
-static int handle_v6(struct sk_buff *skb) +static int handle_v6(struct __sk_buff *skb) { struct bpf_dynptr ptr; struct ipv6hdr *ip6h; @@ -89,9 +89,9 @@ static int handle_v6(struct sk_buff *skb) SEC("netfilter") int defrag(struct bpf_nf_ctx *ctx) { - struct sk_buff *skb = ctx->skb; + struct __sk_buff *skb = (struct __sk_buff *)ctx->skb;
- switch (bpf_ntohs(skb->protocol)) { + switch (bpf_ntohs(ctx->skb->protocol)) { case ETH_P_IP: return handle_v4(skb); case ETH_P_IPV6: diff --git a/tools/testing/selftests/bpf/progs/verifier_netfilter_ctx.c b/tools/testing/selftests/bpf/progs/verifier_netfilter_ctx.c index 65bba330e7e5..ab9f9f2620ed 100644 --- a/tools/testing/selftests/bpf/progs/verifier_netfilter_ctx.c +++ b/tools/testing/selftests/bpf/progs/verifier_netfilter_ctx.c @@ -79,7 +79,7 @@ int with_invalid_ctx_access_test5(struct bpf_nf_ctx *ctx) return NF_ACCEPT; }
-extern int bpf_dynptr_from_skb(struct sk_buff *skb, __u64 flags, +extern int bpf_dynptr_from_skb(struct __sk_buff *skb, __u64 flags, struct bpf_dynptr *ptr__uninit) __ksym; extern void *bpf_dynptr_slice(const struct bpf_dynptr *ptr, uint32_t offset, void *buffer, uint32_t buffer__sz) __ksym; @@ -90,8 +90,8 @@ __success __failure_unpriv __retval(0) int with_valid_ctx_access_test6(struct bpf_nf_ctx *ctx) { + struct __sk_buff *skb = (struct __sk_buff *)ctx->skb; const struct nf_hook_state *state = ctx->state; - struct sk_buff *skb = ctx->skb; const struct iphdr *iph; const struct tcphdr *th; u8 buffer_iph[20] = {}; @@ -99,7 +99,7 @@ int with_valid_ctx_access_test6(struct bpf_nf_ctx *ctx) struct bpf_dynptr ptr; uint8_t ihl;
- if (skb->len <= 20 || bpf_dynptr_from_skb(skb, 0, &ptr)) + if (ctx->skb->len <= 20 || bpf_dynptr_from_skb(skb, 0, &ptr)) return NF_ACCEPT;
iph = bpf_dynptr_slice(&ptr, 0, buffer_iph, sizeof(buffer_iph));
The bpf-nf selftests play various games with aliased types such that folks with CONFIG_NF_CONNTRACK=m/n configs can still build the selftests. See commits:
1058b6a78db2 ("selftests/bpf: Do not fail build if CONFIG_NF_CONNTRACK=m/n") 92afc5329a5b ("selftests/bpf: Fix build errors if CONFIG_NF_CONNTRACK=m")
Thus, it is simpler if these selftests opt out of using generated kfunc prototypes. The preprocessor macro this commit uses will be introduced in the final commit.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz --- tools/testing/selftests/bpf/progs/test_bpf_nf.c | 1 + tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c | 1 + tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c | 1 + 3 files changed, 3 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf.c b/tools/testing/selftests/bpf/progs/test_bpf_nf.c index 0289d8ce2b80..f7b330ddd007 100644 --- a/tools/testing/selftests/bpf/progs/test_bpf_nf.c +++ b/tools/testing/selftests/bpf/progs/test_bpf_nf.c @@ -1,4 +1,5 @@ // SPDX-License-Identifier: GPL-2.0 +#define BPF_NO_KFUNC_PROTOTYPES #include <vmlinux.h> #include <bpf/bpf_helpers.h> #include <bpf/bpf_endian.h> diff --git a/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c b/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c index 0e4759ab38ff..a586f087ffeb 100644 --- a/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c +++ b/tools/testing/selftests/bpf/progs/test_bpf_nf_fail.c @@ -1,4 +1,5 @@ // SPDX-License-Identifier: GPL-2.0 +#define BPF_NO_KFUNC_PROTOTYPES #include <vmlinux.h> #include <bpf/bpf_tracing.h> #include <bpf/bpf_helpers.h> diff --git a/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c b/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c index 7ea9785738b5..f8f5dc9f72b8 100644 --- a/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c +++ b/tools/testing/selftests/bpf/progs/xdp_synproxy_kern.c @@ -1,6 +1,7 @@ // SPDX-License-Identifier: LGPL-2.1 OR BSD-2-Clause /* Copyright (c) 2022, NVIDIA CORPORATION & AFFILIATES. All rights reserved. */
+#define BPF_NO_KFUNC_PROTOTYPES #include "vmlinux.h"
#include <bpf/bpf_helpers.h>
The xfrm_info selftest locally defines an aliased type such that folks with CONFIG_XFRM_INTERFACE=m/n configs can still build the selftests. See commit aa67961f3243 ("selftests/bpf: Allow building bpf tests with CONFIG_XFRM_INTERFACE=[m|n]").
Thus, it is simpler if this selftest opts out of using enerated kfunc prototypes. The preprocessor macro this commit uses will be introduced in the final commit.
Signed-off-by: Daniel Xu dxu@dxuuu.xyz --- tools/testing/selftests/bpf/progs/xfrm_info.c | 1 + 1 file changed, 1 insertion(+)
diff --git a/tools/testing/selftests/bpf/progs/xfrm_info.c b/tools/testing/selftests/bpf/progs/xfrm_info.c index f6a501fbba2b..a1d9f106c3f0 100644 --- a/tools/testing/selftests/bpf/progs/xfrm_info.c +++ b/tools/testing/selftests/bpf/progs/xfrm_info.c @@ -1,4 +1,5 @@ // SPDX-License-Identifier: GPL-2.0 +#define BPF_NO_KFUNC_PROTOTYPES #include "vmlinux.h" #include "bpf_tracing_net.h" #include <bpf/bpf_helpers.h>
On Sat, Jun 08, 2024 at 03:15:56PM GMT, Daniel Xu wrote:
This patchset enables both detecting as well as dumping compilable prototypes for kfuncs.
The first commit instructs pahole to DECL_TAG kfuncs when available. This requires v1.27 or newer. v1.27 is nearing release at time of writing. Following this, users will be able to look at BTF inside vmlinux (or modules) and check if the kfunc they want is available.
The final commit teaches bpftool how to dump kfunc prototypes. This is done for developer convenience.
The rest of the commits are fixups to enable selftests to use the newly dumped kfunc prototypes. With these, selftests will regularly exercise the newly added codepaths.
I tested that this patchset works for both pahole:
<1.27: https://github.com/kernel-patches/bpf/pull/7168 >=1.27: https://github.com/kernel-patches/bpf/pull/7163
I meant to include that in the cover letter but I forgot. I'll try to remember next time.
linux-kselftest-mirror@lists.linaro.org
-
Alexei Starovoitov -
Andrii Nakryiko -
Daniel Xu -
Jiri Olsa