Re: [PATCH v4 5/9] vfio: selftests: igb: Extend memcpy completion timeout for line-rate hardware

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] The selftest driver permanently desynchronizes its RX ring from the hardware upon encountering a dropped TX packet, leading to cascading test failures.
--

--- Patch [5]: [PATCH v4 5/9] vfio: selftests: igb: Extend memcpy completion timeout for line-rate hardware ---
commit 5b4ec7eb56b488ee6f3c3a7fb697c298ba1953f4
Author: Alex Williamson <alex.williamson@nvidia.com>

vfio: selftests: igb: Extend memcpy completion timeout for line-rate hardware
    
This commit increases the timeout in the igb driver selftest to wait for RX
descriptor writeback. It adjusts the wait time from 1 ms to up to 200 ms to
account for line-rate processing on real 82576 hardware. This provides enough
headroom for legitimate transfers to complete while keeping invalid-DMA tests
bounded.

> diff --git a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> index cd51a72b0e7fa..172c95cea3c8a 100644
> --- a/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> +++ b/tools/testing/selftests/vfio/lib/drivers/igb/igb.c
> @@ -428,12 +428,23 @@ static int igb_memcpy_wait(struct vfio_pci_device *device)
>  	prev_tail = (igb->rx_tail + RING_SIZE - 1) % RING_SIZE;
>  	rx = &igb->rx_ring[prev_tail];
>  
> -	retries = 100;
> +	/*
> +	 * Real 82576 hardware processes the descriptor ring at line rate.
> +	 * max_memcpy_size = (RING_SIZE - 1) * IGB_MAX_CHUNK_SIZE ~= 4 MB,
> +	 * split into 4095 1 KB frames.  At 1 Gb/s (~125 MB/s) the worst
> +	 * valid memcpy takes ~32 ms on the wire, plus per-frame preamble,
> +	 * SFD, IFG and FCS overhead (~3%) and descriptor fetch/writeback
> +	 * latency.  Wait up to ~200 ms before declaring the device hung;
> +	 * ~6x the line-rate floor leaves comfortable headroom for host
> +	 * scheduling jitter while keeping the intentional invalid-DMA
> +	 * tests bounded.
> +	 */
> +	retries = 200;
>  	while (retries-- > 0) {
>  		status = le32_to_cpu(READ_ONCE(rx->wb.status_error));
>  		if (status & 1)
>  			break;
> -		usleep(10);
> +		usleep(1000);
>  	}
>  
>  	if (status & 1)

[Severity: Medium]
When the intentional invalid-DMA tests timeout, does the software RX ring
become permanently desynchronized from the hardware?

In igb_memcpy_start(), the software rx_tail is unconditionally advanced:

tools/testing/selftests/vfio/lib/drivers/igb/igb.c:igb_memcpy_start() {
    ...
		igb->tx_tail = (igb->tx_tail + 1) % RING_SIZE;
		igb->rx_tail = (igb->rx_tail + 1) % RING_SIZE;
    ...
}

During an intentional invalid DMA (like in the mix_and_match test), the IOMMU
blocks the TX fetch and the packet is dropped, so the hardware RX ring never
advances.

When igb_memcpy_wait() times out, it returns -ETIMEDOUT but leaves the
software rx_tail out of sync with the hardware RDH.

Could this cause all subsequent valid test loopbacks to fail because they
wait on the wrong descriptor slot?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260710-igb_v3_b4-v4-0-56e7e2576cc1@google.com?part=5