[PATCH net v5 0/4] igb: only strip the Rx timestamp header from the first buffer of a frame

[PATCH net v5 0/4] igb: only strip the Rx timestamp header from the first buffer of a frame

[Tjerk Kusters via B4 Relay]

With Rx hardware timestamping enabled (e.g. by ptp4l), igb strips the
16-byte inline timestamp header from every Rx buffer of a frame instead
of only from the first one. A continuation buffer that starts with 8
zero bytes loses 16 bytes, so jumbo frames with zero payload arrive
short and are dropped.

v3 used skb == NULL to find the first buffer of a frame. The Sashiko
AI review pointed out that this does not hold when an XDP program
consumes the first buffer of a frame that spans several buffers, so v3
was sent back for rework.

The same question came up for igc. There, Maciej suggested not to track
frame boundaries in the data path, but to make sure that frames never
span several buffers while XDP is attached [1]. Paul Moses' igc v3 does
that [2]. This version follows the same approach for igb:

  1/4 limits the receive length (RLPML, or the PF pool's VMOLR with VFs)
      to the configured frame size while XDP is attached, and leaves
      room for the timestamp header in the XDP buffer size checks for
      3 KiB, legacy-rx and PAGE_SIZE >= 8192 buffers. It also refuses
      XDP with rx-all and legacy-rx changes while XDP is attached, as
      both would break the limit. The limit is now set from
      igb_configure() instead of igb_set_rx_mode().
  2/4 is v3, unchanged in code, with a note that it relies on 1/4.
      The Reviewed-by and Tested-by tags of v3 are kept for that reason.
  3/4 gives AF_XDP zero-copy rings a 2 KiB or 3 KiB hardware buffer
      instead of rounding the XSK frame size down to 1 KiB. It refuses
      XSK pools that cannot hold the current maximum frame plus the
      timestamp header, and MTU changes that would no longer fit a bound
      pool. Unlike the igc patch in [2], which checks against the
      largest XDP MTU, this checks the current MTU, as igb allows XDP up
      to MTU 3030 and a fixed check would rule out 2 KiB chunks. A pool
      that does not fit when its queue is configured, for example after
      SR-IOV changed the queue count, is not used in zero-copy mode.
  4/4 drops zero-copy frames that still span several descriptors, as
      ixgbe does, instead of passing each buffer on as a frame. It
      depends on 3/4.

As in igc, 1/4 moves the RLPML programming out of igb_set_rx_mode(),
which can run without RTNL, into igb_configure(), which only runs while
Rx is stopped. This includes the PF pool limit in VMOLR, which
igb_set_rx_mode() now leaves as it is. Attaching or removing XDP on a
running interface goes through igb_close() and igb_open(), and an MTU
change through igb_down() and igb_up(), so the limit is updated before
Rx is enabled again.

The Sashiko review of v4 also points out two older issues that this
series does not change:

- On 2 KiB build_skb rings, igb_skb_pad() leaves exactly
  IGB_MAX_FRAME_BUILD_SKB bytes before skb_shared_info, with no room for
  the 16-byte timestamp header. With Rx timestamping on, frame and
  header can overlap skb_shared_info by up to 12 bytes, or 2 bytes at
  the XDP limit at MTU 1500. This dates from 9cbc948b5a20 ("igb: add XDP
  support"). I left it out of this series: it is a separate problem from
  the one fixed here, and a fix changes the buffer layout and the XDP
  headroom for all igb users, so it needs its own review and testing.
- igb_set_vf_rlpml() let a VF set other VMOLR bits of its own pool. That
  is fixed separately: https://lore.kernel.org/all/20261104-igb-vf-rlpml-v1-1-729621b60eee@aweta.nl/

How to reproduce
----------------
Receiver with igb (eth0), peer with MTU 8228. hwstamp_ctl turns on Rx
timestamping with filter ALL, no ptp4l needed; this ping method is
taken from Paul's igc report. One host with two ports also works if
the sending port is moved into its own network namespace.

1) Truncated jumbo frames (patch 2)

  receiver:  ip link set eth0 mtu 8228
             hwstamp_ctl -i eth0 -r 1
  peer:      ping -c 5 -M do -s 8200 -p 00 <receiver>

  Without the series there are no replies, and IpExtInTruncatedPkts
  goes up by one per frame. With the series all requests are answered.

2) XDP consumes the first buffer (patch 1)

  receiver:  ip link set eth0 mtu 3000
             hwstamp_ctl -i eth0 -r 1
             xdp-filter load -m native eth0
             xdp-filter ip <peer> -m src
             tcpdump -nn -e -i eth0 'ether src 00:00:00:00:00:00'
  peer:      ping -c 3 -M do -s 8200 -p 00 <receiver>

  At MTU 3000 XDP can attach, but without patch 1 the MAC still accepts
  the 8242-byte frames, which then span three Rx buffers. xdp-filter
  drops the first one, and the rest reaches the stack as a separate
  frame with all-zero MAC addresses:

    without the series    5154 bytes (header stripped twice)
    v3 alone              5170 bytes (stripped once)
    this series           no frame; rx_long_length_errors +1 per frame

3) AF_XDP zero-copy with 2 KiB chunks (patch 3)

  receiver:  ip link set eth0 mtu 1500
             ethtool -L eth0 combined 1
             hwstamp_ctl -i eth0 -r 1
             xdpsock -i eth0 -q 0 -r -z -N -f 2048
  peer:      ping -c 50 -i 0.02 -s 1472 <receiver>

  xdpsock is from xdp-project/bpf-examples (AF_XDP-example). It also
  takes ARP, so the peer needs a fresh ARP entry before it starts.
  Compare rx_packets (MAC) with rx_queue_0_packets in ethtool -S:

    without the series    68 frames, 118 descriptors
    this series           64 frames, 64 descriptors

  At MTU 3000, -f 4096 receives 3014-byte frames in one buffer. Binding
  with -f 2048 at MTU 3000 was refused with EOPNOTSUPP by the new check
  on the 6.14.9 test kernel; current kernels refuse it earlier, in
  xp_assign_dev(), with EINVAL.

Tested on an I350 with these patches applied to the igb driver of
kernel 6.14.9 (the series itself is against v7.1-rc7), with the steps
above: results as listed. The v3 line is from an earlier run with an
XDP program dropping ICMP. Also checked: XDP attach at MTU 1500 and
3030 works, at MTU 3046 it is refused, as is an MTU change to 3046
with XDP attached. With two VFs allocated (booted with pci=realloc),
step 2 gives the same picture: three leftover frames without the
series, none with it, as the PF pool limit in VMOLR drops the frames.
In step 3, 98-byte frames and 4 KiB chunks take one descriptor per
frame with and without the series. With XDP attached, rx-all stays off
and legacy-rx cannot be changed, and XDP attach is refused with rx-all
on, or with legacy-rx on at MTU 3000, also when it was set while the
interface was down. RLPML and the PF pool's VMOLR limit read back as
expected with and without XDP and VFs, also after each of about 200 MTU
changes with XDP and two VFs. Patch 4 is not reproduced: with patch 3 the I350
no longer splits frames in zero-copy mode.
The steps above were repeated on kernel 7.3.1 with the series applied
to the v7.3 igb driver, with the same results; without VFs, as that
boot lacked pci=realloc.
Not tested: PAGE_SIZE >= 8192.

