- 13 Feb, 2023 7 commits
-
-
Alexander Lobakin authored
As already mentioned, freeing any &xdp_frame via page_frag_free() is wrong, as it assumes the frame is backed by either an order-0 page or a page with no "patrons" behind them, while in fact frames backed by Page Pool can be redirected to a device, which's driver doesn't use it. Keep storing a pointer to the raw buffer and then freeing it unconditionally via page_frag_free() for %XDP_TX frames, but introduce a separate type in the enum for frames coming through .ndo_xdp_xmit(), and free them via xdp_return_frame_bulk(). Note that saving xdpf as xdp_buff->data_hard_start is intentional and is always true when everything is configured properly. After this change, %XDP_REDIRECT from a Page Pool based driver to ice becomes zero-alloc as it should be and horrendous 3.3 Mpps / queue turn into 6.6, hehe. Let it go with no "Fixes:" tag as it spans across good 5+ commits and can't be trivially backported. Signed-off-by:
Alexander Lobakin <alexandr.lobakin@intel.com> Signed-off-by:
Daniel Borkmann <daniel@iogearbox.net> Acked-by:
Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/bpf/20230210170618.1973430-6-alexandr.lobakin@intel.com
-
Alexander Lobakin authored
When queueing frames from a Page Pool for redirecting to a device backed by the ice driver, `perf top` shows heavy load on page_alloc() and page_frag_free(), despite that on a properly working system it must be fully or at least almost zero-alloc. The problem is in fact a bit deeper and raises from how ice cleans up completed Tx buffers. The story so far: when cleaning/freeing the resources related to a particular completed Tx frame (skbs, DMA mappings etc.), ice uses some heuristics only without setting any type explicitly (except for dummy Flow Director packets, which are marked via ice_tx_buf::tx_flags). This kinda works, but only up to some point. For example, currently ice assumes that each frame coming to __ice_xmit_xdp_ring(), is backed by either plain order-0 page or plain page frag, while it may also be backed by Page Pool or any other possible memory models introduced in future. This means any &xdp_frame must be freed properly via xdp_return_frame() family with no assumptions. In order to do that, the whole heuristics must be replaced with setting the Tx buffer/frame type explicitly, just how it's always been done via an enum. Let us reuse 16 bits from ::tx_flags -- 1 bit-and instr won't hurt much -- especially given that sometimes there was a check for %ICE_TX_FLAGS_DUMMY_PKT, which is now turned from a flag to an enum member. The rest of the changes is straightforward and most of it is just a conversion to rely now on the type set in &ice_tx_buf rather than to some secondary properties. For now, no functional changes intended, the change only prepares the ground for starting freeing XDP frames properly next step. And it must be done atomically/synchronously to not break stuff. Signed-off-by:
-
Alexander Lobakin authored
The tagged commit started sending %XDP_TX frames from XSk Rx ring directly without converting it to an &xdp_frame. However, when XSk is enabled on a queue pair, it has its separate Tx cleaning functions, so neither ice_clean_xdp_irq() nor ice_unmap_and_free_tx_buf() ever happens there. Remove impossible branches in order to reduce the diffstat of the upcoming change. Fixes: a24b4c6e ("ice: xsk: Do not convert to buff to frame for XDP_TX") Suggested-by:
-
Alexander Lobakin authored
Sometimes, under heavy XDP Tx traffic, e.g. when using XDP traffic generator (%BPF_F_TEST_XDP_LIVE_FRAMES), the machine can catch OOM due to the driver not freeing all of the pages passed to it by .ndo_xdp_xmit(). Turned out that during the development of the tagged commit, the check, which ensures that we have a free descriptor to queue a frame, moved into the branch happening only when a buffer has frags. Otherwise, we only run a cleaning cycle, but don't check anything. ATST, there can be situations when the driver gets new frames to send, but there are no buffers that can be cleaned/completed and the ring has no free slots. It's very rare, but still possible (> 6.5 Mpps per ring). The driver then fills the next buffer/descriptor, effectively overwriting the data, which still needs to be freed. Restore the check after the cleaning routine to make sure there is a slot to queue a new frame. When there are frags, there still will be a separate check that we can place all of them, but if the ring is full, there's no point in wasting any more time. (minor: make `!ready_frames` unlikely since it happens ~1-2 times per billion of frames) Fixes: 3246a107 ("ice: Add support for XDP multi-buffer on Tx side") Signed-off-by:
-
Alexander Lobakin authored
xdp_tx_active is used to indicate whether an XDP ring has any %XDP_TX frames queued to shortcut processing Tx cleaning for XSk-enabled queues. When !XSk, it simply indicates whether the ring has any queued frames in general. It gets increased on each frame placed onto the ring and counts the whole frame, not each frag. However, currently it gets decremented in ice_clean_xdp_tx_buf(), which is called per each buffer, i.e. per each frag. Thus, on completing multi-frag frames, an underflow happens. Move the decrement to the outer function and do it once per frame, not buf. Also, do that on the stack and update the ring counter after the loop is done to save several cycles. XSk rings are fine since there are no frags at the moment. Fixes: 3246a107 ("ice: Add support for XDP multi-buffer on Tx side") Signed-off-by:
-
Ilya Leoshkevich authored
Building BPF selftests out of srctree fails with: make: *** No rule to make target '/linux-build//ima_setup.sh', needed by 'ima_setup.sh'. Stop. The culprit is the rule that defines convenient shorthands like "make test_progs", which builds $(OUTPUT)/test_progs. These shorthands make sense only for binaries that are built though; scripts that live in the source tree do not end up in $(OUTPUT). Therefore drop $(TEST_PROGS) and $(TEST_PROGS_EXTENDED) from the rule. The issue exists for a while, but it became a problem only after commit d68ae498 ("selftests/bpf: Install all required files to run selftests"), which added dependencies on these scripts. Fixes: 03dcb784 ("selftests/bpf: Add simple per-test targets to Makefile") Signed-off-by:
Ilya Leoshkevich <iii@linux.ibm.com> Signed-off-by:
-
Alan Maguire authored
v1.25 of pahole supports filtering out functions with multiple inconsistent function prototypes or optimized-out parameters from the BTF representation. These present problems because there is no additional info in BTF saying which inconsistent prototype matches which function instance to help guide attachment, and functions with optimized-out parameters can lead to incorrect assumptions about register contents. So for now, filter out such functions while adding BTF representations for functions that have "."-suffixes (foo.isra.0) but not optimized-out parameters. This patch assumes that below linked changes land in pahole for v1.25. Signed-off-by:
Alan Maguire <alan.maguire@oracle.com> Signed-off-by:
Arnaldo Carvalho de Melo <acme@redhat.com> Acked-by:
Jiri Olsa <jolsa@kernel.org> Link: https://lore.kernel.org/bpf/1675790102-23037-1-git-send-email-alan.maguire@oracle.com Link: https://lore.kernel.org/bpf/1675949331-27935-1-git-send-email-alan.maguire@oracle.com
-
- 11 Feb, 2023 14 commits
-
-
Alexei Starovoitov authored
Yafang Shao says: ==================== The bpf memory accouting has some known problems in contianer environment, - The container memory usage is not consistent if there's pinned bpf program After the container restart, the leftover bpf programs won't account to the new generation, so the memory usage of the container is not consistent. This issue can be resolved by introducing selectable memcg, but we don't have an agreement on the solution yet. See also the discussions at https://lwn.net/Articles/905150/ . - The leftover non-preallocated bpf map can't be limited The leftover bpf map will be reparented, and thus it will be limited by the parent, rather than the container itself. Furthermore, if the parent is destroyed, it be will limited by its parent's parent, and so on. It can also be resolved by introducing selectable memcg. - The memory dynamically allocated in bpf prog is charged into root memcg only Nowdays the bpf prog can dynamically allocate memory, for example via bpf_obj_new(), but it only allocate from the global bpf_mem_alloc pool, so it will charge into root memcg only. That needs to be addressed by a new proposal. So let's give the container user an option to disable bpf memory accouting. The idea of "cgroup.memory=nobpf" is originally by Tejun[1]. [1]. https://lwn.net/ml/linux-mm/YxjOawzlgE458ezL@slm.duckdns.org/ Changes, v1->v2: - squash patches (Roman) - commit log improvement in patch #2. (Johannes) ==================== Signed-off-by:
Alexei Starovoitov <ast@kernel.org>
-
Yafang Shao authored
We can simply disable the bpf prog memory accouting by not setting the GFP_ACCOUNT. Signed-off-by:
Yafang Shao <laoar.shao@gmail.com> Acked-by:
Johannes Weiner <hannes@cmpxchg.org> Acked-by:
Roman Gushchin <roman.gushchin@linux.dev> Link: https://lore.kernel.org/r/20230210154734.4416-5-laoar.shao@gmail.comSigned-off-by:
-
Yafang Shao authored
We can simply set root memcg as the map's memcg to disable bpf memory accounting. bpf_map_area_alloc is a little special as it gets the memcg from current rather than from the map, so we need to disable GFP_ACCOUNT specifically for it. Signed-off-by:
-
Yafang Shao authored
Introduce new helper bpf_map_kvcalloc() for the memory allocation in bpf_local_storage(). Then the allocation will charge the memory from the map instead of from current, though currently they are the same thing as it is only used in map creation path now. By charging map's memory into the memcg from the map, it will be more clear. Signed-off-by:
-
Yafang Shao authored
Add new kernel parameter cgroup.memory=nobpf to allow user disable bpf memory accounting. This is a preparation for the followup patch. Signed-off-by:
-
Daniel Borkmann authored
Given BPF is increasingly being used beyond just the Linux kernel, with implementations in NICs and other hardware, Windows, etc, there is an ongoing effort to document and standardize parts of the existing BPF infrastructure such as its ISA. As "source of truth" we decided some time ago to rely on the in-tree documentation, in particular, starting out with the Documentation/bpf/instruction-set.rst as a base for later RFC drafts on the ISA. Therefore, we want to ensure that changes to that document have bpf@ietf.org in Cc, so add a MAINTAINERS file entry with a section on documents related to standardization efforts. For now, this only relates to instruction-set.rst, and later additional files will be added. Signed-off-by:
-
Jakub Kicinski authored
==================== pull-request: bpf-next 2023-02-11 We've added 96 non-merge commits during the last 14 day(s) which contain a total of 152 files changed, 4884 insertions(+), 962 deletions(-). There is a minor conflict in drivers/net/ethernet/intel/ice/ice_main.c between commit 5b246e53 ("ice: split probe into smaller functions") from the net-next tree and commit 66c0e13a ("drivers: net: turn on XDP features") from the bpf-next tree. Remove the hunk given ice_cfg_netdev() is otherwise there a 2nd time, and add XDP features to the existing ice_cfg_netdev() one: [...] ice_set_netdev_features(netdev); netdev->xdp_features = NETDEV_XDP_ACT_BASIC | NETDEV_XDP_ACT_REDIRECT | NETDEV_XDP_ACT_XSK_ZEROCOPY; ice_set_ops(netdev); [...] Stephen's merge conflict mail: https://lore.kernel.org/bpf/20230207101951.21a114fa@canb.auug.org.au/ The main changes are: 1) Add support for BPF trampoline on s390x which finally allows to remove many test cases from the BPF CI's DENYLIST.s390x, from Ilya Leoshkevich. 2) Add multi-buffer XDP support to ice driver, from Maciej Fijalkowski. 3) Add capability to export the XDP features supported by the NIC. Along with that, add a XDP compliance test tool, from Lorenzo Bianconi & Marek Majtyka. 4) Add __bpf_kfunc tag for marking kernel functions as kfuncs, from David Vernet. 5) Add a deep dive documentation about the verifier's register liveness tracking algorithm, from Eduard Zingerman. 6) Fix and follow-up cleanups for resolve_btfids to be compiled as a host program to avoid cross compile issues, from Jiri Olsa & Ian Rogers. 7) Batch of fixes to the BPF selftest for xdp_hw_metadata which resulted when testing on different NICs, from Jesper Dangaard Brouer. 8) Fix libbpf to better detect kernel version code on Debian, from Hao Xiang. 9) Extend libbpf to add an option for when the perf buffer should wake up, from Jon Doron. 10) Follow-up fix on xdp_metadata selftest to just consume on TX completion, from Stanislav Fomichev. 11) Extend the kfuncs.rst document with description on kfunc lifecycle & stability expectations, from David Vernet. 12) Fix bpftool prog profile to skip attaching to offline CPUs, from Tonghao Zhang. ==================== Link: https://lore.kernel.org/r/20230211002037.8489-1-daniel@iogearbox.netSigned-off-by:
Jakub Kicinski <kuba@kernel.org>
-
Randy Dunlap authored
Correct spelling problems for Documentation/isdn/ as reported by codespell. Signed-off-by:
Randy Dunlap <rdunlap@infradead.org> Cc: Karsten Keil <isdn@linux-pingi.de> Cc: Jonathan Corbet <corbet@lwn.net> Link: https://lore.kernel.org/r/20230209071400.31476-9-rdunlap@infradead.orgSigned-off-by:
-
Jakub Kicinski authored
Xin Long says: ==================== net: move more duplicate code of ovs and tc conntrack into nf_conntrack_ovs We've moved some duplicate code into nf_nat_ovs in: "net: eliminate the duplicate code in the ct nat functions of ovs and tc" This patchset addresses more code duplication in the conntrack of ovs and tc then creates nf_conntrack_ovs for them, and four functions will be extracted and moved into it: nf_ct_handle_fragments() nf_ct_skb_network_trim() nf_ct_helper() nf_ct_add_helper() ==================== Link: https://lore.kernel.org/r/cover.1675810210.git.lucien.xin@gmail.comSigned-off-by:
-
Xin Long authored
Now handle_fragments() in OVS and TC have the similar code, and this patch removes the duplicate code by moving the function to nf_conntrack_ovs. Note that skb_clear_hash(skb) or skb->ignore_df = 1 should be done only when defrag returns 0, as it does in other places in kernel. Signed-off-by:
Xin Long <lucien.xin@gmail.com> Reviewed-by:
Simon Horman <simon.horman@corigine.com> Reviewed-by:
Aaron Conole <aconole@redhat.com> Acked-by:
Florian Westphal <fw@strlen.de> Signed-off-by:
-
Xin Long authored
This patch has no functional changes and just moves frag check and tc_skb_cb update out of handle_fragments, to make it easier to move the duplicate code from handle_fragments() into nf_conntrack_ovs later. Signed-off-by:
-
Xin Long authored
This patch has no functional changes and just moves key and ovs_cb update out of handle_fragments, and skb_clear_hash() and skb->ignore_df change into handle_fragments(), to make it easier to move the duplicate code from handle_fragments() into nf_conntrack_ovs later. Note that it changes to pass info->family to handle_fragments() instead of key for the packet type check, as info->family is set according to key->eth.type in ovs_ct_copy_action() when creating the action. Signed-off-by:
-
Xin Long authored
There are almost the same code in ovs_skb_network_trim() and tcf_ct_skb_network_trim(), this patch extracts them into a function nf_ct_skb_network_trim() and moves the function to nf_conntrack_ovs. Signed-off-by:
-
Xin Long authored
Similar to nf_nat_ovs created by Commit ebddb140 ("net: move the nat function to nf_nat_ovs for ovs and tc"), this patch is to create nf_conntrack_ovs to get these functions shared by OVS and TC only. There are nf_ct_helper() and nf_ct_add_helper() from nf_conntrak_helper in this patch, and will be more in the following patches. Signed-off-by:
-
- 10 Feb, 2023 19 commits
-
-
Ilya Leoshkevich authored
The code assumes that everything that comes after nlmsgerr are nlattrs. When calculating their size, it does not account for the initial nlmsghdr. This may lead to accessing uninitialized memory. Fixes: bbf48c18 ("libbpf: add error reporting in XDP") Signed-off-by:
Andrii Nakryiko <andrii@kernel.org> Link: https://lore.kernel.org/bpf/20230210001210.395194-8-iii@linux.ibm.com
-
Ilya Leoshkevich authored
malloc() and free() may be completely replaced by sanitizers, use fopen() and fclose() instead. Signed-off-by:
-
Ilya Leoshkevich authored
malloc() and free() may be completely replaced by sanitizers, use fopen() and fclose() instead. Signed-off-by:
-
Ilya Leoshkevich authored
To get useful results from the Memory Sanitizer, all code running in a process needs to be instrumented. When building tests with other sanitizers, it's not strictly necessary, but is also helpful. So make sure runqslower and libbpf are compiled with SAN_CFLAGS and linked with SAN_LDFLAGS. Signed-off-by:
-
Ilya Leoshkevich authored
Memory Sanitizer requires passing different options to CFLAGS and LDFLAGS: besides the mandatory -fsanitize=memory, one needs to pass header and library paths, and passing -L to a compilation step triggers -Wunused-command-line-argument. So introduce a separate variable for linker flags. Use $(SAN_CFLAGS) as a default in order to avoid complicating the ASan usage. Signed-off-by:
-
Ilya Leoshkevich authored
This makes it possible to add sanitizer flags. Signed-off-by:
-
Ilya Leoshkevich authored
Using HOSTCC="ccache clang" breaks building the tests, since, when it's forwarded to e.g. bpftool, the child make sees HOSTCC=ccache and "clang" is considered a target. Fix by quoting it, and also HOSTLD and HOSTAR for consistency. Signed-off-by:
-
Jakub Kicinski authored
skbuff_head_cache is misnamed (perhaps for historical reasons?) because it does not hold heads. Head is the buffer which skb->data points to, and also where shinfo lives. struct sk_buff is a metadata structure, not the head. Eric recently added skb_small_head_cache (which allocates actual head buffers), let that serve as an excuse to finally clean this up :) Leave the user-space visible name intact, it could possibly be uAPI. Signed-off-by:
Eric Dumazet <edumazet@google.com> Signed-off-by:
David S. Miller <davem@davemloft.net>
-
David S. Miller authored
Alex Elder says: ==================== net: ipa: prepare for GSI register updtaes An upcoming series (or two) will convert the definitions of GSI registers used by IPA so they use the "IPA reg" mechanism to specify register offsets and their fields. This will simplify implementing the fairly large number of changes required in GSI registers to support more than 32 GSI channels (introduced in IPA v5.0). A few minor problems and inconsistencies were found, and they're fixed here. The last three patches in this series change the "ipa_reg" code to separate the IPA-specific part (the base virtual address, basically) from the generic register part, and the now- generic code is renamed to use just "reg_" or "REG_" as a prefix rather than "ipa_reg" or "IPA_REG_". ==================== Signed-off-by: -
Alex Elder authored
Rename functions related to register fields so they don't appear to be IPA-specific, and move their definitions into "reg.h": ipa_reg_fmask() -> reg_fmask() ipa_reg_bit() -> reg_bit() ipa_reg_field_max() -> reg_field_max() ipa_reg_encode() -> reg_encode() ipa_reg_decode() -> reg_decode() Signed-off-by:Alex Elder <elder@linaro.org> Signed-off-by:
-
Alex Elder authored
Rename ipa_reg_offset() to be reg_offset() and move its definition to "reg.h". Rename ipa_reg_n_offset() to be reg_n_offset() also. Signed-off-by:
-
Alex Elder authored
IPA register definitions have evolved with each new version. The changes required to support more than 32 endpoints in IPA v5.0 made it best to define a unified mechanism for defining registers and their fields. GSI register definitions, meanwhile, have remained fairly stable. And even as the total number of IPA endpoints goes beyond 32, the number of GSI channels on a given EE that underly endpoints still remains 32 or less. Despite that, GSI v3.0 (which is used with IPA v5.0) extends the number of channels (and events) it supports to be about 256, and as a result, many GSI register definitions must change significantly. To address this, we'll use the same "ipa_reg" mechanism to define the GSI registers. As a first step in generalizing the "ipa_reg" to also support GSI registers, isolate the definitions of the "ipa_reg" and "ipa_regs" structure types (and some supporting macros) into a new header file, and remove the "ipa_" and "IPA_" from symbol names. Separate the IPA register ID validity checking from the generic check that a register ID is in range. Aside from that, this is intended to have no functional effect on the code. Signed-off-by:
-
Alex Elder authored
Move some static inline function definitions out of "gsi_reg.h" and into "gsi.c", which is the only place they're used. Rename them so their names identify the register they're associated with. Move the gsi_channel_type enumerated type definition below the offset and field definitions for the CH_C_CNTXT_0 register where it's used. Signed-off-by:
-
Alex Elder authored
There are seven GSI interrupt types that can be signaled by a single GSI IRQ. These are represented in a bitmask, and the gsi_irq_type_id enumerated type defines what each bit position represents. Similarly, the global and general GSI interrupt types each has a set of conditions it signals, and both types have an enumerated type that defines which bit that represents each condition. When used, these enumerated values are passed as an argument to BIT() in *all* cases. So clean up the code a little bit by defining the enumerated type values as one-bit masks rather than bit positions. Rename gsi_general_id to be gsi_general_irq_id for consistency. Signed-off-by:
-
Alex Elder authored
When checking the validity of an IPA register ID, compare it against all possible ipa_reg_id values. Rename the function ipa_reg_id_valid() to be specific about what's being checked. Signed-off-by:
-
Alex Elder authored
Soon IPA v5.0+ will be supported, and when that happens we will be able to enable support for the SDX65 (IPA v5.0), SM8450 (IPA v5.1), and SM8550 (IPA v5.5). Fix the comment about the GSI version used for IPA v3.1. Signed-off-by:
-
Alex Elder authored
The reg_addr field in the IPA structure is set but never used. Get rid of it. Signed-off-by:
-
Alex Elder authored
Starting at IPA v4.11, the GSI_GENERIC_COMMAND GSI register got a new PARAMS field. The code that encodes a value into that field sets it unconditionally, which is wrong. We currently only provide 0 as the field's value, so this error has no real effect. Still, it's a bug, so let's fix it. Fix an (unrelated) incorrect comment as well. Fields in the ERROR_LOG GSI register actually *are* defined for IPA versions prior to v3.5.1. Fixes: fe68c43c ("net: ipa: support enhanced channel flow control") Signed-off-by:
-
Horatiu Vultur authored
Add the following TC flower filter keys to lan966x for IS2: - ipv4_addr (sip and dip) - ipv6_addr (sip and dip) - control (IPv4 fragments) - portnum (tcp and udp port numbers) - basic (L3 and L4 protocol) - vlan (outer vlan tag info) - tcp (tcp flags) - ip (tos field) As the parsing of these keys is similar between lan966x and sparx5, move the code in a separate file to be shared by these 2 chips. And put the specific parsing outside of the common functions. Signed-off-by:
Horatiu Vultur <horatiu.vultur@microchip.com> Signed-off-by:
-
