[PATCH net-next v8 2/5] net: stmmac: drop split frames on AF_XDP zero-copy RX
Nicolai Buchwitz
nb at tipi-net.de
Mon Oct 19 04:16:30 PDT 2026
Hi Lorenzo
On 18.10.2026 19:07, Lorenzo Bianconi wrote:
> stmmac_rx_zc() assumes a 1:1 mapping between a received frame and an
> XSK pool buffer and drops any frame that spans more than one RX
> descriptor:
>
> /* XSK pool expects RX frame 1:1 mapped to XSK buffer */
> if (likely(status & rx_not_ls)) {
> xsk_buff_free(buf->xdp);
> buf->xdp = NULL;
> dirty++;
> count++;
> goto read_again;
> }
>
> The non-last descriptors are freed and dropped, but the frame is not
> marked as errored. When the poll reaches the last descriptor of the
> same frame, error is still 0, so the fragment is handled as a complete
> frame: it is passed to the XDP program, copied into an skb on XDP_PASS,
> or converted for XDP_TX. The tail starts in the middle of the original
> payload at a predictable offset and, parsed as a fresh frame (len ==
> 0),
> stmmac_rx_buf1_len() reports the full dma_buf_sz length for it even
> though the DMA engine only wrote the tail remainder into that buffer,
> so
> the reads extend past the received bytes and can reach beyond the end
> of
> the XSK pool buffer. GMAC4 always enables jumbo frames (GMAC_CONFIG_JE
> in GMAC_CORE_INIT), so an on-link peer can trigger such a split even
> when the interface MTU is left at 1500.
>
> Set error when the 1:1 mapping check drops a descriptor, so the
> remaining descriptors of the frame - including the last one - are
> drained and released with xsk_buff_free() instead of being processed as
> a new frame. Account every freed buffer with dirty and count the
> dropped frame once in the poll budget / rx_pkt_n. The direct 1:1
> mapping drop also increments rx_dropped.
>
> Normalize the legacy normal and enhanced descriptor callbacks to report
> rx_not_ls together with discard_frame for non-last descriptors. This
> allows both stmmac_rx_zc() and stmmac_rx() to drain split frames on
> dwmac100 and dwmac1000 instead of reparsing the final fragment as a new
> packet.
>
> Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy")
> Reviewed-by: Nicolai Buchwitz <nb at tipi-net.de>
This grew quite a bit since v6 (legacy desc changes, stmmac_rx() etc),
so my R-b should have been dropped. I had another look and two small
notes below.
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at oss.qualcomm.com>
> ---
> [...]
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -5695,18 +5695,21 @@ static int stmmac_rx_zc(struct stmmac_priv
> *priv, int limit, u32 queue)
> if (priv->extend_desc)
> stmmac_rx_extended_status(priv, &priv->xstats,
> rx_q->dma_erx + entry);
> - if (unlikely(status == discard_frame)) {
> - xsk_buff_free(buf->xdp);
> - buf->xdp = NULL;
> - dirty++;
> +
> + if (unlikely(status & discard_frame)) {
> error = 1;
> if (!priv->hwts_rx_en)
> rx_errors++;
> }
There's no !error check here, so on the legacy descriptors a split
frame now counts once per descriptor in rx_errors. stmmac_rx() in 3/5
counts it once. Could you add the !error check here too?
> [...]
> @@ -5883,7 +5887,8 @@ static int stmmac_rx(struct stmmac_priv *priv,
> int limit, u32 queue)
>
> if (priv->extend_desc)
> stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx +
> entry);
> - if (unlikely(status == discard_frame)) {
> +
> + if (unlikely(status & discard_frame)) {
AFAIU this also changes GMAC4: a frame with an error plus an IP header
error returns discard_frame | csum_none, which the old "==" let
through. Dropping it is right, but maybe worth a line in the commit
message?
> [...]
With the !error check added:
Reviewed-by: Nicolai Buchwitz <nb at tipi-net.de>
Thanks,
Nicolai
More information about the linux-arm-kernel
mailing list