The patches, their descriptions and this cover letter were prepared
with the help of an AI coding assistant (Claude). I reviewed them and
ran the tests above on my test bench.

[1] https://lore.kernel.org/all/20261025065154.2037149-1-p@1g4.org/
[2] https://lore.kernel.org/all/20261028083344.3047350-1-p@1g4.org/

Changes in v5:
- Patch 1: v4 also subtracted the timestamp header from the build_skb
  frame limit, so XDP no longer attached at MTU 1495-1510 on x86
  (1493-1508 where NET_IP_ALIGN is 2), including the default MTU 1500.
  Only subtract the header where the limit is the hardware buffer size,
  and compute it from the adapter state instead of the ring flags. Skip
  the check when XDP is removed, so a program can always be detached.
- Patch 1: refuse XDP with rx-all and legacy-rx changes with XDP
  attached, and turn rx-all back on after XDP removal if it was
  requested meanwhile. Program RLPML and the PF pool limit from
  igb_configure() instead of igb_set_rx_mode(). With XDP attached, an
  MTU change can now move from 2 KiB to 3 KiB buffers. Added
  Suggested-by: Maciej.
- Patch 1: the issues above were also reported by the Sashiko review
  of v4.
- Patch 2: no code change. Commit message: igc is fixed by Paul's
  series, not by this patch; the loss is per continuation buffer, not
  only at the tail; the possible size wrap is described (Sashiko).
- New patch 3: AF_XDP zero-copy Rx buffer size, as asked by Maciej.
- New patch 4: drop zero-copy frames that span several descriptors;
  depends on patch 3.
- Link to v4: https://patch.msgid.link/20261102-igb-rx-ts-fix-v4-0-8785124c5d23@aweta.nl

Changes in v4:
- New patch 1: limit the receive length while XDP is attached, so that
  skb == NULL reliably marks the first buffer (Sashiko review), as
  done for igc.
- Patch 2: no code change; added a note that it relies on patch 1.
- Link to v3: https://patch.msgid.link/20260625-igb-rx-ts-fix-v3-1-99b3efa08dca@aweta.nl

---
Tjerk Kusters (4):
      igb: do not let XDP frames span more than one Rx buffer
      igb: only strip Rx timestamp header on the first buffer of a frame
      igb: do not split AF_XDP zero-copy frames over several Rx buffers
      igb: drop AF_XDP zero-copy frames that span several Rx buffers

 drivers/net/ethernet/intel/igb/igb.h         |  10 ++
 drivers/net/ethernet/intel/igb/igb_ethtool.c |   9 ++
 drivers/net/ethernet/intel/igb/igb_main.c    | 181 ++++++++++++++++++++-------
 drivers/net/ethernet/intel/igb/igb_xsk.c     |  42 ++++++-
 4 files changed, 194 insertions(+), 48 deletions(-)
---
base-commit: 2d3090a8aeb596a26935db0955d46c9a5db5c6ce
change-id: 20260619-igb-rx-ts-fix-cd70585ee316

Best regards,
--  
Tjerk Kusters <tkusters@aweta.nl>

[PATCH net v5 1/4] igb: do not let XDP frames span more than one Rx buffer

[Tjerk Kusters via B4 Relay]

From: Tjerk Kusters <tkusters@aweta.nl>

igb does not support XDP multi-buffer: igb_xdp_setup() only accepts an
MTU whose frame fits in one Rx buffer, and the XDP program only sees
the first buffer of a frame. The maximum receive length is not limited
accordingly, though. RLPML is only lowered when the frame fits a
build_skb buffer on systems with PAGE_SIZE < 8192 and no VFs. Otherwise
the MAC still accepts frames of up to MAX_JUMBO_FRAME_SIZE while XDP is
attached, for example at MTU 3000 on 3 KiB buffers, and at any MTU when
PAGE_SIZE >= 8192. With VFs allocated, RLPML always stays at
MAX_JUMBO_FRAME_SIZE and the PF pool limit in VMOLR is only lowered for
frames that fit a build_skb buffer, so the same happens at MTU 3000.

Such frames span several descriptors. When the program does not return
XDP_PASS for the first buffer, no skb is built, and igb_clean_rx_irq()
handles the next buffer of the same frame as the start of a new frame:
it runs the program on it and may pass it to the stack as a bogus frame
made from the middle of the original payload.

Limit the receive length to the configured maximum frame size while XDP
is attached: in RLPML when no VFs are allocated, and in the PF pool's
VMOLR otherwise, as RLPML then has to stay large for the VFs. Larger
frames are dropped by the MAC.

Also make sure that the limit holds:

- Leave room for the inline Rx timestamp header when checking that the
  frame fits in one Rx buffer, on XDP setup and on MTU change, where the
  limit is the hardware buffer size. This lowers the largest MTU
  accepted with XDP by 16 bytes: from 3046 to 3030 on 3 KiB buffers,
  and from 2022 to 2006 with legacy-rx or PAGE_SIZE >= 8192. Setups that
  use XDP at those MTUs will have to lower the MTU. The build_skb frame
  limit used with 2 KiB buffers is kept, so XDP still attaches up to MTU
  1510 on x86. The header is only prepended by 82580 and later MACs; the
  room is reserved on all MACs for simplicity.
- Base that check on the buffer size that igb_set_rx_buffer_len() will
  select, not on the ring flags, which are only updated when the rings
  are configured and can be stale on an interface that is down. As a
  result, an MTU change with XDP attached can now also move from 2 KiB
  to 3 KiB buffers. The check is skipped when XDP is removed.
- Refuse XDP while rx-all is enabled, and keep rx-all off while XDP is
  attached, as the MAC then also stores frames longer than RLPML. If
  rx-all was requested meanwhile, it is turned on when XDP is removed.
- Refuse to change the legacy-rx private flag while XDP is attached, as
  that changes the Rx buffer size without checking it against the MTU.
- Program both limits from igb_configure(), which only runs while Rx is
  stopped, and no longer from igb_set_rx_mode(), which can also run
  without RTNL, for example on an IPv6 multicast join or a VF mailbox
  request. Otherwise such a call during an XDP removal or an MTU change
  could apply the new limit while the rings still use the old buffers.
  Attaching or removing an XDP program on a running interface goes
  through igb_close() and igb_open(), and an MTU change through
  igb_down() and igb_up(), so igb_configure() runs in both cases.
  igb_set_rx_mode() still updates other bits of the PF pool's VMOLR, so
  the limit there is changed under the address lock.

