- 06 Feb, 2023 1 commit
-
-
Hao Xiang authored
In a previous commit, Ubuntu kernel code version is correctly set by retrieving the information from /proc/version_signature. commit<5b3d7298 > (libbpf: Improve LINUX_VERSION_CODE detection) The /proc/version_signature file doesn't present in at least the older versions of Debian distributions (eg, Debian 9, 10). The Debian kernel has a similar issue where the release information from uname() syscall doesn't give the kernel code version that matches what the kernel actually expects. Below is an example content from Debian 10. release: 4.19.0-23-amd64 version: #1 SMP Debian 4.19.269-1 (2022-12-20) x86_64 Debian reports incorrect kernel version in utsname::release returned by uname() syscall, which in older kernels (Debian 9, 10) leads to kprobe BPF programs failing to load due to the version check mismatch. Fortunately, the correct kernel code version presents in the utsname::version returned by uname() syscall in Debian kernels. This change adds another get kernel version function to handle Debian in addition to the previously added get kernel version function to handle Ubuntu. Some minor refactoring work is also done to make the code more readable. Signed-off-by:
Hao Xiang <hao.xiang@bytedance.com> Signed-off-by:
Ho-Ren (Jack) Chuang <horenchuang@bytedance.com> Signed-off-by:
Andrii Nakryiko <andrii@kernel.org> Link: https://lore.kernel.org/bpf/20230203234842.2933903-1-hao.xiang@bytedance.com
-
- 04 Feb, 2023 1 commit
-
-
Florian Lehner authored
Fix a simple typo in the documentation for bpf_perf_prog_read_value. Signed-off-by:
Florian Lehner <dev@der-flo.net> Acked-by:
John Fastabend <john.fastabend@gmail.com> Link: https://lore.kernel.org/r/20230203121439.25884-1-dev@der-flo.net Signed-off-by:
Martin KaFai Lau <martin.lau@kernel.org>
-
- 03 Feb, 2023 17 commits
-
-
David Vernet authored
BPF kernel <-> kernel API stability has been discussed at length over the last several weeks and months. Now that we've largely aligned over kfuncs being the way forward, and BPF helpers being considered functionally frozen, it's time to document the expectations for kfunc lifecycles and stability so that everyone (BPF users, kfunc developers, and maintainers) are all aligned, and have a crystal-clear understanding of the expectations surrounding kfuncs. To do that, this patch adds that documentation to the main kfuncs documentation page via a new 'kfunc lifecycle expectations' section. The patch describes how decisions are made in the kernel regarding whether to include, keep, deprecate, or change / remove a kfunc. As described very overtly in the patch itself, but likely worth highlighting here: "kfunc stability" does not mean, nor ever will mean, "BPF APIs may block development elsewhere in the kernel". Rather, the intention and expectation is for kfuncs to be treated like EXPORT_SYMBOL_GPL symbols in the kernel. The goal is for kfuncs to be a safe and valuable option for maintainers and kfunc developers to extend the kernel, without tying anyone's hands, or imposing any kind of restrictions on maintainers in the same way that UAPI changes do. In addition to the 'kfunc lifecycle expectations' section, this patch also adds documentation for a new KF_DEPRECATED kfunc flag which kfunc authors or maintainers can choose to add to kfuncs if and when they decide to deprecate them. Note that as described in the patch itself, a kfunc need not be deprecated before being changed or removed -- this flag is simply provided as an available deprecation mechanism for those that want to provide a deprecation story / timeline to their users. When necessary, kfuncs may be changed or removed to accommodate changes elsewhere in the kernel without any deprecation at all. Reviewed-by:
Bagas Sanjaya <bagasdotme@gmail.com> Co-developed-by:
Toke Høiland-Jørgensen <toke@redhat.com> Signed-off-by:
David Vernet <void@manifault.com> Link: https://lore.kernel.org/r/20230203155727.793518-2-void@manifault.com Signed-off-by:
Alexei Starovoitov <ast@kernel.org>
-
Tonghao Zhang authored
The number of online cpu may be not equal to possible cpu. "bpftool prog profile" can not create pmu event on possible but on online cpu. $ dmidecode -s system-product-name PowerEdge R620 $ cat /sys/devices/system/cpu/possible 0-47 $ cat /sys/devices/system/cpu/online 0-31 Disable cpu dynamically: $ echo 0 > /sys/devices/system/cpu/cpuX/online If one cpu is offline, perf_event_open will return ENODEV. To fix this issue: * check value returned and skip offline cpu. * close pmu_fd immediately on error path, avoid fd leaking. Fixes: 47c09d6a ("bpftool: Introduce "prog profile" command") Signed-off-by:
Tonghao Zhang <tong@infragraf.org> Cc: Quentin Monnet <quentin@isovalent.com> Cc: Alexei Starovoitov <ast@kernel.org> Cc: Daniel Borkmann <daniel@iogearbox.net> Cc: Andrii Nakryiko <andrii@kernel.org> Cc: Martin KaFai Lau <martin.lau@linux.dev> Cc: Song Liu <song@kernel.org> Cc: Yonghong Song <yhs@fb.com> Cc: John Fastabend <john.fastabend@gmail.com> Cc: KP Singh <kpsingh@kernel.org> Cc: Stanislav Fomichev <sdf@google.com> Cc: Hao Luo <haoluo@google.com> Cc: Jiri Olsa <jolsa@kernel.org> Acked-by:
-
Alexei Starovoitov authored
Lorenzo Bianconi says: ==================== Introduce the capability to export the XDP features supported by the NIC. Introduce a XDP compliance test tool (xdp_features) to check the features exported by the NIC match the real features supported by the driver. Allow XDP_REDIRECT of non-linear XDP frames into a devmap. Export XDP features for each XDP capable driver. Extend libbpf netlink implementation in order to support netlink_generic protocol. Introduce a simple generic netlink family for netdev data. Changes since v4: - rebase on top of bpf-next - get rid of XDP_FEATURE_* enum in XDP compliance test tool - rely on AF_INET6 address family and on IPv6-mapped-IPv6 addresses for IPv4 in XDP compliance test tool - add tsnep driver support Changes since v3: - add IPv6 support to XDP compliance test tool - rely on network_helpers in XDP compliance test tool - cosmetics changes Changes since v2: - rebase on top of bpf-next - fix compilation error Changes since v1: - add Documentation to netdev.yaml - use flags instead of enum as type for netdev.yaml definitions - squash XDP_PASS, XDP_DROP, XDP_TX and XDP_ABORTED into XDP_BASIC since they are supported by all drivers. - add notifier event to xdp_features_set_redirect_target() and xdp_features_clear_redirect_target() - add selftest for xdp-features support in bpf_xdp_detach() - add IPv6 preliminary support to XDP compliance test tool Changes since RFCv2: - do not assume fixed layout for genl kernel messages - fix warnings in netdev_nl_dev_fill - fix capabilities for nfp driver - add supported_sg parameter to xdp_features_set_redirect_target and drop __xdp_features_set_redirect_target routine Changes since RFCv1: - Introduce netdev-genl implementation and get rid of rtnl one. - Introduce netlink_generic support in libbpf netlink implementation - Rename XDP_FEATURE_* in NETDEV_XDP_ACT_* - Rename XDP_FEATURE_REDIRECT_TARGET in NETDEV_XDP_ACT_NDO_XMIT - Rename XDP_FEATURE_FRAG_RX in NETDEV_XDP_ACT_RX_SG - Rename XDP_FEATURE_FRAG_TARFET in NETDEV_XDP_ACT_NDO_XMIT - Get rid of XDP_LOCK feature. - Move xdp_feature field in a netdevice struct hole in the 4th cacheline. Jakub Kicinski (1): netdev-genl: create a simple family for netdev stuff Lorenzo Bianconi (5): libbpf: add the capability to specify netlink proto in libbpf_netlink_send_recv libbpf: add API to get XDP/XSK supported features bpf: devmap: check XDP features in __xdp_enqueue routine selftests/bpf: add test for bpf_xdp_query xdp-features support selftests/bpf: introduce XDP compliance test tool ==================== Signed-off-by: -
Lorenzo Bianconi authored
Introduce xdp_features tool in order to test XDP features supported by the NIC and match them against advertised ones. In order to test supported/advertised XDP features, xdp_features must run on the Device Under Test (DUT) and on a Tester device. xdp_features opens a control TCP channel between DUT and Tester devices to send control commands from Tester to the DUT and a UDP data channel where the Tester sends UDP 'echo' packets and the DUT is expected to reply back with the same packet. DUT installs multiple XDP programs on the NIC to test XDP capabilities and reports back to the Tester some XDP stats. Currently xdp_features supports the following XDP features: - XDP_DROP - XDP_ABORTED - XDP_PASS - XDP_TX - XDP_REDIRECT - XDP_NDO_XMIT Co-developed-by:
Kumar Kartikeya Dwivedi <memxor@gmail.com> Signed-off-by:
Lorenzo Bianconi <lorenzo@kernel.org> Acked-by:
Stanislav Fomichev <sdf@google.com> Link: https://lore.kernel.org/r/7c1af8e7e6ef0614cf32fa9e6bdaa2d8d605f859.1675245258.git.lorenzo@kernel.org Signed-off-by:
-
Lorenzo Bianconi authored
Introduce a self-test to verify libbpf bpf_xdp_query capability to dump the xdp-features supported by the device (lo and veth in this case). Acked-by:
-
Lorenzo Bianconi authored
Check if the destination device implements ndo_xdp_xmit callback relying on NETDEV_XDP_ACT_NDO_XMIT flags. Moreover, check if the destination device supports XDP non-linear frame in __xdp_enqueue and is_valid_dst routines. This patch allows to perform XDP_REDIRECT on non-linear XDP buffers. Acked-by:
Jesper Dangaard Brouer <brouer@redhat.com> Co-developed-by:
-
Lorenzo Bianconi authored
Extend bpf_xdp_query routine in order to get XDP/XSK supported features of netdev over route netlink interface. Extend libbpf netlink implementation in order to support netlink_generic protocol. Co-developed-by:
Marek Majtyka <alardam@gmail.com> Signed-off-by:
-
Lorenzo Bianconi authored
This is a preliminary patch in order to introduce netlink_generic protocol support to libbpf. Signed-off-by:
-
Marek Majtyka authored
Change necessary condition check for XSK from ndo functions to xdp features flags. Signed-off-by:
-
Marek Majtyka authored
A summary of the flags being set for various drivers is given below. Note that XDP_F_REDIRECT_TARGET and XDP_F_FRAG_TARGET are features that can be turned off and on at runtime. This means that these flags may be set and unset under RTNL lock protection by the driver. Hence, READ_ONCE must be used by code loading the flag value. Also, these flags are not used for synchronization against the availability of XDP resources on a device. It is merely a hint, and hence the read may race with the actual teardown of XDP resources on the device. This may change in the future, e.g. operations taking a reference on the XDP resources of the driver, and in turn inhibiting turning off this flag. However, for now, it can only be used as a hint to check whether device supports becoming a redirection target. Turn 'hw-offload' feature flag on for: - netronome (nfp) - netdevsim. Turn 'native' and 'zerocopy' features flags on for: - intel (i40e, ice, ixgbe, igc) - mellanox (mlx5). - stmmac - netronome (nfp) Turn 'native' features flags on for: - amazon (ena) - broadcom (bnxt) - freescale (dpaa, dpaa2, enetc) - funeth - intel (igb) - marvell (mvneta, mvpp2, octeontx2) - mellanox (mlx4) - mtk_eth_soc - qlogic (qede) - sfc - socionext (netsec) - ti (cpsw) - tap - tsnep - veth - xen - virtio_net. Turn 'basic' (tx, pass, aborted and drop) features flags on for: - netronome (nfp) - cavium (thunder) - hyperv. Turn 'redirect_target' feature flag on for: - amanzon (ena) - broadcom (bnxt) - freescale (dpaa, dpaa2) - intel (i40e, ice, igb, ixgbe) - ti (cpsw) - marvell (mvneta, mvpp2) - sfc - socionext (netsec) - qlogic (qede) - mellanox (mlx5) - tap - veth - virtio_net - xen Reviewed-by:
Gerhard Engleder <gerhard@engleder-embedded.com> Reviewed-by:
Simon Horman <simon.horman@corigine.com> Acked-by:
Jakub Kicinski <kuba@kernel.org> Co-developed-by:
-
Jakub Kicinski authored
Add a Netlink spec-compatible family for netdevs. This is a very simple implementation without much thought going into it. It allows us to reap all the benefits of Netlink specs, one can use the generic client to issue the commands: $ ./cli.py --spec netdev.yaml --dump dev_get [{'ifindex': 1, 'xdp-features': set()}, {'ifindex': 2, 'xdp-features': {'basic', 'ndo-xmit', 'redirect'}}, {'ifindex': 3, 'xdp-features': {'rx-sg'}}] the generic python library does not have flags-by-name support, yet, but we also don't have to carry strings in the messages, as user space can get the names from the spec. Acked-by: -
Tiezhu Yang authored
Just silence the following checkpatch warning: WARNING: Possible comma where semicolon could be used Signed-off-by:
Tiezhu Yang <yangtiezhu@loongson.cn> Link: https://lore.kernel.org/r/1675319486-27744-3-git-send-email-yangtiezhu@loongson.cn Signed-off-by:
-
Tiezhu Yang authored
Just silence the following build warning: Warning: Kernel ABI header at 'tools/include/uapi/linux/bpf.h' differs from latest version at 'include/uapi/linux/bpf.h' Signed-off-by:
-
Tobias Klauser authored
The do_idr_lock parameter to bpf_map_free_id was introduced by commit bd5f5f4e ("bpf: Add BPF_MAP_GET_FD_BY_ID"). However, all callers set do_idr_lock = true since commit 1e0bd5a0 ("bpf: Switch bpf_map ref counter to atomic64_t so bpf_map_inc() never fails"). While at it also inline __bpf_map_put into its only caller bpf_map_put now that do_idr_lock can be dropped from its signature. Signed-off-by:
Tobias Klauser <tklauser@distanz.ch> Link: https://lore.kernel.org/r/20230202141921.4424-1-tklauser@distanz.ch Signed-off-by:
-
Alexei Starovoitov authored
Eduard Zingerman says: ==================== An overview of the register tracking liveness algorithm. Previous versions posted here: [1], [2], [3]. - Changes from RFC to v2 (suggested by Andrii Nakryiko): - wording corrected to use term "stack slot" instead of "stack spill"; - parentage chain diagram updated to show nil links for frame #1; - added example for non-BPF_DW writes behavior; - explanation in "Read marks propagation for cache hits" is reworked. - Changes from v2 to v3: - lot's of grammatical / wording fixes as suggested by David Vernet; - "Register parentage chains" section is fixed to reflect what happens to r1-r5 when function call is processed (as suggested by David and Alexei); - Example in "Liveness marks tracking" section updated to explain why partial writes should not lead to REG_LIVE_WRITTEN marks (suggested by David); - "Read marks propagation for cache hits" section updates: - Explanation updated to hint why read marks should be propagated before jumping to example (suggested by David); - Removed box around B/D in the diagram updated (suggested by Alexei). - Changes from v3 to v4 (suggested by Edward Cree): - register parentage chain diagram updated to explain why r6 mark is not propagated; - read mark propagation algorithm pseudo-code fixed to correctly show "if state->live & REG_LIVE_WRITTEN" stop condition; - general wording improvements in section "Liveness marks tracking". [1] https://lore.kernel.org/bpf/20230124220343.2942203-1-eddyz87@gmail.com/ [2] https://lore.kernel.org/bpf/20230130182400.630997-1-eddyz87@gmail.com/ [3] https://lore.kernel.org/bpf/20230131181118.733845-1-eddyz87@gmail.com/ ==================== Signed-off-by: -
Eduard Zingerman authored
This is a followup for [1], adds an overview for the register liveness tracking, covers the following points: - why register liveness tracking is useful; - how register parentage chains are constructed; - how liveness marks are applied using the parentage chains. [1] https://lore.kernel.org/bpf/CAADnVQKs2i1iuZ5SUGuJtxWVfGYR9kDgYKhq3rNV+kBLQCu7rA@mail.gmail.com/ Signed-off-by:
Eduard Zingerman <eddyz87@gmail.com> Reviewed-by:
Edward Cree <ecree.xilinx@gmail.com> Link: https://lore.kernel.org/r/20230202125713.821931-2-eddyz87@gmail.com Signed-off-by:
-
Ilya Leoshkevich authored
xdp_synproxy/xdp fails in CI with: Error: bpf_tc_hook_create: File exists The XDP version of the test should not be calling bpf_tc_hook_create(); the reason it's happening anyway is that if we don't specify --tc on the command line, tc variable remains uninitialized. Fixes: 784d5dc0 ("selftests/bpf: Add selftests for raw syncookie helpers in TC mode") Reported-by:Joanne Koong <joannelkoong@gmail.com> Signed-off-by:
Ilya Leoshkevich <iii@linux.ibm.com> Link: https://lore.kernel.org/r/20230202235335.3403781-1-iii@linux.ibm.com Signed-off-by:
-
- 02 Feb, 2023 2 commits
-
-
Ye Xingchen authored
The linux/net_tstamp.h is included more than once, thus clean it up. Signed-off-by:
Ye Xingchen <ye.xingchen@zte.com.cn> Signed-off-by:
Daniel Borkmann <daniel@iogearbox.net> Link: https://lore.kernel.org/bpf/202301311440516312161@zte.com.cn
-
Stanislav Fomichev authored
We only need to consume TX completion instead of refilling 'fill' ring. It's currently not an issue because we never RX more than 8 packets. Fixes: e2a46d54 ("selftests/bpf: Verify xdp_metadata xdp->af_xdp path") Signed-off-by:
-
- 01 Feb, 2023 19 commits
-
-
Jesper Dangaard Brouer authored
The ifname char pointer is taken directly from the command line as input and the string is copied directly into struct ifreq via strcpy. This makes it easy to corrupt other members of ifreq and generally do stack overflows. Most often the ioctl will fail with: ./xdp_hw_metadata: ioctl(SIOCETHTOOL): Bad address As people will likely copy-paste code for getting NIC queue channels (rxq_num) and enabling HW timestamping (hwtstamp_ioctl) lets make this code a bit more secure by using strncpy. Fixes: 297a3f12 ("selftests/bpf: Simple program to dump XDP RX metadata") Signed-off-by:
-
Jesper Dangaard Brouer authored
The glibc error reporting function error(): void error(int status, int errnum, const char *format, ...); The status argument should be a positive value between 0-255 as it is passed over to the exit(3) function as the value as the shell exit status. The least significant byte of status (i.e., status & 0xFF) is returned to the shell parent. Fix this by using 1 instead of -1. As 1 corresponds to C standard constant EXIT_FAILURE. Fixes: 297a3f12 ("selftests/bpf: Simple program to dump XDP RX metadata") Signed-off-by:
-
Jesper Dangaard Brouer authored
Using xdp_hw_metadata I experince Segmentation fault after seeing "detaching bpf program....". On my system the segfault happened when accessing bpf_obj->skeleton in xdp_hw_metadata__destroy(bpf_obj) call. That doesn't make any sense as this memory have not been freed by program at this point in time. Prior to calling xdp_hw_metadata__destroy(bpf_obj) the function close_xsk() is called for each RX-queue xsk. The real bug lays in close_xsk() that unmap via munmap() the wrong memory pointer. The call xsk_umem__delete(xsk->umem) will free xsk->umem, thus the call to munmap(xsk->umem, UMEM_SIZE) will have unpredictable behavior. And man page explain subsequent references to these pages will generate SIGSEGV. Unmapping xsk->umem_area instead removes the segfault. Fixes: 297a3f12 ("selftests/bpf: Simple program to dump XDP RX metadata") Signed-off-by:
-
Jesper Dangaard Brouer authored
The AF_XDP userspace part of xdp_hw_metadata see non-zero as a signal of the availability of rx_timestamp and rx_hash in data_meta area. The kernel-side BPF-prog code doesn't initialize these members when kernel returns an error e.g. -EOPNOTSUPP. This memory area is not guaranteed to be zeroed, and can contain garbage/previous values, which will be read and interpreted by AF_XDP userspace side. Tested this on different drivers. The experiences are that for most packets they will have zeroed this data_meta area, but occasionally it will contain garbage data. Example of failure tested on ixgbe: poll: 1 (0) xsk_ring_cons__peek: 1 0x18ec788: rx_desc[0]->addr=100000000008000 addr=8100 comp_addr=8000 rx_hash: 3697961069 rx_timestamp: 9024981991734834796 (sec:9024981991.7348) 0x18ec788: complete idx=8 addr=8000 Converting to date: date -d @9024981991 2255-12-28T20:26:31 CET I choose a simple fix in this patch. When kfunc fails or isn't supported assign zero to the corresponding struct meta value. It's up to the individual BPF-programmer to do something smarter e.g. that fits their use-case, like getting a software timestamp and marking a flag that gives the type of timestamp. Fixes: 297a3f12 ("selftests/bpf: Simple program to dump XDP RX metadata") Signed-off-by:
-
Jesper Dangaard Brouer authored
The function close_xsk() unmap via munmap() the wrong memory pointer. The call xsk_umem__delete(xsk->umem) have already freed xsk->umem. Thus the call to munmap(xsk->umem, UMEM_SIZE) will have unpredictable behavior that can lead to Segmentation fault elsewhere, as man page explain subsequent references to these pages will generate SIGSEGV. Fixes: e2a46d54 ("selftests/bpf: Verify xdp_metadata xdp->af_xdp path") Reported-by:
-
Daniel Borkmann authored
David Vernet says: ==================== This is v3 of the patchset [0]. v2 can be found at [1]. [0]: https://lore.kernel.org/bpf/Y7kCsjBZ%2FFrsWW%2Fe@maniforge.lan/T/ [1]: https://lore.kernel.org/lkml/20230123171506.71995-1-void@manifault.com/ Changelog: ---------- v2 -> v3: - Go back to the __bpf_kfunc approach from v1. The BPF_KFUNC macro received pushback as it didn't match the more typical EXPORT_SYMBOL* APIs used elsewhere in the kernel. It's the longer term plan, but for now we're proposing something less controversial to fix kfuncs and BTF encoding. - Add __bpf_kfunc macro to newly added cpumask kfuncs. - Add __bpf_kfunc macro to newly added XDP metadata kfuncs, which were failing to be BTF encoded in the thread in [2]. - Update patch description(s) to reference the discussions in [2]. - Add a selftest that validates that a static kfunc with unused args is properly BTF encoded and can be invoked. [2]: https://lore.kernel.org/all/fe5d42d1-faad-d05e-99ad-1c2c04776950@oracle.com/ v1 -> v2: - Wrap entire function signature in BPF_KFUNC macro instead of using __bpf_kfunc tag (Kumar) - Update all kfunc definitions to use this macro. - Update kfuncs.rst documentation to describe and illustrate the macro. - Also clean up a few small parts of kfuncs.rst, e.g. some grammar, and in general making it a bit tighter. ==================== Signed-off-by:
-
David Vernet authored
kfuncs are allowed to be static, or not use one or more of their arguments. For example, bpf_xdp_metadata_rx_hash() in net/core/xdp.c is meant to be implemented by drivers, with the default implementation just returning -EOPNOTSUPP. As described in [0], such kfuncs can have their arguments elided, which can cause BTF encoding to be skipped. The new __bpf_kfunc macro should address this, and this patch adds a selftest which verifies that a static kfunc with at least one unused argument can still be encoded and invoked by a BPF program. Signed-off-by:
-
David Vernet authored
Now that we have the __bpf_kfunc tag, we should use add it to all existing kfuncs to ensure that they'll never be elided in LTO builds. Signed-off-by:
-
David Vernet authored
Now that the __bpf_kfunc macro has been added to linux/btf.h, include a blurb about it in the kfuncs.rst file. In order for the macro to successfully render with .. kernel-doc, we'll also need to add it to the c_id_attributes array. Signed-off-by:
-
David Vernet authored
kfuncs are functions defined in the kernel, which may be invoked by BPF programs. They may or may not also be used as regular kernel functions, implying that they may be static (in which case the compiler could e.g. inline it away, or elide one or more arguments), or it could have external linkage, but potentially be elided in an LTO build if a function is observed to never be used, and is stripped from the final kernel binary. This has already resulted in some issues, such as those discussed in [0] wherein changes in DWARF that identify when a parameter has been optimized out can break BTF encodings (and in general break the kfunc). [0]: https://lore.kernel.org/all/1675088985-20300-2-git-send-email-alan.maguire@oracle.com/ We therefore require some convenience macro that kfunc developers can use just add to their kfuncs, and which will prevent all of the above issues from happening. This is in contrast with what we have today, where some kfunc definitions have "noinline", some have "__used", and others are static and have neither. Note that longer term, this mechanism may be replaced by a macro that more closely resembles EXPORT_SYMBOL_GPL(), as described in [1]. For now, we're going with this shorter-term approach to fix existing issues in kfuncs. [1]: https://lore.kernel.org/lkml/Y9AFT4pTydKh+PD3@maniforge.lan/ Note as well that checkpatch complains about this patch with the following: ERROR: Macros with complex values should be enclosed in parentheses +#define __bpf_kfunc __used noinline There seems to be a precedent for using this pattern in other places such as compiler_types.h (see e.g. __randomize_layout and noinstr), so it seems appropriate. Signed-off-by:
-
Daniel Borkmann authored
Maciej Fijalkowski says: ==================== Although this work started as an effort to add multi-buffer XDP support to ice driver, as usual it turned out that some other side stuff needed to be addressed, so let me give you an overview. First patch adjusts legacy-rx in a way that it will be possible to refer to skb_shared_info being at the end of the buffer when gathering up frame fragments within xdp_buff. Then, patches 2-9 prepare ice driver in a way that actual multi-buffer patches will be easier to swallow. 10 and 11 are the meat. What is worth mentioning is that this set actually *fixes* things as patch 11 removes the logic based on next_dd/rs and we previously stepped away from this for ice_xmit_zc(). Currently, AF_XDP ZC XDP_TX workload is off as there are two cleaning sides that can be triggered and two of them work on different internal logic. This set unifies that and allows us to improve the performance by 2x with a trick on the last (13) patch. 12th is a simple cleanup of no longer fields from Tx ring. I might be wrong but I have not seen anyone reporting performance impact among patches that add XDP multi-buffer support to a particular driver. Numbers below were gathered via xdp_rxq_info and xdp_redirect_map on 1500 MTU: XDP_DROP +1% XDP_PASS -1,2% XDP_TX -0,5% XDP_REDIRECT -3,3% Cherry on top, which is not directly related to mbuf support (last patch): XDP_TX ZC +126% Target the we agreed on was to not degrade performance for any action by anything that would be over 5%, so our goal was met. Basically this set keeps the performance where it was. Redirect is slower due to more frequent tail bumps. ==================== Signed-off-by:
Alexander Lobakin <alexandr.lobakin@intel.com>
-
Maciej Fijalkowski authored
Let us store pointer to xdp_buff that came from xsk_buff_pool on tx_buf so that it will be possible to recycle it via xsk_buff_free() on Tx cleaning side. This way it is not necessary to do expensive copy to another xdp_buff backed by a newly allocated page. Signed-off-by:
Maciej Fijalkowski <maciej.fijalkowski@intel.com> Signed-off-by:
-
Maciej Fijalkowski authored
Now that both ZC and standard XDP data paths stopped using Tx logic based on next_dd and next_rs fields, we can safely remove these fields and shrink Tx ring structure. Signed-off-by:
-
Maciej Fijalkowski authored
Similarly as for Rx side in previous patch, logic on XDP Tx in ice driver needs to be adjusted for multi-buffer support. Specifically, the way how HW Tx descriptors are produced and cleaned. Currently, XDP_TX works on strict ring boundaries, meaning it sets RS bit (on producer side) / looks up DD bit (on consumer/cleaning side) every quarter of the ring. It means that if for example multi buffer frame would span across the ring quarter boundary (say that frame consists of 4 frames and we start from 62 descriptor where ring is sized to 256 entries), RS bit would be produced in the middle of multi buffer frame, which would be a broken behavior as it needs to be set on the last descriptor of the frame. To make it work, set RS bit at the last descriptor from the batch of frames that XDP_TX action was used on and make the first entry remember the index of last descriptor with RS bit set. This way, cleaning side can take the index of descriptor with RS bit, look up DD bit's presence and clean from first entry to last. In order to clean up the code base introduce the common ice_set_rs_bit() which will return index of descriptor that got RS bit produced on so that standard driver can store this within proper ice_tx_buf and ZC driver can simply ignore return value. Co-developed-by:
Martyna Szapar-Mudlaw <martyna.szapar-mudlaw@linux.intel.com> Signed-off-by:
-
Maciej Fijalkowski authored
Ice driver needs to be a bit reworked on Rx data path in order to support multi-buffer XDP. For skb path, it currently works in a way that Rx ring carries pointer to skb so if driver didn't manage to combine fragmented frame at current NAPI instance, it can restore the state on next instance and keep looking for last fragment (so descriptor with EOP bit set). What needs to be achieved is that xdp_buff needs to be combined in such way (linear + frags part) in the first place. Then skb will be ready to go in case of XDP_PASS or BPF program being not present on interface. If BPF program is there, it would work on multi-buffer XDP. At this point xdp_buff resides directly on Rx ring, so given the fact that skb will be built straight from xdp_buff, there will be no further need to carry skb on Rx ring. Besides removing skb pointer from Rx ring, lots of members have been moved around within ice_rx_ring. First and foremost reason was to place rx_buf with xdp_buff on the same cacheline. This means that once we touch rx_buf (which is a preceding step before touching xdp_buff), xdp_buff will already be hot in cache. Second thing was that xdp_rxq is used rather rarely and it occupies a separate cacheline, so maybe it is better to have it at the end of ice_rx_ring. Other change that affects ice_rx_ring is the introduction of ice_rx_ring::first_desc. Its purpose is twofold - first is to propagate rx_buf->act to all the parts of current xdp_buff after running XDP program, so that ice_put_rx_buf() that got moved out of the main Rx processing loop will be able to tak an appriopriate action on each buffer. Second is for ice_construct_skb(). ice_construct_skb() has a copybreak mechanism which had an explicit impact on xdp_buff->skb conversion in the new approach when legacy Rx flag is toggled. It works in a way that linear part is 256 bytes long, if frame is bigger than that, remaining bytes are going as a frag to skb_shared_info. This means while memcpying frags from xdp_buff to newly allocated skb, care needs to be taken when picking the destination frag array entry. Upon the time ice_construct_skb() is called, when dealing with fragmented frame, current rx_buf points to the *last* fragment, but copybreak needs to be done against the first one. That's where ice_rx_ring::first_desc helps. When frame building spans across NAPI polls (DD bit is not set on current descriptor and xdp->data is not NULL) with current Rx buffer handling state there might be some problems. Since calls to ice_put_rx_buf() were pulled out of the main Rx processing loop and were scoped from cached_ntc to current ntc, remember that now mentioned function relies on rx_buf->act, which is set within ice_run_xdp(). ice_run_xdp() is called when EOP bit was found, so currently we could put Rx buffer with rx_buf->act being *uninitialized*. To address this, change scoping to rely on first_desc on both boundaries instead. This also implies that cleaned_count which is used as an input to ice_alloc_rx_buffers() and tells how many new buffers should be refilled has to be adjusted. If it stayed as is, what could happen is a case where ntc would go over ntu. Therefore, remove cleaned_count altogether and use against allocing routine newly introduced ICE_RX_DESC_UNUSED() macro which is an equivalent of ICE_DESC_UNUSED() dedicated for Rx side and based on struct ice_rx_ring::first_desc instead of next_to_clean. Signed-off-by:
-
Maciej Fijalkowski authored
SKB path calculates truesize on three different functions, which could be avoided as xdp_buff carries the already calculated truesize under xdp_buff::frame_sz. If ice_add_rx_frag() is adjusted to take the xdp_buff as an input just like functions responsible for creating sk_buff initially, codebase could be simplified by removing these redundant recalculations and rely on xdp_buff::frame_sz instead. Signed-off-by:
-
Maciej Fijalkowski authored
Currently ice_finalize_xdp_rx() is called only when xdp_prog is present on VSI, which is a good thing. However, this optimization can be enhanced and check only if any of the XDP_TX/XDP_REDIRECT took place in current Rx processing. Non-zero value of @xdp_xmit indicates that xdp_prog is present on VSI. This way XDP_DROP-based workloads will not suffer from unnecessary calls to external function. Signed-off-by:
-
Maciej Fijalkowski authored
This should have been used in there from day 1, let us address that before introducing XDP multi-buffer support for Rx side. Signed-off-by:
-
Maciej Fijalkowski authored
Currently calls to ice_put_rx_buf() are sprinkled through ice_clean_rx_irq() - first place is for explicit flow director's descriptor handling, second is after running XDP prog and the last one is after taking care of skb. 1st callsite was actually only for ntc bump purpose, as Rx buffer to be recycled is not even passed to a function. It is possible to walk through Rx buffers processed in particular NAPI cycle by caching ntc from beginning of the ice_clean_rx_irq(). To do so, let us store XDP verdict inside ice_rx_buf, so action we need to take on will be known. For XDP prog absence, just store ICE_XDP_PASS as a verdict. Signed-off-by:
-
