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

Nicolai Buchwitz
2026-04-02T11:22:07Z

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