This follows the same approach as the igc fix that limits RLPML while
XDP is attached. As before, the PF pool's VMOLR is not written on i354.

Tested on an I350 at MTU 3000 with Rx timestamping on and xdp-filter
dropping all IPv4 from the peer: 8242-byte frames are now dropped by the
MAC (rx_long_length_errors), instead of their last two buffers being
delivered as a separate frame. XDP attach at MTU 1500 and 3030 still
works, at MTU 3046 it is refused, as is an MTU change from 3030 to 3046
with XDP attached; an MTU change from 1500 to 3000 with XDP attached
now works. With XDP attached, rx-all stays off and the legacy-rx flag
cannot be changed. XDP attach is refused with rx-all on, and at MTU
3000 with legacy-rx on, also when the flag was set while the interface
was down. MTU changes between 1500 and 3000 with XDP attached, while
the peer sent 3000-byte zero-payload frames, let no frame through as a
separate frame.

With two VFs allocated, the same frames are counted by the MAC but
dropped by the PF pool limit in VMOLR before they reach the Rx queue;
without this patch their last two buffers are again delivered as a
separate frame. Read back from the registers, RLPML then stays at 9728
and the PF pool limit is 3026 at MTU 3000 and 1526 at MTU 1500 with XDP
attached, also after each of about 200 MTU changes.

Link: https://lore.kernel.org/all/20261028083344.3047350-3-p@1g4.org/
Fixes: 9cbc948b5a20 ("igb: add XDP support")
Cc: stable@vger.kernel.org
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Tjerk Kusters <tkusters@aweta.nl>
---
 drivers/net/ethernet/intel/igb/igb_ethtool.c |   9 ++
 drivers/net/ethernet/intel/igb/igb_main.c    | 146 +++++++++++++++++++--------
 2 files changed, 113 insertions(+), 42 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_ethtool.c b/drivers/net/ethernet/intel/igb/igb_ethtool.c
index f7938c1da835..4f2891fca282 100644
--- a/drivers/net/ethernet/intel/igb/igb_ethtool.c
+++ b/drivers/net/ethernet/intel/igb/igb_ethtool.c
@@ -3438,6 +3438,15 @@ static int igb_set_priv_flags(struct net_device *netdev, u32 priv_flags)
 		flags |= IGB_FLAG_RX_LEGACY;
 
 	if (flags != adapter->flags) {
+		/* the Rx buffer size depends on this flag, see
+		 * igb_xdp_setup()
+		 */
+		if (igb_xdp_is_enabled(adapter)) {
+			netdev_warn(netdev,
+				    "Cannot change legacy-rx while XDP is attached\n");
+			return -EOPNOTSUPP;
+		}
+
 		adapter->flags = flags;
 
 		/* reset interface to repopulate queues */
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index ce91dda00ec0..8abdd4e58550 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -1964,6 +1964,54 @@ static void igb_setup_tx_mode(struct igb_adapter *adapter)
 		   "enabled" : "disabled");
 }
 
+/**
+ *  igb_set_rlpml - set the receive length limits
+ *  @adapter: private board structure
+ *
+ *  Called from igb_configure() while Rx is stopped. igb_set_rx_mode() can
+ *  also run without RTNL, so it does not change these limits.
+ **/
+static void igb_set_rlpml(struct igb_adapter *adapter)
+{
+	u32 rlpml = MAX_JUMBO_FRAME_SIZE, pool_rlpml = MAX_JUMBO_FRAME_SIZE;
+	unsigned int vfn = adapter->vfs_allocated_count;
+	struct e1000_hw *hw = &adapter->hw;
+	u32 vmolr;
+
+#if (PAGE_SIZE < 8192)
+	/* restrict as needed to support build_skb */
+	if (adapter->max_frame_size <= IGB_MAX_FRAME_BUILD_SKB) {
+		pool_rlpml = IGB_MAX_FRAME_BUILD_SKB;
+		if (!vfn)
+			rlpml = IGB_MAX_FRAME_BUILD_SKB;
+	}
+#endif
+	/* XDP does not support frames that span more than one Rx buffer.
+	 * With VFs the global limit stays large and the PF pool is limited
+	 * through VMOLR.
+	 */
+	if (igb_xdp_is_enabled(adapter)) {
+		pool_rlpml = min(pool_rlpml, adapter->max_frame_size);
+		if (!vfn)
+			rlpml = min(rlpml, adapter->max_frame_size);
+	}
+	wr32(E1000_RLPML, rlpml);
+
+	/* the PF pool's VMOLR is used on these MACs, see igb_set_rx_mode() */
+	if (hw->mac.type < e1000_82576 || hw->mac.type > e1000_i350)
+		return;
+
+	/* igb_set_rx_mode() also updates this register, and the core calls
+	 * it under the address lock
+	 */
+	netif_addr_lock_bh(adapter->netdev);
+	vmolr = rd32(E1000_VMOLR(vfn));
+	vmolr &= ~E1000_VMOLR_RLPML_MASK;
+	vmolr |= pool_rlpml | E1000_VMOLR_LPE;
+	wr32(E1000_VMOLR(vfn), vmolr);
+	netif_addr_unlock_bh(adapter->netdev);
+}
+
 /**
  *  igb_configure - configure the hardware for RX and TX
  *  @adapter: private board structure
@@ -1981,6 +2029,7 @@ static void igb_configure(struct igb_adapter *adapter)
 
 	igb_setup_tctl(adapter);
 	igb_setup_mrqc(adapter);
+	igb_set_rlpml(adapter);
 	igb_setup_rctl(adapter);
 
 	igb_nfc_filter_restore(adapter);
@@ -2468,6 +2517,14 @@ void igb_reset(struct igb_adapter *adapter)
 static netdev_features_t igb_fix_features(struct net_device *netdev,
 	netdev_features_t features)
 {
+	struct igb_adapter *adapter = netdev_priv(netdev);
+
+	/* with rx-all the MAC also stores frames longer than RLPML, which
+	 * XDP cannot handle
+	 */
+	if (igb_xdp_is_enabled(adapter))
+		features &= ~NETIF_F_RXALL;
+
 	/* Since there is no support for separate Rx/Tx vlan accel
 	 * enable/disable make sure Tx flag is always in same state as Rx.
 	 */
@@ -2886,6 +2943,27 @@ static int igb_setup_tc(struct net_device *dev, enum tc_setup_type type,
 	}
 }
 
