Skip to contentSkip to conversation

stmmac crash/stall fixes when under memory pressure

PATCH Superseded Sam Edwards 4 participants 12 messages Apr 1 – Apr 11

Subsystems (5)

v4 of 6 Superseded Latest is v6 →

lore
Share & tools
lore
#0 lore raw

v4 cover stmmac crash/stall fixes when under memory pressure

Hi netdev,

This is v4 of my series containing a pair of bugfixes for the stmmac driver's receive pipeline. These issues occur when stmmac_rx_refill() does not (fully) succeed, which happens more frequently when free memory is low.

The first patch closes Bugzilla bug #221010 [1], where stmmac_rx() can circle around to a still-dirty descriptor (with a NULL buffer pointer), mistake it for a filled descriptor (due to OWN=0), and attempt to dereference the buffer.

In testing that patch, I discovered a second issue: starvation of available RX buffers causes the NIC to stop sending interrupts; if the driver stops polling, it will wait indefinitely for an interrupt that will never come. (Note: the first patch makes this issue more prominent -- mostly because it lets the system survive long enough to exhibit it -- but doesn't *cause* it.) The second patch addresses that problem as well.

Both patches are minimal, appropriate for stable, and designated to net. My focus is on small, obviously-correct, easy-to-explain changes: I'll follow up with another patch/series (something like [2]) for net-next that fixes the ring in a more robust way.

The tx and zc paths seem to have similar low-memory bugs, to be addressed in separate series.

Regards,
Sam

---

[1] https://bugzilla.kernel.org/show_bug.cgi?id=221010
[2] https://lore.kernel.org/netdev/[email protected]/

v4:
- Changed patch 2 to tolerate dirty stragglers up to a critical threshold (the same threshold tolerated by the zero-copy path), to avoid nuisance looping during OOM conditions (thanks Jakub)
v3: https://lore.kernel.org/netdev/[email protected]/T/
- Rebased on latest net/main
- Changed patch 2 to require that stmmac_rx_refill() *fully* succeeds before exiting polling, to reduce the chance of rx drops.
- DID NOT use the CIRC_SPACE() macro as suggested by Russell: I fear that the perspective shift (first think of the dirty descriptors as the "work" that refill "consumes" -- therefore the "space" is how much stmmac_rx() may loop) is too counterintuitive for a stable fix, but I'll do it in v4 if reviewers insist.
- Updated the recipients for the series, which was invalidated in v2 due to the Fixes:
v2: https://lore.kernel.org/netdev/[email protected]/T/
- Completely rewrote the commit message of patch 1, now assuming the reader is generally familiar with DMA but wholly unfamiliar with the stmmac device (thanks Jakub!)
- Added missing Fixes: to patch 2
- Moved patch 2's int budget = limit; decl per the reverse-xmas-tree rule
- Dropped patch 3: this was a code improvement not appropriate for stable
- Generated the series with --subject-prefix='PATCH net'
v1: https://lore.kernel.org/netdev/[email protected]/

Sam Edwards (2):
net: stmmac: Prevent NULL deref when RX memory exhausted
net: stmmac: Prevent indefinite RX stall on buffer exhaustion

 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)
Signature · 1 line
2.52.0
#1 lore raw

PATCH v4 1/2 net: stmmac: Prevent NULL deref when RX memory exhausted

The CPU receives frames from the MAC through conventional DMA: the CPU allocates buffers for the MAC, then the MAC fills them and returns ownership to the CPU. For each hardware RX queue, the CPU and MAC coordinate through a shared ring array of DMA descriptors: one descriptor per DMA buffer. Each descriptor includes the buffer's physical address and a status flag ("OWN") indicating which side owns the buffer: OWN=0 for CPU, OWN=1 for MAC. The CPU is only allowed to set the flag and the MAC is only allowed to clear it, and both must move through the ring in sequence: thus the ring is used for both "submissions" and "completions."

In the stmmac driver, stmmac_rx() bookmarks its position in the ring with the cur_rx index. The main receive loop in that function checks for rx_descs[cur_rx].own=0, gives the corresponding buffer to the network stack (NULLing the pointer), and increments cur_rx modulo the ring size. After the loop exits, stmmac_rx_refill(), which bookmarks its position with dirty_rx, allocates fresh buffers and rearms the descriptors (setting OWN=1). If it fails any allocation, it simply stops early (leaving OWN=0) and will retry where it left off when next called.

This means descriptors have a three-stage lifecycle (terms my own):
- empty (OWN=1, buffer valid)
- full (OWN=0, buffer valid and populated)
- dirty (OWN=0, buffer NULL)

But because stmmac_rx() only checks OWN, it confuses full/dirty. In the past (see 'Fixes:'), there was a bug where the loop could cycle cur_rx all the way back to the first descriptor it dirtied, resulting in a NULL dereference when mistaken for full. The aforementioned commit resolved that *specific* failure by capping the loop's iteration limit at dma_rx_size - 1, but this is only a partial fix: if the previous stmmac_rx_refill() didn't complete, then there are leftover dirty descriptors that the loop might encounter without needing to cycle fully around. The current code therefore panics (see 'Closes:') when stmmac_rx_refill() is memory-starved long enough for cur_rx to catch up to dirty_rx.

