Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider.
Critical: 0 · High: 1 · Medium: 0 · Low: 0 If you believe none of the issues are legitimate, please reply to each issue of Medium severity or higher and explain why the AI is wrong. If any of them are legitimate and you plan to respin, please let patchwork know by including "pw-bot: cr" as a separate line at the end of your reply (one such reply per series is enough). - [High] At this commit by itself, the new first-buffer test `!skb && igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP)` in igc_clean_rx_irq()… -- igc: only strip RX timestamp header from first buffer igc_clean_rx_irq() used to subtract IGC_TS_HDR_LEN from every descriptor with TSIP set, which truncated multi-buffer packets. With this patch the timestamp header is only stripped when skb is NULL, which the commit treats as marking the first buffer of a new packet. Link: https://lore.kernel.org/all/[email protected]/ > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c > b/drivers/net/ethernet/intel/igc/igc_main.c > index 1fb5f3cbe93c5..41b293dc10c3a 100644 > --- a/drivers/net/ethernet/intel/igc/igc_main.c > +++ b/drivers/net/ethernet/intel/igc/igc_main.c > @@ -2639,7 +2639,8 @@ static int igc_clean_rx_irq(struct igc_q_vector > *q_vector, const int budget) > > pktbuf = page_address(rx_buffer->page) + rx_buffer->page_offset; > > - if (igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP)) { > + if (!skb && > + igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP)) { > ctx.rx_ts = pktbuf; > pkt_offset = IGC_TS_HDR_LEN; > size -= IGC_TS_HDR_LEN; [Severity: High] Is skb == NULL a reliable first-buffer marker when an XDP program is attached? The commit message says: Only consume the timestamp header when skb is NULL, which identifies the first buffer of a new packet. Suppose igc_xdp_run_prog() returns IGC_XDP_CONSUMED, IGC_XDP_TX or IGC_XDP_REDIRECT for the first, non-EOP buffer of a multi-descriptor frame. Then no skb is built, and igc_clean_rx_irq() moves on to the continuation descriptor with skb still NULL: /* fetch next buffer in frame if non-eop */ if (igc_is_non_eop(rx_ring, rx_desc)) continue; The continuation descriptor still has TSIP set, so this check strips IGC_TS_HDR_LEN a second time. If that descriptor holds fewer than 16 bytes, can the unsigned size wrap here? The continuation buffer then goes through the new-frame path: if (!skb) { xdp_init_buff(&ctx.xdp, truesize, &rx_ring->xdp_rxq); xdp_prepare_buff(&ctx.xdp, pktbuf - igc_rx_offset(rx_ring), igc_rx_offset(rx_ring) + pkt_offset, size, true); In that case the XDP program would see a data_end about 4 GiB past data. On XDP_PASS, the same size would reach igc_build_skb() or igc_construct_skb(), and the __skb_put() there does not check it. At this commit, XDP with multi-descriptor frames looks possible on kernels with PAGE_SIZE >= 8192. igc_set_rx_mode() only clamps RLPML under the small-page #if: #if (PAGE_SIZE < 8192) if (adapter->max_frame_size <= IGC_MAX_FRAME_BUILD_SKB) rlpml = IGC_MAX_FRAME_BUILD_SKB; #endif wr32(IGC_RLPML, rlpml); On large-page kernels, RLPML therefore stays at MAX_JUMBO_FRAME_SIZE while igc_rx_bufsz() returns IGC_RXBUFFER_2048. igc_xdp_set_prog() only checks dev->mtu > ETH_DATA_LEN, not the hardware receive length limit. This needs RX hardware timestamping enabled, an XDP program that returns a non-PASS verdict for a first buffer, and a peer that sends frames larger than about 2032 bytes. The next patch in this series, "igc: fix AF_XDP receive buffer size for 2K UMEM chunks", adds igc_set_rlpml(). It caps RLPML at 1526 bytes whenever XDP is enabled, whatever PAGE_SIZE is. It is called from igc_setup_rctl() and igc_xdp_set_prog(). With that change, XDP frames always fit in a single descriptor, and skb == NULL becomes a correct first-buffer marker. This patch is tagged Cc: stable and could be backported on its own. Should it depend on that follow-up change, or should the two patches be swapped in the series? -- Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261028083344.3047350-1-p%401g4.org
