[PATCH net v3 0/2] igc: fix RX timestamp handling and AF_XDP buffer sizing

[PATCH net v3 0/2] igc: fix RX timestamp handling and AF_XDP buffer sizing

[Paul Moses]

v2..v3
Restore the original timestamp fix and description instead of tracking
frame boundaries. Patch 2 carries Maciej's AF_XDP buffer-size fix unchanged.

v1..v2
Changed commit message in attempt to satisfy bots. No code changes.

Maciej's patch:
https://lore.kernel.org/all/at+t%2FjHczGvyXKRN@boxer/

#
# REPRO
# 
# Receiver: igc / enp1s0 / 10.0.5.175
# Sender:   RTL8125B / enP4p65s0 / 10.0.5.165

# Both hosts
sudo ip link set dev enp1s0 mtu 9000
sudo ip link set dev enP4p65s0 mtu 9000

# Baseline: RX timestamping off
sudo hwstamp_ctl -i enp1s0 -t 0 -r 0
ping -n -M do -c 5 -s 8972 10.0.5.175
# 5/5 received

# Enable RX timestamping only
sudo hwstamp_ctl -i enp1s0 -t 0 -r 1

# Receiver
sudo timeout 20 tcpdump -ni enp1s0 -s 0 -w /tmp/igc-jumbo.pcap \
  'icmp and host 10.0.5.165'

# Sender
ping -n -M do -c 5 -W 2 -s 8972 10.0.5.175
# 0/5 received

# Receiver
sudo tcpdump -nn -e -vvv -r /tmp/igc-jumbo.pcap
# length 8950
# [total length 9000 > length 8936] (invalid)
# wrong icmp cksum

# Expected Ethernet length: 9014
# Captured length:          8950
# Loss:                       64 = 4 continuation buffers * 16 bytes

# Disable RX timestamping again
sudo hwstamp_ctl -i enp1s0 -t 0 -r 0
ping -n -M do -c 5 -s 8972 10.0.5.175
# 5/5 received

-- 
2.56.GIT

[PATCH net v3 1/2] igc: only strip RX timestamp header from first buffer

[Paul Moses]

igc_clean_rx_irq() strips IGC_TS_HDR_LEN whenever a descriptor reports
IGC_RXDADV_STAT_TSIP. For multi-buffer packets, continuation descriptors
retain TSIP even though the inline timestamp is present only in the first
RX buffer.

Subtracting the header length from each continuation buffer truncates
jumbo packets by 16 bytes per continuation and leaves the packet length
larger than the received data.

Only consume the timestamp header when skb is NULL, which identifies the
first buffer of a new packet. An skb carried in rx_ring->skb remains
non-NULL when packet assembly resumes in a later NAPI poll.

Link: https://lore.kernel.org/all/20260625-igb-rx-ts-fix-v3-1-99b3efa08dca@aweta.nl/
Fixes: e1ed4f92a625 ("igc: Refactor Rx timestamp handling")
Cc: stable@vger.kernel.org
Signed-off-by: Paul Moses <p@1g4.org>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Tested-by: Avigail Dahan <avigailx.dahan@intel.com>
---

 drivers/net/ethernet/intel/igc/igc_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 1fb5f3cbe..41b293dc1 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;
-- 
2.56.GIT

[PATCH net v3 2/2] igc: fix AF_XDP receive buffer size for 2K UMEM chunks

[Paul Moses]

From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

SRRCTL.BSIZEPKT has a granularity of 1 KiB. With a 2 KiB UMEM
chunk and the default XDP headroom, xsk_pool_get_rx_frame_size()
returns 1792. Programming this value truncates the hardware receive
buffer size to 1 KiB, even though an MTU-sized frame fits in the
usable UMEM area.

XDP on igc is limited to MTU 1500, so configure a 2 KiB receive
buffer for XSK rings. Limit RLPML to the maximum supported Ethernet
frame size while XDP is attached, regardless of PAGE_SIZE, to avoid
oversized frames spanning descriptors or exceeding the usable UMEM
space.

RLPML is currently programmed from igc_set_rx_mode(), which can run
without RTNL, for example on an IPv6 multicast join, and could then
use a stale XDP state. RLPML does not depend on the address lists.
Program it from igc_setup_rctl() instead, next to RCTL.LPE, and from
igc_xdp_set_prog() while the rings are disabled. This avoids
netif_addr_lock_bh() usage.