Fix this by further tightening the clamp from dma_rx_size - 1 to dma_rx_size - stmmac_rx_dirty() - 1, subtracting any remnant dirty entries and limiting the loop so that cur_rx cannot catch back up to dirty_rx. This carries no risk of arithmetic underflow: since the maximum possible return value of stmmac_rx_dirty() is dma_rx_size - 1, the worst the clamp can do is prevent the loop from running at all.

Fixes b6cb4541853c7 ("net: stmmac: avoid rx queue overrun")
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +2 -1
index 13d3cac056be..fc11f75f7dc0 100644
@@ -5609,7 +5609,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 
 	dma_dir = page_pool_get_dma_dir(rx_q->page_pool);
 	bufsz = DIV_ROUND_UP(priv->dma_conf.dma_buf_sz, PAGE_SIZE) * PAGE_SIZE;
-	limit = min(priv->dma_conf.dma_rx_size - 1, (unsigned int)limit);
+	limit = min(priv->dma_conf.dma_rx_size - stmmac_rx_dirty(priv, queue) - 1,
+		    (unsigned int)limit);
 
 	if (netif_msg_rx_status(priv)) {
 		void *rx_head;
Signature · 1 line
2.52.0
#2 lore raw

PATCH v4 2/2 net: stmmac: Prevent indefinite RX stall on buffer exhaustion

In reply to Sam Edwards

The stmmac driver handles interrupts in the usual NAPI way: an interrupt arrives, the NAPI instance is scheduled and interrupts are masked, and the actual work occurs in the NAPI polling function. Once no further work remains, interrupts are unmasked and the NAPI instance is put to sleep to await a future interrupt. In the receive case, the MAC only sends the interrupt when a DMA operation completes; thus the driver must make sure a usable RX DMA descriptor exists before expecting a future interrupt.

The main receive loop in stmmac_rx() exits under one of 3 conditions:
1) It encounters a DMA descriptor with OWN=1, indicating that no further pending data exists. The MAC will use this descriptor for the next RX DMA operation, so the driver can expect a future interrupt.
2) It exhausts the NAPI budget. In this case, the driver doesn't know whether the MAC has any usable DMA descriptors. But when the driver consumes its full budget, that signals NAPI to keep polling, so the question is moot.
3) It runs out of (non-dirty) descriptors in the RX ring. In this case, the MAC will only have a usable descriptor if stmmac_rx_refill() succeeds (at least partially).

Currently, stmmac_rx() lacks any check against scenario #3 and stmmac_rx_refill() failing: it will stop NAPI polling and unmask interrupts to await an interrupt that will never arrive, stalling the receive pipeline indefinitely.

Also: even if not all descriptors are dirty, letting them accumulate risks dropping frames in the next incoming traffic burst, if large enough to exhaust the remaining valid ones.

Fix both of these problems by checking stmmac_rx_dirty() before return:
Use the same threshold as the zero-copy path (STMMAC_RX_FILL_BATCH) for an unacceptably high number of neglected dirties, and tell NAPI to keep polling (i.e. return budget) when that threshold is met.

Fixes 47dd7a540b8a ("net: add support for STMicroelectronics Ethernet controllers.")
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 9 +++++++++
 1 file changed, 9 insertions(+)
drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +9 -0
index fc11f75f7dc0..6822ca27cb0f 100644
@@ -5604,6 +5604,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	unsigned int desc_size;
 	struct sk_buff *skb = NULL;
 	struct stmmac_xdp_buff ctx;
+	int budget = limit;
 	int xdp_status = 0;
 	int bufsz;
 
@@ -5870,6 +5871,14 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	priv->xstats.rx_dropped += rx_dropped;
 	priv->xstats.rx_errors += rx_errors;
 
+	/* stmmac_rx_refill() may fail, leaving some dirty entries behind.
+	 * A few is OK, but if it gets out of hand, we risk dropping frames
+	 * in the next traffic burst; in the worst case (100% dirty) we won't
+	 * even receive any future "DMA completed" interrupts.
+	 */
+	if (unlikely(stmmac_rx_dirty(priv, queue) >= STMMAC_RX_FILL_BATCH))
+		return budget;
+
 	return count;
 }
 
Signature · 1 line
2.52.0
maintainer
#3 lore raw
In reply to Sam Edwards
Quoted 3 lines · On Tue, 31 Mar 2026 21:19:27 -0700 Sam Edwards wrote:

- Changed patch 2 to tolerate dirty stragglers up to a critical threshold (the same threshold tolerated by the zero-copy path), to avoid nuisance looping during OOM conditions (thanks Jakub)

I meant we need both a threshold, and a delay :(

#4 lore raw
Quoted 1 line · Jakub Kicinski

I meant we need both a threshold, and a delay :(

Hi Jakub - got it: when the critical threshold is reached, allow the NAPI instance to sleep and start a timer instead.

1) We'd either have to leave interrupts masked or let them race against the timer. Either one is manageable, but I feel like those interactions carry *just* enough regression risk to bump that patch to -next.

2) Could you point out which NAPI driver best handles this situation? I'd like to replicate its approach.

