Re: [v6,net-next,15/15] net: fec: add AF_XDP zero-copy support
From: Jakub Kicinski <kuba@kernel.org>
Date: 2026-02-05 05:28:41
Also in:
bpf, imx, lkml
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 --- net: fec: add AF_XDP zero-copy support This commit adds AF_XDP zero-copy support for both TX and RX on the FEC Ethernet driver. It introduces new functions for XSK buffer allocation, RX/TX queue processing in zero-copy mode, and XSK pool setup/teardown.
For TX, there are two cases, one is the frames from the AF_XDP socket, so fec_enet_xsk_xmit() is added to directly transmit the frames from the socket and the buffer type is marked as FEC_TXBUF_T_XSK_XMIT. The other one is the frams from the RX queue (XDP_TX action), the buffer
This isn't a bug, but "frams" should be "frames".
quoted hunk ↗ jump to hunk
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c --- a/drivers/net/ethernet/freescale/fec_main.c +++ b/drivers/net/ethernet/freescale/fec_main.c
[ ... ]
+static int fec_enet_xsk_tx_xmit(struct fec_enet_private *fep,
+ struct xdp_buff *xsk, int cpu,
+ int queue)
+{[ ... ]
+ bdp->cbd_bufaddr = cpu_to_fec32(dma);
+ bdp->cbd_datlen = cpu_to_fec16(len);
+
+ if (fep->bufdesc_ex) {
+ struct bufdesc_ex *ebdp = (struct bufdesc_ex *)bdp;
+
+ if (fep->quirks & FEC_QUIRK_HAS_AVB)
+ estatus |= FEC_TX_BD_FTYPE(txq->bd.qid);
+
+ ebdp->cbd_bdu = 0;
+ ebdp->cbd_esc = cpu_to_fec32(estatus);
+ }
+
+ status |= (BD_ENET_TX_READY | BD_ENET_TX_TC);
+ bdp->cbd_sc = cpu_to_fec16(status);
+ dma_wmb();Is there a missing dma_wmb() before setting BD_ENET_TX_READY? The descriptor fields (cbd_bufaddr, cbd_datlen, cbd_esc) are written before the status, but without a memory barrier, the hardware could see the READY bit before those fields are visible on weakly-ordered architectures. Compare to fec_enet_xsk_xmit() in the same file which does: dma_wmb(); status |= BD_ENET_TX_READY | BD_ENET_TX_TC; bdp->cbd_sc = cpu_to_fec16(status); dma_wmb(); The dma_wmb() after setting status only ensures ordering with subsequent writes (bd.cur update), not with the preceding descriptor field writes.