On Monday, October 26th, 2026 at 2:46 PM, Maciej Fijalkowski
<[email protected]> wrote:
> On Sun, Oct 25, 2026 at 06:52:06AM +0000, Paul Moses wrote:
> > TSIP remains set on continuation descriptors, but the inline timestamp
> > header exists only in the first buffer. Stripping it from every
> > descriptor truncates split frames and can underflow short buffers.
> >
> > skb ownership cannot identify frame boundaries when XDP consumes the
> > first buffer or AF_XDP uses zero-copy.
> >
> > Track rx_in_frame per ring and update it as descriptors are consumed.
> > Strip timestamps only from first buffers, reject unsupported split
> > frames before XDP and drain continuations. Account discarded
> > descriptors against NAPI work without counting successful RX.
> >
> > Fixes: e1ed4f92a625 ("igc: Refactor Rx timestamp handling")
> > Fixes: 26575105d6ed ("igc: Add initial XDP support")
> > Fixes: fc9df2a0b520 ("igc: Enable RX via AF_XDP zero-copy")
> > Cc: [email protected]
> > Signed-off-by: Paul Moses <[email protected]>
> > ---
> > - This supersedes "igc: only strip RX timestamp header from first buffer"
> > following feedback from Simon Horman, Maciej Fijalkowski and Sashiko.
> >
> > Changes:
> > - track frame boundaries independently of skb ownership
> > - guard timestamp stripping and drain unsupported split frames
> > - cover the AF_XDP zero-copy RX path
> > ---
> > drivers/net/ethernet/intel/igc/igc.h | 1 +
> > drivers/net/ethernet/intel/igc/igc_main.c | 87 ++++++++++++++++-------
> > 2 files changed, 64 insertions(+), 24 deletions(-)
>
> Hi Paul,
>
> I think this implementation went off track. We should make sure we *never*
> enter data path for XDP/AF_XDP multi-buffer scenario until we add real
> support to it within this driver. Now you sorta handle fragmented packets
> in data path.
>
> I tried to argue with bot reports that it hallucinated for
> igc_clean_rx_irq() content (which was concise and to the point) and agreed
> the AF_XDP should *not* be exposed to any fragmented traffic, which should
> be addressed on HW RX ring configuration side, not within data path.
>
> Also this should have been a v3 with a link to previous revisions so we
> could keep track what has been changing between versions.
>
> Here's something that would address ZC fragmentation issue. Could you give
> it a spin on your side as I don't currently have igc in my setup handy.
>
>
> From a3edec7db135f13a567565a652da5cc850505f7f Mon Sep 17 00:00:00 2001
> From: Maciej Fijalkowski <[email protected]>
> Date: Mon, 26 Oct 2026 15:00:00 +0100
> Subject: [PATCH net] igc: fix AF_XDP receive buffer size for 2K UMEM chunks
>
> 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 <[email protected]>
> ---
> 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 17f213cc93e4..ad1062f8531a 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 1fb5f3cbe93c..38a5702647f4 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);
> }
>
> @@ -4116,7 +4139,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 */
> @@ -4146,12 +4169,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 9eb47b4beb06..a0f1ecb2ed5f 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.43.0
>
>
> >
> > diff --git a/drivers/net/ethernet/intel/igc/igc.h
> > b/drivers/net/ethernet/intel/igc/igc.h
> > index 17f213cc93e4..635d49701e62 100644
> > --- a/drivers/net/ethernet/intel/igc/igc.h
> > +++ b/drivers/net/ethernet/intel/igc/igc.h
> > @@ -198,6 +198,7 @@ struct igc_ring {
> > struct igc_rx_packet_stats pkt_stats;
> > struct u64_stats_sync rx_syncp;
> > struct sk_buff *skb;
> > + bool rx_in_frame;
> > };
> > };
> >
> > diff --git a/drivers/net/ethernet/intel/igc/igc_main.c
> > b/drivers/net/ethernet/intel/igc/igc_main.c
> > index 1fb5f3cbe93c..9ca5558c762c 100644
> > --- a/drivers/net/ethernet/intel/igc/igc_main.c
> > +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> > @@ -474,6 +474,7 @@ static void igc_clean_rx_ring(struct igc_ring *ring)
> > igc_clean_rx_ring_page_shared(ring);
> >
> > clear_ring_uses_large_buffer(ring);
> > + ring->rx_in_frame = false;
> >
> > ring->next_to_alloc = 0;
> > ring->next_to_clean = 0;
> > @@ -673,6 +674,7 @@ static void igc_configure_rx_ring(struct igc_adapter
> > *adapter,
> > /* reset next-to- use/clean to place SW in sync with hardware */
> > ring->next_to_clean = 0;
> > ring->next_to_use = 0;
> > + ring->rx_in_frame = false;
> >
> > if (ring->xsk_pool)
> > buf_size = xsk_pool_get_rx_frame_size(ring->xsk_pool);
> > @@ -2111,10 +2113,8 @@ static bool igc_can_reuse_rx_page(struct
> > igc_rx_buffer *rx_buffer,
> > * @rx_ring: Rx ring being processed
> > * @rx_desc: Rx descriptor for current buffer
> > *
> > - * This function updates next to clean. If the buffer is an EOP buffer
> > - * this function exits returning false, otherwise it will place the
> > - * sk_buff in the next buffer to be chained and return true indicating
> > - * that this is in fact a non-EOP buffer.
> > + * Advance next to clean and retain frame position for the next descriptor.
> > + * Return true if the consumed descriptor does not end the frame.
> > */
> > static bool igc_is_non_eop(struct igc_ring *rx_ring,
> > union igc_adv_rx_desc *rx_desc)
> > @@ -2127,10 +2127,8 @@ static bool igc_is_non_eop(struct igc_ring *rx_ring,
> >
> > prefetch(IGC_RX_DESC(rx_ring, ntc));
> >
> > - if (likely(igc_test_staterr(rx_desc, IGC_RXD_STAT_EOP)))
> > - return false;
> > -
> > - return true;
> > + rx_ring->rx_in_frame = !igc_test_staterr(rx_desc, IGC_RXD_STAT_EOP);
> > + return unlikely(rx_ring->rx_in_frame);
> > }
> >
> > /**
> > @@ -2601,7 +2599,7 @@ static void igc_update_rx_stats(struct igc_q_vector
> > *q_vector,
> >
> > static int igc_clean_rx_irq(struct igc_q_vector *q_vector, const int
> > budget)
> > {
> > - unsigned int total_bytes = 0, total_packets = 0;
> > + unsigned int total_bytes = 0, total_packets = 0, work_done = 0;
> > struct igc_adapter *adapter = q_vector->adapter;
> > struct igc_ring *rx_ring = q_vector->rx.ring;
> > struct sk_buff *skb = rx_ring->skb;
> > @@ -2609,13 +2607,15 @@ static int igc_clean_rx_irq(struct igc_q_vector
> > *q_vector, const int budget)
> > int xdp_status = 0, rx_buffer_pgcnt;
> > int xdp_res = 0;
> >
> > - while (likely(total_packets < budget)) {
> > + while (likely(work_done < budget)) {
> > struct igc_xdp_buff ctx = { .rx_ts = NULL };
> > + bool first = !rx_ring->rx_in_frame;
> > struct igc_rx_buffer *rx_buffer;
> > union igc_adv_rx_desc *rx_desc;
> > unsigned int size, truesize;
> > int pkt_offset = 0;
> > void *pktbuf;
> > + bool tsip;
> >
> > /* return some buffers to hardware, one at a time is too slow */
> > if (cleaned_count >= IGC_RX_BUFFER_WRITE) {
> > @@ -2635,21 +2635,34 @@ static int igc_clean_rx_irq(struct igc_q_vector
> > *q_vector, const int budget)
> > dma_rmb();
> >
> > rx_buffer = igc_get_rx_buffer(rx_ring, size, &rx_buffer_pgcnt);
> > + tsip = first && igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP);
> > +
> > + if ((!first && !skb) ||
> > + (first && igc_xdp_is_enabled(adapter) &&
> > + !igc_test_staterr(rx_desc, IGC_RXD_STAT_EOP)) ||
> > + unlikely(tsip && size < IGC_TS_HDR_LEN)) {
> > + if (first)
> > + dev_core_stats_rx_dropped_inc(rx_ring->netdev);
> > + xdp_res = IGC_XDP_CONSUMED;
> > + goto xdp_done;
> > + }
> > +
> > truesize = igc_get_rx_frame_truesize(rx_ring, size);
> >
> > pktbuf = page_address(rx_buffer->page) + rx_buffer->page_offset;
> >
> > - if (igc_test_staterr(rx_desc, IGC_RXDADV_STAT_TSIP)) {
> > + if (tsip) {
> > ctx.rx_ts = pktbuf;
> > pkt_offset = IGC_TS_HDR_LEN;
> > size -= IGC_TS_HDR_LEN;
> > }
> >
> > - if (igc_fpe_is_pmac_enabled(adapter) &&
> > + if (first && igc_fpe_is_pmac_enabled(adapter) &&
> > igc_fpe_handle_mpacket(adapter, rx_desc, size, pktbuf +
> > pkt_offset)) {
> > /* Advance the ring next-to-clean */
> > igc_is_non_eop(rx_ring, rx_desc);
> > cleaned_count++;
> > + work_done++;
> > continue;
> > }
> >
> > @@ -2664,6 +2677,12 @@ static int igc_clean_rx_irq(struct igc_q_vector
> > *q_vector, const int budget)
> > xdp_res = igc_xdp_run_prog(adapter, &ctx.xdp);
> > }
> >
> > + if (xdp_res) {
> > + total_bytes += size;
> > + total_packets++;
> > + }
> > +
> > +xdp_done:
> > if (xdp_res) {
> > switch (xdp_res) {
> > case IGC_XDP_CONSUMED:
> > @@ -2676,8 +2695,7 @@ static int igc_clean_rx_irq(struct igc_q_vector
> > *q_vector, const int budget)
> > break;
> > }
> >
> > - total_packets++;
> > - total_bytes += size;
> > + work_done++;
> > } else if (skb)
> > igc_add_rx_frag(rx_ring, rx_buffer, skb, size);
> > else if (ring_uses_build_skb(rx_ring))
> > @@ -2700,8 +2718,15 @@ static int igc_clean_rx_irq(struct igc_q_vector
> > *q_vector, const int budget)
> > if (igc_is_non_eop(rx_ring, rx_desc))
> > continue;
> >
> > + if (xdp_res) {
> > + skb = NULL;
> > + continue;
> > + }
> > +
> > + work_done++;
> > +
> > /* verify the packet layout is correct */
> > - if (xdp_res || igc_cleanup_headers(rx_ring, rx_desc, skb)) {
> > + if (igc_cleanup_headers(rx_ring, rx_desc, skb)) {
> > skb = NULL;
> > continue;
> > }
> > @@ -2717,7 +2742,6 @@ static int igc_clean_rx_irq(struct igc_q_vector
> > *q_vector, const int budget)
> > /* reset skb pointer */
> > skb = NULL;
> >
> > - /* update budget accounting */
> > total_packets++;
> > }
> >
> > @@ -2732,7 +2756,7 @@ static int igc_clean_rx_irq(struct igc_q_vector
> > *q_vector, const int budget)
> > if (cleaned_count)
> > igc_alloc_rx_buffers(rx_ring, cleaned_count);
> >
> > - return total_packets;
> > + return work_done;
> > }
> >
> > static struct sk_buff *igc_construct_skb_zc(struct igc_ring *ring,
> > @@ -2797,10 +2821,10 @@ static struct igc_xdp_buff
> > *xsk_buff_to_igc_ctx(struct xdp_buff *xdp)
> >
> > static int igc_clean_rx_irq_zc(struct igc_q_vector *q_vector, const int
> > budget)
> > {
> > + int total_bytes = 0, total_packets = 0, work_done = 0;
> > struct igc_adapter *adapter = q_vector->adapter;
> > struct igc_ring *ring = q_vector->rx.ring;
> > u16 cleaned_count = igc_desc_unused(ring);
> > - int total_bytes = 0, total_packets = 0;
> > u16 ntc = ring->next_to_clean;
> > struct bpf_prog *prog;
> > bool failure = false;
> > @@ -2810,11 +2834,13 @@ static int igc_clean_rx_irq_zc(struct igc_q_vector
> > *q_vector, const int budget)
> >
> > prog = READ_ONCE(adapter->xdp_prog);
> >
> > - while (likely(total_packets < budget)) {
> > + while (likely(work_done < budget)) {
> > + bool first = !ring->rx_in_frame;
> > union igc_adv_rx_desc *desc;
> > struct igc_rx_buffer *bi;
> > struct igc_xdp_buff *ctx;
> > unsigned int size;
> > + bool eop, tsip;
> > int res;
> >
> > desc = IGC_RX_DESC(ring, ntc);
> > @@ -2829,11 +2855,20 @@ static int igc_clean_rx_irq_zc(struct igc_q_vector
> > *q_vector, const int budget)
> > dma_rmb();
> >
> > bi = &ring->rx_buffer_info[ntc];
> > + tsip = first && igc_test_staterr(desc, IGC_RXDADV_STAT_TSIP);
> > + eop = igc_test_staterr(desc, IGC_RXD_STAT_EOP);
> > +
> > + if (!first || !eop || unlikely(tsip && size < IGC_TS_HDR_LEN)) {
> > + if (first)
> > + dev_core_stats_rx_dropped_inc(ring->netdev);
> > + res = IGC_XDP_CONSUMED;
> > + goto xdp_done;
> > + }
> >
> > ctx = xsk_buff_to_igc_ctx(bi->xdp);
> > ctx->rx_desc = desc;
> >
> > - if (igc_test_staterr(desc, IGC_RXDADV_STAT_TSIP)) {
> > + if (tsip) {
> > ctx->rx_ts = bi->xdp->data;
> >
> > bi->xdp->data += IGC_TS_HDR_LEN;
> > @@ -2851,6 +2886,10 @@ static int igc_clean_rx_irq_zc(struct igc_q_vector
> > *q_vector, const int budget)
> > xsk_buff_dma_sync_for_cpu(bi->xdp);
> >
> > res = __igc_xdp_run_prog(adapter, prog, bi->xdp);
> > + total_bytes += size;
> > + total_packets++;
> > +
> > +xdp_done:
> > switch (res) {
> > case IGC_XDP_PASS:
> > igc_dispatch_skb_zc(q_vector, desc, ctx);
> > @@ -2865,8 +2904,8 @@ static int igc_clean_rx_irq_zc(struct igc_q_vector
> > *q_vector, const int budget)
> > }
> >
> > bi->xdp = NULL;
> > - total_bytes += size;
> > - total_packets++;
> > + ring->rx_in_frame = !eop;
> > + work_done++;
> > cleaned_count++;
> > ntc++;
> > if (ntc == ring->count)
> > @@ -2889,10 +2928,10 @@ static int igc_clean_rx_irq_zc(struct igc_q_vector
> > *q_vector, const int budget)
> > xsk_set_rx_need_wakeup(ring->xsk_pool);
> > else
> > xsk_clear_rx_need_wakeup(ring->xsk_pool);
> > - return total_packets;
> > + return work_done;
> > }
> >
> > - return failure ? budget : total_packets;
> > + return failure ? budget : work_done;
> > }
> >
> > static void igc_update_tx_stats(struct igc_q_vector *q_vector,
> >
>
Looks good to me, passes basic testing, and I would rather you/intel do this
patch
For posterity, I did email Tony with handoff notes stating I'd rather not do
the
xdp/xsk/af_xdp/fpe side of things, but hadn't heard back so I kept going.
