Re: [PATCH v4 net-next 02/15] selftests: xsk: size the TX UMEM in HW_SW_MAX_RING_SIZE

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] bpf: integer underflow calculating xsk batch_size
- [Low] bpf: multi-line comment does not follow kernel style
--

commit 743c541687fdf2d7592285088f7c482d5114a852
Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

selftests: xsk: size the TX UMEM in HW_SW_MAX_RING_SIZE

This commit sizes the TX UMEM like the RX one to avoid failures in
dual-interface mode and caps the batch size below the XSK ring size to avoid
infinite loops.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index 2a13628740924..05457f77ca52c 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
[ ... ]
> @@ -2521,10 +2525,12 @@ int testapp_hw_sw_max_ring_size(struct test_spec *test)
>  		return ret;
>  
>  	/* Set batch_size to 8152 for testing, as the ice HW ignores the 3 lowest bits when
> -	 * updating the Rx HW tail register.
> +	 * updating the Rx HW tail register. __send_pkts() waits until a whole batch fits
> +	 * in the XSK TX ring, so keep it below that ring's size on NICs with larger rings.
>  	 */

[Severity: Low]
This isn't a bug, but does this multi-line comment follow the BPF subsystem
style guidelines?

The guidelines request the opening /* to be on its own line.

> -	test->ifobj_tx->xsk->batch_size = test->ifobj_tx->ring.tx_max_pending - 8;
> -	test->ifobj_rx->xsk->batch_size = test->ifobj_tx->ring.tx_max_pending - 8;
> +	batch_size = min(test->ifobj_tx->ring.tx_max_pending, max_descs) - 8;

[Severity: Medium]
Can this subtraction underflow if tx_max_pending is less than 8?

If an interface supports ethtool get_ringparam but reports a tx_max_pending
smaller than 8 (such as netdevsim altered via debugfs or drivers that leave
it 0), the unsigned subtraction would underflow and produce a massive
batch_size.

Could this cause __send_pkts() to infinite loop or hang the test while trying
to reserve descriptors that exceed the ring capacity?

> +	test->ifobj_tx->xsk->batch_size = batch_size;
> +	test->ifobj_rx->xsk->batch_size = batch_size;
>  	if (pkt_stream_replace(test, max_descs, MIN_PKT_SIZE)) {
>  		clean_sockets(test, test->ifobj_tx);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016174338.787135-1-maciej.fijalkowski@intel.com?part=2