Thanks,
Sam

maintainer
#5 lore raw
Quoted 6 lines · On Thu, 2 Apr 2026 09:53:43 -0700 Sam Edwards wrote:
Quoted 1 line · Jakub Kicinski

I meant we need both a threshold, and a delay :(

Hi Jakub - got it: when the critical threshold is reached, allow the NAPI instance to sleep and start a timer instead.

Quoted 7 lines

1) We'd either have to leave interrupts masked or let them race against the timer. Either one is manageable, but I feel like those interactions carry *just* enough regression risk to bump that patch to -next.

2) Could you point out which NAPI driver best handles this situation? I'd like to replicate its approach.

Not sure, the last few NICs I worked on had the ability for SW to trigger IRQs exactly because of the Rx buffer depletion issue. fbnic_napi_depletion_check() for example.

But let's not overthink it.. say we arm a timer and let the IRQ be unmasked. The timer just runs napi_schedule(). napi_schedule() is thread-safe, if IRQ fires with the timer armed - no problem.

#6 lore raw
In reply to Sam Edwards
Quoted 17 lines · Sam Edwards

Hi netdev,

This is v4 of my series containing a pair of bugfixes for the stmmac driver's receive pipeline. These issues occur when stmmac_rx_refill() does not (fully) succeed, which happens more frequently when free memory is low.

The first patch closes Bugzilla bug #221010 [1], where stmmac_rx() can circle around to a still-dirty descriptor (with a NULL buffer pointer), mistake it for a filled descriptor (due to OWN=0), and attempt to dereference the buffer.

