Re: [net-next,8/8] cadence: macb: introduce xmit support
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-01-19 19:36:27
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html --- cadence: macb: introduce xmit support This patch adds XDP_TX verdict support and introduces the ndo_xdp_xmit function for XDP redirection. It updates macb_tx_unmap() to handle both skbs and xdp frames.
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/cadence/macb_main.c b/drivers/net/ethernet/cadence/macb_main.c index afd8c0f2d895..32f8629bcb25 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -983,7 +994,7 @@ static void macb_tx_unmap(struct macb *bp, struct macb_tx_buff *tx_buff, } if (tx_buff->ptr) { - napi_consume_skb(tx_buff->ptr, budget); + release_buff(tx_buff->ptr, tx_buff->type, budget); tx_buff->ptr = NULL; } }
[ ... ]
+static int macb_xdp_submit_frame(struct macb *bp, struct xdp_frame *xdpf,
+ struct net_device *dev, bool dma_map,
+ dma_addr_t addr)
+{[ ... ]
+ if (dma_map) {
+ mapping = dma_map_single(&bp->pdev->dev,
+ xdpf->data,
+ xdpf->len, DMA_TO_DEVICE);
+ if (unlikely(dma_mapping_error(&bp->pdev->dev, mapping))) {
+ err = -ENOMEM;
+ goto unlock;
+ }
+ } else {
+ mapping = addr;
+ dma_sync_single_for_device(&bp->pdev->dev, mapping,
+ xdpf->len, DMA_BIDIRECTIONAL);
+ }[ ... ]
+ tx_buff->ptr = xdpf; + tx_buff->type = buff_type; + tx_buff->mapping = mapping;
^^^^^^^ For XDP_TX (dma_map=false), the mapping here is the page pool's DMA address passed via the addr parameter. Later, macb_tx_unmap() unconditionally calls dma_unmap_single() when tx_buff->mapping is non-zero. Should macb_tx_unmap() skip the DMA unmap for MACB_TYPE_XDP_TX buffers? Page pool manages its own DMA mappings, and calling dma_unmap_single() on them appears incorrect. For comparison, stmmac_tx_clean() explicitly checks "buf_type != STMMAC_TXBUF_T_XDP_TX" before unmapping. [ ... ]
+ case XDP_TX: + xdpf = xdp_convert_buff_to_frame(&xdp); + + if (!xdpf || macb_xdp_submit_frame(queue->bp, xdpf, dev, false, + addr)) + act = XDP_DROP; + goto out;
When xdp_convert_buff_to_frame() returns NULL, this sets act = XDP_DROP but then unconditionally executes "goto out", which skips the page_pool_put_full_page() call that handles XDP_DROP. Could this leak the page when xdp_convert_buff_to_frame() fails due to insufficient headroom? When macb_xdp_submit_frame() fails, it internally calls release_buff() which returns the page, so that path seems fine. But the xdpf == NULL case appears to need a "break" instead of falling through to "goto out".