From: Tjerk Kusters <[email protected]> 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/[email protected]/ Fixes: 2c6196013f84 ("igb: Add AF_XDP zero-copy Rx support") Cc: [email protected] Suggested-by: Maciej Fijalkowski <[email protected]> Signed-off-by: Tjerk Kusters <[email protected]> --- 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