Validate that the XSK frame size accommodates a maximum-size frame
with two VLAN tags, FCS and the optional inline RX timestamp. Check
against the maximum supported XDP MTU, rather than the current MTU,
which can change after the socket is bound.

Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Paul Moses <p@1g4.org>
---
 drivers/net/ethernet/intel/igc/igc.h      |  1 +
 drivers/net/ethernet/intel/igc/igc_main.c | 33 +++++++++++++++++------
 drivers/net/ethernet/intel/igc/igc_xdp.c  | 11 +++++---
 3 files changed, 33 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 17f213cc9..ad1062f85 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -765,6 +765,7 @@ static inline s32 igc_read_phy_reg(struct igc_hw *hw, u32 offset, u16 *data)
 	return -EOPNOTSUPP;
 }
 
+void igc_set_rlpml(struct igc_adapter *adapter);
 void igc_reinit_locked(struct igc_adapter *);
 struct igc_nfc_rule *igc_get_nfc_rule(struct igc_adapter *adapter,
 				      u32 location);
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index 41b293dc1..6b418dc71 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -674,8 +674,11 @@ static void igc_configure_rx_ring(struct igc_adapter *adapter,
 	ring->next_to_clean = 0;
 	ring->next_to_use = 0;
 
+	/* igc's XDP limits MTU to 1500; all XSK RX buffers can use 2kb;
+	 * RLPML limits the receive length to fit after UMEM headroom.
+	 */
 	if (ring->xsk_pool)
-		buf_size = xsk_pool_get_rx_frame_size(ring->xsk_pool);
+		buf_size = IGC_RXBUFFER_2048;
 	else if (ring_uses_large_buffer(ring))
 		buf_size = IGC_RXBUFFER_3072;
 	else
@@ -830,6 +833,25 @@ static void igc_setup_mrqc(struct igc_adapter *adapter)
 	wr32(IGC_MRQC, mrqc);
 }
 
+void igc_set_rlpml(struct igc_adapter *adapter)
+{
+	struct igc_hw *hw = &adapter->hw;
+	u32 rlpml = MAX_JUMBO_FRAME_SIZE;
+
+#if (PAGE_SIZE < 8192)
+	if (adapter->max_frame_size <= IGC_MAX_FRAME_BUILD_SKB)
+		rlpml = IGC_MAX_FRAME_BUILD_SKB;
+#endif
+	/* igc's XDP currently does not support jumbo frames. Limit the
+	 * accepted packet size regardless of PAGE_SIZE so it fits in a 2kb
+	 * XSK chunk.
+	 */
+	if (igc_xdp_is_enabled(adapter))
+		rlpml = ETH_FRAME_LEN + VLAN_HLEN * 2 + ETH_FCS_LEN;
+
+	wr32(IGC_RLPML, rlpml);
+}
+
 /**
  * igc_setup_rctl - configure the receive control registers
  * @adapter: Board private structure
@@ -874,6 +896,7 @@ static void igc_setup_rctl(struct igc_adapter *adapter)
 			  IGC_RCTL_CFIEN); /* Disable VLAN CFIEN Filter */
 	}
 
+	igc_set_rlpml(adapter);
 	wr32(IGC_RCTL, rctl);
 }
 
@@ -4117,7 +4140,7 @@ static void igc_set_rx_mode(struct net_device *netdev)
 {
 	struct igc_adapter *adapter = netdev_priv(netdev);
 	struct igc_hw *hw = &adapter->hw;
-	u32 rctl = 0, rlpml = MAX_JUMBO_FRAME_SIZE;
+	u32 rctl = 0;
 	int count;
 
 	/* Check for Promiscuous and All Multicast modes */
@@ -4147,12 +4170,6 @@ static void igc_set_rx_mode(struct net_device *netdev)
 	/* update state of unicast and multicast */
 	rctl |= rd32(IGC_RCTL) & ~(IGC_RCTL_UPE | IGC_RCTL_MPE);
 	wr32(IGC_RCTL, rctl);
-
-#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);
 }
 
 /**
diff --git a/drivers/net/ethernet/intel/igc/igc_xdp.c b/drivers/net/ethernet/intel/igc/igc_xdp.c
index 9eb47b4be..a0f1ecb2e 100644
--- a/drivers/net/ethernet/intel/igc/igc_xdp.c
+++ b/drivers/net/ethernet/intel/igc/igc_xdp.c
@@ -34,6 +34,8 @@ int igc_xdp_set_prog(struct igc_adapter *adapter, struct bpf_prog *prog,
 	}
 
 	old_prog = xchg(&adapter->xdp_prog, prog);
+	if (if_running && need_update)
+		igc_set_rlpml(adapter);
 	if (old_prog)
 		bpf_prog_put(old_prog);
 
@@ -69,11 +71,12 @@ static int igc_xdp_enable_pool(struct igc_adapter *adapter,
 		return -EINVAL;
 
 	frame_size = xsk_pool_get_rx_frame_size(pool);
-	if (frame_size < ETH_FRAME_LEN + VLAN_HLEN * 2) {
+	if (frame_size < ETH_FRAME_LEN + VLAN_HLEN * 2 +
+			 ETH_FCS_LEN + IGC_TS_HDR_LEN) {
 		/* When XDP is enabled, the driver doesn't support frames that
-		 * span over multiple buffers. To avoid that, we check if xsk
-		 * frame size is big enough to fit the max ethernet frame size
-		 * + vlan double tagging.
+		 * span multiple buffers. Check the maximum supported MTU, not
+		 * current MTU, as it may change while the pool is bound. Account
+		 * for double VLAN tags, FCS and inline RX timestamping.
 		 */
 		return -EOPNOTSUPP;
 	}
