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

[Maciej Fijalkowski <maciej.fijalkowski@intel.com>]

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