> Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Medium] net: stmmac: missing drop accounting when XDP fragment addition fails > -- > > commit ec049e650a3e45a0def65bd264e60d42d99ff7f9 > Author: Lorenzo Bianconi > > net: stmmac: rework stmmac_rx to support XDP rx multi-buff > > This commit builds the xdp_buff by accumulating descriptors so the XDP > program runs on the full packet. It saves the state in rx_q->state across > NAPI poll boundaries and builds the skb from the head buffer and fragments. > > Link: https://lore.kernel.org/r/20260918-stmmac-rx-mb-v1-1-0b4517d404af@oss.qualcomm.com > > > diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > index bf9e7e4cb1c31..7d1149511c55d 100644 > > --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c > > [ ... ] > > > @@ -5866,6 +5932,33 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) > > xdp_init_buff(&ctx.xdp, bufsz, &rx_q->xdp_rxq); > > xdp_prepare_buff(&ctx.xdp, page_address(buf->page), > > buf->page_offset, buf1_len, true); > > + first_desc = false; > > + buf->page = NULL; > > + } else if (buf1_len) { > > + error |= !stmmac_build_xdp_frags(priv, rx_q, buf1_len, > > + buf->page, > > + buf->page_offset, > > + dma_dir, &ctx.xdp); > > [Severity: Medium] > When stmmac_build_xdp_frags() returns false, the error flag is set. > > This could happen when receiving large frames (e.g. Jumbo frames) on hardware > configured with small DMA buffers, leading to many descriptors per frame. > > Will this cause the packet to be silently dropped without updating > rx_dropped or rx_errors? ack, I will fix it in v3. > > > + buf->page = NULL; > > + } > > + > > + if (buf2_len) { > > + error |= !stmmac_build_xdp_frags(priv, rx_q, buf2_len, > > + buf->sec_page, > > + buf->page_offset, > > + dma_dir, &ctx.xdp); > > + buf->sec_page = NULL; > > + } > > + > > + if (likely(status & rx_not_ls)) > > + goto read_again; > > + > > + if (unlikely(error)) > > + goto error_free_frag; > > [ ... ] > > > @@ -5989,13 +6036,24 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) > > rx_packets++; > > rx_bytes += len; > > count++; > > + continue; > > +error_free_frag: > > + if (!first_desc) { > > + stmmac_xdp_put_buff(rx_q, &ctx.xdp, -1); > > + first_desc = true; > > + } > > + dev_kfree_skb(skb); > > + skb = NULL; > > + count++; > > [Severity: Medium] > If we reach this teardown path due to an error, the frame is consumed and > the polling count is incremented, but no interface drop statistics appear to > be updated. > > Could this make debugging MTU or fragmentation issues difficult, since drops > (such as exceeding MAX_SKB_FRAGS) would be invisible to monitoring? Should > rx_dropped or rx_errors be incremented here? ack, I will fix it in v3. Regards, Lorenzo > > > } > > -- > Sashiko AI review · https://sashiko.dev/#/patchset/20260921-stmmac-rx-mb-v2-1-6e826e1ff306@oss.qualcomm.com?part=1