-- 
2.56.GIT

RE: [PATCH net v3 2/2] igc: fix AF_XDP receive buffer size for 2K UMEM chunks

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Paul Moses <p@1g4.org>
> Sent: Wednesday, October 28, 2026 9:35 AM
> To: intel-wired-lan@lists.osuosl.org
> Cc: netdev@vger.kernel.org; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@kernel.org; kuba@kernel.org;
> pabeni@redhat.com; ast@kernel.org; daniel@iogearbox.net;
> hawk@kernel.org; john.fastabend@gmail.com; sdf@fomichev.me;
> vedang.patel@intel.com; jithu.joseph@intel.com; Fijalkowski, Maciej
> <maciej.fijalkowski@intel.com>; andre.guedes@intel.com; Loktionov,
> Aleksandr <aleksandr.loktionov@intel.com>; Dahan, AvigailX
> <avigailx.dahan@intel.com>; linux-kernel@vger.kernel.org;
> bpf@vger.kernel.org; stable@vger.kernel.org
> Subject: [PATCH net v3 2/2] igc: fix AF_XDP receive buffer size for 2K
> UMEM chunks
> 
nit: Perhaps use "2 KiB" instead of "2K" here, as the latter does not
specify the unit.

> From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> SRRCTL.BSIZEPKT has a granularity of 1 KiB. With a 2 KiB UMEM chunk and
> the default XDP headroom, xsk_pool_get_rx_frame_size() returns 1792.
> Programming this value truncates the hardware receive buffer size to 1
> KiB, even though an MTU-sized frame fits in the usable UMEM area.
> 
> XDP on igc is limited to MTU 1500, so configure a 2 KiB receive buffer
> for XSK rings. Limit RLPML to the maximum supported Ethernet frame size
> while XDP is attached, regardless of PAGE_SIZE, to avoid oversized
> frames spanning descriptors or exceeding the usable UMEM space.
> 
> RLPML is currently programmed from igc_set_rx_mode(), which can run
> without RTNL, for example on an IPv6 multicast join, and could then use
> a stale XDP state. RLPML does not depend on the address lists.
> Program it from igc_setup_rctl() instead, next to RCTL.LPE, and from
> igc_xdp_set_prog() while the rings are disabled. This avoids
> netif_addr_lock_bh() usage.
> 
> Validate that the XSK frame size accommodates a maximum-size frame with
> two VLAN tags, FCS and the optional inline RX timestamp. Check against
> the maximum supported XDP MTU, rather than the current MTU, which can
> change after the socket is bound.
> 
> Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Signed-off-by: Paul Moses <p@1g4.org>
> ---
>  drivers/net/ethernet/intel/igc/igc.h      |  1 +
>  drivers/net/ethernet/intel/igc/igc_main.c | 33 +++++++++++++++++------
> drivers/net/ethernet/intel/igc/igc_xdp.c  | 11 +++++---
>  3 files changed, 33 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc.h
> b/drivers/net/ethernet/intel/igc/igc.h
> index 17f213cc9..ad1062f85 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -765,6 +765,7 @@ static inline s32 igc_read_phy_reg(struct igc_hw
> *hw, u32 offset, u16 *data)
>  	return -EOPNOTSUPP;
>  }
> 
> +void igc_set_rlpml(struct igc_adapter *adapter);
>  void igc_reinit_locked(struct igc_adapter *);  struct igc_nfc_rule
> *igc_get_nfc_rule(struct igc_adapter *adapter,
>  				      u32 location);
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> b/drivers/net/ethernet/intel/igc/igc_main.c
> index 41b293dc1..6b418dc71 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -674,8 +674,11 @@ static void igc_configure_rx_ring(struct
> igc_adapter *adapter,
>  	ring->next_to_clean = 0;
>  	ring->next_to_use = 0;
> 

Also, below:

> +	/* igc's XDP limits MTU to 1500; all XSK RX buffers can use 2kb;
> +	 * RLPML limits the receive length to fit after UMEM headroom.
> +	 */

"2kb" normally means 2 kilobits, whereas the buffer size here is
2048 bytes. I suggest using "2 KiB" consistently in the subject,
commit message, and comments.

...

Also, below:

> +	/* igc's XDP currently does not support jumbo frames. Limit the
> +	 * accepted packet size regardless of PAGE_SIZE so it fits in a
> 2kb
> +	 * XSK chunk.
> +	 */
> +	if (igc_xdp_is_enabled(adapter))
> +		rlpml = ETH_FRAME_LEN + VLAN_HLEN * 2 + ETH_FCS_LEN;
> +
> +	wr32(IGC_RLPML, rlpml);
> +}
> +


