-
Notifications
You must be signed in to change notification settings - Fork 154
bpf: regression-test non-JIT patch stack #9798
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Closed
guidosarducci
wants to merge
78
commits into
kernel-patches:bpf-next_base
from
guidosarducci:bpf-next-test-32-bit
Closed
bpf: regression-test non-JIT patch stack #9798
guidosarducci
wants to merge
78
commits into
kernel-patches:bpf-next_base
from
guidosarducci:bpf-next-test-32-bit
+1,754
−812
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
cb8be49
to
894c6e4
Compare
e94d243
to
7ac014f
Compare
894c6e4
to
9b87644
Compare
eddc6bf
to
0ef7fc8
Compare
9b87644
to
cc05015
Compare
0ef7fc8
to
1bd67e0
Compare
cc05015
to
15f204b
Compare
84ae2d5
to
04d481e
Compare
15f204b
to
a5ad6e5
Compare
c7692c9
to
045005b
Compare
Add a function and a command to iterate over radix tree contents. Duplicate the C implementation in Python, but drop support for tagging. Signed-off-by: Ilya Leoshkevich <[email protected]>
One can debug BPF programs with QEMU gdbstub by setting a breakpoint on bpf_prog_kallsyms_add(), waiting for a hit with a matching aux.name, and then setting a breakpoint on bpf_func. This is tedious, error-prone, and also lacks line numbers. Automate this in a way similar to the existing support for modules in lx-symbols. Enumerate and monitor changes to both BPF kallsyms and JITed progs. For each ksym, generate and compile a synthetic .s file containing the name, code, and size. In addition, if this ksym is also a prog, and not a trampoline, add line number information. Ensure that this is a no-op if the kernel is built without BPF support or if "as" is missing. In theory the "as" dependency may be dropped by generating the synthetic .o file manually, but this is too much complexity for too little benefit. Now one can debug BPF progs out of the box like this: (gdb) lx-symbols (gdb) b bpf_prog_4e612a6a881a086b_arena_list_add Breakpoint 2 (bpf_prog_4e612a6a881a086b_arena_list_add) pending. # ./test_progs -t arena_list Thread 4 hit Breakpoint 2, bpf_prog_4e612a6a881a086b_arena_list_add () at linux/tools/testing/selftests/bpf/progs/arena_list.c:51 51 list_head = &global_head; (gdb) n bpf_prog_4e612a6a881a086b_arena_list_add () at linux/tools/testing/selftests/bpf/progs/arena_list.c:53 53 for (i = zero; i < cnt && can_loop; i++) { This also works for subprogs. Signed-off-by: Ilya Leoshkevich <[email protected]>
Replace usage of glibc extension <error.h> and error() with <err.h>, which is more widely supported and works across glibc and musl libc. Fixes: 50b3ed5 ("selftests/bpf: test bpf flow dissection") Fixes: bf0f0fd ("selftests/bpf: add simple BPF_PROG_TEST_RUN examples for flow dissector") Fixes: 94e8f3c ("selftests: bpf: fix -Wformat-security warning for flow_dissector_load.c") Signed-off-by: Tony Ambardar <[email protected]>
Replace usage of glibc extension <error.h> and error() with <err.h>, which is more widely supported and works across glibc and musl libc. Fixes: 297a3f1 ("selftests/bpf: Simple program to dump XDP RX metadata") Fixes: a5f3a3f ("selftests/bpf: Properly enable hwtstamp in xdp_hw_metadata") Fixes: bb32347 ("selftests/bpf: xdp_hw_metadata track more timestamps") Fixes: bb6a888 ("selftests/bpf: Add options and frags to xdp_hw_metadata") Signed-off-by: Tony Ambardar <[email protected]>
Current test_verifier provides little feedback or argument validation, instead silently falling back to running all tests in case of user error or even expected use cases. Trying to do manual exploratory testing, switching between kernel versions (e.g. with varying tests), or working around problematic tests (e.g. kernel hangs/crashes) can be a frustrating experience. Rework argument parsing to be more robust and strict, and provide basic help on errors. Clamp test ranges to valid values and add an option to list available built-in tests ("-l"). Default "test_verifier" behaviour (run all tests) is unchanged and backwards-compatible. Updated examples: $ test_verifier die die die # previously ran all tests Usage: test_verifier -l | [-v|-vv] [<tst_lo> [<tst_hi>]] $ test_verifier 700 9999 # runs test subset from 700 to end Signed-off-by: Tony Ambardar <[email protected]>
Initially, the .BTF_ids section was created zero-filled and then patched with BTF IDs by resolve_btfids on the build host. Patching was done in native endianness and thus failed to work for cross-endian compile targets. This was fixed in [1] by using libelf-based translation to output patched data in target byte order. The addition of 8-byte BTF sets in [2] lead to .BTF_ids creation with both target-endian values and zero-filled data to be later patched. This again broke cross-endian compilation as the already-correct target-endian values were translated on output by libelf [1]. The problem was worked around [3] by manually converting BTF SET8 values to native endianness, so that final libelf output translation yields data in target byte order. Simplify and make the code more robust against future changes like [2] by employing libelf-based endian translation on both input and output, which is typical of libelf usage. [1]: 61e8aed ("bpf: Fix libelf endian handling in resolv_btfids") [2]: ef2c6f3 ("tools/resolve_btfids: Add support for 8-byte BTF sets") [3]: 903fad4 ("tools/resolve_btfids: Fix cross-compilation to non-host endianness") Acked-by: Eduard Zingerman <[email protected]> Acked-by: Jiri Olsa <[email protected]> Acked-by: Viktor Malik <[email protected]> Signed-off-by: Tony Ambardar <[email protected]>
While building of vmlinux employs a linker script to align the .BTF_ids section to 4 bytes, other usage leaves .BTF_ids unaligned and may lead to problems (e.g. [1]). Post-processing and libelf-based endian translation by resolve_btfids may also potentially suffer from misalignment. Update encoding macros in btf_ids.h to always align BTF ID data to 4 bytes. [1]: 3effc06 ("selftests/bpf: Fix alignment of .BTF_ids") Signed-off-by: Tony Ambardar <[email protected]>
Update to include recent changes for BTF_SET8 and kmem cache support, and correct alignment of .BTF_ids sections. Also remove the now-redundant fix introduced in 3effc06 ("selftests/bpf: Fix alignment of .BTF_ids"). CC: Jean-Philippe Brucker <[email protected]> Signed-off-by: Tony Ambardar <[email protected]>
The code assumes 64-bit usage and fails to build targeting 32-bit systems: trace_helpers.c: In function 'procmap_query': trace_helpers.c:261:24: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] 261 | q.query_addr = (__u64)addr; | ^ trace_helpers.c:262:27: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] 262 | q.vma_name_addr = (__u64)path_buf; | ^ trace_helpers.c:264:27: error: cast from pointer to integer of different size [-Werror=pointer-to-int-cast] 264 | q.build_id_addr = (__u64)build_id_buf; | ^ cc1: all warnings being treated as errors Fix by adding an intermediate (uintptr_t) cast. Fixes: 4e9e076 ("selftests/bpf: make use of PROCMAP_QUERY ioctl if available") Signed-off-by: Tony Ambardar <[email protected]>
Sources for get(peer|sock)name(4|6)_prog.c include a "<string.h>" which conflicts with a typedef in "vmlinux.h" when compiled for 32-bit systems: In file included from /usr/include/string.h:33: In file included from /usr/lib/llvm-18/lib/clang/18/include/stddef.h:77: /usr/lib/llvm-18/lib/clang/18/include/__stddef_size_t.h:18:23: error: typedef redefinition with different types ('unsigned long' vs '__kernel_size_t' (aka 'unsigned int')) 18 | typedef __SIZE_TYPE__ size_t; | ^ .../tools/testing/selftests/bpf/tools/include/vmlinux.h:20705:25: note: previous definition is here 20705 | typedef __kernel_size_t size_t; | ^ 1 error generated. The conflicts arise because "vmlinux.h" reflects the 32-bit host kernel, while "<string.h>" reflects the 64-bit BPF VM. Fix this by dropping the latter includes since they are unneeded anyway. Fixes: bc467e9 ("selftests/bpf: Expand getsockname and getpeername tests") Signed-off-by: Tony Ambardar <[email protected]>
The source includes a "<string.h>" which conflicts with a typedef in "vmlinux.h" when compiled for 32-bit systems: In file included from /usr/include/string.h:33: In file included from /usr/lib/llvm-18/lib/clang/18/include/stddef.h:77: /usr/lib/llvm-18/lib/clang/18/include/__stddef_size_t.h:18:23: error: typedef redefinition with different types ('unsigned long' vs '__kernel_size_t' (aka 'unsigned int')) 18 | typedef __SIZE_TYPE__ size_t; | ^ .../tools/testing/selftests/bpf/tools/include/vmlinux.h:20705:25: note: previous definition is here 20705 | typedef __kernel_size_t size_t; | ^ 1 error generated. The conflict arises because "vmlinux.h" reflects the 32-bit host kernel, while "<string.h>" reflects the 64-bit BPF VM. Fix this by dropping the latter include since they are unneeded anyway. Fixes: 9e7f74e ("selftests/bpf: Add uretprobe syscall call from user space test") Signed-off-by: Tony Ambardar <[email protected]>
Prior commit [0] added arena tests to the existing streams tests. As kernel arena code is not always compiled (only 64-bit), building streams tests in test_progs fails for 32-bit systems: progs/stream.c:136:21: error: incomplete definition of type 'struct bpf_arena' 136 | user_vm_start = ptr->user_vm_start; | ~~~^ .../include/vmlinux.h:34574:8: note: forward declaration of 'struct bpf_arena' 34574 | struct bpf_arena; | ^ progs/stream.c:170:21: error: incomplete definition of type 'struct bpf_arena' 170 | user_vm_start = ptr->user_vm_start; | ~~~^ .../include/vmlinux.h:34574:8: note: forward declaration of 'struct bpf_arena' 34574 | struct bpf_arena; | ^ 2 errors generated. Previous arena test code was mainly added as separate source files [1], allowing either their temporary removal as a workaround or, ultimately, exclusion via Makefile to build successfully. Follow the prior approach by separating out arena-specific stream test sources. 0: 86f2225 ("selftests/bpf: Add tests for arena fault reporting") 1: arena_spin_lock.c, arena_atomics.c, arena_htab.c, arena_htab_asm.c, arena_list.c, verifier_arena.c, verifier_arena_large.c Signed-off-by: Tony Ambardar <[email protected]>
Past commit [0] added arena LDSX tests to existing LDSX tests. As kernel arena code is not always compiled (only 64-bit), building LDSX tests in test_progs fails for 32-bit systems. Previous arena test code was mainly kept in separate source files [1], allowing either their temporary removal as a workaround or, ultimately, exclusion via Makefile to build successfully. Follow the prior approach by moving arena LDSX test code to new verifier_arena_ldsx test. 0: f616549 ("selftests: bpf: Add tests for signed loads from arena") 1: arena_spin_lock.c, arena_atomics.c, arena_htab.c, arena_htab_asm.c, arena_list.c, verifier_arena.c, verifier_arena_large.c Signed-off-by: Tony Ambardar <[email protected]>
BPF selftests always try building arena tests, but since kernel arena support is not built-in on unsupported archs, this leads to failures trying to build e.g. test_progs on 32-bit, and complicates overall testing. Past arena test code has been mainly added as separate source files, making it simpler to selectively build the tests. Update Makefile to filter out arena test sources from build rules and the tests.h header, and pass macro ENABLE_ARENA_TESTS via CFLAGS (e.g. for prog_tests/verifier.c). Signed-off-by: Tony Ambardar <[email protected]>
Test stream_deadlock currently fails on 32-bit armhf: run_subtest:PASS:obj_open_mem 0 nsec run_subtest:PASS:unexpected_load_failure 0 nsec do_prog_test_run:PASS:bpf_prog_test_run 0 nsec get_stream:FAIL:stream read unexpected stream read: actual 0 <= expected 0 run_subtest:FAIL:1201 Unexpected retval from get_stream(): 0, errno = 95 kernel-patches#399/3 stream_success/stream_deadlock:FAIL This fails because the test depends on arch_bpf_stack_walk() i.e. support for BPF Exceptions, which is only availble on x86_64, arm64 and s390x. Update the BPF source with tags for these 3 archs so the test is skipped when unsupported, as with test stream_cond_break and "may_goto" support: root@qemu-armhf:/usr/libexec/kselftests-bpf# ./test_progs -t stream kernel-patches#398/1 stream_failure/stream_vprintk_null_arg:OK kernel-patches#398/2 stream_failure/stream_vprintk_scalar_arg:OK kernel-patches#398/3 stream_failure/stream_vprintk_string_arg:OK kernel-patches#398 stream_failure:OK kernel-patches#399/1 stream_success/stream_exhaust:OK kernel-patches#399/2 stream_success/stream_cond_break:SKIP kernel-patches#399/3 stream_success/stream_deadlock:SKIP kernel-patches#399/4 stream_success/stream_syscall:OK kernel-patches#399 stream_success:OK (SKIP: 2/4) kernel-patches#400 stream_syscall:OK Summary: 3/5 PASSED, 2 SKIPPED, 0 FAILED Signed-off-by: Tony Ambardar <[email protected]>
This test_progs test fails on 32-bit armhf: root@qemu-armhf:/usr/libexec/kselftests-bpf# test_progs -a lwt_seg6local [...] test_lwt_seg6local:PASS:setup 0 nsec test_lwt_seg6local:PASS:open ns6 0 nsec test_lwt_seg6local:PASS:start server 0 nsec test_lwt_seg6local:PASS:open ns1 0 nsec test_lwt_seg6local:PASS:start client 0 nsec test_lwt_seg6local:PASS:build target addr 0 nsec test_lwt_seg6local:PASS:send packet 0 nsec test_lwt_seg6local:FAIL:receive packet unexpected receive packet: actual 4 != expected 7 kernel-patches#183 lwt_seg6local:FAIL This happens because a sendto() call mistakenly uses 'sizeof(char *)' as message length rather than the actual string ("foobar\0") size. e.g. bytes = sendto(cfd, foobar, sizeof(foobar), 0, ... This likely passed by accident till now because BPF CI only tests 64-bit targets. Fix by using strlen() to determine the message length. Fixes: 1041b8b ("selftests/bpf: lwt_seg6local: Move test to test_progs") Signed-off-by: Tony Ambardar <[email protected]>
The Fixes: commit made use of the lower 3 bits of (void *)sk->sk_user_data for flags, and refactored to simplify adding even more. This change immediately broke 32-bit usage: in BPF's reuseport_array for example, 'struct reuseport_array' has an array 'struct sock __rcu *ptrs[]' whose members must be cleared on socket close via now-broken references from sk->sk_user_data. This leads to subtle memory corruption and lock issues that result in kernel hangs and panics while running BPF selftests: root@qemu-armhf:/usr/libexec/kselftests-bpf# test_progs -a select_reuseport bpf_testmod.ko is already unloaded. Loading bpf_testmod.ko... Successfully loaded bpf_testmod.ko. test_config:PASS:netns_new 0 nsec kernel-patches#356/1 select_reuseport/reuseport_sockarray IPv4/TCP LOOPBACK test_err_inner_map:OK [...] ------------[ cut here ]------------ WARNING: CPU: 0 PID: 87 at kernel/locking/lockdep.c:238 __lock_acquire+0xac0/0xd1c DEBUG_LOCKS_WARN_ON(1) Modules linked in: bpf_testmod(OE) bpf_preload CPU: 0 UID: 0 PID: 87 Comm: test_progs Tainted: G OE 6.17.0-rc1-00233-ge37b36224f81-dirty kernel-patches#114 NONE Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE Hardware name: Generic DT based system Call trace: dump_backtrace from show_stack+0x20/0x24 r7:c01e2ebc r6:00000080 r5:60010093 r4:c14d3d80 show_stack from dump_stack_lvl+0x90/0xc0 dump_stack_lvl from dump_stack+0x18/0x1c r7:c01e2ebc r6:00000009 r5:000000ee r4:c14c5bc4 dump_stack from __warn+0x8c/0x1b4 __warn from warn_slowpath_fmt+0x130/0x1a4 r8:c01e2ebc r7:c14bd144 r6:c14c5bc4 r5:c3cad400 r4:c1cf8a04 warn_slowpath_fmt from __lock_acquire+0xac0/0xd1c r8:c2896b50 r7:00000000 r6:c58863b8 r5:c3cad400 r4:c3cadcc0 __lock_acquire from lock_acquire.part.0+0xbc/0x240 r10:00000000 r9:1c0ed000 r8:00000000 r7:60010013 r6:c1b902f0 r5:c1b902f0 r4:df865cd0 lock_acquire.part.0 from lock_acquire+0x90/0x168 r10:c5886100 r9:c46a6c04 r8:00000000 r7:00000000 r6:00000000 r5:00000000 r4:c58863b8 lock_acquire from _raw_write_lock_bh+0x54/0x90 r9:c46a6c04 r8:00000000 r7:00000055 r6:c58863b8 r5:c58863a8 r4:c0394774 _raw_write_lock_bh from bpf_fd_reuseport_array_update_elem+0x16c/0x26c r6:c59a4000 r5:c5191400 r4:c58863a8 bpf_fd_reuseport_array_update_elem from bpf_map_update_value+0x454/0x5dc r10:c329a901 r9:c329a900 r8:c1cf72f0 r7:c3cad400 r6:c595dc00 r5:00000000 r4:00000000 bpf_map_update_value from map_update_elem+0x210/0x430 r10:c329a901 r9:00000004 r8:c595df40 r7:df865ec0 r6:c329a900 r5:c46a6c00 r4:c46a6cf8 map_update_elem from __sys_bpf+0x594/0xc94 r10:00000000 r9:befb18b0 r8:00000051 r7:00000000 r6:00000002 r5:df865eb0 r4:00000020 __sys_bpf from sys_bpf+0x34/0x3c r10:00000182 r9:c3cad400 r8:c0100234 r7:00000182 r6:00000002 r5:befb18b0 r4:00000020 sys_bpf from ret_fast_syscall+0x0/0x1c Exception stack(0xdf865fa8 to 0xdf865ff0) 5fa0: 00000020 befb18b0 00000002 befb18b0 00000020 00000000 5fc0: 00000020 befb18b0 00000002 00000182 00839395 b6fa3ce0 00000000 012ac774 5fe0: befb1880 befb1870 00863133 b6ec3312 irq event stamp: 260676 hardirqs last enabled at (260676): [<c0149fac>] __local_bh_enable_ip+0xc4/0x1b0 hardirqs last disabled at (260675): [<c014a024>] __local_bh_enable_ip+0x13c/0x1b0 softirqs last enabled at (260668): [<c0a1c31c>] release_sock+0x94/0x98 softirqs last disabled at (260674): [<c03946f4>] bpf_fd_reuseport_array_update_elem+0xec/0x26c ---[ end trace 0000000000000000 ]--- Reviewing kernel usage of sk->sk_user_data and the current flag bits: #define SK_USER_DATA_NOCOPY 1UL #define SK_USER_DATA_BPF 2UL #define SK_USER_DATA_PSOCK 4UL reveals that SK_USER_DATA_PSOCK and SK_USER_DATA_BPF both imply SK_USER_DATA_NOCOPY, and suggests we can instead use an equivalent 2-bit enum like: enum sk_user_data { SK_USER_DATA_NONE = 0, SK_USER_DATA_NOCOPY = 1, SK_USER_DATA_BPF = 2, SK_USER_DATA_PSOCK = 3, }; Implement this to fix the pointer corruption, and update related call signatures and comments to clarify the change from multiple flag bits to an enum value, with a note highlighting the 2-bit limitation. Fixes: 2a01337 ("net: fix refcount bug in sk_psock_get (2)") Signed-off-by: Tony Ambardar <[email protected]>
The test employs 'sizeof(struct xdp_frame)' on the BPF side, which is not portable and results in test failures when run on 32-bit systems: root@qemu-armhf:/usr/libexec/kselftests-bpf# test_progs -a xdp_pull_data [...] run_test:PASS:calloc buf 0 nsec run_test:PASS:bpf_prog_test_run_opts 0 nsec run_test:FAIL:xdp_pull_data_prog retval unexpected xdp_pull_data_prog retval: actual 2 != expected 1 run_test:PASS:calloc buf 0 nsec run_test:PASS:bpf_prog_test_run_opts 0 nsec run_test:FAIL:xdp_pull_data_prog retval unexpected xdp_pull_data_prog retval: actual 2 != expected 1 run_test:PASS:calloc buf 0 nsec run_test:PASS:bpf_prog_test_run_opts 0 nsec run_test:FAIL:xdp_pull_data_prog retval unexpected xdp_pull_data_prog retval: actual 2 != expected 1 run_test:PASS:calloc buf 0 nsec run_test:PASS:bpf_prog_test_run_opts 0 nsec run_test:FAIL:xdp_pull_data_prog retval unexpected xdp_pull_data_prog retval: actual 2 != expected 1 kernel-patches#639/1 xdp_pull_data/xdp_pull_data:FAIL Fix by using 'bpf_core_type_size(struct xdp_frame)' instead. Fixes: 323302f ("selftests/bpf: Test bpf_xdp_pull_data") Signed-off-by: Tony Ambardar <[email protected]>
a4b9800
to
517017e
Compare
89448cf
to
2c620e5
Compare
The sources include unnecessary headers, including "<string.h>", which conflict with a typedef in "vmlinux.h" when compiled for 32-bit systems: In file included from /usr/include/string.h:33: In file included from /usr/lib/llvm-18/lib/clang/18/include/stddef.h:77: /usr/lib/llvm-18/lib/clang/18/include/__stddef_size_t.h:18:23: error: typedef redefinition with different types ('unsigned long' vs '__kernel_size_t' (aka 'unsigned int')) 18 | typedef __SIZE_TYPE__ size_t; | ^ .../tools/testing/selftests/bpf/tools/include/vmlinux.h:22723:25: note: previous definition is here 22723 | typedef __kernel_size_t size_t; | ^ 1 error generated. The conflict arises because "vmlinux.h" reflects the 32-bit host kernel, while "<string.h>" reflects the 64-bit BPF VM. Fix this by dropping the latter include since they are unneeded anyway, as well as instances of "<stdbool.h>" and "errno.h". Fixes: 39fd74d ("selftests/bpf: BPF task work scheduling tests") Signed-off-by: Tony Ambardar <[email protected]>
94422d7
to
5558007
Compare
Automatically cleaning up stale PR; feel free to reopen if needed |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.