In testing that patch, I discovered a second issue: starvation of available RX buffers causes the NIC to stop sending interrupts; if the driver stops polling, it will wait indefinitely for an interrupt that will never come. (Note: the first patch makes this issue more prominent -- mostly because it lets the system survive long enough to exhibit it -- but doesn't *cause* it.) The second patch addresses that problem as well.

Quoted 7 lines

Both patches are minimal, appropriate for stable, and designated to net. My focus is on small, obviously-correct, easy-to-explain changes: I'll follow up with another patch/series (something like [2]) for net-next that fixes the ring in a more robust way.

The tx and zc paths seem to have similar low-memory bugs, to be addressed in separate series.

I've tested this on my Jetson Xavier platform. One of the issues I've had is that running iperf3 results in the receive side stalling because it runs out of descriptors. However, despite the receive ring eventually being re-filled and the hardware appropriately prodded, it steadfastly refuses to restart, despite the descriptors having been updated.

What I can see is there's 40 packets in the internal FIFOs via the PRXQ[13:0] field of the ETH_MTLRXQxDR register.

With your patches applied:

root@tegra-ubuntu:~# iperf3 -c 192.168.248.1 -R
Connecting to host 192.168.248.1, port 5201
Reverse mode, remote host 192.168.248.1 is sending
[  5] local 192.168.248.174 port 43728 connected to 192.168.248.1 port 5201
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-1.00   sec  30.3 MBytes   254 Mbits/sec
[  5]   1.00-2.00   sec  0.00 Bytes  0.00 bits/sec
[  5]   2.00-3.00   sec  0.00 Bytes  0.00 bits/sec
[  5]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec
[  5]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec
[  5]   5.00-6.00   sec  0.00 Bytes  0.00 bits/sec
...

The remote system says:

Accepted connection from 192.168.248.174, port 43720
[ 5] local 192.168.248.1 port 5201 connected to 192.168.248.174 port 43728

[ ID] Interval           Transfer     Bitrate         Retr  Cwnd
[  5]   0.00-1.00   sec  31.8 MBytes   266 Mbits/sec    1   1.41 KBytes
[  5]   1.00-2.00   sec  0.00 Bytes  0.00 bits/sec    1   1.41 KBytes
[  5]   2.00-3.00   sec  0.00 Bytes  0.00 bits/sec    0   1.41 KBytes

This is a dwmac v5.0. I think the relevant registers are:

ETH_MTLQxICSR: Value at address 0x02490d2c: 0x00000002

ETH_MTLRXQxDR: Value at address 0x02490d38: 0x00280020

PRXQ[13:0]: Number of Packets in Receive Queue = 40 RXQSTS[1:0]: MTL Rx Queue Fill-Level status = 2 (Rx queue fill-level above flow-control activate threshold)
RRXSTS[1:0]: MTL Rx Queue Read Controller State = 0 (Idle)
RWCSTS: MTL Rx Queue Write Controller Active Status = 0

ETH_DMADS1R: Value at address 0x0249100c: 0x00006300

RPS0[3:0]: DMA Channel Receive Process State = Running (Waiting for Rx packet)

ETH_DMACxRXDTPR: Value at address 0x02491128: 0xffffe7d0

RDT[31:0]: Receive Descriptor Tail Pointer (writes should apparently trigger DMA to start, but doesn't seem to)

ETH_DMACxCARXDR: Value at address 0x0249114c: 0xffffe7d0

CURRDESAPTR[31:0]: Application receive descriptor address pointer

ETH_DMACxCARXBR: Value at address 0x0249115c: 0xffb55040

CURRBUFAPTR[31:0]: Application receive buffer address pointer

This makes it look like it's read the descriptor at 0x7fffffe7d0.

ETH_DMAC0SR: Value at address 0x02491160: 0x00000484

ETI (early transmit interrupt)=1 RBU (receive buffer unavailable)=1 TBU (transmit buffer unavailable)=1

Clearing RBU doesn't seem to help.

Descriptors:

123 [0x0000007fffffe7b0]: 0xffbba040 0x7f 0x0 0x81000000 124 [0x0000007fffffe7c0]: 0xffbb9040 0x7f 0x0 0x81000000 125 [0x0000007fffffe7d0]: 0xffb55040 0x7f 0x0 0x81000000 <--- 126 [0x0000007fffffe7e0]: 0xffc26040 0x7f 0x0 0x81000000 127 [0x0000007fffffe7f0]: 0xfff95040 0x7f 0x0 0x81000000

So they're all refilled.

Any ideas?

Full register dump via devmem2 (ethtool -d doesn't dump all registers):

Value at address 0x02490000: 0x08072203 Value at address 0x02490004: 0x00200000 Value at address 0x02490008: 0x00010404 Value at address 0x0249000c: 0x00000000 Value at address 0x02490010: 0x00040008 Value at address 0x02490014: 0x20000008 Value at address 0x02490018: 0x00000001 Value at address 0x0249001c: 0x00000001 Value at address 0x02490020: 0x00000000 Value at address 0x02490024: 0x00000000 Value at address 0x02490028: 0x00000000 Value at address 0x0249002c: 0x00000000 Value at address 0x02490030: 0x00000000 Value at address 0x02490034: 0x00000000 Value at address 0x02490038: 0x00000000 Value at address 0x0249003c: 0x00000000 Value at address 0x02490040: 0x00000000 Value at address 0x02490044: 0x00000000 Value at address 0x02490048: 0x00000000 Value at address 0x0249004c: 0x00000000 Value at address 0x02490050: 0x01600000 Value at address 0x02490054: 0x00000000 Value at address 0x02490058: 0x00000000 Value at address 0x0249005c: 0x00000000 Value at address 0x02490060: 0x00120000 Value at address 0x02490064: 0x00000000 Value at address 0x02490068: 0x00000000 Value at address 0x0249006c: 0x00000000 Value at address 0x02490070: 0xffff0002 Value at address 0x02490074: 0x00000000 Value at address 0x02490078: 0x00000000 Value at address 0x0249007c: 0x00000000 Value at address 0x02490080: 0x00000000 Value at address 0x02490084: 0x00000000 Value at address 0x02490088: 0x00000000 Value at address 0x0249008c: 0x00000000 Value at address 0x02490090: 0x00000001 Value at address 0x02490094: 0x00000000 Value at address 0x02490098: 0x00000000 Value at address 0x0249009c: 0x00000000 Value at address 0x024900a0: 0x00000002 Value at address 0x024900a4: 0x00000000 Value at address 0x024900a8: 0x00000000 Value at address 0x024900ac: 0x00000000 Value at address 0x024900b0: 0x00000001 Value at address 0x024900b4: 0x00001030 Value at address 0x024900b8: 0x00000000 Value at address 0x024900bc: 0x00000000 Value at address 0x024900c0: 0x00000000 Value at address 0x024900c4: 0x00000000 Value at address 0x024900c8: 0x00000000 Value at address 0x024900cc: 0x00000000 Value at address 0x024900d0: 0x003b0200 Value at address 0x024900d4: 0x03e8001e Value at address 0x024900d8: 0x000f4240 Value at address 0x024900dc: 0x0000007c Value at address 0x024900e0: 0x00000000 Value at address 0x024900e4: 0x00000000 Value at address 0x024900e8: 0x00000000 Value at address 0x024900ec: 0x00000000 Value at address 0x024900f0: 0x00000000 Value at address 0x024900f4: 0x00000000 Value at address 0x024900f8: 0x000d0000 Value at address 0x024900fc: 0x00000000 Value at address 0x02490100: 0x00000000 Value at address 0x02490104: 0x00000000 Value at address 0x02490108: 0x00000000 Value at address 0x0249010c: 0x00000000 Value at address 0x02490110: 0x00001050 Value at address 0x02490114: 0x00000000 Value at address 0x02490118: 0x00000000 Value at address 0x0249011c: 0x1bfd73f7 Value at address 0x02490120: 0x421e7a49 Value at address 0x02490124: 0x100c30c3 Value at address 0x02490128: 0x00320220 Value at address 0x02490200: 0x000e010c Value at address 0x02490204: 0x00000006 Value at address 0x02490208: 0x00000000 Value at address 0x0249020c: 0x00000000 Value at address 0x02490210: 0x00000000 Value at address 0x02490214: 0x00000000 Value at address 0x02490218: 0x00000000 Value at address 0x0249021c: 0x00000000 Value at address 0x02490220: 0x00000000 Value at address 0x02490224: 0x00000000 Value at address 0x02490228: 0x00000000 Value at address 0x0249022c: 0x00000000 Value at address 0x02490230: 0x00000000 Value at address 0x02490234: 0x00000000 Value at address 0x02490238: 0x00000102 Value at address 0x02490d00: 0x00ff000a Value at address 0x02490d04: 0x00000000 Value at address 0x02490d08: 0x00000000 Value at address 0x02490d0c: 0x00000000 Value at address 0x02490d10: 0x00000000 Value at address 0x02490d14: 0x00000030 Value at address 0x02490d18: 0x00000000 Value at address 0x02490d1c: 0x00000000 Value at address 0x02490d20: 0x00000000 Value at address 0x02490d24: 0x00000000 Value at address 0x02490d28: 0x00000000 Value at address 0x02490d2c: 0x00000002 Value at address 0x02490d30: 0x0ff1c4e0 Value at address 0x02490d34: 0x00000000 Value at address 0x02490d38: 0x00280020 Value at address 0x02490d3c: 0x00000000 Value at address 0x02491000: 0x00000000 Value at address 0x02491004: 0x0002180e Value at address 0x02491008: 0x00000000 Value at address 0x0249100c: 0x00006300 Value at address 0x02491010: 0x00000000 Value at address 0x02491014: 0x00000000 Value at address 0x02491018: 0x00000000 Value at address 0x0249101c: 0x00000000 Value at address 0x02491020: 0x00000000 Value at address 0x02491024: 0x00000000 Value at address 0x02491028: 0x00000000 Value at address 0x0249102c: 0x00000000 Value at address 0x02491030: 0x00000000 Value at address 0x02491034: 0x00000000 Value at address 0x02491038: 0x00000000 Value at address 0x0249103c: 0x00000000 Value at address 0x02491040: 0x00000000 Value at address 0x02491044: 0x00000000 Value at address 0x02491048: 0x00000000 Value at address 0x0249104c: 0x00000000 Value at address 0x02491050: 0x00000000 Value at address 0x02491054: 0x00000000 Value at address 0x02491100: 0x00010000 Value at address 0x02491104: 0x00101011 Value at address 0x02491108: 0x00080c01 Value at address 0x0249110c: 0x00000000 Value at address 0x02491110: 0x0000007f Value at address 0x02491114: 0xffffc000 Value at address 0x02491118: 0x0000007f Value at address 0x0249111c: 0xffffe000 Value at address 0x02491120: 0xffffc500 Value at address 0x02491124: 0x00000000 Value at address 0x02491128: 0xffffe7d0 Value at address 0x0249112c: 0x000001ff Value at address 0x02491130: 0x000001ff Value at address 0x02491134: 0x0000d041 Value at address 0x02491138: 0x000000a0 Value at address 0x0249113c: 0x000d07c0 Value at address 0x02491140: 0x00000000 Value at address 0x02491144: 0xffffc500 Value at address 0x02491148: 0x00000000 Value at address 0x0249114c: 0xffffe7d0 Value at address 0x02491150: 0x0000007f Value at address 0x02491154: 0xffc45b02 Value at address 0x02491158: 0x0000007f Value at address 0x0249115c: 0xffb55040 Value at address 0x02491160: 0x00000484 Value at address 0x02491164: 0x00000000 Value at address 0x02491168: 0x00000000 Value at address 0x0249116c: 0x00000000 Value at address 0x02491170: 0x00000000 Value at address 0x02491174: 0x00000000 Value at address 0x02491178: 0x00000000 Value at address 0x0249117c: 0x00000000

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#7 lore raw
Quoted 26 lines · Russell King (Oracle)
Quoted 17 lines · Sam Edwards

Hi netdev,

This is v4 of my series containing a pair of bugfixes for the stmmac driver's receive pipeline. These issues occur when stmmac_rx_refill() does not (fully) succeed, which happens more frequently when free memory is low.

The first patch closes Bugzilla bug #221010 [1], where stmmac_rx() can circle around to a still-dirty descriptor (with a NULL buffer pointer), mistake it for a filled descriptor (due to OWN=0), and attempt to dereference the buffer.

In testing that patch, I discovered a second issue: starvation of available RX buffers causes the NIC to stop sending interrupts; if the driver stops polling, it will wait indefinitely for an interrupt that will never come. (Note: the first patch makes this issue more prominent -- mostly because it lets the system survive long enough to exhibit it -- but doesn't *cause* it.) The second patch addresses that problem as well.

Quoted 7 lines

Both patches are minimal, appropriate for stable, and designated to net. My focus is on small, obviously-correct, easy-to-explain changes: I'll follow up with another patch/series (something like [2]) for net-next that fixes the ring in a more robust way.

The tx and zc paths seem to have similar low-memory bugs, to be addressed in separate series.

Quoted 6 lines

I've tested this on my Jetson Xavier platform. One of the issues I've had is that running iperf3 results in the receive side stalling because it runs out of descriptors. However, despite the receive ring eventually being re-filled and the hardware appropriately prodded, it steadfastly refuses to restart, despite the descriptors having been updated.

I'll make it clear: this problem exists without your patches, so it is not a regression.

Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#8 lore raw
In reply to Russell King (Oracle)
Quoted 6 lines · Russell King (Oracle)

I've tested this on my Jetson Xavier platform. One of the issues I've had is that running iperf3 results in the receive side stalling because it runs out of descriptors. However, despite the receive ring eventually being re-filled and the hardware appropriately prodded, it steadfastly refuses to restart, despite the descriptors having been updated.

Hi Russell,

Just to make sure I understand correctly: before my patches, you've been observing this problem on Xavier for a while (no interrupts, ring goes dry); with my patches, the ring is refilled, but the dwmac5 doesn't resume DMA. (Ah, just saw your follow-up email.)

Quoted 1 line

Any ideas?

Off the top of my head, my hypothesis is that dwmac5 has an additional tripwire when the receive DMA is exhausted, and the stmmac_set_rx_tail_ptr()/stmmac_enable_dma_reception() at the end of stmmac_rx_refill() aren't sufficient to wake it back up.

I think this is new to dwmac5, because my RK3588 (dwmac4.20 iirc) happily resumes after the same condition.

You gave a lot of info; thanks! I'll try to scrape up some documentation on dwmac5 to see if there's something more stmmac_rx_refill() ought to be doing. I think I have a Xavier NX around here somewhere, I'll see if I can repro the problem.

Cheers,
Sam

#9 lore raw
Quoted 23 lines · Sam Edwards
Quoted 6 lines · Russell King (Oracle)

I've tested this on my Jetson Xavier platform. One of the issues I've had is that running iperf3 results in the receive side stalling because it runs out of descriptors. However, despite the receive ring eventually being re-filled and the hardware appropriately prodded, it steadfastly refuses to restart, despite the descriptors having been updated.

Hi Russell,

Just to make sure I understand correctly: before my patches, you've been observing this problem on Xavier for a while (no interrupts, ring goes dry); with my patches, the ring is refilled, but the dwmac5 doesn't resume DMA. (Ah, just saw your follow-up email.)

Quoted 1 line

Any ideas?

Off the top of my head, my hypothesis is that dwmac5 has an additional tripwire when the receive DMA is exhausted, and the stmmac_set_rx_tail_ptr()/stmmac_enable_dma_reception() at the end of stmmac_rx_refill() aren't sufficient to wake it back up.

Quoted 7 lines

I think this is new to dwmac5, because my RK3588 (dwmac4.20 iirc) happily resumes after the same condition.

You gave a lot of info; thanks! I'll try to scrape up some documentation on dwmac5 to see if there's something more stmmac_rx_refill() ought to be doing. I think I have a Xavier NX around here somewhere, I'll see if I can repro the problem.

I've added dma_rmb() into dwmac4_wrback_get_tx_status() and dwmac4_wrback_get_rx_status(), and with that I've had an iperf3 instance finally complete... but only once:

root@tegra-ubuntu:~# iperf3 -c 192.168.248.1 -R
Connecting to host 192.168.248.1, port 5201
Reverse mode, remote host 192.168.248.1 is sending
[  5] local 192.168.248.174 port 42232 connected to 192.168.248.1 port 5201
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-1.00   sec  50.8 MBytes   426 Mbits/sec
[  5]   1.00-2.00   sec  54.9 MBytes   460 Mbits/sec
[  5]   2.00-3.00   sec  54.0 MBytes   453 Mbits/sec
[  5]   3.00-4.00   sec  53.8 MBytes   452 Mbits/sec
[  5]   4.00-5.00   sec  52.4 MBytes   438 Mbits/sec
[  5]   5.00-6.00   sec  54.3 MBytes   455 Mbits/sec
[  5]   6.00-7.00   sec  53.7 MBytes   452 Mbits/sec
[  5]   7.00-8.00   sec  52.8 MBytes   443 Mbits/sec
[  5]   8.00-9.00   sec  53.7 MBytes   451 Mbits/sec
[  5]   9.00-10.00  sec  54.3 MBytes   455 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.01  sec   537 MBytes   450 Mbits/sec   13             sender
[  5]   0.00-10.00  sec   535 MBytes   448 Mbits/sec                  receiver

iperf Done.

So, it seems better, but not completely solved.

drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c +8 -4
index 2994df41ec2c..119f31c94b61 100644
@@ -17,10 +17,12 @@ static int dwmac4_wrback_get_tx_status(struct stmmac_extra_stats *x,
 				       struct dma_desc *p,
 				       void __iomem *ioaddr)
 {
-	u32 tdes3 = le32_to_cpu(p->des3);
+	u32 tdes3;
 	int ret = tx_done;
 
 	/* Get tx owner first */
+	dma_rmb();
+	tdes3 = le32_to_cpu(p->des3);
 	if (unlikely(tdes3 & TDES3_OWN))
 		return tx_dma_own;
 
@@ -70,12 +72,12 @@ static int dwmac4_wrback_get_tx_status(struct stmmac_extra_stats *x,
 static int dwmac4_wrback_get_rx_status(struct stmmac_extra_stats *x,
 				       struct dma_desc *p)
 {
-	u32 rdes1 = le32_to_cpu(p->des1);
-	u32 rdes2 = le32_to_cpu(p->des2);
-	u32 rdes3 = le32_to_cpu(p->des3);
+	u32 rdes1, rdes2, rdes3;
 	int message_type;
 	int ret = good_frame;
 
+	dma_rmb();
+	rdes3 = le32_to_cpu(p->des3);
 	if (unlikely(rdes3 & RDES3_OWN))
 		return dma_own;
 
@@ -107,6 +109,7 @@ static int dwmac4_wrback_get_rx_status(struct stmmac_extra_stats *x,
 
 	message_type = FIELD_GET(RDES1_PTP_MSG_TYPE_MASK, rdes1);
 
+	rdes1 = le32_to_cpu(p->des1);
 	if (rdes1 & RDES1_IP_HDR_ERROR) {
 		x->ip_hdr_err++;
 		ret |= csum_none;
@@ -152,6 +155,7 @@ static int dwmac4_wrback_get_rx_status(struct stmmac_extra_stats *x,
 	if (rdes1 & RDES1_TIMESTAMP_DROPPED)
 		x->timestamp_dropped++;
 
+	rdes2 = le32_to_cpu(p->des2);
 	if (unlikely(rdes2 & RDES2_SA_FILTER_FAIL)) {
 		x->sa_rx_filter_fail++;
 		ret = discard_frame;
Signature · 2 lines
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!
#10 lore raw
Quoted 28 lines · Russell King (Oracle)
Quoted 26 lines · Sam Edwards
Quoted 6 lines · Russell King (Oracle)

I've tested this on my Jetson Xavier platform. One of the issues I've had is that running iperf3 results in the receive side stalling because it runs out of descriptors. However, despite the receive ring eventually being re-filled and the hardware appropriately prodded, it steadfastly refuses to restart, despite the descriptors having been updated.

Hi Russell,

Just to make sure I understand correctly: before my patches, you've been observing this problem on Xavier for a while (no interrupts, ring goes dry); with my patches, the ring is refilled, but the dwmac5 doesn't resume DMA. (Ah, just saw your follow-up email.)

Quoted 1 line

Any ideas?

Off the top of my head, my hypothesis is that dwmac5 has an additional tripwire when the receive DMA is exhausted, and the stmmac_set_rx_tail_ptr()/stmmac_enable_dma_reception() at the end of stmmac_rx_refill() aren't sufficient to wake it back up.

I think this is new to dwmac5, because my RK3588 (dwmac4.20 iirc) happily resumes after the same condition.

Quoted 8 lines
Quoted 4 lines

You gave a lot of info; thanks! I'll try to scrape up some documentation on dwmac5 to see if there's something more stmmac_rx_refill() ought to be doing. I think I have a Xavier NX around here somewhere, I'll see if I can repro the problem.

I've added dma_rmb() into dwmac4_wrback_get_tx_status() and dwmac4_wrback_get_rx_status(), and with that I've had an iperf3 instance finally complete... but only once:

Hi Russell,

To me it feels relevant that the T194 doesn't use first-party ARM/Cortex cores but rather Nvidia's in-house "Carmel" architecture. Do you suppose the cache there is quirky in such a way that either:
1) We're seeing poor cache hygiene in stmmac where other caches are more forgiving (more likely)
2) Carmel's cache has a subtle hardware bug triggered by stmmac's specific access pattern (less likely)?

I'm still trying to get my Xavier NX to boot on net-next. It's running into eMMC corruption/stalls very early in the boot process (at slightly different times; feels like a problem in autocalibration) that I'm not seeing on older kernels. Once I'm done bisecting that regression I'll take a deeper look at this stmmac mystery. :)

Cheers,
Sam

Quoted 79 lines
root@tegra-ubuntu:~# iperf3 -c 192.168.248.1 -R
Connecting to host 192.168.248.1, port 5201
Reverse mode, remote host 192.168.248.1 is sending
[  5] local 192.168.248.174 port 42232 connected to 192.168.248.1 port 5201
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-1.00   sec  50.8 MBytes   426 Mbits/sec
[  5]   1.00-2.00   sec  54.9 MBytes   460 Mbits/sec
[  5]   2.00-3.00   sec  54.0 MBytes   453 Mbits/sec
[  5]   3.00-4.00   sec  53.8 MBytes   452 Mbits/sec
[  5]   4.00-5.00   sec  52.4 MBytes   438 Mbits/sec
[  5]   5.00-6.00   sec  54.3 MBytes   455 Mbits/sec
[  5]   6.00-7.00   sec  53.7 MBytes   452 Mbits/sec
[  5]   7.00-8.00   sec  52.8 MBytes   443 Mbits/sec
[  5]   8.00-9.00   sec  53.7 MBytes   451 Mbits/sec
[  5]   9.00-10.00  sec  54.3 MBytes   455 Mbits/sec
- - - - - - - - - - - - - - - - - - - - - - - - -
[ ID] Interval           Transfer     Bitrate         Retr
[  5]   0.00-10.01  sec   537 MBytes   450 Mbits/sec   13             sender
[  5]   0.00-10.00  sec   535 MBytes   448 Mbits/sec                  receiver

iperf Done.

So, it seems better, but not completely solved.

drivers/net/ethernet/stmicro/stmmac/dwmac4_descs.c +8 -5
index 2994df41ec2c..119f31c94b61 100644
@@ -17,10 +17,12 @@ static int dwmac4_wrback_get_tx_status(struct stmmac_extra_stats *x,
                                       struct dma_desc *p,
                                       void __iomem *ioaddr)
 {
-       u32 tdes3 = le32_to_cpu(p->des3);
+       u32 tdes3;
        int ret = tx_done;
 
        /* Get tx owner first */
+       dma_rmb();
+       tdes3 = le32_to_cpu(p->des3);
        if (unlikely(tdes3 & TDES3_OWN))
                return tx_dma_own;
 
@@ -70,12 +72,12 @@ static int dwmac4_wrback_get_tx_status(struct stmmac_extra_stats *x,
 static int dwmac4_wrback_get_rx_status(struct stmmac_extra_stats *x,
                                       struct dma_desc *p)
 {
-       u32 rdes1 = le32_to_cpu(p->des1);
-       u32 rdes2 = le32_to_cpu(p->des2);
-       u32 rdes3 = le32_to_cpu(p->des3);
+       u32 rdes1, rdes2, rdes3;
        int message_type;
        int ret = good_frame;
 
+       dma_rmb();
+       rdes3 = le32_to_cpu(p->des3);
        if (unlikely(rdes3 & RDES3_OWN))
                return dma_own;
 
@@ -107,6 +109,7 @@ static int dwmac4_wrback_get_rx_status(struct stmmac_extra_stats *x,
 
        message_type = FIELD_GET(RDES1_PTP_MSG_TYPE_MASK, rdes1);
 
+       rdes1 = le32_to_cpu(p->des1);
        if (rdes1 & RDES1_IP_HDR_ERROR) {
                x->ip_hdr_err++;
                ret |= csum_none;
@@ -152,6 +155,7 @@ static int dwmac4_wrback_get_rx_status(struct stmmac_extra_stats *x,
        if (rdes1 & RDES1_TIMESTAMP_DROPPED)
                x->timestamp_dropped++;
 
+       rdes2 = le32_to_cpu(p->des2);
        if (unlikely(rdes2 & RDES2_SA_FILTER_FAIL)) {
                x->sa_rx_filter_fail++;
                ret = discard_frame;
 
--

RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!

#11 lore raw
In reply to Russell King (Oracle)

Hi Russell

Quoted 38 lines · Russell King (Oracle)
Quoted 17 lines · Sam Edwards

Hi netdev,

This is v4 of my series containing a pair of bugfixes for the stmmac driver's receive pipeline. These issues occur when stmmac_rx_refill() does not (fully) succeed, which happens more frequently when free memory is low.

The first patch closes Bugzilla bug #221010 [1], where stmmac_rx() can circle around to a still-dirty descriptor (with a NULL buffer pointer), mistake it for a filled descriptor (due to OWN=0), and attempt to dereference the buffer.

In testing that patch, I discovered a second issue: starvation of available RX buffers causes the NIC to stop sending interrupts; if the driver stops polling, it will wait indefinitely for an interrupt that will never come. (Note: the first patch makes this issue more prominent -- mostly because it lets the system survive long enough to exhibit it -- but doesn't *cause* it.) The second patch addresses that problem as well.

Quoted 7 lines

Both patches are minimal, appropriate for stable, and designated to net. My focus is on small, obviously-correct, easy-to-explain changes: I'll follow up with another patch/series (something like [2]) for net-next that fixes the ring in a more robust way.

The tx and zc paths seem to have similar low-memory bugs, to be addressed in separate series.

I've tested this on my Jetson Xavier platform. One of the issues I've had is that running iperf3 results in the receive side stalling because it runs out of descriptors. However, despite the receive ring eventually being re-filled and the hardware appropriately prodded, it steadfastly refuses to restart, despite the descriptors having been updated.

What I can see is there's 40 packets in the internal FIFOs via the PRXQ[13:0] field of the ETH_MTLRXQxDR register.

With your patches applied:

Quoted 12 lines
root@tegra-ubuntu:~# iperf3 -c 192.168.248.1 -R
Connecting to host 192.168.248.1, port 5201
Reverse mode, remote host 192.168.248.1 is sending
[  5] local 192.168.248.174 port 43728 connected to 192.168.248.1 port 5201
[ ID] Interval           Transfer     Bitrate
[  5]   0.00-1.00   sec  30.3 MBytes   254 Mbits/sec
[  5]   1.00-2.00   sec  0.00 Bytes  0.00 bits/sec
[  5]   2.00-3.00   sec  0.00 Bytes  0.00 bits/sec
[  5]   3.00-4.00   sec  0.00 Bytes  0.00 bits/sec
[  5]   4.00-5.00   sec  0.00 Bytes  0.00 bits/sec
[  5]   5.00-6.00   sec  0.00 Bytes  0.00 bits/sec
...

Ah !!

I have been struggling with that problem this week too. I stumbled upon it while trying to test your TSO series, and at firts I thought it was because of the TSO patches, but turns out it's not, I reproduce it on net-next.

The main problem for me is that it's not always reproducible, it may or may not show up when I run iperf3 after a fresh restart.

This is on socfpga (dwmac1000), so it seems the problem exists across IP versions.

I've been on and off trying to make progress on that during the week, but without success so far...

Maxime