Re: [PATCH net 1/3] net: macb: never give hardware a NULL RX buffer

Hello netdev-bot+sashiko,

On Tue Sep 22, 2026 at 10:37 PM CEST,  wrote:

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider.

Critical: 0 · High: 2 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] gem_init_rx_ring()
  (drivers/net/ethernet/cadence/macb_main.c:2793) now sets…
- [High] gem_init_rx_ring() now publishes MACB_BIT(RX_USED) on every RX
  descriptor of a queue, resets queue->rx_tail/rx_prepared_head and then…

--

net: macb: never give hardware a NULL RX buffer

This makes gem_init_rx_ring() set the RX_USED bit on every RX descriptor
before calling gem_rx_refill(), so that a refill that fails part way
through does not leave descriptors owned by hardware while their skb
pointers are still NULL.  The change affects macb_open(), macb_resume(),
the HRESP error recovery path and set_ringparam().

quoted

diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
index b8234ac4b602..751fa9e68099 100644
--- a/drivers/net/ethernet/cadence/macb_main.c
+++ b/drivers/net/ethernet/cadence/macb_main.c

@@ -2785,9 +2785,14 @@ static int macb_alloc(struct macb *bp)
 
 static void gem_init_rx_ring(struct macb_queue *queue)
 {
+	unsigned int i;
+
 	queue->rx_tail = 0;
 	queue->rx_prepared_head = 0;
 
+	for (i = 0; i < queue->bp->rx_ring_size; i++)
+		macb_rx_desc(queue, i)->addr |= MACB_BIT(RX_USED);
+

[Severity: High]

Does RX_USED now carry two different meanings that the RX consume path
cannot tell apart?

gem_rx() uses RX_USED at queue->rx_tail as the only indication that
hardware delivered a frame, and it has no bound against
queue->rx_prepared_head:

Yes that is correct! We can fix that using our cursors
rx_tail/rx_prepared_head that tell us which descriptors were
reallocated properly.

IE in gem_rx() we stop once `queue->rx_tail == queue->rx_prepared_head`.

[...]

[Severity: High]

Is any exclusion against a running RX poll needed here now that the whole
ring is published as RX_USED?

macb_hresp_error_task() runs from system_bh_wq, masks queue interrupts,
clears RE/TE, and then calls mog_init_rings() with no napi_disable(), no
napi_synchronize() and no lock:

This is a known race, but we won't fix it in this series. And in
particular it is completely unrelated to this first patch.

It is semi-related to the last patch that puts in place the mechanisms
that could be reused to fix macb_hresp_error_task(). But again, not
here, not today.

[...]

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com