Re: [PATCH net-next 06/11] net: macb: introduce macb_context struct for buffer management

[Nicolai Buchwitz <nb@tipi-net.de>]

On 1.4.2026 18:39, Théo Lebrun wrote:
> Whenever an operation requires buffer realloc, we close the interface,
> update parameters and reopen. To improve reliability under memory
> pressure, we should rather alloc new buffers, reconfigure HW and free
> old buffers. This requires MACB to support having multiple "contexts"
> in parallel.
> 
> Introduce this concept by adding the macb_context struct, which owns 
> all
> queue buffers and the parameters associated. We do not yet support
> multiple contexts in parallel, because all functions access bp->ctx
> (the currently active context) directly.
> 
> Steps:
> 
>  - Introduce `struct macb_context` and its children `struct macb_rxq`
>    and `struct macb_txq`. Context fields are stolen from `struct macb`
>    and rxq/txq fields are from `struct macb_queue`.
> 
>    Making it two separate structs per queue simplifies accesses: we 
> grab
>    a txq/rxq local variable and access fields like txq->head instead of
>    queue->tx_head. It also anecdotally improves data locality.
> 
>  - macb_init_dflt() does not set bp->ctx->{rx,tx}_ring_size to default
>    values as ctx is not allocated yet. Instead, introduce
>    bp->configured_{rx,tx}_ring_size which get updated on user requests.
> 
>  - macb_open() starts by allocating bp->ctx. It gets freed in the
>    open error codepath or by macb_close().
> 
>  - Guided by compile errors, update all codepaths. Most diff is 
> changing
>    `queue->tx_*` to `txq->*` and `queue->rx_*` to `rxq->*`, with a new
>    local variable. Also rx_buffer_size / rx_ring_size / tx_ring_size
>    move from bp to bp->ctx.
> 
>    Introduce two helpers macb_tx|rx() functions to convert macb_queue
>    pointers.
> 
> Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
> ---
>  drivers/net/ethernet/cadence/macb.h      |  49 ++--
>  drivers/net/ethernet/cadence/macb_main.c | 442 
> ++++++++++++++++++-------------
>  2 files changed, 296 insertions(+), 195 deletions(-)
> 

> [...]

> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index d5023fdc0756..0f63d9b89c11 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c

> [...]

> @@ -3596,14 +3677,15 @@ static void macb_get_regs(struct net_device 
> *netdev, struct ethtool_regs *regs,
>  			  void *p)
>  {
>  	struct macb *bp = netdev_priv(netdev);
> +	struct macb_txq *txq = &bp->ctx->txq[0];

bp->ctx is NULL when the interface is down. This will crash if
ethtool -d is called while the interface is not running. Same
issue below in macb_get_ringparam().

>  	unsigned int tail, head;
>  	u32 *regs_buff = p;
> 
>  	regs->version = (macb_readl(bp, MID) & ((1 << MACB_REV_SIZE) - 1))
>  			| MACB_GREGS_VERSION;
> 
> -	tail = macb_tx_ring_wrap(bp, bp->queues[0].tx_tail);
> -	head = macb_tx_ring_wrap(bp, bp->queues[0].tx_head);
> +	tail = macb_tx_ring_wrap(bp, txq->tail);
> +	head = macb_tx_ring_wrap(bp, txq->head);
> 
>  	regs_buff[0]  = macb_readl(bp, NCR);
>  	regs_buff[1]  = macb_or_gem_readl(bp, NCFGR);
> @@ -3682,8 +3764,8 @@ static void macb_get_ringparam(struct net_device 
> *netdev,
>  	ring->rx_max_pending = MAX_RX_RING_SIZE;
>  	ring->tx_max_pending = MAX_TX_RING_SIZE;
> 
> -	ring->rx_pending = bp->rx_ring_size;
> -	ring->tx_pending = bp->tx_ring_size;
> +	ring->rx_pending = bp->ctx->rx_ring_size;
> +	ring->tx_pending = bp->ctx->tx_ring_size;

Same NULL ctx issue as above. This one could just read from
bp->configured_{rx,tx}_ring_size instead.

> [...]

Thanks
Nicolai