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

On Sun Jul 19, 2026 at 12:46 PM CEST, Nicolai Buchwitz wrote:

On 17.7.2026 21:48, Théo Lebrun wrote:

quoted

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@microchip.com>
Signed-off-by: Théo Lebrun <theo.lebrun@bootlin.com>
---
 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

quoted

[...]

quoted

@@ -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?

Yes, IDK why/how I landed on this.

⟩ # This looks much more sensible.
⟩ git log -L:at91ether_open:drivers/net/ethernet/cadence/macb_main.c net-next/main..macb-context

net: macb: unify device pointer naming convention
net: macb: unify variable naming convention in at91ether functions
=> variable renames

net: macb: introduce macb_context struct for buffer management
=> adds allocation with `bp->ctx = kzalloc_obj(...)` and cleanup on error

net: macb: make `struct macb` subset reachable from macb_context struct
=> adds `bp->ctx->info = &bp->info;` when introducing info

Thanks,

--
Théo Lebrun, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com