- 27 Aug, 2024 28 commits
-
-
Liao Chen authored
Add MODULE_DEVICE_TABLE(), so modules could be properly autoloaded based on the alias from of_device_id table. Signed-off-by: Liao Chen <liaochen4@huawei.com> Link: https://patch.msgid.link/20240826091858.369910-2-liaochen4@huawei.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Yu Liao authored
PCI core API pci_dev_id() can be used to get the BDF number for a PCI device. We don't need to compose it manually. Use pci_dev_id() to simplify the code a little bit. Signed-off-by: Yu Liao <liaoyu15@huawei.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/20240826012100.3975175-1-liaoyu15@huawei.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Rosen Penev authored
Currently, of_get_ethdev_address() return is checked for any return error code which means that trying to get the MAC from NVMEM cells that is backed by MTD will fail if it was not probed before ag71xx. So, lets check the return error code for EPROBE_DEFER and defer the ag71xx probe in that case until the underlying NVMEM device is live. Signed-off-by: Robert Marko <robimarko@gmail.com> Signed-off-by: Rosen Penev <rosenp@gmail.com> Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Andrew Lunn <andrew@lunn.ch> Link: https://patch.msgid.link/20240824200249.137209-1-rosenp@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Yue Haibing authored
Commit da15c78b ("liquidio CN23XX: VF register access") declared cn23xx_dump_vf_initialized_regs() but never implemented it. octeon_dump_soft_command() is never implemented and used since introduction in commit 35878618 ("liquidio: Added delayed work for periodically updating the link statistics."). And finally, a few other declarations were never implenmented since introduction in commit f21fb3ed ("Add support of Cavium Liquidio ethernet adapters"). Signed-off-by: Yue Haibing <yuehaibing@huawei.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/20240824083107.3639602-1-yuehaibing@huawei.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Yue Haibing authored
Commit 4863dea3 ("net: Adding support for Cavium ThunderX network controller") declared nicvf_qset_reg_{write,read}() but never implemented. Commit 4863dea3 ("net: Adding support for Cavium ThunderX network controller") declared bgx_add_dmac_addr() but no implementation. After commit 5fc7cf17 ("net: thunderx: Cleanup PHY probing code.") octeon_mdiobus_force_mod_depencency() is not used any more. Signed-off-by: Yue Haibing <yuehaibing@huawei.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/20240824082754.3637963-1-yuehaibing@huawei.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Jakub Kicinski authored
Dmitry Safonov via says: ==================== net/selftests: TCP-AO selftests updates First 3 patches are more-or-less cleanups/preparations. Patches 4/5 are fixes for netns file descriptors leaks/open. Patch 6 was sent to me/contributed off-list by Mohammad, who wants 32-bit kernels to run TCP-AO. Patch 7 is a workaround/fix for slow VMs. Albeit, I can't reproduce the issue, but I hope it will fix netdev flakes for connect-deny-* tests. And the biggest change is adding TCP-AO tracepoints to selftests. I think it's a good addition by the following reasons: - The related tracepoints are now tested; - It allows tcp-ao selftests to raise expectations on the kernel behavior - up from the syscalls exit statuses + net counters. - Provides tracepoints usage samples. As tracepoints are not a stable ABI, any kernel changes done to them will be reflected to the selftests, which also will allow users to see how to change their code. It's quite better than parsing dmesg (what BGP was doing pre-tracepoints, ugh). Somewhat arguably, the code parses trace_pipe, rather than uses libtraceevent (which any sane user should do). The reason behind that is the same as for rt-netlink macros instead of libmnl: I'm trying to minimize the library dependencies of the selftests. And the performance of formatting text in kernel and parsing it again in a test is not critical. Current output sample: > ok 73 Trace events matched expectations: 13 tcp_hash_md5_required[2] tcp_hash_md5_unexpected[4] tcp_hash_ao_required[3] tcp_ao_key_not_found[4] Previously, tracepoints selftests were part of kernel tcp tracepoints submission [1], but since then the code was quite changed: - Now generic tracing setup is in lib/ftrace.c, separate from lib/ftrace-tcp.c which utilizes TCP trace points. This separation allows future selftests to trace non-TCP events, i.e. to find out an skb's drop reason, which was useful in the creation of TCP-CLOSE stress-test (not in this patch set, but used in attempt to reproduce the issue from [2]). - Another change is that in the previous submission the trace events where used only to detect unexpected TCP-AO/TCP-MD5 events. In this version the selftests will fail if an expected trace event didn't appear. Let's see how reliable this is on the netdev bot - it obviously passes on my testing, but potentially may require a temporary XFAIL patch if it misbehaves on a slow VM. [1] https://lore.kernel.org/lkml/20240224-tcp-ao-tracepoints-v1-0-15f31b7f30a7@arista.com/ [2] https://git.kernel.org/pub/scm/linux/kernel/git/netdev/net.git/commit/?id=33700a0c9b56 v3: https://lore.kernel.org/20240815-tcp-ao-selftests-upd-6-12-v3-0-7bd2e22bb81c@gmail.com v2: https://lore.kernel.org/20240802-tcp-ao-selftests-upd-6-12-v2-0-370c99358161@gmail.com v1: https://lore.kernel.org/20240730-tcp-ao-selftests-upd-6-12-v1-0-ffd4bf15d638@gmail.com ==================== Link: https://patch.msgid.link/20240823-tcp-ao-selftests-upd-6-12-v4-0-05623636fe8c@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Dmitry Safonov authored
Setup trace points, add a new ftrace instance in order to not interfere with the rest of the system, filtering by net namespace cookies. Raise a new background thread that parses trace_pipe, matches them with the list of expected events. Wiring up trace events to selftests provides another insight if there is anything unexpected happining in the tcp-ao code (i.e. key rotation when it's not expected). Note: in real programs libtraceevent should be used instead of this manual labor of setting ftrace up and parsing. I'm not using it here as I don't want to have an .so library dependency that one would have to bring into VM or DUT (Device Under Test). Please, don't copy it over into any real world programs, that aren't tests. Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com> Link: https://patch.msgid.link/20240823-tcp-ao-selftests-upd-6-12-v4-8-05623636fe8c@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Dmitry Safonov authored
On tests that are expecting failure the timeout value is TEST_RETRANSMIT_SEC == 1 second. Which is big enough for most of devices under tests. But on a particularly slow machine/VM, 1 second might be not enough for another thread to be scheduled and attempt to connect(). It is not a problem for tests that expect connect() to succeed as the timeout value for them (TEST_TIMEOUT_SEC) is intentionally bigger. One obvious way to solve this would be to increase TEST_RETRANSMIT_SEC. But as all tests would increase the timeouts, that's going to sum up. But here is less obvious way that keeps timeouts for expected connect() failures low: just synchronize the two threads, which will assure that before counter checks the other thread got a chance to run and timeout on connect(). The expected increase of the related counter for listen() socket will yet test the expected failure. Never happens on my machine, but I suppose the majority of netdev's connect-deny-* flakes [1] are caused by this. Prevents the following testing issue: > # selftests: net/tcp_ao: connect-deny_ipv6 > # 1..21 > # # 462[lib/setup.c:243] rand seed 1720905426 > # TAP version 13 > # ok 1 Non-AO server + AO client > # not ok 2 Non-AO server + AO client: TCPAOKeyNotFound counter did not increase: 0 <= 0 > # ok 3 AO server + Non-AO client > # ok 4 AO server + Non-AO client: counter TCPAORequired increased 0 => 1 ... [1]: https://netdev-3.bots.linux.dev/vmksft-tcp-ao/results/681741/6-connect-deny-ipv6/stdoutSigned-off-by: Dmitry Safonov <0x7f454c46@gmail.com> Link: https://patch.msgid.link/20240823-tcp-ao-selftests-upd-6-12-v4-7-05623636fe8c@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Mohammad Nassiri authored
It's not safe to use '%zu' specifier for printing uint64_t on 32-bit systems. For uint64_t, we should use the 'PRIu64' macro from the inttypes.h library. This ensures that the uint64_t is printed correctly from the selftests regardless of the system architecture. Signed-off-by: Mohammad Nassiri <mnassiri@ciena.com> [Added missing spaces in fail/ok messages and uint64_t cast in setsockopt-closed, as otherwise it was giving warnings on 64bit. And carried it to netdev ml] Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com> Link: https://patch.msgid.link/20240823-tcp-ao-selftests-upd-6-12-v4-6-05623636fe8c@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Dmitry Safonov authored
The switch_save_ns() helper suppose to help switching to another namespace for some action and to return back to original namespace. The fd should be closed. Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com> Link: https://patch.msgid.link/20240823-tcp-ao-selftests-upd-6-12-v4-5-05623636fe8c@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Dmitry Safonov authored
It turns to be that open_netns() is called rarely from the child-thread and more often from parent-thread. Yet, on initialization of kconfig checks, either of threads may reach kconfig_lock mutex first. VRF-related checks do create a temporary ksft-check VRF in an unshare()'d namespace and than setns() back to the original. As original was opened from "/proc/self/ns/net", it's valid for thread-leader (parent), but it's invalid for the child, resulting in the following failure on tests that check has_vrfs() support: > # ok 54 TCP-AO required on socket + TCP-MD5 key: prefailed as expected: Key was rejected by service > # not ok 55 # error 381[unsigned-md5.c:24] Failed to add a VRF: -17 > # not ok 56 # error 383[unsigned-md5.c:33] Failed to add a route to VRF: -22: Key was rejected by service > not ok 1 selftests: net/tcp_ao: unsigned-md5_ipv6 # exit=1 Use "/proc/thread-self/ns/net" which is valid for any thread. Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com> Link: https://patch.msgid.link/20240823-tcp-ao-selftests-upd-6-12-v4-4-05623636fe8c@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Dmitry Safonov authored
Most of the functions in tcp-ao lib/ return negative errno or -1 in case of a failure. That creates inconsistencies in lib/kconfig, which saves what was the error code. As well as the uninitialized kconfig value is -1, which also may be the result of a check. Define KCONFIG_UNKNOWN and save negative return code, rather than libc-style errno. Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com> Link: https://patch.msgid.link/20240823-tcp-ao-selftests-upd-6-12-v4-3-05623636fe8c@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Dmitry Safonov authored
Instead of pre-allocating a fixed-sized buffer of TEST_MSG_BUFFER_SIZE and printing into it, call vsnprintf() with str = NULL, which will return the needed size of the buffer. This hack is documented in man 3 vsnprintf. Essentially, in C++ terms, it re-invents std::stringstream, which is going to be used to print different tracing paths and formatted strings. Use it straight away in __test_print() - which is thread-safe version of printing in selftests. Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com> Link: https://patch.msgid.link/20240823-tcp-ao-selftests-upd-6-12-v4-2-05623636fe8c@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Dmitry Safonov authored
Correct copy'n'paste typo: the previous line already initialises get_all to 1. Reported-by: Nassiri, Mohammad <mnassiri@ciena.com> Closes: https://lore.kernel.org/all/DM6PR04MB4202BC58A9FD5BDD24A16E8EC56F2@DM6PR04MB4202.namprd04.prod.outlook.com/Signed-off-by: Dmitry Safonov <0x7f454c46@gmail.com> Link: https://patch.msgid.link/20240823-tcp-ao-selftests-upd-6-12-v4-1-05623636fe8c@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
James Chapman authored
Recent commit fc7ec7f5 ("l2tp: delete sessions using work queue") incorrectly uses drain_workqueue. The use of drain_workqueue in l2tp_pre_exit_net is flawed because the workqueue is shared by all nets and it is therefore possible for new work items to be queued for other nets while drain_workqueue runs. Instead of using drain_workqueue, use __flush_workqueue twice. The first one will run all tunnel delete work items and any work already queued. When tunnel delete work items are run, they may queue new session delete work items, which the second __flush_workqueue will run. In l2tp_exit_net, warn if any of the net's idr lists are not empty. Fixes: fc7ec7f5 ("l2tp: delete sessions using work queue") Signed-off-by: James Chapman <jchapman@katalix.com> Link: https://patch.msgid.link/20240823142257.692667-1-jchapman@katalix.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Jakub Kicinski authored
Detlev Casanova says: ==================== Add GMAC support for rk3576 Add the necessary constants and functions to support the GMAC devices on the rk3576. ==================== Link: https://patch.msgid.link/20240823141318.51201-1-detlev.casanova@collabora.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
David Wu authored
Add constants and callback functions for the dwmac on RK3576 soc. Signed-off-by: David Wu <david.wu@rock-chips.com> [rebase, extracted bindings] Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> Reviewed-by: Heiko Stuebner <heiko@sntech.de> Link: https://patch.msgid.link/20240823141318.51201-4-detlev.casanova@collabora.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Detlev Casanova authored
Add a rockchip,rk3576-gmac compatible for supporting the 2 gmac devices on the rk3576. Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> Acked-by: Rob Herring (Arm) <robh@kernel.org> Reviewed-by: Heiko Stuebner <heiko@sntech.de> Link: https://patch.msgid.link/20240823141318.51201-3-detlev.casanova@collabora.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Detlev Casanova authored
Fix SELET -> SELECT in RK3588_GMAC_CLK_SELET_CRU and RK3588_GMAC_CLK_SELET_IO Signed-off-by: Detlev Casanova <detlev.casanova@collabora.com> Reviewed-by: Heiko Stuebner <heiko@sntech.de> Link: https://patch.msgid.link/20240823141318.51201-2-detlev.casanova@collabora.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Andy Shevchenko authored
of_node_to_fwnode() is a IRQ domain specific implementation of of_fwnode_handle(). Replace the former with more suitable API. Signed-off-by: Andy Shevchenko <andy.shevchenko@gmail.com> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/20240822230550.708112-1-andy.shevchenko@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Diogo Jahchan Koike authored
fix an unreleased lock in out_dev_put path by removing the (now) unnecessary path. Reported-by: syzbot+c641161e97237326ea74@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=c641161e97237326ea74 Fixes: 3688ff30 ("net: ethtool: cable-test: Target the command to the requested PHY") Signed-off-by: Diogo Jahchan Koike <djahchankoike@gmail.com> Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com> Link: https://patch.msgid.link/20240826134656.94892-1-djahchankoike@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Paolo Abeni authored
Boris Sukholitko says: ==================== tc: adjust network header after 2nd vlan push <tldr> skb network header of the single-tagged vlan packet continues to point the vlan payload (e.g. IP) after second vlan tag is pushed by tc act_vlan. This causes problem at the dissector which expects double-tagged packet network header to point to the inner vlan. The fix is to adjust network header in tcf_act_vlan.c but requires refactoring of skb_vlan_push function. </tldr> Consider the following shell script snippet configuring TC rules on the veth interface: ip link add veth0 type veth peer veth1 ip link set veth0 up ip link set veth1 up tc qdisc add dev veth0 clsact tc filter add dev veth0 ingress pref 10 chain 0 flower \ num_of_vlans 2 cvlan_ethtype 0x800 action goto chain 5 tc filter add dev veth0 ingress pref 20 chain 0 flower \ num_of_vlans 1 action vlan push id 100 \ protocol 0x8100 action goto chain 5 tc filter add dev veth0 ingress pref 30 chain 5 flower \ num_of_vlans 2 cvlan_ethtype 0x800 action simple sdata "success" Sending double-tagged vlan packet with the IP payload inside: cat <<ENDS | text2pcap - - | tcpreplay -i veth1 - 0000 00 00 00 00 00 11 00 00 00 00 00 22 81 00 00 64 ..........."...d 0010 81 00 00 14 08 00 45 04 00 26 04 d2 00 00 7f 11 ......E..&...... 0020 18 ef 0a 00 00 01 14 00 00 02 00 00 00 00 00 12 ................ 0030 e1 c7 00 00 00 00 00 00 00 00 00 00 ............ ENDS will match rule 10, goto rule 30 in chain 5 and correctly emit "success" to the dmesg. OTOH, sending single-tagged vlan packet: cat <<ENDS | text2pcap - - | tcpreplay -i veth1 - 0000 00 00 00 00 00 11 00 00 00 00 00 22 81 00 00 14 ...........".... 0010 08 00 45 04 00 2a 04 d2 00 00 7f 11 18 eb 0a 00 ..E..*.......... 0020 00 01 14 00 00 02 00 00 00 00 00 16 e1 bf 00 00 ................ 0030 00 00 00 00 00 00 00 00 00 00 00 00 ............ ENDS will match rule 20, will push the second vlan tag but will *not* match rule 30. IOW, the match at rule 30 fails if the second vlan was freshly pushed by the kernel. Lets look at __skb_flow_dissect working on the double-tagged vlan packet. Here is the relevant code from around net/core/flow_dissector.c:1277 copy-pasted here for convenience: if (dissector_vlan == FLOW_DISSECTOR_KEY_MAX && skb && skb_vlan_tag_present(skb)) { proto = skb->protocol; } else { vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan), data, hlen, &_vlan); if (!vlan) { fdret = FLOW_DISSECT_RET_OUT_BAD; break; } proto = vlan->h_vlan_encapsulated_proto; nhoff += sizeof(*vlan); } The "else" clause above gets the protocol of the encapsulated packet from the skb data at the network header location. printk debugging has showed that in the good double-tagged packet case proto is htons(0x800 == ETH_P_IP) as expected. However in the single-tagged packet case proto is garbage leading to the failure to match tc filter 30. proto is being set from the skb header pointed by nhoff parameter which is defined at the beginning of __skb_flow_dissect (net/core/flow_dissector.c:1055 in the current version): nhoff = skb_network_offset(skb); Therefore the culprit seems to be that the skb network offset is different between double-tagged packet received from the interface and single-tagged packet having its vlan tag pushed by TC. Lets look at the interesting points of the lifetime of the single/double tagged packets as they traverse our packet flow. Both of them will start at __netif_receive_skb_core where the first vlan tag will be stripped: if (eth_type_vlan(skb->protocol)) { skb = skb_vlan_untag(skb); if (unlikely(!skb)) goto out; } At this stage in double-tagged case skb->data points to the second vlan tag while in single-tagged case skb->data points to the network (eg. IP) header. Looking at TC vlan push action (net/sched/act_vlan.c) we have the following code at tcf_vlan_act (interesting points are in square brackets): if (skb_at_tc_ingress(skb)) [1] skb_push_rcsum(skb, skb->mac_len); .... case TCA_VLAN_ACT_PUSH: err = skb_vlan_push(skb, p->tcfv_push_proto, p->tcfv_push_vid | (p->tcfv_push_prio << VLAN_PRIO_SHIFT), 0); if (err) goto drop; break; .... out: if (skb_at_tc_ingress(skb)) [3] skb_pull_rcsum(skb, skb->mac_len); And skb_vlan_push (net/core/skbuff.c:6204) function does: err = __vlan_insert_tag(skb, skb->vlan_proto, skb_vlan_tag_get(skb)); if (err) return err; skb->protocol = skb->vlan_proto; [2] skb->mac_len += VLAN_HLEN; in the case of pushing the second tag. Lets look at what happens with skb->data of the single-tagged packet at each of the above points: 1. As a result of the skb_push_rcsum, skb->data is moved back to the start of the packet. 2. First VLAN tag is moved from the skb into packet buffer, skb->mac_len is incremented, skb->data still points to the start of the packet. 3. As a result of the skb_pull_rcsum, skb->data is moved forward by the modified skb->mac_len, thus pointing to the network header again. Then __skb_flow_dissect will get confused by having double-tagged vlan packet with the skb->data at the network header. The solution for the bug is to preserve "skb->data at second vlan header" semantics in the skb_vlan_push function. We do this by manipulating skb->network_header rather than skb->mac_len. skb_vlan_push callers are updated to do skb_reset_mac_len. More about the patch series: * patch 1 fixes skb_vlan_push and the callers * patch 2 adds ingress tc_actions test * patch 3 adds egress tc_actions test ==================== Link: https://patch.msgid.link/20240822103510.468293-1-boris.sukholitko@broadcom.comSigned-off-by: Paolo Abeni <pabeni@redhat.com>
-
Boris Sukholitko authored
Add new test checking the correctness of inner vlan flushing to the skb data when outer vlan tag is added through act_vlan on egress. Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
-
Boris Sukholitko authored
Add new test checking the correctness of inner vlan flushing to the skb data when outer vlan tag is added through act_vlan on ingress. Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
-
Boris Sukholitko authored
<tldr> skb network header of the single-tagged vlan packet continues to point the vlan payload (e.g. IP) after second vlan tag is pushed by tc act_vlan. This causes problem at the dissector which expects double-tagged packet network header to point to the inner vlan. The fix is to adjust network header in tcf_act_vlan.c but requires refactoring of skb_vlan_push function. </tldr> Consider the following shell script snippet configuring TC rules on the veth interface: ip link add veth0 type veth peer veth1 ip link set veth0 up ip link set veth1 up tc qdisc add dev veth0 clsact tc filter add dev veth0 ingress pref 10 chain 0 flower \ num_of_vlans 2 cvlan_ethtype 0x800 action goto chain 5 tc filter add dev veth0 ingress pref 20 chain 0 flower \ num_of_vlans 1 action vlan push id 100 \ protocol 0x8100 action goto chain 5 tc filter add dev veth0 ingress pref 30 chain 5 flower \ num_of_vlans 2 cvlan_ethtype 0x800 action simple sdata "success" Sending double-tagged vlan packet with the IP payload inside: cat <<ENDS | text2pcap - - | tcpreplay -i veth1 - 0000 00 00 00 00 00 11 00 00 00 00 00 22 81 00 00 64 ..........."...d 0010 81 00 00 14 08 00 45 04 00 26 04 d2 00 00 7f 11 ......E..&...... 0020 18 ef 0a 00 00 01 14 00 00 02 00 00 00 00 00 12 ................ 0030 e1 c7 00 00 00 00 00 00 00 00 00 00 ............ ENDS will match rule 10, goto rule 30 in chain 5 and correctly emit "success" to the dmesg. OTOH, sending single-tagged vlan packet: cat <<ENDS | text2pcap - - | tcpreplay -i veth1 - 0000 00 00 00 00 00 11 00 00 00 00 00 22 81 00 00 14 ...........".... 0010 08 00 45 04 00 2a 04 d2 00 00 7f 11 18 eb 0a 00 ..E..*.......... 0020 00 01 14 00 00 02 00 00 00 00 00 16 e1 bf 00 00 ................ 0030 00 00 00 00 00 00 00 00 00 00 00 00 ............ ENDS will match rule 20, will push the second vlan tag but will *not* match rule 30. IOW, the match at rule 30 fails if the second vlan was freshly pushed by the kernel. Lets look at __skb_flow_dissect working on the double-tagged vlan packet. Here is the relevant code from around net/core/flow_dissector.c:1277 copy-pasted here for convenience: if (dissector_vlan == FLOW_DISSECTOR_KEY_MAX && skb && skb_vlan_tag_present(skb)) { proto = skb->protocol; } else { vlan = __skb_header_pointer(skb, nhoff, sizeof(_vlan), data, hlen, &_vlan); if (!vlan) { fdret = FLOW_DISSECT_RET_OUT_BAD; break; } proto = vlan->h_vlan_encapsulated_proto; nhoff += sizeof(*vlan); } The "else" clause above gets the protocol of the encapsulated packet from the skb data at the network header location. printk debugging has showed that in the good double-tagged packet case proto is htons(0x800 == ETH_P_IP) as expected. However in the single-tagged packet case proto is garbage leading to the failure to match tc filter 30. proto is being set from the skb header pointed by nhoff parameter which is defined at the beginning of __skb_flow_dissect (net/core/flow_dissector.c:1055 in the current version): nhoff = skb_network_offset(skb); Therefore the culprit seems to be that the skb network offset is different between double-tagged packet received from the interface and single-tagged packet having its vlan tag pushed by TC. Lets look at the interesting points of the lifetime of the single/double tagged packets as they traverse our packet flow. Both of them will start at __netif_receive_skb_core where the first vlan tag will be stripped: if (eth_type_vlan(skb->protocol)) { skb = skb_vlan_untag(skb); if (unlikely(!skb)) goto out; } At this stage in double-tagged case skb->data points to the second vlan tag while in single-tagged case skb->data points to the network (eg. IP) header. Looking at TC vlan push action (net/sched/act_vlan.c) we have the following code at tcf_vlan_act (interesting points are in square brackets): if (skb_at_tc_ingress(skb)) [1] skb_push_rcsum(skb, skb->mac_len); .... case TCA_VLAN_ACT_PUSH: err = skb_vlan_push(skb, p->tcfv_push_proto, p->tcfv_push_vid | (p->tcfv_push_prio << VLAN_PRIO_SHIFT), 0); if (err) goto drop; break; .... out: if (skb_at_tc_ingress(skb)) [3] skb_pull_rcsum(skb, skb->mac_len); And skb_vlan_push (net/core/skbuff.c:6204) function does: err = __vlan_insert_tag(skb, skb->vlan_proto, skb_vlan_tag_get(skb)); if (err) return err; skb->protocol = skb->vlan_proto; [2] skb->mac_len += VLAN_HLEN; in the case of pushing the second tag. Lets look at what happens with skb->data of the single-tagged packet at each of the above points: 1. As a result of the skb_push_rcsum, skb->data is moved back to the start of the packet. 2. First VLAN tag is moved from the skb into packet buffer, skb->mac_len is incremented, skb->data still points to the start of the packet. 3. As a result of the skb_pull_rcsum, skb->data is moved forward by the modified skb->mac_len, thus pointing to the network header again. Then __skb_flow_dissect will get confused by having double-tagged vlan packet with the skb->data at the network header. The solution for the bug is to preserve "skb->data at second vlan header" semantics in the skb_vlan_push function. We do this by manipulating skb->network_header rather than skb->mac_len. skb_vlan_push callers are updated to do skb_reset_mac_len. Signed-off-by: Boris Sukholitko <boris.sukholitko@broadcom.com> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
-
Jakub Kicinski authored
Arkadiusz Kubalewski says: ==================== Add Embedded SYNC feature for a dpll's pin Introduce and allow DPLL subsystem users to get/set capabilities of Embedded SYNC on a dpll's pin. Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> ==================== Link: https://patch.msgid.link/20240822222513.255179-1-arkadiusz.kubalewski@intel.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Arkadiusz Kubalewski authored
Allow the user to get and set configuration of Embedded SYNC feature on the ice driver dpll pins. Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://patch.msgid.link/20240822222513.255179-3-arkadiusz.kubalewski@intel.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Arkadiusz Kubalewski authored
Implement and document new pin attributes for providing Embedded SYNC capabilities to the DPLL subsystem users through a netlink pin-get do/dump messages. Allow the user to set Embedded SYNC frequency with pin-set do netlink message. Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com> Reviewed-by: Jiri Pirko <jiri@nvidia.com> Link: https://patch.msgid.link/20240822222513.255179-2-arkadiusz.kubalewski@intel.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
- 26 Aug, 2024 12 commits
-
-
Xi Huang authored
In the function dpaa_napi_del(), we execute the netif_napi_del() for each cpu, which is actually a high overhead operation because each call to netif_napi_del() contains a synchronize_net(), i.e. an RCU operation. In fact, it is only necessary to call __netif_napi_del and use synchronize_net() once outside of the loop. This change is similar to commit 2543a600 ("gro_cells: reduce number of synchronize_net() calls") and commit 5198d545 (" net: remove napi_hash_del() from driver-facing API") 5198d545. Signed-off-by: Xi Huang <xuiagnh@gmail.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://patch.msgid.link/20240822072042.42750-1-xuiagnh@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Eric Dumazet authored
ipv6_setsockopt() can directly call ip_setsockopt() instead of going through udp_prot.setsockopt() ipv6_getsockopt() can directly call ip_getsockopt() instead of going through udp_prot.getsockopt() These indirections predate git history, not sure why they were there. Signed-off-by: Eric Dumazet <edumazet@google.com> Link: https://patch.msgid.link/20240823140019.3727643-1-edumazet@google.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Hongbo Li authored
The macro sk_for_each_bound_bhash accepts a parameter __sk, but it was not used, rather the sk2 is directly used, so we replace the sk2 with __sk in macro. Signed-off-by: Hongbo Li <lihongbo22@huawei.com> Link: https://patch.msgid.link/20240823070453.3327832-1-lihongbo22@huawei.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Jason Xing authored
We found that one close-wait socket was reset by the other side due to a new connection reusing the same port which is beyond our expectation, so we have to investigate the underlying reason. The following experiment is conducted in the test environment. We limit the port range from 40000 to 40010 and delay the time to close() after receiving a fin from the active close side, which can help us easily reproduce like what happened in production. Here are three connections captured by tcpdump: 127.0.0.1.40002 > 127.0.0.1.9999: Flags [S], seq 2965525191 127.0.0.1.9999 > 127.0.0.1.40002: Flags [S.], seq 2769915070 127.0.0.1.40002 > 127.0.0.1.9999: Flags [.], ack 1 127.0.0.1.40002 > 127.0.0.1.9999: Flags [F.], seq 1, ack 1 // a few seconds later, within 60 seconds 127.0.0.1.40002 > 127.0.0.1.9999: Flags [S], seq 2965590730 127.0.0.1.9999 > 127.0.0.1.40002: Flags [.], ack 2 127.0.0.1.40002 > 127.0.0.1.9999: Flags [R], seq 2965525193 // later, very quickly 127.0.0.1.40002 > 127.0.0.1.9999: Flags [S], seq 2965590730 127.0.0.1.9999 > 127.0.0.1.40002: Flags [S.], seq 3120990805 127.0.0.1.40002 > 127.0.0.1.9999: Flags [.], ack 1 As we can see, the first flow is reset because: 1) client starts a new connection, I mean, the second one 2) client tries to find a suitable port which is a timewait socket (its state is timewait, substate is fin_wait2) 3) client occupies that timewait port to send a SYN 4) server finds a corresponding close-wait socket in ehash table, then replies with a challenge ack 5) client sends an RST to terminate this old close-wait socket. I don't think the port selection algo can choose a FIN_WAIT2 socket when we turn on tcp_tw_reuse because on the server side there remain unread data. In some cases, if one side haven't call close() yet, we should not consider it as expendable and treat it at will. Even though, sometimes, the server isn't able to call close() as soon as possible like what we expect, it can not be terminated easily, especially due to a second unrelated connection happening. After this patch, we can see the expected failure if we start a connection when all the ports are occupied in fin_wait2 state: "Ncat: Cannot assign requested address." Reported-by: Jade Dong <jadedong@tencent.com> Signed-off-by: Jason Xing <kernelxing@tencent.com> Reviewed-by: Eric Dumazet <edumazet@google.com> Link: https://patch.msgid.link/20240823001152.31004-1-kerneljasonxing@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Jakub Kicinski authored
Kyle Swenson says: ==================== net: pse-pd: tps23881: Reset GPIO support On some boards, the TPS2388x's reset line (active low) is pulled low to keep the chip in reset until the SoC pulls the device out of reset. This series updates the device-tree binding for the tps23881 and then adds support for the reset gpio handling in the tps23881 driver. v1: https://lore.kernel.org/20240819190151.93253-1-kyle.swenson@est.tech ==================== Link: https://patch.msgid.link/20240822220100.3030184-1-kyle.swenson@est.techSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Kyle Swenson authored
The TPS23880/1 has an active-low reset pin that some boards connect to the SoC to control when the TPS23880 is pulled out of reset. Add support for this via a reset-gpios property in the DTS. Signed-off-by: Kyle Swenson <kyle.swenson@est.tech> Acked-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Kory Maincent <kory.maincent@bootlin.com> Link: https://patch.msgid.link/20240822220100.3030184-3-kyle.swenson@est.techSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Kyle Swenson authored
The TPS23881 has an active-low reset pin that can be connected to an SoC. Document this with the device-tree binding. Signed-off-by: Kyle Swenson <kyle.swenson@est.tech> Reviewed-by: Krzysztof Kozlowski <krzysztof.kozlowski@linaro.org> Reviewed-by: Oleksij Rempel <o.rempel@pengutronix.de> Reviewed-by: Kory Maincent <kory.maincent@bootlin.com> Link: https://patch.msgid.link/20240822220100.3030184-2-kyle.swenson@est.techSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Rosen Penev authored
It's only used in one place. It doesn't need to be in the struct. Signed-off-by: Rosen Penev <rosenp@gmail.com> Link: https://patch.msgid.link/20240822192758.141201-1-rosenp@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Cong Wang authored
Although commit 4a4cd703 ("l2tp: don't set sk_user_data in tunnel socket") removed sk->sk_user_data usage, setup_udp_tunnel_sock() still touches sk->sk_user_data, this conflicts with sockmap which also leverages sk->sk_user_data to save psock. Restore this sk->sk_user_data check to avoid such conflicts. Fixes: 4a4cd703 ("l2tp: don't set sk_user_data in tunnel socket") Reported-by: syzbot+8dbe3133b840c470da0e@syzkaller.appspotmail.com Cc: Tom Parkin <tparkin@katalix.com> Signed-off-by: Cong Wang <cong.wang@bytedance.com> Tested-by: James Chapman <jchapman@katalix.com> Reviewed-by: James Chapman <jchapman@katalix.com> Link: https://patch.msgid.link/20240822182544.378169-1-xiyou.wangcong@gmail.comSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Jakub Kicinski authored
Sean Anderson says: ==================== net: xilinx: axienet: Multicast fixes and improvements This series has a few small patches improving the handling of multicast addresses. In particular, it makes the driver a whole lot less spammy, and adjusts things so we aren't in promiscuous mode when we have more than four multicast addresses (a common occurance on modern systems). As the hardware has a 4-entry CAM, the ideal method would be to "pack" multiple addresses into one CAM entry. Something like: entry.address = address[0] | address[1]; entry.mask = ~(address[0] ^ address[1]); Which would make the entry match both addresses (along with some others that would need to be filtered in software). Mapping addresses to entries in an efficient way is a bit tricky. If anyone knows of an in-tree example of something like this, I'd be glad to hear about it. ==================== Link: https://patch.msgid.link/20240822154059.1066595-1-sean.anderson@linux.devSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Sean Anderson authored
Add support for IFF_ALLMULTI by configuring a single filter to match the multicast address bit. This allows us to keep promiscuous mode disabled, even when we have more than four multicast addresses. An even better solution would be to "pack" addresses into the available CAM registers, but that can wait for a future series. Signed-off-by: Sean Anderson <sean.anderson@linux.dev> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/20240822154059.1066595-6-sean.anderson@linux.devSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-
Sean Anderson authored
Contrary to the comment, we don't have to inform the net subsystem. Signed-off-by: Sean Anderson <sean.anderson@linux.dev> Reviewed-by: Simon Horman <horms@kernel.org> Link: https://patch.msgid.link/20240822154059.1066595-5-sean.anderson@linux.devSigned-off-by: Jakub Kicinski <kuba@kernel.org>
-