From: Tjerk Kusters 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. With VFs, the PF keeps a single Tx queue, and one Rx queue on i350 or two on 82576. A pool bound to a queue above that would lose its Rx queue, which the MTU check then no longer sees, or its Tx queue, which breaks the socket. The core refuses ethtool -L in that case (with -EINVAL). Likewise, refuse to enable VFs with -EBUSY while an XSK pool is bound to a queue the PF would lose. On that error, igb_enable_sriov() also restores the DMA coalescing flag it cleared. The MAC filter entries it reserved for the VFs before the check stay reserved until the next reset. A queue can also be hidden without such a request, for example when the driver falls back from MSI-X to MSI and keeps one queue. The MTU check does not see a pool on that queue, so igb_xsk_pool() checks the frame size again when the ring is set up, and does not use zero-copy if the frame does not fit. The socket then receives through the copy path, and its Tx wakeup fails with EINVAL, until the ring is set up again with an MTU that fits. This path was not tested. 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. On 7.3.1, with xdpsock bound to queue 1 of an I350, enabling two VFs is now refused with EBUSY and the socket keeps its queue; with the socket on queue 0, which the PF keeps, the VFs are enabled. The 82576 case, with two Rx queues and one Tx queue for the PF, was checked with the driver built so that the I350 keeps two Rx queues with VFs: with the socket on queue 1, the VF enable is refused as well. Only the refusal was checked. 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 Signed-off-by: Tjerk Kusters --- drivers/net/ethernet/intel/igb/igb.h | 9 ++++++ drivers/net/ethernet/intel/igb/igb_main.c | 51 +++++++++++++++++++++++++++++-- drivers/net/ethernet/intel/igb/igb_xsk.c | 20 +++++++++++- 3 files changed, 76 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 e787095d3cdb..c9351a8c4f5e 100644 --- a/drivers/net/ethernet/intel/igb/igb_main.c +++ b/drivers/net/ethernet/intel/igb/igb_main.c @@ -3777,9 +3777,29 @@ static int igb_sriov_reinit(struct pci_dev *dev) struct net_device *netdev = pci_get_drvdata(dev); struct igb_adapter *adapter = netdev_priv(netdev); struct pci_dev *pdev = adapter->pdev; + unsigned int i, rxq, txq, end; rtnl_lock(); + /* With VFs the PF gets fewer Rx queues and a single Tx queue, see + * igb_set_interrupt_capability(). Refuse that while an XSK pool is + * bound to a queue the PF would lose; the core refuses ethtool -L in + * the same case. + */ + rxq = min_t(unsigned int, igb_get_max_rss_queues(adapter), + num_online_cpus()); + txq = adapter->vfs_allocated_count ? 1 : rxq; + end = max(netdev->real_num_rx_queues, netdev->real_num_tx_queues); + for (i = min(rxq, txq); i < end; i++) { + if (xsk_get_pool_from_qid(netdev, i)) { + rtnl_unlock(); + dev_err(&pdev->dev, + "Cannot change the number of queues, an AF_XDP socket is bound to queue %u\n", + i); + return -EBUSY; + } + } + if (netif_running(netdev)) igb_close(netdev); else @@ -3844,6 +3864,7 @@ static int igb_enable_sriov(struct pci_dev *pdev, int num_vfs, bool reinit) { struct net_device *netdev = pci_get_drvdata(pdev); struct igb_adapter *adapter = netdev_priv(netdev); + bool dmac = adapter->flags & IGB_FLAG_DMAC; int old_vfs = pci_num_vf(pdev); struct vf_mac_filter *mac_list; int err = 0; @@ -3933,6 +3954,8 @@ static int igb_enable_sriov(struct pci_dev *pdev, int num_vfs, bool reinit) kfree(adapter->vf_data); adapter->vf_data = NULL; adapter->vfs_allocated_count = 0; + if (dmac) + adapter->flags |= IGB_FLAG_DMAC; out: return err; } @@ -4836,9 +4859,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; @@ -6778,6 +6805,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)) { @@ -6791,6 +6819,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..7f99e834c1ed 100644 --- a/drivers/net/ethernet/intel/igb/igb_xsk.c +++ b/drivers/net/ethernet/intel/igb/igb_xsk.c @@ -91,7 +91,21 @@ 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; + + /* igb_change_mtu() does not see a pool on a queue that is hidden, for + * example after a fallback from MSI-X to MSI. The hardware buffer can + * be larger than the XSK frame, so check again when the ring is set up. + */ + 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 +124,10 @@ static int igb_xsk_pool_enable(struct igb_adapter *adapter, qid >= netdev->real_num_tx_queues) return -EINVAL; + /* checked again on MTU changes and when the ring is set up */ + 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