Re: [PATCH net-next v4 11/15] net: macb: change function signatures to take contexts

[Nicolai Buchwitz]

Hi Théo

On 17.7.2026 21:48, Théo Lebrun wrote:
> For parallel MACB context to start become a reality, many functions 
> need
> to stop operating on bp->ctx (the currently active context) and instead
> work on a context they get passed. That context might be
> (1) the new one that is getting allocated and initialised, or,
> (2) the old one to be freed.
>
> To reduce bug surface area, taint those functions to *only* take a
> context `struct macb_context *ctx` and no `struct macb *bp`. That way,
> no bug of using `bp->ctx` instead of `ctx` will ever occur.
>
> We also convert functions that take a `struct macb_queue *queue` to
> instead take `struct macb_context *ctx, unsigned int q`, with q
> indexing ctx->txq[] and ctx->rxq[].
>
> Full list:
>
>    macb_adj_dma_desc_idx()
>    macb_tx_ring_wrap()
>    macb_tx_desc()
>    macb_rx_ring_wrap()
>    macb_rx_desc()
>    macb_get_addr()
>    gem_rx_refill()
>    macb_init_rx_ring()
>    gem_free_rx_buffers()
>    macb_free_rx_buffers()
>    macb_tx_ring_size_per_queue()
>    macb_rx_ring_size_per_queue()
>    macb_free()
>    gem_alloc_rx_buffers()
>    macb_alloc_rx_buffers()
>    macb_alloc()
>    gem_init_rx_ring()
>    gem_init_rings()
>    macb_init_rings()
>
> Note about gem_rx_refill(): it ends with a netdev_vdbg() that prints 
> the
> queue pointer. Change to print the queue index because we do not have
> access to the queue anymore.
>
> Acked-by: Conor Dooley <conor.dooley@xxxxxxxxxxxxx>
> Signed-off-by: Théo Lebrun <theo.lebrun@xxxxxxxxxxx>
> ---
>  drivers/net/ethernet/cadence/macb.h      |   7 +-
>  drivers/net/ethernet/cadence/macb_main.c | 398 
> ++++++++++++++++---------------
>  2 files changed, 215 insertions(+), 190 deletions(-)
>
> diff --git a/drivers/net/ethernet/cadence/macb.h 
> b/drivers/net/ethernet/cadence/macb.h
> index c551d7db8ebe..ac2f2d8065d7 100644
> --- a/drivers/net/ethernet/cadence/macb.h
> +++ b/drivers/net/ethernet/cadence/macb.h
> @@ -1196,11 +1196,12 @@ static const struct gem_statistic 
> queue_statistics[] = {
>
>  struct macb;
>  struct macb_queue;
> +struct macb_context;
>
>  struct macb_or_gem_ops {
> -	int	(*mog_alloc_rx_buffers)(struct macb *bp);
> -	void	(*mog_free_rx_buffers)(struct macb *bp);
> -	void	(*mog_init_rings)(struct macb *bp);
> +	int	(*mog_alloc_rx_buffers)(struct macb_context *ctx);
> +	void	(*mog_free_rx_buffers)(struct macb_context *ctx);
> +	void	(*mog_init_rings)(struct macb_context *ctx);
>  	int	(*mog_rx)(struct macb_queue *queue, struct napi_struct *napi,
>  			  int budget);
>  };
> diff --git a/drivers/net/ethernet/cadence/macb_main.c 
> b/drivers/net/ethernet/cadence/macb_main.c
> index 7574418d5094..d396a307310b 100644
> --- a/drivers/net/ethernet/cadence/macb_main.c
> +++ b/drivers/net/ethernet/cadence/macb_main.c

> [...]

> @@ -5083,7 +5107,7 @@ static int at91ether_start(struct macb *bp)
>
>  	addr = rxq->buffers_dma;
>  	for (i = 0; i < AT91ETHER_MAX_RX_DESCR; i++) {
> -		desc = macb_rx_desc(queue, i);
> +		desc = macb_rx_desc(bp->ctx, 0, i);

AFAIU at91ether_open() doesn't set bp->ctx->info at this point in the
series, so with CONFIG_MACB_USE_HWSTAMP=y this should oops on ifup:

    macb_rx_desc()
      macb_adj_dma_desc_idx()
        macb_dma_ptp(ctx->info)   -> NULL deref

The next patch adds the missing assignment to at91ether_open(), so
only bisection is affected. Maybe move that line here or into patch 9?

> [...]

Thanks
Nicolai