>-----Original Message-----
>From: Intel-wired-lan <[email protected]> On Behalf Of Tjerk
>Kusters via B4 Relay
>Sent: Friday, June 19, 2026 9:15 AM
>To: Nguyen, Anthony L <[email protected]>; Kitszel, Przemyslaw
><[email protected]>; Andrew Lunn <[email protected]>; David S.
>Miller <[email protected]>; Eric Dumazet <[email protected]>; Jakub
>Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Richard Cochran
><[email protected]>; Jesper Dangaard Brouer <[email protected]>; Kurt
>Kanzenbach <[email protected]>
>Cc: [email protected]; [email protected];
>[email protected]; [email protected]; Tjerk Kusters
><[email protected]>
>Subject: [Intel-wired-lan] [PATCH net v2] igb: only strip Rx timestamp header
>on the first buffer of a frame
>
>From: Tjerk Kusters <[email protected]>
>
>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]
>Reviewed-by: Kurt Kanzenbach <[email protected]>
>Signed-off-by: Tjerk Kusters <[email protected]>
>---
>Changes in v2:
> - resend via b4 (v1 was sent with a mail client)
> - use full author name "Tjerk Kusters" (Jacob Keller)
> - add Reviewed-by from Kurt Kanzenbach
> - no functional change
>
>Link to v1:
>https://lore.kernel.org/all/pawpr05mb1069106d52f4e17f1edb99c67b9...@pawpr05mb10691.eurprd05.prod.outlook.com/
>---
> 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 */
Is this comment up-to-date now ?
Reviewed-by: Piotr Kwapulinski <[email protected]>
>- 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,
>
>---
>base-commit: 2d3090a8aeb596a26935db0955d46c9a5db5c6ce
>change-id: 20260619-igb-rx-ts-fix-cd70585ee316
>
>Best regards,
>--
>Tjerk Kusters <[email protected]>
>
>
