- 21 Apr, 2022 10 commits
-
-
Artem Savkov authored
Switching to libbpf 1.0 API broke test_lpm_map and test_lru_map as error reporting changed. Instead of setting errno and returning -1 bpf calls now return -Exxx directly. Drop errno checks and look at return code directly. Fixes: b858ba8c ("selftests/bpf: Use libbpf 1.0 API mode instead of RLIMIT_MEMLOCK") Signed-off-by:
Artem Savkov <asavkov@redhat.com> Signed-off-by:
Andrii Nakryiko <andrii@kernel.org> Reviewed-by:
Yafang Shao <laoar.shao@gmail.com> Link: https://lore.kernel.org/bpf/20220421094320.1563570-1-asavkov@redhat.com
-
Artem Savkov authored
I am getting the following compilation error for prog_tests/uprobe_autoattach.c: tools/testing/selftests/bpf/prog_tests/uprobe_autoattach.c: In function ‘test_uprobe_autoattach’: ./test_progs.h:209:26: error: pointer ‘mem’ may be used after ‘free’ [-Werror=use-after-free] The value of mem is now used in one of the asserts, which is why it may be confusing compilers. However, it is not dereferenced. Silence this by moving free(mem) after the assert block. Fixes: 1717e248 ("selftests/bpf: Uprobe tests should verify param/return values") Signed-off-by:
Daniel Borkmann <daniel@iogearbox.net> Link: https://lore.kernel.org/bpf/20220421132317.1583867-1-asavkov@redhat.com
-
Artem Savkov authored
Switching to libbpf 1.0 API broke test_sock and test_sysctl as they check for return of bpf_prog_attach to be exactly -1. Switch the check to '< 0' instead. Fixes: b858ba8c ("selftests/bpf: Use libbpf 1.0 API mode instead of RLIMIT_MEMLOCK") Signed-off-by:
-
Maciej Fijalkowski authored
Intel drivers translate actions returned from XDP programs to their own return codes that have the following mapping: XDP_REDIRECT -> I40E_XDP_{REDIR,CONSUMED} XDP_TX -> I40E_XDP_{TX,CONSUMED} XDP_DROP -> I40E_XDP_CONSUMED XDP_ABORTED -> I40E_XDP_CONSUMED XDP_PASS -> I40E_XDP_PASS Commit b8aef650 ("i40e, xsk: Terminate Rx side of NAPI when XSK Rx queue gets full") introduced new translation XDP_REDIRECT -> I40E_XDP_EXIT which is set when XSK RQ gets full and to indicate that driver should stop further Rx processing. This happens for unsuccessful xdp_do_redirect() so it is valuable to call trace_xdp_exception() for this case. In order to avoid I40E_XDP_EXIT -> IXGBE_XDP_CONSUMED overwrite, XDP_DROP case was moved above which in turn made the 'fallthrough' that is in XDP_ABORTED useless as it became the last label in the switch statement. Simply drop this leftover. Fixes: b8aef650 ("i40e, xsk: Terminate Rx side of NAPI when XSK Rx queue gets full") Signed-off-by:Maciej Fijalkowski <maciej.fijalkowski@intel.com> Signed-off-by:
-
Maciej Fijalkowski authored
Intel drivers translate actions returned from XDP programs to their own return codes that have the following mapping: XDP_REDIRECT -> IXGBE_XDP_{REDIR,CONSUMED} XDP_TX -> IXGBE_XDP_{TX,CONSUMED} XDP_DROP -> IXGBE_XDP_CONSUMED XDP_ABORTED -> IXGBE_XDP_CONSUMED XDP_PASS -> IXGBE_XDP_PASS Commit c7dd09fd ("ixgbe, xsk: Terminate Rx side of NAPI when XSK Rx queue gets full") introduced new translation XDP_REDIRECT -> IXGBE_XDP_EXIT which is set when XSK RQ gets full and to indicate that driver should stop further Rx processing. This happens for unsuccessful xdp_do_redirect() so it is valuable to call trace_xdp_exception() for this case. In order to avoid IXGBE_XDP_EXIT -> IXGBE_XDP_CONSUMED overwrite, XDP_DROP case was moved above which in turn made the 'fallthrough' that is in XDP_ABORTED useless as it became the last label in the switch statement. Simply drop this leftover. Fixes: c7dd09fd ("ixgbe, xsk: Terminate Rx side of NAPI when XSK Rx queue gets full") Signed-off-by: -
Kumar Kartikeya Dwivedi authored
Some functions in next patch want to use this function, and those functions will be called by check_map_access, hence move it before check_map_access. Signed-off-by:
Kumar Kartikeya Dwivedi <memxor@gmail.com> Signed-off-by:
Alexei Starovoitov <ast@kernel.org> Signed-off-by:
Joanne Koong <joannelkoong@gmail.com> Link: https://lore.kernel.org/bpf/20220415160354.1050687-3-memxor@gmail.com
-
Kumar Kartikeya Dwivedi authored
Next commit introduces field type 'kptr' whose kind will not be struct, but pointer, and it will not be limited to one offset, but multiple ones. Make existing btf_find_struct_field and btf_find_datasec_var functions amenable to use for finding kptrs in map value, by moving spin_lock and timer specific checks into their own function. The alignment, and name are checked before the function is called, so it is the last point where we can skip field or return an error before the next loop iteration happens. Size of the field and type is meant to be checked inside the function. Signed-off-by:
-
Grant Seltzer authored
This adds documentation for the following API functions: - bpf_program__set_expected_attach_type() - bpf_program__set_type() - bpf_program__set_attach_target() - bpf_program__attach() - bpf_program__pin() - bpf_program__unpin() Signed-off-by:
Grant Seltzer <grantseltzer@gmail.com> Signed-off-by:
-
Grant Seltzer authored
This updates usage of the following API functions within libbpf so their newly added error return is checked: - bpf_program__set_expected_attach_type() - bpf_program__set_type() Signed-off-by:
-
Grant Seltzer authored
This adds an error return to the following API functions: - bpf_program__set_expected_attach_type() - bpf_program__set_type() In both cases, the error occurs when the BPF object has already been loaded when the function is called. In this case -EBUSY is returned. Signed-off-by:
-
- 20 Apr, 2022 8 commits
-
-
Zhengchao Shao authored
If interval is 2, and sum - prev[key] = 1, the result = 0. This will mislead the tester that the port has no traffic right now. So reduce the sampling interval to 1. Signed-off-by:
Zhengchao Shao <shaozhengchao@huawei.com> Signed-off-by:
-
Liu Jian authored
Use bpf_prog_test_run_opts to test the skb_load_bytes function. Tests the behavior when offset is greater than INT_MAX or a normal value. Signed-off-by:
Liu Jian <liujian56@huawei.com> Signed-off-by:
Song Liu <songliubraving@fb.com> Link: https://lore.kernel.org/bpf/20220416105801.88708-4-liujian56@huawei.com
-
Liu Jian authored
Both pskb_may_pull() and skb_clone_writable()'s length parameters are of type unsigned int already. Therefore, change this function's write_len param to unsigned int type. Signed-off-by:
-
Liu Jian authored
The data length of skb frags + frag_list may be greater than 0xffff, and skb_header_pointer can not handle negative offset. So, here INT_MAX is used to check the validity of offset. Add the same change to the related function skb_store_bytes. Fixes: 05c74e5e ("bpf: add bpf_skb_load_bytes helper") Signed-off-by:
-
Andrii Nakryiko authored
Pu Lehui says: ==================== patch 1 fix a minor issue where usdt_cookie is cast to 32 bits. patch 2 add support riscv libbpf USDT argument parsing logic, both RV32 and RV64 tests have been passed as like follow: # ./test_progs -t usdt #169 usdt:OK Summary: 1/4 PASSED, 0 SKIPPED, 0 FAILED ==================== Signed-off-by: -
Pu Lehui authored
Add riscv-specific USDT argument specification parsing logic. riscv USDT argument format is shown below: - Memory dereference case: "size@off(reg)", e.g. "-8@-88(s0)" - Constant value case: "size@val", e.g. "4@5" - Register read case: "size@reg", e.g. "-8@a1" s8 will be marked as poison while it's a reg of riscv, we need to alias it in advance. Both RV32 and RV64 have been tested. Signed-off-by:
Pu Lehui <pulehui@huawei.com> Signed-off-by:
-
Pu Lehui authored
The usdt_cookie is defined as __u64, which should not be used as a long type because it will be cast to 32 bits in 32-bit platforms. Signed-off-by:
-
KP Singh authored
bpf_{sk,task,inode}_storage_free() do not need to use call_rcu_tasks_trace as no BPF program should be accessing the owner as it's being destroyed. The only other reader at this point is bpf_local_storage_map_free() which uses normal RCU. The only path that needs trace RCU are: * bpf_local_storage_{delete,update} helpers * map_{delete,update}_elem() syscalls Fixes: 0fe4b381 ("bpf: Allow bpf_local_storage to be used by sleepable programs") Signed-off-by:KP Singh <kpsingh@kernel.org> Signed-off-by:
Martin KaFai Lau <kafai@fb.com> Link: https://lore.kernel.org/bpf/20220418155158.2865678-1-kpsingh@kernel.org
-
- 19 Apr, 2022 9 commits
-
-
Alexei Starovoitov authored
Kumar Kartikeya Dwivedi says: ==================== When iterating over modifiers, ensure that type tags can only occur at head of the chain, and don't occur later, such that checking for them once in the start tells us there are no more type tags in later modifiers. Clang already ensures to emit such BTF, but user can craft their own BTF which violates such assumptions if relied upon in the kernel. Changelog: ---------- v2 -> v3 v2: https://lore.kernel.org/bpf/20220418224719.1604889-1-memxor@gmail.com * Address nit from Yonghong, add Acked-by v1 -> v2 v1: https://lore.kernel.org/bpf/20220406004121.282699-1-memxor@gmail.com * Fix for bug pointed out by Yonghong * Update selftests to include Yonghong's example ==================== Signed-off-by:
-
Kumar Kartikeya Dwivedi authored
Add a few test cases that ensure we catch cases of badly ordered type tags in modifier chains. Signed-off-by:
Yonghong Song <yhs@fb.com> Link: https://lore.kernel.org/bpf/20220419164608.1990559-3-memxor@gmail.com
-
Kumar Kartikeya Dwivedi authored
It is guaranteed that for modifiers, clang always places type tags before other modifiers, and then the base type. We would like to rely on this guarantee inside the kernel to make it simple to parse type tags from BTF. However, a user would be allowed to construct a BTF without such guarantees. Hence, add a pass to check that in modifier chains, type tags only occur at the head of the chain, and then don't occur later in the chain. If we see a type tag, we can have one or more type tags preceding other modifiers that then never have another type tag. If we see other modifiers, all modifiers following them should never be a type tag. Instead of having to walk chains we verified previously, we can remember the last good modifier type ID which headed a good chain. At that point, we must have verified all other chains headed by type IDs less than it. This makes the verification process less costly, and it becomes a simple O(n) pass. Signed-off-by:
-
Andrii Nakryiko authored
Take advantage of new libbpf feature for declarative non-autoloaded BPF program SEC() definitions in few test that test single program at a time out of many available programs within the single BPF object. Signed-off-by:
-
Andrii Nakryiko authored
Establish SEC("?abc") naming convention (i.e., adding question mark in front of otherwise normal section name) that allows to set corresponding program's autoload property to false. This is effectively just a declarative way to do bpf_program__set_autoload(prog, false). Having a way to do this declaratively in BPF code itself is useful and convenient for various scenarios. E.g., for testing, when BPF object consists of multiple independent BPF programs that each needs to be tested separately. Opting out all of them by default and then setting autoload to true for just one of them at a time simplifies testing code (see next patch for few conversions in BPF selftests taking advantage of this new feature). Another real-world use case is in libbpf-tools for cases when different BPF programs have to be picked depending on particulars of the host kernel due to various incompatible changes (like kernel function renames or signature change, or to pick kprobe vs fentry depending on corresponding kernel support for the latter). Marking all the different BPF program candidates as non-autoloaded declaratively makes this more obvious in BPF source code and allows simpler code in user-space code. When BPF program marked as SEC("?abc") it is otherwise treated just like SEC("abc") and bpf_program__section_name() reported will be "abc". Signed-off-by: -
Yonghong Song authored
The llvm patch [1] enabled opaque pointer which caused selftest 'exhandler' failure. ... ; work = task->task_works; 7: (79) r1 = *(u64 *)(r6 +2120) ; R1_w=ptr_callback_head(off=0,imm=0) R6_w=ptr_task_struct(off=0,imm=0) ; func = work->func; 8: (79) r2 = *(u64 *)(r1 +8) ; R1_w=ptr_callback_head(off=0,imm=0) R2_w=scalar() ; if (!work && !func) 9: (4f) r1 |= r2 math between ptr_ pointer and register with unbounded min value is not allowed below is insn 10 and 11 10: (55) if r1 != 0 goto +5 11: (18) r1 = 0 ll ... In llvm, the code generation of 'r1 |= r2' happened in codegen selectiondag phase due to difference of opaque pointer vs. non-opaque pointer. Without [1], the related code looks like: r2 = *(u64 *)(r6 + 2120) r1 = *(u64 *)(r2 + 8) if r2 != 0 goto +6 <LBB0_4> if r1 != 0 goto +5 <LBB0_4> r1 = 0 ll ... I haven't found a good way in llvm to fix this issue. So let us workaround the problem first so bpf CI won't be blocked. [1] https://reviews.llvm.org/D123300Signed-off-by:
-
Yonghong Song authored
LLVM commit [1] changed loop pragma behavior such that full loop unroll is always honored with user pragma. Previously, unroll count also depends on the unrolled code size. For pyperf600, without [1], the loop unroll count is 150. With [1], the loop unroll count is 600. The unroll count of 600 caused the program size close to 298k and this caused the following code is generated: 0: 7b 1a 00 ff 00 00 00 00 *(u64 *)(r10 - 256) = r1 ; uint64_t pid_tgid = bpf_get_current_pid_tgid(); 1: 85 00 00 00 0e 00 00 00 call 14 2: bf 06 00 00 00 00 00 00 r6 = r0 ; pid_t pid = (pid_t)(pid_tgid >> 32); 3: bf 61 00 00 00 00 00 00 r1 = r6 4: 77 01 00 00 20 00 00 00 r1 >>= 32 5: 63 1a fc ff 00 00 00 00 *(u32 *)(r10 - 4) = r1 6: bf a2 00 00 00 00 00 00 r2 = r10 7: 07 02 00 00 fc ff ff ff r2 += -4 ; PidData* pidData = bpf_map_lookup_elem(&pidmap, &pid); 8: 18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0 ll 10: 85 00 00 00 01 00 00 00 call 1 11: bf 08 00 00 00 00 00 00 r8 = r0 ; if (!pidData) 12: 15 08 15 e8 00 00 00 00 if r8 == 0 goto -6123 <LBB0_27588+0xffffffffffdae100> Note that insn 12 has a branch offset -6123 which is clearly illegal and will be rejected by the verifier. The negative offset is due to the branch range is greater than INT16_MAX. This patch changed the unroll count to be 150 to avoid above branch target insn out-of-range issue. Also the llvm is enhanced ([2]) to assert if the branch target insn is out of INT16 range. [1] https://reviews.llvm.org/D119148 [2] https://reviews.llvm.org/D123877Signed-off-by: -
Stanislav Fomichev authored
Commit 7d08c2c9 ("bpf: Refactor BPF_PROG_RUN_ARRAY family of macros into functions") switched a bunch of BPF_PROG_RUN macros to inline routines. This changed the semantic a bit. Due to arguments expansion of macros, it used to be: rcu_read_lock(); array = rcu_dereference(cgrp->bpf.effective[atype]); ... Now, with with inline routines, we have: array_rcu = rcu_dereference(cgrp->bpf.effective[atype]); /* array_rcu can be kfree'd here */ rcu_read_lock(); array = rcu_dereference(array_rcu); I'm assuming in practice rcu subsystem isn't fast enough to trigger this but let's use rcu API properly. Also, rename to lower caps to not confuse with macros. Additionally, drop and expand BPF_PROG_CGROUP_INET_EGRESS_RUN_ARRAY. See [1] for more context. [1] https://lore.kernel.org/bpf/CAKH8qBs60fOinFdxiiQikK_q0EcVxGvNTQoWvHLEUGbgcj1UYg@mail.gmail.com/T/#u v2 - keep rcu locks inside by passing cgroup_bpf Fixes: 7d08c2c9 ("bpf: Refactor BPF_PROG_RUN_ARRAY family of macros into functions") Signed-off-by:
Stanislav Fomichev <sdf@google.com> Signed-off-by:
-
Mykola Lysenko authored
This is a pre-req to add separate logging for each subtest in test_progs. Move all the mutable test data to the test_result struct. Move per-test init/de-init into the run_one_test function. Consolidate data aggregation and final log output in calculate_and_print_summary function. As a side effect, this patch fixes double counting of errors for subtests and possible duplicate output of subtest log on failures. Also, add prog_tests_framework.c test to verify some of the counting logic. As part of verification, confirmed that number of reported tests is the same before and after the change for both parallel and sequential test execution. Signed-off-by:
Mykola Lysenko <mykolal@fb.com> Signed-off-by:
-
- 15 Apr, 2022 13 commits
-
-
Maciej Fijalkowski authored
Simplify the mentioned helper function by removing ternary operator. The expression that is there outputs the boolean value by itself. This helper might be used in the hot path so this simplification can also be considered as micro optimization. Signed-off-by:
-
Maciej Fijalkowski authored
Call alloc Rx routine for ZC driver only when the amount of unallocated descriptors exceeds given threshold. Signed-off-by:
-
Maciej Fijalkowski authored
Currently, when debugging AF_XDP workloads, one can correlate the -ENXIO return code as the case that XSK is not in the bound state. Returning same code from ndo_xsk_wakeup can be misleading and simply makes it harder to follow what is going on. Change ENXIOs in stmmac's ndo_xsk_wakeup() implementation to EINVALs, so that when probing it is clear that something is wrong on the driver side, not the xsk_{recv,send}msg. There is a -ENETDOWN that can happen from both kernel/driver sides though, but I don't have a correct replacement for this on one of the sides, so let's keep it that way. Signed-off-by: -
Maciej Fijalkowski authored
Currently, when debugging AF_XDP workloads, one can correlate the -ENXIO return code as the case that XSK is not in the bound state. Returning same code from ndo_xsk_wakeup can be misleading and simply makes it harder to follow what is going on. Change ENXIO in mlx5's ndo_xsk_wakeup() implementation to EINVAL, so that when probing it is clear that something is wrong on the driver side, not the xsk_{recv,send}msg. There is a -ENETDOWN that can happen from both kernel/driver sides though, but I don't have a correct replacement for this on one of the sides, so let's keep it that way. Signed-off-by: -
Maciej Fijalkowski authored
Currently, when debugging AF_XDP workloads, one can correlate the -ENXIO return code as the case that XSK is not in the bound state. Returning same code from ndo_xsk_wakeup can be misleading and simply makes it harder to follow what is going on. Change ENXIOs in ixgbe's ndo_xsk_wakeup() implementation to EINVALs, so that when probing it is clear that something is wrong on the driver side, not the xsk_{recv,send}msg. There is a -ENETDOWN that can happen from both kernel/driver sides though, but I don't have a correct replacement for this on one of the sides, so let's keep it that way. Signed-off-by: -
Maciej Fijalkowski authored
Currently, when debugging AF_XDP workloads, one can correlate the -ENXIO return code as the case that XSK is not in the bound state. Returning same code from ndo_xsk_wakeup can be misleading and simply makes it harder to follow what is going on. Change ENXIOs in i40e's ndo_xsk_wakeup() implementation to EINVALs, so that when probing it is clear that something is wrong on the driver side, not the xsk_{recv,send}msg. There is a -ENETDOWN that can happen from both kernel/driver sides though, but I don't have a correct replacement for this on one of the sides, so let's keep it that way. Signed-off-by: -
Maciej Fijalkowski authored
Currently, when debugging AF_XDP workloads, one can correlate the -ENXIO return code as the case that XSK is not in the bound state. Returning same code from ndo_xsk_wakeup can be misleading and simply makes it harder to follow what is going on. Change ENXIOs in ice's ndo_xsk_wakeup() implementation to EINVALs, so that when probing it is clear that something is wrong on the driver side, not the xsk_{recv,send}msg. There is a -ENETDOWN that can happen from both kernel/driver sides though, but I don't have a correct replacement for this on one of the sides, so let's keep it that way. Signed-off-by: -
Maciej Fijalkowski authored
When XSK pool uses need_wakeup feature, correlate -ENOBUFS that was returned from xdp_do_redirect() with a XSK Rx queue being full. In such case, terminate the Rx processing that is being done on the current HW Rx ring and let the user space consume descriptors from XSK Rx queue so that there is room that driver can use later on. Introduce new internal return code IXGBE_XDP_EXIT that will indicate case described above. Note that it does not affect Tx processing that is bound to the same NAPI context, nor the other Rx rings. Signed-off-by:
-
Maciej Fijalkowski authored
When XSK pool uses need_wakeup feature, correlate -ENOBUFS that was returned from xdp_do_redirect() with a XSK Rx queue being full. In such case, terminate the Rx processing that is being done on the current HW Rx ring and let the user space consume descriptors from XSK Rx queue so that there is room that driver can use later on. Introduce new internal return code I40E_XDP_EXIT that will indicate case described above. Note that it does not affect Tx processing that is bound to the same NAPI context, nor the other Rx rings. Signed-off-by:
-
Maciej Fijalkowski authored
When XSK pool uses need_wakeup feature, correlate -ENOBUFS that was returned from xdp_do_redirect() with a XSK Rx queue being full. In such case, terminate the Rx processing that is being done on the current HW Rx ring and let the user space consume descriptors from XSK Rx queue so that there is room that driver can use later on. Introduce new internal return code ICE_XDP_EXIT that will indicate case described above. Note that it does not affect Tx processing that is bound to the same NAPI context, nor the other Rx rings. Signed-off-by:
-
Maciej Fijalkowski authored
ixgbe_run_xdp_zc() suggests to compiler that XDP_REDIRECT is the most probable action returned from BPF program that AF_XDP has in its pipeline. Let's also bring this suggestion up to the callsite of ixgbe_run_xdp_zc() so that compiler will be able to generate more optimized code which in turn will make branch predictor happy. Suggested-by:
Jesper Dangaard Brouer <brouer@redhat.com> Signed-off-by:
-
Maciej Fijalkowski authored
ice_run_xdp_zc() suggests to compiler that XDP_REDIRECT is the most probable action returned from BPF program that AF_XDP has in its pipeline. Let's also bring this suggestion up to the callsite of ice_run_xdp_zc() so that compiler will be able to generate more optimized code which in turn will make branch predictor happy. Suggested-by:
-
Maciej Fijalkowski authored
Inspired by patch that made xdp_do_redirect() return values for XSKMAP more meaningful, return -ENXIO instead of -EINVAL for socket being unbound in xsk_rcv_check() as this is the usual value that is returned for such event. In turn, it is now possible to easily distinguish what went wrong, which is a bit harder when for both cases checked, -EINVAL was returned. Return codes can be counted in a nice way via bpftrace oneliner that Jesper has shown: bpftrace -e 'tracepoint:xdp:xdp_redirect* {@err[-args->err] = count();}' Signed-off-by:
-