+/* Check that a frame fits in one Rx buffer together with the inline Rx
+ * timestamp header, for the buffer size igb_set_rx_buffer_len() selects.
+ * The ring flags are only updated when the rings are configured, so they
+ * are not used here. For build_skb rings the frame is limited to
+ * IGB_MAX_FRAME_BUILD_SKB, which is below the 2 KiB hardware buffer size
+ * even with the header.
+ */
+static bool igb_xdp_frame_fits(struct igb_adapter *adapter, int max_frame)
+{
+#if (PAGE_SIZE < 8192)
+	if (!(adapter->flags & IGB_FLAG_RX_LEGACY)) {
+		if (max_frame <= IGB_MAX_FRAME_BUILD_SKB &&
+		    !IGB_2K_TOO_SMALL_WITH_PADDING)
+			return true;
+
+		return max_frame + IGB_TS_HDR_LEN <= IGB_RXBUFFER_3072;
+	}
+#endif
+	return max_frame + IGB_TS_HDR_LEN <= IGB_RXBUFFER_2048;
+}
+
 static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
 {
 	int i, frame_size = dev->mtu + IGB_ETH_PKT_HDR_PAD;
@@ -2894,17 +2972,21 @@ static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
 	bool running = netif_running(dev);
 	bool need_reset;
 
-	/* verify igb ring attributes are sufficient for XDP */
-	for (i = 0; i < adapter->num_rx_queues; i++) {
-		struct igb_ring *ring = adapter->rx_ring[i];
+	/* with rx-all the MAC also stores frames longer than RLPML */
+	if (prog && (dev->features & NETIF_F_RXALL)) {
+		NL_SET_ERR_MSG_MOD(bpf->extack, "XDP is not supported with rx-all");
+		return -EOPNOTSUPP;
+	}
 
-		if (frame_size > igb_rx_bufsz(ring)) {
-			NL_SET_ERR_MSG_MOD(bpf->extack,
-					   "The RX buffer size is too small for the frame size");
-			netdev_warn(dev, "XDP RX buffer size %d is too small for the frame size %d\n",
-				    igb_rx_bufsz(ring), frame_size);
-			return -EINVAL;
-		}
+	/* verify igb ring attributes are sufficient for XDP, leaving room
+	 * for the inline Rx timestamp header
+	 */
+	if (prog && !igb_xdp_frame_fits(adapter, frame_size)) {
+		NL_SET_ERR_MSG_MOD(bpf->extack,
+				   "The RX buffer size is too small for the frame size");
+		netdev_warn(dev, "XDP RX buffer size is too small for the frame size %d\n",
+			    frame_size);
+		return -EINVAL;
 	}
 
 	old_prog = xchg(&adapter->xdp_prog, prog);
@@ -2935,6 +3017,10 @@ static int igb_xdp_setup(struct net_device *dev, struct netdev_bpf *bpf)
 	if (running)
 		igb_open(dev);
 
+	/* rx-all may have been requested while XDP was attached */
+	if (!prog && (dev->wanted_features & NETIF_F_RXALL))
+		netdev_update_features(dev);
+
 	return 0;
 }
 
@@ -5323,7 +5409,7 @@ static void igb_set_rx_mode(struct net_device *netdev)
 	struct igb_adapter *adapter = netdev_priv(netdev);
 	struct e1000_hw *hw = &adapter->hw;
 	unsigned int vfn = adapter->vfs_allocated_count;
-	u32 rctl = 0, vmolr = 0, rlpml = MAX_JUMBO_FRAME_SIZE;
+	u32 rctl = 0, vmolr = 0;
 	int count;
 
 	/* Check for Promiscuous and All Multicast modes */
@@ -5380,14 +5466,6 @@ static void igb_set_rx_mode(struct net_device *netdev)
 				     E1000_RCTL_VFE);
 	wr32(E1000_RCTL, rctl);
 
-#if (PAGE_SIZE < 8192)
-	if (!adapter->vfs_allocated_count) {
-		if (adapter->max_frame_size <= IGB_MAX_FRAME_BUILD_SKB)
-			rlpml = IGB_MAX_FRAME_BUILD_SKB;
-	}
-#endif
-	wr32(E1000_RLPML, rlpml);
-
 	/* In order to support SR-IOV and eventually VMDq it is necessary to set
 	 * the VMOLR to enable the appropriate modes.  Without this workaround
 	 * we will have issues with VLAN tag stripping not being done for frames
@@ -5399,19 +5477,10 @@ static void igb_set_rx_mode(struct net_device *netdev)
 	/* set UTA to appropriate mode */
 	igb_set_uta(adapter, !!(vmolr & E1000_VMOLR_ROPE));
 
+	/* keep the receive length limit set by igb_set_rlpml() */
 	vmolr |= rd32(E1000_VMOLR(vfn)) &
 		 ~(E1000_VMOLR_ROPE | E1000_VMOLR_MPME | E1000_VMOLR_ROMPE);
 
-	/* enable Rx jumbo frames, restrict as needed to support build_skb */
-	vmolr &= ~E1000_VMOLR_RLPML_MASK;
-#if (PAGE_SIZE < 8192)
-	if (adapter->max_frame_size <= IGB_MAX_FRAME_BUILD_SKB)
-		vmolr |= IGB_MAX_FRAME_BUILD_SKB;
-	else
-#endif
-		vmolr |= MAX_JUMBO_FRAME_SIZE;
-	vmolr |= E1000_VMOLR_LPE;
-
 	wr32(E1000_VMOLR(vfn), vmolr);
 
 	igb_restore_vf_multicasts(adapter);
@@ -6707,19 +6776,12 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
 	struct igb_adapter *adapter = netdev_priv(netdev);
 	int max_frame = new_mtu + IGB_ETH_PKT_HDR_PAD;
 
