Re: [PATCH net-next v2 5/5] selftests/xsk: make pkt_stream_even_odd_sequence rollback-safe

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index f84f0edecebe..63a6403e6d5f 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> @@ -602,26 +602,41 @@ static int pkt_stream_receive_half(struct test_spec *test)
>
>  static int pkt_stream_even_odd_sequence(struct test_spec *test)
>  {
> +	struct pkt_stream *tx_streams[MAX_SOCKETS] = {};
> +	struct pkt_stream *rx_streams[MAX_SOCKETS] = {};
>  	struct pkt_stream *pkt_stream;
>  	u32 i;
>
>  	for (i = 0; i < test->nb_sockets; i++) {
>  		pkt_stream = test->ifobj_tx->xsk_arr[i].pkt_stream;
> -		pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
> -						   pkt_stream->pkts[0].len, i, 2);
> -		if (!pkt_stream)
> -			return -ENOMEM;
> -		test->ifobj_tx->xsk_arr[i].pkt_stream = pkt_stream;
> +		tx_streams[i] = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
> +						      pkt_stream->pkts[0].len, i, 2);
> +		if (!tx_streams[i])
> +			goto err;
>
>  		pkt_stream = test->ifobj_rx->xsk_arr[i].pkt_stream;
> -		pkt_stream = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
> -						   pkt_stream->pkts[0].len, i, 2);
> -		if (!pkt_stream)
> -			return -ENOMEM;
> -		test->ifobj_rx->xsk_arr[i].pkt_stream = pkt_stream;
> +		rx_streams[i] = __pkt_stream_generate(pkt_stream->nb_pkts / 2,
> +						      pkt_stream->pkts[0].len, i, 2);
> +		if (!rx_streams[i])
> +			goto err;
> +	}
> +
> +	for (i = 0; i < test->nb_sockets; i++) {
> +		test->ifobj_tx->xsk_arr[i].pkt_stream = tx_streams[i];
> +		test->ifobj_rx->xsk_arr[i].pkt_stream = rx_streams[i];
>  	}
>
>  	return 0;
> +
> +err:
> +	for (i = 0; i < test->nb_sockets; i++) {
> +		if (tx_streams[i])
> +			pkt_stream_delete(tx_streams[i]);
> +		if (rx_streams[i])
> +			pkt_stream_delete(rx_streams[i]);
> +	}
> +
> +	return -ENOMEM;
>  }

This isn't a bug, but now that pkt_stream_even_odd_sequence(),
pkt_stream_len_seq() and pkt_stream_uneven_dist_seq() share the same
temporary-array / publish / rollback skeleton, would it be worth factoring
the generate-then-publish and the err: rollback loop into one helper the
three can call?

After this rewrite, all three functions follow the same pattern: allocate
temporary tx_streams and rx_streams arrays, generate streams in a loop with
goto err on failure, publish in a second loop, then have an identical 8-line
err: cleanup block. The only difference between them is the packet-length
expression passed to __pkt_stream_generate().


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/32069138770