Re: [PATCH 2/5] selftests/xsk: Eliminate umem reference from ifobject

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] The poll timeout validation in `__receive_pkts()` checks the local RX UMEM instead of the peer TX UMEM, breaking tests that expect an intentional timeout.
- [Medium] In shared UMEM mode, the TX and RX interfaces share the exact same `xsk_umem_info` object, causing the TX thread to inherit the RX's `base_addr` and corrupt the RX thread's memory partition.
--

commit 71e22f338d62604f1b7ed413a8905cfd021770d6
Author: Tushar Vyavahare <tushar.vyavahare@intel.com>

selftests/xsk: Eliminate umem reference from ifobject

This commit moves UMEM storage into the xsk_socket_info structure and uses
that pointer instead of the ifobject-level UMEM reference. This change is
intended to support multiple distinct UMEM instances on the same interface.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index d5cd8f590b25..ad5ada9216fc 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c

[ ... ]

> @@ -1007,7 +1010,7 @@ static int __receive_pkts(struct test_spec *test, struct xsk_socket_info *xsk)
>  			return TEST_FAILURE;
>  
>  		if (!ret) {
> -			if (!is_umem_valid(test->ifobj_tx))
> +			if (!is_umem_valid(umem))
>  				return TEST_PASS;
>  
>  			ksft_print_msg("ERROR: [%s] Poll timed out\n", __func__);

Will this cause tests that expect an intentional timeout to loop infinitely
or falsely fail?

Previously, this code handled a poll() timeout by checking the peer TX
thread's validity (!is_umem_valid(test->ifobj_tx)). If the TX thread was
inactive or had torn down its UMEM, this correctly evaluated to true.

By checking the local RX socket's UMEM (umem), which is actively polling
and always valid, the check will always fail. This causes the test to
incorrectly print an error and return TEST_CONTINUE instead of passing
gracefully.

[ ... ]

> @@ -1504,14 +1507,20 @@ static int xsk_configure(struct test_spec *test, struct ifobject *ifobject,
>  
>  static int thread_common_ops_tx(struct test_spec *test, struct ifobject *ifobject)
>  {
> -	int ret = xsk_configure(test, ifobject, test->ifobj_rx->umem, true);
> +	struct xsk_umem_info *umem_rx;
> +	int ret;
>  
> +	if (!test->ifobj_rx || !test->ifobj_rx->xsk_arr[0].umem ||
> +	    !test->ifobj_rx->xsk_arr[0].umem->umem) {
> +		ksft_print_msg("Error: RX UMEM is not initialized before shared-UMEM TX setup\n");
> +		return -EINVAL;
> +	}
> +
> +	umem_rx = test->ifobj_rx->xsk_arr[0].umem;
> +	ret = xsk_configure(test, ifobject, umem_rx, true);
>  	if (ret)
>  		return ret;
> -	ifobject->xsk = &ifobject->xsk_arr[0];
>  	ifobject->xskmap = test->ifobj_rx->xskmap;
> -	memcpy(ifobject->umem, test->ifobj_rx->umem, sizeof(struct xsk_umem_info));
> -	ifobject->umem->base_addr = 0;
>  
>  	return 0;
>  }

Can the removal of the deep copy and base_addr reset cause the TX thread to
inherit the RX's base_addr and corrupt the RX thread's memory partition?

In shared UMEM mode, the TX thread previously made a copy of the UMEM object
and explicitly set its base_addr = 0 to allocate from the first half of the
doubled UMEM.

By directly assigning the RX's UMEM object to the TX socket without
adjusting the base address, the TX thread allocates frames using the exact
same base_addr and next_buffer state as the RX thread. This causes a data
race on next_buffer and makes the TX thread allocate packets into the RX
thread's designated memory partition.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260428080317.702124-1-tushar.vyavahare@intel.com?part=2