Re: [PATCH 2/3] net: macb: propagate RX ring refill errors

[Nicolai Buchwitz]

Hi Théo

On 18.9.2026 22:32, Théo Lebrun wrote:
> gem_rx_refill() is responsible for Rx SKB allocation, including at 
> open,
> but its prototype indicates a void return value.
>
> Therefore we change the code to propagate allocation and DMA mapping
> errors back up the stack, making sure the open fails if it occurs.
> Change all those to return errno-style ints:
>  - gem_rx_refill()
>  - its parent gem_init_rx_ring()
>  - its grand-parent gem_init_rings()
>  - the macbgem_ops.mog_init_rings function pointer
>  - its grand-uncle macb_init_rings()
>
> Theoretical bugfix, never encountered in practice. To reproduce,
> introduce memory pressure (less than 512 SKBs of free memory) and open
> the interface. I expect the last queue to be unuseable because it has
> zero usable rx buffers. Nothing will ever trigger a refill on that
> queue which only happens once a frame has been received.
>
> Note that other callers of refill (resume, HRESP error task, NAPI)
> cannot do anything useful with that error and keep their best-effort
> refill, hoping it will improve.
>
> Fixes: 4df95131ea80 ("net/macb: change RX path for GEM")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
> ---

> [...]

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

> [...]

>  	for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> @@ -2813,11 +2820,15 @@ static void gem_init_rings(struct macb *bp)
>  		queue->tx_head = 0;
>  		queue->tx_tail = 0;
>
> -		gem_init_rx_ring(queue);
> +		err = gem_init_rx_ring(queue);
> +		if (err)
> +			last_err = err;

A partly filled queue refills itself on the next gem_rx().  AFAIU only a
queue with zero buffers can get stuck. Fail the open just for that case?

> [...]

>  static void macb_reset_hw(struct macb *bp)
> @@ -3162,7 +3175,9 @@ static int macb_open(struct net_device *netdev)
>  		goto pm_exit;
>  	}
>
> -	bp->macbgem_ops.mog_init_rings(bp);
> +	err = bp->macbgem_ops.mog_init_rings(bp);
> +	if (err)
> +		goto free_rings;

macb_set_ringparam() ignores macb_open()'s error, so NAPI stays disabled
with netif_running() == true and the next close will hang in 
napi_disable().

Should macb_set_ringparam() return the error?

> [...]

Thanks,
Nicolai