From 8e5717a6188103f98c4b9322b10a59a745bd12f6 Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Fri, 5 Jan 2024 18:48:17 +0800 Subject: [PATCH 1/5] bpf: Support inlining bpf_kptr_xchg() helper ANBZ: #28340 commit 7c05e7f3e74e7e550534d524e04d7e6f78d6fa24 upstream The motivation of inlining bpf_kptr_xchg() comes from the performance profiling of bpf memory allocator benchmark. The benchmark uses bpf_kptr_xchg() to stash the allocated objects and to pop the stashed objects for free. After inling bpf_kptr_xchg(), the performance for object free on 8-CPUs VM increases about 2%~10%. The inline also has downside: both the kasan and kcsan checks on the pointer will be unavailable. bpf_kptr_xchg() can be inlined by converting the calling of bpf_kptr_xchg() into an atomic_xchg() instruction. But the conversion depends on two conditions: 1) JIT backend supports atomic_xchg() on pointer-sized word 2) For the specific arch, the implementation of xchg is the same as atomic_xchg() on pointer-sized words. It seems most 64-bit JIT backends satisfies these two conditions. But as a precaution, defining a weak function bpf_jit_supports_ptr_xchg() to state whether such conversion is safe and only supporting inline for 64-bit host. For x86-64, it supports BPF_XCHG atomic operation and both xchg() and atomic_xchg() use arch_xchg() to implement the exchange, so enabling the inline of bpf_kptr_xchg() on x86-64 first. Reviewed-by: Eduard Zingerman Signed-off-by: Hou Tao Link: https://lore.kernel.org/r/20240105104819.3916743-2-houtao@huaweicloud.com Signed-off-by: Alexei Starovoitov Signed-off-by: Jun He --- arch/x86/net/bpf_jit_comp.c | 5 +++++ include/linux/filter.h | 1 + kernel/bpf/core.c | 10 ++++++++++ kernel/bpf/helpers.c | 1 + kernel/bpf/verifier.c | 17 +++++++++++++++++ 5 files changed, 34 insertions(+) diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index 8b9b1d2d95f2..b4503fc71360 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/arch/x86/net/bpf_jit_comp.c @@ -3171,3 +3171,8 @@ void bpf_arch_poke_desc_update(struct bpf_jit_poke_descriptor *poke, BUG_ON(ret < 0); } } + +bool bpf_jit_supports_ptr_xchg(void) +{ + return true; +} diff --git a/include/linux/filter.h b/include/linux/filter.h index c0619fd1e27f..d356580ffadd 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -918,6 +918,7 @@ bool bpf_jit_needs_zext(void); bool bpf_jit_supports_subprog_tailcalls(void); bool bpf_jit_supports_kfunc_call(void); bool bpf_jit_supports_far_kfunc_call(void); +bool bpf_jit_supports_ptr_xchg(void); bool bpf_helper_changes_pkt_data(enum bpf_func_id func_id); static inline bool bpf_dump_raw_ok(const struct cred *cred) diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index baebfdd12515..de54e1de9f40 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2957,6 +2957,16 @@ bool __weak bpf_jit_supports_far_kfunc_call(void) return false; } +/* Return TRUE if the JIT backend satisfies the following two conditions: + * 1) JIT backend supports atomic_xchg() on pointer-sized words. + * 2) Under the specific arch, the implementation of xchg() is the same + * as atomic_xchg() on pointer-sized words. + */ +bool __weak bpf_jit_supports_ptr_xchg(void) +{ + return false; +} + /* To execute LD_ABS/LD_IND instructions __bpf_prog_run() may call * skb_copy_bits(), so provide a weak definition of it for NET-less config. */ diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index 285f86babb1f..b1560c5300ea 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1498,6 +1498,7 @@ BPF_CALL_2(bpf_kptr_xchg, void *, map_value, void *, ptr) { unsigned long *kptr = map_value; + /* This helper may be inlined by verifier. */ return xchg(kptr, (unsigned long)ptr); } diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 1dfd17d2acd1..5ab3bfd4df95 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -19526,6 +19526,23 @@ static int do_misc_fixups(struct bpf_verifier_env *env) continue; } + /* Implement bpf_kptr_xchg inline */ + if (prog->jit_requested && BITS_PER_LONG == 64 && + insn->imm == BPF_FUNC_kptr_xchg && + bpf_jit_supports_ptr_xchg()) { + insn_buf[0] = BPF_MOV64_REG(BPF_REG_0, BPF_REG_2); + insn_buf[1] = BPF_ATOMIC_OP(BPF_DW, BPF_XCHG, BPF_REG_1, BPF_REG_0, 0); + cnt = 2; + + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + if (!new_prog) + return -ENOMEM; + + delta += cnt - 1; + env->prog = prog = new_prog; + insn = new_prog->insnsi + i + delta; + continue; + } patch_call_imm: fn = env->ops->get_func_proto(insn->imm, env->prog); /* all functions that have prototype and verifier allowed -- Gitee From 8b2ec7e34aa27921d21af7ce769f3c8468ebc901 Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Fri, 5 Jan 2024 18:48:18 +0800 Subject: [PATCH 2/5] selftests/bpf: Factor out get_xlated_program() helper ANBZ: #28340 commit b4b7a4099b8ccea224577003fcf9d321bf0817b7 upstream Both test_verifier and test_progs use get_xlated_program(), so moving the helper into testing_helpers.h to reuse it. Acked-by: Eduard Zingerman Signed-off-by: Hou Tao Link: https://lore.kernel.org/r/20240105104819.3916743-3-houtao@huaweicloud.com Signed-off-by: Alexei Starovoitov Signed-off-by: Jun He --- .../selftests/bpf/prog_tests/ctx_rewrite.c | 44 ----------------- tools/testing/selftests/bpf/test_verifier.c | 47 +------------------ tools/testing/selftests/bpf/testing_helpers.c | 42 +++++++++++++++++ tools/testing/selftests/bpf/testing_helpers.h | 6 +++ 4 files changed, 50 insertions(+), 89 deletions(-) diff --git a/tools/testing/selftests/bpf/prog_tests/ctx_rewrite.c b/tools/testing/selftests/bpf/prog_tests/ctx_rewrite.c index 4951aa978f33..3b7c57fe55a5 100644 --- a/tools/testing/selftests/bpf/prog_tests/ctx_rewrite.c +++ b/tools/testing/selftests/bpf/prog_tests/ctx_rewrite.c @@ -626,50 +626,6 @@ static bool match_pattern(struct btf *btf, char *pattern, char *text, char *reg_ return false; } -/* Request BPF program instructions after all rewrites are applied, - * e.g. verifier.c:convert_ctx_access() is done. - */ -static int get_xlated_program(int fd_prog, struct bpf_insn **buf, __u32 *cnt) -{ - struct bpf_prog_info info = {}; - __u32 info_len = sizeof(info); - __u32 xlated_prog_len; - __u32 buf_element_size = sizeof(struct bpf_insn); - - if (bpf_prog_get_info_by_fd(fd_prog, &info, &info_len)) { - perror("bpf_prog_get_info_by_fd failed"); - return -1; - } - - xlated_prog_len = info.xlated_prog_len; - if (xlated_prog_len % buf_element_size) { - printf("Program length %d is not multiple of %d\n", - xlated_prog_len, buf_element_size); - return -1; - } - - *cnt = xlated_prog_len / buf_element_size; - *buf = calloc(*cnt, buf_element_size); - if (!buf) { - perror("can't allocate xlated program buffer"); - return -ENOMEM; - } - - bzero(&info, sizeof(info)); - info.xlated_prog_len = xlated_prog_len; - info.xlated_prog_insns = (__u64)(unsigned long)*buf; - if (bpf_prog_get_info_by_fd(fd_prog, &info, &info_len)) { - perror("second bpf_prog_get_info_by_fd failed"); - goto out_free_buf; - } - - return 0; - -out_free_buf: - free(*buf); - return -1; -} - static void print_insn(void *private_data, const char *fmt, ...) { va_list args; diff --git a/tools/testing/selftests/bpf/test_verifier.c b/tools/testing/selftests/bpf/test_verifier.c index 98107e0452d3..b92586efcb5e 100644 --- a/tools/testing/selftests/bpf/test_verifier.c +++ b/tools/testing/selftests/bpf/test_verifier.c @@ -1341,48 +1341,6 @@ static bool cmp_str_seq(const char *log, const char *exp) return true; } -static struct bpf_insn *get_xlated_program(int fd_prog, int *cnt) -{ - __u32 buf_element_size = sizeof(struct bpf_insn); - struct bpf_prog_info info = {}; - __u32 info_len = sizeof(info); - __u32 xlated_prog_len; - struct bpf_insn *buf; - - if (bpf_prog_get_info_by_fd(fd_prog, &info, &info_len)) { - perror("bpf_prog_get_info_by_fd failed"); - return NULL; - } - - xlated_prog_len = info.xlated_prog_len; - if (xlated_prog_len % buf_element_size) { - printf("Program length %d is not multiple of %d\n", - xlated_prog_len, buf_element_size); - return NULL; - } - - *cnt = xlated_prog_len / buf_element_size; - buf = calloc(*cnt, buf_element_size); - if (!buf) { - perror("can't allocate xlated program buffer"); - return NULL; - } - - bzero(&info, sizeof(info)); - info.xlated_prog_len = xlated_prog_len; - info.xlated_prog_insns = (__u64)(unsigned long)buf; - if (bpf_prog_get_info_by_fd(fd_prog, &info, &info_len)) { - perror("second bpf_prog_get_info_by_fd failed"); - goto out_free_buf; - } - - return buf; - -out_free_buf: - free(buf); - return NULL; -} - static bool is_null_insn(struct bpf_insn *insn) { struct bpf_insn null_insn = {}; @@ -1505,7 +1463,7 @@ static void print_insn(struct bpf_insn *buf, int cnt) static bool check_xlated_program(struct bpf_test *test, int fd_prog) { struct bpf_insn *buf; - int cnt; + unsigned int cnt; bool result = true; bool check_expected = !is_null_insn(test->expected_insns); bool check_unexpected = !is_null_insn(test->unexpected_insns); @@ -1513,8 +1471,7 @@ static bool check_xlated_program(struct bpf_test *test, int fd_prog) if (!check_expected && !check_unexpected) goto out; - buf = get_xlated_program(fd_prog, &cnt); - if (!buf) { + if (get_xlated_program(fd_prog, &buf, &cnt)) { printf("FAIL: can't get xlated program\n"); result = false; goto out; diff --git a/tools/testing/selftests/bpf/testing_helpers.c b/tools/testing/selftests/bpf/testing_helpers.c index 6acffe0426f0..622c2c4245e0 100644 --- a/tools/testing/selftests/bpf/testing_helpers.c +++ b/tools/testing/selftests/bpf/testing_helpers.c @@ -387,3 +387,45 @@ int kern_sync_rcu(void) { return syscall(__NR_membarrier, MEMBARRIER_CMD_SHARED, 0, 0); } + +int get_xlated_program(int fd_prog, struct bpf_insn **buf, __u32 *cnt) +{ + __u32 buf_element_size = sizeof(struct bpf_insn); + struct bpf_prog_info info = {}; + __u32 info_len = sizeof(info); + __u32 xlated_prog_len; + + if (bpf_prog_get_info_by_fd(fd_prog, &info, &info_len)) { + perror("bpf_prog_get_info_by_fd failed"); + return -1; + } + + xlated_prog_len = info.xlated_prog_len; + if (xlated_prog_len % buf_element_size) { + printf("Program length %u is not multiple of %u\n", + xlated_prog_len, buf_element_size); + return -1; + } + + *cnt = xlated_prog_len / buf_element_size; + *buf = calloc(*cnt, buf_element_size); + if (!buf) { + perror("can't allocate xlated program buffer"); + return -ENOMEM; + } + + bzero(&info, sizeof(info)); + info.xlated_prog_len = xlated_prog_len; + info.xlated_prog_insns = (__u64)(unsigned long)*buf; + if (bpf_prog_get_info_by_fd(fd_prog, &info, &info_len)) { + perror("second bpf_prog_get_info_by_fd failed"); + goto out_free_buf; + } + + return 0; + +out_free_buf: + free(*buf); + *buf = NULL; + return -1; +} diff --git a/tools/testing/selftests/bpf/testing_helpers.h b/tools/testing/selftests/bpf/testing_helpers.h index 5b7a55136741..a7b779191a97 100644 --- a/tools/testing/selftests/bpf/testing_helpers.h +++ b/tools/testing/selftests/bpf/testing_helpers.h @@ -43,4 +43,10 @@ static inline __u64 get_time_ns(void) return (u64)t.tv_sec * 1000000000 + t.tv_nsec; } +struct bpf_insn; +/* Request BPF program instructions after all rewrites are applied, + * e.g. verifier.c:convert_ctx_access() is done. + */ +int get_xlated_program(int fd_prog, struct bpf_insn **buf, __u32 *cnt); + #endif /* __TESTING_HELPERS_H */ -- Gitee From 8479be28a777d5e952e77fb84d6e20c3f40805e2 Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Fri, 5 Jan 2024 18:48:19 +0800 Subject: [PATCH 3/5] selftests/bpf: Test the inlining of bpf_kptr_xchg() ANBZ: #28340 commit 17bda53e43bc41d881ca6a02b3c6f5376c55b3d3 upstream The test uses bpf_prog_get_info_by_fd() to obtain the xlated instructions of the program first. Since these instructions have already been rewritten by the verifier, the tests then checks whether the rewritten instructions are as expected. And to ensure LLVM generates code exactly as expected, use inline assembly and a naked function. Suggested-by: Eduard Zingerman Signed-off-by: Hou Tao Acked-by: Eduard Zingerman Link: https://lore.kernel.org/r/20240105104819.3916743-4-houtao@huaweicloud.com Signed-off-by: Alexei Starovoitov Signed-off-by: Jun He --- .../bpf/prog_tests/kptr_xchg_inline.c | 51 +++++++++++++++++++ .../selftests/bpf/progs/kptr_xchg_inline.c | 48 +++++++++++++++++ 2 files changed, 99 insertions(+) create mode 100644 tools/testing/selftests/bpf/prog_tests/kptr_xchg_inline.c create mode 100644 tools/testing/selftests/bpf/progs/kptr_xchg_inline.c diff --git a/tools/testing/selftests/bpf/prog_tests/kptr_xchg_inline.c b/tools/testing/selftests/bpf/prog_tests/kptr_xchg_inline.c new file mode 100644 index 000000000000..5a4bee1cf970 --- /dev/null +++ b/tools/testing/selftests/bpf/prog_tests/kptr_xchg_inline.c @@ -0,0 +1,51 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2023. Huawei Technologies Co., Ltd */ +#include + +#include "linux/filter.h" +#include "kptr_xchg_inline.skel.h" + +void test_kptr_xchg_inline(void) +{ + struct kptr_xchg_inline *skel; + struct bpf_insn *insn = NULL; + struct bpf_insn exp; + unsigned int cnt; + int err; + +#if !defined(__x86_64__) + test__skip(); + return; +#endif + + skel = kptr_xchg_inline__open_and_load(); + if (!ASSERT_OK_PTR(skel, "open_load")) + return; + + err = get_xlated_program(bpf_program__fd(skel->progs.kptr_xchg_inline), &insn, &cnt); + if (!ASSERT_OK(err, "prog insn")) + goto out; + + /* The original instructions are: + * r1 = map[id:xxx][0]+0 + * r2 = 0 + * call bpf_kptr_xchg#yyy + * + * call bpf_kptr_xchg#yyy will be inlined as: + * r0 = r2 + * r0 = atomic64_xchg((u64 *)(r1 +0), r0) + */ + if (!ASSERT_GT(cnt, 5, "insn cnt")) + goto out; + + exp = BPF_MOV64_REG(BPF_REG_0, BPF_REG_2); + if (!ASSERT_OK(memcmp(&insn[3], &exp, sizeof(exp)), "mov")) + goto out; + + exp = BPF_ATOMIC_OP(BPF_DW, BPF_XCHG, BPF_REG_1, BPF_REG_0, 0); + if (!ASSERT_OK(memcmp(&insn[4], &exp, sizeof(exp)), "xchg")) + goto out; +out: + free(insn); + kptr_xchg_inline__destroy(skel); +} diff --git a/tools/testing/selftests/bpf/progs/kptr_xchg_inline.c b/tools/testing/selftests/bpf/progs/kptr_xchg_inline.c new file mode 100644 index 000000000000..2414ac20b6d5 --- /dev/null +++ b/tools/testing/selftests/bpf/progs/kptr_xchg_inline.c @@ -0,0 +1,48 @@ +// SPDX-License-Identifier: GPL-2.0 +/* Copyright (C) 2023. Huawei Technologies Co., Ltd */ +#include +#include + +#include "bpf_experimental.h" +#include "bpf_misc.h" + +char _license[] SEC("license") = "GPL"; + +struct bin_data { + char blob[32]; +}; + +#define private(name) SEC(".bss." #name) __hidden __attribute__((aligned(8))) +private(kptr) struct bin_data __kptr * ptr; + +SEC("tc") +__naked int kptr_xchg_inline(void) +{ + asm volatile ( + "r1 = %[ptr] ll;" + "r2 = 0;" + "call %[bpf_kptr_xchg];" + "if r0 == 0 goto 1f;" + "r1 = r0;" + "r2 = 0;" + "call %[bpf_obj_drop_impl];" + "1:" + "r0 = 0;" + "exit;" + : + : __imm_addr(ptr), + __imm(bpf_kptr_xchg), + __imm(bpf_obj_drop_impl) + : __clobber_all + ); +} + +/* BTF FUNC records are not generated for kfuncs referenced + * from inline assembly. These records are necessary for + * libbpf to link the program. The function below is a hack + * to ensure that BTF FUNC records are generated. + */ +void __btf_root(void) +{ + bpf_obj_drop(NULL); +} -- Gitee From b80d3896f97053c7af811c2cccf0264d26490121 Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Fri, 19 Jan 2024 18:25:28 +0800 Subject: [PATCH 4/5] bpf, arm64: Enable the inline of bpf_kptr_xchg() ANBZ: #28340 commit 18a45f12d746c06b7361b0cce59cf8e8b9e38da6 upstream ARM64 bpf jit satisfies the following two conditions: 1) support BPF_XCHG() on pointer-sized word. 2) the implementation of xchg is the same as atomic_xchg() on pointer-sized words. Both of these two functions use arch_xchg() to implement the exchange. So enable the inline of bpf_kptr_xchg() for arm64 bpf jit. Signed-off-by: Hou Tao Link: https://lore.kernel.org/r/20240119102529.99581-2-houtao@huaweicloud.com Signed-off-by: Alexei Starovoitov Signed-off-by: Jun He --- arch/arm64/net/bpf_jit_comp.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/arch/arm64/net/bpf_jit_comp.c b/arch/arm64/net/bpf_jit_comp.c index d8012d1a2e15..77c4b243aea7 100644 --- a/arch/arm64/net/bpf_jit_comp.c +++ b/arch/arm64/net/bpf_jit_comp.c @@ -2358,3 +2358,8 @@ int bpf_arch_text_poke(void *ip, enum bpf_text_poke_type poke_type, return ret; } + +bool bpf_jit_supports_ptr_xchg(void) +{ + return true; +} -- Gitee From 463cbba22cc99170c0881cfc24c25bf6e44b5c1d Mon Sep 17 00:00:00 2001 From: Hou Tao Date: Fri, 19 Jan 2024 18:25:29 +0800 Subject: [PATCH 5/5] selftests/bpf: Enable kptr_xchg_inline test for arm64 ANBZ: #28340 commit 29f868887a7dd3efc6faecc6fc91b28fc25cf5b0 upstream Now arm64 bpf jit has enable bpf_jit_supports_ptr_xchg(), so enable the test for arm64 as well. Signed-off-by: Hou Tao Link: https://lore.kernel.org/r/20240119102529.99581-3-houtao@huaweicloud.com Signed-off-by: Alexei Starovoitov Signed-off-by: Jun He --- tools/testing/selftests/bpf/prog_tests/kptr_xchg_inline.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/testing/selftests/bpf/prog_tests/kptr_xchg_inline.c b/tools/testing/selftests/bpf/prog_tests/kptr_xchg_inline.c index 5a4bee1cf970..15144943e88b 100644 --- a/tools/testing/selftests/bpf/prog_tests/kptr_xchg_inline.c +++ b/tools/testing/selftests/bpf/prog_tests/kptr_xchg_inline.c @@ -13,7 +13,7 @@ void test_kptr_xchg_inline(void) unsigned int cnt; int err; -#if !defined(__x86_64__) +#if !(defined(__x86_64__) || defined(__aarch64__)) test__skip(); return; #endif -- Gitee