- 29 Sep, 2022 1 commit
-
-
Wang Yufen authored
Convert the selftest to use the preferred ASSERT_* macros instead of the deprecated CHECK(). Signed-off-by:
Wang Yufen <wangyufen@huawei.com> Signed-off-by:
Andrii Nakryiko <andrii@kernel.org> Link: https://lore.kernel.org/bpf/1664169131-32405-2-git-send-email-wangyufen@huawei.com
-
- 28 Sep, 2022 6 commits
-
-
Andrii Nakryiko authored
Kui-Feng Lee says: ==================== Allow creating an iterator that loops through resources of one task/thread. People could only create iterators to loop through all resources of files, vma, and tasks in the system, even though they were interested in only the resources of a specific task or process. Passing the additional parameters, people can now create an iterator to go through all resources or only the resources of a task. Major Changes: - Add new parameters in bpf_iter_link_info to indicate to go through all tasks or to go through a specific task. - Change the implementations of BPF iterators of vma, files, and tasks to allow going through only the resources of a specific task. - Provide the arguments of parameterized task iterators in bpf_link_info. Differences from v10: - Check pid_alive() to avoid potential errors. Differences from v9: - Fix the boundary check of computing page_shift. - Rewording the reason of checking and returning the same task. Differences from v8: - Fix uninitialized variable. - Avoid redundant work of getting task from pid. - Change format string to use %u instead of %d. - Use the value of page_shift to compute correct offset in bpf_iter_vm_offset.c. Differences from v7: - Travel the tasks of a process through task_group linked list instead of traveling through the whole namespace. Differences from v6: - Add part 5 to make bpftool show the value of parameters. - Change of wording of show_fdinfo() to show pid or tid instead of always pid. - Simplify error handling and naming of test cases. Differences from v5: - Use user-space tid/pid terminologies in bpf_iter_link_info and bpf_link_info. - Fix reference count - Merge all variants to one 'u32 pid' in internal structs. (bpf_iter_aux_info and bpf_iter_seq_task_common) - Compare the result of get_uprobe_offset() with the implementation with the vma iterators. - Implement show_fdinfo. Differences from v4: - Remove 'type' from bpf_iter_link_info and bpf_link_info. v10: https://lore.kernel.org/all/20220831181039.2680134-1-kuifeng@fb.com/ v9: https://lore.kernel.org/bpf/20220829192317.486946-1-kuifeng@fb.com/ v8: https://lore.kernel.org/bpf/20220829192317.486946-1-kuifeng@fb.com/ v7: https://lore.kernel.org/bpf/20220826003712.2810158-1-kuifeng@fb.com/ v6: https://lore.kernel.org/bpf/20220819220927.3409575-1-kuifeng@fb.com/ v5: https://lore.kernel.org/bpf/20220811001654.1316689-1-kuifeng@fb.com/ v4: https://lore.kernel.org/bpf/20220809195429.1043220-1-kuifeng@fb.com/ v3: https://lore.kernel.org/bpf/20220809063501.667610-1-kuifeng@fb.com/ v2: https://lore.kernel.org/bpf/20220801232649.2306614-1-kuifeng@fb.com/ v1: https://lore.kernel.org/bpf/20220726051713.840431-1-kuifeng@fb.com/ ==================== Signed-off-by:
-
Kui-Feng Lee authored
Show tid or pid of iterators if giving an argument of tid or pid For example, the command `bpftool link list` may list following lines. 1: iter prog 2 target_name bpf_map 2: iter prog 3 target_name bpf_prog 33: iter prog 225 target_name task_file tid 1644 pids test_progs(1644) Link 33 is a task_file iterator with tid 1644. For now, only targets of task, task_file and task_vma may be with tid or pid to filter out tasks other than those belonging to a process (pid) or a thread (tid). Signed-off-by:Kui-Feng Lee <kuifeng@fb.com> Signed-off-by:
Quentin Monnet <quentin@isovalent.com> Acked-by:
Yonghong Song <yhs@fb.com> Acked-by:
Martin KaFai Lau <martin.lau@kernel.org> Link: https://lore.kernel.org/bpf/20220926184957.208194-6-kuifeng@fb.com
-
Kui-Feng Lee authored
Test iterators of vma, files and tasks. Ensure the API works appropriately to visit all tasks, tasks in a process, or a particular task. Signed-off-by:
-
Kui-Feng Lee authored
Show information of iterators in the respective files under /proc/<pid>/fdinfo/. For example, for a task file iterator with 1723 as the value of tid parameter, its fdinfo would look like the following lines. pos: 0 flags: 02000000 mnt_id: 14 ino: 38 link_type: iter link_id: 51 prog_tag: a590ac96db22b825 prog_id: 299 target_name: task_file task_type: TID tid: 1723 This patch add the last three fields. task_type is the type of the task parameter. TID means the iterator visit only the thread specified by tid. The value of tid in the above example is 1723. For the case of PID task_type, it means the iterator visits only threads of a process and will show the pid value of the process instead of a tid. Signed-off-by: -
Kui-Feng Lee authored
Add new fields to bpf_link_info that users can query it through bpf_obj_get_info_by_fd(). Signed-off-by:
-
Kui-Feng Lee authored
Allow creating an iterator that loops through resources of one thread/process. People could only create iterators to loop through all resources of files, vma, and tasks in the system, even though they were interested in only the resources of a specific task or process. Passing the additional parameters, people can now create an iterator to go through all resources or only the resources of a task. Signed-off-by:
-
- 27 Sep, 2022 13 commits
-
-
Andrii Nakryiko authored
Drop the requirement for system-wide kernel UAPI headers to provide full struct btf_enum64 definition. This is an unexpected requirement that slipped in libbpf 1.0 and put unnecessary pressure ([0]) on users to have a bleeding-edge kernel UAPI header from unreleased Linux 6.0. To achieve this, we forward declare struct btf_enum64. But that's not enough as there is btf_enum64_value() helper that expects to know the layout of struct btf_enum64. So we get a bit creative with reinterpreting memory layout as array of __u32 and accesing lo32/hi32 fields as array elements. Alternative way would be to have a local pointer variable for anonymous struct with exactly the same layout as struct btf_enum64, but that gets us into C++ compiler errors complaining about invalid type casts. So play it safe, if ugly. [0] Closes: https://github.com/libbpf/libbpf/issues/562 Fixes: d90ec262 ("libbpf: Add enum64 support for btf_dump") Reported-by:
Toke Høiland-Jørgensen <toke@toke.dk> Signed-off-by:
Daniel Borkmann <daniel@iogearbox.net> Acked-by:
-
Yauheni Kaliuta authored
Since the tests are run in a function $@ there actually contains the function arguments, not the script ones. Pass "$@" to the function as well. Fixes: 272d1f4c ("selftests: bpf: test_kmod.sh: Pass parameters to the module") Signed-off-by:
Yauheni Kaliuta <ykaliuta@redhat.com> Signed-off-by:
-
Jon Doron authored
When running rootless with special capabilities like: FOWNER / DAC_OVERRIDE / DAC_READ_SEARCH The "access" API will not make the proper check if there is really access to a file or not. >From the access man page: " The check is done using the calling process's real UID and GID, rather than the effective IDs as is done when actually attempting an operation (e.g., open(2)) on the file. Similarly, for the root user, the check uses the set of permitted capabilities rather than the set of effective capabilities; ***and for non-root users, the check uses an empty set of capabilities.*** " What that means is that for non-root user the access API will not do the proper validation if the process really has permission to a file or not. To resolve this this patch replaces all the access API calls with faccessat with AT_EACCESS flag. Signed-off-by:
Jon Doron <jond@wiz.io> Signed-off-by:
-
Alexei Starovoitov authored
Song Liu says: ==================== Changes v1 => v2: 1. Update arch_prepare_bpf_dispatcher to use a RO image and a RW buffer. (Alexei) Note: I haven't found an existing test to cover this part, so this part was tested manually (comparing the generated dispatcher is the same). Jeff Layton reported CPA W^X warning linux-next [1]. It turns out to be W^X issue with bpf trampoline and bpf dispatcher. Fix these by: 1. Use bpf_prog_pack for bpf_dispatcher; 2. Set memory permission properly with bpf trampoline. [1] https://lore.kernel.org/lkml/c84cc27c1a5031a003039748c3c099732a718aec.camel@kernel.org/ ==================== Signed-off-by:
Alexei Starovoitov <ast@kernel.org>
-
Song Liu authored
Mark the trampoline as RO+X after arch_prepare_bpf_trampoline, so that the trampoine follows W^X rule strictly. This will turn off warnings like CPA refuse W^X violation: 8000000000000163 -> 0000000000000163 range: ... Also remove bpf_jit_alloc_exec_page(), since it is not used any more. Signed-off-by:
Song Liu <song@kernel.org> Link: https://lore.kernel.org/r/20220926184739.3512547-3-song@kernel.orgSigned-off-by:
-
Song Liu authored
Allocate bpf_dispatcher with bpf_prog_pack_alloc so that bpf_dispatcher can share pages with bpf programs. arch_prepare_bpf_dispatcher() is updated to provide a RW buffer as working area for arch code to write to. This also fixes CPA W^X warnning like: CPA refuse W^X violation: 8000000000000163 -> 0000000000000163 range: ... Signed-off-by:
-
Alexei Starovoitov authored
Jiri Olsa says: ==================== Martynas reported bpf_get_func_ip returning +4 address when CONFIG_X86_KERNEL_IBT option is enabled and I found there are some failing bpf tests when this option is enabled. The CONFIG_X86_KERNEL_IBT option adds endbr instruction at the function entry, so the idea is to 'fix' entry ip for kprobe_multi and trampoline probes, because they are placed on the function entry. v5 changes: - updated uapi/linux/bpf.h headers with comment for bpf_get_func_ip returning 0 [Andrii] - added acks v4 changes: - used get_kernel_nofault to read previous instruction [Peter] - used movabs instruction in trampoline comment [Peter] - renamed fentry_ip argument in kprobe_multi_link_handler [Peter] v3 changes: - using 'unused' bpf function to get IBT config option into selftest skeleton - rebased to current bpf-next/master - added ack/review from Masami v2 changes: - change kprobes get_func_ip to return zero for kprobes attached within the function body [Andrii] - detect IBT config and properly test kprobe with offset [Andrii] v1 changes: - read previous instruction in kprobe_multi link handler and adjust entry_ip for CONFIG_X86_KERNEL_IBT option - split first patch into 2 separate changes - update changelogs ==================== Acked-by:Peter Zijlstra (Intel) <peterz@infradead.org> Signed-off-by:
-
Jiri Olsa authored
With CONFIG_X86_KERNEL_IBT enabled the test for kprobe with offset won't work because of the extra endbr instruction. As suggested by Andrii adding CONFIG_X86_KERNEL_IBT detection and using appropriate offset value based on that. Also removing test7 program, because it does the same as test6. Suggested-by:
Jiri Olsa <jolsa@kernel.org> Link: https://lore.kernel.org/r/20220926153340.1621984-7-jolsa@kernel.orgSigned-off-by:
-
Jiri Olsa authored
Changing return value of kprobe's version of bpf_get_func_ip to return zero if the attach address is not on the function's entry point. For kprobes attached in the middle of the function we can't easily get to the function address especially now with the CONFIG_X86_KERNEL_IBT support. If user cares about current IP for kprobes attached within the function body, they can get it with PT_REGS_IP(ctx). Suggested-by:
Martynas Pumputis <m@lambda.lt> Signed-off-by:
-
Jiri Olsa authored
Martynas reported bpf_get_func_ip returning +4 address when CONFIG_X86_KERNEL_IBT option is enabled. When CONFIG_X86_KERNEL_IBT is enabled we'll have endbr instruction at the function entry, which screws return value of bpf_get_func_ip() helper that should return the function address. There's short term workaround for kprobe_multi bpf program made by Alexei [1], but we need this fixup also for bpf_get_attach_cookie, that returns cookie based on the entry_ip value. Moving the fixup in the fprobe handler, so both bpf_get_func_ip and bpf_get_attach_cookie get expected function address when CONFIG_X86_KERNEL_IBT option is enabled. Also renaming kprobe_multi_link_handler entry_ip argument to fentry_ip so it's clearer this is an ftrace __fentry__ ip. [1] commit 7f0059b5 ("selftests/bpf: Fix kprobe_multi test.") Cc: Peter Zijlstra <peterz@infradead.org> Reported-by:
-
Jiri Olsa authored
Using function address given at the generation time as the trampoline ip argument. This way we get directly the function address that we need, so we don't need to: - read the ip from the stack - subtract X86_PATCH_SIZE - subtract ENDBR_INSN_SIZE if CONFIG_X86_KERNEL_IBT is enabled which is not even implemented yet ;-) Signed-off-by: -
Jiri Olsa authored
Keeping the resolved 'addr' in kallsyms_callback, instead of taking ftrace_location value, because we depend on symbol address in the cookie related code. With CONFIG_X86_KERNEL_IBT option the ftrace_location value differs from symbol address, which screwes the symbol address cookies matching. There are 2 users of this function: - bpf_kprobe_multi_link_attach for which this fix is for - get_ftrace_locations which is used by register_fprobe_syms this function needs to get symbols resolved to addresses, but does not need 'ftrace location addresses' at this point there's another ftrace location translation in the path done by ftrace_set_filter_ips call: register_fprobe_syms addrs = get_ftrace_locations register_fprobe_ips(addrs) ... ftrace_set_filter_ips ... __ftrace_match_addr ip = ftrace_location(ip); ... Reviewed-by:Masami Hiramatsu (Google) <mhiramat@kernel.org> Signed-off-by:
-
Jiri Olsa authored
Adding KPROBE_FLAG_ON_FUNC_ENTRY kprobe flag to indicate that attach address is on function entry. This is used in following changes in get_func_ip helper to return correct function address. Acked-by:
-
- 26 Sep, 2022 3 commits
-
-
Liu Jian authored
In sk_psock_backlog function, for ingress direction skb, if no new data packet arrives after the skb is cached, the cached skb does not have a chance to be added to the receive queue of psock. As a result, the cached skb cannot be received by the upper-layer application. Fix this by reschedule the psock work to dispose the cached skb in sk_msg_recvmsg function. Fixes: 604326b4 ("bpf, sockmap: convert to generic sk_msg interface") Signed-off-by:
Liu Jian <liujian56@huawei.com> Signed-off-by:
John Fastabend <john.fastabend@gmail.com> Link: https://lore.kernel.org/bpf/20220907071311.60534-1-liujian56@huawei.com
-
Liu Jian authored
Add one test for wait redirect sock's send memory test for sockmap. Signed-off-by:
-
Liu Jian authored
Fixes the below NULL pointer dereference: [...] [ 14.471200] Call Trace: [ 14.471562] <TASK> [ 14.471882] lock_acquire+0x245/0x2e0 [ 14.472416] ? remove_wait_queue+0x12/0x50 [ 14.473014] ? _raw_spin_lock_irqsave+0x17/0x50 [ 14.473681] _raw_spin_lock_irqsave+0x3d/0x50 [ 14.474318] ? remove_wait_queue+0x12/0x50 [ 14.474907] remove_wait_queue+0x12/0x50 [ 14.475480] sk_stream_wait_memory+0x20d/0x340 [ 14.476127] ? do_wait_intr_irq+0x80/0x80 [ 14.476704] do_tcp_sendpages+0x287/0x600 [ 14.477283] tcp_bpf_push+0xab/0x260 [ 14.477817] tcp_bpf_sendmsg_redir+0x297/0x500 [ 14.478461] ? __local_bh_enable_ip+0x77/0xe0 [ 14.479096] tcp_bpf_send_verdict+0x105/0x470 [ 14.479729] tcp_bpf_sendmsg+0x318/0x4f0 [ 14.480311] sock_sendmsg+0x2d/0x40 [ 14.480822] ____sys_sendmsg+0x1b4/0x1c0 [ 14.481390] ? copy_msghdr_from_user+0x62/0x80 [ 14.482048] ___sys_sendmsg+0x78/0xb0 [ 14.482580] ? vmf_insert_pfn_prot+0x91/0x150 [ 14.483215] ? __do_fault+0x2a/0x1a0 [ 14.483738] ? do_fault+0x15e/0x5d0 [ 14.484246] ? __handle_mm_fault+0x56b/0x1040 [ 14.484874] ? lock_is_held_type+0xdf/0x130 [ 14.485474] ? find_held_lock+0x2d/0x90 [ 14.486046] ? __sys_sendmsg+0x41/0x70 [ 14.486587] __sys_sendmsg+0x41/0x70 [ 14.487105] ? intel_pmu_drain_pebs_core+0x350/0x350 [ 14.487822] do_syscall_64+0x34/0x80 [ 14.488345] entry_SYSCALL_64_after_hwframe+0x63/0xcd [...] The test scenario has the following flow: thread1 thread2 ----------- --------------- tcp_bpf_sendmsg tcp_bpf_send_verdict tcp_bpf_sendmsg_redir sock_close tcp_bpf_push_locked __sock_release tcp_bpf_push //inet_release do_tcp_sendpages sock->ops->release sk_stream_wait_memory // tcp_close sk_wait_event sk->sk_prot->close release_sock(__sk); *** lock_sock(sk); __tcp_close sock_orphan(sk) sk->sk_wq = NULL release_sock **** lock_sock(__sk); remove_wait_queue(sk_sleep(sk), &wait); sk_sleep(sk) //NULL pointer dereference &rcu_dereference_raw(sk->sk_wq)->wait While waiting for memory in thread1, the socket is released with its wait queue because thread2 has closed it. This caused by tcp_bpf_send_verdict didn't increase the f_count of psock->sk_redir->sk_socket->file in thread1. We should check if SOCK_DEAD flag is set on wakeup in sk_stream_wait_memory before accessing the wait queue. Suggested-by:Jakub Sitnicki <jakub@cloudflare.com> Signed-off-by:
-
- 24 Sep, 2022 6 commits
-
-
Alexei Starovoitov authored
Andrii Nakryiko says: ==================== A small patch set adding few usability improvements and features making veristat a more convenient tool to be used for work on BPF verifier: - patch #2 speeds up and makes stats parsing from BPF verifier log more robust; - patch #3 makes veristat less strict about input object files; veristat will ignore non-BPF ELF files; - patch #4 adds progress log, by default, so that user doing mass-verification is aware that veristat is not stuck; - patch #5 allows to tune requested BPF verifier log level, which makes veristat a simplest way to get BPF verifier log, especially successfully verified ones. v1->v2: - don't emit progress in non-table mode, as it breaks CSV output. ==================== Signed-off-by: -
Andrii Nakryiko authored
Add -l (--log-level) flag to override default BPF verifier log lever. This only matters in verbose mode, which is the mode in which veristat emits verifier log for each processed BPF program. This is important because for successfully verified BPF programs log_level 1 is empty, as BPF verifier truncates all the successfully verified paths. So -l2 is the only way to actually get BPF verifier log in practice. It looks sometihng like this: [vmuser@archvm bpf]$ sudo ./veristat xdp_tx.bpf.o -vl2 Processing 'xdp_tx.bpf.o'... PROCESSING xdp_tx.bpf.o/xdp_tx, DURATION US: 19, VERDICT: success, VERIFIER LOG: func#0 @0 0: R1=ctx(off=0,imm=0) R10=fp0 ; return XDP_TX; 0: (b4) w0 = 3 ; R0_w=3 1: (95) exit verification time 19 usec stack depth 0 processed 2 insns (limit 1000000) max_states_per_insn 0 total_states 0 peak_states 0 mark_read 0 File Program Verdict Duration (us) Total insns Total states Peak states ------------ ------- ------- ------------- ----------- ------------ ----------- xdp_tx.bpf.o xdp_tx success 19 2 0 0 ------------ ------- ------- ------------- ----------- ------------ ----------- Done. Processed 1 files, 0 programs. Skipped 1 files, 0 programs. Signed-off-by:
-
Andrii Nakryiko authored
Emit "Processing <filepath>..." for each BPF object file to be processed, to show progress. But also add -q (--quiet) flag to silence such messages. Doing something more clever (like overwriting same output line) is to cumbersome and easily breakable if there is any other console output (e.g., errors from libbpf). Signed-off-by:
-
Andrii Nakryiko authored
Make veristat ignore non-BPF object files. This allows simpler mass-verification (e.g., `sudo ./veristat *.bpf.o` in selftests/bpf directory). Note that `sudo ./veristat *.o` would also work, but with selftests's multiple copies of BPF object files (.bpf.o and .bpf.linked{1,2,3}.o) it's 4x slower. Also, given some of BPF object files could be incomplete in the sense that they are meant to be statically linked into final BPF object file (like linked_maps, linked_funcs, linked_vars), note such instances in stderr, but proceed anyways. This seems like a better trade off between completely silently ignoring BPF object file and aborting mass-verification altogether. Signed-off-by: -
Andrii Nakryiko authored
Make sure veristat doesn't spend ridiculous amount of time parsing verifier stats from verifier log, especially for very large logs or truncated logs (e.g., when verifier returns -ENOSPC due to too small buffer). For this, parse lines from the end of the log and make sure we parse only up to 100 last lines, where stats should be, if at all. Suggested-by:
-
Andrii Nakryiko authored
Add sign-file to .gitignore to avoid accidentally checking it in. Signed-off-by:
-
- 23 Sep, 2022 3 commits
-
-
Andrii Nakryiko authored
When attach_prog_fd field was removed in libbpf 1.0 and replaced with `long: 0` placeholder, it actually shifted all the subsequent fields by 8 byte. This is due to `long: 0` promising to adjust next field's offset to long-aligned offset. But in this case we were already long-aligned as pin_root_path is a pointer. So `long: 0` had no effect, and thus didn't feel the gap created by removed attach_prog_fd. Non-zero bitfield should have been used instead. I validated using pahole. Originally kconfig field was at offset 40. With `long: 0` it's at offset 32, which is wrong. With this change it's back at offset 40. While technically libbpf 1.0 is allowed to break backwards compatibility and applications should have been recompiled against libbpf 1.0 headers, but given how trivial it is to preserve memory layout, let's fix this. Reported-by:
Grant Seltzer Richman <grantseltzer@gmail.com> Fixes: 146bf811 ("libbpf: remove most other deprecated high-level APIs") Signed-off-by:
-
Wang Yufen authored
Move snprintf and len check to common helper pathname_concat() to make the code simpler. Signed-off-by:
-
Yosry Ahmed authored
The cgroup_hierarchical_stats selftest is complicated. It has to be, because it tests an entire workflow of recording, aggregating, and dumping cgroup stats. However, some of the complexity is unnecessary. The test now enables the memory controller in a cgroup hierarchy, invokes reclaim, measure reclaim time, THEN uses that reclaim time to test the stats collection and aggregation. We don't need to use such a complicated stat, as the context in which the stat is collected is orthogonal. Simplify the test by using a simple stat instead of reclaim time, the total number of times a process has ever entered a cgroup. This makes the test simpler and removes the dependency on the memory controller and the memory reclaim interface. Signed-off-by:
Yosry Ahmed <yosryahmed@google.com> Signed-off-by:
KP Singh <kpsingh@kernel.org> Link: https://lore.kernel.org/bpf/20220919175330.890793-1-yosryahmed@google.com
-
- 22 Sep, 2022 8 commits
-
-
Martin KaFai Lau authored
Hou Tao says: ==================== From: Hou Tao <houtao1@huawei.com> Hi, It is just a tiny patch set aims to fix the resource leaks in test_maps after test case succeeds or is skipped. And these leaks are spotted by using address sanitizer and checking the content of /proc/$pid/fd. Please see indiviual patch for more details. Change Log: v2: * Add the missing header file unistd.h for close() (From kernel-patches/bpf) The reason Why I miss that is that -Werror is removed from Makefile when enabling clang address sanitizer. v1: * https://lore.kernel.org/bpf/20220921025855.115463-1-houtao@huaweicloud.com/T/ ==================== Signed-off-by:
-
Hou Tao authored
Free the created fd or allocated bpf_object after test case succeeds, else there will be resource leaks. Spotted by using address sanitizer and checking the content of /proc/$pid/fd directory. Signed-off-by:
Hou Tao <houtao1@huawei.com> Link: https://lore.kernel.org/r/20220921070035.2016413-3-houtao@huaweicloud.comSigned-off-by:
-
Hou Tao authored
Destroy the created skeleton when CONFIG_PREEMPT is off, else will be resource leak. Fixes: 73b97bc7 ("selftests/bpf: Test concurrent updates on bpf_task_storage_busy") Signed-off-by:
-
Yauheni Kaliuta authored
Added urandom_read shared lib is missing from the list of installed files what makes urandom_read test after `make install` or `make gen_tar` broken. Add the library to TEST_GEN_FILES. The names in the list do not contain $(OUTPUT) since it's added by lib.mk code. Fixes: 00a0fa2d ("selftests/bpf: Add urandom_read shared lib and USDTs") Signed-off-by:
-
Alexei Starovoitov authored
Andrii Nakryiko says: ==================== Add three more critical features to veristat tool, which make it sufficient for a practical work on BPF verifier: - CSV output, which allows easier programmatic post-processing of stats; - building upon CSV output, veristat now supports comparison mode, in which two previously captured CSV outputs from veristat are compared with each other in a convenient form; - flexible allow/deny filtering using globs for BPF object files and programs, allowing to narrow down target BPF programs to be verified. See individual patches for more details and examples. v1->v2: - split out double-free fix into patch #1 (Yonghong); - fixed typo in verbose flag (Quentin); - baseline and comparison stats were reversed in output table, fixed that. ==================== Signed-off-by: -
Andrii Nakryiko authored
Add -f (--filter) argument which accepts glob-based filters for narrowing down what BPF object files and programs within them should be processed by veristat. This filtering applies both to comparison and main (verification) mode. Filter can be of two forms: - file (object) filter: 'strobemeta*'; in this case all the programs within matching files are implicitly allowed (or denied, depending if it's positive or negative rule, see below); - file and prog filter: 'strobemeta*/*unroll*' will further filter programs within matching files to only allow those program names that match '*unroll*' glob. As mentioned, filters can be positive (allowlisting) and negative (denylisting). Negative filters should start with '!': '!strobemeta*' will deny any filename which basename starts with "strobemeta". Further, one extra special syntax is supported to allow more convenient use in practice. Instead of specifying rule on the command line, veristat allows to specify file that contains rules, both positive and negative, one line per one filter. This is achieved with -f @<filepath> use, where <filepath> points to a text file containing rules (negative and positive rules can be mixed). For convenience empty lines and lines starting with '#' are ignored. This feature is useful to have some pre-canned list of object files and program names that are tested repeatedly, allowing to check in a list of rules and quickly specify them on the command line. As a demonstration (and a short cut for nearest future), create a small list of "interesting" BPF object files from selftests/bpf and commit it as veristat.cfg. It currently includes 73 programs, most of which are the most complex and largest BPF programs in selftests, as judged by total verified instruction count and verifier states total. If there is overlap between positive or negative filters, negative filter takes precedence (denylisting is stronger than allowlisting). If no allow filter is specified, veristat implicitly assumes '*/*' rule. If no deny rule is specified, veristat (logically) assumes no negative filters. Also note that -f (just like -e and -s) can be specified multiple times and their effect is cumulative. Signed-off-by: -
Andrii Nakryiko authored
Add ability to compare and contrast two veristat runs, previously recorded with veristat using CSV output format. When veristat is called with -C (--compare) flag, veristat expects exactly two input files specified, both should be in CSV format. Expectation is that it's output from previous veristat runs, but as long as column names and formats match, it should just work. First CSV file is designated as a "baseline" provided, and the second one is comparison (experiment) data set. Establishing baseline matters later when calculating difference percentages, see below. Veristat parses these two CSV files and "reconstructs" verifier stats (it could be just a subset of all possible stats). File and program names are mandatory as they are used as joining key (these two "stats" are designated as "key stats" in the code). Veristat currently enforces that the set of stats recorded in both CSV has to exactly match, down to exact order. This is just a simplifying condition which can be lifted with a bit of additional pre-processing to reorded stat specs internally, which I didn't bother doing, yet. For all the non-key stats, veristat will output three columns: one for baseline data, one for comparison data, and one with an absolute and relative percentage difference. If either baseline or comparison values are missing (that is, respective CSV file doesn't have a row with *exactly* matching file and program name), those values are assumed to be empty or zero. In such case relative percentages are forced to +100% or -100% output, for consistency with a typical case. Veristat's -e (--emit) and -s (--sort) specs still apply, so even if CSV contains lots of stats, user can request to compare only a subset of them (and specify desired column order as well). Similarly, both CSV and human-readable table output is honored. Note that input is currently always expected to be CSV. Here's an example shell session, recording data for biosnoop tool on two different kernels and comparing them afterwards, outputting data in table format. # on slightly older production kernel $ sudo ./veristat biosnoop_bpf.o File Program Verdict Duration (us) Total insns Total states Peak states -------------- ------------------------ ------- ------------- ----------- ------------ ----------- biosnoop_bpf.o blk_account_io_merge_bio success 37 24 1 1 biosnoop_bpf.o blk_account_io_start failure 0 0 0 0 biosnoop_bpf.o block_rq_complete success 76 104 6 6 biosnoop_bpf.o block_rq_insert success 83 85 7 7 biosnoop_bpf.o block_rq_issue success 79 85 7 7 -------------- ------------------------ ------- ------------- ----------- ------------ ----------- Done. Processed 1 object files, 5 programs. $ sudo ./veristat ~/local/tmp/fbcode-bpf-objs/biosnoop_bpf.o -o csv > baseline.csv $ cat baseline.csv file_name,prog_name,verdict,duration,total_insns,total_states,peak_states biosnoop_bpf.o,blk_account_io_merge_bio,success,36,24,1,1 biosnoop_bpf.o,blk_account_io_start,failure,0,0,0,0 biosnoop_bpf.o,block_rq_complete,success,82,104,6,6 biosnoop_bpf.o,block_rq_insert,success,78,85,7,7 biosnoop_bpf.o,block_rq_issue,success,74,85,7,7 # on latest bpf-next kernel $ sudo ./veristat biosnoop_bpf.o File Program Verdict Duration (us) Total insns Total states Peak states -------------- ------------------------ ------- ------------- ----------- ------------ ----------- biosnoop_bpf.o blk_account_io_merge_bio success 31 24 1 1 biosnoop_bpf.o blk_account_io_start failure 0 0 0 0 biosnoop_bpf.o block_rq_complete success 76 104 6 6 biosnoop_bpf.o block_rq_insert success 83 91 7 7 biosnoop_bpf.o block_rq_issue success 74 91 7 7 -------------- ------------------------ ------- ------------- ----------- ------------ ----------- Done. Processed 1 object files, 5 programs. $ sudo ./veristat biosnoop_bpf.o -o csv > comparison.csv $ cat comparison.csv file_name,prog_name,verdict,duration,total_insns,total_states,peak_states biosnoop_bpf.o,blk_account_io_merge_bio,success,71,24,1,1 biosnoop_bpf.o,blk_account_io_start,failure,0,0,0,0 biosnoop_bpf.o,block_rq_complete,success,82,104,6,6 biosnoop_bpf.o,block_rq_insert,success,83,91,7,7 biosnoop_bpf.o,block_rq_issue,success,87,91,7,7 # now let's compare with human-readable output (note that no sudo needed) # we also ignore verification duration in this case to shortned output $ ./veristat -C baseline.csv comparison.csv -e file,prog,verdict,insns File Program Verdict (A) Verdict (B) Verdict (DIFF) Total insns (A) Total insns (B) Total insns (DIFF) -------------- ------------------------ ----------- ----------- -------------- --------------- --------------- ------------------ biosnoop_bpf.o blk_account_io_merge_bio success success MATCH 24 24 +0 (+0.00%) biosnoop_bpf.o blk_account_io_start failure failure MATCH 0 0 +0 (+100.00%) biosnoop_bpf.o block_rq_complete success success MATCH 104 104 +0 (+0.00%) biosnoop_bpf.o block_rq_insert success success MATCH 91 85 -6 (-6.59%) biosnoop_bpf.o block_rq_issue success success MATCH 91 85 -6 (-6.59%) -------------- ------------------------ ----------- ----------- -------------- --------------- --------------- ------------------ While not particularly exciting example (it turned out to be kind of hard to quickly find a nice example with significant difference just because of kernel version bump), it should demonstrate main features. Signed-off-by:
-
Andrii Nakryiko authored
Teach veristat to output results as CSV table for easier programmatic processing. Change what was --output/-o argument to now be --emit/-e. And then use --output-format/-o <fmt> to specify output format. Currently "table" and "csv" is supported, table being default. For CSV output mode veristat is using spec identifiers as column names. E.g., instead of "Total states" veristat uses "total_states" as a CSV header name. Internally veristat recognizes three formats, one of them (RESFMT_TABLE_CALCLEN) is a special format instructing veristat to calculate column widths for table output. This felt a bit cleaner and more uniform than either creating separate functions just for this. Signed-off-by:
-
