netdev - Re: [PATCH net-next v5 2/2] selftests/bpf: add a new test to check the consumer update case

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aGVYNMZEZQV1SetF@mini-arch>
Date: Wed, 2 Jul 2025 09:03:00 -0700
From: Stanislav Fomichev <stfomichev@...il.com>
To: Jason Xing <kerneljasonxing@...il.com>
Cc: davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
	pabeni@...hat.com, bjorn@...nel.org, magnus.karlsson@...el.com,
	maciej.fijalkowski@...el.com, jonathan.lemon@...il.com,
	sdf@...ichev.me, ast@...nel.org, daniel@...earbox.net,
	hawk@...nel.org, john.fastabend@...il.com, joe@...a.to,
	willemdebruijn.kernel@...il.com, bpf@...r.kernel.org,
	netdev@...r.kernel.org, Jason Xing <kernelxing@...cent.com>
Subject: Re: [PATCH net-next v5 2/2] selftests/bpf: add a new test to check
 the consumer update case

On 07/02, Jason Xing wrote:
> From: Jason Xing <kernelxing@...cent.com>
> 
> The subtest sends 33 packets at one time on purpose to see if xsk
> exitting __xsk_generic_xmit() updates the global consumer of tx queue
> when reaching the max loop (max_tx_budget, 32 by default). The number 33
> can avoid xskq_cons_peek_desc() updates the consumer when it's about to
> quit sending, to accurately check if the issue that the first patch
> resolves remains. The new case will not check this issue in zero copy
> mode.
> 
> Signed-off-by: Jason Xing <kernelxing@...cent.com>
> ---
> v5
> Link: https://lore.kernel.org/all/20250627085745.53173-1-kerneljasonxing@gmail.com/
> 1. use the initial approach to add a new testcase
> 2. add a new flag 'check_consumer' to see if the check is needed
> ---
>  tools/testing/selftests/bpf/xskxceiver.c | 51 +++++++++++++++++++++++-
>  tools/testing/selftests/bpf/xskxceiver.h |  1 +
>  2 files changed, 51 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 0ced4026ee44..ed12a55ecf2a 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -109,6 +109,8 @@
>  
>  #include <network_helpers.h>
>  
> +#define MAX_TX_BUDGET_DEFAULT 32
> +
>  static bool opt_verbose;
>  static bool opt_print_tests;
>  static enum test_mode opt_mode = TEST_MODE_ALL;
> @@ -1091,11 +1093,45 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len)
>  	return true;
>  }
>  
> +static u32 load_value(u32 *counter)
> +{
> +	return __atomic_load_n(counter, __ATOMIC_ACQUIRE);
> +}
> +
> +static bool kick_tx_with_check(struct xsk_socket_info *xsk, int *ret)
> +{
> +	u32 max_budget = MAX_TX_BUDGET_DEFAULT;
> +	u32 cons, ready_to_send;
> +	int delta;
> +
> +	cons = load_value(xsk->tx.consumer);
> +	ready_to_send = load_value(xsk->tx.producer) - cons;
> +	*ret = sendto(xsk_socket__fd(xsk->xsk), NULL, 0, MSG_DONTWAIT, NULL, 0);
> +
> +	delta = load_value(xsk->tx.consumer) - cons;
> +	/* By default, xsk should consume exact @max_budget descs at one
> +	 * send in this case where hitting the max budget limit in while
> +	 * loop is triggered in __xsk_generic_xmit(). Please make sure that
> +	 * the number of descs to be sent is larger than @max_budget, or
> +	 * else the tx.consumer will be updated in xskq_cons_peek_desc()
> +	 * in time which hides the issue we try to verify.
> +	 */
> +	if (ready_to_send > max_budget && delta != max_budget)
> +		return false;
> +
> +	return true;
> +}
> +
>  static int kick_tx(struct xsk_socket_info *xsk)
>  {
>  	int ret;
>  
> -	ret = sendto(xsk_socket__fd(xsk->xsk), NULL, 0, MSG_DONTWAIT, NULL, 0);
> +	if (xsk->check_consumer) {
> +		if (!kick_tx_with_check(xsk, &ret))
> +			return TEST_FAILURE;
> +	} else {
> +		ret = sendto(xsk_socket__fd(xsk->xsk), NULL, 0, MSG_DONTWAIT, NULL, 0);
> +	}
>  	if (ret >= 0)
>  		return TEST_PASS;
>  	if (errno == ENOBUFS || errno == EAGAIN || errno == EBUSY || errno == ENETDOWN) {
> @@ -2613,6 +2649,18 @@ static int testapp_adjust_tail_grow_mb(struct test_spec *test)
>  				   XSK_UMEM__LARGE_FRAME_SIZE * 2);
>  }
>  
> +static int testapp_tx_queue_consumer(struct test_spec *test)
> +{
> +	int nr_packets = MAX_TX_BUDGET_DEFAULT + 1;
> +
> +	pkt_stream_replace(test, nr_packets, MIN_PKT_SIZE);
> +	test->ifobj_tx->xsk->batch_size = nr_packets;
> +	if (!(test->mode & TEST_MODE_ZC))
> +		test->ifobj_tx->xsk->check_consumer = true;

The test looks good to me, thank you!

One question here: why not exit/return for TEST_MODE_ZC instead
of conditionally setting check_consumer?