-	if (igb_xdp_is_enabled(adapter)) {
-		int i;
-
-		for (i = 0; i < adapter->num_rx_queues; i++) {
-			struct igb_ring *ring = adapter->rx_ring[i];
-
-			if (max_frame > igb_rx_bufsz(ring)) {
-				netdev_warn(adapter->netdev,
-					    "Requested MTU size is not supported with XDP. Max frame size is %d\n",
-					    max_frame);
-				return -EINVAL;
-			}
-		}
+	if (igb_xdp_is_enabled(adapter) &&
+	    !igb_xdp_frame_fits(adapter, max_frame)) {
+		netdev_warn(adapter->netdev,
+			    "Requested MTU size is not supported with XDP. Frame size is %d\n",
+			    max_frame);
+		return -EINVAL;
 	}
 
 	/* adjust max frame to be at least the size of a standard frame */

-- 
2.27.0

[PATCH net v5 2/4] 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, and the caller reduces that buffer's size by 16.
igb_add_rx_frag() adds a continuation buffer from its original offset, so
the buffer loses its last 16 bytes and the data of the next buffer
follows directly. A frame spanning N buffers loses 16 bytes per affected
continuation buffer, in the middle of the payload when N > 2. If the last
buffer of a frame holds 8 to 15 bytes, all zero, the unsigned size wraps
and a fragment of about 4 GiB is added to the skb. The sender chooses the
frame length and content, so this can be triggered remotely.

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 are dropped as truncated while frames with non-zero
content are fine. The driver reports no error; only the IP layer counts
the frames in IpExtInTruncatedPkts.

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.

This relies on the previous patch. Without it, an XDP program that does
not return XDP_PASS for the first buffer of a multi-buffer frame leaves
skb NULL for the rest of that frame. With the receive length limited
while XDP is attached, such frames no longer reach the XDP path.

igc has the same bug; Paul Moses' igc series, linked below, fixes it the
same way.

Link: https://lore.kernel.org/all/20261028083344.3047350-2-p@1g4.org/
Fixes: 5379260852b0 ("igb: Fix XDP with PTP enabled")
Cc: stable@vger.kernel.org
Reviewed-by: Piotr Kwapulinski <piotr.kwapulinski@intel.com>
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
Reviewed-by: Kurt Kanzenbach <kurt@linutronix.de>
Tested-by: Alexander Nowlin <alexander.nowlin@intel.com>
Signed-off-by: Tjerk Kusters <tkusters@aweta.nl>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index 8abdd4e58550..cf9911137bd3 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -9122,8 +9122,11 @@ static int igb_clean_rx_irq(struct igb_q_vector *q_vector, const int budget)
 		rx_buffer = igb_get_rx_buffer(rx_ring, size, &rx_buf_pgcnt);
 		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)) {
+		/* pull rx packet timestamp if available and valid; it is only
+		 * present on the first buffer of a frame
+		 */
+		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

[PATCH net v5 3/4] igb: do not split AF_XDP zero-copy frames over several Rx buffers

[Tjerk Kusters via B4 Relay]

From: Tjerk Kusters <tkusters@aweta.nl>

SRRCTL.BSIZEPKT has a granularity of 1 KiB. For AF_XDP zero-copy rings,
igb_setup_srrctl() programs xsk_pool_get_rx_frame_size(), which is
rounded down. With 2 KiB UMEM chunks and the default headroom this is
1792 bytes, so the hardware buffer becomes 1 KiB and a full-size frame
spans two Rx buffers. igb_clean_rx_irq_zc() handles every descriptor as
a complete frame, so the AF_XDP socket gets such a frame as two
packets, the second one without headers. With Rx timestamping enabled,
the timestamp header is also stripped from the second buffer, and a
short final buffer makes the size wrap.

Program the same buffer size as for the other rings, 2 KiB or 3 KiB.
This depends on "igb: do not let XDP frames span more than one Rx
buffer": the receive length limit set there while XDP is attached keeps
the hardware from writing more than the frame plus the timestamp header
into the larger buffer. Without it, the hardware can write past the XSK
frame.

Refuse an XSK pool whose frame size cannot hold that, and an MTU change
that would no longer fit a bound pool, also while no XDP program is
attached. The current MTU is checked rather than the largest XDP MTU,
as igb allows XDP up to MTU 3030 and that would rule out 2 KiB chunks.
A pool that still does not fit when the ring is configured, for example
because its queue was hidden by SR-IOV during an MTU change, is not used
in zero-copy mode, see igb_xsk_pool(); its Tx then fails with EINVAL
until the pool is bound again.

This is the igb counterpart of the igc XSK buffer change.

Tested on an I350 with xdpsock from bpf-examples in rxdrop zero-copy
mode on one queue, Rx timestamping on and 1514-byte frames from the
peer. With 2 KiB chunks, the receive descriptors went from about two
per frame to one per frame; 98-byte frames and 4 KiB chunks were one
per frame with and without this patch. At MTU 3000, 4 KiB chunks
receive 3014-byte frames in one buffer. A 2 KiB chunk pool at MTU 3000
was refused by the new check on the 6.14.9 test kernel; current kernels
already refuse it in xp_assign_dev(). There, with 2 KiB chunks and the
default headroom, the new check refuses MTU 1751 to 1766, which the core
accepts.

Link: https://lore.kernel.org/all/20261028083344.3047350-3-p@1g4.org/
Fixes: 2c6196013f84 ("igb: Add AF_XDP zero-copy Rx support")
Cc: stable@vger.kernel.org
Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Tjerk Kusters <tkusters@aweta.nl>
---
 drivers/net/ethernet/intel/igb/igb.h      |  9 +++++++++
 drivers/net/ethernet/intel/igb/igb_main.c | 28 +++++++++++++++++++++++++---
 drivers/net/ethernet/intel/igb/igb_xsk.c  | 19 ++++++++++++++++++-
 3 files changed, 52 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 0fff1df81b7b..0495c02544d6 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -856,6 +856,15 @@ int igb_add_mac_steering_filter(struct igb_adapter *adapter,
 int igb_del_mac_steering_filter(struct igb_adapter *adapter,
 				const u8 *addr, u8 queue, u8 flags);
 
+/* XDP does not support frames that span more than one Rx buffer, so a
+ * frame plus the inline Rx timestamp header must fit in one XSK buffer.
+ */
+static inline bool igb_xsk_frame_fits(struct xsk_buff_pool *pool,
+				      int max_frame)
+{
+	return max_frame + IGB_TS_HDR_LEN <= xsk_pool_get_rx_frame_size(pool);
+}
+
 struct xsk_buff_pool *igb_xsk_pool(struct igb_adapter *adapter,
 				   struct igb_ring *ring);
 int igb_xsk_pool_setup(struct igb_adapter *adapter,
diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index cf9911137bd3..b286adedad5d 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -4833,9 +4833,13 @@ void igb_setup_srrctl(struct igb_adapter *adapter, struct igb_ring *ring)
 	u32 srrctl = 0;
 	u32 buf_size;
 
-	if (ring->xsk_pool)
-		buf_size = xsk_pool_get_rx_frame_size(ring->xsk_pool);
-	else if (ring_uses_large_buffer(ring))
+	/* SRRCTL.BSIZEPKT has a 1 KiB granularity, so programming the XSK
+	 * frame size would round it down and split frames over several
+	 * buffers. XSK rings use the same buffer size as the other rings.
+	 * While XDP is attached, the receive length limit keeps frames
+	 * within the XSK frame, see igb_xsk_frame_fits().
+	 */
+	if (ring_uses_large_buffer(ring))
 		buf_size = IGB_RXBUFFER_3072;
 	else
 		buf_size = IGB_RXBUFFER_2048;
@@ -6775,6 +6779,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
 {
 	struct igb_adapter *adapter = netdev_priv(netdev);
 	int max_frame = new_mtu + IGB_ETH_PKT_HDR_PAD;
+	int i;
 
 	if (igb_xdp_is_enabled(adapter) &&
 	    !igb_xdp_frame_fits(adapter, max_frame)) {
@@ -6788,6 +6793,23 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
 	if (max_frame < (ETH_FRAME_LEN + ETH_FCS_LEN))
 		max_frame = ETH_FRAME_LEN + ETH_FCS_LEN;
 
+	/* the frame must fit the buffers of a bound XSK pool, also while no
+	 * XDP program is attached, as attaching one does not check it again
+	 */
+	for (i = 0; i < adapter->num_rx_queues; i++) {
+		struct xsk_buff_pool *pool = xsk_get_pool_from_qid(netdev, i);
+
+		if (!pool || !pool->dev)
+			continue;
+
+		if (!igb_xsk_frame_fits(pool, max_frame)) {
+			netdev_warn(adapter->netdev,
+				    "Requested MTU size is not supported with AF_XDP. Frame size is %d\n",
+				    max_frame);
+			return -EINVAL;
+		}
+	}
+
 	while (test_and_set_bit(__IGB_RESETTING, &adapter->state))
 		usleep_range(1000, 2000);
 
diff --git a/drivers/net/ethernet/intel/igb/igb_xsk.c b/drivers/net/ethernet/intel/igb/igb_xsk.c
index ce4a7b58cad2..fb04df82272a 100644
--- a/drivers/net/ethernet/intel/igb/igb_xsk.c
+++ b/drivers/net/ethernet/intel/igb/igb_xsk.c
@@ -91,7 +91,20 @@ struct xsk_buff_pool *igb_xsk_pool(struct igb_adapter *adapter,
 	if (!igb_xdp_is_enabled(adapter))
 		return NULL;
 
-	return (pool && pool->dev) ? pool : NULL;
+	if (!pool || !pool->dev)
+		return NULL;
+
+	/* the MTU check in igb_change_mtu() does not see a pool on a queue
+	 * that was hidden at the time, for example by SR-IOV
+	 */
+	if (!igb_xsk_frame_fits(pool, adapter->max_frame_size)) {
+		netdev_warn_once(adapter->netdev,
+				 "XSK frame size is too small for the MTU, queue %d is not used in zero-copy mode\n",
+				 qid);
+		return NULL;
+	}
+
+	return pool;
 }
 
 static int igb_xsk_pool_enable(struct igb_adapter *adapter,
@@ -110,6 +123,10 @@ static int igb_xsk_pool_enable(struct igb_adapter *adapter,
 	    qid >= netdev->real_num_tx_queues)
 		return -EINVAL;
 
+	/* igb_change_mtu() keeps this true while the pool is bound */
+	if (!igb_xsk_frame_fits(pool, adapter->max_frame_size))
+		return -EOPNOTSUPP;
+
 	err = xsk_pool_dma_map(pool, &adapter->pdev->dev, IGB_RX_DMA_ATTR);
 	if (err)
 		return err;

-- 
2.27.0

Re: [PATCH net v5 3/4] igb: do not split AF_XDP zero-copy frames over several Rx buffers

[Maciej Fijalkowski]

On Wed, Nov 04, 2026 at 07:40:04AM +0100, Tjerk Kusters via B4 Relay wrote:
> From: Tjerk Kusters <tkusters@aweta.nl>
> 
> SRRCTL.BSIZEPKT has a granularity of 1 KiB. For AF_XDP zero-copy rings,
> igb_setup_srrctl() programs xsk_pool_get_rx_frame_size(), which is
> rounded down. With 2 KiB UMEM chunks and the default headroom this is
> 1792 bytes, so the hardware buffer becomes 1 KiB and a full-size frame
> spans two Rx buffers. igb_clean_rx_irq_zc() handles every descriptor as
> a complete frame, so the AF_XDP socket gets such a frame as two
> packets, the second one without headers. With Rx timestamping enabled,
> the timestamp header is also stripped from the second buffer, and a
> short final buffer makes the size wrap.
> 
> Program the same buffer size as for the other rings, 2 KiB or 3 KiB.
> This depends on "igb: do not let XDP frames span more than one Rx
> buffer": the receive length limit set there while XDP is attached keeps
> the hardware from writing more than the frame plus the timestamp header
> into the larger buffer. Without it, the hardware can write past the XSK
> frame.
> 
> Refuse an XSK pool whose frame size cannot hold that, and an MTU change
> that would no longer fit a bound pool, also while no XDP program is
> attached. The current MTU is checked rather than the largest XDP MTU,
> as igb allows XDP up to MTU 3030 and that would rule out 2 KiB chunks.
> A pool that still does not fit when the ring is configured, for example
> because its queue was hidden by SR-IOV during an MTU change, is not used
> in zero-copy mode, see igb_xsk_pool(); its Tx then fails with EINVAL
> until the pool is bound again.

I would rather return -EBUSY when enabling VFs on queues with active
xsk_pool. Seems igb sort of steal queues from PF?

We do this in core when xsk_pool enabled queue would get wiped out via
ethtool -L shrinking the queue count.

> 
> This is the igb counterpart of the igc XSK buffer change.
> 
> Tested on an I350 with xdpsock from bpf-examples in rxdrop zero-copy
> mode on one queue, Rx timestamping on and 1514-byte frames from the
> peer. With 2 KiB chunks, the receive descriptors went from about two
> per frame to one per frame; 98-byte frames and 4 KiB chunks were one
> per frame with and without this patch. At MTU 3000, 4 KiB chunks
> receive 3014-byte frames in one buffer. A 2 KiB chunk pool at MTU 3000
> was refused by the new check on the 6.14.9 test kernel; current kernels
> already refuse it in xp_assign_dev(). There, with 2 KiB chunks and the
> default headroom, the new check refuses MTU 1751 to 1766, which the core
> accepts.
> 
> Link: https://lore.kernel.org/all/20261028083344.3047350-3-p@1g4.org/
> Fixes: 2c6196013f84 ("igb: Add AF_XDP zero-copy Rx support")
> Cc: stable@vger.kernel.org
> Suggested-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Signed-off-by: Tjerk Kusters <tkusters@aweta.nl>
> ---
>  drivers/net/ethernet/intel/igb/igb.h      |  9 +++++++++
>  drivers/net/ethernet/intel/igb/igb_main.c | 28 +++++++++++++++++++++++++---
>  drivers/net/ethernet/intel/igb/igb_xsk.c  | 19 ++++++++++++++++++-
>  3 files changed, 52 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
> index 0fff1df81b7b..0495c02544d6 100644
> --- a/drivers/net/ethernet/intel/igb/igb.h
> +++ b/drivers/net/ethernet/intel/igb/igb.h
> @@ -856,6 +856,15 @@ int igb_add_mac_steering_filter(struct igb_adapter *adapter,
>  int igb_del_mac_steering_filter(struct igb_adapter *adapter,
>  				const u8 *addr, u8 queue, u8 flags);
>  
> +/* XDP does not support frames that span more than one Rx buffer, so a
> + * frame plus the inline Rx timestamp header must fit in one XSK buffer.
> + */
> +static inline bool igb_xsk_frame_fits(struct xsk_buff_pool *pool,
> +				      int max_frame)
> +{
> +	return max_frame + IGB_TS_HDR_LEN <= xsk_pool_get_rx_frame_size(pool);
> +}
> +
>  struct xsk_buff_pool *igb_xsk_pool(struct igb_adapter *adapter,
>  				   struct igb_ring *ring);
>  int igb_xsk_pool_setup(struct igb_adapter *adapter,
> diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
> index cf9911137bd3..b286adedad5d 100644
> --- a/drivers/net/ethernet/intel/igb/igb_main.c
> +++ b/drivers/net/ethernet/intel/igb/igb_main.c
> @@ -4833,9 +4833,13 @@ void igb_setup_srrctl(struct igb_adapter *adapter, struct igb_ring *ring)
>  	u32 srrctl = 0;
>  	u32 buf_size;
>  
> -	if (ring->xsk_pool)
> -		buf_size = xsk_pool_get_rx_frame_size(ring->xsk_pool);
> -	else if (ring_uses_large_buffer(ring))
> +	/* SRRCTL.BSIZEPKT has a 1 KiB granularity, so programming the XSK
> +	 * frame size would round it down and split frames over several
> +	 * buffers. XSK rings use the same buffer size as the other rings.
> +	 * While XDP is attached, the receive length limit keeps frames
> +	 * within the XSK frame, see igb_xsk_frame_fits().
> +	 */
> +	if (ring_uses_large_buffer(ring))
>  		buf_size = IGB_RXBUFFER_3072;
>  	else
>  		buf_size = IGB_RXBUFFER_2048;
> @@ -6775,6 +6779,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
>  {
>  	struct igb_adapter *adapter = netdev_priv(netdev);
>  	int max_frame = new_mtu + IGB_ETH_PKT_HDR_PAD;
> +	int i;
>  
>  	if (igb_xdp_is_enabled(adapter) &&
>  	    !igb_xdp_frame_fits(adapter, max_frame)) {
> @@ -6788,6 +6793,23 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
>  	if (max_frame < (ETH_FRAME_LEN + ETH_FCS_LEN))
>  		max_frame = ETH_FRAME_LEN + ETH_FCS_LEN;
>  
> +	/* the frame must fit the buffers of a bound XSK pool, also while no
> +	 * XDP program is attached, as attaching one does not check it again
> +	 */
> +	for (i = 0; i < adapter->num_rx_queues; i++) {
> +		struct xsk_buff_pool *pool = xsk_get_pool_from_qid(netdev, i);
> +
> +		if (!pool || !pool->dev)
> +			continue;
> +
> +		if (!igb_xsk_frame_fits(pool, max_frame)) {
> +			netdev_warn(adapter->netdev,
> +				    "Requested MTU size is not supported with AF_XDP. Frame size is %d\n",
> +				    max_frame);
> +			return -EINVAL;
> +		}
> +	}
> +
>  	while (test_and_set_bit(__IGB_RESETTING, &adapter->state))
>  		usleep_range(1000, 2000);
>  
> diff --git a/drivers/net/ethernet/intel/igb/igb_xsk.c b/drivers/net/ethernet/intel/igb/igb_xsk.c
> index ce4a7b58cad2..fb04df82272a 100644
> --- a/drivers/net/ethernet/intel/igb/igb_xsk.c
> +++ b/drivers/net/ethernet/intel/igb/igb_xsk.c
> @@ -91,7 +91,20 @@ struct xsk_buff_pool *igb_xsk_pool(struct igb_adapter *adapter,
>  	if (!igb_xdp_is_enabled(adapter))
>  		return NULL;
>  
> -	return (pool && pool->dev) ? pool : NULL;
> +	if (!pool || !pool->dev)
> +		return NULL;
> +
> +	/* the MTU check in igb_change_mtu() does not see a pool on a queue
> +	 * that was hidden at the time, for example by SR-IOV
> +	 */
> +	if (!igb_xsk_frame_fits(pool, adapter->max_frame_size)) {
> +		netdev_warn_once(adapter->netdev,
> +				 "XSK frame size is too small for the MTU, queue %d is not used in zero-copy mode\n",
> +				 qid);
> +		return NULL;
> +	}
> +
> +	return pool;
>  }
>  
>  static int igb_xsk_pool_enable(struct igb_adapter *adapter,
> @@ -110,6 +123,10 @@ static int igb_xsk_pool_enable(struct igb_adapter *adapter,
>  	    qid >= netdev->real_num_tx_queues)
>  		return -EINVAL;
>  
> +	/* igb_change_mtu() keeps this true while the pool is bound */
> +	if (!igb_xsk_frame_fits(pool, adapter->max_frame_size))
> +		return -EOPNOTSUPP;
> +
>  	err = xsk_pool_dma_map(pool, &adapter->pdev->dev, IGB_RX_DMA_ATTR);
>  	if (err)
>  		return err;
> 
> -- 
> 2.27.0
> 
> 

[PATCH net v5 4/4] igb: drop AF_XDP zero-copy frames that span several Rx buffers

[Tjerk Kusters via B4 Relay]

From: Tjerk Kusters <tkusters@aweta.nl>

igb_clean_rx_irq_zc() handles every descriptor as a complete frame and
strips the inline Rx timestamp header from each of them. If a frame
spans several descriptors, the AF_XDP socket gets its later buffers as
separate packets without headers. If such a buffer is shorter than the
header, the size wraps and the XDP program sees data past data_end. On
XDP_PASS, igb_construct_skb_zc() then fails to allocate an skb, and the
queue can stall on that descriptor.

The previous patches make sure that frames fit in one buffer while XDP
is attached. As ixgbe does, also drop any frame that still spans several
descriptors instead of passing its buffers on: free the buffers without
EOP, and the EOP buffer that ends such a frame. The state is kept in a
ring flag, as a frame can be split over two NAPI polls.

This depends on the previous patch. Without it, every full-size frame
on 2 KiB chunks spans two buffers and would be dropped.

Not reproduced on hardware, as with the previous patches the I350 no
longer splits frames in zero-copy mode.

Fixes: 2c6196013f84 ("igb: Add AF_XDP zero-copy Rx support")
Cc: stable@vger.kernel.org
Signed-off-by: Tjerk Kusters <tkusters@aweta.nl>
---
 drivers/net/ethernet/intel/igb/igb.h     |  1 +
 drivers/net/ethernet/intel/igb/igb_xsk.c | 23 +++++++++++++++++++++++
 2 files changed, 24 insertions(+)

diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
index 0495c02544d6..6613ea0eedb8 100644
--- a/drivers/net/ethernet/intel/igb/igb.h
+++ b/drivers/net/ethernet/intel/igb/igb.h
@@ -393,6 +393,7 @@ enum e1000_ring_flags_t {
 	IGB_RING_FLAG_TX_DETECT_HANG,
 	IGB_RING_FLAG_TX_DISABLED,
 	IGB_RING_FLAG_RX_ALLOC_FAILED,
+	IGB_RING_FLAG_RX_XSK_DISCARD,
 };
 
 #define ring_uses_large_buffer(ring) \
diff --git a/drivers/net/ethernet/intel/igb/igb_xsk.c b/drivers/net/ethernet/intel/igb/igb_xsk.c
index fb04df82272a..7b098069fff2 100644
--- a/drivers/net/ethernet/intel/igb/igb_xsk.c
+++ b/drivers/net/ethernet/intel/igb/igb_xsk.c
@@ -269,6 +269,8 @@ void igb_clean_rx_ring_zc(struct igb_ring *rx_ring)
 	u16 ntc = rx_ring->next_to_clean;
 	u16 ntu = rx_ring->next_to_use;
 
+	clear_bit(IGB_RING_FLAG_RX_XSK_DISCARD, &rx_ring->flags);
+
 	while (ntc != ntu) {
 		struct xdp_buff *xdp = rx_ring->rx_buffer_info_zc[ntc];
 
@@ -390,6 +392,27 @@ int igb_clean_rx_irq_zc(struct igb_q_vector *q_vector,
 		dma_rmb();
 
 		xdp = rx_ring->rx_buffer_info_zc[ntc];
+
+		/* A frame should never span several buffers here, see
+		 * igb_xsk_frame_fits(). If one does, drop all its buffers.
+		 */
+		if (unlikely(!igb_test_staterr(rx_desc, E1000_RXD_STAT_EOP) ||
+			     test_bit(IGB_RING_FLAG_RX_XSK_DISCARD,
+				      &rx_ring->flags))) {
+			if (igb_test_staterr(rx_desc, E1000_RXD_STAT_EOP))
+				clear_bit(IGB_RING_FLAG_RX_XSK_DISCARD,
+					  &rx_ring->flags);
+			else
+				set_bit(IGB_RING_FLAG_RX_XSK_DISCARD,
+					&rx_ring->flags);
+
+			xsk_buff_free(xdp);
+			ntc++;
+			if (ntc == rx_ring->count)
+				ntc = 0;
+			continue;
+		}
+
 		xsk_buff_set_size(xdp, size);
 		xsk_buff_dma_sync_for_cpu(xdp);
 

-- 
2.27.0

Re: [PATCH net v5 4/4] igb: drop AF_XDP zero-copy frames that span several Rx buffers

[Maciej Fijalkowski]

On Wed, Nov 04, 2026 at 07:40:05AM +0100, Tjerk Kusters via B4 Relay wrote:
> From: Tjerk Kusters <tkusters@aweta.nl>
> 
> igb_clean_rx_irq_zc() handles every descriptor as a complete frame and
> strips the inline Rx timestamp header from each of them. If a frame
> spans several descriptors, the AF_XDP socket gets its later buffers as
> separate packets without headers. If such a buffer is shorter than the
> header, the size wraps and the XDP program sees data past data_end. On
> XDP_PASS, igb_construct_skb_zc() then fails to allocate an skb, and the
> queue can stall on that descriptor.
> 
> The previous patches make sure that frames fit in one buffer while XDP
> is attached. As ixgbe does, also drop any frame that still spans several
> descriptors instead of passing its buffers on: free the buffers without
> EOP, and the EOP buffer that ends such a frame. The state is kept in a
> ring flag, as a frame can be split over two NAPI polls.
> 
> This depends on the previous patch. Without it, every full-size frame
> on 2 KiB chunks spans two buffers and would be dropped.
> 
> Not reproduced on hardware, as with the previous patches the I350 no
> longer splits frames in zero-copy mode.

I am missing the point of this patch then. Are you adding a dead code to
hot path?

ixgbe was probably problematic to address all supported hw (i think 82599
has no per-q registers and would need different treatment on config side?)
that's why it was chosen to carry the discard logic.

But here if you specifically are addressing the config side of things on
igb I am having a hard time to understand the purpose of this patch.

> 
> Fixes: 2c6196013f84 ("igb: Add AF_XDP zero-copy Rx support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tjerk Kusters <tkusters@aweta.nl>
> ---
>  drivers/net/ethernet/intel/igb/igb.h     |  1 +
>  drivers/net/ethernet/intel/igb/igb_xsk.c | 23 +++++++++++++++++++++++
>  2 files changed, 24 insertions(+)
> 
> diff --git a/drivers/net/ethernet/intel/igb/igb.h b/drivers/net/ethernet/intel/igb/igb.h
> index 0495c02544d6..6613ea0eedb8 100644
> --- a/drivers/net/ethernet/intel/igb/igb.h
> +++ b/drivers/net/ethernet/intel/igb/igb.h
> @@ -393,6 +393,7 @@ enum e1000_ring_flags_t {
>  	IGB_RING_FLAG_TX_DETECT_HANG,
>  	IGB_RING_FLAG_TX_DISABLED,
>  	IGB_RING_FLAG_RX_ALLOC_FAILED,
> +	IGB_RING_FLAG_RX_XSK_DISCARD,
>  };
>  
>  #define ring_uses_large_buffer(ring) \
> diff --git a/drivers/net/ethernet/intel/igb/igb_xsk.c b/drivers/net/ethernet/intel/igb/igb_xsk.c
> index fb04df82272a..7b098069fff2 100644
> --- a/drivers/net/ethernet/intel/igb/igb_xsk.c
> +++ b/drivers/net/ethernet/intel/igb/igb_xsk.c
> @@ -269,6 +269,8 @@ void igb_clean_rx_ring_zc(struct igb_ring *rx_ring)
>  	u16 ntc = rx_ring->next_to_clean;
>  	u16 ntu = rx_ring->next_to_use;
>  
> +	clear_bit(IGB_RING_FLAG_RX_XSK_DISCARD, &rx_ring->flags);
> +
>  	while (ntc != ntu) {
>  		struct xdp_buff *xdp = rx_ring->rx_buffer_info_zc[ntc];
>  
> @@ -390,6 +392,27 @@ int igb_clean_rx_irq_zc(struct igb_q_vector *q_vector,
>  		dma_rmb();
>  
>  		xdp = rx_ring->rx_buffer_info_zc[ntc];
> +
> +		/* A frame should never span several buffers here, see
> +		 * igb_xsk_frame_fits(). If one does, drop all its buffers.
> +		 */
> +		if (unlikely(!igb_test_staterr(rx_desc, E1000_RXD_STAT_EOP) ||
> +			     test_bit(IGB_RING_FLAG_RX_XSK_DISCARD,
> +				      &rx_ring->flags))) {
> +			if (igb_test_staterr(rx_desc, E1000_RXD_STAT_EOP))
> +				clear_bit(IGB_RING_FLAG_RX_XSK_DISCARD,
> +					  &rx_ring->flags);
> +			else
> +				set_bit(IGB_RING_FLAG_RX_XSK_DISCARD,
> +					&rx_ring->flags);
> +
> +			xsk_buff_free(xdp);
> +			ntc++;
> +			if (ntc == rx_ring->count)
> +				ntc = 0;
> +			continue;
> +		}
> +
>  		xsk_buff_set_size(xdp, size);
>  		xsk_buff_dma_sync_for_cpu(xdp);
>  
> 
> -- 
> 2.27.0
> 
>