Re: [PATCH net-next v4 07/15] net: macb: introduce macb_context struct for buffer management

[Nicolai Buchwitz]

Hi Théo

On 17.7.2026 21:48, 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() / macb_get_ringparam() do not access
>    bp->ctx->{rx,tx}_ring_size as they will/might run while interface is
>    offline and ctx is not NULL. Instead, introduce
>    bp->configured_{rx,tx}_ring_size which get updated on user requests.

nit: In the commit message: "ctx is not NULL" should be "ctx is NULL".

>  - 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_txq|rxq() functions to convert macb_queue
>    pointers.
>
>  - macb_get_regs() is tweaked to support being ran while interface is
>    offline (and context is NULL). Use default values at zero and
>    override them only if context is present.
>
> Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
> ---

> [...]

Reviewed-by: Nicolai Buchwitz <nb@xxxxxxxxxxx>

Thanks,
Nicolai