Since the use of "pcbc(des)" in rxkad_decrypt_ticket() is the only
remaining user of the crypto API "pcbc" template, just implement
DES-PCBC by locally implementing PCBC mode on top of the DES library.
Note that only the decryption direction is needed.
This will allow support for the obsolete PCBC mode to be removed from
the crypto API.
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Tested-by: Marc Dionne <marc.dionne@auristor.com>
Link: https://patch.msgid.link/20260522050740.84561-4-ebiggers@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Use the local implementation of FCrypt-PCBC instead of the crypto API
one. This will allow the crypto API one to be removed. It also
simplifies the code quite a bit.
The local FCrypt-PCBC implementation is also significantly faster than
the crypto API one, since the crypto API one had a lot of overhead. For
example, benchmarking on an x86_64 CPU, I see that FCrypt-PCBC
decryption throughput improved from 83 MB/s to 157 MB/s.
(Meanwhile, AES-256-GCM decryption is 8064 MB/s on the same CPU.
Clearly, anyone looking for good performance, or anything that is
actually secure for that matter, needs to look elsewhere anyway.)
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Tested-by: Marc Dionne <marc.dionne@auristor.com>
Link: https://patch.msgid.link/20260522050740.84561-3-ebiggers@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Add a local implementation of FCrypt-PCBC encryption and decryption.
This will be used instead of the crypto API one, allowing the crypto API
one to be removed. It will also simplify rxkad.c quite a bit.
A KUnit test is included. The FCrypt-PCBC test vectors are borrowed
from the existing ones in crypto/testmgr.h. Note that this adds the
first KUnit test for net/rxrpc/, which previously had no KUnit tests.
The FCrypt code is based on crypto/fcrypt.c, but I simplified it a bit.
The PCBC part is straightforward and I just wrote it from scratch.
Tested with:
kunit.py run --kunitconfig net/rxrpc/
Acked-by: David Howells <dhowells@redhat.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Tested-by: Marc Dionne <marc.dionne@auristor.com>
Link: https://patch.msgid.link/20260522050740.84561-2-ebiggers@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Jakub Kicinski says:
====================
net: ethtool: let ops locked drivers run without rtnl_lock
With the ethtool_get_link_ksettings() situation hopefully ironed out
the previous series (commit 6a5d837f0c) let's return to the main
part of the series.
We have been slowly moving towards removing the rtnl_lock dependency
in driver ops since the concept of "ops-locked" drivers have been
introduced last year. Since last year will take the netdev instance
lock before invoking any ndo or ethtool op of "ops-locked" drivers.
We dipped our toes into rtnl_lock-less ops with the queue binding API.
Queue stats, NAPI, and other netdev-netlink objects are also queried
without holding rtnl_lock already. It's time to take the next logical
step and lift the requirement from ethtool ops.
The direct motivation for this patchset is that ethtool ops often
involve communicating with device FW, and may take a long time
to complete. Aggressive polling of device state on machines
with 10+ NICs have been shown to significantly increase rtnl_lock
pressure.
There's a handful of areas which still need rtnl_lock (see below).
I decided to convert everything to rtnl_lock-less by default, and
add a set of flags which let the drivers request rtnl_lock to still
be taken. I don't love this, but I'm worried that opt-in would be
even more confusing.
Known issues / exclusions:
- qdiscs - qdisc configuration currently assumes rtnl_lock, this
is mostly impacting set_channels callback. qdisc config is probably
the easiest one of the exclusions to tackle, it's fairly self-contained.
- features - even tho feature changes are (correctly) plumbed to
the driver thru ndos they are part of ethtool uAPI. ethtool itself
calls netdev_features_change() if it has spotted device feature change
before vs after to the callback. Some drivers also call
netdev_features_change() directly in response to various changes,
e.g. setting priv flags.
Since features have to propagate to upper and lower devices anything
that touches features is quite hard to move from under rtnl_lock.
- phylink - phylink and SFP depend on rtnl_lock today, I suspect
that this is purely for historic reasons. I started poking at
it and don't really see a need for a global lock. But accessing
the netdev instance lock from the SFP entry points will require
some attention from the phylink folks.
- phydev - similar to phylink, looks quite doable. But no ops-locked
driver currently has a phydev (fbnic only uses phylink) so phydev
related paths retain a ASSERT_RTNL() for now.
Tested on mlx5, bnxt and fbnic.
====================
Link: https://patch.msgid.link/20260605002912.3456868-1-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Catch up various bits of documentation after the locking changes.
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-13-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Convert the IOCTL path similarly to how we converted Netlink.
The device lookup gets a little hairy. We could take rtnl_lock
unconditionally and drop it before calling the driver (this would
avoid the reference + liveness check). But I think being able
to make progress even if rtnl is dead-locked is quite useful.
First extra concern is handling features. List all the cmds which
modify features and always take rtnl_lock. We could fold this list
into ethtool_ioctl_needs_rtnl() but seems cleaner to keep
ethtool_ioctl_needs_rtnl() driver-related. If a driver changed
features and we were not holding rtnl_lock - warn about it.
It can only happen on buggy ops locked drivers (buggy because
they should have set appropriate "I need rtnl for op X" bit).
Second wrinkle is the PHY ID hack which drops the locks while
sleeping. Convert its static "busy" variable which used to
be protected by rtnl_lock to a field in struct ethtool_netdev_state.
This feature is about identifying an adapter or a port within
a system, so being able to blink multiple LEDs at the same
time is likely not very useful in practice. But it's the simplest
fix, we can add a mutex if someone thinks a system should only
be ID'ing one port at a time.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-12-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Add another layer of helper functions to make upcoming locking
changes easier. Otherwise we'd need a pretty complex goto
structure. netdev instance lock is now taken slightly sooner
but that should not be an issue since rtnl_lock is already held,
anyway.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-11-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Skip rtnl_lock in RSS context handlers if device is ops-locked.
Fairly trivial conversion. bnxt needed rtnl_lock for changing
the main context but looks like additional contexts are fine
without it.
Note (for review bots?) that ethnl_ops_begin() checks whether
the device is still registered.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-10-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Module firmware flashing reads SFF-8024 identifier bytes via
.get_module_eeprom_by_page(). Other than that it modifies
a bit in the netdev->ethtool struct. Both should be ops-locked
at this point.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-9-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
ethnl_tsinfo_dumpit() iterates netdevs and per-netdev PHY topology
calling ops->get_ts_info(). Switch to the "ops compat locking"
helpers which take either rtnl_lock or instance lock, depending
on what the device needs.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-8-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Skip rtnl_lock in cable test handlers. This is really a noop since
no ops locked device supports these.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-7-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Make ethtool not take rtnl_lock for SET commands when operation
is performed on an ops-locked driver. cfg/cfg_pending are now
ops-locked, since only ethtool modifies them.
Some SET driver callbacks will still need rtnl_lock, most notably
those which may end up calling netdev_update_features() or the qdisc
layer (via netif_set_real_num_tx_queues()). Let drivers selectively
opt back into the rtnl_lock with a new bitfield in ops.
We need two helpers since Netlink and ioctl cmds have different
values. Keep the helpers side by side in common.h to make sure
they get updated together, even tho they will only get called
from ioctl.c and netlink.c.
SET commands which don't use ethnl_default_set_doit() are converted
by subsequent commits.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-6-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
ethnl_default_doit() and ethnl_default_dump_one() are both used
exclusively for GET callbacks (former to get info for a single
device or get global strings). ops-locked devices don't need
rtnl_lock for GET callbacks, stop taking it.
Introduce an opt-out mechanism for devices which use phylink (fbnic)
since phylink currently depends on rtnl_lock protection. Subsequent
patches will add more exceptions, anyway. Practically the new helpers
for judging if command needs rtnl_lock could also call
netdev_need_ops_lock() but I find that it makes the code in the callers
slightly less obvious.
Add a helper for IOCTLs already, even tho it's unused so that
we can keep them in sync as the series progresses.
This is the first user-visible step of moving ethtool ops out
from under rtnl. Subsequent patches do the same for SET ops,
as well as the ioctl path.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-5-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
dev->hwprov tracks the active hwtstamp provider for the device.
Make it ops protected (instance lock if the netdev driver opts
into holding instance lock around callbacks, otherwise rtnl_lock).
hwprov is written and read in:
- drivers/net/phy/phy_device.c
phydev and ops protection don't currently mix, add a comment
- net/ethtool/
as of now holds both rtnl lock and ops lock, this one will
soon only hold one lock or the other
read in:
- net/core/dev_ioctl.c
holds both rtnl lock and ops lock
- net/core/timestamping.c
RCU reader
The new netdev_ops_lock_dereference() helper does not have
"compat" in the name. The name would be quite long and I think
in this case it should be obvious that we need _a_ lock.
netdev_lock_dereference() already exists and means dev->lock
is always expected.
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-4-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
phydev <> netdev linking and lifecycle depends on rtnl_lock.
We want to switch to instance locks for most ethtool ops.
Let's add an assert that ops locked devices don't use phydev
today. If one does we can either opt the phy ops out of
being purely ops locked, or do deeper surgery to make phy
locking ops-compatible. I don't think there's any fundamental
challenge to make that work.
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-3-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
ethnl_bcast_seq is a global counter stamped into the nlmsg_seq field
of every multicast notification, allowing userspace to detect dropped
messages. Today the ordering is achieved by using rtnl_lock().
Moving forward we will want ethtool ops to run under just the netdev
instance lock so to establish ordering we need a separate lock
for notifications. With the netdev instance locks operations on
different devices may bypass each other but the expectation is
that it should not matter. What we need to prevent is:
- notification IDs getting out of order
- operations on one device getting out of order
For simplicity defer allocating the ID of the notification right
before the notification is delivered. This removes the need for
special handling in ethnl_rss_create_send_ntf().
Reviewed-by: Eric Dumazet <edumazet@google.com>
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20260605002912.3456868-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When ip-link output is unavailable, when the upcall daemon log has not
been written yet, or when pahole does not know the OVS drop subsystem
ID, the affected command substitutions silently produce empty strings.
The caller then passes empty sha= or pid= arguments to ovs_add_flow,
or matches against wrong drop reason codes, all without a diagnostic.
Add [ -z ] guards immediately after each assignment. For test_arp_ping,
also align the MAC extraction to use awk '/link\/ether/' as in
test_pop_vlan. The drop_reason guard returns ksft_skip because an
absent subsystem ID is an environment issue, not a test failure.
Signed-off-by: Minxi Hou <houminxi@gmail.com>
Reviewed-by: Aaron Conole <aconole@redhat.com>
Link: https://patch.msgid.link/20260604163016.3929371-1-houminxi@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Yuyang Huang says:
====================
ipv4: igmp: annotate diagnostic procfs data races
This patch series addresses several unannotated data races between lockless
RCU-protected diagnostic reads in /proc/net/igmp (igmp_mc_seq_show())
and concurrent writes in serialized paths (RTNL and group spinlocks).
Following the precedent in commit 061c0aa740 ("ipv4: igmp: annotate
data-races around im->users"), we annotate these intentional data races
using READ_ONCE() and WRITE_ONCE() macros.
- Patch 1 annotates races around `in_dev->mc_count` (interface-level joins).
- Patch 2 annotates races around active timer-related state tracking fields
(`tm_running`, `reporter`, `expires`) on individual multicast groups.
====================
Link: https://patch.msgid.link/20260605014318.3890804-1-yuyanghuang@google.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
/proc/net/igmp walks the multicast list locklessly under RCU and reads
timer-related fields (im->tm_running, im->reporter, im->timer.expires)
to print the timer state of multicast memberships. Concurrently, these
fields are modified under im->lock spinlock in timer management paths
(igmp_stop_timer(), igmp_start_timer(), and igmp_timer_expire()). Fix this
intentional lockless snapshot by annotating the lockless reads with
READ_ONCE() and the updates with WRITE_ONCE().
Signed-off-by: Yuyang Huang <yuyanghuang@google.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Link: https://patch.msgid.link/20260605014318.3890804-3-yuyanghuang@google.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
/proc/net/igmp walks the multicast list for IPv4 interfaces locklessly
under RCU and prints state->in_dev->mc_count. Concurrently, device
init/destruction and multicast join/leave paths update the count
under the RTNL lock. Fix this intentional lockless snapshot by
annotating the read with READ_ONCE() and the updates with WRITE_ONCE().
Signed-off-by: Yuyang Huang <yuyanghuang@google.com>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Link: https://patch.msgid.link/20260605014318.3890804-2-yuyanghuang@google.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Breno Leitao says:
====================
netconsole: Fix reported problems
These are some of the issues that LLM reported to netconsole, and they
are being addressed here before big refactors.
I was doing some big refactors, and got some "pre-existent-issues"
during LLM review of the refactor, that make them hard to guarantee that
refactor is not introducing any bug, so, let's clean these pre-existent
bugs first, and then submit the refactor.
The issues fixed in this patchset were reported during the review of
https://lore.kernel.org/all/20260524-netconsole_move_more-v1-0-909d1ab398b4@debian.org/
Not all of them got fixed, but, those that were easy to reason about.
Why net-next and not 'net' tree.
Most of the functions that are being fixed here moved from netpoll to
netconsole, thus, fixing this on net will cause merge conflicts from
'net' to 'net-next', thus I decided to fix it on 'net-next', given we
are on 7.1-rc6 already. Sorry if that is not the right approach.
====================
Link: https://patch.msgid.link/20260604-netcons_fix_before_move-v3-0-ab055b3a6aa5@debian.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
process_resume_target() removes the target from target_list before
calling resume_target() so that netpoll_setup() can run with interrupts
enabled, then re-adds it once setup completes. netpoll_setup() acquires a
net_device reference (netdev_hold()) and releases the RTNL before
returning.
While the target is off target_list and the RTNL is not held,
netconsole_netdev_event() cannot find it. If the egress device is
unregistered in that window, the NETDEV_UNREGISTER notifier walks
target_list, misses the resuming target, and never tears it down. The
target is then re-added in STATE_ENABLED still holding a reference to the
now-unregistered device, leaking it and hanging unregister_netdevice() in
netdev_wait_allrefs().
Re-check under RTNL before re-publishing the target: if the device left
NETREG_REGISTERED while we were off the list, run do_netpoll_cleanup() and
mark the target disabled. Taking the RTNL across the check and the
list_add() serialises against the NETDEV_UNREGISTER notifier, which also
runs under RTNL, so the device is either still registered (and the
notifier will find the re-added target later) or already unregistering
(and we drop the reference here). netdev_wait_allrefs() runs from
netdev_run_todo() outside the RTNL, so dropping the reference here cannot
deadlock against the pending unregister.
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260604-netcons_fix_before_move-v3-5-ab055b3a6aa5@debian.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
drop_netconsole_target() downgrades a STATE_DEACTIVATED target to
STATE_DISABLED and then only calls netpoll_cleanup() when the target is
STATE_ENABLED. A target becomes STATE_DEACTIVATED when its underlying
interface is unregistered: netconsole_netdev_event() moves it to
target_cleanup_list, and netconsole_process_cleanups_core() is expected
to run do_netpoll_cleanup() on it.
Now that drop_netconsole_target() takes target_cleanup_list_lock around
the unlink, a configfs removal racing with NETDEV_UNREGISTER can pull the
target off target_cleanup_list before the cleanup worker processes it.
The notifier drops the lock before calling
netconsole_process_cleanups_core(), so the worker then iterates a list
that no longer contains the target and never runs do_netpoll_cleanup() on
it. Because drop_netconsole_target() has already rewritten the state to
STATE_DISABLED, its own STATE_ENABLED check is false and netpoll_cleanup()
is skipped too. The net_device reference taken by netpoll_setup() is then
leaked and unregister_netdevice() hangs forever in netdev_wait_allrefs().
Capture whether the target still owns a netpoll before the state is
downgraded and clean it up for both STATE_ENABLED and STATE_DEACTIVATED
targets. netpoll_cleanup() is idempotent -- it skips when np->dev is
already NULL -- so it is safe even when the cleanup worker won the race
and already tore the netpoll down.
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260604-netcons_fix_before_move-v3-4-ab055b3a6aa5@debian.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
drop_netconsole_target() unlinks the target while only holding
target_list_lock. However, when the underlying interface has been
unregistered, netconsole_netdev_event() moves the target from
target_list to target_cleanup_list, and netconsole_process_cleanups_core()
walks that list under target_cleanup_list_lock only.
If a user removes the configfs target at the same time the cleanup
worker is iterating target_cleanup_list, list_del() can corrupt the list
because the two paths take disjoint locks while operating on the same
list node.
Acquire target_cleanup_list_lock around the list_del() so the unlink is
serialised against netconsole_process_cleanups_core() regardless of
which list the target currently belongs to. The state transition that
downgrades STATE_DEACTIVATED to STATE_DISABLED is left intact and is
performed under the same combined locking, preserving the existing
ordering with resume_target().
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260604-netcons_fix_before_move-v3-3-ab055b3a6aa5@debian.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
find_skb() falls back to np->skb_pool when the GFP_ATOMIC alloc_skb()
fails. The pool is refilled by refill_skbs(), which always allocates
buffers of MAX_SKB_SIZE (ethhdr + iphdr + udphdr + MAX_UDP_CHUNK ==
1502 bytes).
netconsole, however, computes the requested length dynamically as
total_len + np->dev->needed_tailroom
If the egress device declares a non-zero needed_tailroom (e.g. some
tunnel or hardware accelerator devices), the required length can exceed
MAX_SKB_SIZE. The pooled skb is then handed back to the caller, which
immediately performs skb_put(skb, len), trips the tail > end check, and
triggers skb_over_panic().
Leave the normal alloc_skb(len, GFP_ATOMIC) path untouched -- the slab
allocator can still satisfy oversized requests when memory is available,
so senders to devices with non-zero needed_tailroom keep working in the
common case. Only the pool fallback is gated: when alloc_skb() failed
and len exceeds the pool buffer size, skip the skb_dequeue() instead of
burning a pre-allocated skb on a request that would later trip
skb_over_panic(). Reserving pool entries for requests they can actually
satisfy also keeps the panic path, which depends on the pool being
primed, intact.
When that drop happens, emit a rate-limited net_warn() so the user
notices that netconsole is unable to push messages on the egress device.
The warn is skipped under in_nmi() for the same reason schedule_work()
is: printk machinery taken by net_warn_ratelimited() is not NMI-safe and
would risk recursing into the same nbcon console we are servicing.
MAX_SKB_SIZE / MAX_UDP_CHUNK were private to net/core/netpoll.c. Move
them to include/linux/netpoll.h so netconsole can reference the same
definition that refill_skbs() uses, keeping the two in sync by
construction. The header now pulls in <linux/ip.h> and <linux/udp.h>
explicitly so MAX_SKB_SIZE remains self-contained for any future user.
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260604-netcons_fix_before_move-v3-2-ab055b3a6aa5@debian.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
When alloc_skb() fails in find_skb(), the fallback path dequeues an skb
from np->skb_pool and unconditionally calls schedule_work() to top the
pool back up. schedule_work() ends up taking the workqueue pool locks,
which are not NMI-safe.
netconsole_write() is registered as the nbcon write_atomic callback and
is explicitly marked CON_NBCON_ATOMIC_UNSAFE, meaning it is invoked from
emergency/panic contexts including NMIs. If the NMI interrupts a thread
already holding the workqueue pool lock, calling schedule_work()
self-deadlocks and the panic message that was being printed is lost.
Introduce netcons_skb_pop() to fold the pool dequeue and the refill
request into a single helper. The helper skips schedule_work() when
called from NMI context; the pool is best-effort, so the refill is simply
deferred to the next non-NMI find_skb() call that exhausts alloc_skb()
and hits the fallback again. This keeps the fast path untouched and the
locking rules around the fallback pool documented in one place.
Note this only removes the schedule_work() hazard from the NMI path. The
allocation itself is still not fully NMI-safe: the alloc_skb(GFP_ATOMIC)
attempted first may take slab locks, and the skb_dequeue() fallback takes
np->skb_pool.lock, so either can deadlock if the NMI interrupts a holder
of those locks. Closing those windows requires an NMI-safe (lockless) skb
pool and is left to a follow-up; this patch addresses the schedule_work()
deadlock, which is both the most likely and the easiest to trigger.
Signed-off-by: Breno Leitao <leitao@debian.org>
Link: https://patch.msgid.link/20260604-netcons_fix_before_move-v3-1-ab055b3a6aa5@debian.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
GRO test is very timing-sensitive, packets may be delayed
by the network or just sent slowly. Because of this we retry
each test case up to 6 times.
This makes perfect sense for positive cases, in which we want
to see coalescing. Negative test cases, which modify headers
and expect no coalescing should have opposite treatment.
We should really try 6 times and make sure that each time
the test failed. This would, however, require that we annotate
each test to indicate whether its positive or negative.
Let's start with a simpler improvement. Do not allow
retries if we detected over-coalescing. Previously the negative
case would have to get lucky at least once in 6 tries to pass.
Now the first failure breaks the retry loop.
For background - NICs tend to ignore the contents of the TCP
timestamp option, so that test case commonly fails. In NIPA
having 6 attempts, however, was enough for some NICs to get
multiple successful runs in a row, getting the test cases
auto-classified as expected to pass, even tho the NIC does
not comply with the expectations.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Reviewed-by: Willem de Bruijn <willemb@google.com>
Link: https://patch.msgid.link/20260607002401.212976-1-kuba@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Commit 2ccccf5fb4 ("net_sched: update hierarchical backlog too") renamed
qdisc_tree_decrease_qlen() to qdisc_tree_reduce_backlog(), but this comment
was missed. Update it.
Signed-off-by: Victor Nogueira <victor@mojatatu.com>
Link: https://patch.msgid.link/20260605212239.2261320-1-victor@mojatatu.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Chuck Lever says:
====================
tls: receive-path fixes and clean-ups
I'd like to encourage in-kernel kTLS consumers (NFSD, NVMe/TCP) to
coalesce on the use of read_sock. While auditing read_sock for that
purpose, Hannes and Sabrina flagged a few rough edges in the receive
paths.
This series is a set of clean-ups, not a performance series. Async
batch decryption and its submit/deliver scaffolding were dropped
during previous review: async_capable is always false for TLS 1.3,
the version NFSD and NVMe/TCP both require, so async-related
improvements were unreachable for the in-kernel consumers this
work targets.
A subsequent series will introduce infrastructure to support
KeyUpdate for in-kernel kTLS consumers, which need to handle TLS
Alert messages that trigger a tlshd upcall.
====================
Link: https://patch.msgid.link/20260604-tls-read-sock-v12-0-b114efa6e3e2@oracle.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
While lock_sock is held, incoming TCP segments land on
sk->sk_backlog rather than sk->sk_receive_queue.
tls_rx_rec_wait() inspects only sk_receive_queue, so backlog
data remains invisible. For non-blocking callers (read_sock,
and recvmsg or splice_read with MSG_DONTWAIT) this causes a
spurious -EAGAIN. For blocking callers it forces an
unnecessary sleep/wakeup cycle.
Flush the backlog inside tls_rx_rec_wait() before checking
sk_receive_queue so the strparser can parse newly-arrived
segments immediately. On the next loop iteration
tls_read_flush_backlog() may redundantly flush, but this
path is cold and the cost is negligible.
Backlog processing can run tcp_reset(), which calls
tcp_done_with_error() to set sk->sk_err = ECONNRESET and then
tcp_done() to set sk->sk_shutdown = SHUTDOWN_MASK. The pre-existing
top-of-loop sk_err check already ran before the flush, so the
freshly-set error would be masked by the next-line sk_shutdown test
returning 0 (EOF). Re-check sk_err immediately before the sk_shutdown
test so a connection abort surfaces as -ECONNRESET rather than a clean
EOF.
Commit f508262ae9 ("tls: Preserve sk_err across recvmsg() when
data has been copied") gave the top-of-loop sk_err check a
has_copied split. The recheck applies the same handling: when the
caller has already copied bytes, sk_err is reported but preserved
so the error surfaces on the next call; otherwise sock_error()
consumes it so the error is reported exactly once.
Suggested-by: Sabrina Dubroca <sd@queasysnail.net>
Link: https://lore.kernel.org/netdev/ahgHgQ84RCc8uYrG@krikkit/
Reviewed-by: Hannes Reinecke <hare@suse.de>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Link: https://patch.msgid.link/20260604-tls-read-sock-v12-6-b114efa6e3e2@oracle.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Each record release via tls_strp_msg_done() triggered
tls_strp_check_rcv(), which called tls_rx_msg_ready() and
fired saved_data_ready(). During a multi-record receive, the
first N-1 wakeups are pure overhead: the caller is already
running and will pick up subsequent records on the next loop
iteration. The recvmsg and splice_read paths share this waste.
Suppress per-record notifications and emit a single one on
reader exit. tls_rx_rec_done() releases the current record
and parses the next without announcing; tls_strp_check_rcv()
gains a bool announce parameter so callers can request the
quiet form. tls_rx_reader_release() fires the deferred
announce on exit through tls_rx_msg_maybe_announce(), an
idempotent helper that calls saved_data_ready() only when a
record is parsed and has not yet been announced.
To keep the final notification idempotent against records that
the BH or the worker has already announced, tls_strparser gains
a msg_announced bit. tls_rx_msg_maybe_announce() sets the bit
when firing saved_data_ready(); the bit is cleared whenever
the parsed record is wiped, by tls_strp_msg_consume() on
consumption or by tls_strp_msg_load() when the lower socket
loses bytes from under the parse. A second call for the same
parsed record -- as when recvmsg() satisfies the request from
ctx->rx_list without touching the strparser -- becomes a
no-op.
With no remaining callers, tls_strp_msg_done() is removed.
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Link: https://patch.msgid.link/20260604-tls-read-sock-v12-5-b114efa6e3e2@oracle.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
tls_strp_msg_done() conflates releasing the current record with
checking for the next one via tls_strp_check_rcv(). A subsequent
patch needs to release a record without immediately triggering
that check, so the release step is separated into
tls_strp_msg_consume(). tls_strp_msg_done() is preserved as a
wrapper for existing callers.
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Alistair Francis <alistair.francis@wdc.com>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Link: https://patch.msgid.link/20260604-tls-read-sock-v12-4-b114efa6e3e2@oracle.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Three receive paths -- recvmsg, read_sock, and splice_read --
each follow tls_rx_one_record() with the same tls_err_abort()
call. Consolidate the abort into tls_rx_one_record() so the
decrypt-and-abort sequence lives in one place.
A tls_check_pending_rekey() failure after successful
decryption no longer triggers tls_err_abort(). That path
fires only when skb_copy_bits() fails on a valid skb,
which is not a realistic scenario.
Suggested-by: Sabrina Dubroca <sd@queasysnail.net>
Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Link: https://patch.msgid.link/20260604-tls-read-sock-v12-3-b114efa6e3e2@oracle.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
The tls_sw_read_sock() loop releases the current skb whether
read_actor() consumed the full record or only a prefix. When
the actor takes only part of the record and leaves desc->count
non-zero, the remainder is lost: skb is neither requeued nor
freed, and the next iteration overwrites it during dequeue or
tls_rx_rec_wait().
No mainline consumer reaches this path today. The only
in-tree TLS read_sock user is nvme/tcp, whose actor
nvme_tcp_recv_skb() loops until the input length is exhausted
and returns either the full length or a negative error.
The path becomes reachable with the upcoming NFSD svcsock
receive built on read_sock_cmsg. Its data actor,
svc_tcp_recv_actor(), parses an RPC fragment stream
incrementally and returns at fragment boundaries. When a TLS
record carries the tail of one RPC fragment plus the head of
the next, the actor returns fewer bytes than offered while
leaving desc->count non-zero, and without re-presentation the
trailing fragment header vanishes.
__tcp_read_sock() handles the equivalent case for plain TCP
by leaving the unread bytes available for the next iteration
to re-present, via sequence-number re-lookup. Adopt the same
loop-level behavior: when read_actor() consumes only part of
the record, update rxm->offset and rxm->full_len and requeue
the skb to the head of rx_list so the next iteration
re-presents the unread bytes. Switch the open-ended for-loop
to "while (desc->count)" so the partial- and full-consume
arms share a single exit check and read_actor() is not
re-invoked once desc->count is exhausted.
Cc: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Link: https://patch.msgid.link/20260604-tls-read-sock-v12-2-b114efa6e3e2@oracle.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
tls_sw_read_sock() ends its receive loop with while (skb), but
the else branch in the body calls consume_skb(skb) before the
predicate is re-evaluated. A pointer becomes indeterminate when
the object it points to reaches end-of-lifetime (C2011 6.2.4p2),
and using an indeterminate value is undefined behavior (Annex
J.2). The pointer is not dereferenced today -- the predicate
either exits the loop or skb is overwritten at the top of the
next iteration -- but any future change that adds a dereference
between consume_skb() and the predicate would silently introduce
a use-after-free.
Replace the do/while form with an explicit for(;;) loop so
termination happens through a break statement rather than
predicate evaluation of a freed pointer.
Cc: Sagi Grimberg <sagi@grimberg.me>
Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
Reviewed-by: Hannes Reinecke <hare@kernel.org>
Reviewed-by: Sabrina Dubroca <sd@queasysnail.net>
Link: https://patch.msgid.link/20260604-tls-read-sock-v12-1-b114efa6e3e2@oracle.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Because LLC wasn't complicated/annoying enough, there's 2 more
"ethertypes" being used for it:
- 0x8870 is pretty "normal", it got standardized in
802.1AC-2016/Cor1-2018 for transporting LLC frames > 1500 bytes.
It simply replaces the length value (which is no longer encoded, and
must now be derived from the packet.) The actual value dates back to
2001; https://datatracker.ietf.org/doc/html/draft-ietf-isis-ext-eth-01
(it was used without "proper" standardization for a long time)
- 0x00fe is a doozy - actually "invalid" depending on how you look at
it; it's used in GRE (and possibly GENEVE) tunnels to transport the
IS-IS routing protocol. https://seclists.org/tcpdump/2002/q4/61 is
the best/oldest source I could find. It's inspired by the 0xfe SAP
value, a GRE packet with protocol 0x00fe is followed by a payload "as
if" it was Ethernet with "<length> 0xfe 0xfe 0x03". (Again the length
isn't encoded explicitly anymore.)
The 0x00fe value is quite close to other values the kernel is using
internally for various things (after all they "won't clash for 1500
types"). Except this one does clash, and if someone unknowingly starts
using it for something internal... we end up in a world of pain in
getting IS-IS running on GRE tunnels. Hence the "WARNING".
Signed-off-by: David 'equinox' Lamparter <equinox@diac24.net>
Cc: Andrew Lunn <andrew+netdev@lunn.ch>
Link: https://patch.msgid.link/20260605164144.81184-1-equinox@diac24.net
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
garp_pdu_parse_attr() keeps a pointer into the skb linear area across
pskb_may_pull(skb, ga->len), and garp_pdu_parse_msg() dereferences gm
on every loop iteration even though the nested parse may pull again.
pskb_may_pull() can reallocate the skb head, which would leave those
pointers stale.
This is not reachable today: GARP PDUs arrive via the 802.2 LLC SAP
path, where llc_fixup_skb() already pulls and trims the whole payload
into the linear area, so the inner pulls never reallocate. Reload ga
after the pull and snapshot gm->attrtype into a local anyway, to harden
the parser and match the skb_header_pointer() discipline used by mrp.c.
No functional change.
Signed-off-by: David Carlier <devnexen@gmail.com>
Link: https://patch.msgid.link/20260604141925.237746-1-devnexen@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Combine cpumask_copy() and cpumask_andnot() into a single
cpumask_andnot() since the function can take cpu_online_mask
directly as the source.
Signed-off-by: Fushuai Wang <wangfushuai@baidu.com>
Reviewed-by: Shay Drory <shayd@nvidia.com>
Reviewed-by: Tariq Toukan <tariqt@nvidia.com>
Link: https://patch.msgid.link/20260605101756.91275-1-fushuai.wang@linux.dev
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
napi_disable() is not idempotent and calling it on an already-disabled
or unenabled NAPI context will cause the kernel to spin indefinitely
waiting for the NAPI_STATE_SCHED bit to clear.
In mal_remove(), napi_disable() is called unconditionally. If no MACs were
registered, NAPI was never enabled. Also, if they were registered but
subsequently unregistered, NAPI was already disabled in
mal_unregister_commac(). In either case, calling napi_disable() causes
the kernel to hang upon module removal.
Fix this by only calling napi_disable() in mal_remove() if the commac list
is not empty (which implies NAPI is enabled).
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Link: https://patch.msgid.link/20260603230821.5619-1-rosenp@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Clear MAL descriptor rings with explicit field stores instead of
memset(). The descriptor rings are carved from MAL coherent DMA memory,
which may be mapped uncached on 32-bit powerpc. The optimized memset()
path can use dcbz there and trigger an alignment warning.
Use WRITE_ONCE() for each field to prevent the compiler from merging
the stores back into a memset() call.
The skb tracking arrays remain ordinary CPU memory and still use memset().
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Link: https://patch.msgid.link/20260603230754.5535-1-rosenp@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
platform_get_irq() returns a negative errno on failure.
Commit c4f5d0454c moved the platform_get_irq() calls and explicitly
removed the error checks that were previously present, claiming
devm_request_irq() can handle it. However, a negative IRQ number
passed to devm_request_irq() fails with -EINVAL instead of
propagating the real error from platform_get_irq().
Restore the missing error checks with proper errno propagation.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20260603211734.30750-1-rosenp@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
pdsc_teardown() frees DMA buffers but does not disable bus mastering,
leaving the device able to perform DMA after the buffers are freed.
This can lead to use-after-free if the device writes to freed memory.
Add pci_clear_master() to pdsc_teardown() to disable bus mastering
before freeing resources, ensuring all DMA is quiesced.
Add pci_set_master() to pdsc_setup() to re-enable bus mastering,
which is needed for the firmware recovery path since pdsc_teardown()
now disables it.
Signed-off-by: Nikhil P. Rao <nikhil.rao@amd.com>
Link: https://patch.msgid.link/20260604213637.3844317-1-nikhil.rao@amd.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Kuniyuki Iwashima says:
====================
ip6mr: No RTNL for RTNL_FAMILY_IP6MR rtnetlink.
This series is the IPv6 version of
https://lore.kernel.org/netdev/20260228221800.1082070-1-kuniyu@google.com/
and removes RTNL from ip6mr rtnetlink handlers.
After this series, there are a few RTNL left in net/ipv6/ip6mr.c
and such users will be converted to per-netns RTNL in another
series.
Patch 1 extends the ipmr selftest to exercise most of the RTNL
paths in net/ipv6/ipmr.c
Patch 2 - 6 converts RTM_GETROUTE handlers to RCU.
Patch 7 removes struct fib_dump_filter.rtnl_held.
Patch 8 use RCU for mr_table for CONFIG_IPV6_MROUTE_MULTIPLE_TABLES=n
for ->exit_rtnl().
Patch 9 move fib_rules_unregister() to ->exit()
Patch 10 - 12 converts ->exit_batch() to ->exit_rtnl() to
save one RTNL in cleanup_net().
Patch 13 removes unnecessary RTNL during setup_net() failure.
Patch 14 drops RTNL for MRT6_(ADD|DEL)_MFC(_PROXY)?.
Patch 15 misc clean up
v2: https://lore.kernel.org/20260410211726.1668756-1-kuniyu@google.com
v1: https://lore.kernel.org/20260407212001.2368593-1-kuniyu@google.com
====================
Link: https://patch.msgid.link/20260604224712.3209821-1-kuniyu@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
net->ipv6.ip6mr_notifier_ops and net->ipv6.ipmr_seq are used
only in net/ipv6/ip6mr.c.
Let's move these definitions under CONFIG_IPV6_MROUTE.
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20260604224712.3209821-16-kuniyu@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
ip6mr does not have rtnetlink interface for MFC unlike ipmr,
which uses dev_get_by_index_rcu() to set struct mfcctl.mfcc_parent.
ip6mr_mfc_add() and ip6mr_mfc_delete() are called under RTNL
from ip6_mroute_setsockopt() only.
There are no RTNL dependant, but ip6_mroute_setsockopt() reuses
RTNL just for mrt->mfc_hash and mrt->mfc_cache_list.
Let's replace RTNL with a new per-netns mutex.
Later, ip6mr_notifier_ops and ipmr_seq will be moved under
CONFIG_IPV6_MROUTE.
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20260604224712.3209821-15-kuniyu@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
When ip6mr_free_table() is called from ip6mr_rules_init() or
ip6mr_net_init(), the netns is not yet published.
Thus, no device should have been registered, and
mroute_clean_tables() will not call mif6_delete(), so
unregister_netdevice_many() is unnecessary.
unregister_netdevice_many() does nothing if the list is empty,
but it requires RTNL due to the unconditional ASSERT_RTNL()
at the entry of unregister_netdevice_many_notify().
Let's remove unnecessary RTNL and ASSERT_RTNL() and instead
add WARN_ON_ONCE() in ip6mr_free_table().
Note that we use a local list for the new WARN_ON_ONCE() because
dev_kill_list passed from ip6mr_rules_exit_rtnl() may have some
devices when other ops->init() fails after ipmr durnig setup_net().
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20260604224712.3209821-14-kuniyu@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
ip6mr_net_ops uses ->exit_batch() to acquire RTNL only once
for dying network namespaces.
ip6mr does not depend on the ordering of ->exit_rtnl() and
->exit_batch() of other pernet_operations (unlike fib_net_ops).
Once ip6mr_free_table() is called and all devices are
queued for destruction in ->exit_rtnl(), later during
NETDEV_UNREGISTER, ip6mr_device_event() will not see anything
in vif table and just do nothing.
Let's convert ip6mr_net_exit_batch() to ->exit_rtnl().
We will remove RTNL and unregister_netdevice_many() in
ip6mr_rules_init().
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20260604224712.3209821-13-kuniyu@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This is a prep commit to convert ip6mr_net_exit_batch() to
->exit_rtnl().
Let's move unregister_netdevice_many() in ip6mr_free_table()
to its callers.
Now ip6mr_rules_exit() can do batching all tables per netns.
Note that later we will remove RTNL and unregister_netdevice_many()
in ip6mr_rules_init().
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20260604224712.3209821-12-kuniyu@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This is a prep commit to convert ip6mr_net_exit_batch() to
->exit_rtnl().
Let's move unregister_netdevice_many() in mroute_clean_tables()
to its callers.
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20260604224712.3209821-11-kuniyu@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
fib_rules_unregister() removes ops from net->rules_ops under
spinlock, calls ops->delete() for each rule, and frees the ops.
ip6mr_rules_ops_template does not have ->delete(), and any
operation does not require RTNL there.
Let's move fib_rules_unregister() from ip6mr_rules_exit() to
ip6mr_net_exit().
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
Link: https://patch.msgid.link/20260604224712.3209821-10-kuniyu@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
