[PATCH net v2] igb: only strip Rx timestamp header on the first buffer of a frame

[PATCH net v2] igb: only strip Rx timestamp header on the first buffer of a frame

[Tjerk Kusters via B4 Relay]

From: Tjerk Kusters <tkusters@aweta.nl>

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, &timestamp);
		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: stable@vger.kernel.org
Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
Signed-off-by: Tjerk Kusters <tkusters@aweta.nl>
---
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/PAWPR05MB1069106D52F4E17F1EDB99C67B9182@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 */
-		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 <tkusters@aweta.nl>

RE: [Intel-wired-lan] [PATCH net v2] igb: only strip Rx timestamp header on the first buffer of a frame

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf
> Of Tjerk Kusters via B4 Relay
> Sent: Friday, June 19, 2026 9:15 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel,
> Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn
> <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric
> Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo
> Abeni <pabeni@redhat.com>; Richard Cochran <richardcochran@gmail.com>;
> Jesper Dangaard Brouer <hawk@kernel.org>; Kurt Kanzenbach
> <kurt@linutronix.de>
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; stable@vger.kernel.org; Tjerk Kusters
> <tkusters@aweta.nl>
> Subject: [Intel-wired-lan] [PATCH net v2] igb: only strip Rx timestamp
> header on the first buffer of a frame
> 
> From: Tjerk Kusters <tkusters@aweta.nl>
> 
> 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, &timestamp);
> 		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: stable@vger.kernel.org
> Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
> Signed-off-by: Tjerk Kusters <tkusters@aweta.nl>
> ---
> 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/PAWPR05MB1069106D52F4E17F1EDB99C67B9182@PA
> WPR05MB10691.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 */
> -		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 <tkusters@aweta.nl>
> 

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

RE: [Intel-wired-lan] [PATCH net v2] igb: only strip Rx timestamp header on the first buffer of a frame

[Kwapulinski, Piotr]

>-----Original Message-----
>From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Tjerk Kusters via B4 Relay
>Sent: Friday, June 19, 2026 9:15 AM
>To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Richard Cochran <richardcochran@gmail.com>; Jesper Dangaard Brouer <hawk@kernel.org>; Kurt Kanzenbach <kurt@linutronix.de>
>Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; stable@vger.kernel.org; Tjerk Kusters <tkusters@aweta.nl>
>Subject: [Intel-wired-lan] [PATCH net v2] igb: only strip Rx timestamp header on the first buffer of a frame
>
>From: Tjerk Kusters <tkusters@aweta.nl>
>
>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, &timestamp);
>		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: stable@vger.kernel.org
>Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
>Signed-off-by: Tjerk Kusters <tkusters@aweta.nl>
>---
>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/PAWPR05MB1069106D52F4E17F1EDB99C67B9182@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 <piotr.kwapulinski@intel.com>

>-		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 <tkusters@aweta.nl>
>
>

RE: [Intel-wired-lan] [PATCH net v2] igb: only strip Rx timestamp header on the first buffer of a frame

[Tjerk Kusters]

Hi,

> >
> >               /* pull rx packet timestamp if available and valid */
> Is this comment up-to-date now ?
> Reviewed-by: Piotr Kwapulinski <piotr.kwapulinski@intel.com>
> 

Good point,  the comment doesn't fully match the code anymore. I'll update it in v3 to:

/* pull rx packet timestamp if available and valid; it is only
 * present on the first buffer of a frame
 */

Thanks for the review.
Tjerk