On Tuesday, October 13th, 2026 at 8:40 PM, Paul Moses <[email protected]> wrote:
>
>
>
>
> On Tuesday, October 13th, 2026 at 8:10 PM, [email protected]
> <[email protected]> wrote:
>
> > Thank you for your contribution! Sashiko AI review found 2 potential
> > issue(s) to consider:
> > - [High] igc: Frame Preemption express packets (mPackets) lose TSIP
> > stripping when interleaved
> > - [High] igc: incomplete TSIP underflow fix due to XDP fragment drop
> > --
> >
> > commit 61d038c91b93b927335a69df60dbf6cc817c79e2
> > Author: Paul Moses <[email protected]>
> > igc: only strip RX timestamp header from first buffer
> >
> > This patch modifies the RX path to only subtract the 16-byte TSIP inline
> > timestamp from the first buffer of an skb-assembled packet. This is intended
> > to prevent an unsigned length underflow on continuation buffers shorter than
> > 16 bytes.
> >
> > > 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]
> > Does this condition corrupt interleaved express packets?
> >
> > Frame Preemption allows express packets (mPackets) to be interleaved in the
> > RX ring while a preemptable packet is being assembled. When an mPacket
> > arrives during this assembly, skb will be non-NULL.
> >
> > Because of the new !skb check, the mPacket's TSIP header won't be stripped,
> > leaving pkt_offset at 0 and size unmodified.
> >
> > Shortly after this check in igc_clean_rx_irq(), the mPacket is handled:
> >
> > if (igc_fpe_is_pmac_enabled(adapter) &&
> > igc_fpe_handle_mpacket(adapter, rx_desc, size, pktbuf +
> > pkt_offset)) {
> > /* Advance the ring next-to-clean */
> > igc_is_non_eop(rx_ring, rx_desc);
> > ...
> >
> > Will this pass the unadjusted size and unstripped TSIP header (pktbuf + 0)
> > into igc_fpe_handle_mpacket(), treating the 16-byte hardware timestamp as
> > packet payload?
>
> It'll drop, not do that, but I'll fix in this series.
>
>
> >
> > [Severity: High]
> > Does this fix fully prevent the continuation fragment underflow?
> >
> > The patch attempts to fix a queue stall caused by an integer underflow on
> > continuation fragments, but relies on skb being non-NULL to detect them.
> >
> > If XDP is enabled and the administrator has enabled the rx-all feature
> > (disabling MTU enforcement), an oversized packet can be split across
> > multiple
> > descriptors. If the XDP program drops the first descriptor (e.g., returning
> > XDP_DROP), skb remains NULL.
> >
> > When the loop hits igc_is_non_eop() and continues to the next descriptor,
> > skb will still be NULL. If that continuation fragment is shorter than
> > 16 bytes and has the TSIP status bit set, won't this check evaluate to true
> > and cause size to underflow exactly as it did before the patch?
> >
>
> No, xdp was still no bueno, but I've had some breakthroughs already today and
> this is all mostly finished, I'll submit v3 series shortly.
>
> Thanks,
> Paul
>
>
Is this list private? Can I openly discuss remaining security matters?
