Re: [PATCH net-next v2 13/14] net: macb: use context swapping in .set_ringparam()

Jakub Kicinski
2026-04-14T00:50:49Z

On Fri, 10 Apr 2026 21:52:01 +0200 Théo Lebrun wrote:
> ethtool_ops.set_ringparam() is implemented using the primitive close /
> update ring size / reopen sequence. Under memory pressure this does not
> fly: we free our buffers at close and cannot reallocate new ones at
> open. Also, it triggers a slow PHY reinit.
>
> Instead, exploit the new context mechanism and improve our sequence to:
> - allocate a new context (including buffers) first
> - if it fails, early return without any impact to the interface
> - stop interface
> - update global state (bp, netdev, etc)
> - pass buffer pointers to the hardware
> - start interface
> - free old context.
>
> The HW disable sequence is inspired by macb_reset_hw() but avoids
> (1) setting NCR bit CLRSTAT and (2) clearing register PBUFRXCUT.
>
> The HW re-enable sequence is inspired by macb_mac_link_up(), skipping
> over register writes which would be redundant (because values have not
> changed).
>
> The generic context swapping parts are isolated into helper functions
> macb_context_swap_start|end(), reusable by other operations (change_mtu,
> set_channels, etc).

> diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c
> index 81beb67b206a..340ae7d881c6 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c
> @@ -3081,6 +3081,89 @@ static void macb_configure_dma(struct macb *bp)
> }
> }
>
> +static void macb_context_swap_start(struct macb *bp)
> +{
> + struct macb_queue *queue;
> + unsigned long flags;
> + unsigned int q;
> + u32 ctrl;
> +
> + /* Disable software Tx, disable HW Tx/Rx and disable NAPI. */
> +
> + netif_tx_disable(bp->netdev);

AFAIR netif_tx_disable() just stops all the queues, if the NAPIs and
whatever else may wake queues is still running the queues may get
restarted right away.

> + spin_lock_irqsave(&bp->lock, flags);
> +
> + ctrl = macb_readl(bp, NCR);
> + macb_writel(bp, NCR, ctrl & ~(MACB_BIT(RE) | MACB_BIT(TE)));
> +
> + macb_writel(bp, TSR, -1);
> + macb_writel(bp, RSR, -1);
> +
> + for (q = 0, queue = bp->queues; q < bp->num_queues; ++q, ++queue) {
> + queue_writel(queue, IDR, -1);
> + queue_readl(queue, ISR);
> + if (bp->caps & MACB_CAPS_ISR_CLEAR_ON_WRITE)
> + queue_writel(queue, ISR, -1);
> + }
> +