Re: [PATCH 10/13] selftests/bpf: test_xsk: Split xskxceiver

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

On Thu, Mar 13, 2025 at 11:48:08AM +0100, Bastien Curutchet (eBPF Foundation) wrote:
> AF_XDP features are tested by the test_xsk.sh script but not by the
> test_progs framework. The tests used by the script are defined in
> xksxceiver.c which can't be integrated in the test_progs framework as is.
> 
> Extract these test definitions from xskxceiver{.c/.h} to put them in new
> test_xsk{.c/.h} files.
> Keep the main() function and its unshared dependencies in xksxceiver to
> avoid impacting the test_xsk.sh script which is often used to test real
> hardware.
> 
> Signed-off-by: Bastien Curutchet (eBPF Foundation) <bastien.curutchet@bootlin.com>
> ---
>  tools/testing/selftests/bpf/Makefile     |    2 +-
>  tools/testing/selftests/bpf/test_xsk.c   | 2416 ++++++++++++++++++++++++++++
>  tools/testing/selftests/bpf/test_xsk.h   |  279 ++++
>  tools/testing/selftests/bpf/xskxceiver.c | 2503 +-----------------------------
>  tools/testing/selftests/bpf/xskxceiver.h |  156 --
>  5 files changed, 2727 insertions(+), 2629 deletions(-)
> +

(...)

> +int testapp_hw_sw_max_ring_size(struct test_spec *test)
> +{
> +	u32 max_descs = XSK_RING_PROD__DEFAULT_NUM_DESCS * 4;
> +	int ret;
> +
> +	test->set_ring = true;
> +	test->total_steps = 2;
> +	test->ifobj_tx->ring.tx_pending = test->ifobj_tx->ring.tx_max_pending;
> +	test->ifobj_tx->ring.rx_pending  = test->ifobj_tx->ring.rx_max_pending;
> +	test->ifobj_rx->umem->num_frames = max_descs;
> +	test->ifobj_rx->umem->fill_size = max_descs;
> +	test->ifobj_rx->umem->comp_size = max_descs;
> +	test->ifobj_tx->xsk->batch_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
> +	test->ifobj_rx->xsk->batch_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
> +
> +	ret = testapp_validate_traffic(test);
> +	if (ret)
> +		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.
> +	 */
> +	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;
> +	if (!pkt_stream_replace(test, max_descs, MIN_PKT_SIZE))

Here's the victim of test failures that i reported last week. This
function succeds with 0 and you interpret it as failure:) One sign wrong
caused two days of debugging, but it was kinda fun.

What was happening was due to the failure here one of the sockets was not
deleted and later on whole test suite could not attach the socket for
every other test case which caused the ever going failures. Which makes me
think that since you changed the logic from exits to returning failures
probably we need to take care of state cleanup in a better way so case
like this one described here wouldn't take place.

This test is only executed for hw tests that's why you haven't seen this
problem as you were focused on veth case.

If you want to proceed with that then i would like to ask you to grab the
hw and check you're not introducing regressions. FWIW i have been working
with i40e and ice drivers.

One last note is that verbose mode seemed to be broken for me.

> +		return TEST_FAILURE;
> +	return testapp_validate_traffic(test);
> +}
> +

(...)