mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH net v4 0/2] igb: only strip the Rx timestamp header from the first buffer of a frame
@ 2026-11-02  9:58 Tjerk Kusters via B4 Relay
  2026-11-02  9:58 ` [PATCH net v4 1/2] igb: do not let XDP frames span more than one Rx buffer Tjerk Kusters via B4 Relay
                   ` (2 more replies)
  0 siblings, 3 replies; 5+ messages in thread
From: Tjerk Kusters via B4 Relay @ 2026-11-02  9:58 UTC (permalink / raw)
  To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	Stanislav Fomichev, Sven Auhagen, Richard Cochran,
	Kurt Kanzenbach
  Cc: intel-wired-lan, netdev, linux-kernel, bpf, Maciej Fijalkowski,
	Paul Moses, stable, Tjerk Kusters, Piotr Kwapulinski,
	Aleksandr Loktionov, Alexander Nowlin

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/2 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.
  2/2 is v3, unchanged in code, with a note that it relies on 1/2.
      The Reviewed-by and Tested-by tags of v3 are kept for that reason.

Not addressed here: the AF_XDP zero-copy Rx path. igb_clean_rx_irq_zc()
has the same unguarded strip, and with 2 KiB UMEM chunks the hardware
buffer is rounded down to 1 KiB, so frames span descriptors there. The
igc counterpart is Maciej's buffer-size patch in [2]. I do not use
AF_XDP and cannot test it, so I would prefer to leave that to someone
who can.

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

Tested on an I350, kernel 6.14.9 with these patches, 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 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.
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 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 (2):
      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

 drivers/net/ethernet/intel/igb/igb_main.c | 37 ++++++++++++++++++++++---------
 1 file changed, 26 insertions(+), 11 deletions(-)
---
base-commit: 2d3090a8aeb596a26935db0955d46c9a5db5c6ce
change-id: 20260619-igb-rx-ts-fix-cd70585ee316

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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net v4 1/2] igb: do not let XDP frames span more than one Rx buffer
  2026-11-02  9:58 [PATCH net v4 0/2] igb: only strip the Rx timestamp header from the first buffer of a frame Tjerk Kusters via B4 Relay
