Re: [PATCH net-next v2 3/4] selftests/xsk: Use umem_size() helper consistently

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

On Wed, Jun 03, 2026 at 11:33:26AM +0530, Tushar Vyavahare wrote:
> Replace remaining open-coded umem->num_frames * umem->frame_size
> calculations in test_xsk.c with the existing umem_size() helper.
> 
> This keeps UMEM size computation centralized, avoids duplicated arithmetic,
> and improves readability with no intended behavior change.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> Signed-off-by: Tushar Vyavahare <tushar.vyavahare@intel.com>

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

> ---
>  .../selftests/bpf/prog_tests/test_xsk.c       | 36 +++++++++----------
>  1 file changed, 18 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index 7e0d34111b2f..f1730466ffd9 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> @@ -829,11 +829,11 @@ static bool is_frag_valid(struct xsk_umem_info *umem, u64 addr, u32 len, u32 exp
>  {
>  	u32 seqnum, pkt_nb, *pkt_data, words_to_end, expected_seqnum;
>  	void *data = xsk_umem__get_data(umem->buffer, addr);
> +	u64 umem_sz = umem_size(umem);
>  
>  	addr -= umem->base_addr;
>  
> -	if (addr >= umem->num_frames * umem->frame_size ||
> -	    addr + len > umem->num_frames * umem->frame_size) {
> +	if (addr >= umem_sz || addr + len > umem_sz) {
>  		ksft_print_msg("Frag invalid addr: %llx len: %u\n",
>  			       (unsigned long long)addr, len);
>  		return false;
> @@ -1586,7 +1586,7 @@ static int thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
>  	int ret;
>  	u32 i;
>  
> -	umem_sz = umem->num_frames * umem->frame_size;
> +	umem_sz = umem_size(umem);
>  	mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
>  
>  	if (umem->unaligned_mode)
> @@ -1706,12 +1706,13 @@ void *worker_testapp_validate_rx(void *arg)
>  static void testapp_clean_xsk_umem(struct ifobject *ifobj)
>  {
>  	struct xsk_umem_info *umem = ifobj->xsk->umem;
> -	u64 umem_sz = umem->num_frames * umem->frame_size;
> +	u64 umem_sz = umem_size(umem);
>  
>  	if (ifobj->shared_umem)
>  		umem_sz *= 2;
>  
>  	umem_sz = ceil_u64(umem_sz, HUGEPAGE_SIZE) * HUGEPAGE_SIZE;
> +
>  	xsk_umem__delete(umem->umem);
>  	munmap(umem->buffer, umem_sz);
>  }
> @@ -2097,7 +2098,7 @@ int testapp_send_receive_mb(struct test_spec *test)
>  int testapp_invalid_desc_mb(struct test_spec *test)
>  {
>  	struct xsk_umem_info *umem = test->ifobj_tx->xsk->umem;
> -	u64 umem_size = umem->num_frames * umem->frame_size;
> +	u64 umem_sz = umem_size(umem);
>  	struct pkt pkts[] = {
>  		/* Valid packet for synch to start with */
>  		{0, MIN_PKT_SIZE, 0, true, 0},
> @@ -2107,7 +2108,7 @@ int testapp_invalid_desc_mb(struct test_spec *test)
>  		{0, 0, 0, false, 0},
>  		/* Invalid address in the second frame */
>  		{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
> -		{umem_size, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
> +		{umem_sz, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
>  		/* Invalid len in the middle */
>  		{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
>  		{0, XSK_UMEM__INVALID_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
> @@ -2138,7 +2139,7 @@ int testapp_invalid_desc_mb(struct test_spec *test)
>  int testapp_invalid_desc(struct test_spec *test)
>  {
>  	struct xsk_umem_info *umem = test->ifobj_tx->xsk->umem;
> -	u64 umem_size = umem->num_frames * umem->frame_size;
> +	u64 umem_sz = umem_size(umem);
>  	struct pkt pkts[] = {
>  		/* Zero packet address allowed */
>  		{0, MIN_PKT_SIZE, 0, true},
> @@ -2149,11 +2150,11 @@ int testapp_invalid_desc(struct test_spec *test)
>  		/* Packet too large */
>  		{0, XSK_UMEM__INVALID_FRAME_SIZE, 0, false},
>  		/* Up to end of umem allowed */
> -		{umem_size - MIN_PKT_SIZE - 2 * umem->frame_size, MIN_PKT_SIZE, 0, true},
> +		{umem_sz - MIN_PKT_SIZE - 2 * umem->frame_size, MIN_PKT_SIZE, 0, true},
>  		/* After umem ends */
> -		{umem_size, MIN_PKT_SIZE, 0, false},
> +		{umem_sz, MIN_PKT_SIZE, 0, false},
>  		/* Straddle the end of umem */
> -		{umem_size - MIN_PKT_SIZE / 2, MIN_PKT_SIZE, 0, false},
> +		{umem_sz - MIN_PKT_SIZE / 2, MIN_PKT_SIZE, 0, false},
>  		/* Straddle a 4K boundary */
>  		{0x1000 - MIN_PKT_SIZE / 2, MIN_PKT_SIZE, 0, false},
>  		/* Straddle a 2K boundary */
> @@ -2171,9 +2172,9 @@ int testapp_invalid_desc(struct test_spec *test)
>  	}
>  
>  	if (test->ifobj_tx->shared_umem) {
> -		pkts[4].offset += umem_size;
> -		pkts[5].offset += umem_size;
> -		pkts[6].offset += umem_size;
> +		pkts[4].offset += umem_sz;
> +		pkts[5].offset += umem_sz;
> +		pkts[6].offset += umem_sz;
>  	}
>  
>  	if (pkt_stream_generate_custom(test, pkts, ARRAY_SIZE(pkts)))
> @@ -2406,7 +2407,7 @@ int testapp_unaligned_inv_desc(struct test_spec *test)
>  
>  int testapp_unaligned_inv_desc_4001_frame(struct test_spec *test)
>  {
> -	u64 page_size, umem_size;
> +	u64 page_size, umem_sz;
>  
>  	/* Odd frame size so the UMEM doesn't end near a page boundary. */
>  	test_spec_set_frame_size(test, 4001);
> @@ -2415,10 +2416,9 @@ int testapp_unaligned_inv_desc_4001_frame(struct test_spec *test)
>  	 * the UMEM but not a page.
>  	 */
>  	page_size = sysconf(_SC_PAGESIZE);
> -	umem_size = test->ifobj_tx->xsk->umem->num_frames *
> -		test->ifobj_tx->xsk->umem->frame_size;
> -	assert(umem_size % page_size > MIN_PKT_SIZE);
> -	assert(umem_size % page_size < page_size - MIN_PKT_SIZE);
> +	umem_sz = umem_size(test->ifobj_tx->xsk->umem);
> +	assert(umem_sz % page_size > MIN_PKT_SIZE);
> +	assert(umem_sz % page_size < page_size - MIN_PKT_SIZE);
>  
>  	return testapp_invalid_desc(test);
>  }
> -- 
> 2.43.0
>