Otherwise, looks fine for me
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>

Re: [PATCH net v3 1/2] igc: only strip RX timestamp header from first buffer

[netdev-bot+sashiko]

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/20260625-igb-rx-ts-fix-v3-1-99b3efa08dca@aweta.nl/

> 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

Re: [PATCH net v3 1/2] igc: only strip RX timestamp header from first buffer

[Maciej Fijalkowski]

On Thu, Oct 29, 2026 at 11:37:20PM +0000, netdev-bot+sashiko@kernel.org wrote:
> 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/20260625-igb-rx-ts-fix-v3-1-99b3efa08dca@aweta.nl/
> 
> > 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.

Wow - finally.

> 
> 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?

How would we make sure both commits from this set get actually backported
together?

> 
> -- 
> Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261028083344.3047350-1-p%401g4.org

Re: [PATCH net v3 1/2] igc: only strip RX timestamp header from first buffer

[Paul Moses]

On Friday, October 30th, 2026 at 6:45 AM, Maciej Fijalkowski <maciej.fijalkowski@intel.com> wrote:

> On Thu, Oct 29, 2026 at 11:37:20PM +0000, netdev-bot+sashiko@kernel.org wrote:
> > 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/20260625-igb-rx-ts-fix-v3-1-99b3efa08dca@aweta.nl/
> >
> > > 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.
> 
> Wow - finally.
> 
> >
> > 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?
> 
> How would we make sure both commits from this set get actually backported
> together?
> 
> >
> > --
> > Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261028083344.3047350-1-p%401g4.org
> 

I did forget to tag yours for stable, so maybe the will satisfy it?

I think ultimately, it sees all related bugs as one big problem in
an effort to catch adjacent. My understanding has been that unless
there's true interdependence of patches, ie. kernel will be left broken
unless taken together, a series in itself adequately satisfies this requirement.

Should I do a v4 to add the tag? Or can this be rectified through 
intel's process?

Paul

Re: [PATCH net v3 1/2] igc: only strip RX timestamp header from first buffer

[Maciej Fijalkowski]

On Fri, Oct 30, 2026 at 04:47:36PM +0000, Paul Moses wrote:
> 
> 
> 
> 
> On Friday, October 30th, 2026 at 6:45 AM, Maciej Fijalkowski <maciej.fijalkowski@intel.com> wrote:
> 
> > On Thu, Oct 29, 2026 at 11:37:20PM +0000, netdev-bot+sashiko@kernel.org wrote:
> > > 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/20260625-igb-rx-ts-fix-v3-1-99b3efa08dca@aweta.nl/
> > >
> > > > 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.
> > 
> > Wow - finally.
> > 
> > >
> > > 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?
> > 
> > How would we make sure both commits from this set get actually backported
> > together?
> > 
> > >
> > > --
> > > Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261028083344.3047350-1-p%401g4.org
> > 
> 
> I did forget to tag yours for stable, so maybe the will satisfy it?
> 
> I think ultimately, it sees all related bugs as one big problem in
> an effort to catch adjacent. My understanding has been that unless
> there's true interdependence of patches, ie. kernel will be left broken
> unless taken together, a series in itself adequately satisfies this requirement.
> 
> Should I do a v4 to add the tag? Or can this be rectified through 
> intel's process?

CC: Jake

Jake could you make sure we tag 2/2 as stable backport candidate?

Let's not send v4 and start the whole process again, bots are rather happy
so the ball is on tester's side now.

Thanks a lot Paul!

> 
> Paul