@ 2026-11-02  9:58 ` Tjerk Kusters via B4 Relay
  2026-11-02  9:58 ` [PATCH net v4 2/2] igb: only strip Rx timestamp header on the first buffer of a frame Tjerk Kusters via B4 Relay
  2026-11-02 13:28 ` [PATCH net v4 0/2] igb: only strip the Rx timestamp header from " Maciej Fijalkowski
  2 siblings, 0 replies; 5+ messages in thread
From: Tjerk Kusters via B4 Relay @ 2026-11-02  9:58 UTC (permalink / raw)
  To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	Stanislav Fomichev, Sven Auhagen, Richard Cochran,
	Kurt Kanzenbach
  Cc: intel-wired-lan, netdev, linux-kernel, bpf, Maciej Fijalkowski,
	Paul Moses, stable, Tjerk Kusters

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, and the PF pool limit
in VMOLR is set the same way. Otherwise the MAC still accepts frames of
up to MAX_JUMBO_FRAME_SIZE while XDP is attached, for example at MTU
3000, at any MTU when PAGE_SIZE >= 8192, or when VFs are allocated.

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 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; this lowers the largest MTU accepted with
XDP on 3 KiB buffers by 16 bytes.

Both registers are programmed from igb_set_rx_mode(), which
igb_configure() calls, and attaching or removing an XDP program on a
running interface goes through igb_close() and igb_open(), as does an
MTU change.

This follows the same approach as the igc fix that limits RLPML while
XDP is attached.

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 3030 still works, at
MTU 3046 it is refused, as is an MTU change from 3030 to 3046 with XDP
attached.

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.

Link: https://lore.kernel.org/all/20261028083344.3047350-3-p@1g4.org/
Fixes: 9cbc948b5a20 ("igb: add XDP support")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-5-5
Signed-off-by: Tjerk Kusters <tkusters@aweta.nl>
---
 drivers/net/ethernet/intel/igb/igb_main.c | 30 +++++++++++++++++++++---------
 1 file changed, 21 insertions(+), 9 deletions(-)

diff --git a/drivers/net/ethernet/intel/igb/igb_main.c b/drivers/net/ethernet/intel/igb/igb_main.c
index ce91dda00ec0..5bf489cd7b06 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -2894,11 +2894,13 @@ 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 */
+	/* verify igb ring attributes are sufficient for XDP, leaving room
+	 * for the inline Rx timestamp header
+	 */
 	for (i = 0; i < adapter->num_rx_queues; i++) {
 		struct igb_ring *ring = adapter->rx_ring[i];
 
-		if (frame_size > igb_rx_bufsz(ring)) {
+		if (frame_size + IGB_TS_HDR_LEN > 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",
@@ -5324,6 +5326,7 @@ static void igb_set_rx_mode(struct net_device *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 pool_rlpml;
 	int count;
 
 	/* Check for Promiscuous and All Multicast modes */
@@ -5386,6 +5389,12 @@ static void igb_set_rx_mode(struct net_device *netdev)
 			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 below.
+	 */
+	if (!adapter->vfs_allocated_count && igb_xdp_is_enabled(adapter))
+		rlpml = min_t(u32, rlpml, adapter->max_frame_size);
 	wr32(E1000_RLPML, rlpml);
 
 	/* In order to support SR-IOV and eventually VMDq it is necessary to set
@@ -5402,15 +5411,18 @@ static void igb_set_rx_mode(struct net_device *netdev)
 	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;
+	/* enable Rx jumbo frames, restrict as needed to support build_skb
+	 * and XDP
+	 */
+	pool_rlpml = MAX_JUMBO_FRAME_SIZE;
 #if (PAGE_SIZE < 8192)
 	if (adapter->max_frame_size <= IGB_MAX_FRAME_BUILD_SKB)
-		vmolr |= IGB_MAX_FRAME_BUILD_SKB;
-	else
+		pool_rlpml = IGB_MAX_FRAME_BUILD_SKB;
 #endif
-		vmolr |= MAX_JUMBO_FRAME_SIZE;
-	vmolr |= E1000_VMOLR_LPE;
+	if (igb_xdp_is_enabled(adapter))
+		pool_rlpml = min_t(u32, pool_rlpml, adapter->max_frame_size);
+	vmolr &= ~E1000_VMOLR_RLPML_MASK;
+	vmolr |= pool_rlpml | E1000_VMOLR_LPE;
 
 	wr32(E1000_VMOLR(vfn), vmolr);
 
@@ -6713,7 +6725,7 @@ static int igb_change_mtu(struct net_device *netdev, int new_mtu)
 		for (i = 0; i < adapter->num_rx_queues; i++) {
 			struct igb_ring *ring = adapter->rx_ring[i];
 
-			if (max_frame > igb_rx_bufsz(ring)) {
+			if (max_frame + IGB_TS_HDR_LEN > igb_rx_bufsz(ring)) {
 				netdev_warn(adapter->netdev,
 					    "Requested MTU size is not supported with XDP. Max frame size is %d\n",
 					    max_frame);

-- 
2.27.0



^ permalink raw reply	[flat|nested] 5+ messages in thread

* [PATCH net v4 2/2] igb: only strip Rx timestamp header on the first buffer of a frame
  2026-11-02  9:58 [PATCH net v4 0/2] igb: only strip the Rx timestamp header from the first buffer of a frame Tjerk Kusters via B4 Relay
  2026-11-02  9:58 ` [PATCH net v4 1/2] igb: do not let XDP frames span more than one Rx buffer Tjerk Kusters via B4 Relay
@ 2026-11-02  9:58 ` Tjerk Kusters via B4 Relay
  2026-11-02 13:28 ` [PATCH net v4 0/2] igb: only strip the Rx timestamp header from " Maciej Fijalkowski
  2 siblings, 0 replies; 5+ messages in thread
From: Tjerk Kusters via B4 Relay @ 2026-11-02  9:58 UTC (permalink / raw)
  To: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	Stanislav Fomichev, Sven Auhagen, Richard Cochran,
	Kurt Kanzenbach
  Cc: intel-wired-lan, netdev, linux-kernel, bpf, Maciej Fijalkowski,
	Paul Moses, stable, Piotr Kwapulinski, Aleksandr Loktionov,
	Alexander Nowlin, Tjerk Kusters

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.

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 and is fixed 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>
Assisted-by: Claude:claude-opus-5-5
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 5bf489cd7b06..c4eb9f589fbe 100644
--- a/drivers/net/ethernet/intel/igb/igb_main.c
+++ b/drivers/net/ethernet/intel/igb/igb_main.c
@@ -9072,8 +9072,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



^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH net v4 0/2] igb: only strip the Rx timestamp header from the first buffer of a frame
  2026-11-02  9:58 [PATCH net v4 0/2] igb: only strip the Rx timestamp header from the first buffer of a frame Tjerk Kusters via B4 Relay
  2026-11-02  9:58 ` [PATCH net v4 1/2] igb: do not let XDP frames span more than one Rx buffer Tjerk Kusters via B4 Relay
  2026-11-02  9:58 ` [PATCH net v4 2/2] igb: only strip Rx timestamp header on the first buffer of a frame Tjerk Kusters via B4 Relay
@ 2026-11-02 13:28 ` Maciej Fijalkowski
  2026-11-02 14:55   ` Tjerk Kusters
  2 siblings, 1 reply; 5+ messages in thread
