Message-ID: <ZOiYOw0eSsU6dfRX@boxer>
Date: Fri, 25 Aug 2023 14:02:03 +0200
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: Magnus Karlsson <magnus.karlsson@...il.com>
CC: <magnus.karlsson@...el.com>, <bjorn@...nel.org>, <ast@...nel.org>,
	<daniel@...earbox.net>, <netdev@...r.kernel.org>, <bpf@...r.kernel.org>,
	<yhs@...com>, <andrii@...nel.org>, <martin.lau@...ux.dev>, <song@...nel.org>,
	<john.fastabend@...il.com>, <kpsingh@...nel.org>, <sdf@...gle.com>,
	<haoluo@...gle.com>, <jolsa@...nel.org>, <przemyslaw.kitszel@...el.com>
Subject: Re: [PATCH bpf-next v2 03/11] selftests/xsk: add option to only run
 tests in a single mode

On Thu, Aug 24, 2023 at 02:28:45PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@...el.com>
> 
> Add an option -m on the command line that allows the user to run the
> tests in a single mode instead of all of them. Valid modes are skb,
> drv, and zc (zero-copy). An example:
> 
> To run test suite in drv mode only:
> 
> ./test_xsk.sh -m drv
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@...el.com>
> ---
>  tools/testing/selftests/bpf/test_xsk.sh  | 10 ++++++-
>  tools/testing/selftests/bpf/xskxceiver.c | 34 +++++++++++++++++++++---
>  tools/testing/selftests/bpf/xskxceiver.h |  4 +--
>  3 files changed, 40 insertions(+), 8 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> index 2aa5a3445056..5ae2b3c27e21 100755
> --- a/tools/testing/selftests/bpf/test_xsk.sh
> +++ b/tools/testing/selftests/bpf/test_xsk.sh
> @@ -73,17 +73,21 @@
>  #
>  # Run test suite for physical device in loopback mode
>  #   sudo ./test_xsk.sh -i IFACE
> +#
> +# Run test suite in a specific mode only [skb,drv,zc]
> +#   sudo ./test_xsk.sh -m MODE
>  
>  . xsk_prereqs.sh
>  
>  ETH=""
>  
> -while getopts "vi:d" flag
> +while getopts "vi:dm:" flag
>  do
>  	case "${flag}" in
>  		v) verbose=1;;
>  		d) debug=1;;
>  		i) ETH=${OPTARG};;
> +		m) XSKTEST_MODE=${OPTARG};;
>  	esac
>  done
>  
> @@ -153,6 +157,10 @@ if [[ $verbose -eq 1 ]]; then
>  	ARGS+="-v "
>  fi
>  
> +if [ -n "$XSKTEST_MODE" ]; then
> +	ARGS+="-m ${XSKTEST_MODE} "
> +fi
> +
>  retval=$?
>  test_status $retval "${TEST_NAME}"
>  
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 514fe994e02b..9f79c2b6aa97 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -107,6 +107,9 @@
>  static const char *MAC1 = "\x00\x0A\x56\x9E\xEE\x62";
>  static const char *MAC2 = "\x00\x0A\x56\x9E\xEE\x61";
>  
> +static bool opt_verbose;
> +static enum test_mode opt_mode = TEST_MODE_ALL;
> +
>  static void __exit_with_error(int error, const char *file, const char *func, int line)
>  {
>  	ksft_test_result_fail("[%s:%s:%i]: ERROR: %d/\"%s\"\n", file, func, line, error,
> @@ -310,17 +313,19 @@ static struct option long_options[] = {
>  	{"interface", required_argument, 0, 'i'},
>  	{"busy-poll", no_argument, 0, 'b'},
>  	{"verbose", no_argument, 0, 'v'},
> +	{"mode", required_argument, 0, 'm'},
>  	{0, 0, 0, 0}
>  };
>  
>  static void usage(const char *prog)
>  {
>  	const char *str =
> -		"  Usage: %s [OPTIONS]\n"
> +		"  Usage: xskxceiver [OPTIONS]\n"
>  		"  Options:\n"
>  		"  -i, --interface      Use interface\n"
>  		"  -v, --verbose        Verbose output\n"
> -		"  -b, --busy-poll      Enable busy poll\n";
> +		"  -b, --busy-poll      Enable busy poll\n"
> +		"  -m, --mode           Run only mode skb, drv, or zc\n";
>  
>  	ksft_print_msg(str, prog);
>  }
> @@ -342,7 +347,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  	opterr = 0;
>  
>  	for (;;) {
> -		c = getopt_long(argc, argv, "i:vb", long_options, &option_index);
> +		c = getopt_long(argc, argv, "i:vbm:", long_options, &option_index);
>  		if (c == -1)
>  			break;
>  
> @@ -371,6 +376,21 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  			ifobj_tx->busy_poll = true;
>  			ifobj_rx->busy_poll = true;
>  			break;
> +		case 'm':
> +			if (!strncmp("skb", optarg, min_t(size_t, strlen(optarg),
> +							  strlen("skb")))) {
> +				opt_mode = TEST_MODE_SKB;
> +			} else if (!strncmp("drv", optarg, min_t(size_t, strlen(optarg),
> +								 strlen("drv")))) {
> +				opt_mode = TEST_MODE_DRV;
> +			} else if (!strncmp("zc", optarg, min_t(size_t, strlen(optarg),
> +								strlen("zc")))) {
> +				opt_mode = TEST_MODE_ZC;
> +			} else {
> +				usage(basename(argv[0]));
> +				ksft_exit_xfail();
> +			}
> +			break;
>  		default:
>  			usage(basename(argv[0]));
>  			ksft_exit_xfail();
> @@ -2365,9 +2385,15 @@ int main(int argc, char **argv)
>  	test.tx_pkt_stream_default = tx_pkt_stream_default;
>  	test.rx_pkt_stream_default = rx_pkt_stream_default;
>  
> -	ksft_set_plan(modes * TEST_TYPE_MAX);
> +	if (opt_mode == TEST_MODE_ALL)
> +		ksft_set_plan(modes * TEST_TYPE_MAX);
> +	else
> +		ksft_set_plan(TEST_TYPE_MAX);

what will happen if i run zc mode for a device that does not support it?
what will happen if i run xdp mode for a device that does not support it?

I know we will do nothing and exit gracefully, but i am wondering if
xskxceiver should catch it.

>  
>  	for (i = 0; i < modes; i++) {
> +		if (opt_mode != TEST_MODE_ALL && i != opt_mode)
> +			continue;
> +
>  		for (j = 0; j < TEST_TYPE_MAX; j++) {
>  			test_spec_init(&test, ifobj_tx, ifobj_rx, i);
>  			run_pkt_test(&test, i, j);
> diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> index 233b66cef64a..1412492e9618 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.h
> +++ b/tools/testing/selftests/bpf/xskxceiver.h
> @@ -63,7 +63,7 @@ enum test_mode {
>  	TEST_MODE_SKB,
>  	TEST_MODE_DRV,
>  	TEST_MODE_ZC,
> -	TEST_MODE_MAX
> +	TEST_MODE_ALL
>  };
>  
>  enum test_type {
> @@ -98,8 +98,6 @@ enum test_type {
>  	TEST_TYPE_MAX
>  };
>  
> -static bool opt_verbose;
> -
>  struct xsk_umem_info {
>  	struct xsk_ring_prod fq;
>  	struct xsk_ring_cons cq;
> -- 
> 2.34.1
> 

Powered by blists - more mailing lists