> 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 > > 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. Hi Nicolai, ack, right. > > > Signed-off-by: Lorenzo Bianconi > > --- > > > [...] > > > --- 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? ack, I agree we should align stmmac_rx() and stmmac_rx_zc(). In order to avoid reposting the full series for this change, do you agree to address it as a follow-up patch? Regards, Lorenzo > > > [...] > > > @@ -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 > > Thanks, > Nicolai