| Age | Commit message (Collapse) | Author |
|---|
|
When mapping a fragment fails, bgmac_dma_tx_add() unwinds with the
wrong slots. It passes the failed fragment's slot->dma_addr, which
holds the mapping error, to dma_unmap_single() for the head. The
fragment loop indexes from ring->end instead of ring->end + 1, so it
unmaps the head a second time as a page and leaves the last mapped
fragment mapped. None of the affected slots get their dma_addr cleared
either, which includes the head slot when mapping the head fails.
bgmac_dma_tx_ring_free() unmaps every slot with a non-zero dma_addr,
so tearing down the ring unmaps these addresses again. It also frees
the skbs of pending TX slots but leaves slot->skb and slot->dma_addr
set. The slots survive a down/up cycle, and bgmac_dma_tx_add() only
sets slot->skb on the last slot of a packet. If a slot with a stale
pointer is later used for the head or a fragment of a scatter/gather
packet, bgmac_dma_tx_free() frees the old skb a second time when that
slot completes.
bgmac_dma_tx_ring_free() also picks dma_unmap_single() or
dma_unmap_page() by looking at slot->skb, which is set on the last slot
of a packet, while the head was mapped with dma_map_single() and the
fragments with skb_frag_dma_map().
In the error path, unmap the head and each mapped fragment from their
own slots, and clear dma_addr on every slot the failed packet touched.
In bgmac_dma_tx_ring_free(), choose the unmap function by the SOF flag
of the descriptor like bgmac_dma_tx_free() does, unmap every slot
before freeing any skb, since a packet can wrap around the end of the
ring with its skb on the last slot, and clear both fields.
This was found through code inspection by an LLM while hitting other
bugs on an Asus RT-N16. It did not hit these ones specifically.
Fixes: 9cde94506eac ("bgmac: implement scatter/gather support")
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20261028164839.28605-1-rosenp@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
With a multi-buffer XDP program attached, a packet only has agg
buffers if it filled its head buffer, which in page mode holds most
of a page. The program can still move the start of the packet forward
with bpf_xdp_adjust_head() and leave at most rx_copybreak bytes in the
head. bnxt_rx_pkt() then copies the head into a new skb, puts the head
page back on the ring, and calls bnxt_xdp_build_skb() on the copy.
bnxt_xdp_build_skb() only sets nr_frags and the skb's length
accounting, because it expects an skb built around the head page,
sharing its shinfo with the xdp_buff. The copy ends up with nr_frags
set but no frag descriptors, and the stack later reads and releases
whatever its frags array happens to contain. The real frags leak.
Copy the frag descriptors to the new skb in bnxt_copy_xdp(), so that
bnxt_xdp_build_skb() finds the frags where it expects them. The frags
now belong to the skb, and the head page goes back on the ring as
before.
Pointed out in LLM review of the first patch of the series.
Confirmed by reading the code, the unfixed kernel was not run with
the reproducer below.
Tested on a BCM57504 at MTU 9000 with rx-copybreak raised to 1024
and a frags-capable XDP program moving the start of multi-buffer
frames 3000B forward with bpf_xdp_adjust_head(), receiving 8000B
ICMPv6 echo requests (a head and two agg buffers each). All 60k
frames took the copybreak path with frags attached, the sampled skbs
reached GRO with len=4998 data_len=4492 nr_frags=2, and were freed by
the stack with the page pools' inflight count staying flat, no KASAN
or DEBUG_PAGEALLOC splats.
Fixes: 1dc4c557bfed ("bnxt: adding bnxt_xdp_build_skb to build skb from multibuffer xdp_buff")
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20261027234940.1267225-6-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When xdp_do_redirect() fails bnxt_rx_xdp() recycles the head page but
not the frags of a multi-buffer frame. Their agg ring slots have
already been refilled, so the pages leak, and the page pool can't be
destroyed until they are returned, which will never happen. Redirect
to an AF_XDP socket in copy mode fails this way with -ENOMEM whenever
the socket's fill ring runs out of buffers.
A socket bound without XDP_USE_SG also fails with -ENOSPC for every
frame that doesn't fit in a single chunk.
All the other drop paths in this function free the frags.
Pointed out in LLM review of the first patch of the series.
Tested on a BCM57504 at MTU 9000 by redirecting 60k 8000B ICMPv6
echo requests (a head and two agg buffers each) to lo, which can't
transmit XDP frames. The page pools' inflight count stayed flat
rather than growing by two pages per frame.
Fixes: a7559bc8c17c ("bnxt: support transmit and free of aggregation buffers")
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20261027234940.1267225-5-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
If a replacement agg buffer can't be allocated for a packet received
with XDP attached, bnxt_rx_pkt() drops the packet without returning
its head buffer to the ring. The head buffer leaks, and since rx_prod
still advances, the device gets a BD with a stale DMA address which
points at a buffer the driver no longer owns. When that BD completes
the driver finds no buffer in the slot.
The agg buffers consumed before the failing one have already been
replaced on the agg ring, they are referenced only by the frags of
the xdp_buff, which nobody frees. bnxt_xdp_buff_frags_free() would
skip them since the frags flag is set only on success, so set it
upfront.
Pointed out in LLM review of the first patch of the series.
Tested on a BCM57504 at MTU 9000 with a frags-capable XDP_PASS
program, receiving 8000B ICMPv6 echo requests (a head and two agg
buffers each) while page_pool_alloc_netmems() called from
bnxt_rx_agg_netmems_xdp() failed half of the time (bpftrace
override()). 44.7k packets were dropped, the page pools' inflight
count stayed flat and there were no KASAN or DEBUG_PAGEALLOC splats.
Fixes: 4c6c123c9af9 ("bnxt: adding bnxt_rx_agg_pages_xdp for aggregated xdp")
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20261027234940.1267225-4-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Since rx-copybreak became configurable, bnxt_set_ring_params() sizes
the head buffers to fit at least rx_copybreak bytes, and the copybreak
path in bnxt_copy_data() syncs rx_copybreak bytes of the head buffer.
bnxt_set_tunable() however only updates bp->rx_copybreak, and nothing
on the open path recalculates the ring parameters. After raising
rx-copybreak the head buffers keep their old size until some other
setting changes, and bnxt_copy_data() syncs past their end, into the
next buffer in the page or past the end of the page:
DMA-API: bnxt_en 0000:01:00.0: device driver tries to sync DMA memory it has not allocated [device address=0x00000002aa210c40] [size=1024 bytes]
WARNING: kernel/dma/debug.c:1162 at check_sync+0x577/0x1eb0, CPU#22: swapper/22/0
Call Trace:
<IRQ>
debug_dma_sync_single_for_cpu+0x66/0x90
bnxt_copy_data+0x12f/0x290
bnxt_rx_pkt+0x24ea/0x50f0
__bnxt_poll_work+0x42c/0xc00
bnxt_poll_p5+0x579/0x17a0
__napi_poll+0x8b/0x2c0
net_rx_action+0x58f/0xbb0
Here the head buffers were still sized for the 640B HDS threshold,
four 1kB buffers per page, while rx-copybreak was raised to 1024.
Without coherent DMA or with swiotlb the oversized sync can corrupt
a neighboring buffer owned by the device or by an skb.
The device must be down for rx-copybreak to change, so recalculate the
ring parameters right away, like bnxt_set_ringparam() does.
Hit while testing patch 1, tested on BCM57504 - boot; set rx-copybreak
to 1024 with the link down (CONFIG_DMA_API_DEBUG); ifup.
Fixes: 152f4da05aee ("bnxt_en: add support for rx-copybreak ethtool command")
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20261027234940.1267225-3-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
bnxt_rx_pkt() tries to return the aggregation buffers of a packet
to the ring when the copybreak allocation fails, but the same logic
is missing on the rx_skb_func() failure path. The agg BDs consumed
by the device are then never posted again. The agg ring is only
refilled one-for-one as completions consume its buffers, so each
such failure permanently shrinks it. Under sustained memory pressure
the ring drains completely, and the queue stops receiving anything
that does not fit into the head buffer, even once memory is available
again. Only re-creating the rings (e.g. ifdown / ifup) recovers it.
Commit 296d5b541639 ("bnxt_en: Fix aggregation buffer leak under OOM
condition.") fixed the copybreak path only. Recently
commit bd3beb5c8691 ("eth: bnxt: disable rx-copybreak by default")
made the non-copybreak path the default, re-surfacing the issue.
With XDP, the agg buffers are attached to the xdp_buff as frags by
this point. Their agg ring slots have already been refilled.
We need to keep the original head page (and therefore shinfo)
around until the xdp_buff cleanup.
Note the odd split of ownership here: rx_skb_func() only deals with
the head buffer, the agg buffers stay with the caller in whatever
state they are in, still on the ring without XDP, or frags of the
xdp_buff with XDP. This complicates rx_skb_func() cleaning up all
the buffers (head and frags), which would be more intuitive.
Discovered in production, severe packet loss would persist after
a transient OOM event. Tested on a BCM57504 by failing the head
buffer allocations in bnxt_rx_skb() (bpftrace override() of
should_failslab() and should_fail_alloc_page()) while receiving
1400B ICMPv6 echo requests. 341k packets were dropped on that queue,
enough to drain its 8188 entry agg ring 40 times over, and the queue
kept receiving large frames afterwards with no loss and no new HW drops.
With a frags-capable XDP_PASS program at MTU 9000, receiving 8000B
ICMPv6 echo requests while napi_build_skb() called from
bnxt_rx_multi_page_skb() failed, 9.2k packets were dropped,
the page pools' inflight count stayed flat and
there were no KASAN or DEBUG_PAGEALLOC splats.
Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.")
Fixes: 4c6c123c9af9 ("bnxt: adding bnxt_rx_agg_pages_xdp for aggregated xdp")
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20261027234940.1267225-2-kuba@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Commit 9fc3bc764334 ("tg3: power down device only on SYSTEM_POWER_OFF")
bypassed `tg3_power_down()` on system restarts to prevent reboot hangs and
NMIs on servers where NICs were initialized via SNP (such as Dell PowerEdge
R650 and R750).
However, skipping the power-down flow globally causes the ACPI _PTS method
to trigger fatal PCIe AER events and hangs on other models. Commit
e0efe83ed325 ("tg3: Disable tg3 PCIe AER on system reboot") attempted to
mitigate this by adding an AER-disable DMI quirk table. Maintaining a
growing list of models for a secondary symptom misses the root cause,
leaving systems like the PowerEdge T440 (sharing the BCM5720 NIC) exposed.
Fix this by inverting the quirk logic:
1. Restrict SNP power-down avoidance strictly to
`tg3_restart_snp_quirk_table` for the specific systems requiring it
(Dell PowerEdge R650 and R750).
2. Restore standard power-down behavior for all other systems, eliminating
the separate AER-disable quirk table.
For systems affected by the original SNP restart issue, their shutdown flow
remains completely untouched.
Link: https://lore.kernel.org/netdev/20251221175050.14089-1-noam.d.eliyahu@gmail.com/T/#u
Fixes: 9fc3bc764334 ("tg3: power down device only on SYSTEM_POWER_OFF")
Fixes: e0efe83ed325 ("tg3: Disable tg3 PCIe AER on system reboot")
Cc: stable@vger.kernel.org
Signed-off-by: Noam D. Eliyahu <noam.d.eliyahu@gmail.com>
Reviewed-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Link: https://patch.msgid.link/20261026220955.473247-1-noam.d.eliyahu@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
bnge_adjust_pow_two() adds one block before rounding up to a power of
two only when the quotient is 0 or 1. A larger quotient that is
already a power of two is returned unchanged, so 129-192 RX rings
get a 128-entry RSS indirection table and 257-320 rings a 256-entry
one. The rings above the table size never receive default RSS traffic.
bnxt_calc_nr_ring_pages() always adds one, which is correct.
Make bnge_adjust_pow_two() always add one block before rounding up.
bnge_adjust_pow_two() also sizes the RX, RX aggregation, TX and
completion ring pages. For the default 511-entry rings their page
counts do not change. The completion ring is sized from more than
the default depth, so its page count changes where the old code
dropped a partial page. With 8K ring pages, TPA off and the default
rings, its budget is 1533 entries and the page count goes from 2 to 4.
With 4K ring pages and TPA on, when the firmware reports a
max_aggs_supported of 260 to 387, it goes from 16 to 32.
In these cases the old completion ring held fewer entries than
its budget.
Fixes: 627c67f038d2 ("bng_en: Add resource management support")
Signed-off-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20261026092757.9168-3-vikas.gupta@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The hardware supports at most BNGE_RSS_TABLE_MAX_TBL (8) RSS contexts.
They are stored in vnic->fw_rss_cos_lb_ctx[8] and back a 512-entry
(BNGE_MAX_RSS_TABLE_ENTRIES) indirection table and DMA buffer, so at
most 512 RX rings can be served by RSS.
bnge_cal_nr_rss_ctxs() has no upper bound, so with more RX rings it
returns more than 8 contexts: bnge_setup_vnic() writes past
fw_rss_cos_lb_ctx[], bnge_set_dflt_rss_indir_tbl() and
bnge_fill_hw_rss_tbl() write past the indirection table and its DMA
buffer, and the pad computation in bnge_set_dflt_rss_indir_tbl()
underflows into a huge memset().
Cap the number of contexts at BNGE_RSS_TABLE_MAX_TBL and the number
of RX rings at BNGE_MAX_RSS_TABLE_ENTRIES, so the sizes derived from
the ring count always fit the arrays.
Fixes: 627c67f038d2 ("bng_en: Add resource management support")
Signed-off-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20261026092757.9168-2-vikas.gupta@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
bcmgenet_set_coalesce() writes tx_max_coalesced_frames straight into
DMA_MBUF_DONE_THRESH and keeps no copy of it, while
bcmgenet_init_tx_ring() programs a fixed 1. An ifdown and ifup or a resume
then puts the Tx coalescing back to one frame per interrupt.
Rx keeps its value per ring and reapplies it on init. Do the same for Tx.
Fixes: 2f9130709d2c ("net: bcmgenet: Implement TX coalescing control knobs")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20261022-nb-genet-fini-dma-net-v3-2-6e9fc89c0156@tipi-net.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
A resume that fails in bcmgenet_init_dma() returns with the interface
still IFF_UP and the datapath gone. The next ifdown runs the teardown
a second time and napi_disable() waits for a NAPI_STATE_SCHED bit that
nothing will clear:
task:ip state:D
usleep_range_state+0xec/0x158
napi_disable_locked+0x254/0x318
napi_disable+0x3c/0x68
bcmgenet_netif_stop+0x1c8/0x318
bcmgenet_close+0x24/0xd0
__dev_close_many+0x18c/0x310
RTNL is held across the wait, so everything else blocks behind it.
The same resume gates the clock and detaches the device. Close releases
the clock a second time and ifup fails with -ENODEV. The hardware filters
stay armed, so the next WoL suspend wakes on any frame. That suspend sets
CMD_RX_EN for magic packet detection, so the WoL resume reuses rings that
are gone.
A failed resume should leave a state the ordinary paths can handle. Clear
rx_cbs and tx_cbs as they are freed so they answer whether a datapath
exists, and leave the clock alone so close and suspend release it once.
The WoL resume rebuilds instead, so stop the PHY first, phy_start() only
takes PHY_READY or PHY_HALTED.
Flagged in a sashiko review and reproduced on a CM4 by forcing the resume
allocation to fail.
Fixes: b6e978e50444 ("net: bcmgenet: add suspend/resume callbacks")
Fixes: 254f3239dd07 ("net: bcmgenet: revise suspend/resume") # for the WoL parts
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20261022-nb-genet-fini-dma-net-v3-1-6e9fc89c0156@tipi-net.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Merge in late fixes in preparation for the net-next PR.
Conflicts:
net/netfilter/nf_conntrack_netlink.c
cd37ab22a74e0 ("netfilter: nf_conntrack_netlink: fix inverted IPv6 tuple filter")
1ae2d094b0200 ("netfilter: ctnetlink: fix inverted IPv6 address match in dump filter")
Adjacent changes:
drivers/net/ethernet/marvell/octeontx2/af/rvu_nix.c
8830e65ed46de ("net: use assign_bit() where applicable")
07bd391eb957 ("octeontx2-af: Fix CQ enable state tracking after AQ writes")
net/core/sock.c
31c5ea8a3296 ("ipv6: Remove IPV6_ADDRFORM.")
31b564f70d88 ("net: use the full socket in sk_mc_loop()")
net/mptcp/protocol.h
0cb6910130a3 ("mptcp: remove thmac from subflow ctx")
7538fa33bb2f ("mptcp: fix subflow bitfield misuse")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
The Datto SIRIS 5 1U Server is a Dell OEMR XL R640 (Dell's OEM-ready
variant of the PowerEdge R640) sold under Datto's identity. Its DMI
strings report sys_vendor "datto" and product_name "SIRIS 5 1U Server",
so it does not match the existing "PowerEdge R640" entry in
tg3_restart_aer_quirk_table.
As on the PowerEdge R640, a system restart triggers a fatal PCIe AER
on the BCM5720 during ACPI _PTS for S5, which the iDRAC logs as
PCI1318 and which hangs the reboot. Power off is unaffected.
Clearing the same PCI_EXP_DEVCTL error-reporting bits on both BCM5720
functions from userspace immediately before restart allows the system
to reboot normally, confirming the quirk applies to this platform.
Fixes: e0efe83ed325 ("tg3: Disable tg3 PCIe AER on system reboot")
Signed-off-by: Travis Howard <thoward@lafayetteinstrument.com>
Link: https://patch.msgid.link/20261019-tg3-datto-aer-quirk-v1-1-f0eccfb2ab5d@lafayetteinstrument.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When the RX ring is full the RX FIFO overflows. With RX overflow
continue enabled, the DMA engine then writes one frame with a bogus
length (always 0x45c2 on a BCM4716), which the driver drops with
"Found oversized packet at slot %d, DMA issue!".
Under sustained overload this happens on every NAPI poll. On an Asus
RT-N16 a 950 Mbit/s UDP stream to the router printed about 1450 such
lines in 20 seconds. The messages are printed from NAPI context, so
with a serial console the router spends most of its time printing.
The frames are already accounted in rx_length_errors and rx_errors.
Rate limit both RX error messages. With this change the same flood
printed 60 lines while rx_length_errors counted 14297 dropped frames.
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Daniel Machon <daniel.machon@microchip.com>
Link: https://patch.msgid.link/20261018231629.39734-1-rosenp@gmail.com
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>
|
|
bnxt_xdp_set() stores the new program and drops the reference on the old
one before reopening the NIC. If bnxt_open_nic() then fails, ndo_bpf()
returns an error with the new program still in bp->xdp_prog. The caller
treats the error as "nothing was installed" and drops its own reference,
so bp->xdp_prog is left pointing at a freed program and the next XDP
update dereferences it:
BUG: unable to handle page fault for address: ff78ecc80ddd1038
RIP: 0010:__bpf_prog_put+0x5/0x80
Call Trace:
bnxt_xdp_set+0xad/0x1b0 [bnxt_en]
dev_xdp_propagate+0x36/0xa0
bond_xdp_set+0xeb/0x2d0 [bonding]
dev_xdp_install+0x1b1/0x350
bpf_xdp_link_update+0xc5/0x1b0
link_update+0x104/0x1e0
__sys_bpf+0x662/0xcf0
Seen on a 6.12 based kernel after bnxt_alloc_mem() failed an order-4
allocation on a fragmented host:
bnxt_en 0000:a0:00.1 ens4f1np1: nic open fail (rc: fffffff4)
bond1: (slave ens4f1np1): Error -12 calling ndo_bpf
Bonding is not required to hit this; a plain XDP attach on a bnxt
interface takes the same path.
Restore the previous program when an attach or a replace fails, and
recompute the page-mode and XDP TX ring configuration from it. The
configuration matters because bnxt_init_one_rx_ring() only assigns
rxr->xdp_prog in page mode, and __bnxt_set_rx_skb_mode() derives
dev->max_mtu from the installed program. Release the old program only
once the change has been committed.
A detach is not undone. dev_xdp_detach_link() clears the core's record
of the program even when the driver returns an error, so putting the
program back would leave the driver running a program the core has
already dropped. If the reopen fails during a detach, bnxt_xdp_set()
therefore returns 0 with the device closed; bnxt_open_nic() has already
logged the failure.
Found in a crash dump; the change was not tested on hardware.
Fixes: c6d30e8391b8 ("bnxt_en: Add basic XDP support.")
Cc: stable@vger.kernel.org
Signed-off-by: Vaibhav Nagare <vnagare@redhat.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20261012072802.2153706-1-vnagare@redhat.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Firmware can leave BCM5718 vendor registers byte-swapped while standard
PCI fields retain normal byte order. Detect this from the host-control
revision marker, byte-swap bit and swapped product ID, without matching
a platform model or PCI subsystem. Prefer the normal revision marker
so already-normal controllers do not enter the normalization path.
Hardware swapping can accommodate big-endian register accesses, but tg3
already initializes misc_host_ctrl with WORD_SWAP set and BYTE_SWAP clear
on both little- and big-endian builds. Its normal MMIO path uses
readl()/writel(); on SPARC64 these use little-endian accesses. Restore
that existing register mode rather than choosing a mode based on host
endianness. The swab32() on the initial write compensates for the device's
inherited state; subsequent accesses use the normal driver path.
DMA byte ordering is controlled separately through GRC_MODE. Leave those
settings, including the __BIG_ENDIAN handling for non-frame data,
unchanged. Check both register readbacks and keep this in probe so PCI
access or validation failures can abort initialization.
Limit detection to BCM5718, whose register layout and inherited state
were checked on M3000 hardware. Other controller families retain their
existing initialization path.
Signed-off-by: Magnus Lindholm <linmag7@gmail.com>
Reviewed-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Link: https://patch.msgid.link/20261011194228.981141-1-linmag7@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
bgmac_open() calls bgmac_chip_init(), which unmasks the interrupts and
enables the MAC receiver, before request_irq() and napi_enable().
bgmac_enet_resume() also enables NAPI only after bgmac_chip_init().
When frames arrive in that window, the first RX interrupt masks all
interrupts in bgmac_interrupt() and calls napi_schedule() on a NAPI
instance that is still disabled, so nothing is scheduled. Nothing
unmasks the interrupts again, the RX ring fills up and the interface
stays deaf until it is brought down and up without traffic.
On an Asus RT-N16 (BCM4716) a single "ip link set eth0 down/up" while
a host sends UDP to the router left INT_MASK at 0, INT_STATUS with RX
pending and the RX DMA idle on a full ring, with the RX packet and
interrupt counters no longer moving. With this change, 10 of 10 such
cycles recovered on their own.
Request the IRQ and enable NAPI before the chip is initialized.
Fixes: dd4544f05469 ("bgmac: driver for GBit MAC core on BCMA bus")
Fixes: f3537b3494c0 ("net: ethernet: bgmac: driver power manangement")
Cc: stable@vger.kernel.org
Signed-off-by: Rosen Penev <rosenp@gmail.com>
Reviewed-by: Daniel Machon <daniel.machon@microchip.com>
Link: https://patch.msgid.link/20261010070011.60403-1-rosenp@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Commit 80bfab79b835 ("net: adopt skb_network_offset() and similar helpers")
replaced the difference between the inner transport header and the outer
network header with skb_inner_transport_offset().
The helper is relative to skb->data, so the new calculation incorrectly
includes the outer Ethernet header in fw_ip_hdr_to_payload_w.
On a BCM57810 transmitting TCP over VXLAN with an outer MTU of 1500,
hardware segmentation emits packets declaring an outer IPv4 total length
of 1514 while the actual IP packet is only 1500 bytes long. The receiver
drops these malformed packets, severely degrading TCP connectivity.
Subtract skb_network_offset() to restore the original calculation of the
length from the outer network header to the inner TCP payload.
Reproduced with plain kernel VXLAN and one TCP flow on kernel
7.0.0-31-generic: 50 of 200 captured packets had an invalid outer IP length
with the original driver, versus none with this fix. Disabling TSO/GSO on
the test VXLAN device also eliminated the error with the original driver.
Fixes: 80bfab79b835 ("net: adopt skb_network_offset() and similar helpers")
Signed-off-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20261009-bnx2x-tso-fix-v1-1-1ebea422422d@gmelikov.ru
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Cross-merge networking fixes after downstream PR (net-7.3-rc8).
No conflicts. Adjacent changes:
drivers/net/ethernet/microchip/lan865x/lan865x.c
2a3f6d2a9ac3 ("microchip: lan865x: move setting of netops to fix crash")
15358c2c3dca ("net: ethernet: oa_tc6: return ERR_PTR from oa_tc6_init()")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Each SW USO segment is hdr_len + seg_payload bytes. With IPv4, hdr_len
is 42 bytes, so a segment carrying less than 10 bytes of UDP payload is
sent shorter than BNXT_MIN_PKT_SIZE. This happens with a short last
segment (e.g. gso_size 1400 and 1405 bytes of payload) or with a
gso_size below 10 bytes.
Fix the short last segment in bnxt_sw_udp_gso_xmit(): zero the pad
bytes after the header in the segment's inline header slot and add a
pad BD pointing at them after the payload BDs. Only the last segment
can need padding, and its pad BD fits within the existing bound in
bds_needed and BNXT_SW_USO_MAX_DESCS, since payload BDs are at most
num_segs + nr_frags.
If gso_size itself is too small, every segment would need a pad BD,
exceeding the descriptor bound. This case is unlikely, so
bnxt_features_check() clears GSO features for these skbs and the stack
segments them instead; bnxt_start_xmit() then pads each segment.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Joe Damato <joe@dama.to>
Link: https://patch.msgid.link/20261009215426.2377946-3-joe@dama.to
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Move the code that fills a payload BD and its software ring entry in
bnxt_sw_udp_gso_xmit() into a helper, bnxt_sw_gso_data_bd().
No functional change. A following patch uses the helper to add a pad BD
to short SW USO segments.
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Signed-off-by: Joe Damato <joe@dama.to>
Link: https://patch.msgid.link/20261009215426.2377946-2-joe@dama.to
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
bcmgenet stores first_cb, last_cb and bytes_sent in the skb control
block and reads them back on completion to free the skb and credit BQL.
ether_setup() leaves IFF_TX_SKB_SHARING set, so the stack can resubmit
a shared skb that is still in the ring. The next transmit overwrites
last_cb, so the earlier slots complete without crediting their bytes
and BQL stalls the queue until the watchdog resets it.
Unshare the skb before writing the control block.
Reproduce with pktgen in clone_skb mode, which hands the driver the same
skb while earlier copies are still in the ring:
modprobe pktgen
pg=/proc/net/pktgen
echo "add_device eth0" > $pg/kpktgend_0
echo "dst_mac AA:BB:CC:DD:EE:FF" > $pg/eth0
echo "dst 192.0.2.1" > $pg/eth0
echo "clone_skb 8" > $pg/eth0
echo "count 0" > $pg/eth0
echo start > $pg/pgctrl &
sleep 5
echo stop > $pg/pgctrl
wait
cat /sys/class/net/eth0/queues/tx-0/byte_queue_limits/inflight
inflight stays nonzero on the now idle queue. clone_skb 0 leaves it at
zero.
Fixes: f48bed16a756 ("net: bcmgenet: Free skb after last Tx frag")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20261009-nb-genet-shared-skb-net-v1-1-c6d0a841977d@tipi-net.de
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
The default VNIC programs the device's MAC as L2 filter 0 in
bnge_init_chip() via bnge_hwrm_set_vnic_filter(bn, 0, 0, dev_addr).
bnge_cfg_rx_mode() then walks dev->uc and programs each unicast
address as a further L2 filter, without excluding dev_addr.
Skip dev_addr when building uc_list, since it is already installed as
filter 0. As dev_addr now takes neither a uc_list slot nor an
l2_filters[] slot, count only the non-dev_addr entries for the
uc_list overflow check too, otherwise an input such as
{dev_addr, A, B, C} would needlessly fall back to promiscuous mode
even though A, B and C fit in exact-match filters.
bnge_uc_list_updated() applies the same skip and count so the
change detection stays consistent and does not loop.
This issue was caught by Sashiko (nipa).
Tested on Thor Ultra device.
Fixes: 9afad4a17174 ("bng_en: Configure default VNIC")
Signed-off-by: Vikas Gupta <vikas.gupta@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Link: https://patch.msgid.link/20261006141928.1432927-1-vikas.gupta@broadcom.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
|
|
Cross-merge networking fixes after downstream PR (net-7.3-rc7).
Conflicts:
include/net/strparser.h
0984ebc631792 ("strparser: make sure __strp_recv isn't running before tearing down the parser")
02fd0a2111374 ("net: strparser: removed the aborted bit and aborts counter")
https://lore.kernel.org/asYfmLsfy-SCxXjr@sirena.co.uk
Adjacent changes:
Documentation/networking/strparser.rst
5f193cdabf25 ("docs: networking: strparser: remove general mode")
0984ebc63179 ("strparser: make sure __strp_recv isn't running before tearing down the parser")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
bnge_start_xmit() samples skb_shinfo(skb)->nr_frags into last_frag,
and stores it in tx_buf->nr_frags, before calling eth_skb_pad().
If a nonlinear skb is shorter than ETH_ZLEN, __skb_pad() calls
skb_linearize(), which pulls the frags into the linear part,
releases the frag pages and sets nr_frags to zero.
bnge_start_xmit() then uses the stale last_frag to fill TX_BD_CNT()
and to walk skb_shinfo(skb)->frags[], DMA-mapping pages the skb
no longer holds a reference on, and sending their content twice.
Fix this by calling eth_skb_pad() before nr_frags is sampled.
Found by Sashiko while reviewing the similar bnxt_en fix.
Fixes: bd5ad9c052c8 ("bng_en: Add TX support")
Link: https://lore.kernel.org/netdev/179134695789.434549.53526916334276884@kernel.org/
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
Reviewed-by: Vikas Gupta <vikas.gupta@broadcom.com>
Link: https://patch.msgid.link/20261007075649.468033-1-edumazet@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Stefan Fleischmann reported Intel IOMMU DMA Read faults on BCM57412
NetXtreme-E NICs when transmitting packets on VLAN/macvlan interfaces:
DMAR: [DMA Read NO_PASID] Request device [18:00.0] fault addr 0xfc499000
[fault reason 0x06] PTE Read access is not set
bnxt_en 0000:18:00.0 eno1np0: Abandoning msg {0xb4 0x41a} len: 0 due to firmware status: 0x2000001
...
NETDEV WATCHDOG: eno1np0 (bnxt_en): transmit queue 0 timed out
The fault address (0xfc499000) is on an exact 4KB page boundary,
pointing to a DMA read buffer overrun.
In bnxt_start_xmit(), packets smaller than BNXT_MIN_PKT_SIZE (52 bytes),
such as 42-byte untagged ARP frames, are padded:
if (length < BNXT_MIN_PKT_SIZE) {
pad = BNXT_MIN_PKT_SIZE - length;
if (skb_pad(skb, pad))
goto tx_kick_pending;
length = BNXT_MIN_PKT_SIZE;
}
mapping = dma_map_single(&pdev->dev, skb->data, len, DMA_TO_DEVICE);
...
dma_unmap_len_set(tx_buf, len, len);
However, 'len' was initialized earlier to skb_headlen(skb) (e.g. 42 bytes)
and is left unadjusted after padding. Consequently, dma_map_single() and
dma_unmap_len_set() map and track only 42 bytes.
Later, the hardware TX buffer descriptor is programmed with the padded length:
txbd->tx_bd_len_flags_type =
cpu_to_le32(((len + pad) << TX_BD_LEN_SHIFT) | flags |
TX_BD_FLAGS_PACKET_END);
The NIC DMA engine is thus instructed to read 52 bytes from a region where
only 42 bytes were DMA-mapped. If skb->data ends near the boundary of a 4KB
page (within 'pad' bytes of the next page), the hardware DMA read overruns
into the unmapped adjacent page, triggering an IOMMU fault.
This issue was exposed after commit 447cbe95ebb9 ("vlan: fix skb_under_panic
and races when toggling HW VLAN offload") because reserving extra VLAN
headroom rounded LL_RESERVED_SPACE from 48 up to 64 bytes, shifting skb->data
offsets and potentially causing small frames to land right against page
boundaries.
Fix this by using skb_put_padto(skb, BNXT_MIN_PKT_SIZE) in
bnxt_start_xmit(), before skb_shinfo(skb)->nr_frags is sampled,
because padding might linearize the skb.
This ensures skb->len and skb_headlen(skb) reflect the padded size so
that dma_map_single() maps the full buffer and the descriptor length is
consistent. This also removes the temporary 'pad' variable and masking logic.
The padding is done after the SW USO branch, otherwise
bnxt_sw_udp_gso_xmit() would account the padding as UDP payload
and send it in extra segments.
Note that SW USO segments are still not padded: with IPv4, a segment
carrying less than 10 bytes of UDP payload (small gso_size, or a short
last segment) is sent as a frame shorter than BNXT_MIN_PKT_SIZE.
This is a separate issue, left for a followup patch.
Fixes: c0c050c58d84 ("bnxt_en: New Broadcom ethernet driver.")
Cc: stable@vger.kernel.org
Reported-by: Stefan Fleischmann <sfle@kth.se>
Closes: https://lore.kernel.org/netdev/20261004122616.56714cbd@nargothrond/
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Michael Chan <michael.chan@broadcom.com>
Link: https://patch.msgid.link/20261007051747.455273-1-edumazet@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Currently the driver zeroes the BARs only when fatal PCIe errors
are reported so that pci_restore_state() restores it. However
firmware handles both fatal and non-fatal errors the same way when
it sees the slot reset resulting from the PCI_ERS_RESULT_NEED_RESET
return code from the driver. This means that we must re-write the
BARs post recovery even during non-fatal errors. Otherwise we will
see that every MMIO access returns all-ones and the firmware appears
dead.
Zero-out the BARs during PCIe error recovery regardless of type of
PCIe error, and make the wait after the hot reset unconditional.
Disable memory decode and bus mastering before rewriting the BARs so
the device doesn't decode a half-updated address, bailing out if
config space is still inaccessible. Defer pci_enable_device() until
after the BAR rewrite and restore, so the device isn't re-enabled
while its BARs are still being rewritten, then re-enable the device
and re-assert bus mastering. Guard the same Command register cleanup
on the re-enable failure path against an inaccessible device.
Skip re-enabling the device in bnxt_io_slot_reset() if it is already
enabled, so enable_cnt does not go unbalanced. A concurrent
bnxt_fw_reset_task() can also be re-enabling the same device in its
ENABLE_DEV state, so guard that call the same way.
Fixes: f75d9a0aa967 ("bnxt_en: Re-write PCI BARs after PCI fatal error.")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Scott Branden <scott.branden@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20261005204246.3822563-4-michael.chan@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Fix and strengthen the FLR sequence when initializing in the kdump
kernel. If the NIC is behind a PCIe switch in synthetic (smart)
mode, the switch may need to see that the BARs have been initialized
before it will pass mem read/write TLPs to the NIC.
On a Dell system with a PEX89144 PCIe switch, echo c > /proc/sysrq-trigger
will trigger fatal AER without this patch:
bnxt_en 0000:67:00.0: enabling device (0000 -> 0002)
[Hardware Error]: Hardware error from APEI Generic Hardware Error Source: 5
[Hardware Error]: event severity: recoverable
[Hardware Error]: Error 0, type: fatal
[Hardware Error]: section_type: PCIe error
[Hardware Error]: port_type: 5, upstream switch port
...
Add a new bnxt_kdump_reset() to do the expanded FLR sequence in the
kdump kernel. We now disable bus master and memory, save the PCI
state, do the FLR, clear the BARs, and restore the PCI state. The
BARs have to be cleared to ensure that they get re-initialized and
visible to the PCIe switch.
Since it is the kdump kernel, we make every effort to continue in
the best possible way even if pci_save_state() or pcie_flr() returns
error. After FLR, we poll for an additional 5 seconds before
aborting in case the device is not properly returning CRS. This is
similar to the 5-second wait in bnxt_io_slot_reset().
Fixes: 8743db4a9acf ("bnxt_en: Issue PCIe FLR in kdump kernel to cleanup pending DMAs.")
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20261005204246.3822563-3-michael.chan@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
In bnxt_io_slot_reset(), we clear the 6 BAR registers. Add a helper
function to do that. The helper will be used again in the next patch.
Reviewed-by: Kalesh AP <kalesh-anakkur.purayil@broadcom.com>
Reviewed-by: Somnath Kotur <somnath.kotur@broadcom.com>
Reviewed-by: Joe Damato <joe@dama.to>
Signed-off-by: Pavan Chebbi <pavan.chebbi@broadcom.com>
Signed-off-by: Michael Chan <michael.chan@broadcom.com>
Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>
Link: https://patch.msgid.link/20261005204246.3822563-2-michael.chan@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When the revised suspend/resume sequence was introduced this led to an edge
case where the TX is left disabled in the following sequence.
1. phy link is down, so UMAC is held in reset and then network interface
is WoL enabled
2. Enter suspend, bcmgenet_wol_power_down_cfg() enables UMAC_RX since MAC
is in SW_RESET
4. Enter resume, UMAC_RX is left enabled. Since we only enable UMAC_TX
and UMAC_RX in SW_RESET. The UMAC_TX is never enabled again on link up.
The issue was hit in production and I verified the sequence keeps
the TX disabled in my standalone test. I tested on an internal
development board with bcm77122, but should be reproducible
on any genet HW that supports power management.
Fixes: 254f3239dd07 ("net: bcmgenet: revise suspend/resume")
Signed-off-by: Justin Chen <justin.chen@broadcom.com>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Link: https://patch.msgid.link/20260930230634.3132729-1-justin.chen@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
netif_stop_queue() in bcmasp_xmit() raced with netif_wake_queue() in
bcmasp_tx_poll(): a reclaim landing between the ring-full check and
the stop call left the queue stopped despite free descriptors,
hanging TX until timeout.
Switch to netif_txq_maybe_stop()/netif_txq_completed_wake() (the
lockless TX queue API, which also brings in BQL), replace
tx_spb_ring_full() with a live bcmasp_tx_avail() count.
This issue was discovered using LLM and is only theoretical.
Tested on a BCM74110 that no regressions were encountered.
Cc: stable+noautosel@kernel.org # LLM report + LLM fix, not seen in real life
Fixes: 490cb412007d ("net: bcmasp: Add support for ASP2.0 Ethernet controller")
Signed-off-by: Justin Chen <justin.chen@broadcom.com>
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20260929203400.1588868-1-justin.chen@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
If bnge_netdev_alloc() fails after storing the net_device pointer in
bd->netdev, it calls free_netdev() but leaves bd->netdev as a dangling
pointer.
Explicitly set bd->netdev to NULL on the error path to prevent potential
use-after-free or invalid pointer dereferences.
Cc: stable+noautosel@kernel.org # LLM report + LLM fix, not seen in real life
Fixes: 13a68c1ed754 ("bng_en: Add a network device")
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
Reviewed-by: Ramakrishna Koduri <ramakrishna.koduri@broadcom.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Link: https://patch.msgid.link/20261005121704.227866-5-bhargava.marreddy@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
IRQ affinity hints are advisory and not fatal to driver operations.
On uniprocessor builds (CONFIG_SMP=n), irq_set_affinity_hint() always
returns -EINVAL, which currently prevents ndo_open() from succeeding.
Log a warning and continue instead of failing open on affinity error.
Cc: stable+noautosel@kernel.org # LLM report + LLM fix, not seen in real life
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
Reviewed-by: Ramakrishna Koduri <ramakrishna.koduri@broadcom.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Link: https://patch.msgid.link/20261005121704.227866-4-bhargava.marreddy@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
If exactly aux_msix vectors are granted, probe continues with zero NQ
vectors. This leads to zero-sized allocations and out-of-bounds array
accesses during ring setup.
Require at least 'min' vectors beyond the aux reservation to ensure the
driver always has an NQ vector to work with.
Cc: stable+noautosel@kernel.org # fix to unlikely driver error path
Fixes: 18a975389fcc ("bng_en: Add irq allocation support")
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
Reviewed-by: Ramakrishna Koduri <ramakrishna.koduri@broadcom.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Link: https://patch.msgid.link/20261005121704.227866-3-bhargava.marreddy@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
bnge_alloc_core() previously jumped to a single error label that called
bnge_free_core() to unwind. However, bnge_free_core() assumes a fully
initialized state and dereferences rx/tx ring arrays unconditionally.
If ring allocations fail early, this causes a near-NULL dereference.
Introduce a standard goto ladder to unwind only the resources that were
successfully allocated, preventing NULL dereferences.
Cc: stable+noautosel@kernel.org # fix to unlikely driver error path
Fixes: 0259379037ca ("bng_en: Add initial support for RX and TX rings")
Signed-off-by: Bhargava Marreddy <bhargava.marreddy@broadcom.com>
Reviewed-by: Dharmender Garg <dharmender.garg@broadcom.com>
Reviewed-by: Rajashekar Hudumula <rajashekar.hudumula@broadcom.com>
Reviewed-by: Ramakrishna Koduri <ramakrishna.koduri@broadcom.com>
Reviewed-by: Przemek Kitszel <przemyslaw.kitszel@intel.com>
Link: https://patch.msgid.link/20261005121704.227866-2-bhargava.marreddy@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Cross-merge networking fixes after downstream PR (net-7.3-rc6).
Conflicts:
net/mac80211/tx.c
net/mac80211/ieee80211_i.h
8effab902fa34 ("wifi: mac80211: prevent AP VLAN tx from other interfaces")
78b843974fb83 ("wifi: mac80211: report multicast transmission from lookup_ra_sta()")
https://lore.kernel.org/arJmcuhpJ69wAvYK@sirena.co.uk
drivers/net/ethernet/realtek/r8169_main.c
3cdeaef1754ab ("r8169: disable EEE on RTL8168h/8111h")
8a3c76523e449 ("r8169: add support for phylink")
https://lore.kernel.org/ar5vAxEqS8tO8aVT@sirena.org.uk
Adjacent changes:
rust/kernel/net/netlink.rs
5e5916923759 ("rust: net: netlink: Migrate to zerocopy's IntoBytes")
0923198be4ae ("rust: net: netlink: validate attribute length before casting to `c_int`")
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
After reclaiming, the Tx poll asks to be polled again. That extra poll
finds nothing to do and does not reduce the interrupt rate.
Complete the NAPI and unmask the ring interrupt right after the reclaim.
On a CM4 this raises 60 byte pktgen Tx from about 187k to 224k pps and
saves about 5% of one core at TCP line rate.
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Eric Dumazet <edumazet@google.com>
Link: https://patch.msgid.link/20260928121454.901306-1-nb@tipi-net.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
The Rx ring priv and index are only set in bcmgenet_init_rx_ring(),
which runs at open. Setting the Rx coalescing parameters on an interface
that was never opened, e.g. "ethtool -C eth0 rx-usecs 50", dereferences
a NULL ring->priv. The oops happens with RTNL held, so networking
stays stuck until a reboot:
Unable to handle kernel paging request at virtual address 0000000000002788
pc : bcmgenet_set_rx_coalesce.isra.0+0x10/0x88
lr : bcmgenet_set_coalesce+0xe8/0x180
Call trace:
bcmgenet_set_rx_coalesce.isra.0+0x10/0x88 (P)
__ethnl_set_coalesce.isra.0+0x490/0x570
ethnl_set_coalesce+0x48/0xc0
ethnl_default_set_doit+0xf4/0x230
genl_family_rcv_msg_doit+0xe8/0x160
genl_rcv_msg+0x220/0x2a0
netlink_rcv_skb+0x68/0x140
genl_rcv+0x40/0x60
netlink_unicast+0x338/0x3c0
netlink_sendmsg+0x19c/0x3f8
__sock_sendmsg+0x64/0xc0
__sys_sendto+0x124/0x198
__arm64_sys_sendto+0x30/0x48
Set priv and index at probe. Writing the registers while the interface
is down is harmless because open resets the MAC and
bcmgenet_init_rx_coalesce() reapplies the stored values.
It was observed on a Raspberry Pi CM4, but should apply to all other
genet devices.
Fixes: 9f4ca05827a2 ("net: bcmgenet: Add support for adaptive RX coalescing")
Signed-off-by: Nicolai Buchwitz <nb@tipi-net.de>
Reviewed-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20260928105642.856210-1-nb@tipi-net.de
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
In bcm_sysport_stop(), the error messages printed when tdma_enable_set()
and rdma_enable_set() time out were inverted: the failure of
tdma_enable_set() logged 'timeout disabling RDMA' and the failure of
rdma_enable_set() logged 'timeout disabling TDMA'.
Swap the error messages so they correctly describe the failing engine.
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20260922232440.598918-9-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
In bcm_sysport_suspend_to_wol(), the loop enabling programmed RXCHK
filters in RXCHK_CONTROL used an auxiliary counter 'i' instead of the
actual set filter index 'index'.
When non-contiguous filters were configured (for example, if filter 0
was deleted and filter 1 remained), the code would enable bit
(RXCHK_BRCM_TAG_MATCH_SHIFT + 0) corresponding to filter 0 rather than
filter 1, causing Wake-on-LAN filter matching to fail.
Fix this by using the filter 'index' to set the appropriate match bit
in RXCHK_CONTROL.
Cc: stable+noautosel@kernel.org # never worked
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20260922232440.598918-5-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
If allocation of priv->rx_cbs fails during bcm_sysport_init_rx_ring(),
error unwinding in bcm_sysport_open() calls bcm_sysport_fini_rx_ring().
Without checking if priv->rx_cbs is non-NULL, bcm_sysport_fini_rx_ring()
dereferences priv->rx_cbs, resulting in a NULL pointer dereference.
Add a check for !priv->rx_cbs at the beginning of
bcm_sysport_fini_rx_ring(), matching bcm_sysport_fini_tx_ring().
Cc: stable+noautosel@kernel.org # unlikely driver error path
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20260922232440.598918-4-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
In UniMAC hardware, there is a 0xC byte gap between the RX MIB counters
and the TX MIB counters, and a second 0xC byte gap between the TX MIB
counters and the RX RUNT MIB counters.
In bcm_sysport_update_mib_counters(), 'offset' was only set to
UMAC_MIB_STAT_OFFSET (0xC) for all non-RX counters, omitting the second
0xC gap for BCM_SYSPORT_STAT_RUNT counters. As a result, all 4 RUNT MIB
counters were read from unmapped gap register space.
Fix this by setting offset to 2 * UMAC_MIB_STAT_OFFSET (0x18) when
reading BCM_SYSPORT_STAT_RUNT counters.
Fixes: 80105befdb4b ("net: systemport: add Broadcom SYSTEMPORT Ethernet MAC driver")
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20260922232440.598918-6-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
When running on SYSTEMPORT Lite, certain statistics are unsupported and
skipped during bcm_sysport_get_stats(). The variable 'j' tracks the
compacted index into the destination data buffer, whereas 'i' iterates
over all elements in bcm_sysport_gstrings_stats.
Because the buffer allocated by ethtool is sized only according to
bcm_sysport_get_sset_count(), storing values at data[i] instead of
data[j] writes past the allocated array bounds, leading to memory
corruption.
Fix this by writing to data[j] instead of data[i].
Fixes: 10377ba7673d ("net: systemport: Support 64bit statistics")
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Link: https://patch.msgid.link/20260922232440.598918-2-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
|
Remove the redundant duplicate definitions of ASP_RX_FILTER_MDA_CFG,
ASP_RX_FILTER_MDA_PAT_H, ASP_RX_FILTER_MDA_PAT_L, ASP_RX_FILTER_MDA_MSK_H,
and ASP_RX_FILTER_MDA_MSK_L in bcmasp.h.
Signed-off-by: Florian Fainelli <florian.fainelli@broadcom.com>
Reviewed-by: Simon Horman <horms@kernel.org>
Reviewed-by: Nicolai Buchwitz <nb@tipi-net.de>
Link: https://patch.msgid.link/20260922222525.3992167-3-florian.fainelli@broadcom.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
|
