Re: [PATCH net-next v6 7/7] net: macb: introduce ndo_xdp_xmit support
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-03-29 19:47:53
This is an AI-generated review of your patch. The human sending this email has considered the AI review valid, or at least plausible. --- net: macb: introduce ndo_xdp_xmit support This commit introduces ndo_xdp_xmit support for the macb driver to enable XDP redirection. It adds the gem_xdp_xmit function and updates macb_xdp_submit_frame to handle DMA mapping for redirected 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 537d02264851..38b15d1e7127 100644 --- a/drivers/net/ethernet/cadence/macb_main.c +++ b/drivers/net/ethernet/cadence/macb_main.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1708,8 +1723,8 @@ static int macb_xdp_submit_frame(struct macb *bp, struct xdp_frame *xdpf, desc = macb_tx_desc(queue, queue->tx_head); tx_buff = macb_tx_buff(queue, queue->tx_head); tx_buff->ptr = xdpf; - tx_buff->type = MACB_TYPE_XDP_TX; - tx_buff->mapping = 0; + tx_buff->type = buff_type; + tx_buff->mapping = dma_map ? mapping : 0; tx_buff->size = xdpf->len;
Is a bounds check needed on the frame length here? Since ndo_xdp_xmit can receive frames redirected from other devices, the frame length could potentially be up to ~64KB on architectures with 64KB pages. macb_xdp_submit_frame() casts this length to a 32-bit integer and bitwise-ORs it into the descriptor's control word, which has a 14-bit length field (maximum 16383 bytes). Frame lengths exceeding this limit could silently overflow into adjacent control bits like TX_LAST, corrupting the descriptor ring. [ ... ]
+static int gem_xdp_xmit(struct net_device *dev, int num_frame,
+ struct xdp_frame **frames, u32 flags)
+{
+ struct macb *bp = netdev_priv(dev);
+ u32 xmitted = 0;
+ int i;
+
+ if (!macb_is_gem(bp))
+ return -EOPNOTSUPP;
+
+ if (unlikely(!netif_carrier_ok(dev)))
+ return -ENETDOWN;
+
+ if (unlikely(flags & ~XDP_XMIT_FLAGS_MASK))
+ return -EINVAL;
+
+ for (i = 0; i < num_frame; i++) {
+ if (macb_xdp_submit_frame(bp, frames[i], dev, true, 0))
+ break;Should this loop batch the doorbell rings based on the XDP_XMIT_FLUSH flag? Currently, the code calls macb_xdp_submit_frame() for each frame, which independently acquires the global bp->lock and performs an MMIO write to the NCR register to ring the hardware doorbell per packet. Doing this per packet ignores the batching semantics of the bulk transmission API and could cause severe lock contention and MMIO overhead.
+ + xmitted++; + } + + return xmitted; +}