From: Maciej Fijalkowski @ 2026-11-02 13:28 UTC (permalink / raw)
  To: tkusters
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	Stanislav Fomichev, Sven Auhagen, Richard Cochran,
	Kurt Kanzenbach, intel-wired-lan, netdev, linux-kernel, bpf,
	Paul Moses, stable, Piotr Kwapulinski, Aleksandr Loktionov,
	Alexander Nowlin

On Mon, Nov 02, 2026 at 10:58:12AM +0100, Tjerk Kusters via B4 Relay wrote:
> 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/2 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.
>   2/2 is v3, unchanged in code, with a note that it relies on 1/2.
>       The Reviewed-by and Tested-by tags of v3 are kept for that reason.
> 
> Not addressed here: the AF_XDP zero-copy Rx path. igb_clean_rx_irq_zc()
> has the same unguarded strip, and with 2 KiB UMEM chunks the hardware
> buffer is rounded down to 1 KiB, so frames span descriptors there. The
> igc counterpart is Maciej's buffer-size patch in [2]. I do not use
> AF_XDP and cannot test it, so I would prefer to leave that to someone
> who can.

I assume review bots will give you a hard time if you don't address this
within your series. Seems you have HW handy plus you do use LLM so I don't
see why you couldn't ask it to address AF_XDP zc rx ?

FWIW blessed user space tool has always been xdpsock:
https://github.com/xdp-project/bpf-examples/tree/main/AF_XDP-example

so you can ask llm to run this in rxdrop mode on receiver side and feed it
with some traffic from peer side and then see that frames were not
fragmented.

It's easier for you to cover this as you have HW.

> 
> 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
> 
> Tested on an I350, kernel 6.14.9 with these patches, 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 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.
> 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 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 (2):
>       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
> 
>  drivers/net/ethernet/intel/igb/igb_main.c | 37 ++++++++++++++++++++++---------
>  1 file changed, 26 insertions(+), 11 deletions(-)
> ---
> base-commit: 2d3090a8aeb596a26935db0955d46c9a5db5c6ce
> change-id: 20260619-igb-rx-ts-fix-cd70585ee316
> 
> Best regards,
> --  
> Tjerk Kusters <tkusters@aweta.nl>
> 
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH net v4 0/2] igb: only strip the Rx timestamp header from the first buffer of a frame
  2026-11-02 13:28 ` [PATCH net v4 0/2] igb: only strip the Rx timestamp header from " Maciej Fijalkowski
@ 2026-11-02 14:55   ` Tjerk Kusters
  0 siblings, 0 replies; 5+ messages in thread
From: Tjerk Kusters @ 2026-11-02 14:55 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: Tony Nguyen, Przemek Kitszel, Andrew Lunn, David S. Miller,
	Eric Dumazet, Jakub Kicinski, Paolo Abeni, Alexei Starovoitov,
	Daniel Borkmann, Jesper Dangaard Brouer, John Fastabend,
	Stanislav Fomichev, Sven Auhagen, Richard Cochran,
	Kurt Kanzenbach, intel-wired-lan, netdev, linux-kernel, bpf,
	Paul Moses, stable, Piotr Kwapulinski, Aleksandr Loktionov,
	Alexander Nowlin

> Not addressed here: the AF_XDP zero-copy Rx path.
> igb_clean_rx_irq_zc() has the same unguarded strip, and with 2 KiB
> UMEM chunks the hardware buffer is rounded down to 1 KiB, so frames
> span descriptors there. The igc counterpart is Maciej's buffer-size
> patch in [2]. I do not use AF_XDP and cannot test it, so I would
> prefer to leave that to someone who can.

> I assume review bots will give you a hard time if you don't address
> this within your series. Seems you have HW handy plus you do use LLM
> so I don't see why you couldn't ask it to address AF_XDP zc rx ?
>
> FWIW blessed user space tool has always been xdpsock:
> https://github.com/xdp-project/bpf-examples/tree/main/AF_XDP-example
>
> so you can ask llm to run this in rxdrop mode on receiver side and
> feed it with some traffic from peer side and then see that frames
> were not fragmented.


OK, fair point. I will look into it, and I will hold v5 until that is done and tested.

Thanks,
Tjerk

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-11-02 14:55 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-11-02  9:58 [PATCH net v4 0/2] igb: only strip the Rx timestamp header from the first buffer of a frame Tjerk Kusters via B4 Relay
2026-11-02  9:58 ` [PATCH net v4 1/2] igb: do not let XDP frames span more than one Rx buffer Tjerk Kusters via B4 Relay
2026-11-02  9:58 ` [PATCH net v4 2/2] igb: only strip Rx timestamp header on the first buffer of a frame Tjerk Kusters via B4 Relay
2026-11-02 13:28 ` [PATCH net v4 0/2] igb: only strip the Rx timestamp header from " Maciej Fijalkowski
2026-11-02 14:55   ` Tjerk Kusters

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®