- 12 Aug, 2024 16 commits
-
-
Jakub Kicinski authored
Count packets, bytes and drop on the datapath, and report to the user. Since queues are completely freed when the device is down - accumulate the stats in the main netdev struct. This means that per-queue stats will only report values since last reset (per qstat recommendation). Reviewed-by:
Joe Damato <jdamato@fastly.com> Link: https://patch.msgid.link/20240810054322.2766421-2-kuba@kernel.orgSigned-off-by:
Jakub Kicinski <kuba@kernel.org>
-
David S. Miller authored
Jakub Kicinski says: ==================== ethtool: rss: driver tweaks and netlink context dumps This series is a semi-related collection of RSS patches. Main point is supporting dumping RSS contexts via ethtool netlink. At present additional RSS contexts can be queried one by one, and assuming user know the right IDs. This series uses the XArray added by Ed to provide netlink dump support for ETHTOOL_GET_RSS. Patch 1 is a trivial selftest debug patch. Patch 2 coverts mvpp2 for no real reason other than that I had a grand plan of converting all drivers at some stage. Patch 3 removes a now moot check from mlx5 so that all tests can pass. Patch 4 and 5 make a bit used for context support optional, for easier grepping of drivers which need converting if nothing else. Patch 6 OTOH adds a new cap bit; some devices don't support using a different key per context and currently act in surprising ways. Patch 7 and 8 update the RSS netlink code to use XArray. Patch 9 and 10 add support for dumping contexts. Patch 11 and 12 are small adjustments to spec and a new test. I'm getting distracted with other work, so probably won't have the time soon to complete next steps, but things which are missing are (and some of these may be bad ideas): - better discovery Some sort of API to tell the user who many contexts the device can create. Upper bound, devices often share contexts between ports etc. so it's hard to tell exactly and upfront number of contexts for a netdev. But order of magnitude (4 vs 10s) may be enough for container management system to know whether to bother. - create/modify/delete via netlink The only question here is how to handle all the tricky IOCTL legacy. "No change" maps trivially to attribute not present. "reset" (indir_size = 0) probably needs to be a new NLA_FLAG? - better table size handling The current API assumes the LUT has fixed size, which isn't true for modern devices. We should have better APIs for the drivers to resize the tables, and in user facing API - the ability to specify pattern and min size rather than exact table expected (sort of like ethtool CLI already does). - recounted / socket-bound contexts Support for contexts which get "cleaned up" when their parent netlink socket gets closed. The major catch is that ntuple filters (which we don't currently track) depend on the context, so we need auto-removal for both. v5: - fix build v4: https://lore.kernel.org/20240809031827.2373341-1-kuba@kernel.org - adjust to the meaning of max context from net v3: https://lore.kernel.org/20240806193317.1491822-1-kuba@kernel.org - quite a few code comments and commit message changes - mvpp2: fix interpretation of max_context_id (I'll take care of the net -> net-next merge as needed) - filter by ifindex in the selftest v2: https://lore.kernel.org/20240803042624.970352-1-kuba@kernel.org - fix bugs and build in mvpp2 v1: https://lore.kernel.org/20240802001801.565176-1-kuba@kernel.org ==================== Signed-off-by:
David S. Miller <davem@davemloft.net>
-
Jakub Kicinski authored
Add a test for dumping RSS contexts. Make sure indir table and key are sane when contexts are created with various combination of inputs. Test the dump filtering by ifname and start-context. Reviewed-by:
Petr Machata <petrm@nvidia.com> Reviewed-by:
Edward Cree <ecree.xilinx@gmail.com> Signed-off-by:
-
Jakub Kicinski authored
Indirection table is dumped as a raw u32 array, decode it. It's tempting to decode hash key, too, but it is an actual bitstream, so leave it be for now. Signed-off-by:
Donald Hunter <donald.hunter@gmail.com> Signed-off-by:
-
Jakub Kicinski authored
Applications may want to deal with dynamic RSS contexts only. So dumping context 0 will be counter-productive for them. Support starting the dump from a given context ID. Alternative would be to implement a dump flag to skip just context 0, not sure which is better... Reviewed-by:
-
Jakub Kicinski authored
Now that we track RSS contexts in the core we can easily dump them. This is a major introspection improvement, as previously the only way to find all contexts would be to try all ids (of which there may be 2^32 - 1). Don't use the XArray iterators (like xa_for_each_start()) as they do not move the index past the end of the array once done, which caused multiple bugs in Netlink dumps in the past. Reviewed-by:
-
Jakub Kicinski authored
IOCTL already uses the XArray when reporting info about additional contexts. Do the same thing in netlink code. Reviewed-by:
-
Jakub Kicinski authored
Factor calling device ops out of rss_prepare_data(). Next patch will add alternative path using xarray. No functional changes. Reviewed-by:
-
Jakub Kicinski authored
marvell/otx2 and mvpp2 do not support setting different keys for different RSS contexts. Contexts have separate indirection tables but key is shared with all other contexts. This is likely fine, indirection table is the most important piece. Don't report the key-related parameters from such drivers. This prevents driver-errors, e.g. otx2 always writes the main key, even when user asks to change per-context key. The second reason is that without this change tracking the keys by the core gets complicated. Even if the driver correctly reject setting key with rss_context != 0, change of the main key would have to be reflected in the XArray for all additional contexts. Since the additional contexts don't have their own keys not including the attributes (in Netlink speak) seems intuitive. ethtool CLI seems to deal with it just fine. Having to set the flag in majority of the drivers is a bit tedious but not reporting the key is a safer default. Reviewed-by:
-
Jakub Kicinski authored
Remove .cap_rss_ctx_supported from drivers which moved to the new API. This makes it easy to grep for drivers which still need to be converted. Reviewed-by:
Gal Pressman <gal@nvidia.com> Reviewed-by:
-
Jakub Kicinski authored
cap_rss_ctx_supported was created because the API for creating and configuring additional contexts is mux'ed with the normal RSS API. Presence of ops does not imply driver can actually support rss_context != 0 (in fact drivers mostly ignore that field). cap_rss_ctx_supported lets core check that the driver is context-aware before calling it. Now that we have .create_rxfh_context, there is no such ambiguity. We can depend on presence of the op. Make setting the bit optional. Reviewed-by:
-
Jakub Kicinski authored
Since commit 24ac7e54 ("ethtool: use the rss context XArray in ring deactivation safety-check") core will prevent queues from being disabled while being used by additional RSS contexts. The safety check is no longer necessary, and core will do a more accurate job of only rejecting changes which can actually break things. Reviewed-by:
-
Jakub Kicinski authored
Implement the separate create/modify/delete ops for RSS. No problems with IDs - even tho RSS tables are per device the driver already seems to allocate IDs linearly per port. There's a translation table from per-port context ID to device context ID. mvpp2 doesn't have a key for the hash, it defaults to an empty/previous indir table. Note that there is no key at all, so we don't have to be concerned with reporting the wrong one (which is addressed by a patch later in the series). Compile-tested only. Reviewed-by:
-
Jakub Kicinski authored
Include the "name" of the context in the comment for traffic checks. Makes it easier to reason about which context failed when we loop over 32 contexts (it may matter if we failed in first vs last, for example). Reviewed-by:
-
Menglong Dong authored
The variable "did_rsc" is initialized twice, which is unnecessary. Just remove one of them. Signed-off-by:
Menglong Dong <dongml2@chinatelecom.cn> Reviewed-by:
Simon Horman <horms@kernel.org> Signed-off-by:
-
Rosen Penev authored
Simpler and allows avoiding manual pointer addition. Signed-off-by:
Rosen Penev <rosenp@gmail.com> Signed-off-by:
-
- 11 Aug, 2024 24 commits
-
-
Enguerrand de Ribaucourt authored
In order to respect the 80 columns limit, split the half-duplex monitoring function in two. This is just a styling change, no functional change. Signed-off-by:
Enguerrand de Ribaucourt <enguerrand.de-ribaucourt@savoirfairelinux.com> Acked-by:
Arun Ramadoss <arun.ramadoss@microchip.com> Signed-off-by:
-
David S. Miller authored
Russell King says: ==================== net: phylib: fix fixed-speed >= 1G This is v2 of the patch (now patches) adding support for ethtool !autoneg while respecting the requirements of IEEE 802.3. v2 fixes the build errors in the previous patch by first constifying the "advertisement" argument to the linkmode functions that only read from this pointer. It also fixes the incorrectly named linkmode_set function. ==================== Signed-off-by: -
Russell King (Oracle) authored
We have an increasing number of drivers that are forcing auto-negotiation to be enabled for speeds of 1G or faster. It would appear that auto-negotiation is mandatory for speeds above 100M. In 802.3, Annex 40C's state diagrams seems to imply that mr_autoneg_enable (BMCR AN ENABLE) doesn't affect whether or not the AN state machines work for 1000base-T, and some PHY datasheets (e.g. Marvell Alaska) state that disabling mr_autoneg_enable leaves AN enabled but forced to 1G full duplex. Other PHY datasheets imply that BMCR AN ENABLE should not be cleared for >= 1G. Thus, this should be handled in phylib rather than in each driver. Rather than erroring out, arrange to implement the Marvell Alaska solution but in software for all PHYs: generate an appropriate single-speed advertisement for the requested speed, and keep AN enabled to the PHY driver. However, to avoid userspace API breakage, continue to report to userspace that we have AN disabled. Signed-off-by:
Russell King (Oracle) <rmk+kernel@armlinux.org.uk> Signed-off-by:
-
Russell King (Oracle) authored
Constify the advertising mask to linkmode functions that only read from the advertising mask. Signed-off-by:
-
David S. Miller authored
Javier Carrasco says: ==================== net: mvpp2: rework child node/port removal handling These two patches used to be part of another series [1] that did not apply to the networking tree without conflicts. This is therefore just a partial resend with no code modifications, just rebased onto net/main. Link: https://lore.kernel.org/all/20240806181026.5fe7f777@kernel.org/ [1] ==================== Signed-off-by:
Javier Carrasco <javier.carrasco.cruz@gmail.com> Signed-off-by:
-
Javier Carrasco authored
The iterated nodes are direct children of the device node, and the `device_for_each_child_node()` macro accounts for child node availability. `fwnode_for_each_available_child_node()` is meant to access the child nodes of an fwnode, and therefore not direct child nodes of the device node. The child nodes within mvpp2_probe are not accessed outside the loops, and the scoped version of the macro can be used to automatically decrement the refcount on early exits. Use `device_for_each_child_node()` and its scoped variant to indicate device's direct child nodes. Reviewed-by:
Jonathan Cameron <Jonathan.Cameron@huawei.com> Signed-off-by:
-
Javier Carrasco authored
As discussed in [1], there is no need to iterate over child nodes to remove the list of ports. Instead, a loop up to `port_count` ports can be used, and is in fact more reliable in case the child node availability changes. The suggested approach removes the need for the `fwnode` and `port_fwnode` variables in mvpp2_remove() as well. Link: https://lore.kernel.org/all/ZqdRgDkK1PzoI2Pf@shell.armlinux.org.uk/ [1] Suggested-by:
Russell King <linux@armlinux.org.uk> Signed-off-by:
-
David S. Miller authored
David Wei says: ==================== fix bnxt_en queue reset when queue is active The current bnxt_en queue API implementation is buggy when resetting a queue that has active traffic. The problem is that there is no FW involved to stop the flow of packets and relying on napi_disable() isn't enough. To fix this, call bnxt_hwrm_vnic_update() with MRU set to 0 for both the default and the ntuple vnic to stop the flow of packets. This works for any Rx queue and not only those that have ntuple rules since every Rx queue is either in the default or the ntuple vnic. For bnxt_hwrm_vnic_update() to work, proper flushing must be done by the FW. A FW flag is there to indicate support and queue_mgmt_ops is keyed behind this. The first three patches are from Michael Chan and adds the prerequisite vnic functions and FW flags indicating that it will properly flush during vnic update. Tested on BCM957504 while iperf3 is active: 1. Reset a queue that has an ntuple rule steering flow into it 2. Reset all queues in order, one at a time In both cases the flow is not interrupted. Sending this to net-next as there is no in-tree kernel consumer of queue API just yet, and there is a patch that changes when the queue_mgmt_ops is registered. Reviewed-by:
Wojciech Drewek <wojciech.drewek@intel.com> --- v3: - include patches from Michael Chan that adds a FW flag for vnic flush capability - key support for queue_mgmt_ops behind this new flag v2: - split setting vnic->mru into a separate patch (Wojciech) - clarify why napi_enable()/disable() is removed ==================== Signed-off-by:
-
David Wei authored
The queue API calls bnxt_hwrm_vnic_update() to stop/start the flow of packets, which can only properly flush the pipeline if FW indicates support. Add a macro BNXT_SUPPORTS_QUEUE_API that checks for the required flags and only set queue_mgmt_ops if true. Signed-off-by:
David Wei <dw@davidwei.uk> Signed-off-by:
-
David Wei authored
The current implementation when resetting a queue while packets are flowing puts the queue into an inconsistent state. There needs to be some synchronisation with the FW. Add calls to bnxt_hwrm_vnic_update() to set the MRU for both the default and ntuple vnic during queue start/stop. When the MRU is set to 0, flow is stopped. Each Rx queue belongs to either the default or the ntuple vnic. With calling bnxt_hwrm_vnic_update() the calls to napi_enable() and napi_disable() must be removed for reset to work on a queue that has active traffic flowing e.g. iperf3. Co-developed-by:
Somnath Kotur <somnath.kotur@broadcom.com> Signed-off-by:
-
David Wei authored
Set the newly added vnic->mru field in bnxt_hwrm_vnic_cfg(). Signed-off-by:
-
Michael Chan authored
Check the HWRM_VNIC_QCAPS FW response for the receive engine flush capability. This capability indicates that we can reliably support RX ring restart when calling HWRM_VNIC_UPDATE with MRU set to 0. Signed-off-by:
Michael Chan <michael.chan@broadcom.com> Signed-off-by:
-
Michael Chan authored
Add the function bnxt_hwrm_vnic_update() to call FW to update a VNIC. This call can be used when disabling and enabling a receive ring within a VNIC. The mru which is the maximum receive size of packets received by the VNIC can be updated. Signed-off-by:
-
Michael Chan authored
The main changes are: 1. HWRM_VNIC_UPDATE used to safely disable and enable an RX ring within the VNIC. 2. New flag in HWRM_VNIC_QCAPS to indicate FW will do the proper flush during HWRM_VNIC_UPDATE. 3. New flag in HWRM_FUNC_QCAPS to indicate that reservations for some resources such as VNIC can be reduced. 4. New backing store memory types not used by the driver yet. Signed-off-by:
-
David S. Miller authored
James Chapman says: ==================== l2tp: misc improvements This series makes several improvements to l2tp: * update documentation to be consistent with recent l2tp changes. * move l2tp_ip socket tables to per-net data. * fix handling of hash key collisions in l2tp_v3_session_get * implement and use get-next APIs for management and procfs/debugfs. * improve l2tp refcount helpers. * use per-cpu dev->tstats in l2tpeth devices. * fix a lockdep splat. * fix a race between l2tp_pre_exit_net and pppol2tp_release. ==================== Signed-off-by: -
James Chapman authored
syzbot exposes a race where a net used by l2tp is removed while an existing pppol2tp socket is closed. In l2tp_pre_exit_net, l2tp queues TUNNEL_DELETE work items to close each tunnel in the net. When these are run, new SESSION_DELETE work items are queued to delete each session in the tunnel. This all happens in drain_workqueue. However, drain_workqueue allows only new work items if they are queued by other work items which are already in the queue. If pppol2tp_release runs after drain_workqueue has started, it may queue a SESSION_DELETE work item, which results in the warning below in drain_workqueue. Address this by flushing the workqueue before drain_workqueue such that all queued TUNNEL_DELETE work items run before drain_workqueue is started. This will queue SESSION_DELETE work items for each session in the tunnel, hence pppol2tp_release or other API requests won't queue SESSION_DELETE requests once drain_workqueue is started. WARNING: CPU: 1 PID: 5467 at kernel/workqueue.c:2259 __queue_work+0xcd3/0xf50 kernel/workqueue.c:2258 Modules linked in: CPU: 1 UID: 0 PID: 5467 Comm: syz.3.43 Not tainted 6.11.0-rc1-syzkaller-00247-g3608d6ac #0 Hardware name: Google Compute Engine/Google Compute Engine, BIOS Google 06/27/2024 RIP: 0010:__queue_work+0xcd3/0xf50 kernel/workqueue.c:2258 Code: ff e8 11 84 36 00 90 0f 0b 90 e9 1e fd ff ff e8 03 84 36 00 eb 13 e8 fc 83 36 00 eb 0c e8 f5 83 36 00 eb 05 e8 ee 83 36 00 90 <0f> 0b 90 48 83 c4 60 5b 41 5c 41 5d 41 5e 41 5f 5d c3 cc cc cc cc RSP: 0018:ffffc90004607b48 EFLAGS: 00010093 RAX: ffffffff815ce274 RBX: ffff8880661fda00 RCX: ffff8880661fda00 RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000000 RBP: 0000000000000000 R08: ffffffff815cd6d4 R09: 0000000000000000 R10: ffffc90004607c20 R11: fffff520008c0f85 R12: ffff88802ac33800 R13: ffff88802ac339c0 R14: dffffc0000000000 R15: 0000000000000008 FS: 00005555713eb500(0000) GS:ffff8880b9300000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 0000000000000008 CR3: 000000001eda6000 CR4: 00000000003506f0 DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 Call Trace: <TASK> queue_work_on+0x1c2/0x380 kernel/workqueue.c:2392 pppol2tp_release+0x163/0x230 net/l2tp/l2tp_ppp.c:445 __sock_release net/socket.c:659 [inline] sock_close+0xbc/0x240 net/socket.c:1421 __fput+0x24a/0x8a0 fs/file_table.c:422 task_work_run+0x24f/0x310 kernel/task_work.c:228 resume_user_mode_work include/linux/resume_user_mode.h:50 [inline] exit_to_user_mode_loop kernel/entry/common.c:114 [inline] exit_to_user_mode_prepare include/linux/entry-common.h:328 [inline] __syscall_exit_to_user_mode_work kernel/entry/common.c:207 [inline] syscall_exit_to_user_mode+0x168/0x370 kernel/entry/common.c:218 do_syscall_64+0x100/0x230 arch/x86/entry/common.c:89 entry_SYSCALL_64_after_hwframe+0x77/0x7f RIP: 0033:0x7f061e9779f9 Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48 RSP: 002b:00007ffff1c1fce8 EFLAGS: 00000246 ORIG_RAX: 00000000000001b4 RAX: 0000000000000000 RBX: 000000000001017d RCX: 00007f061e9779f9 RDX: 0000000000000000 RSI: 000000000000001e RDI: 0000000000000003 RBP: 00007ffff1c1fdc0 R08: 0000000000000001 R09: 00007ffff1c1ffcf R10: 00007f061e800000 R11: 0000000000000246 R12: 0000000000000032 R13: 00007ffff1c1fde0 R14: 00007ffff1c1fe00 R15: ffffffffffffffff </TASK> Fixes: fc7ec7f5 ("l2tp: delete sessions using work queue") Reported-by: syzbot+0e85b10481d2f5478053@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=0e85b10481d2f5478053Signed-off-by:
James Chapman <jchapman@katalix.com> Signed-off-by:
Tom Parkin <tparkin@katalix.com> Signed-off-by:
-
James Chapman authored
l2tp_eth uses old-style dev->stats for fastpath packet/byte counters. Convert it to use dev->tstats per-cpu counters. Signed-off-by:
-
James Chapman authored
l2tp_tunnel_inc_refcount and l2tp_session_inc_refcount wrap refcount_inc. They add no value so just use the refcount APIs directly and drop l2tp's helpers. l2tp already uses refcount_inc_not_zero anyway. Rename l2tp_tunnel_dec_refcount and l2tp_session_dec_refcount to l2tp_tunnel_put and l2tp_session_put to better match their use pairing various _get getters. Signed-off-by:
-
James Chapman authored
l2tp netlink and procfs/debugfs iterate over tunnel and session lists to obtain data. They currently use very inefficient get_nth functions to do so. Replace these with get_next. For netlink, use nl cb->ctx[] for passing state instead of the obsolete cb->args[]. l2tp_tunnel_get_nth and l2tp_session_get_nth are no longer used so they can be removed. Signed-off-by:
-
James Chapman authored
l2tp management APIs and procfs/debugfs iterate over l2tp tunnel and session lists. Since these lists are now implemented using IDR, we can use IDR get_next APIs to iterate them. Add tunnel/session get_next functions to do so. The session get_next functions get the next session in a given tunnel and need to account for l2tpv2 and l2tpv3 differences: * l2tpv2 sessions are keyed by tunnel ID / session ID. Iteration for a given tunnel ID, TID, can therefore start with a key given by TID/0 and finish when the next entry's tunnel ID is not TID. This is possible only because the tunnel ID part of the key is the upper 16 bits and the session ID part the lower 16 bits; when idr_next increments the key value, it therefore finds the next sessions of the current tunnel before those of the next tunnel. Entries with session ID 0 are always skipped because they are used internally by pppol2tp. * l2tpv3 sessions are keyed by session ID. Iteration starts at the first IDR entry and skips entries where the tunnel does not match. Iteration must also consider session ID collisions and walk the list of colliding sessions (if any) for one which matches the supplied tunnel. Signed-off-by:
-
James Chapman authored
To handle colliding l2tpv3 session IDs, l2tp_v3_session_get searches a hashed list keyed by ID and sk. Although unlikely, if hash keys collide, it is possible that hash_for_each_possible loops over a session which doesn't have the ID that we are searching for. So check for session ID match when looping over possible hash key matches. Signed-off-by:
-
James Chapman authored
l2tp_ip[6] have always used global socket tables. It is therefore not possible to create l2tpip sockets in different namespaces with the same socket address. To support this, move l2tpip socket tables to pernet data. Signed-off-by:
-
James Chapman authored
Update l2tp to remove the inline keyword from several functions in C sources, since this is now discouraged. Signed-off-by:
-
James Chapman authored
l2tp no longer uses sk_user_data in tunnel sockets and now manages tunnel/session lifetimes slightly differently. Update docs to cover this. CC: linux-doc@vger.kernel.org CC: corbet@lwn.net Signed-off-by:
-
