summaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* samples/bpf: fix broken cgroup socket testingDaniel T. Lee2023-01-151-6/+6
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | Currently, executing test_cgrp2_sock2 fails due to wrong section header. This 'cgroup/sock1' style section is previously used at 'samples/bpf_load' (deprecated) BPF loader. Because this style isn't supported in libbpf, this commit fixes this problem by correcting the section header. $ sudo ./test_cgrp2_sock2.sh libbpf: prog 'bpf_prog1': missing BPF prog type, check ELF section name 'cgroup/sock1' libbpf: prog 'bpf_prog1': failed to load: -22 libbpf: failed to load object './sock_flags_kern.o' ERROR: loading BPF object file failed In addition, this BPF program filters ping packets by comparing whether the socket type uses SOCK_RAW. However, after the ICMP socket[1] was developed, ping sends ICMP packets using SOCK_DGRAM. Therefore, in this commit, the packet filtering is changed to use SOCK_DGRAM instead of SOCK_RAW. $ strace --trace socket ping -6 -c1 -w1 ::1 socket(AF_INET6, SOCK_DGRAM, IPPROTO_ICMPV6) = 3 [1]: https://lwn.net/Articles/422330/ Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> Link: https://lore.kernel.org/r/20230115071613.125791-5-danieltimlee@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
* samples/bpf: fix broken lightweight tunnel testingDaniel T. Lee2023-01-152-22/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The test_lwt_bpf is a script that tests the functionality of BPF through the output of the ftrace with bpf_trace_printk. Currently, this program is not operating normally for several reasons. First of all, this test script can't parse the ftrace results properly. GNU sed tries to be as greedy as possible when attempting pattern matching. Due to this, cutting metadata (such as timestamp) from the log entry of ftrace doesn't work properly, and also desired log isn't extracted properly. To make sed stripping clearer, 'nocontext-info' option with the ftrace has been used to remove metadata from the log. Also, instead of using unclear pattern matching, this commit specifies an explicit parse pattern. Also, unlike before when this test was introduced, the way bpf_trace_printk behaves has changed[1]. The previous bpf_trace_printk had to always have '\n' in order to print newline, but now that the bpf_trace_printk call includes newline by default, so '\n' is no longer needed. Lastly with the lwt ENCAP_BPF out, the context information with the sk_buff protocol is preserved. Therefore, this commit changes the previous test result from 'protocol 0' to 'protocol 8', which means ETH_P_IP. [1]: commit ac5a72ea5c89 ("bpf: Use dedicated bpf_trace_printk event instead of trace_printk()") Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> Link: https://lore.kernel.org/r/20230115071613.125791-4-danieltimlee@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
* samples/bpf: refactor BPF functionality testing scriptsDaniel T. Lee2023-01-155-15/+22
| | | | | | | | | | | | | | | | | Currently, some test scripts are experiencing minor errors related to executing tests. $ sudo ./test_cgrp2_sock.sh ./test_cgrp2_sock.sh: 22: test_cgrp2_sock: not found This problem occurs because the path to the execution target is not properly specified. Therefore, this commit solves this problem by specifying a relative path to its executables. This commit also makes a concise refactoring of hard-coded BPF program names. Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> Link: https://lore.kernel.org/r/20230115071613.125791-3-danieltimlee@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
* samples/bpf: ensure ipv6 is enabled before running testsDaniel T. Lee2023-01-153-1/+8
| | | | | | | | | | | | | | | | Currently, a few of BPF tests use ipv6 functionality. The problem here is that if ipv6 is disabled, these tests will fail, and even if the test fails, it will not tell you why it failed. $ sudo ./test_cgrp2_sock2.sh RTNETLINK answers: Permission denied In order to fix this, this commit ensures ipv6 is enabled prior to running tests. Signed-off-by: Daniel T. Lee <danieltimlee@gmail.com> Link: https://lore.kernel.org/r/20230115071613.125791-2-danieltimlee@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
* Merge branch 'bpf: Add ipip6 and ip6ip decap support for bpf_skb_adjust_room()'Martin KaFai Lau2023-01-155-9/+142
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Ziyang Xuan says: ==================== Add ipip6 and ip6ip decap support for bpf_skb_adjust_room(). Main use case is for using cls_bpf on ingress hook to decapsulate IPv4 over IPv6 and IPv6 over IPv4 tunnel packets. And add ipip6 and ip6ip decap testcases to verify that bpf_skb_adjust_room() correctly decapsulate ipip6 and ip6ip tunnel packets. $./test_tc_tunnel.sh ipip encap 192.168.1.1 to 192.168.1.2, type ipip, mac none len 100 test basic connectivity 0 test bpf encap without decap (expect failure) Ncat: TIMEOUT. 1 test bpf encap with tunnel device decap 0 test bpf encap with bpf decap 0 OK ipip6 encap 192.168.1.1 to 192.168.1.2, type ipip6, mac none len 100 test basic connectivity 0 test bpf encap without decap (expect failure) Ncat: TIMEOUT. 1 test bpf encap with tunnel device decap 0 test bpf encap with bpf decap 0 OK ip6ip6 encap fd::1 to fd::2, type ip6tnl, mac none len 100 test basic connectivity 0 test bpf encap without decap (expect failure) Ncat: TIMEOUT. 1 test bpf encap with tunnel device decap 0 test bpf encap with bpf decap 0 OK sit encap fd::1 to fd::2, type sit, mac none len 100 test basic connectivity 0 test bpf encap without decap (expect failure) Ncat: TIMEOUT. 1 test bpf encap with tunnel device decap 0 test bpf encap with bpf decap 0 OK ... OK. All tests passed v3: - Fix compilation failure of selftests/bpf. - Combine two new branches in bpf_skb_adjust_room(). - Simplify description for new flags BPF_F_ADJ_ROOM_DECAP_L3_IP*. v2: - Use decap flags to indicate the new IP header. Do not rely on skb->encapsulation. ==================== Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
| * selftests/bpf: add ipip6 and ip6ip decap to test_tc_tunnelZiyang Xuan2023-01-152-8/+98
| | | | | | | | | | | | | | | | | | | | Add ipip6 and ip6ip decap testcases. Verify that bpf_skb_adjust_room() correctly decapsulate ipip6 and ip6ip tunnel packets. Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> Reviewed-by: Willem de Bruijn <willemb@google.com> Link: https://lore.kernel.org/r/dfd2d8cfdf9111bd129170d4345296f53bee6a67.1673574419.git.william.xuanziyang@huawei.com Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
| * bpf: Add ipip6 and ip6ip decap support for bpf_skb_adjust_room()Ziyang Xuan2023-01-153-1/+44
|/ | | | | | | | | | | | | | | Add ipip6 and ip6ip decap support for bpf_skb_adjust_room(). Main use case is for using cls_bpf on ingress hook to decapsulate IPv4 over IPv6 and IPv6 over IPv4 tunnel packets. Add two new flags BPF_F_ADJ_ROOM_DECAP_L3_IPV{4,6} to indicate the new IP header version after decapsulating the outer IP header. Suggested-by: Willem de Bruijn <willemb@google.com> Signed-off-by: Ziyang Xuan <william.xuanziyang@huawei.com> Reviewed-by: Willem de Bruijn <willemb@google.com> Link: https://lore.kernel.org/r/b268ec7f0ff9431f4f43b1b40ab856ebb28cb4e1.1673574419.git.william.xuanziyang@huawei.com Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
* selftests/bpf: Fix missing space errorRoberto Valenzuela2023-01-131-1/+1
| | | | | | | | | | | | Add the missing space after 'dest' variable assignment. This change will resolve the following checkpatch.pl script error: ERROR: spaces required around that '+=' (ctx:VxW) Signed-off-by: Roberto Valenzuela <valenzuelarober@gmail.com> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Link: https://lore.kernel.org/bpf/20230113180257.39769-1-valenzuelarober@gmail.com
* libbpf: Replace '.' with '_' in legacy kprobe event nameMenglong Dong2023-01-131-0/+7
| | | | | | | | | | | | | | '.' is not allowed in the event name of kprobe. Therefore, we will get a EINVAL if the kernel function name has a '.' in legacy kprobe attach case, such as 'icmp_reply.constprop.0'. In order to adapt this case, we need to replace the '.' with other char in gen_kprobe_legacy_event_name(). And I use '_' for this propose. Signed-off-by: Menglong Dong <imagedong@tencent.com> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Reviewed-by: Alan Maguire <alan.maguire@oracle.com> Link: https://lore.kernel.org/bpf/20230113093427.1666466-1-imagedong@tencent.com
* bpftool: Always disable stack protection for BPF objectsHolger Hoffstätte2023-01-131-1/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the clang toolchain has stack protection enabled in order to be consistent with gcc - which just happens to be the case on Gentoo - the bpftool build fails: [...] clang \ -I. \ -I/tmp/portage/dev-util/bpftool-6.0.12/work/linux-6.0/tools/include/uapi/ \ -I/tmp/portage/dev-util/bpftool-6.0.12/work/linux-6.0/tools/bpf/bpftool/bootstrap/libbpf/include \ -g -O2 -Wall -target bpf -c skeleton/pid_iter.bpf.c -o pid_iter.bpf.o clang \ -I. \ -I/tmp/portage/dev-util/bpftool-6.0.12/work/linux-6.0/tools/include/uapi/ \ -I/tmp/portage/dev-util/bpftool-6.0.12/work/linux-6.0/tools/bpf/bpftool/bootstrap/libbpf/include \ -g -O2 -Wall -target bpf -c skeleton/profiler.bpf.c -o profiler.bpf.o skeleton/profiler.bpf.c:40:14: error: A call to built-in function '__stack_chk_fail' is not supported. int BPF_PROG(fentry_XXX) ^ skeleton/profiler.bpf.c:94:14: error: A call to built-in function '__stack_chk_fail' is not supported. int BPF_PROG(fexit_XXX) ^ 2 errors generated. [...] Since stack-protector makes no sense for the BPF bits just unconditionally disable it. Bug: https://bugs.gentoo.org/890638 Signed-off-by: Holger Hoffstätte <holger@applied-asynchrony.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Acked-by: Quentin Monnet <quentin@isovalent.com> Link: https://lore.kernel.org/bpf/74cd9d2e-6052-312a-241e-2b514a75c92c@applied-asynchrony.com
* Merge branch 'selftests/xsk: speed-ups, fixes, and new XDP programs'Alexei Starovoitov2023-01-118-947/+316
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Magnus Karlsson says: ==================== This is a patch set of various performance improvements, fixes, and the introduction of more than one XDP program to the xsk selftests framework so we can test more things in the future such as upcoming multi-buffer and metadata support for AF_XDP. The new programs just reuse the framework that all the other eBPF selftests use. The new feature is used to implement one new test that does XDP_DROP on every other packet. More tests using this will be added in future commits. Contents: * The run-time of the test suite is cut by 10x when executing the tests on a real NIC, by only attaching the XDP program once per mode tested, instead of once per test program. * Over 700 lines of code have been removed. The xsk.c control file was moved straight over from libbpf when the xsk support was deprecated there. As it is now not used as library code that has to work with all kinds of versions of Linux, a lot of code could be dropped or simplified. * Add a new command line option "-d" that can be used when a test fails and you want to debug it with gdb or some other debugger. The option creates the two veth netdevs and prints them to the screen without deleting them afterwards. This way these veth netdevs can be used when running xskxceiver in a debugger. * Implemented the possibility to load external XDP programs so we can have more than the default one. This feature is used to implement a test where every other packet is dropped. Good exercise for the recycling mechanism of the xsk buffer pool used in zero-copy mode. * Various clean-ups and small fixes in patches 1 to 5. None of these fixes has any impact on the correct execution of the tests when they pass, though they can be irritating when a test fails. IMHO, they do not need to go to bpf as they will not fix anything there. The first version of patches 1, 2, and 4 where previously sent to bpf, but has now been included here. v2 -> v3: * Fixed compilation error for llvm [David] * Made the function xsk_is_in_drv_mode(ifobj) more generic by changing it to xsk_is_in_mode(ifobj, xdp_mode) [Maciej] * Added Maciej's acks to all the patches v1 -> v2: * Fixed spelling error in commit message of patch #6 [Björn] * Added explanation on why it is safe to use C11 atomics in patch #7 [Daniel] * Put all XDP programs in the same file so that adding more XDP programs to xskxceiver.c becomes more scalable in patches #11 and #12 [Maciej] * Removed more dead code in patch #8 [Maciej] * Removed stale %s specifier in error print, patch #9 [Maciej] * Changed name of XDP_CONSUMES_SOME_PACKETS to XDP_DROP_HALF to hopefully make it clearer [Maciej] * ifobj_rx and ifobj_tx name changes in patch #13 [Maciej] * Simplified XDP attachment code in patch #15 [Maciej] Patches: 1-5: Small fixes and clean-ups 6: New convenient debug option when using a debugger such as gdb 7-8: Removal of unnecessary code 9: Add the ability to load external XDP programs 10-11: Removal of more unnecessary code 12: Implement a new test where every other packet is XDP_DROP:ed 13: Unify the thread dispatching code 14-15: Simplify the way tests are written when using custom packet_streams or custom XDP programs Thanks: Magnus ==================== Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: automatically switch XDP programsMagnus Karlsson2023-01-114-68/+96
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Implement automatic switching of XDP programs and execution modes if needed by a test. This makes it much simpler to write a test as it only has to say what XDP program it needs if it is not the default one. This also makes it possible to remove the initial explicit attachment of the XDP program as well as the explicit mode switch in the code. These are now handled by the same code that just checks if a switch is necessary, so no special cases are needed. The default XDP program for all tests is one that sends all packets to the AF_XDP socket. If you need another one, please use the new function test_spec_set_xdp_prog() to specify what XDP programs and maps to use for this test. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-16-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: automatically restore packet streamMagnus Karlsson2023-01-111-14/+1
| | | | | | | | | | | | | | | | | | | | Automatically restore the default packet stream if needed at the end of each test. This so that test writers do not forget to do this. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-15-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: merge dual and single thread dispatchersMagnus Karlsson2023-01-111-66/+54
| | | | | | | | | | | | | | | | | | | | | | Make the thread dispatching code common by unifying the dual and single thread dispatcher code. This so we do not have to add code in two places in upcoming commits. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-14-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: add test when some packets are XDP_DROPedMagnus Karlsson2023-01-113-0/+43
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a new test where some of the packets are not passed to the AF_XDP socket and instead get a XDP_DROP verdict. This is important as it tests the recycling mechanism of the buffer pool. If a packet is not sent to the AF_XDP socket, the buffer the packet resides in is instead recycled so it can be used again without the round-trip to user space. The test introduces a new XDP program that drops every other packet. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-13-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: get rid of built-in XDP programMagnus Karlsson2023-01-116-106/+92
| | | | | | | | | | | | | | | | | | | | | | | | Get rid of the built-in XDP program that was part of the old libbpf code in xsk.c and replace it with an eBPF program build using the framework by all the other bpf selftests. This will form the base for adding more programs in later commits. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-12-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: remove unnecessary code in control pathMagnus Karlsson2023-01-113-631/+3
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Remove unnecessary code in the control path. This is located in the file xsk.c that was moved from libbpf when the xsk support there was deprecated. Some of the code there is not needed anymore as the selftests are only guaranteed to run on the kernel it is shipped with. Therefore, all the code that has to deal with compatibility of older kernels can be dropped and also any other function that is not of any use for the tests. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-11-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: load and attach XDP program only once per modeMagnus Karlsson2023-01-114-87/+162
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Load and attach the XDP program only once per XDP mode that is being executed. Today, the XDP program is loaded and attached for every test, then unloaded, which takes a long time on real NICs, since they have to reconfigure their HW, in contrast to veth. The test suite now completes in 21 seconds, instead of 207 seconds previously on my machine. This is a speed-up of around 10x. This is accomplished by moving the XDP loading from the worker threads to the main thread and replacing the XDP loading interfaces of xsk.c that was taken from the xsk support in libbpf, with something more explicit that is more useful for these tests. Instead, the relevant file descriptors and ifindexes are just passed down to the new functions. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-10-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: remove namespacesMagnus Karlsson2023-01-114-74/+20
| | | | | | | | | | | | | | | | | | | | Remove the namespaces used as they fill no function. This will simplify the code for speeding up the tests in the following commits. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-9-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: replace asm acquire/release implementationsMagnus Karlsson2023-01-111-76/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Replace our own homegrown assembly store/release and load/acquire implementations with the HW agnositic atomic APIs C11 offers. This to make the code more portable, easier to read, and reduce the maintenance burden. The original code used load-acquire and store-release barriers hand-coded in assembly. Since C11, these kind of operations are offered as built-ins in gcc and llvm. The load-acquire operation prevents hoisting of non-atomic memory operations to before this operation and it corresponds to the __ATOMIC_ACQUIRE operation in the built-in atomics. The store-release operation prevents hoisting of non-atomic memory operations to after this operation and it corresponds to the __ATOMIC_RELEASE operation in the built-in atomics. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-8-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: add debug option for creating netdevsMagnus Karlsson2023-01-111-1/+10
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add a new option to the test_xsk.sh script that only creates the two veth netdevs and the extra namespace, then exits without running any tests. The failed test can then be executed in the debugger without having to create the netdevs and namespace manually. For ease-of-use, the veth netdevs to use are printed so they can be copied into the debugger. Here is an example how to use it: > sudo ./test_xsk.sh -d veth10 veth11 > gdb xskxceiver In gdb: run -i veth10 -i veth11 And now the test cases can be debugged with gdb. If you want to debug the test suite on a real NIC in loopback mode, there is no need to use this feature as you already know the netdev of your NIC. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-7-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: remove unused variable outstanding_txMagnus Karlsson2023-01-111-2/+0
| | | | | | | | | | | | | | | | | | Remove the unused variable outstanding_tx. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-6-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: print correct error codes when exitingMagnus Karlsson2023-01-111-9/+9
| | | | | | | | | | | | | | | | | | | | | | | | Print the correct error codes when exiting the test suite due to some terminal error. Some of these had a switched sign and some of them printed zero instead of errno. Fixes: facb7cb2e909 ("selftests/bpf: Xsk selftests - SKB POLL, NOPOLL") Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-5-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: submit correct number of frames in populate_fill_ringMagnus Karlsson2023-01-111-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | Submit the correct number of frames in the function xsk_populate_fill_ring(). For the tests that set the flag use_addr_for_fill, uninitialized buffers were sent to the fill ring following the correct ones. This has no impact on the tests, since they only use the ones that were initialized. But for correctness, this should be fixed. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-4-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: do not close unused file descriptorsMagnus Karlsson2023-01-111-2/+11
| | | | | | | | | | | | | | | | | | | | | | | | Do not close descriptors that have never been used. File descriptor fields that are not in use are erroneously marked with the number 0, which is a valid fd. Mark unused fds with -1 instead and do not close these when deleting the socket. Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-3-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
| * selftests/xsk: print correct payload for packet dumpMagnus Karlsson2023-01-111-2/+2
|/ | | | | | | | | | | | Print the correct payload when the packet dump option is selected. The network to host conversion was forgotten and the payload was erronously declared to be an int instead of an unsigned int. Fixes: facb7cb2e909 ("selftests/bpf: Xsk selftests - SKB POLL, NOPOLL") Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com> Link: https://lore.kernel.org/r/20230111093526.11682-2-magnus.karlsson@gmail.com Signed-off-by: Alexei Starovoitov <ast@kernel.org>
* bpf_doc: Fix build error with older python versionsMichal Suchanek2023-01-111-1/+1
| | | | | | | | | | | | | | | | | | The ability to subscript match result as an array is only available since python 3.6. Existing code in bpf_doc uses the older group() interface but commit 8a76145a2ec2 adds code using the new interface. Use the old interface consistently to avoid build error on older distributions like the below: + make -j48 -s -C /dev/shm/kbuild/linux.33946/current ARCH=powerpc HOSTCC=gcc CROSS_COMPILE=powerpc64-suse-linux- clean TypeError: '_sre.SRE_Match' object is not subscriptable Fixes: 8a76145a2ec2 ("bpf: explicitly define BPF_FUNC_xxx integer values") Signed-off-by: Michal Suchanek <msuchanek@suse.de> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Acked-by: Quentin Monnet <quentin@isovalent.com> Link: https://lore.kernel.org/bpf/20230109113442.20946-1-msuchanek@suse.de
* libbpf: Fix map creation flags sanitizationLudovic L'Hours2023-01-101-1/+1
| | | | | | | | | | | As BPF_F_MMAPABLE flag is now conditionnaly set (by map_is_mmapable), it should not be toggled but disabled if not supported by kernel. Fixes: 4fcac46c7e10 ("libbpf: only add BPF_F_MMAPABLE flag for data maps with global vars") Signed-off-by: Ludovic L'Hours <ludovic.lhours@gmail.com> Acked-by: Jiri Olsa <jolsa@kernel.org> Link: https://lore.kernel.org/r/20230108182018.24433-1-ludovic.lhours@gmail.com Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
* bpftool: fix output for skipping kernel config checkChethan Suresh2023-01-101-4/+4
| | | | | | | | | | | | | When bpftool feature does not find kernel config files under default path or wrong format, do not output CONFIG_XYZ is not set. Skip kernel config check and continue. Signed-off-by: Chethan Suresh <chethan.suresh@sony.com> Signed-off-by: Kenta Tada <Kenta.Tada@sony.com> Acked-by: Quentin Monnet <quentin@isovalent.com> Link: https://lore.kernel.org/r/20230109023742.29657-1-chethan.suresh@sony.com Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
* bpf: btf: limit logging of ignored BTF mismatchesConnor O'Brien2023-01-101-11/+27
| | | | | | | | | | | | | | | | | | | | | | | Enabling CONFIG_MODULE_ALLOW_BTF_MISMATCH is an indication that BTF mismatches are expected and module loading should proceed anyway. Logging with pr_warn() on every one of these "benign" mismatches creates unnecessary noise when many such modules are loaded. Instead, handle this case with a single log warning that BTF info may be unavailable. Mismatches also result in calls to __btf_verifier_log() via __btf_verifier_log_type() or btf_verifier_log_member(), adding several additional lines of logging per mismatched module. Add checks to these paths to skip logging for module BTF mismatches in the "allow mismatch" case. All existing logging behavior is preserved in the default CONFIG_MODULE_ALLOW_BTF_MISMATCH=n case. Signed-off-by: Connor O'Brien <connoro@google.com> Acked-by: Yonghong Song <yhs@fb.com> Link: https://lore.kernel.org/r/20230107025331.3240536-1-connoro@google.com Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
* bpf, x86: Simplify the parsing logic of structure parametersPu Lehui2023-01-101-53/+48
| | | | | | | | | | | | Extra_nregs of structure parameters and nr_args can be added directly at the beginning, and using a flip flag to identifiy structure parameters. Meantime, renaming some variables to make them more sense. Signed-off-by: Pu Lehui <pulehui@huawei.com> Acked-by: Yonghong Song <yhs@fb.com> Link: https://lore.kernel.org/r/20230105035026.3091988-1-pulehui@huaweicloud.com Signed-off-by: Martin KaFai Lau <martin.lau@kernel.org>
* bpf: Replace 0-length arrays with flexible arraysKees Cook2023-01-101-3/+3
| | | | | | | | | | | | | | | | | | | | | | | Zero-length arrays are deprecated [1]. Replace struct bpf_array's union of 0-length arrays with flexible arrays. Detected with GCC 13, by using -fstrict-flex-arrays=3: arch/x86/net/bpf_jit_comp.c: In function 'bpf_tail_call_direct_fixup': arch/x86/net/bpf_jit_comp.c:606:37: warning: array subscript <unknown> is outside array bounds of 'void *[0]' [-Warray-bounds=] 606 | target = array->ptrs[poke->tail_call.key]; | ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~ In file included from include/linux/filter.h:9, from arch/x86/net/bpf_jit_comp.c:9: include/linux/bpf.h:1527:23: note: while referencing 'ptrs' 1527 | void *ptrs[0] __aligned(8); | ^~~~ [1] https://www.kernel.org/doc/html/latest/process/deprecated.html#zero-length-and-one-element-arrays Signed-off-by: Kees Cook <keescook@chromium.org> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Acked-by: Stanislav Fomichev <sdf@google.com> Link: https://lore.kernel.org/bpf/20230105192646.never.154-kees@kernel.org
* bpftool: Add missing quotes to libbpf bootstrap submake varsJames Hilliard2023-01-101-1/+1
| | | | | | | | | | | | | | | | | | | | | | When passing compiler variables like CC=$(HOSTCC) to a submake we must ensure the variable is quoted in order to handle cases where $(HOSTCC) may be multiple binaries. For example when using ccache $HOSTCC may be: "/usr/bin/ccache /usr/bin/gcc" If we pass CC without quotes like CC=$(HOSTCC) only the first "/usr/bin/ccache" part will be assigned to the CC variable which will cause an error due to dropping the "/usr/bin/gcc" part of the variable in the submake invocation. This fixes errors such as: /usr/bin/ccache: invalid option -- 'd' Signed-off-by: James Hilliard <james.hilliard1@gmail.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Acked-by: Quentin Monnet <quentin@isovalent.com> Link: https://lore.kernel.org/bpf/20230110014504.3120711-1-james.hilliard1@gmail.com
* bpf: Remove the unnecessary insn buffer comparisonHaiyue Wang2023-01-101-6/+0
| | | | | | | | | | | | The variable 'insn' is initialized to 'insn_buf' without being changed, only some helper macros are defined, so the insn buffer comparison is unnecessary. Just remove it. This missed removal back in 2377b81de527 ("bpf: split shared bpf_tcp_sock and bpf_sock_ops implementation"). Signed-off-by: Haiyue Wang <haiyue.wang@intel.com> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Acked-by: Stanislav Fomichev <sdf@google.com> Link: https://lore.kernel.org/bpf/20230108151258.96570-1-haiyue.wang@intel.com
* libbpf: Poison strlcpy()Rong Tao2023-01-061-2/+2
| | | | | | | | | | | Since commit 9fc205b413b3("libbpf: Add sane strncpy alternative and use it internally") introduce libbpf_strlcpy(), thus add strlcpy() to a poison list to prevent accidental use of it. Signed-off-by: Rong Tao <rongtao@cestc.cn> Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Acked-by: Stanislav Fomichev <sdf@google.com> Link: https://lore.kernel.org/bpf/tencent_5695A257C4D16B4413036BA1DAACDECB0B07@qq.com
* Merge branch 'devlink-unregister'David S. Miller2023-01-066-112/+137
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Jakub Kicinski says: ==================== devlink: remove the wait-for-references on unregister Move the registration and unregistration of the devlink instances under their instance locks. Don't perform the netdev-style wait for all references when unregistering the instance. Instead the devlink instance refcount will only ensure that the memory of the instance is not freed. All places which acquire access to devlink instances via a reference must check that the instance is still registered under the instance lock. This fixes the problem of the netdev code accessing devlink instances before they are registered. RFC: https://lore.kernel.org/all/20221217011953.152487-1-kuba@kernel.org/ - rewrite the cover letter - rewrite the commit message for patch 1 - un-export and rename devl_is_alive - squash the netdevsim patches ==================== Signed-off-by: David S. Miller <davem@davemloft.net>
| * netdevsim: move devlink registration under the instance lockJakub Kicinski2023-01-061-3/+8
| | | | | | | | | | | | | | | | | | To prevent races with netdev code accessing free devlink instances move the registration under the devlink instance lock. Core now waits for the instance to be registered before accessing it. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
| * netdevsim: rename a labelJakub Kicinski2023-01-061-2/+2
| | | | | | | | | | | | | | | | | | err_dl_unregister should unregister the devlink instance. Looks like renaming it was missed in one of the reshufflings. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
| * devlink: allow registering parameters after the instanceJakub Kicinski2023-01-061-11/+11
| | | | | | | | | | | | | | | | | | It's most natural to register the instance first and then its subobjects. Now that we can use the instance lock to protect the atomicity of all init - it should also be safe. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
| * devlink: don't require setting features before registrationJakub Kicinski2023-01-061-2/+0
| | | | | | | | | | | | | | | | | | | | | | Requiring devlink_set_features() to be run before devlink is registered is overzealous. devlink_set_features() itself is a leftover from old workarounds which were trying to prevent initiating reload before probe was complete. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: David S. Miller <davem@davemloft.net>
| * devlink: remove the registration guarantee of referencesJakub Kicinski2023-01-063-38/+30
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | The objective of exposing the devlink instance locks to drivers was to let them use these locks to prevent user space from accessing the device before it's fully initialized. This is difficult because devlink_unregister() waits for all references to be released, meaning that devlink_unregister() can't itself be called under the instance lock. To avoid this issue devlink_register() was moved after subobject registration a while ago. Unfortunately the netdev paths get a hold of the devlink instances _before_ they are registered. Ideally netdev should wait for devlink init to finish (synchronizing on the instance lock). This can't work because we don't know if the instance will _ever_ be registered (in case of failures it may not). The other option of returning an error until devlink_register() is called is unappealing (user space would get a notification netdev exist but would have to wait arbitrary amount of time before accessing some of its attributes). Weaken the guarantees of the devlink references. Holding a reference will now only guarantee that the memory of the object is around. Another way of looking at it is that the reference now protects the object not its "registered" status. Use devlink instance lock to synchronize unregistration. This implies that releasing of the "main" reference of the devlink instance moves from devlink_unregister() to devlink_free(). Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: David S. Miller <davem@davemloft.net>
| * devlink: always check if the devlink instance is registeredJakub Kicinski2023-01-064-12/+60
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Always check under the instance lock whether the devlink instance is still / already registered. This is a no-op for the most part, as the unregistration path currently waits for all references. On the init path, however, we may temporarily open up a race with netdev code, if netdevs are registered before the devlink instance. This is temporary, the next change fixes it, and this commit has been split out for the ease of review. Note that in case of iterating over sub-objects which have their own lock (regions and line cards) we assume an implicit dependency between those objects existing and devlink unregistration. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: David S. Miller <davem@davemloft.net>
| * devlink: protect devlink->dev by the instance lockJakub Kicinski2023-01-063-8/+11
| | | | | | | | | | | | | | | | | | | | | | devlink->dev is assumed to be always valid as long as any outstanding reference to the devlink instance exists. In prep for weakening of the references take the instance lock. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: David S. Miller <davem@davemloft.net>
| * devlink: update the code in netns move to latest helpersJakub Kicinski2023-01-061-3/+4
| | | | | | | | | | | | | | | | | | | | | | devlink_pernet_pre_exit() is the only obvious place which takes the instance lock without using the devl_ helpers. Update the code and move the error print after releasing the reference (having unlock and put together feels slightly idiomatic). Reviewed-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org> Signed-off-by: David S. Miller <davem@davemloft.net>
| * devlink: bump the instance index directly when iteratingJakub Kicinski2023-01-062-35/+13
|/ | | | | | | | | | | | | | | xa_find_after() is designed to handle multi-index entries correctly. If a xarray has two entries one which spans indexes 0-3 and one at index 4 xa_find_after(0) will return the entry at index 4. Having to juggle the two callbacks, however, is unnecessary in case of the devlink xarray, as there is 1:1 relationship with indexes. Always use xa_find() and increment the index manually. Signed-off-by: Jakub Kicinski <kuba@kernel.org> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* sysctl: expose all net/core sysctls inside netnsMahesh Bandewar2023-01-061-5/+0
| | | | | | | | | | | | | | | | | | | All were not visible to the non-priv users inside netns. However, with 4ecb90090c84 ("sysctl: allow override of /proc/sys/net with CAP_NET_ADMIN"), these vars are protected from getting modified. A proc with capable(CAP_NET_ADMIN) can change the values so not having them visible inside netns is just causing nuisance to process that check certain values (e.g. net.core.somaxconn) and see different behavior in root-netns vs. other-netns Cc: "David S. Miller" <davem@davemloft.net> Cc: Eric Dumazet <edumazet@google.com> Cc: Jakub Kicinski <kuba@kernel.org> Cc: Paolo Abeni <pabeni@redhat.com> Cc: Soheil Hassas Yeganeh <soheil@google.com> Signed-off-by: Mahesh Bandewar <maheshb@google.com> Acked-by: Soheil Hassas Yeganeh <soheil@google.com> Signed-off-by: David S. Miller <davem@davemloft.net>
* Merge branch 'devlink-code-split-and-structured-instance-walk'Jakub Kicinski2023-01-0510-1106/+1161
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Jakub Kicinski says: ==================== devlink: code split and structured instance walk Split devlink.c into a handful of files, trying to keep the "core" code away from all the command-specific implementations. The core code has been quite scattered until now. Going forward we can consider using a source file per-subobject, I think that it's quite beneficial to newcomers (based on relative ease with which folks contribute to ethtool vs devlink). But this series doesn't split everything out, yet - partially due to backporting concerns, but mostly due to lack of time. Bulk of the netlink command handling is left in a leftover.c file. Introduce a context structure for dumps, and use it to store the devlink instance ID of the last dumped devlink instance. This means we don't have to restart the walk from 0 each time. Finally - introduce a "structured walk". A centralized dump handler in devlink/netlink.c which walks the devlink instances, deals with refcounting/locking, simplifying the per-object implementations quite a bit. Inspired by the ethtool code. v1: https://lore.kernel.org/all/20230104041636.226398-1-kuba@kernel.org/ RFC: https://lore.kernel.org/all/20221215020155.1619839-1-kuba@kernel.org/ ==================== Link: https://lore.kernel.org/r/20230105040531.353563-1-kuba@kernel.org Signed-off-by: Jakub Kicinski <kuba@kernel.org>
| * devlink: convert remaining dumps to the by-instance schemeJakub Kicinski2023-01-053-394/+320
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Soon we'll have to check if a devlink instance is alive after locking it. Convert to the by-instance dumping scheme to make refactoring easier. Most of the subobject code no longer has to worry about any devlink locking / lifetime rules (the only ones that still do are the two subject types which stubbornly use their own locking). Both dump and do callbacks are given a devlink instance which is already locked and good-to-access (do from the .pre_doit handler, dump from the new dump indirection). Note that we'll now check presence of an op (e.g. for sb_pool_get) under the devlink instance lock, that will soon be necessary anyway, because we don't hold refs on the driver modules so the memory in which ops live may be gone for a dead instance, after upcoming locking changes. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
| * devlink: add by-instance dump infraJakub Kicinski2023-01-053-31/+68
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Most dumpit implementations walk the devlink instances. This requires careful lock taking and reference dropping. Factor the loop out and provide just a callback to handle a single instance dump. Convert one user as an example, other users converted in the next change. Slightly inspired by ethtool netlink code. Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
| * devlink: uniformly take the devlink instance lock in the dump loopJakub Kicinski2023-01-051-7/+6
| | | | | | | | | | | | | | | | | | | | Move the lock taking out of devlink_nl_cmd_region_get_devlink_dumpit(). This way all dumps will take the instance lock in the main iteration loop directly, making refactoring and reading the code easier. Reviewed-by: Jiri Pirko <jiri@nvidia.com> Reviewed-by: Jacob Keller <jacob.e.keller@intel.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org>