Hi, On Fri Jun 12 2026, Tjerk Kusters wrote: > Hi, > > The patch is attached > (0001-igb-only-strip-Rx-timestamp-header-on-the-first-buff.patch) > as my mail setup cannot send it inline via git send-email; apologies for the > attachment.
b4 has a web submission endpoint. Maybe you can use that one: https://b4.docs.kernel.org/en/latest/contributor/send.html [snip] > From fee3e3452dfcd7e109332369672a3e0090cadeb3 Mon Sep 17 00:00:00 2001 > From: T Kusters <[email protected]> > Date: Tue, 9 Jun 2026 14:06:24 +0200 > Subject: [PATCH net] igb: only strip Rx timestamp header on the first buffer > of a frame > > When Rx hardware timestamping is enabled (e.g. ptp4l, which configures > HWTSTAMP_FILTER_ALL), the NIC prepends a 16-byte timestamp header to the > first Rx buffer of every received frame. igb_clean_rx_irq() strips this > header inside its per-buffer loop: > > if (igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP)) { > ts_hdr_len = igb_ptp_rx_pktstamp(rx_ring->q_vector, > pktbuf, ×tamp); > pkt_offset += ts_hdr_len; > size -= ts_hdr_len; > } > > For a frame that spans more than one Rx buffer (e.g. a jumbo frame), this > block runs once per buffer. The timestamp header only exists at the start > of the first buffer, but igb_ptp_rx_pktstamp() is called for every buffer. > > On a continuation buffer the data is packet payload, not a timestamp > header. igb_ptp_rx_pktstamp() already has two guards against acting on a > non-header buffer: it returns 0 if PTP is disabled, and returns 0 if the > reserved dwords (the first 8 bytes) are non-zero. Neither is sufficient > here: PTP is enabled, and a continuation buffer whose payload happens to > begin with 8 zero bytes passes the reserved-dword check. In that case the > payload is mistaken for a valid timestamp header and igb_ptp_rx_pktstamp() > returns IGB_TS_HDR_LEN, so the caller strips 16 bytes of real data from > that buffer. A frame spanning N buffers whose continuation buffers start > with zero bytes therefore loses 16 * (N - 1) bytes from its tail. > > This is easily triggered by a GigE Vision camera streaming dark frames > (mostly 0x00 pixel data) over jumbo UDP with PTP active on the receiver: > the all-zero frames arrive truncated while frames with non-zero content > are fine. There is no error indication. > > No content-based check can reliably tell a continuation buffer that begins > with zero bytes from a real timestamp header, because both are all zero. > Fix it structurally instead: only attempt the strip on the first buffer of > a frame, which is the only buffer that can contain a timestamp header. In > igb_clean_rx_irq() skb is NULL until the first buffer has been processed, > so guarding the strip with !skb restricts it to the first buffer > regardless of payload content. > > Fixes: 5379260852b0 ("igb: Fix XDP with PTP enabled") > Cc: [email protected] > Signed-off-by: T Kusters <[email protected]> Great explanation! igb_clean_rx_irq_zc() does not need the same treatment, correct? Reviewed-by: Kurt Kanzenbach <[email protected]> > --- > drivers/net/ethernet/intel/igb/igb_main.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/intel/igb/igb_main.c > b/drivers/net/ethernet/intel/igb/igb_main.c > index ce91dda00ec0..abb55cd589a9 100644 > --- a/drivers/net/ethernet/intel/igb/igb_main.c > +++ b/drivers/net/ethernet/intel/igb/igb_main.c > @@ -9061,7 +9061,8 @@ static int igb_clean_rx_irq(struct igb_q_vector > *q_vector, const int budget) > pktbuf = page_address(rx_buffer->page) + rx_buffer->page_offset; > > /* pull rx packet timestamp if available and valid */ > - if (igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP)) { > + if (!skb && > + igb_test_staterr(rx_desc, E1000_RXDADV_STAT_TSIP)) { > int ts_hdr_len; > > ts_hdr_len = igb_ptp_rx_pktstamp(rx_ring->q_vector, > -- > 2.27.0 >
signature.asc
Description: PGP signature
