| Age | Commit message (Collapse) | Author |
|
mac_finish()
ethqos->phy_mode is read by serdes_powerup() and
qcom_ethqos_set_sgmii_loopback() to configure the SerDes and its
wrapper for the correct interface. It is set once at probe from the DT
phy-mode property and never updated.
phylink resolves the actual runtime interface and passes it to the
mac_finish() callback. On platforms that only ever operate at one speed
these two values are always the same, but when the interface changes
at runtime - for example switching between SGMII and 2500BASE-X - the
stored value becomes stale. The next serdes_powerup() call, on
a subsequent link cycle, then reinitializes the SerDes to the wrong mode.
Update ethqos->phy_mode from the interface argument in
ethqos_mac_finish_serdes() so that it always reflects the last
resolved interface.
With that, we now update the phy_mode to the runtime-resolved interface,
so any subsequent resume with a 1G SGMII link skips the loopback setup
and the DMA reset hangs. To remedy this, extend the guard to include
PHY_INTERFACE_MODE_SGMII.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>
Link: https://patch.msgid.link/20261014-qcom-sa8255p-emac-v16-4-0266c2ead980@oss.qualcomm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Call phy_set_mode_ext() before phy_power_on() in
qcom_ethqos_serdes_powerup(). This is harmless for existing users but on
SCMI systems this is required for the PHY driver to select the right
performance level - which translates to the link speed. This is done
ahead of adding support for the firmware-managed EMAC on Qualcomm sa8255p.
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Reviewed-by: Mohd Ayaan Anwar <mohd.anwar@oss.qualcomm.com>
Link: https://patch.msgid.link/20261014-qcom-sa8255p-emac-v16-3-0266c2ead980@oss.qualcomm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
dwmac4_core_init() derives the TIC counter value from the rate of the CSR
clock. If no such clock is available (for instance because clocks are
managed by firmware and not described in DT), the rate is 0 and
(0 / 1000000) - 1 underflows to 0xffffffff, which is then written
to the register and can break the LPI timers that count in microseconds.
Leave the register at its reset value when the CSR clock rate is
unknown. The RX watchdog conversion helpers already cope with a missing
clock by falling back to clk_ref_rate or returning 0, so nothing else
needs to change.
This is first hit with the Qualcomm sa8255p ethernet, whose DT has no
clocks.
Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/all/179052656185.2160803.1751830002645522416@kernel.org/
Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com>
Link: https://patch.msgid.link/20261014-qcom-sa8255p-emac-v16-1-0266c2ead980@oss.qualcomm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
si3474 prints the error of a failed controller registration as a hex
number, such as 0xfffffff4 for -ENOMEM. Use dev_err_probe(), as
pd692x0 and tps23881 already do. It prints the error by name, and if
the registration ever returns -EPROBE_DEFER, it does not log that as
an error.
Only the log message changes.
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
Link: https://patch.msgid.link/20261020171134.852878-3-github@szelinsky.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
If devm_add_action_or_reset() fails, it has already unregistered the
ancillary I2C client, and the error message then reads that client's
device. It can only fail with -ENOMEM, which is already reported, so
drop the message.
Found by code inspection. Not reproduced.
Fixes: a2317231df4b ("net: pse-pd: Add Si3474 PSE controller driver")
Reviewed-by: Kory Maincent <kory.maincent@bootlin.com>
Signed-off-by: Carlo Szelinsky <github@szelinsky.de>
Link: https://patch.msgid.link/20261020171134.852878-2-github@szelinsky.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
vxlan_vnifilter_process() parses the message with vni_filter_policy,
whose VXLAN_VNIFILTER_ENTRY is a bare NLA_NESTED with no nested policy
attached. The entry attributes are therefore validated only later, one
entry at a time, by the nla_parse_nested() inside
vxlan_process_vni_filter(). Entries are parsed as they are dispatched:
in a message whose first entry is valid and whose second is not, the
first entry is applied and its RTM_NEWTUNNEL notification sent before
the second is rejected.
Link the nest to vni_filter_entry_policy with NLA_POLICY_NESTED() so
every entry is checked against the entry policy up front, before any
entry is acted on. A message whose entry the policy rejects -- a
malformed attribute, or, with the next patch, an out-of-range endpoint
-- is now refused as a unit and installs nothing. Faults the policy does
not cover, such as a device or handler error raised on a later entry,
are unchanged: entries are still dispatched and applied one at a time.
This reorders two faults. The nest is validated before the device is
looked up and before the vnifilter flag is checked, so a request
rejected by the entry policy, aimed at a missing or non-vnifilter
device, now returns that policy error where it previously returned
-ENODEV or -EOPNOTSUPP. The request was invalid either way; only the
errno an operator sees changes.
The nla_parse_nested() in vxlan_process_vni_filter() stays: it is what
fills the per-entry attribute table the handler reads. It can no longer
fail on a message that has reached it.
Suggested-by: Jakub Kicinski <kuba@kernel.org>
Link: https://lore.kernel.org/netdev/20260921150904.65a704eb@kernel.org/
Signed-off-by: Ali Firas <alishmery18@gmail.com>
Link: https://patch.msgid.link/20261014123814.388557-2-alishmery18@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
ibmveth_rx_mss_helper() and ibmveth_rx_csum_helper() trust the IP and
TCP headers of received frames, which come from another partition on
the virtual LAN.
The worst case is an IPv4 frame with ihl 0 and iph->check 0xffff: the
checksum helper calls ip_fast_csum(iph, 0), and the ppc64 version
loops ihl - 1 times on an unsigned count, reading up to 16 GiB in
softirq: a long stall, or an oops once the read leaves mapped memory.
Both helpers also read and write header fields past the end of a short
frame. The large send helper takes the segment size from the TCP
checksum field and gso_segs from a TCP header length it never checks,
and treats an IPv4 TCP fragment as a large send, which can zero two of
its bytes; the checksum helper can write a checksum into a later
fragment's payload.
The ihl problem dates from the first commit in Fixes:, the large send
helper from the second, and the checksum helper from the third.
Before using a header, check that it fits the frame, that the IPv4 ihl
is at least 5, and that an IPv4 TCP frame is not a fragment. In the
large send helper, also require a TCP doff of at least 5, TCP payload
within the IP length, and an IP length equal to the frame length:
Ethernet padding or other bytes past the IP length would otherwise
stay in a bridged or switched frame, which the stack does not trim,
and be segmented but not counted in gso_segs. Compute gso_segs from
the IP length, and keep the segment size at or above TCP_MIN_GSO_SIZE,
as virtio_net_hdr_to_skb() does since commit c27c449d455a ("virtio-net:
Ensure that TCP packets don't overflow gso_segs"), and below
GSO_BY_FRAGS, which virtio rejects. With the headers checked and
gso_segs computed, the skb is not marked SKB_GSO_DODGY, which would
also keep it from hardware TSO on the egress device.
A frame that fails a check is passed up unchanged, without the GSO
fields or CHECKSUM_UNNECESSARY, so nothing downstream trusts its
segment count or checksum. The skb is linear, so the checks use
skb_headlen(); pskb_may_pull() could reallocate the head under the
helpers' header pointers.
Add KUnit cases for both helpers: valid IPv4 and IPv6 large sends, the
segment size clamp, and refused frames. With any one of the ihl, doff,
IP length, TCP payload, fragment or clamp checks, or the checksum
helper's IPv4 options, IPv6 header or TCP header length checks,
removed, a case fails; without the checksum helper's ihl check,
ip_fast_csum() oopses on qemu.
Found by the Sashiko review of the previous version of this series and
confirmed by code inspection and the KUnit cases above. Tested with
KUnit on qemu pseries (ppc64le) and on a POWER10 LPAR with iperf3 large
sends over IPv4 and IPv6 at MTU 1500 and 9000, threaded NAPI and busy
polling, with no new drops or InCsumErrors. No kernel selftests cover
ibmveth.
Fixes: 9c7e8bc584f5 ("ibmveth: Add support for Large Receive Offload")
Fixes: 7b5967389f5a ("ibmveth: set correct gso_size and gso_type")
Fixes: 66aa0678efc2 ("ibmveth: Support to enable LSO/CSO for Trunk VEA.")
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Link: https://patch.msgid.link/20261013182926.30417-10-mmc@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
napi_disable() returns once ibmveth_poll() has called
napi_complete_done(), but the poll is not finished: it then re-enables
the interrupt and calls ibmveth_rxq_pending_buffer(), which reads the
RX queue. ibmveth_close() can free that queue in the meantime, and the
late enable can leave the interrupt unmasked after close() masked it.
The request_irq() failure path in ibmveth_open() frees the same memory
after napi_disable() too.
Call synchronize_net() after napi_disable() on both paths. Every
caller of ibmveth_poll() runs it with bottom halves or interrupts
disabled, so this waits for the poll to return.
Found in review of the ibmveth multi-queue RX series and confirmed by
code inspection; also raised by the Sashiko review of the first version
of this series. The race was not reproduced. Tested on a POWER10 LPAR
under an incoming ping flood with 30 rapid link down/up cycles and 30
rapid MTU cycles (1500 <-> 9000); ran cleanly with no warnings or
faults. No kernel selftests cover ibmveth.
Fixes: bea3348eef27 ("[NET]: Make NAPI polling independent of struct net_device objects.")
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Link: https://patch.msgid.link/20261013182926.30417-9-mmc@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
ibmveth_close() frees the TX buffers after netif_tx_stop_all_queues(),
which does not wait for an ibmveth_start_xmit() already running on
another CPU. That transmit can copy into a freed buffer and hand PHYP
a stale DMA address.
MTU, csum/TSO and buffer pool changes call ibmveth_close() directly
while traffic flows. ifdown and the reset work go through dev_close(),
which waits for running transmits only when the qdisc has an enqueue
function, so with noqueue they race the same way.
Use netif_tx_disable(), which waits for running transmits.
Found in review of the ibmveth multi-queue RX series and confirmed by
code inspection; the race was not reproduced. Tested on a POWER10 LPAR
with MTU changes during a ping flood, with no warnings. No kernel
selftests cover ibmveth.
Fixes: d6832ca48d8a ("ibmveth: Copy tx skbs into a premapped buffer")
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Link: https://patch.msgid.link/20261013182926.30417-8-mmc@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When ibmveth_set_channels() cannot allocate a TX buffer for a new
queue, it falls back to the old queue count, and the successful
netif_set_real_num_tx_queues() call then overwrites rc. ethtool -L
reports success while the queue count is unchanged.
Return the allocation error when the fallback succeeds.
Found in review of the ibmveth multi-queue RX series and confirmed by
code inspection. Tested on a POWER10 LPAR with the TX buffer allocation
forced to fail by a test-only module parameter (not part of this patch):
ethtool -L tx 8 now fails with -ENOMEM and the device keeps its four
queues and passes traffic. No kernel selftests cover ibmveth.
Fixes: 10c2aba89cc0 ("ibmveth: Ethtool set queue support")
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Link: https://patch.msgid.link/20261013182926.30417-7-mmc@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
If register_netdev() fails in ibmveth_probe(), for example with -EINTR
when the binding task is killed, probe frees the netdev but leaves the
pool%d kobjects embedded in it registered in sysfs. Reading
/sys/devices/vio/<unit>/pool0/num is then a use-after-free, and the
next probe cannot add pool0.
Put the kobjects before freeing the netdev, as ibmveth_remove() does,
on this path and on the netif_set_real_num_tx_queues() one.
The kobjects also have no release(). With CONFIG_DEBUG_KOBJECT_RELEASE,
kobject_put() defers their cleanup, including removing the sysfs files,
to a work item, so free_netdev() can free them first, here and in
remove(). Add a release() that signals a per-pool completion, and wait
for it in both places before free_netdev(). Without that config,
release() runs from kobject_put() and the wait returns at once.
Found in review of the ibmveth multi-queue RX series and confirmed by
code inspection; the release() part was raised by the Sashiko review of
the first version. Tested on a POWER10 LPAR with register_netdev()
forced to fail with -EINTR by a test-only module parameter (not part of
this patch): no pool%d directories remain and the device binds again. No
kernel selftests cover ibmveth.
Fixes: 860f242eb534 ("[PATCH] ibmveth change buffer pools dynamically")
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Link: https://patch.msgid.link/20261013182926.30417-6-mmc@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
ibmveth_poll() mishandles a bad RX correlator from PHYP in two ways.
If harvest fails or ibmveth_rxq_get_buffer() returns NULL, poll
breaks out without advancing the ring and restarts on the same slot
forever. For an out-of-range correlator it also fires a WARN_ON() and
schedules a reset each pass, but the reset is queued on that CPU and
never runs, and the CPU stalls RCU, so RTNL holders hang too. This
dates from the first commit in Fixes:, which turned the BUG_ON()s
here into WARN_ON() plus a reset.
The range check also accepts a correlator naming an inactive buffer
pool (pools 2 and 3 by default), whose skbuff array is NULL, so the
lookup dereferences NULL in softirq. Pools became inactive with the
second commit in Fixes:.
Validate the correlator in one helper that also rejects a pool with
no skbuff array. On a bad slot, advance the ring, count the drop in
rx_dropped and still schedule the reset, which rebuilds the pools.
The slot has moved, so stay in the budget loop and count it in
frames_processed. Breaking out would complete NAPI, re-arm, and
napi_schedule() with budget left. When the final check finds more
work, queue the NAPI and return instead of going back to
restart_poll: after napi_complete_done() the NAPI is no longer this
call's, and with threaded NAPI and busy polling the kthread can
already be polling it on another CPU. frames_processed is below
budget there, so busy_poll_stop() does not queue it a second time.
Mask the interrupt before queueing, as ibmveth_interrupt() does;
masking after napi_schedule() could land after that kthread has
finished and unmasked, leaving RX masked with nothing scheduled.
Drop a frame whose offset + length does not fit the pool buffer:
copybreak would read past the RX buffer and skb_put() would BUG().
Also drop one shorter than an Ethernet header, which eth_type_trans()
would read past. Read the correlator once per slot and use that value
for the lookup, the bound and the harvest.
The base driver never checked frame bounds, and they have not been
seen in the field, so they have no Fixes: tag of their own. The tags
below are for the correlator hang.
The correlator comes from PHYP, and with the ring advancing a burst
of bad slots would WARN once per slot (and panic with
panic_on_warn), so use a ratelimited netdev_err() instead. Also free
the rx_copybreak skb if harvest fails there.
Add a KUnit case for harvest advancing on errors and extend the
existing cases to an inactive pool. Run against the unfixed driver,
with the calls changed back to its signatures, the first fails and
the others oops.
Found in review of the ibmveth multi-queue RX series (the frame bound by
the Sashiko review of v2 of this series) and confirmed by code
inspection and the KUnit cases above. Hitting either correlator bug
needs PHYP to return a bad correlator, so neither was reproduced on
hardware. Tested with KUnit on qemu pseries (ppc64le), and on a POWER10
LPAR with a ping flood and MTU changes under traffic. No kernel
selftests cover ibmveth.
Fixes: 2c91e2319ed9 ("net: ibmveth: Reset the adapter when unexpected states are detected")
Fixes: 860f242eb534 ("[PATCH] ibmveth change buffer pools dynamically")
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Link: https://patch.msgid.link/20261013182926.30417-5-mmc@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
ibmveth_remove() cancels the reset work before unregister_netdev(),
but NAPI keeps running until unregister closes the device and can
queue the reset again, on an interrupt enable failure, a bad
free_map entry or a bad RX slot. The reset can then run on the
adapter after free_netdev(), or reopen the device during unregister.
Use disable_work_sync(), which waits for a running reset and keeps
the work from being queued again. A bad RX slot also makes poll
spin, which can stall unregister; a later patch in this series
fixes that.
Found in review of the ibmveth multi-queue RX series and confirmed by
code inspection; it was not reproduced. Tested on a POWER10 LPAR with
unbind and bind cycles under traffic. No kernel selftests cover ibmveth.
Fixes: 2c91e2319ed9 ("net: ibmveth: Reset the adapter when unexpected states are detected")
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Link: https://patch.msgid.link/20261013182926.30417-4-mmc@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
ibmveth_change_mtu(), veth_pool_store(), ibmveth_set_csum_offload()
and ibmveth_set_tso() call close() and open() directly. If open() fails,
NAPI is left disabled while IFF_UP stays set, so the next close()
(ifdown, unregister or another reconfiguration) calls napi_disable()
again and waits forever with RTNL held. Networking and shutdown
hang; only a reboot recovers. ethtool -L in that state also wakes
queues that have no TX buffer and dereferences NULL in
ibmveth_start_xmit().
Any open() failure on those paths triggers it, for example an
allocation failure on an MTU change to jumbo frames.
Track a successful open in adapter->opened. close() returns early
when it is clear, and set_channels() checks it instead of IFF_UP.
The open() error-path leaks were fixed separately in net by
commit af0524bf4ce1 ("ibmveth: h_free logical LAN on open-fail after
register") and commit 84bec0bf0352 ("ibmveth: fix TX LTB and filter
unwind on open-fail"), which are now in net-next too, so a failed
open() no longer leaves TX buffers or the logical LAN registration
behind for the early return to skip.
Found in review of the ibmveth multi-queue RX series and confirmed by
code inspection. Tested on a POWER10 LPAR with ibmveth_open() forced to
fail by a test-only module parameter (not part of this patch): 'ip link
set dev eth1 mtu 9000' fails, then 'ip link set dev eth1 down' returns
at once and 'ip link set dev eth1 up' recovers the interface. No kernel
selftests cover ibmveth.
Fixes: 860f242eb534 ("[PATCH] ibmveth change buffer pools dynamically")
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Link: https://patch.msgid.link/20261013182926.30417-3-mmc@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
ibmveth_poll_controller() runs RX replenish outside NAPI and without
a lock, racing NAPI's replenish on another CPU. Both can fill the
same slot, so an skb and its DMA mapping leak and PHYP can write
into an unmapped buffer. netpoll calls it from netconsole and from
netpoll-enabled bonds.
ibmveth_open() also enables NAPI before the RX resources exist.
ibmveth_change_mtu(), veth_pool_store(), ibmveth_set_csum_offload()
and ibmveth_set_tso() call close() and open() directly, so while
open() is still setting up, netpoll and the direct ibmveth_interrupt()
calls can replenish NULL pools and read freed memory.
Remove the callback, as Eric Dumazet did for many drivers after
commit ac3d9dd034e5 ("netpoll: make ndo_poll_controller() optional"),
including ibmvnic in commit 0c3b9d1b37df ("ibmvnic: remove
ndo_poll_controller"). netpoll then polls NAPI itself with budget 0.
napi->poll_owner serializes that with NAPI, but not with a NAPI poll
that was already running when netpoll was set up, so skip RX
replenish at budget 0, which netpoll uses for TX only. TX completes
synchronously, so ibmveth_poll() has nothing else to do for netpoll.
Enable NAPI just before request_irq(), once everything
ibmveth_poll() touches exists.
Found in review of the ibmveth multi-queue RX series and confirmed by
code inspection of poll_one_napi() and the direct close()/open()
callers; neither race was reproduced. Tested on a POWER10 LPAR with
netconsole over ibmveth: a ping flood (678,470 packets, no loss) during
a printk flood, and MTU changes and buffer pool toggles under traffic,
with no warnings. No kernel selftests cover ibmveth.
Fixes: 6b4223748895 ("[PATCH] ibmveth: Add netpoll function")
Fixes: bea3348eef27 ("[NET]: Make NAPI polling independent of struct net_device objects.")
Signed-off-by: Mingming Cao <mmc@linux.ibm.com>
Link: https://patch.msgid.link/20261013182926.30417-2-mmc@linux.ibm.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The "threshold" ethtool statistic reports a single TX DMA threshold for
the whole device. However, since commit d62a107a4f15 ("net: stmmac:
prepare dma interrupt treatment for multiple queues"), a single TX DMA
threshold per device doesn't make sense anymore, as each individual queue
that underflows gets its threshold increased, so the queues are out of
sync.
To make the reporting accurate, remove it and report the threshold of
each TX queue instead, as "qN_tx_threshold".
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
Link: https://patch.msgid.link/20261013114503.57487-3-ovidiu.panait.rb@renesas.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Commit 3a6c12a0c6c3 ("net: stmmac: bump tc when get underflow error from
DMA descriptor") made dwmac4 raise the DMA threshold of the queues that
report TX underflows. This is done by incrementing a module global
parameter 'tc' by 64 on each underflow.
This leads to inconsistent TX DMA thresholds between queues and causes
issues on platforms with multiple stmmac controllers, as 'tc' is shared
by all of them. Once 'tc' goes over 256 the bumping stops, and queues
that did not underflow before are left at the initial threshold.
On the Renesas RZ/V2H platform, which has two dwmac4 controllers, we
observed sporadic iperf3 TCP TX failures:
iperf3 -c <server> -t5 --bind-dev end1
...
[ 5] 0.00-5.00 sec 1.00 MBytes 1.68 Mbits/sec 87 sender
[ 5] 0.00-5.00 sec 768 KBytes 1.26 Mbits/sec receiver
This happens because the TX queues of end1 were underflowing on a large
fraction of the packets, but the DMA TX threshold was never raised from
the default of 64 bytes. The threshold was not raised because end0
had underflowed first and had already pushed 'tc' past 256. Which
controller is affected depends on which one underflows first.
To fix this, make the TX DMA threshold per queue instead of global.
Each time an underflow happens on a queue, its TX DMA threshold gets
incremented, without affecting other queues or controllers. With this
change, 'tc' is only used as the initial value, and the RX threshold
is no longer bumped together with the TX one, as TX underflows have
nothing to do with the RX side.
Fixes: 3a6c12a0c6c3 ("net: stmmac: bump tc when get underflow error from DMA descriptor")
Signed-off-by: Ovidiu Panait <ovidiu.panait.rb@renesas.com>
Reviewed-by: Maxime Chevallier <maxime.chevallier@bootlin.com>
Link: https://patch.msgid.link/20261013114503.57487-2-ovidiu.panait.rb@renesas.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Add test ensuring that network device stats are updated appropriately
when a previously pushed packet fails to send during a subsequent push.
Signed-off-by: James Lee <james@codeconstruct.com.au>
Link: https://patch.msgid.link/20261014-dev-mctp-usb-lib-test-v3-7-ee5bc3ea1db1@codeconstruct.com.au
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Add tests for the transmit path, where MCTP packets are handed to
outgoing USB transfer data. Testing a spanning transfer that is expected
to succeed, the failure paths when sends fail, and large sequential
sends.
Signed-off-by: James Lee <james@codeconstruct.com.au>
Link: https://patch.msgid.link/20261014-dev-mctp-usb-lib-test-v3-6-ee5bc3ea1db1@codeconstruct.com.au
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The if statement testing rx's allocation is unneeded.
Signed-off-by: James Lee <james@codeconstruct.com.au>
Link: https://patch.msgid.link/20261014-dev-mctp-usb-lib-test-v3-5-ee5bc3ea1db1@codeconstruct.com.au
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Add rx tests without spanning, covering cases where a packet is shorter
than allowed without spanning, isn't completed within one submission.
Fully cover rx functions except for memory allocation failures and
trivial functions.
Signed-off-by: James Lee <james@codeconstruct.com.au>
Link: https://patch.msgid.link/20261014-dev-mctp-usb-lib-test-v3-4-ee5bc3ea1db1@codeconstruct.com.au
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Add tests where the length field of the USB transport header is below
the minimum value, and where the DMTF ID is invalid.
Signed-off-by: James Lee <james@codeconstruct.com.au>
Link: https://patch.msgid.link/20261014-dev-mctp-usb-lib-test-v3-3-ee5bc3ea1db1@codeconstruct.com.au
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Add test covering every possible split of a packet's headers between
transfers and remove parameterized tests that are now covered.
Signed-off-by: James Lee <james@codeconstruct.com.au>
Link: https://patch.msgid.link/20261014-dev-mctp-usb-lib-test-v3-2-ee5bc3ea1db1@codeconstruct.com.au
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Add parameterized tests to cover worst case scenarios for packet
splitting and length.
Signed-off-by: James Lee <james@codeconstruct.com.au>
Link: https://patch.msgid.link/20261014-dev-mctp-usb-lib-test-v3-1-ee5bc3ea1db1@codeconstruct.com.au
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When the first port has a non-Ethernet type, team_setup_by_port()
copies port_dev->type to the team device while netdev_priv() of the
team device still holds a struct team. Several L2 stacks identify their
devices by dev->type alone and then access type specific data that the
team driver never sets up:
- CAN protocols rely on the CAN mid-layer private data (ml_priv)
- IEEE 802.15.4 code dereferences ieee802154_ptr and the mlme ops in
ml_priv
- the 6LoWPAN netdev notifier writes through lowpan_dev(dev) on
NETDEV_DOWN and reads it on NETDEV_UP/NETDEV_CHANGE
Enslaving such a device (e.g. vxcan, wpan0, lowpan0) therefore leads to
NULL pointer dereferences or to writes into struct team.
This is the same problem that syzbot reported for the bonding driver
with a vxcan device enslaved to a bond master, which crashed in
can_rx_unregister() and was fixed by commit 8ba68464e478 ("bonding:
refuse to enslave CAN devices"). The team driver has the same gap.
Reject ARPHRD_CAN, ARPHRD_IEEE802154, ARPHRD_IEEE802154_MONITOR and
ARPHRD_6LOWPAN devices in team_port_add().
Link: https://lore.kernel.org/all/b58d98e0-8fe3-4e4e-b8a5-c1c7e647ccca@hartkopp.net/
Link: https://syzkaller.appspot.com/bug?extid=8ed98cbd0161632bce95 # bonding splat
Fixes: 1d76efe1577b ("team: add support for non-ethernet devices")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
Signed-off-by: Oliver Hartkopp <socketcan@hartkopp.net>
Reviewed-by: Hangbin Liu <liuhangbin@kylinos.cn>
Link: https://patch.msgid.link/20261019155407.61487-1-socketcan@hartkopp.net
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
MFGD register queries can exhaust EMAD retries and leave Mellanox
Spectrum switches stuck during boot or shutdown when running OpenWrt
built with GCC 15.
An example failure is shown below with the second line wrapped:
EMAD retries (5/5) (tid=9b544fc100001f35)
EMAD reg access failed (tid=9b544fc100001f35,reg_id=90f0(mfgd),
type=query,status=0(operation performed))
On an SN2100, the failure occurs during driver teardown on reboot.
On an SN3420, it occurs while probing mlxsw_spectrum during boot.
The register query API transmits the supplied payload to the device
before replacing it with the response. Both MFGD callers pass an
uninitialized stack buffer, so the request contains arbitrary register
field values and reserved bits. This makes the request depend on stack
contents and compiler code generation.
Initialize both MFGD query buffers in the firmware fatal event
configuration and reporter test paths. The initialization has been
confirmed to eliminate the boot and shutdown hangs on both switches.
The subsequent read-modify-write still preserves the other fields
returned by the device.
Also initialize the FORE fan fault query and the MOGCR mirroring policer
base query, which pass entirely uninitialized payloads. Restore the MTMP
query initialization in the temperature history reset path, where only
the slot and sensor index fields are set before querying the register.
This follows commit 5b0907407e7f ("mlxsw: core: Zero payload buffers for
couple of registers"), which initialized other query payloads after a
firmware field length bug caused firmware to consume uninitialized data.
Tested with firmware 2020.1148 and with 2016.3900 from linux-firmware.
Fixes: 7d83ee111008 ("mlxsw: core: Introduce fw_fatal health reporter")
Fixes: 2c6a33cd334b ("mlxsw: core: Extend hwmon interface with fan fault attribute")
Fixes: 4039504e6a0c ("mlxsw: spectrum_span: Allow setting policer on a SPAN agent")
Fixes: b890ad418e1f ("mlxsw: core_hwmon: Extend internal structures to support multi hwmon objects")
Cc: stable@vger.kernel.org
Signed-off-by: Til Kaiser <mail@tk154.de>
Reviewed-by: Ido Schimmel <idosch@nvidia.com>
Link: https://patch.msgid.link/20261014173117.26305-1-mail@tk154.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Switch from <module>-objs to <module>-y to eliminate ifdefs and simplify
conditional object list construction. Drop the obsolete trailing
comment.
Signed-off-by: Luiz Angelo Daros de Luca <luizluca@gmail.com>
Reviewed-by: Alvin Šipraga <alvin.sipraga@analog.com>
Link: https://patch.msgid.link/20261019-realtek_mkf-v1-1-3246a77d5782@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
mlx5 couples HW-GRO with tcp-data-split. If a user sets rx-gro-hw on ->
tcp-data-split on -> rx-gro-hw off, the driver will leave HW-GRO on.
mlx5e_ethtool_set_tcp_data_split() calls netdev_update_features() when
tcp-data-split goes from enabled to disabled in case HW-GRO needs to be
released. mlx5e_fix_features() reads the new HDS config from
dev->cfg_pending, so HW-GRO is released before all ring parameters are
validated.
If the subsequent call to mlx5e_ethtool_set_ringparam() fails, the
ethtool core discards the pending change and reports tcp-data-split
enabled. The device is no longer splitting, and subsequent ring changes
are blocked.
Move the call to netdev_update_features() after
mlx5e_ethtool_set_ringparam() exits successfully.
The issue was discovered by manual code inspection during review of
a change that stops mlx5 users from disabling tcp-data-split. It was
reproduced on a ConnectX-7 with ethtool alone.
Steps to reproduce:
ethtool -K eth0 rx-gro-hw on
ethtool -G eth0 tcp-data-split on
ethtool -K eth0 rx-gro-hw off # HW-GRO kept on for tcp-data-split
ethtool -G eth0 tcp-data-split off rx 1 # de-sync driver & kernel state
netlink error: mlx5_core: rx (1) < min (16)
netlink error: Invalid argument
ethtool -G eth0 rx 1024 # valid ring changes are now blocked
netlink error: mlx5_core: TCP-data-split is not supported when GRO HW is disabled
netlink error: Invalid argument
This is not a terminal state, users can recover by setting rx-gro-hw on.
With the fix, the failing ringparam set leaves rx-gro-hw on and later
ring changes succeed.
Fixes: 46bcce5dfd33 ("net/mlx5e: Support ethtool tcp-data-split settings")
Reported-by: Björn Töpel <bjorn@kernel.org>
Closes: https://lore.kernel.org/87se2grw3d.fsf@all.your.base.are.belong.to.us/
Signed-off-by: Dimitri Daskalakis <daskald@meta.com>
Reviewed-by: Björn Töpel <bjorn@kernel.org>
Reviewed-by: Nimrod Oren <noren@nvidia.com>
Acked-by: Tariq Toukan <tariqt@nvidia.com>
Link: https://patch.msgid.link/20261017005401.1582755-1-dimitri.daskalakis1@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
MANA doesn't support byte queue limits, so each TX queue can hold 256
WQEs by default, and up to 16384, of up to 64 KB TSO packets, before the
qdisc layer sees any backpressure. This adds latency to all the flows
sharing a queue, and defeats AQM qdiscs such as fq_codel, which rely on
BQL to keep the driver queues short.
Report the bytes queued and completed to BQL, except for the XDP_TX and
XDP_REDIRECT frames. These share the SQs, but bypass the qdisc and are
sent even when the queue is stopped, so BQL can't hold them back.
Counting them would let sustained XDP traffic keep the queue stopped,
starving the stack, and the TX watchdog would then either fire while the
completions still arrive, or, if the XDP sends refreshed trans_start,
not fire when the completions stop. The XDP SKBs are marked in their
skb->cb, next to the WQE size, so that their completions are skipped as
well.
Reset the BQL state when destroying the queues, once their NAPI is
disabled, since the SKBs still pending after a drain timeout are freed
without being completed.
Lost TX completions can now stop a queue sooner: it also stops when the
bytes in flight exceed the BQL limit, not only when the SQ is full, and
the limit may never grow enough to cover the lost bytes. This includes
the completions skipped by the WARN_ON_ONCE() early returns in
mana_poll_tx_cq(), which already leak SQ space: completing the SKBs
already freed there wouldn't reliably balance BQL, since
mana_gd_poll_cq() has already consumed the CQEs of the rest of the
batch. The TX watchdog then fires, as nothing refreshes the trans_start
of a queue stopped by BQL, and its handler resets the queues along with
their BQL state. The exception, as before this patch, is XDP traffic
keeping the SQ full: each XDP send then stops the queue again,
refreshing its trans_start.
Tested on two Standard_D16ds_v6 Azure VMs in a proximity placement
group, where the throughput is capped at 10.6 Gbps, with the DUT
alternately booting net-next (c1c1f0a31712) with and without this patch
applied.
The throughput stays at the cap with 1, 8 and 64 TCP flows, and the idle
TCP_RR latency doesn't change (p50 of 47 us with vs. without the patch).
With 16 senders of 64-byte UDP packets, the DUT sends 3.7 to 4.0 Mpps
without the patch and 3.7 to 4.1 Mpps with it. With bulk TCP flows
sharing the queues, the netperf TCP_RR latency drops (medians of 6
runs):
16 queues, 64 flows: p50 4649 -> 2182 us, p99 5621 -> 2677 us
1 queue, 8 flows: p50 2554 -> 418 us, p99 2723 -> 624 us
The standing queue moves from the SQs to fq_codel, which now drops
packets: about 2500 qdisc drops and 88K TCP retransmissions per 48 s run
with 64 flows, against at most 29 retransmissions before. The
retransmissions are about 0.2% of the segments sent. The TCP_RR flows
lose no packets, and the packets of the bulk flows are dropped before
they reach the wire, so the goodput changes by less than 0.01%.
Signed-off-by: Hamza Mahfooz <hamzamahfooz@linux.microsoft.com>
Reviewed-by: Haiyang Zhang <haiyangz@microsoft.com>
Link: https://patch.msgid.link/20261013214937.145312-1-hamzamahfooz@linux.microsoft.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
marvell_config_intr() rewrites the whole interrupt enable register, so
the config_intr call from phy_init_hw() on resume silently cleared
WOL_EIE on the 88E1318S and 88E1510: m88e1318_get_wol() still reported
WAKE_MAGIC, but a matched magic packet was no longer routed to INTn and
the board did not wake.
Bit 7 of that register is WOL_EIE only on these two PHYs; on others,
such as the 88E1111, it enables a FIFO error interrupt. So give the
88E1318S and 88E1510 their own config_intr and handle_interrupt, and
leave the shared callbacks unchanged.
m88e1318_set_wol() now records whether WoL is enabled, and both it and
the new config_intr derive WOL_EIE from that state under the MDIO bus
lock. WOL_EIE stays set across config_intr while WoL is enabled, except
when the PHY has an interrupt line and interrupts are being disabled,
as in phy_free_interrupt(): INTn is then the PHY IRQ, and nothing would
read IEVENT after free_irq() to release it. The bit is set again once
interrupts are re-enabled. phy_probe() still clears a WOL_EIE left by a
previous boot, since WoL starts out disabled. The new handler also
claims WoL events, so they do not go unacknowledged when the PHY IRQ is
in use.
Helios4 (Armada 388, 88E1512), 7.2.9. With this patch WOL_EIE stays set
over 3 link down/up cycles, "wol d" clears it, and a magic packet on a
running board is a handled interrupt. Without it, WOL_EIE is lost on the
first down/up, survives "wol d", and the magic packet is an unhandled
interrupt.
Fixes: 3871c3876f80 ("mv643xx_eth with 88E1318S: support Wake on LAN")
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Tested-by: Igor Velkov <iav@iav.lv>
Link: https://patch.msgid.link/20261014194818.11640-2-rosenp@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The RX walk in axienet_dma_bd_release() stops at the first descriptor
without an skb, assuming that such a slot and all after it were never
filled. That holds on the axienet_dma_bd_init() error path, but not once
the ring has been running: when axienet_rx_poll() fails to allocate or
map a replacement buffer it leaves the slot it emptied with a NULL skb,
in the middle of a populated ring, until a later poll refills it.
Closing the interface before then leaks the skb and the streaming DMA
mapping of every descriptor after that slot, which can be most of the
ring, and is most likely under memory pressure.
The walk also frees each skb before unmapping it. With swiotlb, the
DMA_FROM_DEVICE unmap copies the bounce buffer back into the skb data
that was just freed, and on non-coherent platforms it invalidates cache
lines that may already belong to a new owner.
Skip empty slots instead of stopping at the first one. An emptied slot
still has a non-zero cntrl but its buffer is already unmapped, so the
skip has to cover the unmap too, which it does by coming before it.
Then unmap before freeing, as axienet_rx_poll() does.
This was reported by the Sashiko AI review bot.
Tested on the AXI Ethernet MAC of an ADVA TimeCard X2 (PCIe card, with
the built-in AXI DMA): traffic passes after each of ten down/up cycles,
each running axienet_dma_bd_release(), so the unmap-then-free order is
exercised on every buffer. A failed RX refill was not reproduced, so
the skip over an empty slot was not exercised.
Fixes: f26667a373f3 ("net: axienet: Fix DMA descriptor cleanup path")
Fixes: 7a7d340ba4d9 ("net: axienet: fix RX ring refill allocation failure handling")
Signed-off-by: Sagi Maimon <maimon.sagi@gmail.com>
Link: https://patch.msgid.link/20261015084743.1798326-1-maimon.sagi@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
hinic_xmit_frame() pads short frames with skb_pad(), then
sets skb->len directly, leaving skb->tail stale.
Use skb_put_padto(), which updates both and is a litle less confusing.
Also remove one pointless netdev_err() call.
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Link: https://patch.msgid.link/20261016093518.2756935-1-edumazet@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Replace the open-coded skb_pad() + __skb_put() sequence
in fm10k_xmit_frame() with skb_put_padto().
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Link: https://patch.msgid.link/20261016094312.2759956-1-edumazet@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Replace skb_pad() and the manual skb->len and tail pointer updates
in iavf_xmit_frame() with skb_put_padto().
No functional change.
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Link: https://patch.msgid.link/20261016094925.2762785-1-edumazet@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
In bcmasp_netfilt_get_init(), when looking up an existing filter (!init)
for a specified location, if the filter at loc was not claimed, the
previous loop continued searching higher indices and could return an
arbitrary unrelated filter belonging to the port. This caused flow get or
delete operations on an empty rule location to return or delete an
unintended filter.
Fix this by checking only the requested location on lookup and rejecting
RX_CLS_LOC_ANY when !init. In addition, harden wake filter allocation
and release by ensuring wake filter pair searches stay on even boundaries
where both entries are free, checking that loc + 1 is free for positioned
wake filters, and validating parity and bounds on release.
Fixes: c5d511c49587 ("net: bcmasp: Add support for wake on net filters")
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20261013204439.3494046-7-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When hardware checksum offload is enabled for an skb,
bcmasp_csum_offload() prepends a struct bcmasp_pkt_offload header
(20 bytes) to the skb via skb_push(). This increases skb->len and
skb_headlen(skb) by sizeof(struct bcmasp_pkt_offload).
Because the hardware descriptor processor strips this offload header
before transmitting the packet on the wire, the wire packet length is
smaller by sizeof(struct bcmasp_pkt_offload). The padding calculation
must account for this extra header; otherwise, short frames are sent on
the wire smaller than the minimum Ethernet frame size (ETH_ZLEN +
ETH_FCS_LEN).
Furthermore, small frames with payload in page fragments (such as small
TCP segments with CHECKSUM_PARTIAL) have nr_frags > 0 and were skipping
padding entirely because the padding check was restricted to
nr_frags == 0.
Perform the padding check on skb->len before the descriptor loop using
skb_put_padto(), adding sizeof(struct bcmasp_pkt_offload) to min_size
when checksum offload is enabled, and re-read nr_frags after padding in
case the skb was linearized.
Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller")
Reviewed-by: Justin Chen <justin.chen@broadcom.com>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20261013204439.3494046-6-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
In bcmasp_interface_create(), intf->phy_dn is obtained via
of_parse_phandle() or assigned ndev_dn. of_parse_phandle() returns a
node reference with its refcount incremented, but of_node_put() was
never called on intf->phy_dn in bcmasp_interface_destroy() or the
error unwind path in bcmasp_interface_create().
Acquire a reference on ndev_dn for the fixed-link case as well so that
intf->phy_dn consistently holds a reference, and release it with
of_node_put() on teardown and error.
Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller")
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Justin Chen <justin.chen@broadcom.com>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20261013204439.3494046-5-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
In bcmasp_rx_poll(), the driver removes a 2-byte alignment pad and
optionally strips the ETH_FCS_LEN CRC from received packets before
passing them to eth_type_trans().
If the hardware reports a descriptor size smaller than the sum of the
2-byte pad, the Ethernet header (ETH_HLEN), and optional CRC
(ETH_FCS_LEN), subtracting the pad and CRC lengths underflows u32 len.
This adds ~4 GiB to rx_bytes statistics, while the undersized frame
reaches eth_type_trans(), which reads past skb->len into stale buffer
data.
Check that desc->size is at least (2 + ETH_HLEN + (crc_fwd ? ETH_FCS_LEN
: 0)) before proceeding to process the descriptor.
Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller")
Reviewed-by: Justin Chen <justin.chen@broadcom.com>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20261013204439.3494046-4-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When mapping an skb with fragments for transmission in bcmasp_xmit(),
if mapping fails on fragment i > 0, the error handler calls
bcmasp_clean_txcb() for previous iterations j < i. However,
bcmasp_clean_txcb() only zeroes the control block fields without
unmapping the DMA buffers, leaking the DMA mappings allocated for the
head and earlier fragments.
Call dma_unmap_single() before calling bcmasp_clean_txcb() in the error
cleanup loop.
Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller")
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Justin Chen <justin.chen@broadcom.com>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20261013204439.3494046-3-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When EDPKT stats were removed in commit e9f31435ee7d ("net: bcmasp: Add
support for asp-v3.0"), the corresponding entries were removed from
bcmasp_gstrings_stats in bcmasp_ethtool.c, but the edpkt_* fields were
left at the beginning of struct bcmasp_mib_counters in bcmasp.h.
Because bcmasp_update_mib_counters() and bcmasp_get_ethtool_stats() index
into struct bcmasp_mib_counters sequentially based on the order of
bcmasp_gstrings_stats, this 16-byte offset caused ethtool to read from
shifted offsets: the first four software counters read unused fields and
reported 0, the next four software counters reported the values of
alloc_rx_skb_failed, tx_dma_failed, mc_filters_full_cnt, and
uc_filters_full_cnt, while filters_combine_cnt, promisc_filters_cnt,
tx_realloc_offload_failed, and tx_timeout_cnt were never reported.
Remove the obsolete edpkt_* fields from struct bcmasp_mib_counters so
that it aligns with bcmasp_gstrings_stats.
Fixes: e9f31435ee7d ("net: bcmasp: Add support for asp-v3.0")
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Justin Chen <justin.chen@broadcom.com>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20261013204439.3494046-2-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The timeout worker calls the driver's close and open methods without
RTNL or a running-state check. A timeout queued before an interface is
closed can call close again after ndo_stop has disabled NAPI. The second
napi_disable() can then wait indefinitely. The worker can also race with
an ongoing close.
Take RTNL and skip recovery when the interface is no longer running.
Use the netdev core close and open helpers so a failed reopen leaves the
interface down, rather than leaving it marked running with NAPI disabled.
After unregister_netdev() has stopped the watchdog, drain the worker
before releasing its MDIO and descriptor resources. This wait is outside
RTNL, allowing an already queued worker to take RTNL, observe the down
state, and finish.
This issue was identified as part of our ongoing research while
reviewing kernel code.
Fixes: 57c5bc9ad7d7 ("net: hisilicon: add hix5hd2 mac driver")
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
Reviewed-by: Jijie Shao <shaojijie@huawei.com>
Link: https://patch.msgid.link/179203116093.2621539.3718842986255459438.G202@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
RTL8211F and RTL8211F-VD both use rtl8211f_config_rgmii_delay() during
initialization and share rtl821x_resume(). The RTL8211F-VD loses its
RGMII TX and RX delay enable bits across a software power-down, but the
shared resume path only clears BMCR_PDOWN and waits for the receive clock
to stabilize.
On a BeagleV Ahead configured for rgmii-id, carrier and the negotiated
speed return after an interface close/open, but the receive path does not
work. At 100 Mbit/s no frames were received. At 1000 Mbit/s all 11
observed frames had CRC errors.
Reapply the RGMII delay configuration after the existing resume delay.
Tested on a BeagleV Ahead with an RTL8211F-VD at 100 and 1000 Mbit/s,
including close/open without a link partner.
Fixes: bb726b753f75 ("net: phy: realtek: add support for RTL8211F(D)(I)-VD-CG")
Reviewed-by: Andrew Lunn <andrew@lunn.ch>
Signed-off-by: Karl Mehltretter <kmehltretter@gmail.com>
Link: https://patch.msgid.link/20261015045001.83986-1-kmehltretter@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
qede_flow_spec_to_rule() declares a netlink_ext_ack on the stack
without initializing it. qede_parse_flow_attr() and
qede_flow_spec_validate() only set extack._msg on error paths via
NL_SET_ERR_MSG_MOD(), so on a successful parse the err_out block
reads an uninitialized pointer. If that garbage happens to be
non-NULL, DP_NOTICE("%s", extack._msg) dereferences it inside
vsnprintf, causing a page fault and kernel panic.
This manifests when configuring ethtool ntuple flow rules (ethtool -U
flow-type tcp4/udp4) on kernels built with CONFIG_INIT_STACK_NONE=y.
With CONFIG_INIT_STACK_ALL_ZERO=y the bug is masked because the
compiler zero-initializes extack, making _msg NULL.
Reproduced on RHEL 9.8 (kernel-5.14.0-687.47.1.el9_8) and mainline
(v7.3-rc6) with the following reproducer:
IFACE=ens2f0
ip link set $IFACE up
ethtool -K $IFACE ntuple on
for i in $(seq 1 100); do
ethtool -U $IFACE flow-type udp4 src-ip 192.168.2.$i action -1
done
The loop panics reliably after 10-15 iterations with:
BUG: unable to handle page fault
RIP: string+0x49/0xd0
Call Trace:
vsnprintf
qede_flow_spec_to_rule+0x2b6/0x4b0 [qede]
qede_add_cls_rule
ethtool_set_rxnfc
Zero-initialize extack so _msg is NULL unless explicitly set by an
error path.
Fixes: eb705d734525 ("net: qede: use faked extack in qede_flow_spec_to_rule()")
Signed-off-by: Anirudh Virdi <avirdi@redhat.com>
Reviewed-by: Asbjørn Sloth Tønnesen <ast@fiberby.net>
Reviewed-by: Daniel Machon <daniel.machon@microchip.com>
Link: https://patch.msgid.link/20261013105451.201211-1-avirdi@redhat.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
hash_del_rcu() only unlinks the node; a reader that already holds the
bucket iterator still uses the node. The immediate kfree() has no
grace period, so a concurrent transmit on a socket whose skb priority
carries the deleted classid reads node->classid / node->qid from freed
memory (use-after-free in the softirq transmit path).
Fix this by calling synchronize_net() before the node is freed, as
mlx5e does.
Found during a static-analysis-assisted audit: candidate sites were
surfaced by a Coccinelle rule, and the reader/writer paths were
confirmed by LLM-assisted (GLM-5.3) code inspection. The issue was
not observed at runtime; it is identified by code inspection only.
Fixes: 5e6808b4c68d ("octeontx2-pf: Add support for HTB offload")
Signed-off-by: Binbin Deng <18983559317@163.com>
Reviewed-by: Ratheesh Kannoth <rkannoth@marvell.com>
Link: https://patch.msgid.link/20261014064204.291688-1-18983559317@163.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
axienet_start_xmit_dmaengine() bumps lp->tx_ring_head before
skb_to_sgvec(), dma_map_sg() and device_prep_slave_sg(), and none of
their error paths undo it. The skb is freed and nothing is submitted,
but the slot stays claimed. axienet_dma_tx_cb() walks tx_ring_tail
once per completion, so the next completion lands on the abandoned slot
and dereferences whatever skb pointer is left there: NULL if the slot
was never used or was already completed, or a stale skb that gets
consumed a second time. Every completion after that is accounted
against the wrong slot.
dma_map_sg() and device_prep_slave_sg() can fail at runtime, and so can
skb_to_sgvec() on a malformed skb. Only advance tx_ring_head once
those have succeeded. The completion callback can't run before
dmaengine_submit(), so claiming the slot just before that is fine.
dmaengine_submit() itself can also fail on xilinx_dma after a channel
error, and its return isn't checked either. That's a separate problem
and isn't changed here.
This was found by the Sashiko AI review of the skbuff BUG_ON() series.
Cc: stable+noautosel@kernel.org # LLM report + LLM fix, untested
Fixes: 6a91b846af85 ("net: axienet: Introduce dmaengine support")
Link: https://lore.kernel.org/all/20261007-b4-skbuff-bug-on-v2-0-b9a5f732895b@toxicpanda.com/
Signed-off-by: Josef Bacik <josef@toxicpanda.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20261013-b4-axienet-dmaengine-ring-head-v1-1-7add24e74477@toxicpanda.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
In mtk_wed_wo_queue_refill(), the entry->buf is DMA mapped with length
q->buf_size which saved in entry->len. However, in mtk_wed_wo_dequeue(),
the same buffer is DMA unmapped with length buf_len which is set to
SKB_WITH_OVERHEAD(q->buf_size).
Change the size to q->buf_size.
This was discovered with static analysis and was only compile-tested.
Cc: stable+noautosel@kernel.org # LLM report + LLM fix, untested
Fixes: 799684448e3e ("net: ethernet: mtk_wed: introduce wed wo support")
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>
Link: https://patch.msgid.link/20261015140110.41988-2-fourier.thomas@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
veth_newlink() creates the peer netdev from tbp, which points at the
nested VETH_INFO_PEER attributes when they are supplied (peer_tb) and
only aliases the primary device's tb otherwise. However the queue
reduction is gated on the primary device's attributes:
err = veth_init_queues(peer, tb);
veth_init_queues() checks !tb[IFLA_NUM_{TX,RX}_QUEUES], so for the peer
it inspects the wrong attribute set whenever nested peer attributes are
used:
- If the user requests IFLA_NUM_TX/RX_QUEUES only for the peer, tb
carries no such attribute and veth_init_queues() still shrinks the
peer's real_num_{tx,rx}_queues back to 1, silently dropping the
user's explicit multi-queue request for the peer.
- If the user requests the counts only for the primary device, the
peer keeps nr_possible_cpus real queues instead of the legacy
default of 1.
Pass tbp so the peer's queue count is decided from the peer's own
attributes. When no nested peer attributes are given tbp aliases tb, so
the common case is unchanged.
Cc: stable+noautosel@kernel.org # never worked, high regression risk
Fixes: 9d3684c24a52 ("veth: create by default nr_possible_cpus queues")
Signed-off-by: Li RongQing <lirongqing@baidu.com>
Link: https://patch.msgid.link/20261014013620.2497-1-lirongqing@baidu.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
In xgene_enet_refill_bufpool(), the skb->data is dma-mapped with length
len which is set to XGENE_ENET_STD_MTU which equals 1536. However, in
xgene_enet_delete_bufpool(), the same buffer is unmapped with length
XGENE_ENET_MAX_MTU which is 9600. This may erroneously unmap other
DMA buffers or cause other issues.
Change unmap length to XGENE_ENET_STD_MTU.
Cc: stable+noautosel@kernel.org # LLM report + LLM fix, untested
Fixes: a9380b0f7be8 ("drivers: net: xgene: Add support for Jumbo frame")
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
Reviewed-by: Daniel Machon <daniel.machon@microchip.com>
Link: https://patch.msgid.link/20261015124547.29996-2-fourier.thomas@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Qualcomm/Sierra SDX55/SDX65 modems withhold unsolicited AT result codes
(URCs such as +CREG, and the +DMI OMA-DM/LwM2M session indications) on
an AT port until the host asserts DTR. mhi_wwan_ctrl exposed the AT
(DUN) ports but had no way to signal DTR, so URCs never reached
userspace.
Carry the host serial-control lines to the modem over the dedicated
IP_CTRL MHI channel, which this module now also binds. IP_CTRL uses a
separate mhi_driver with its own callbacks so the AT/QMI/MBIM data path
is untouched; the control-channel device for each MHI controller is
tracked in a small registry so an AT port drives the IP_CTRL channel of
its own modem (multiple modems are supported).
The IP_CTRL channel has to be declared in the controller's channel config
for this driver to bind. For the Sierra EM919x/EM929x this is done by
commit 83c29a55b89e ("bus: mhi: host: pci_generic: Add IP_CTRL channel
for Sierra EM919x/EM929x") in the MHI tree. On controllers that do not
declare IP_CTRL the driver does not bind and ->dtr_rts is a no-op.
The wwan core, with "net: wwan: core: propagate modem control signals to
port drivers", raises DTR/RTS on first open and drops them on last close
(if HUPCL is set) for any AT port whose driver implements ->dtr_rts, so
no open/close handling is needed here. The new ->dtr_rts op lets
userspace assert or de-assert them via TIOCMSET/TIOCMBIC/TIOCMBIS.
Received device->host serial state messages are silently discarded; a
single recycled sink buffer keeps the IP_CTRL DL ring live so the
modem's transmit path does not stall.
->dtr_rts is void, following the tty_port_operations model it is
modelled on. It records the requested state and schedules a per-port
work item that sends the latest state to the modem, so a burst of
changes is coalesced into one message and cannot fill the IP_CTRL UL
ring. If a send fails because the ring is full or memory is short, the
work retries shortly with whatever state is latest by then, so the modem
ends up with the final state that was requested. The DL sink buffer is
requeued from a work item, which retries a transient -ENOMEM so the DL
ring does not stay empty, and which remove() disables before tearing
down the channel.
Signed-off-by: Peter Hunt <peter.hunt@opengear.com>
Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Link: https://patch.msgid.link/20261013183500.261229-3-peter.hunt@opengear.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The WWAN character device emulates the TTY modem-control ioctls
(TIOCMGET/TIOCMSET/TIOCMBIC/TIOCMBIS) for AT and QCDM ports, but the
result is only stored in port->at_data.mdmbits and never reaches the port
driver. A driver therefore cannot act on the host raising or dropping
DTR/RTS, even though some modems depend on it (e.g. they withhold
unsolicited AT result codes until the host asserts DTR).
Add an optional ->dtr_rts(port, mdmbits) operation to struct wwan_port_ops.
Drivers that implement it receive the TIOCM output bitmask so they can
assert or de-assert DTR and RTS independently. As in tty_tiocmset(), the
wwan core keeps only the output bits (DTR, RTS, OUT1, OUT2, LOOP) of the
TIOCMSET/TIOCMBIS/TIOCMBIC value in port->at_data.mdmbits, and calls
->dtr_rts when DTR or RTS actually changes, gated on WWAN_PORT_AT to match
the open/close raise/drop behaviour.
Also raise DTR/RTS in wwan_port_op_start on first open of an AT port
when the driver implements ->dtr_rts, and drop them in wwan_port_op_stop
on last close if HUPCL is set in the port's termios. This follows TTY
open/close semantics, although the termios baud rate is not used, and
means individual drivers do not need to implement this themselves. As on
a TTY, HUPCL defaults to on for AT ports, and userspace can clear it
with TCSETS to keep DTR asserted across close, for example so that
closing the port does not end a call on a modem set to AT&D1/AT&D2.
TCSETS and TCSETS2 now only commit the new termios when the copy from
userspace succeeds, so a faulting call cannot silently clear HUPCL.
->dtr_rts must be called under ops_lock, which serialises port ops and
protects against port removal (removal clears port->ops under it). So
that updates to the TIOCM state and the ->dtr_rts calls are serialised
together, the termios/TIOCM state now also lives under ops_lock and the
separate data_lock is removed. The AT/QCDM ioctl handler only holds
ops_lock while it reads or updates that state, never across a
user-memory copy, and takes it interruptibly since a driver's blocking
write holds it while it waits. Ioctls that do not touch that state, such
as TCFLSH, TIOCEXCL and the FIONREAD fallback, do not take it.
wwan_remove_port() also drops DTR/RTS before ->stop() when a port is
removed while still open, regardless of HUPCL since the device is going
away. The raise and drop paths go through two small helpers, which do
the AT port and ->dtr_rts checks and pass the driver the resulting
bitmask.
Signed-off-by: Peter Hunt <peter.hunt@opengear.com>
Reviewed-by: Loic Poulain <loic.poulain@oss.qualcomm.com>
Link: https://patch.msgid.link/20261013183500.261229-2-peter.hunt@opengear.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|