[PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver

netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed

* [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver
@ 2023-08-09 12:43 Magnus Karlsson
  2023-08-09 12:43 ` [PATCH bpf-next 01/10] selftests/xsk: print per packet info in verbose mode Magnus Karlsson
                   ` (9 more replies)
  0 siblings, 10 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa
  Cc: Magnus Karlsson

This patch set implements several improvements to the xsk selftests
test suite that I thought were useful while debugging the xsk
multi-buffer code and tests. The largest new feature is the ability to
be able to execute a single test instead of the whole test suite. This
required some surgery on the current code, details below.

Anatomy of the path set:

1: Print useful info on a per packet basis with the option -v

2: Add a timeout in the transmission loop too. We only used to have
   one for the Rx thread, but Tx can lock up too waiting for
   completions.

3: Add an option (-m) to only run the tests (or a single test with a
   later patch) in a single mode: skb, drv, or zc (zero-copy).

4-5: Preparatory patches to be able to specify a test to run. Need to
     define the test names in a single structure and their entry
     points, so we can use this when wanting to run a specific test.

6: Adds a command line option (-l) that lists all the tests.

7: Adds a command line option (-t) that runs a specific test instead
   of the whole test suite. Can be combined with -m to specify a
   single mode too.

8: Use ksft_print_msg() uniformly throughout the tests. It was a mix
   of printf() and ksft_print_msg() before.

9: In some places, we failed the whole test suite instead of a single
   test in certain circumstances. Fix this so only the test in
   question is failed and the rest of the test suite continues.

10: Display the available command line options with -h

Thanks: Magnus

Magnus Karlsson (10):
  selftests/xsk: print per packet info in verbose mode
  selftests/xsk: add timeout for Tx thread
  selftests/xsk: add option to only run tests in a single mode
  selftests/xsk: move all tests to separate functions
  selftests/xsk: declare test names in struct
  selftests/xsk: add option that lists all tests
  selftests/xsk: add option to run single test
  selftests/xsk: use ksft_print_msg uniformly
  selftests/xsk: fail single test instead of all tests
  selftests/xsk: display command line options with -h

 tools/testing/selftests/bpf/test_xsk.sh    |  36 +-
 tools/testing/selftests/bpf/xsk_prereqs.sh |  10 +-
 tools/testing/selftests/bpf/xskxceiver.c   | 517 ++++++++++++---------
 tools/testing/selftests/bpf/xskxceiver.h   |  44 +-
 4 files changed, 353 insertions(+), 254 deletions(-)

base-commit: 2adbb7637fd1fcec93f4680ddb5ddbbd1a91aefb
--
2.34.1

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH bpf-next 01/10] selftests/xsk: print per packet info in verbose mode
  2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
@ 2023-08-09 12:43 ` Magnus Karlsson
  2023-08-09 12:43 ` [PATCH bpf-next 02/10] selftests/xsk: add timeout for Tx thread Magnus Karlsson
                   ` (8 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa

From: Magnus Karlsson <magnus.karlsson@intel.com>

Print info about every packet in verbose mode, both for Tx and
Rx. This is useful to have when a test fails or to validate that a
test is really doing what it was designed to do. Info on what is
supposed to be received and sent is also printed for the custom packet
streams since they differ from the base line. Here is an example:

Tx addr: 37e0 len: 64 options: 0 pkt_nb: 8
Tx addr: 4000 len: 64 options: 0 pkt_nb: 9
Rx: addr: 100 len: 64 options: 0 pkt_nb: 0 valid: 1
Rx: addr: 1100 len: 64 options: 0 pkt_nb: 1 valid: 1
Rx: addr: 2100 len: 64 options: 0 pkt_nb: 4 valid: 1
Rx: addr: 3100 len: 64 options: 0 pkt_nb: 8 valid: 1
Rx: addr: 4100 len: 64 options: 0 pkt_nb: 9 valid: 1

One pointless verbose print statement is also deleted and another one
is made clearer.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 2827f2d7cf30..c595c0b65417 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -747,6 +747,9 @@ static struct pkt_stream *__pkt_stream_generate_custom(struct ifobject *ifobj, s
 			len = 0;
 		}
 
+		print_verbose("offset: %d len: %u valid: %u options: %u pkt_nb: %u\n",
+			      pkt->offset, pkt->len, pkt->valid, pkt->options, pkt->pkt_nb);
+
 		if (pkt->valid && pkt->len > pkt_stream->max_pkt_len)
 			pkt_stream->max_pkt_len = pkt->len;
 		pkt_nb++;
@@ -1042,6 +1045,9 @@ static int receive_pkts(struct test_spec *test, struct pollfd *fds)
 				return TEST_FAILURE;
 			}
 
+			print_verbose("Rx: addr: %lx len: %u options: %u pkt_nb: %u valid: %u\n",
+				      addr, desc->len, desc->options, pkt->pkt_nb, pkt->valid);
+
 			if (!is_frag_valid(umem, addr, desc->len, pkt->pkt_nb, pkt_len) ||
 			    !is_offset_correct(umem, pkt, addr) ||
 			    (ifobj->use_metadata && !is_metadata_correct(pkt, umem->buffer, addr)))
@@ -1165,6 +1171,9 @@ static int __send_pkts(struct ifobject *ifobject, struct pollfd *fds, bool timeo
 					     bytes_written);
 			bytes_written += tx_desc->len;
 
+			print_verbose("Tx addr: %llx len: %u options: %u pkt_nb: %u\n",
+				      tx_desc->addr, tx_desc->len, tx_desc->options, pkt->pkt_nb);
+
 			if (nb_frags_left) {
 				i++;
 				if (pkt_stream->verbatim)
@@ -1475,8 +1484,6 @@ static void *worker_testapp_validate_tx(void *arg)
 			thread_common_ops_tx(test, ifobject);
 	}
 
-	print_verbose("Sending %d packets on interface %s\n", ifobject->pkt_stream->nb_pkts,
-		      ifobject->ifname);
 	err = send_pkts(test, ifobject);
 
 	if (!err && ifobject->validation_func)
@@ -1715,7 +1722,7 @@ static int testapp_bidi(struct test_spec *test)
 	if (testapp_validate_traffic(test))
 		return TEST_FAILURE;
 
-	print_verbose("Switching Tx/Rx vectors\n");
+	print_verbose("Switching Tx/Rx direction\n");
 	swap_directions(&test->ifobj_rx, &test->ifobj_tx);
 	res = __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH bpf-next 02/10] selftests/xsk: add timeout for Tx thread
  2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
  2023-08-09 12:43 ` [PATCH bpf-next 01/10] selftests/xsk: print per packet info in verbose mode Magnus Karlsson
@ 2023-08-09 12:43 ` Magnus Karlsson
  2023-08-09 12:43 ` [PATCH bpf-next 03/10] selftests/xsk: add option to only run tests in a single mode Magnus Karlsson
                   ` (7 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa

From: Magnus Karlsson <magnus.karlsson@intel.com>

Add a timeout for the transmission thread. If packets are not
completed properly, for some reason, the test harness would previously
get stuck forever in a while loop. But with this patch, this timeout
will trigger, flag the test as a failure, and continue with the next
test.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 26 ++++++++++++++++++++----
 1 file changed, 22 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index c595c0b65417..514fe994e02b 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1216,10 +1216,29 @@ static int __send_pkts(struct ifobject *ifobject, struct pollfd *fds, bool timeo
 	return TEST_CONTINUE;
 }
 
-static void wait_for_tx_completion(struct xsk_socket_info *xsk)
+static int wait_for_tx_completion(struct xsk_socket_info *xsk)
 {
-	while (xsk->outstanding_tx)
+	struct timeval tv_end, tv_now, tv_timeout = {THREAD_TMOUT, 0};
+	int ret;
+
+	ret = gettimeofday(&tv_now, NULL);
+	if (ret)
+		exit_with_error(errno);
+	timeradd(&tv_now, &tv_timeout, &tv_end);
+
+	while (xsk->outstanding_tx) {
+		ret = gettimeofday(&tv_now, NULL);
+		if (ret)
+			exit_with_error(errno);
+		if (timercmp(&tv_now, &tv_end, >)) {
+			ksft_print_msg("ERROR: [%s] Transmission loop timed out\n", __func__);
+			return TEST_FAILURE;
+		}
+
 		complete_pkts(xsk, BATCH_SIZE);
+	}
+
+	return TEST_PASS;
 }
 
 static int send_pkts(struct test_spec *test, struct ifobject *ifobject)
@@ -1242,8 +1261,7 @@ static int send_pkts(struct test_spec *test, struct ifobject *ifobject)
 			return ret;
 	}
 
-	wait_for_tx_completion(ifobject->xsk);
-	return TEST_PASS;
+	return wait_for_tx_completion(ifobject->xsk);
 }
 
 static int get_xsk_stats(struct xsk_socket *xsk, struct xdp_statistics *stats)
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH bpf-next 03/10] selftests/xsk: add option to only run tests in a single mode
  2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
  2023-08-09 12:43 ` [PATCH bpf-next 01/10] selftests/xsk: print per packet info in verbose mode Magnus Karlsson
  2023-08-09 12:43 ` [PATCH bpf-next 02/10] selftests/xsk: add timeout for Tx thread Magnus Karlsson
@ 2023-08-09 12:43 ` Magnus Karlsson
  2023-08-10 12:14   ` Przemek Kitszel
  2023-08-09 12:43 ` [PATCH bpf-next 04/10] selftests/xsk: move all tests to separate functions Magnus Karlsson
                   ` (6 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa

From: Magnus Karlsson <magnus.karlsson@intel.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@intel.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..4ec621f4d3db 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) MODE=${OPTARG};;
 	esac
 done
 
@@ -153,6 +157,10 @@ if [[ $verbose -eq 1 ]]; then
 	ARGS+="-v "
 fi
 
+if [ ! -z $MODE ]; then
+	ARGS+="-m ${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);
 
 	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


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 03/10] selftests/xsk: add option to only run tests in a single mode
  2023-08-09 12:43 ` [PATCH bpf-next 03/10] selftests/xsk: add option to only run tests in a single mode Magnus Karlsson
@ 2023-08-10 12:14   ` Przemek Kitszel
  2023-08-10 12:36     ` Magnus Karlsson
  0 siblings, 1 reply; 22+ messages in thread
From: Przemek Kitszel @ 2023-08-10 12:14 UTC (permalink / raw)
  To: Magnus Karlsson, Karlsson, Magnus, bjorn@kernel.org,
	ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
	Fijalkowski, Maciej, bpf@vger.kernel.org, yhs@fb.com,
	andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
	haoluo@google.com, jolsa@kernel.org

On 8/9/23 14:43, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.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@intel.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..4ec621f4d3db 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) MODE=${OPTARG};;
>   	esac
>   done
>   
> @@ -153,6 +157,10 @@ if [[ $verbose -eq 1 ]]; then
>   	ARGS+="-v "
>   fi
>   
> +if [ ! -z $MODE ]; then

better: `if [ -n "$MODE" ]`

note that quotes are really good invention for such cases, especially 
that default value of MODE is "take such named variable from user env".

> +	ARGS+="-m ${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);
>   
>   	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;


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 03/10] selftests/xsk: add option to only run tests in a single mode
  2023-08-10 12:14   ` Przemek Kitszel
@ 2023-08-10 12:36     ` Magnus Karlsson
  0 siblings, 0 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-10 12:36 UTC (permalink / raw)
  To: Przemek Kitszel
  Cc: Karlsson, Magnus, bjorn@kernel.org, ast@kernel.org,
	daniel@iogearbox.net, netdev@vger.kernel.org, Fijalkowski, Maciej,
	bpf@vger.kernel.org, yhs@fb.com, andrii@kernel.org,
	martin.lau@linux.dev, song@kernel.org, john.fastabend@gmail.com,
	kpsingh@kernel.org, sdf@google.com, haoluo@google.com,
	jolsa@kernel.org

On Thu, 10 Aug 2023 at 14:14, Przemek Kitszel
<przemyslaw.kitszel@intel.com> wrote:
>
> On 8/9/23 14:43, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.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@intel.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..4ec621f4d3db 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) MODE=${OPTARG};;
> >       esac
> >   done
> >
> > @@ -153,6 +157,10 @@ if [[ $verbose -eq 1 ]]; then
> >       ARGS+="-v "
> >   fi
> >
> > +if [ ! -z $MODE ]; then
>
> better: `if [ -n "$MODE" ]`
>
> note that quotes are really good invention for such cases, especially
> that default value of MODE is "take such named variable from user env".

Definitely better. Will fix. Thanks!

> > +     ARGS+="-m ${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);
> >
> >       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;
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH bpf-next 04/10] selftests/xsk: move all tests to separate functions
  2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
                   ` (2 preceding siblings ...)
  2023-08-09 12:43 ` [PATCH bpf-next 03/10] selftests/xsk: add option to only run tests in a single mode Magnus Karlsson
@ 2023-08-09 12:43 ` Magnus Karlsson
  2023-08-09 12:43 ` [PATCH bpf-next 05/10] selftests/xsk: declare test names in struct Magnus Karlsson
                   ` (5 subsequent siblings)
  9 siblings, 0 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa

From: Magnus Karlsson <magnus.karlsson@intel.com>

Prepare for the capability to be able to run a single test by moving
all the tests to their own functions. This function can then be called
to execute that test in the next commit.

Also, the tests named RUN_TO_COMPLETION_* were not named well, so
change them to SEND_RECEIVE_* as it is just a basic send and receive
test of 4K packets.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 170 +++++++++++++++--------
 1 file changed, 115 insertions(+), 55 deletions(-)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 9f79c2b6aa97..ee72bb0a8978 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1875,13 +1875,14 @@ static int testapp_single_pkt(struct test_spec *test)
 {
 	struct pkt pkts[] = {{0, MIN_PKT_SIZE, 0, true}};
 
+	test_spec_set_name(test, "SEND_RECEIVE_SINGLE_PKT");
 	pkt_stream_generate_custom(test, pkts, ARRAY_SIZE(pkts));
 	return testapp_validate_traffic(test);
 }
 
 static int testapp_multi_buffer(struct test_spec *test)
 {
-	test_spec_set_name(test, "RUN_TO_COMPLETION_9K_PACKETS");
+	test_spec_set_name(test, "SEND_RECEIVE_9K_PACKETS");
 	test->mtu = MAX_ETH_JUMBO_SIZE;
 	pkt_stream_replace(test, DEFAULT_PKT_CNT, MAX_ETH_JUMBO_SIZE);
 
@@ -1986,7 +1987,7 @@ static int testapp_xdp_drop(struct test_spec *test)
 	return testapp_validate_traffic(test);
 }
 
-static int testapp_xdp_metadata_count(struct test_spec *test)
+static int testapp_xdp_metadata_copy(struct test_spec *test)
 {
 	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
 	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
@@ -2136,6 +2137,105 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
 	}
 }
 
+static int testapp_send_receive(struct test_spec *test)
+{
+	test_spec_set_name(test, "SEND_RECEIVE");
+	return testapp_validate_traffic(test);
+}
+
+static int testapp_send_receive_2k_frame(struct test_spec *test)
+{
+	test_spec_set_name(test, "SEND_RECEIVE_2K_FRAME_SIZE");
+	test->ifobj_tx->umem->frame_size = 2048;
+	test->ifobj_rx->umem->frame_size = 2048;
+	pkt_stream_replace(test, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
+	return testapp_validate_traffic(test);
+}
+
+static int testapp_poll_rx(struct test_spec *test)
+{
+	test->ifobj_rx->use_poll = true;
+	test_spec_set_name(test, "POLL_RX");
+	return testapp_validate_traffic(test);
+}
+
+static int testapp_poll_tx(struct test_spec *test)
+{
+	test->ifobj_tx->use_poll = true;
+	test_spec_set_name(test, "POLL_TX");
+	return testapp_validate_traffic(test);
+}
+
+static int testapp_aligned_inv_desc(struct test_spec *test)
+{
+	test_spec_set_name(test, "ALIGNED_INV_DESC");
+	return testapp_invalid_desc(test);
+}
+
+static int testapp_aligned_inv_desc_2k_frame(struct test_spec *test)
+{
+	test_spec_set_name(test, "ALIGNED_INV_DESC_2K_FRAME_SIZE");
+	test->ifobj_tx->umem->frame_size = 2048;
+	test->ifobj_rx->umem->frame_size = 2048;
+	return testapp_invalid_desc(test);
+}
+
+static int testapp_unaligned_inv_desc(struct test_spec *test)
+{
+	test_spec_set_name(test, "UNALIGNED_INV_DESC");
+	test->ifobj_tx->umem->unaligned_mode = true;
+	test->ifobj_rx->umem->unaligned_mode = true;
+	return testapp_invalid_desc(test);
+}
+
+static int testapp_unaligned_inv_desc_4001_frame(struct test_spec *test)
+{
+	u64 page_size, umem_size;
+
+	test_spec_set_name(test, "UNALIGNED_INV_DESC_4K1_FRAME_SIZE");
+	/* Odd frame size so the UMEM doesn't end near a page boundary. */
+	test->ifobj_tx->umem->frame_size = 4001;
+	test->ifobj_rx->umem->frame_size = 4001;
+	test->ifobj_tx->umem->unaligned_mode = true;
+	test->ifobj_rx->umem->unaligned_mode = true;
+	/* This test exists to test descriptors that staddle the end of
+	 * the UMEM but not a page.
+	 */
+	page_size = sysconf(_SC_PAGESIZE);
+	umem_size = test->ifobj_tx->umem->num_frames * test->ifobj_tx->umem->frame_size;
+	assert(umem_size % page_size > MIN_PKT_SIZE);
+	assert(umem_size % page_size < page_size - MIN_PKT_SIZE);
+
+	return testapp_invalid_desc(test);
+}
+
+static int testapp_aligned_inv_desc_mb(struct test_spec *test)
+{
+	test_spec_set_name(test, "ALIGNED_INV_DESC_MULTI_BUFF");
+	return testapp_invalid_desc_mb(test);
+}
+
+static int testapp_unaligned_inv_desc_mb(struct test_spec *test)
+{
+	test_spec_set_name(test, "UNALIGNED_INV_DESC_MULTI_BUFF");
+	test->ifobj_tx->umem->unaligned_mode = true;
+	test->ifobj_rx->umem->unaligned_mode = true;
+	return testapp_invalid_desc_mb(test);
+}
+
+static int testapp_xdp_metadata(struct test_spec *test)
+{
+	test_spec_set_name(test, "XDP_METADATA_COPY");
+	return testapp_xdp_metadata_copy(test);
+}
+
+static int testapp_xdp_metadata_mb(struct test_spec *test)
+{
+	test_spec_set_name(test, "XDP_METADATA_COPY_MULTI_BUFF");
+	test->mtu = MAX_ETH_JUMBO_SIZE;
+	return testapp_xdp_metadata_copy(test);
+}
+
 static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
 {
 	int ret = TEST_SKIP;
@@ -2163,32 +2263,22 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
 		ret = testapp_bpf_res(test);
 		break;
 	case TEST_TYPE_RUN_TO_COMPLETION:
-		test_spec_set_name(test, "RUN_TO_COMPLETION");
-		ret = testapp_validate_traffic(test);
+		ret = testapp_send_receive(test);
 		break;
 	case TEST_TYPE_RUN_TO_COMPLETION_MB:
 		ret = testapp_multi_buffer(test);
 		break;
 	case TEST_TYPE_RUN_TO_COMPLETION_SINGLE_PKT:
-		test_spec_set_name(test, "RUN_TO_COMPLETION_SINGLE_PKT");
 		ret = testapp_single_pkt(test);
 		break;
 	case TEST_TYPE_RUN_TO_COMPLETION_2K_FRAME:
-		test_spec_set_name(test, "RUN_TO_COMPLETION_2K_FRAME_SIZE");
-		test->ifobj_tx->umem->frame_size = 2048;
-		test->ifobj_rx->umem->frame_size = 2048;
-		pkt_stream_replace(test, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
-		ret = testapp_validate_traffic(test);
+		ret = testapp_send_receive_2k_frame(test);
 		break;
 	case TEST_TYPE_RX_POLL:
-		test->ifobj_rx->use_poll = true;
-		test_spec_set_name(test, "POLL_RX");
-		ret = testapp_validate_traffic(test);
+		ret = testapp_poll_rx(test);
 		break;
 	case TEST_TYPE_TX_POLL:
-		test->ifobj_tx->use_poll = true;
-		test_spec_set_name(test, "POLL_TX");
-		ret = testapp_validate_traffic(test);
+		ret = testapp_poll_tx(test);
 		break;
 	case TEST_TYPE_POLL_TXQ_TMOUT:
 		ret = testapp_poll_txq_tmout(test);
@@ -2197,49 +2287,22 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
 		ret = testapp_poll_rxq_tmout(test);
 		break;
 	case TEST_TYPE_ALIGNED_INV_DESC:
-		test_spec_set_name(test, "ALIGNED_INV_DESC");
-		ret = testapp_invalid_desc(test);
+		ret = testapp_aligned_inv_desc(test);
 		break;
 	case TEST_TYPE_ALIGNED_INV_DESC_2K_FRAME:
-		test_spec_set_name(test, "ALIGNED_INV_DESC_2K_FRAME_SIZE");
-		test->ifobj_tx->umem->frame_size = 2048;
-		test->ifobj_rx->umem->frame_size = 2048;
-		ret = testapp_invalid_desc(test);
+		ret = testapp_aligned_inv_desc_2k_frame(test);
 		break;
 	case TEST_TYPE_UNALIGNED_INV_DESC:
-		test_spec_set_name(test, "UNALIGNED_INV_DESC");
-		test->ifobj_tx->umem->unaligned_mode = true;
-		test->ifobj_rx->umem->unaligned_mode = true;
-		ret = testapp_invalid_desc(test);
+		ret = testapp_unaligned_inv_desc(test);
 		break;
-	case TEST_TYPE_UNALIGNED_INV_DESC_4K1_FRAME: {
-		u64 page_size, umem_size;
-
-		test_spec_set_name(test, "UNALIGNED_INV_DESC_4K1_FRAME_SIZE");
-		/* Odd frame size so the UMEM doesn't end near a page boundary. */
-		test->ifobj_tx->umem->frame_size = 4001;
-		test->ifobj_rx->umem->frame_size = 4001;
-		test->ifobj_tx->umem->unaligned_mode = true;
-		test->ifobj_rx->umem->unaligned_mode = true;
-		/* This test exists to test descriptors that staddle the end of
-		 * the UMEM but not a page.
-		 */
-		page_size = sysconf(_SC_PAGESIZE);
-		umem_size = test->ifobj_tx->umem->num_frames * test->ifobj_tx->umem->frame_size;
-		assert(umem_size % page_size > MIN_PKT_SIZE);
-		assert(umem_size % page_size < page_size - MIN_PKT_SIZE);
-		ret = testapp_invalid_desc(test);
+	case TEST_TYPE_UNALIGNED_INV_DESC_4K1_FRAME:
+		ret = testapp_unaligned_inv_desc_4001_frame(test);
 		break;
-	}
 	case TEST_TYPE_ALIGNED_INV_DESC_MB:
-		test_spec_set_name(test, "ALIGNED_INV_DESC_MULTI_BUFF");
-		ret = testapp_invalid_desc_mb(test);
+		ret = testapp_aligned_inv_desc_mb(test);
 		break;
 	case TEST_TYPE_UNALIGNED_INV_DESC_MB:
-		test_spec_set_name(test, "UNALIGNED_INV_DESC_MULTI_BUFF");
-		test->ifobj_tx->umem->unaligned_mode = true;
-		test->ifobj_rx->umem->unaligned_mode = true;
-		ret = testapp_invalid_desc_mb(test);
+		ret = testapp_unaligned_inv_desc_mb(test);
 		break;
 	case TEST_TYPE_UNALIGNED:
 		ret = testapp_unaligned(test);
@@ -2254,13 +2317,10 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
 		ret = testapp_xdp_drop(test);
 		break;
 	case TEST_TYPE_XDP_METADATA_COUNT:
-		test_spec_set_name(test, "XDP_METADATA_COUNT");
-		ret = testapp_xdp_metadata_count(test);
+		ret = testapp_xdp_metadata(test);
 		break;
 	case TEST_TYPE_XDP_METADATA_COUNT_MB:
-		test_spec_set_name(test, "XDP_METADATA_COUNT_MULTI_BUFF");
-		test->mtu = MAX_ETH_JUMBO_SIZE;
-		ret = testapp_xdp_metadata_count(test);
+		ret = testapp_xdp_metadata_mb(test);
 		break;
 	case TEST_TYPE_TOO_MANY_FRAGS:
 		ret = testapp_too_many_frags(test);
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH bpf-next 05/10] selftests/xsk: declare test names in struct
  2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
                   ` (3 preceding siblings ...)
  2023-08-09 12:43 ` [PATCH bpf-next 04/10] selftests/xsk: move all tests to separate functions Magnus Karlsson
@ 2023-08-09 12:43 ` Magnus Karlsson
  2023-08-10 12:15   ` Przemek Kitszel
  2023-08-22 12:28   ` Maciej Fijalkowski
  2023-08-09 12:43 ` [PATCH bpf-next 06/10] selftests/xsk: add option that lists all tests Magnus Karlsson
                   ` (4 subsequent siblings)
  9 siblings, 2 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa

From: Magnus Karlsson <magnus.karlsson@intel.com>

Declare the test names statically in a struct so that we can refer to
them when adding the support to execute a single test in the next
commit. Before this pathc, the names of them was not declared in a
single place which made it not possible to refer to them.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 191 +++++++----------------
 tools/testing/selftests/bpf/xskxceiver.h |  37 +----
 2 files changed, 57 insertions(+), 171 deletions(-)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index ee72bb0a8978..b1d0c69f21b8 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -447,7 +447,8 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 }
 
 static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
-			   struct ifobject *ifobj_rx, enum test_mode mode)
+			   struct ifobject *ifobj_rx, enum test_mode mode,
+			   const struct test_spec *test_to_run)
 {
 	struct pkt_stream *tx_pkt_stream;
 	struct pkt_stream *rx_pkt_stream;
@@ -469,6 +470,8 @@ static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 			ifobj->bind_flags |= XDP_COPY;
 	}
 
+	strncpy(test->name, test_to_run->name, MAX_TEST_NAME_SIZE);
+	test->test_func = test_to_run->test_func;
 	test->mode = mode;
 	__test_spec_init(test, ifobj_tx, ifobj_rx);
 }
@@ -478,11 +481,6 @@ static void test_spec_reset(struct test_spec *test)
 	__test_spec_init(test, test->ifobj_tx, test->ifobj_rx);
 }
 
-static void test_spec_set_name(struct test_spec *test, const char *name)
-{
-	strncpy(test->name, name, MAX_TEST_NAME_SIZE);
-}
-
 static void test_spec_set_xdp_prog(struct test_spec *test, struct bpf_program *xdp_prog_rx,
 				   struct bpf_program *xdp_prog_tx, struct bpf_map *xskmap_rx,
 				   struct bpf_map *xskmap_tx)
@@ -1727,7 +1725,6 @@ static int testapp_teardown(struct test_spec *test)
 {
 	int i;
 
-	test_spec_set_name(test, "TEARDOWN");
 	for (i = 0; i < MAX_TEARDOWN_ITER; i++) {
 		if (testapp_validate_traffic(test))
 			return TEST_FAILURE;
@@ -1749,11 +1746,10 @@ static void swap_directions(struct ifobject **ifobj1, struct ifobject **ifobj2)
 	*ifobj2 = tmp_ifobj;
 }
 
-static int testapp_bidi(struct test_spec *test)
+static int testapp_bidirectional(struct test_spec *test)
 {
 	int res;
 
-	test_spec_set_name(test, "BIDIRECTIONAL");
 	test->ifobj_tx->rx_on = true;
 	test->ifobj_rx->tx_on = true;
 	test->total_steps = 2;
@@ -1782,9 +1778,8 @@ static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj
 		exit_with_error(errno);
 }
 
-static int testapp_bpf_res(struct test_spec *test)
+static int testapp_xdp_prog_cleanup(struct test_spec *test)
 {
-	test_spec_set_name(test, "BPF_RES");
 	test->total_steps = 2;
 	test->nb_sockets = 2;
 	if (testapp_validate_traffic(test))
@@ -1796,14 +1791,12 @@ static int testapp_bpf_res(struct test_spec *test)
 
 static int testapp_headroom(struct test_spec *test)
 {
-	test_spec_set_name(test, "UMEM_HEADROOM");
 	test->ifobj_rx->umem->frame_headroom = UMEM_HEADROOM_TEST_SIZE;
 	return testapp_validate_traffic(test);
 }
 
 static int testapp_stats_rx_dropped(struct test_spec *test)
 {
-	test_spec_set_name(test, "STAT_RX_DROPPED");
 	if (test->mode == TEST_MODE_ZC) {
 		ksft_test_result_skip("Can not run RX_DROPPED test for ZC mode\n");
 		return TEST_SKIP;
@@ -1819,7 +1812,6 @@ static int testapp_stats_rx_dropped(struct test_spec *test)
 
 static int testapp_stats_tx_invalid_descs(struct test_spec *test)
 {
-	test_spec_set_name(test, "STAT_TX_INVALID");
 	pkt_stream_replace_half(test, XSK_UMEM__INVALID_FRAME_SIZE, 0);
 	test->ifobj_tx->validation_func = validate_tx_invalid_descs;
 	return testapp_validate_traffic(test);
@@ -1827,7 +1819,6 @@ static int testapp_stats_tx_invalid_descs(struct test_spec *test)
 
 static int testapp_stats_rx_full(struct test_spec *test)
 {
-	test_spec_set_name(test, "STAT_RX_FULL");
 	pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
 	test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
 							 DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
@@ -1840,7 +1831,6 @@ static int testapp_stats_rx_full(struct test_spec *test)
 
 static int testapp_stats_fill_empty(struct test_spec *test)
 {
-	test_spec_set_name(test, "STAT_RX_FILL_EMPTY");
 	pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
 	test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
 							 DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
@@ -1850,9 +1840,8 @@ static int testapp_stats_fill_empty(struct test_spec *test)
 	return testapp_validate_traffic(test);
 }
 
-static int testapp_unaligned(struct test_spec *test)
+static int testapp_send_receive_unaligned(struct test_spec *test)
 {
-	test_spec_set_name(test, "UNALIGNED_MODE");
 	test->ifobj_tx->umem->unaligned_mode = true;
 	test->ifobj_rx->umem->unaligned_mode = true;
 	/* Let half of the packets straddle a 4K buffer boundary */
@@ -1861,9 +1850,8 @@ static int testapp_unaligned(struct test_spec *test)
 	return testapp_validate_traffic(test);
 }
 
-static int testapp_unaligned_mb(struct test_spec *test)
+static int testapp_send_receive_unaligned_mb(struct test_spec *test)
 {
-	test_spec_set_name(test, "UNALIGNED_MODE_9K");
 	test->mtu = MAX_ETH_JUMBO_SIZE;
 	test->ifobj_tx->umem->unaligned_mode = true;
 	test->ifobj_rx->umem->unaligned_mode = true;
@@ -1875,14 +1863,12 @@ static int testapp_single_pkt(struct test_spec *test)
 {
 	struct pkt pkts[] = {{0, MIN_PKT_SIZE, 0, true}};
 
-	test_spec_set_name(test, "SEND_RECEIVE_SINGLE_PKT");
 	pkt_stream_generate_custom(test, pkts, ARRAY_SIZE(pkts));
 	return testapp_validate_traffic(test);
 }
 
-static int testapp_multi_buffer(struct test_spec *test)
+static int testapp_send_receive_mb(struct test_spec *test)
 {
-	test_spec_set_name(test, "SEND_RECEIVE_9K_PACKETS");
 	test->mtu = MAX_ETH_JUMBO_SIZE;
 	pkt_stream_replace(test, DEFAULT_PKT_CNT, MAX_ETH_JUMBO_SIZE);
 
@@ -1979,7 +1965,6 @@ static int testapp_xdp_drop(struct test_spec *test)
 	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
 	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
 
-	test_spec_set_name(test, "XDP_DROP_HALF");
 	test_spec_set_xdp_prog(test, skel_rx->progs.xsk_xdp_drop, skel_tx->progs.xsk_xdp_drop,
 			       skel_rx->maps.xsk, skel_tx->maps.xsk);
 
@@ -2012,8 +1997,6 @@ static int testapp_xdp_metadata_copy(struct test_spec *test)
 
 static int testapp_poll_txq_tmout(struct test_spec *test)
 {
-	test_spec_set_name(test, "POLL_TXQ_FULL");
-
 	test->ifobj_tx->use_poll = true;
 	/* create invalid frame by set umem frame_size and pkt length equal to 2048 */
 	test->ifobj_tx->umem->frame_size = 2048;
@@ -2023,7 +2006,6 @@ static int testapp_poll_txq_tmout(struct test_spec *test)
 
 static int testapp_poll_rxq_tmout(struct test_spec *test)
 {
-	test_spec_set_name(test, "POLL_RXQ_EMPTY");
 	test->ifobj_rx->use_poll = true;
 	return testapp_validate_traffic_single_thread(test, test->ifobj_rx);
 }
@@ -2033,7 +2015,6 @@ static int testapp_too_many_frags(struct test_spec *test)
 	struct pkt pkts[2 * XSK_DESC__MAX_SKB_FRAGS + 2] = {};
 	u32 max_frags, i;
 
-	test_spec_set_name(test, "TOO_MANY_FRAGS");
 	if (test->mode == TEST_MODE_ZC)
 		max_frags = test->ifobj_tx->xdp_zc_max_segs;
 	else
@@ -2139,13 +2120,11 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
 
 static int testapp_send_receive(struct test_spec *test)
 {
-	test_spec_set_name(test, "SEND_RECEIVE");
 	return testapp_validate_traffic(test);
 }
 
 static int testapp_send_receive_2k_frame(struct test_spec *test)
 {
-	test_spec_set_name(test, "SEND_RECEIVE_2K_FRAME_SIZE");
 	test->ifobj_tx->umem->frame_size = 2048;
 	test->ifobj_rx->umem->frame_size = 2048;
 	pkt_stream_replace(test, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
@@ -2155,26 +2134,22 @@ static int testapp_send_receive_2k_frame(struct test_spec *test)
 static int testapp_poll_rx(struct test_spec *test)
 {
 	test->ifobj_rx->use_poll = true;
-	test_spec_set_name(test, "POLL_RX");
 	return testapp_validate_traffic(test);
 }
 
 static int testapp_poll_tx(struct test_spec *test)
 {
 	test->ifobj_tx->use_poll = true;
-	test_spec_set_name(test, "POLL_TX");
 	return testapp_validate_traffic(test);
 }
 
 static int testapp_aligned_inv_desc(struct test_spec *test)
 {
-	test_spec_set_name(test, "ALIGNED_INV_DESC");
 	return testapp_invalid_desc(test);
 }
 
 static int testapp_aligned_inv_desc_2k_frame(struct test_spec *test)
 {
-	test_spec_set_name(test, "ALIGNED_INV_DESC_2K_FRAME_SIZE");
 	test->ifobj_tx->umem->frame_size = 2048;
 	test->ifobj_rx->umem->frame_size = 2048;
 	return testapp_invalid_desc(test);
@@ -2182,7 +2157,6 @@ static int testapp_aligned_inv_desc_2k_frame(struct test_spec *test)
 
 static int testapp_unaligned_inv_desc(struct test_spec *test)
 {
-	test_spec_set_name(test, "UNALIGNED_INV_DESC");
 	test->ifobj_tx->umem->unaligned_mode = true;
 	test->ifobj_rx->umem->unaligned_mode = true;
 	return testapp_invalid_desc(test);
@@ -2192,7 +2166,6 @@ static int testapp_unaligned_inv_desc_4001_frame(struct test_spec *test)
 {
 	u64 page_size, umem_size;
 
-	test_spec_set_name(test, "UNALIGNED_INV_DESC_4K1_FRAME_SIZE");
 	/* Odd frame size so the UMEM doesn't end near a page boundary. */
 	test->ifobj_tx->umem->frame_size = 4001;
 	test->ifobj_rx->umem->frame_size = 4001;
@@ -2211,13 +2184,11 @@ static int testapp_unaligned_inv_desc_4001_frame(struct test_spec *test)
 
 static int testapp_aligned_inv_desc_mb(struct test_spec *test)
 {
-	test_spec_set_name(test, "ALIGNED_INV_DESC_MULTI_BUFF");
 	return testapp_invalid_desc_mb(test);
 }
 
 static int testapp_unaligned_inv_desc_mb(struct test_spec *test)
 {
-	test_spec_set_name(test, "UNALIGNED_INV_DESC_MULTI_BUFF");
 	test->ifobj_tx->umem->unaligned_mode = true;
 	test->ifobj_rx->umem->unaligned_mode = true;
 	return testapp_invalid_desc_mb(test);
@@ -2225,109 +2196,20 @@ static int testapp_unaligned_inv_desc_mb(struct test_spec *test)
 
 static int testapp_xdp_metadata(struct test_spec *test)
 {
-	test_spec_set_name(test, "XDP_METADATA_COPY");
 	return testapp_xdp_metadata_copy(test);
 }
 
 static int testapp_xdp_metadata_mb(struct test_spec *test)
 {
-	test_spec_set_name(test, "XDP_METADATA_COPY_MULTI_BUFF");
 	test->mtu = MAX_ETH_JUMBO_SIZE;
 	return testapp_xdp_metadata_copy(test);
 }
 
-static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
-{
-	int ret = TEST_SKIP;
-
-	switch (type) {
-	case TEST_TYPE_STATS_RX_DROPPED:
-		ret = testapp_stats_rx_dropped(test);
-		break;
-	case TEST_TYPE_STATS_TX_INVALID_DESCS:
-		ret = testapp_stats_tx_invalid_descs(test);
-		break;
-	case TEST_TYPE_STATS_RX_FULL:
-		ret = testapp_stats_rx_full(test);
-		break;
-	case TEST_TYPE_STATS_FILL_EMPTY:
-		ret = testapp_stats_fill_empty(test);
-		break;
-	case TEST_TYPE_TEARDOWN:
-		ret = testapp_teardown(test);
-		break;
-	case TEST_TYPE_BIDI:
-		ret = testapp_bidi(test);
-		break;
-	case TEST_TYPE_BPF_RES:
-		ret = testapp_bpf_res(test);
-		break;
-	case TEST_TYPE_RUN_TO_COMPLETION:
-		ret = testapp_send_receive(test);
-		break;
-	case TEST_TYPE_RUN_TO_COMPLETION_MB:
-		ret = testapp_multi_buffer(test);
-		break;
-	case TEST_TYPE_RUN_TO_COMPLETION_SINGLE_PKT:
-		ret = testapp_single_pkt(test);
-		break;
-	case TEST_TYPE_RUN_TO_COMPLETION_2K_FRAME:
-		ret = testapp_send_receive_2k_frame(test);
-		break;
-	case TEST_TYPE_RX_POLL:
-		ret = testapp_poll_rx(test);
-		break;
-	case TEST_TYPE_TX_POLL:
-		ret = testapp_poll_tx(test);
-		break;
-	case TEST_TYPE_POLL_TXQ_TMOUT:
-		ret = testapp_poll_txq_tmout(test);
-		break;
-	case TEST_TYPE_POLL_RXQ_TMOUT:
-		ret = testapp_poll_rxq_tmout(test);
-		break;
-	case TEST_TYPE_ALIGNED_INV_DESC:
-		ret = testapp_aligned_inv_desc(test);
-		break;
-	case TEST_TYPE_ALIGNED_INV_DESC_2K_FRAME:
-		ret = testapp_aligned_inv_desc_2k_frame(test);
-		break;
-	case TEST_TYPE_UNALIGNED_INV_DESC:
-		ret = testapp_unaligned_inv_desc(test);
-		break;
-	case TEST_TYPE_UNALIGNED_INV_DESC_4K1_FRAME:
-		ret = testapp_unaligned_inv_desc_4001_frame(test);
-		break;
-	case TEST_TYPE_ALIGNED_INV_DESC_MB:
-		ret = testapp_aligned_inv_desc_mb(test);
-		break;
-	case TEST_TYPE_UNALIGNED_INV_DESC_MB:
-		ret = testapp_unaligned_inv_desc_mb(test);
-		break;
-	case TEST_TYPE_UNALIGNED:
-		ret = testapp_unaligned(test);
-		break;
-	case TEST_TYPE_UNALIGNED_MB:
-		ret = testapp_unaligned_mb(test);
-		break;
-	case TEST_TYPE_HEADROOM:
-		ret = testapp_headroom(test);
-		break;
-	case TEST_TYPE_XDP_DROP_HALF:
-		ret = testapp_xdp_drop(test);
-		break;
-	case TEST_TYPE_XDP_METADATA_COUNT:
-		ret = testapp_xdp_metadata(test);
-		break;
-	case TEST_TYPE_XDP_METADATA_COUNT_MB:
-		ret = testapp_xdp_metadata_mb(test);
-		break;
-	case TEST_TYPE_TOO_MANY_FRAGS:
-		ret = testapp_too_many_frags(test);
-		break;
-	default:
-		break;
-	}
+static void run_pkt_test(struct test_spec *test)
+{
+	int ret;
+
+	ret = test->test_func(test);
 
 	if (ret == TEST_PASS)
 		ksft_test_result_pass("PASS: %s %s%s\n", mode_string(test), busy_poll_string(test),
@@ -2395,6 +2277,39 @@ static bool is_xdp_supported(int ifindex)
 	return true;
 }
 
+static const struct test_spec tests[] = {
+	{.name = "SEND_RECEIVE", .test_func = testapp_send_receive},
+	{.name = "SEND_RECEIVE_2K_FRAME", .test_func = testapp_send_receive_2k_frame},
+	{.name = "SEND_RECEIVE_SINGLE_PKT", .test_func = testapp_single_pkt},
+	{.name = "POLL_RX", .test_func = testapp_poll_rx},
+	{.name = "POLL_TX", .test_func = testapp_poll_tx},
+	{.name = "POLL_RXQ_FULL", .test_func = testapp_poll_rxq_tmout},
+	{.name = "POLL_TXQ_FULL", .test_func = testapp_poll_txq_tmout},
+	{.name = "SEND_RECEIVE_UNALIGNED", .test_func = testapp_send_receive_unaligned},
+	{.name = "ALIGNED_INV_DESC", .test_func = testapp_aligned_inv_desc},
+	{.name = "ALIGNED_INV_DESC_2K_FRAME_SIZE", .test_func = testapp_aligned_inv_desc_2k_frame},
+	{.name = "UNALIGNED_INV_DESC", .test_func = testapp_unaligned_inv_desc},
+	{.name = "UNALIGNED_INV_DESC_4001_FRAME_SIZE",
+	 .test_func = testapp_unaligned_inv_desc_4001_frame},
+	{.name = "UMEM_HEADROOM", .test_func = testapp_headroom},
+	{.name = "TEARDOWN", .test_func = testapp_teardown},
+	{.name = "BIDIRECTIONAL", .test_func = testapp_bidirectional},
+	{.name = "STAT_RX_DROPPED", .test_func = testapp_stats_rx_dropped},
+	{.name = "STAT_TX_INVALID", .test_func = testapp_stats_tx_invalid_descs},
+	{.name = "STAT_RX_FULL", .test_func = testapp_stats_rx_full},
+	{.name = "STAT_FILL_EMPTY", .test_func = testapp_stats_fill_empty},
+	{.name = "XDP_PROG_CLEANUP", .test_func = testapp_xdp_prog_cleanup},
+	{.name = "XDP_DROP_HALF", .test_func = testapp_xdp_drop},
+	{.name = "XDP_METADATA_COPY", .test_func = testapp_xdp_metadata},
+	{.name = "XDP_METADATA_COPY_MULTI_BUFF", .test_func = testapp_xdp_metadata_mb},
+	{.name = "SEND_RECEIVE_9K_PACKETS", .test_func = testapp_send_receive_mb},
+	{.name = "SEND_RECEIVE_UNALIGNED_9K_PACKETS",
+	 .test_func = testapp_send_receive_unaligned_mb},
+	{.name = "ALIGNED_INV_DESC_MULTI_BUFF", .test_func = testapp_aligned_inv_desc_mb},
+	{.name = "UNALIGNED_INV_DESC_MULTI_BUFF", .test_func = testapp_unaligned_inv_desc_mb},
+	{.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
+};
+
 int main(int argc, char **argv)
 {
 	struct pkt_stream *rx_pkt_stream_default;
@@ -2437,7 +2352,7 @@ int main(int argc, char **argv)
 	init_iface(ifobj_rx, MAC1, MAC2, worker_testapp_validate_rx);
 	init_iface(ifobj_tx, MAC2, MAC1, worker_testapp_validate_tx);
 
-	test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
+	test_spec_init(&test, ifobj_tx, ifobj_rx, 0, &tests[0]);
 	tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
 	rx_pkt_stream_default = pkt_stream_generate(ifobj_rx->umem, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
 	if (!tx_pkt_stream_default || !rx_pkt_stream_default)
@@ -2446,17 +2361,17 @@ int main(int argc, char **argv)
 	test.rx_pkt_stream_default = rx_pkt_stream_default;
 
 	if (opt_mode == TEST_MODE_ALL)
-		ksft_set_plan(modes * TEST_TYPE_MAX);
+		ksft_set_plan(modes * ARRAY_SIZE(tests));
 	else
-		ksft_set_plan(TEST_TYPE_MAX);
+		ksft_set_plan(ARRAY_SIZE(tests));
 
 	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);
+		for (j = 0; j < ARRAY_SIZE(tests); j++) {
+			test_spec_init(&test, ifobj_tx, ifobj_rx, i, &tests[j]);
+			run_pkt_test(&test);
 			usleep(USLEEP_MAX);
 
 			if (test.fail)
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index 1412492e9618..3a71d490db3e 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -34,7 +34,7 @@
 #define MAX_INTERFACES 2
 #define MAX_INTERFACE_NAME_CHARS 16
 #define MAX_SOCKETS 2
-#define MAX_TEST_NAME_SIZE 32
+#define MAX_TEST_NAME_SIZE 48
 #define MAX_TEARDOWN_ITER 10
 #define PKT_HDR_SIZE (sizeof(struct ethhdr) + 2) /* Just to align the data in the packet */
 #define MIN_PKT_SIZE 64
@@ -66,38 +66,6 @@ enum test_mode {
 	TEST_MODE_ALL
 };
 
-enum test_type {
-	TEST_TYPE_RUN_TO_COMPLETION,
-	TEST_TYPE_RUN_TO_COMPLETION_2K_FRAME,
-	TEST_TYPE_RUN_TO_COMPLETION_SINGLE_PKT,
-	TEST_TYPE_RX_POLL,
-	TEST_TYPE_TX_POLL,
-	TEST_TYPE_POLL_RXQ_TMOUT,
-	TEST_TYPE_POLL_TXQ_TMOUT,
-	TEST_TYPE_UNALIGNED,
-	TEST_TYPE_ALIGNED_INV_DESC,
-	TEST_TYPE_ALIGNED_INV_DESC_2K_FRAME,
-	TEST_TYPE_UNALIGNED_INV_DESC,
-	TEST_TYPE_UNALIGNED_INV_DESC_4K1_FRAME,
-	TEST_TYPE_HEADROOM,
-	TEST_TYPE_TEARDOWN,
-	TEST_TYPE_BIDI,
-	TEST_TYPE_STATS_RX_DROPPED,
-	TEST_TYPE_STATS_TX_INVALID_DESCS,
-	TEST_TYPE_STATS_RX_FULL,
-	TEST_TYPE_STATS_FILL_EMPTY,
-	TEST_TYPE_BPF_RES,
-	TEST_TYPE_XDP_DROP_HALF,
-	TEST_TYPE_XDP_METADATA_COUNT,
-	TEST_TYPE_XDP_METADATA_COUNT_MB,
-	TEST_TYPE_RUN_TO_COMPLETION_MB,
-	TEST_TYPE_UNALIGNED_MB,
-	TEST_TYPE_ALIGNED_INV_DESC_MB,
-	TEST_TYPE_UNALIGNED_INV_DESC_MB,
-	TEST_TYPE_TOO_MANY_FRAGS,
-	TEST_TYPE_MAX
-};
-
 struct xsk_umem_info {
 	struct xsk_ring_prod fq;
 	struct xsk_ring_cons cq;
@@ -137,8 +105,10 @@ struct pkt_stream {
 };
 
 struct ifobject;
+struct test_spec;
 typedef int (*validation_func_t)(struct ifobject *ifobj);
 typedef void *(*thread_func_t)(void *arg);
+typedef int (*test_func_t)(struct test_spec *test);
 
 struct ifobject {
 	char ifname[MAX_INTERFACE_NAME_CHARS];
@@ -180,6 +150,7 @@ struct test_spec {
 	struct bpf_program *xdp_prog_tx;
 	struct bpf_map *xskmap_rx;
 	struct bpf_map *xskmap_tx;
+	test_func_t test_func;
 	int mtu;
 	u16 total_steps;
 	u16 current_step;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 05/10] selftests/xsk: declare test names in struct
  2023-08-09 12:43 ` [PATCH bpf-next 05/10] selftests/xsk: declare test names in struct Magnus Karlsson
@ 2023-08-10 12:15   ` Przemek Kitszel
  2023-08-10 12:36     ` Magnus Karlsson
  2023-08-22 12:28   ` Maciej Fijalkowski
  1 sibling, 1 reply; 22+ messages in thread
From: Przemek Kitszel @ 2023-08-10 12:15 UTC (permalink / raw)
  To: Magnus Karlsson, Karlsson, Magnus, bjorn@kernel.org,
	ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
	Fijalkowski, Maciej, bpf@vger.kernel.org, yhs@fb.com,
	andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
	haoluo@google.com, jolsa@kernel.org

On 8/9/23 14:43, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Declare the test names statically in a struct so that we can refer to
> them when adding the support to execute a single test in the next
> commit. Before this pathc, the names of them was not declared in a

patch

> single place which made it not possible to refer to them.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>   tools/testing/selftests/bpf/xskxceiver.c | 191 +++++++----------------
>   tools/testing/selftests/bpf/xskxceiver.h |  37 +----
>   2 files changed, 57 insertions(+), 171 deletions(-)

[...]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 05/10] selftests/xsk: declare test names in struct
  2023-08-10 12:15   ` Przemek Kitszel
@ 2023-08-10 12:36     ` Magnus Karlsson
  0 siblings, 0 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-10 12:36 UTC (permalink / raw)
  To: Przemek Kitszel
  Cc: Karlsson, Magnus, bjorn@kernel.org, ast@kernel.org,
	daniel@iogearbox.net, netdev@vger.kernel.org, Fijalkowski, Maciej,
	bpf@vger.kernel.org, yhs@fb.com, andrii@kernel.org,
	martin.lau@linux.dev, song@kernel.org, john.fastabend@gmail.com,
	kpsingh@kernel.org, sdf@google.com, haoluo@google.com,
	jolsa@kernel.org

On Thu, 10 Aug 2023 at 14:16, Przemek Kitszel
<przemyslaw.kitszel@intel.com> wrote:
>
> On 8/9/23 14:43, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Declare the test names statically in a struct so that we can refer to
> > them when adding the support to execute a single test in the next
> > commit. Before this pathc, the names of them was not declared in a
>
> patch

Thanks for catching. Will fix it in the next revision.

> > single place which made it not possible to refer to them.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >   tools/testing/selftests/bpf/xskxceiver.c | 191 +++++++----------------
> >   tools/testing/selftests/bpf/xskxceiver.h |  37 +----
> >   2 files changed, 57 insertions(+), 171 deletions(-)
>
> [...]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 05/10] selftests/xsk: declare test names in struct
  2023-08-09 12:43 ` [PATCH bpf-next 05/10] selftests/xsk: declare test names in struct Magnus Karlsson
  2023-08-10 12:15   ` Przemek Kitszel
@ 2023-08-22 12:28   ` Maciej Fijalkowski
  1 sibling, 0 replies; 22+ messages in thread
From: Maciej Fijalkowski @ 2023-08-22 12:28 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, bpf, yhs, andrii,
	martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa

On Wed, Aug 09, 2023 at 02:43:38PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Declare the test names statically in a struct so that we can refer to
> them when adding the support to execute a single test in the next
> commit. Before this pathc, the names of them was not declared in a

s/was/were

pathc was caught by Przemek

> single place which made it not possible to refer to them.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/xskxceiver.c | 191 +++++++----------------
>  tools/testing/selftests/bpf/xskxceiver.h |  37 +----
>  2 files changed, 57 insertions(+), 171 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index ee72bb0a8978..b1d0c69f21b8 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -447,7 +447,8 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  }
>  
>  static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> -			   struct ifobject *ifobj_rx, enum test_mode mode)
> +			   struct ifobject *ifobj_rx, enum test_mode mode,
> +			   const struct test_spec *test_to_run)
>  {
>  	struct pkt_stream *tx_pkt_stream;
>  	struct pkt_stream *rx_pkt_stream;
> @@ -469,6 +470,8 @@ static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  			ifobj->bind_flags |= XDP_COPY;
>  	}
>  
> +	strncpy(test->name, test_to_run->name, MAX_TEST_NAME_SIZE);
> +	test->test_func = test_to_run->test_func;
>  	test->mode = mode;
>  	__test_spec_init(test, ifobj_tx, ifobj_rx);
>  }
> @@ -478,11 +481,6 @@ static void test_spec_reset(struct test_spec *test)
>  	__test_spec_init(test, test->ifobj_tx, test->ifobj_rx);
>  }
>  
> -static void test_spec_set_name(struct test_spec *test, const char *name)
> -{
> -	strncpy(test->name, name, MAX_TEST_NAME_SIZE);
> -}
> -
>  static void test_spec_set_xdp_prog(struct test_spec *test, struct bpf_program *xdp_prog_rx,
>  				   struct bpf_program *xdp_prog_tx, struct bpf_map *xskmap_rx,
>  				   struct bpf_map *xskmap_tx)
> @@ -1727,7 +1725,6 @@ static int testapp_teardown(struct test_spec *test)
>  {
>  	int i;
>  
> -	test_spec_set_name(test, "TEARDOWN");
>  	for (i = 0; i < MAX_TEARDOWN_ITER; i++) {
>  		if (testapp_validate_traffic(test))
>  			return TEST_FAILURE;
> @@ -1749,11 +1746,10 @@ static void swap_directions(struct ifobject **ifobj1, struct ifobject **ifobj2)
>  	*ifobj2 = tmp_ifobj;
>  }
>  
> -static int testapp_bidi(struct test_spec *test)
> +static int testapp_bidirectional(struct test_spec *test)
>  {
>  	int res;
>  
> -	test_spec_set_name(test, "BIDIRECTIONAL");
>  	test->ifobj_tx->rx_on = true;
>  	test->ifobj_rx->tx_on = true;
>  	test->total_steps = 2;
> @@ -1782,9 +1778,8 @@ static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  		exit_with_error(errno);
>  }
>  
> -static int testapp_bpf_res(struct test_spec *test)
> +static int testapp_xdp_prog_cleanup(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "BPF_RES");
>  	test->total_steps = 2;
>  	test->nb_sockets = 2;
>  	if (testapp_validate_traffic(test))
> @@ -1796,14 +1791,12 @@ static int testapp_bpf_res(struct test_spec *test)
>  
>  static int testapp_headroom(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "UMEM_HEADROOM");
>  	test->ifobj_rx->umem->frame_headroom = UMEM_HEADROOM_TEST_SIZE;
>  	return testapp_validate_traffic(test);
>  }
>  
>  static int testapp_stats_rx_dropped(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "STAT_RX_DROPPED");
>  	if (test->mode == TEST_MODE_ZC) {
>  		ksft_test_result_skip("Can not run RX_DROPPED test for ZC mode\n");
>  		return TEST_SKIP;
> @@ -1819,7 +1812,6 @@ static int testapp_stats_rx_dropped(struct test_spec *test)
>  
>  static int testapp_stats_tx_invalid_descs(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "STAT_TX_INVALID");
>  	pkt_stream_replace_half(test, XSK_UMEM__INVALID_FRAME_SIZE, 0);
>  	test->ifobj_tx->validation_func = validate_tx_invalid_descs;
>  	return testapp_validate_traffic(test);
> @@ -1827,7 +1819,6 @@ static int testapp_stats_tx_invalid_descs(struct test_spec *test)
>  
>  static int testapp_stats_rx_full(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "STAT_RX_FULL");
>  	pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
>  	test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
>  							 DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> @@ -1840,7 +1831,6 @@ static int testapp_stats_rx_full(struct test_spec *test)
>  
>  static int testapp_stats_fill_empty(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "STAT_RX_FILL_EMPTY");
>  	pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, MIN_PKT_SIZE);
>  	test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
>  							 DEFAULT_UMEM_BUFFERS, MIN_PKT_SIZE);
> @@ -1850,9 +1840,8 @@ static int testapp_stats_fill_empty(struct test_spec *test)
>  	return testapp_validate_traffic(test);
>  }
>  
> -static int testapp_unaligned(struct test_spec *test)
> +static int testapp_send_receive_unaligned(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "UNALIGNED_MODE");
>  	test->ifobj_tx->umem->unaligned_mode = true;
>  	test->ifobj_rx->umem->unaligned_mode = true;
>  	/* Let half of the packets straddle a 4K buffer boundary */
> @@ -1861,9 +1850,8 @@ static int testapp_unaligned(struct test_spec *test)
>  	return testapp_validate_traffic(test);
>  }
>  
> -static int testapp_unaligned_mb(struct test_spec *test)
> +static int testapp_send_receive_unaligned_mb(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "UNALIGNED_MODE_9K");
>  	test->mtu = MAX_ETH_JUMBO_SIZE;
>  	test->ifobj_tx->umem->unaligned_mode = true;
>  	test->ifobj_rx->umem->unaligned_mode = true;
> @@ -1875,14 +1863,12 @@ static int testapp_single_pkt(struct test_spec *test)
>  {
>  	struct pkt pkts[] = {{0, MIN_PKT_SIZE, 0, true}};
>  
> -	test_spec_set_name(test, "SEND_RECEIVE_SINGLE_PKT");
>  	pkt_stream_generate_custom(test, pkts, ARRAY_SIZE(pkts));
>  	return testapp_validate_traffic(test);
>  }
>  
> -static int testapp_multi_buffer(struct test_spec *test)
> +static int testapp_send_receive_mb(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "SEND_RECEIVE_9K_PACKETS");
>  	test->mtu = MAX_ETH_JUMBO_SIZE;
>  	pkt_stream_replace(test, DEFAULT_PKT_CNT, MAX_ETH_JUMBO_SIZE);
>  
> @@ -1979,7 +1965,6 @@ static int testapp_xdp_drop(struct test_spec *test)
>  	struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
>  	struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
>  
> -	test_spec_set_name(test, "XDP_DROP_HALF");
>  	test_spec_set_xdp_prog(test, skel_rx->progs.xsk_xdp_drop, skel_tx->progs.xsk_xdp_drop,
>  			       skel_rx->maps.xsk, skel_tx->maps.xsk);
>  
> @@ -2012,8 +1997,6 @@ static int testapp_xdp_metadata_copy(struct test_spec *test)
>  
>  static int testapp_poll_txq_tmout(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "POLL_TXQ_FULL");
> -
>  	test->ifobj_tx->use_poll = true;
>  	/* create invalid frame by set umem frame_size and pkt length equal to 2048 */
>  	test->ifobj_tx->umem->frame_size = 2048;
> @@ -2023,7 +2006,6 @@ static int testapp_poll_txq_tmout(struct test_spec *test)
>  
>  static int testapp_poll_rxq_tmout(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "POLL_RXQ_EMPTY");
>  	test->ifobj_rx->use_poll = true;
>  	return testapp_validate_traffic_single_thread(test, test->ifobj_rx);
>  }
> @@ -2033,7 +2015,6 @@ static int testapp_too_many_frags(struct test_spec *test)
>  	struct pkt pkts[2 * XSK_DESC__MAX_SKB_FRAGS + 2] = {};
>  	u32 max_frags, i;
>  
> -	test_spec_set_name(test, "TOO_MANY_FRAGS");
>  	if (test->mode == TEST_MODE_ZC)
>  		max_frags = test->ifobj_tx->xdp_zc_max_segs;
>  	else
> @@ -2139,13 +2120,11 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
>  
>  static int testapp_send_receive(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "SEND_RECEIVE");
>  	return testapp_validate_traffic(test);
>  }
>  
>  static int testapp_send_receive_2k_frame(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "SEND_RECEIVE_2K_FRAME_SIZE");
>  	test->ifobj_tx->umem->frame_size = 2048;
>  	test->ifobj_rx->umem->frame_size = 2048;
>  	pkt_stream_replace(test, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
> @@ -2155,26 +2134,22 @@ static int testapp_send_receive_2k_frame(struct test_spec *test)
>  static int testapp_poll_rx(struct test_spec *test)
>  {
>  	test->ifobj_rx->use_poll = true;
> -	test_spec_set_name(test, "POLL_RX");
>  	return testapp_validate_traffic(test);
>  }
>  
>  static int testapp_poll_tx(struct test_spec *test)
>  {
>  	test->ifobj_tx->use_poll = true;
> -	test_spec_set_name(test, "POLL_TX");
>  	return testapp_validate_traffic(test);
>  }
>  
>  static int testapp_aligned_inv_desc(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "ALIGNED_INV_DESC");
>  	return testapp_invalid_desc(test);
>  }
>  
>  static int testapp_aligned_inv_desc_2k_frame(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "ALIGNED_INV_DESC_2K_FRAME_SIZE");
>  	test->ifobj_tx->umem->frame_size = 2048;
>  	test->ifobj_rx->umem->frame_size = 2048;
>  	return testapp_invalid_desc(test);
> @@ -2182,7 +2157,6 @@ static int testapp_aligned_inv_desc_2k_frame(struct test_spec *test)
>  
>  static int testapp_unaligned_inv_desc(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "UNALIGNED_INV_DESC");
>  	test->ifobj_tx->umem->unaligned_mode = true;
>  	test->ifobj_rx->umem->unaligned_mode = true;
>  	return testapp_invalid_desc(test);
> @@ -2192,7 +2166,6 @@ static int testapp_unaligned_inv_desc_4001_frame(struct test_spec *test)
>  {
>  	u64 page_size, umem_size;
>  
> -	test_spec_set_name(test, "UNALIGNED_INV_DESC_4K1_FRAME_SIZE");
>  	/* Odd frame size so the UMEM doesn't end near a page boundary. */
>  	test->ifobj_tx->umem->frame_size = 4001;
>  	test->ifobj_rx->umem->frame_size = 4001;
> @@ -2211,13 +2184,11 @@ static int testapp_unaligned_inv_desc_4001_frame(struct test_spec *test)
>  
>  static int testapp_aligned_inv_desc_mb(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "ALIGNED_INV_DESC_MULTI_BUFF");
>  	return testapp_invalid_desc_mb(test);
>  }
>  
>  static int testapp_unaligned_inv_desc_mb(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "UNALIGNED_INV_DESC_MULTI_BUFF");
>  	test->ifobj_tx->umem->unaligned_mode = true;
>  	test->ifobj_rx->umem->unaligned_mode = true;
>  	return testapp_invalid_desc_mb(test);
> @@ -2225,109 +2196,20 @@ static int testapp_unaligned_inv_desc_mb(struct test_spec *test)
>  
>  static int testapp_xdp_metadata(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "XDP_METADATA_COPY");
>  	return testapp_xdp_metadata_copy(test);
>  }
>  
>  static int testapp_xdp_metadata_mb(struct test_spec *test)
>  {
> -	test_spec_set_name(test, "XDP_METADATA_COPY_MULTI_BUFF");
>  	test->mtu = MAX_ETH_JUMBO_SIZE;
>  	return testapp_xdp_metadata_copy(test);
>  }
>  
> -static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
> -{
> -	int ret = TEST_SKIP;
> -
> -	switch (type) {
> -	case TEST_TYPE_STATS_RX_DROPPED:
> -		ret = testapp_stats_rx_dropped(test);
> -		break;
> -	case TEST_TYPE_STATS_TX_INVALID_DESCS:
> -		ret = testapp_stats_tx_invalid_descs(test);
> -		break;
> -	case TEST_TYPE_STATS_RX_FULL:
> -		ret = testapp_stats_rx_full(test);
> -		break;
> -	case TEST_TYPE_STATS_FILL_EMPTY:
> -		ret = testapp_stats_fill_empty(test);
> -		break;
> -	case TEST_TYPE_TEARDOWN:
> -		ret = testapp_teardown(test);
> -		break;
> -	case TEST_TYPE_BIDI:
> -		ret = testapp_bidi(test);
> -		break;
> -	case TEST_TYPE_BPF_RES:
> -		ret = testapp_bpf_res(test);
> -		break;
> -	case TEST_TYPE_RUN_TO_COMPLETION:
> -		ret = testapp_send_receive(test);
> -		break;
> -	case TEST_TYPE_RUN_TO_COMPLETION_MB:
> -		ret = testapp_multi_buffer(test);
> -		break;
> -	case TEST_TYPE_RUN_TO_COMPLETION_SINGLE_PKT:
> -		ret = testapp_single_pkt(test);
> -		break;
> -	case TEST_TYPE_RUN_TO_COMPLETION_2K_FRAME:
> -		ret = testapp_send_receive_2k_frame(test);
> -		break;
> -	case TEST_TYPE_RX_POLL:
> -		ret = testapp_poll_rx(test);
> -		break;
> -	case TEST_TYPE_TX_POLL:
> -		ret = testapp_poll_tx(test);
> -		break;
> -	case TEST_TYPE_POLL_TXQ_TMOUT:
> -		ret = testapp_poll_txq_tmout(test);
> -		break;
> -	case TEST_TYPE_POLL_RXQ_TMOUT:
> -		ret = testapp_poll_rxq_tmout(test);
> -		break;
> -	case TEST_TYPE_ALIGNED_INV_DESC:
> -		ret = testapp_aligned_inv_desc(test);
> -		break;
> -	case TEST_TYPE_ALIGNED_INV_DESC_2K_FRAME:
> -		ret = testapp_aligned_inv_desc_2k_frame(test);
> -		break;
> -	case TEST_TYPE_UNALIGNED_INV_DESC:
> -		ret = testapp_unaligned_inv_desc(test);
> -		break;
> -	case TEST_TYPE_UNALIGNED_INV_DESC_4K1_FRAME:
> -		ret = testapp_unaligned_inv_desc_4001_frame(test);
> -		break;
> -	case TEST_TYPE_ALIGNED_INV_DESC_MB:
> -		ret = testapp_aligned_inv_desc_mb(test);
> -		break;
> -	case TEST_TYPE_UNALIGNED_INV_DESC_MB:
> -		ret = testapp_unaligned_inv_desc_mb(test);
> -		break;
> -	case TEST_TYPE_UNALIGNED:
> -		ret = testapp_unaligned(test);
> -		break;
> -	case TEST_TYPE_UNALIGNED_MB:
> -		ret = testapp_unaligned_mb(test);
> -		break;
> -	case TEST_TYPE_HEADROOM:
> -		ret = testapp_headroom(test);
> -		break;
> -	case TEST_TYPE_XDP_DROP_HALF:
> -		ret = testapp_xdp_drop(test);
> -		break;
> -	case TEST_TYPE_XDP_METADATA_COUNT:
> -		ret = testapp_xdp_metadata(test);
> -		break;
> -	case TEST_TYPE_XDP_METADATA_COUNT_MB:
> -		ret = testapp_xdp_metadata_mb(test);
> -		break;
> -	case TEST_TYPE_TOO_MANY_FRAGS:
> -		ret = testapp_too_many_frags(test);
> -		break;
> -	default:
> -		break;
> -	}
> +static void run_pkt_test(struct test_spec *test)
> +{
> +	int ret;
> +
> +	ret = test->test_func(test);
>  
>  	if (ret == TEST_PASS)
>  		ksft_test_result_pass("PASS: %s %s%s\n", mode_string(test), busy_poll_string(test),
> @@ -2395,6 +2277,39 @@ static bool is_xdp_supported(int ifindex)
>  	return true;
>  }
>  
> +static const struct test_spec tests[] = {
> +	{.name = "SEND_RECEIVE", .test_func = testapp_send_receive},
> +	{.name = "SEND_RECEIVE_2K_FRAME", .test_func = testapp_send_receive_2k_frame},
> +	{.name = "SEND_RECEIVE_SINGLE_PKT", .test_func = testapp_single_pkt},
> +	{.name = "POLL_RX", .test_func = testapp_poll_rx},
> +	{.name = "POLL_TX", .test_func = testapp_poll_tx},
> +	{.name = "POLL_RXQ_FULL", .test_func = testapp_poll_rxq_tmout},
> +	{.name = "POLL_TXQ_FULL", .test_func = testapp_poll_txq_tmout},
> +	{.name = "SEND_RECEIVE_UNALIGNED", .test_func = testapp_send_receive_unaligned},
> +	{.name = "ALIGNED_INV_DESC", .test_func = testapp_aligned_inv_desc},
> +	{.name = "ALIGNED_INV_DESC_2K_FRAME_SIZE", .test_func = testapp_aligned_inv_desc_2k_frame},
> +	{.name = "UNALIGNED_INV_DESC", .test_func = testapp_unaligned_inv_desc},
> +	{.name = "UNALIGNED_INV_DESC_4001_FRAME_SIZE",
> +	 .test_func = testapp_unaligned_inv_desc_4001_frame},
> +	{.name = "UMEM_HEADROOM", .test_func = testapp_headroom},
> +	{.name = "TEARDOWN", .test_func = testapp_teardown},
> +	{.name = "BIDIRECTIONAL", .test_func = testapp_bidirectional},
> +	{.name = "STAT_RX_DROPPED", .test_func = testapp_stats_rx_dropped},
> +	{.name = "STAT_TX_INVALID", .test_func = testapp_stats_tx_invalid_descs},
> +	{.name = "STAT_RX_FULL", .test_func = testapp_stats_rx_full},
> +	{.name = "STAT_FILL_EMPTY", .test_func = testapp_stats_fill_empty},
> +	{.name = "XDP_PROG_CLEANUP", .test_func = testapp_xdp_prog_cleanup},
> +	{.name = "XDP_DROP_HALF", .test_func = testapp_xdp_drop},
> +	{.name = "XDP_METADATA_COPY", .test_func = testapp_xdp_metadata},
> +	{.name = "XDP_METADATA_COPY_MULTI_BUFF", .test_func = testapp_xdp_metadata_mb},
> +	{.name = "SEND_RECEIVE_9K_PACKETS", .test_func = testapp_send_receive_mb},
> +	{.name = "SEND_RECEIVE_UNALIGNED_9K_PACKETS",
> +	 .test_func = testapp_send_receive_unaligned_mb},
> +	{.name = "ALIGNED_INV_DESC_MULTI_BUFF", .test_func = testapp_aligned_inv_desc_mb},
> +	{.name = "UNALIGNED_INV_DESC_MULTI_BUFF", .test_func = testapp_unaligned_inv_desc_mb},
> +	{.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
> +};

can you move this to a header file? looks weird to have these declarations
in the middle of the file.

> +
>  int main(int argc, char **argv)
>  {
>  	struct pkt_stream *rx_pkt_stream_default;
> @@ -2437,7 +2352,7 @@ int main(int argc, char **argv)
>  	init_iface(ifobj_rx, MAC1, MAC2, worker_testapp_validate_rx);
>  	init_iface(ifobj_tx, MAC2, MAC1, worker_testapp_validate_tx);
>  
> -	test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
> +	test_spec_init(&test, ifobj_tx, ifobj_rx, 0, &tests[0]);
>  	tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
>  	rx_pkt_stream_default = pkt_stream_generate(ifobj_rx->umem, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
>  	if (!tx_pkt_stream_default || !rx_pkt_stream_default)
> @@ -2446,17 +2361,17 @@ int main(int argc, char **argv)
>  	test.rx_pkt_stream_default = rx_pkt_stream_default;
>  
>  	if (opt_mode == TEST_MODE_ALL)
> -		ksft_set_plan(modes * TEST_TYPE_MAX);
> +		ksft_set_plan(modes * ARRAY_SIZE(tests));
>  	else
> -		ksft_set_plan(TEST_TYPE_MAX);
> +		ksft_set_plan(ARRAY_SIZE(tests));
>  
>  	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);
> +		for (j = 0; j < ARRAY_SIZE(tests); j++) {
> +			test_spec_init(&test, ifobj_tx, ifobj_rx, i, &tests[j]);
> +			run_pkt_test(&test);
>  			usleep(USLEEP_MAX);
>  
>  			if (test.fail)
> diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> index 1412492e9618..3a71d490db3e 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.h
> +++ b/tools/testing/selftests/bpf/xskxceiver.h
> @@ -34,7 +34,7 @@
>  #define MAX_INTERFACES 2
>  #define MAX_INTERFACE_NAME_CHARS 16
>  #define MAX_SOCKETS 2
> -#define MAX_TEST_NAME_SIZE 32
> +#define MAX_TEST_NAME_SIZE 48
>  #define MAX_TEARDOWN_ITER 10
>  #define PKT_HDR_SIZE (sizeof(struct ethhdr) + 2) /* Just to align the data in the packet */
>  #define MIN_PKT_SIZE 64
> @@ -66,38 +66,6 @@ enum test_mode {
>  	TEST_MODE_ALL
>  };
>  
> -enum test_type {
> -	TEST_TYPE_RUN_TO_COMPLETION,
> -	TEST_TYPE_RUN_TO_COMPLETION_2K_FRAME,
> -	TEST_TYPE_RUN_TO_COMPLETION_SINGLE_PKT,
> -	TEST_TYPE_RX_POLL,
> -	TEST_TYPE_TX_POLL,
> -	TEST_TYPE_POLL_RXQ_TMOUT,
> -	TEST_TYPE_POLL_TXQ_TMOUT,
> -	TEST_TYPE_UNALIGNED,
> -	TEST_TYPE_ALIGNED_INV_DESC,
> -	TEST_TYPE_ALIGNED_INV_DESC_2K_FRAME,
> -	TEST_TYPE_UNALIGNED_INV_DESC,
> -	TEST_TYPE_UNALIGNED_INV_DESC_4K1_FRAME,
> -	TEST_TYPE_HEADROOM,
> -	TEST_TYPE_TEARDOWN,
> -	TEST_TYPE_BIDI,
> -	TEST_TYPE_STATS_RX_DROPPED,
> -	TEST_TYPE_STATS_TX_INVALID_DESCS,
> -	TEST_TYPE_STATS_RX_FULL,
> -	TEST_TYPE_STATS_FILL_EMPTY,
> -	TEST_TYPE_BPF_RES,
> -	TEST_TYPE_XDP_DROP_HALF,
> -	TEST_TYPE_XDP_METADATA_COUNT,
> -	TEST_TYPE_XDP_METADATA_COUNT_MB,
> -	TEST_TYPE_RUN_TO_COMPLETION_MB,
> -	TEST_TYPE_UNALIGNED_MB,
> -	TEST_TYPE_ALIGNED_INV_DESC_MB,
> -	TEST_TYPE_UNALIGNED_INV_DESC_MB,
> -	TEST_TYPE_TOO_MANY_FRAGS,
> -	TEST_TYPE_MAX
> -};
> -
>  struct xsk_umem_info {
>  	struct xsk_ring_prod fq;
>  	struct xsk_ring_cons cq;
> @@ -137,8 +105,10 @@ struct pkt_stream {
>  };
>  
>  struct ifobject;
> +struct test_spec;
>  typedef int (*validation_func_t)(struct ifobject *ifobj);
>  typedef void *(*thread_func_t)(void *arg);
> +typedef int (*test_func_t)(struct test_spec *test);
>  
>  struct ifobject {
>  	char ifname[MAX_INTERFACE_NAME_CHARS];
> @@ -180,6 +150,7 @@ struct test_spec {
>  	struct bpf_program *xdp_prog_tx;
>  	struct bpf_map *xskmap_rx;
>  	struct bpf_map *xskmap_tx;
> +	test_func_t test_func;
>  	int mtu;
>  	u16 total_steps;
>  	u16 current_step;
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH bpf-next 06/10] selftests/xsk: add option that lists all tests
  2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
                   ` (4 preceding siblings ...)
  2023-08-09 12:43 ` [PATCH bpf-next 05/10] selftests/xsk: declare test names in struct Magnus Karlsson
@ 2023-08-09 12:43 ` Magnus Karlsson
  2023-08-22 12:37   ` Maciej Fijalkowski
  2023-08-09 12:43 ` [PATCH bpf-next 07/10] selftests/xsk: add option to run single test Magnus Karlsson
                   ` (3 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa

From: Magnus Karlsson <magnus.karlsson@intel.com>

Add a command line option (-l) that lists all the tests. The number
before the test will be used in the next commit for specifying a
single test to run. Here is an example of the output:

Tests:
0: SEND_RECEIVE
1: SEND_RECEIVE_2K_FRAME
2: SEND_RECEIVE_SINGLE_PKT
3: POLL_RX
4: POLL_TX
5: POLL_RXQ_FULL
6: POLL_TXQ_FULL
7: SEND_RECEIVE_UNALIGNED
:
:

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh    | 11 +++++++++-
 tools/testing/selftests/bpf/xsk_prereqs.sh | 10 +++++----
 tools/testing/selftests/bpf/xskxceiver.c   | 24 ++++++++++++++++++++--
 3 files changed, 38 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 4ec621f4d3db..00a504f0929a 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -81,13 +81,14 @@
 
 ETH=""
 
-while getopts "vi:dm:" flag
+while getopts "vi:dm:l" flag
 do
 	case "${flag}" in
 		v) verbose=1;;
 		d) debug=1;;
 		i) ETH=${OPTARG};;
 		m) MODE=${OPTARG};;
+		l) list=1;;
 	esac
 done
 
@@ -157,6 +158,10 @@ if [[ $verbose -eq 1 ]]; then
 	ARGS+="-v "
 fi
 
+if [[ $list -eq 1 ]]; then
+	ARGS+="-l "
+fi
+
 if [ ! -z $MODE ]; then
 	ARGS+="-m ${MODE} "
 fi
@@ -183,6 +188,10 @@ else
 	cleanup_iface ${ETH} ${MTU}
 fi
 
+if [[ $list -eq 1 ]]; then
+    exit
+fi
+
 TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
 busy_poll=1
 
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index 29175682c44d..47c7b8064f38 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -83,9 +83,11 @@ exec_xskxceiver()
 	fi
 
 	./${XSKOBJ} -i ${VETH0} -i ${VETH1} ${ARGS}
-
 	retval=$?
-	test_status $retval "${TEST_NAME}"
-	statusList+=($retval)
-	nameList+=(${TEST_NAME})
+
+	if [[ $list -ne 1 ]]; then
+	    test_status $retval "${TEST_NAME}"
+	    statusList+=($retval)
+	    nameList+=(${TEST_NAME})
+	fi
 }
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index b1d0c69f21b8..a063b9af7fff 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -108,6 +108,7 @@ 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 bool opt_print_tests;
 static enum test_mode opt_mode = TEST_MODE_ALL;
 
 static void __exit_with_error(int error, const char *file, const char *func, int line)
@@ -314,6 +315,7 @@ static struct option long_options[] = {
 	{"busy-poll", no_argument, 0, 'b'},
 	{"verbose", no_argument, 0, 'v'},
 	{"mode", required_argument, 0, 'm'},
+	{"list", no_argument, 0, 'l'},
 	{0, 0, 0, 0}
 };
 
@@ -325,7 +327,8 @@ static void usage(const char *prog)
 		"  -i, --interface      Use interface\n"
 		"  -v, --verbose        Verbose output\n"
 		"  -b, --busy-poll      Enable busy poll\n"
-		"  -m, --mode           Run only mode skb, drv, or zc\n";
+		"  -m, --mode           Run only mode skb, drv, or zc\n"
+		"  -l, --list           List all available tests\n";
 
 	ksft_print_msg(str, prog);
 }
@@ -347,7 +350,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "i:vbm:", long_options, &option_index);
+		c = getopt_long(argc, argv, "i:vbm:l", long_options, &option_index);
 		if (c == -1)
 			break;
 
@@ -391,6 +394,9 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 				ksft_exit_xfail();
 			}
 			break;
+		case 'l':
+			opt_print_tests = true;
+			break;
 		default:
 			usage(basename(argv[0]));
 			ksft_exit_xfail();
@@ -2310,6 +2316,15 @@ static const struct test_spec tests[] = {
 	{.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
 };
 
+static void print_tests(void)
+{
+	u32 i;
+
+	printf("Tests:\n");
+	for (i = 0; i < ARRAY_SIZE(tests); i++)
+		printf("%u: %s\n", i, tests[i].name);
+}
+
 int main(int argc, char **argv)
 {
 	struct pkt_stream *rx_pkt_stream_default;
@@ -2334,6 +2349,11 @@ int main(int argc, char **argv)
 
 	parse_command_line(ifobj_tx, ifobj_rx, argc, argv);
 
+	if (opt_print_tests) {
+		print_tests();
+		ksft_exit_xpass();
+	}
+
 	shared_netdev = (ifobj_tx->ifindex == ifobj_rx->ifindex);
 	ifobj_tx->shared_umem = shared_netdev;
 	ifobj_rx->shared_umem = shared_netdev;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 06/10] selftests/xsk: add option that lists all tests
  2023-08-09 12:43 ` [PATCH bpf-next 06/10] selftests/xsk: add option that lists all tests Magnus Karlsson
@ 2023-08-22 12:37   ` Maciej Fijalkowski
  2023-08-22 13:52     ` Magnus Karlsson
  0 siblings, 1 reply; 22+ messages in thread
From: Maciej Fijalkowski @ 2023-08-22 12:37 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, bpf, yhs, andrii,
	martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa

On Wed, Aug 09, 2023 at 02:43:39PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Add a command line option (-l) that lists all the tests. The number
> before the test will be used in the next commit for specifying a
> single test to run. Here is an example of the output:

I was thinking whether we should have a way of combining -l with -m, but I
believe there is only a single test currently that can not be run in ZC
mode (rx dropped) ?

> 
> Tests:
> 0: SEND_RECEIVE
> 1: SEND_RECEIVE_2K_FRAME
> 2: SEND_RECEIVE_SINGLE_PKT
> 3: POLL_RX
> 4: POLL_TX
> 5: POLL_RXQ_FULL
> 6: POLL_TXQ_FULL
> 7: SEND_RECEIVE_UNALIGNED
> :
> :
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/test_xsk.sh    | 11 +++++++++-
>  tools/testing/selftests/bpf/xsk_prereqs.sh | 10 +++++----
>  tools/testing/selftests/bpf/xskxceiver.c   | 24 ++++++++++++++++++++--
>  3 files changed, 38 insertions(+), 7 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> index 4ec621f4d3db..00a504f0929a 100755
> --- a/tools/testing/selftests/bpf/test_xsk.sh
> +++ b/tools/testing/selftests/bpf/test_xsk.sh
> @@ -81,13 +81,14 @@
>  
>  ETH=""
>  
> -while getopts "vi:dm:" flag
> +while getopts "vi:dm:l" flag
>  do
>  	case "${flag}" in
>  		v) verbose=1;;
>  		d) debug=1;;
>  		i) ETH=${OPTARG};;
>  		m) MODE=${OPTARG};;
> +		l) list=1;;
>  	esac
>  done
>  
> @@ -157,6 +158,10 @@ if [[ $verbose -eq 1 ]]; then
>  	ARGS+="-v "
>  fi
>  
> +if [[ $list -eq 1 ]]; then
> +	ARGS+="-l "
> +fi
> +
>  if [ ! -z $MODE ]; then
>  	ARGS+="-m ${MODE} "
>  fi
> @@ -183,6 +188,10 @@ else
>  	cleanup_iface ${ETH} ${MTU}
>  fi
>  
> +if [[ $list -eq 1 ]]; then
> +    exit
> +fi
> +
>  TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
>  busy_poll=1
>  
> diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
> index 29175682c44d..47c7b8064f38 100755
> --- a/tools/testing/selftests/bpf/xsk_prereqs.sh
> +++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
> @@ -83,9 +83,11 @@ exec_xskxceiver()
>  	fi
>  
>  	./${XSKOBJ} -i ${VETH0} -i ${VETH1} ${ARGS}
> -
>  	retval=$?
> -	test_status $retval "${TEST_NAME}"
> -	statusList+=($retval)
> -	nameList+=(${TEST_NAME})
> +
> +	if [[ $list -ne 1 ]]; then
> +	    test_status $retval "${TEST_NAME}"
> +	    statusList+=($retval)
> +	    nameList+=(${TEST_NAME})
> +	fi
>  }
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index b1d0c69f21b8..a063b9af7fff 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -108,6 +108,7 @@ 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 bool opt_print_tests;
>  static enum test_mode opt_mode = TEST_MODE_ALL;
>  
>  static void __exit_with_error(int error, const char *file, const char *func, int line)
> @@ -314,6 +315,7 @@ static struct option long_options[] = {
>  	{"busy-poll", no_argument, 0, 'b'},
>  	{"verbose", no_argument, 0, 'v'},
>  	{"mode", required_argument, 0, 'm'},
> +	{"list", no_argument, 0, 'l'},
>  	{0, 0, 0, 0}
>  };
>  
> @@ -325,7 +327,8 @@ static void usage(const char *prog)
>  		"  -i, --interface      Use interface\n"
>  		"  -v, --verbose        Verbose output\n"
>  		"  -b, --busy-poll      Enable busy poll\n"
> -		"  -m, --mode           Run only mode skb, drv, or zc\n";
> +		"  -m, --mode           Run only mode skb, drv, or zc\n"
> +		"  -l, --list           List all available tests\n";
>  
>  	ksft_print_msg(str, prog);
>  }
> @@ -347,7 +350,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  	opterr = 0;
>  
>  	for (;;) {
> -		c = getopt_long(argc, argv, "i:vbm:", long_options, &option_index);
> +		c = getopt_long(argc, argv, "i:vbm:l", long_options, &option_index);
>  		if (c == -1)
>  			break;
>  
> @@ -391,6 +394,9 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  				ksft_exit_xfail();
>  			}
>  			break;
> +		case 'l':
> +			opt_print_tests = true;
> +			break;
>  		default:
>  			usage(basename(argv[0]));
>  			ksft_exit_xfail();
> @@ -2310,6 +2316,15 @@ static const struct test_spec tests[] = {
>  	{.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
>  };
>  
> +static void print_tests(void)
> +{
> +	u32 i;
> +
> +	printf("Tests:\n");
> +	for (i = 0; i < ARRAY_SIZE(tests); i++)

Nit: I believe you can do
	for (u32 i = 0; i < ARRAY_SIZE(tests); i++)

> +		printf("%u: %s\n", i, tests[i].name);
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	struct pkt_stream *rx_pkt_stream_default;
> @@ -2334,6 +2349,11 @@ int main(int argc, char **argv)
>  
>  	parse_command_line(ifobj_tx, ifobj_rx, argc, argv);
>  
> +	if (opt_print_tests) {
> +		print_tests();
> +		ksft_exit_xpass();
> +	}
> +
>  	shared_netdev = (ifobj_tx->ifindex == ifobj_rx->ifindex);
>  	ifobj_tx->shared_umem = shared_netdev;
>  	ifobj_rx->shared_umem = shared_netdev;
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 06/10] selftests/xsk: add option that lists all tests
  2023-08-22 12:37   ` Maciej Fijalkowski
@ 2023-08-22 13:52     ` Magnus Karlsson
  0 siblings, 0 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-22 13:52 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, bpf, yhs, andrii,
	martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa

On Tue, 22 Aug 2023 at 14:37, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Wed, Aug 09, 2023 at 02:43:39PM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Add a command line option (-l) that lists all the tests. The number
> > before the test will be used in the next commit for specifying a
> > single test to run. Here is an example of the output:
>
> I was thinking whether we should have a way of combining -l with -m, but I
> believe there is only a single test currently that can not be run in ZC
> mode (rx dropped) ?
>
> >
> > Tests:
> > 0: SEND_RECEIVE
> > 1: SEND_RECEIVE_2K_FRAME
> > 2: SEND_RECEIVE_SINGLE_PKT
> > 3: POLL_RX
> > 4: POLL_TX
> > 5: POLL_RXQ_FULL
> > 6: POLL_TXQ_FULL
> > 7: SEND_RECEIVE_UNALIGNED
> > :
> > :
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >  tools/testing/selftests/bpf/test_xsk.sh    | 11 +++++++++-
> >  tools/testing/selftests/bpf/xsk_prereqs.sh | 10 +++++----
> >  tools/testing/selftests/bpf/xskxceiver.c   | 24 ++++++++++++++++++++--
> >  3 files changed, 38 insertions(+), 7 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > index 4ec621f4d3db..00a504f0929a 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -81,13 +81,14 @@
> >
> >  ETH=""
> >
> > -while getopts "vi:dm:" flag
> > +while getopts "vi:dm:l" flag
> >  do
> >       case "${flag}" in
> >               v) verbose=1;;
> >               d) debug=1;;
> >               i) ETH=${OPTARG};;
> >               m) MODE=${OPTARG};;
> > +             l) list=1;;
> >       esac
> >  done
> >
> > @@ -157,6 +158,10 @@ if [[ $verbose -eq 1 ]]; then
> >       ARGS+="-v "
> >  fi
> >
> > +if [[ $list -eq 1 ]]; then
> > +     ARGS+="-l "
> > +fi
> > +
> >  if [ ! -z $MODE ]; then
> >       ARGS+="-m ${MODE} "
> >  fi
> > @@ -183,6 +188,10 @@ else
> >       cleanup_iface ${ETH} ${MTU}
> >  fi
> >
> > +if [[ $list -eq 1 ]]; then
> > +    exit
> > +fi
> > +
> >  TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
> >  busy_poll=1
> >
> > diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
> > index 29175682c44d..47c7b8064f38 100755
> > --- a/tools/testing/selftests/bpf/xsk_prereqs.sh
> > +++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
> > @@ -83,9 +83,11 @@ exec_xskxceiver()
> >       fi
> >
> >       ./${XSKOBJ} -i ${VETH0} -i ${VETH1} ${ARGS}
> > -
> >       retval=$?
> > -     test_status $retval "${TEST_NAME}"
> > -     statusList+=($retval)
> > -     nameList+=(${TEST_NAME})
> > +
> > +     if [[ $list -ne 1 ]]; then
> > +         test_status $retval "${TEST_NAME}"
> > +         statusList+=($retval)
> > +         nameList+=(${TEST_NAME})
> > +     fi
> >  }
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index b1d0c69f21b8..a063b9af7fff 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -108,6 +108,7 @@ 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 bool opt_print_tests;
> >  static enum test_mode opt_mode = TEST_MODE_ALL;
> >
> >  static void __exit_with_error(int error, const char *file, const char *func, int line)
> > @@ -314,6 +315,7 @@ static struct option long_options[] = {
> >       {"busy-poll", no_argument, 0, 'b'},
> >       {"verbose", no_argument, 0, 'v'},
> >       {"mode", required_argument, 0, 'm'},
> > +     {"list", no_argument, 0, 'l'},
> >       {0, 0, 0, 0}
> >  };
> >
> > @@ -325,7 +327,8 @@ static void usage(const char *prog)
> >               "  -i, --interface      Use interface\n"
> >               "  -v, --verbose        Verbose output\n"
> >               "  -b, --busy-poll      Enable busy poll\n"
> > -             "  -m, --mode           Run only mode skb, drv, or zc\n";
> > +             "  -m, --mode           Run only mode skb, drv, or zc\n"
> > +             "  -l, --list           List all available tests\n";
> >
> >       ksft_print_msg(str, prog);
> >  }
> > @@ -347,7 +350,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >       opterr = 0;
> >
> >       for (;;) {
> > -             c = getopt_long(argc, argv, "i:vbm:", long_options, &option_index);
> > +             c = getopt_long(argc, argv, "i:vbm:l", long_options, &option_index);
> >               if (c == -1)
> >                       break;
> >
> > @@ -391,6 +394,9 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >                               ksft_exit_xfail();
> >                       }
> >                       break;
> > +             case 'l':
> > +                     opt_print_tests = true;
> > +                     break;
> >               default:
> >                       usage(basename(argv[0]));
> >                       ksft_exit_xfail();
> > @@ -2310,6 +2316,15 @@ static const struct test_spec tests[] = {
> >       {.name = "TOO_MANY_FRAGS", .test_func = testapp_too_many_frags},
> >  };
> >
> > +static void print_tests(void)
> > +{
> > +     u32 i;
> > +
> > +     printf("Tests:\n");
> > +     for (i = 0; i < ARRAY_SIZE(tests); i++)
>
> Nit: I believe you can do
>         for (u32 i = 0; i < ARRAY_SIZE(tests); i++)

Thank you for reviewing this Maciej. Will fix all your comments in all
the patches except this one. There are none of these embedded
declarations previously in the file, so let us just stick to declaring
variables right after "{", for consistency.

> > +             printf("%u: %s\n", i, tests[i].name);
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> >       struct pkt_stream *rx_pkt_stream_default;
> > @@ -2334,6 +2349,11 @@ int main(int argc, char **argv)
> >
> >       parse_command_line(ifobj_tx, ifobj_rx, argc, argv);
> >
> > +     if (opt_print_tests) {
> > +             print_tests();
> > +             ksft_exit_xpass();
> > +     }
> > +
> >       shared_netdev = (ifobj_tx->ifindex == ifobj_rx->ifindex);
> >       ifobj_tx->shared_umem = shared_netdev;
> >       ifobj_rx->shared_umem = shared_netdev;
> > --
> > 2.34.1
> >

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH bpf-next 07/10] selftests/xsk: add option to run single test
  2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
                   ` (5 preceding siblings ...)
  2023-08-09 12:43 ` [PATCH bpf-next 06/10] selftests/xsk: add option that lists all tests Magnus Karlsson
@ 2023-08-09 12:43 ` Magnus Karlsson
  2023-08-22 12:50   ` Maciej Fijalkowski
  2023-08-09 12:43 ` [PATCH bpf-next 08/10] selftests/xsk: use ksft_print_msg uniformly Magnus Karlsson
                   ` (2 subsequent siblings)
  9 siblings, 1 reply; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa

From: Magnus Karlsson <magnus.karlsson@intel.com>

Add a command line option to be able to run a single test. This option
(-t) takes a number from the list of tests available with the "-l"
option. Here are two examples:

Run test number 2, the "receive single packet" test in all available modes:

./test_xsk.sh -t 2

Run test number 21, the metadata copy test in zero-copy mode only

./test_xsh.sh -t 21 -m zc

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh  | 10 ++++++-
 tools/testing/selftests/bpf/xskxceiver.c | 38 +++++++++++++++++++-----
 tools/testing/selftests/bpf/xskxceiver.h |  3 ++
 3 files changed, 42 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 00a504f0929a..94b4b86d5239 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -76,12 +76,15 @@
 #
 # Run test suite in a specific mode only [skb,drv,zc]
 #   sudo ./test_xsk.sh -m MODE
+#
+# Run a specific test from the test suite
+#   sudo ./test_xsk.sh -t TEST_NAME
 
 . xsk_prereqs.sh
 
 ETH=""
 
-while getopts "vi:dm:l" flag
+while getopts "vi:dm:lt:" flag
 do
 	case "${flag}" in
 		v) verbose=1;;
@@ -89,6 +92,7 @@ do
 		i) ETH=${OPTARG};;
 		m) MODE=${OPTARG};;
 		l) list=1;;
+		t) TEST=${OPTARG};;
 	esac
 done
 
@@ -166,6 +170,10 @@ if [ ! -z $MODE ]; then
 	ARGS+="-m ${MODE} "
 fi
 
+if [ ! -z $TEST ]; then
+	ARGS+="-t ${TEST} "
+fi
+
 retval=$?
 test_status $retval "${TEST_NAME}"
 
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index a063b9af7fff..38ec66292e03 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -110,6 +110,7 @@ static const char *MAC2 = "\x00\x0A\x56\x9E\xEE\x61";
 static bool opt_verbose;
 static bool opt_print_tests;
 static enum test_mode opt_mode = TEST_MODE_ALL;
+static u32 opt_run_test = RUN_ALL_TESTS;
 
 static void __exit_with_error(int error, const char *file, const char *func, int line)
 {
@@ -316,6 +317,7 @@ static struct option long_options[] = {
 	{"verbose", no_argument, 0, 'v'},
 	{"mode", required_argument, 0, 'm'},
 	{"list", no_argument, 0, 'l'},
+	{"test", required_argument, 0, 'y'},
 	{0, 0, 0, 0}
 };
 
@@ -328,7 +330,8 @@ static void usage(const char *prog)
 		"  -v, --verbose        Verbose output\n"
 		"  -b, --busy-poll      Enable busy poll\n"
 		"  -m, --mode           Run only mode skb, drv, or zc\n"
-		"  -l, --list           List all available tests\n";
+		"  -l, --list           List all available tests\n"
+		"  -t, --test           Run a specific test. Enter number from -l option\n";
 
 	ksft_print_msg(str, prog);
 }
@@ -350,7 +353,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "i:vbm:l", long_options, &option_index);
+		c = getopt_long(argc, argv, "i:vbm:lt:", long_options, &option_index);
 		if (c == -1)
 			break;
 
@@ -397,6 +400,9 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 		case 'l':
 			opt_print_tests = true;
 			break;
+		case 't':
+			opt_run_test = atol(optarg);
+			break;
 		default:
 			usage(basename(argv[0]));
 			ksft_exit_xfail();
@@ -2330,8 +2336,8 @@ int main(int argc, char **argv)
 	struct pkt_stream *rx_pkt_stream_default;
 	struct pkt_stream *tx_pkt_stream_default;
 	struct ifobject *ifobj_tx, *ifobj_rx;
+	u32 i, j, failed_tests = 0, nb_tests;
 	int modes = TEST_MODE_SKB + 1;
-	u32 i, j, failed_tests = 0;
 	struct test_spec test;
 	bool shared_netdev;
 
@@ -2353,6 +2359,10 @@ int main(int argc, char **argv)
 		print_tests();
 		ksft_exit_xpass();
 	}
+	if (opt_run_test != RUN_ALL_TESTS && opt_run_test >= ARRAY_SIZE(tests)) {
+		ksft_print_msg("Error: test %u does not exist.\n", opt_run_test);
+		ksft_exit_xfail();
+	}
 
 	shared_netdev = (ifobj_tx->ifindex == ifobj_rx->ifindex);
 	ifobj_tx->shared_umem = shared_netdev;
@@ -2380,19 +2390,31 @@ int main(int argc, char **argv)
 	test.tx_pkt_stream_default = tx_pkt_stream_default;
 	test.rx_pkt_stream_default = rx_pkt_stream_default;
 
+	if (opt_run_test == RUN_ALL_TESTS)
+		nb_tests = ARRAY_SIZE(tests);
+	else
+		nb_tests = 1;
 	if (opt_mode == TEST_MODE_ALL)
-		ksft_set_plan(modes * ARRAY_SIZE(tests));
+		ksft_set_plan(modes * nb_tests);
 	else
-		ksft_set_plan(ARRAY_SIZE(tests));
+		ksft_set_plan(nb_tests);
 
 	for (i = 0; i < modes; i++) {
 		if (opt_mode != TEST_MODE_ALL && i != opt_mode)
 			continue;
 
-		for (j = 0; j < ARRAY_SIZE(tests); j++) {
-			test_spec_init(&test, ifobj_tx, ifobj_rx, i, &tests[j]);
+		if (opt_run_test == RUN_ALL_TESTS) {
+			for (j = 0; j < ARRAY_SIZE(tests); j++) {
+				test_spec_init(&test, ifobj_tx, ifobj_rx, i, &tests[j]);
+				run_pkt_test(&test);
+				usleep(USLEEP_MAX);
+
+				if (test.fail)
+					failed_tests++;
+			}
+		} else {
+			test_spec_init(&test, ifobj_tx, ifobj_rx, i, &tests[opt_run_test]);
 			run_pkt_test(&test);
-			usleep(USLEEP_MAX);
 
 			if (test.fail)
 				failed_tests++;
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index 3a71d490db3e..8015aeea839d 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -5,6 +5,8 @@
 #ifndef XSKXCEIVER_H_
 #define XSKXCEIVER_H_
 
+#include <limits.h>
+
 #include "xsk_xdp_progs.skel.h"
 
 #ifndef SOL_XDP
@@ -56,6 +58,7 @@
 #define XSK_DESC__MAX_SKB_FRAGS 18
 #define HUGEPAGE_SIZE (2 * 1024 * 1024)
 #define PKT_DUMP_NB_TO_PRINT 16
+#define RUN_ALL_TESTS UINT_MAX
 
 #define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); } while (0)
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 07/10] selftests/xsk: add option to run single test
  2023-08-09 12:43 ` [PATCH bpf-next 07/10] selftests/xsk: add option to run single test Magnus Karlsson
@ 2023-08-22 12:50   ` Maciej Fijalkowski
  0 siblings, 0 replies; 22+ messages in thread
From: Maciej Fijalkowski @ 2023-08-22 12:50 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, bpf, yhs, andrii,
	martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa

On Wed, Aug 09, 2023 at 02:43:40PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Add a command line option to be able to run a single test. This option
> (-t) takes a number from the list of tests available with the "-l"
> option. Here are two examples:
> 
> Run test number 2, the "receive single packet" test in all available modes:
> 
> ./test_xsk.sh -t 2
> 
> Run test number 21, the metadata copy test in zero-copy mode only
> 
> ./test_xsh.sh -t 21 -m zc

for above you have to provide -i $IFACE as well

> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/test_xsk.sh  | 10 ++++++-
>  tools/testing/selftests/bpf/xskxceiver.c | 38 +++++++++++++++++++-----
>  tools/testing/selftests/bpf/xskxceiver.h |  3 ++
>  3 files changed, 42 insertions(+), 9 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> index 00a504f0929a..94b4b86d5239 100755
> --- a/tools/testing/selftests/bpf/test_xsk.sh
> +++ b/tools/testing/selftests/bpf/test_xsk.sh
> @@ -76,12 +76,15 @@
>  #
>  # Run test suite in a specific mode only [skb,drv,zc]
>  #   sudo ./test_xsk.sh -m MODE
> +#
> +# Run a specific test from the test suite
> +#   sudo ./test_xsk.sh -t TEST_NAME
>  
>  . xsk_prereqs.sh
>  
>  ETH=""
>  
> -while getopts "vi:dm:l" flag
> +while getopts "vi:dm:lt:" flag
>  do
>  	case "${flag}" in
>  		v) verbose=1;;
> @@ -89,6 +92,7 @@ do
>  		i) ETH=${OPTARG};;
>  		m) MODE=${OPTARG};;
>  		l) list=1;;
> +		t) TEST=${OPTARG};;
>  	esac
>  done
>  
> @@ -166,6 +170,10 @@ if [ ! -z $MODE ]; then
>  	ARGS+="-m ${MODE} "
>  fi
>  
> +if [ ! -z $TEST ]; then
> +	ARGS+="-t ${TEST} "
> +fi
> +
>  retval=$?
>  test_status $retval "${TEST_NAME}"
>  
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index a063b9af7fff..38ec66292e03 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -110,6 +110,7 @@ static const char *MAC2 = "\x00\x0A\x56\x9E\xEE\x61";
>  static bool opt_verbose;
>  static bool opt_print_tests;
>  static enum test_mode opt_mode = TEST_MODE_ALL;
> +static u32 opt_run_test = RUN_ALL_TESTS;
>  
>  static void __exit_with_error(int error, const char *file, const char *func, int line)
>  {
> @@ -316,6 +317,7 @@ static struct option long_options[] = {
>  	{"verbose", no_argument, 0, 'v'},
>  	{"mode", required_argument, 0, 'm'},
>  	{"list", no_argument, 0, 'l'},
> +	{"test", required_argument, 0, 'y'},
>  	{0, 0, 0, 0}
>  };
>  
> @@ -328,7 +330,8 @@ static void usage(const char *prog)
>  		"  -v, --verbose        Verbose output\n"
>  		"  -b, --busy-poll      Enable busy poll\n"
>  		"  -m, --mode           Run only mode skb, drv, or zc\n"
> -		"  -l, --list           List all available tests\n";
> +		"  -l, --list           List all available tests\n"
> +		"  -t, --test           Run a specific test. Enter number from -l option\n";
>  
>  	ksft_print_msg(str, prog);
>  }
> @@ -350,7 +353,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  	opterr = 0;
>  
>  	for (;;) {
> -		c = getopt_long(argc, argv, "i:vbm:l", long_options, &option_index);
> +		c = getopt_long(argc, argv, "i:vbm:lt:", long_options, &option_index);
>  		if (c == -1)
>  			break;
>  
> @@ -397,6 +400,9 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  		case 'l':
>  			opt_print_tests = true;
>  			break;
> +		case 't':
> +			opt_run_test = atol(optarg);

how are you protecting against something like:

sudo ./test_xsk.sh -t asdasd -m zc -i enp24s0f1np1

?

> +			break;
>  		default:
>  			usage(basename(argv[0]));
>  			ksft_exit_xfail();
> @@ -2330,8 +2336,8 @@ int main(int argc, char **argv)
>  	struct pkt_stream *rx_pkt_stream_default;
>  	struct pkt_stream *tx_pkt_stream_default;
>  	struct ifobject *ifobj_tx, *ifobj_rx;
> +	u32 i, j, failed_tests = 0, nb_tests;
>  	int modes = TEST_MODE_SKB + 1;
> -	u32 i, j, failed_tests = 0;
>  	struct test_spec test;
>  	bool shared_netdev;
>  
> @@ -2353,6 +2359,10 @@ int main(int argc, char **argv)
>  		print_tests();
>  		ksft_exit_xpass();
>  	}
> +	if (opt_run_test != RUN_ALL_TESTS && opt_run_test >= ARRAY_SIZE(tests)) {
> +		ksft_print_msg("Error: test %u does not exist.\n", opt_run_test);
> +		ksft_exit_xfail();
> +	}
>  
>  	shared_netdev = (ifobj_tx->ifindex == ifobj_rx->ifindex);
>  	ifobj_tx->shared_umem = shared_netdev;
> @@ -2380,19 +2390,31 @@ int main(int argc, char **argv)
>  	test.tx_pkt_stream_default = tx_pkt_stream_default;
>  	test.rx_pkt_stream_default = rx_pkt_stream_default;
>  
> +	if (opt_run_test == RUN_ALL_TESTS)
> +		nb_tests = ARRAY_SIZE(tests);
> +	else
> +		nb_tests = 1;
>  	if (opt_mode == TEST_MODE_ALL)
> -		ksft_set_plan(modes * ARRAY_SIZE(tests));
> +		ksft_set_plan(modes * nb_tests);
>  	else
> -		ksft_set_plan(ARRAY_SIZE(tests));
> +		ksft_set_plan(nb_tests);
>  
>  	for (i = 0; i < modes; i++) {
>  		if (opt_mode != TEST_MODE_ALL && i != opt_mode)
>  			continue;
>  
> -		for (j = 0; j < ARRAY_SIZE(tests); j++) {
> -			test_spec_init(&test, ifobj_tx, ifobj_rx, i, &tests[j]);
> +		if (opt_run_test == RUN_ALL_TESTS) {
> +			for (j = 0; j < ARRAY_SIZE(tests); j++) {
> +				test_spec_init(&test, ifobj_tx, ifobj_rx, i, &tests[j]);
> +				run_pkt_test(&test);
> +				usleep(USLEEP_MAX);
> +
> +				if (test.fail)
> +					failed_tests++;
> +			}
> +		} else {
> +			test_spec_init(&test, ifobj_tx, ifobj_rx, i, &tests[opt_run_test]);
>  			run_pkt_test(&test);
> -			usleep(USLEEP_MAX);
>  
>  			if (test.fail)
>  				failed_tests++;
> diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> index 3a71d490db3e..8015aeea839d 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.h
> +++ b/tools/testing/selftests/bpf/xskxceiver.h
> @@ -5,6 +5,8 @@
>  #ifndef XSKXCEIVER_H_
>  #define XSKXCEIVER_H_
>  
> +#include <limits.h>
> +
>  #include "xsk_xdp_progs.skel.h"
>  
>  #ifndef SOL_XDP
> @@ -56,6 +58,7 @@
>  #define XSK_DESC__MAX_SKB_FRAGS 18
>  #define HUGEPAGE_SIZE (2 * 1024 * 1024)
>  #define PKT_DUMP_NB_TO_PRINT 16
> +#define RUN_ALL_TESTS UINT_MAX
>  
>  #define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); } while (0)
>  
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

* [PATCH bpf-next 08/10] selftests/xsk: use ksft_print_msg uniformly
  2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
                   ` (6 preceding siblings ...)
  2023-08-09 12:43 ` [PATCH bpf-next 07/10] selftests/xsk: add option to run single test Magnus Karlsson
@ 2023-08-09 12:43 ` Magnus Karlsson
  2023-08-09 12:43 ` [PATCH bpf-next 09/10] selftests/xsk: fail single test instead of all tests Magnus Karlsson
  2023-08-09 12:43 ` [PATCH bpf-next 10/10] selftests/xsk: display command line options with -h Magnus Karlsson
  9 siblings, 0 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa

From: Magnus Karlsson <magnus.karlsson@intel.com>

Use ksft_print_msg() instead of printf() and fprintf() in all places
as the ksefltests framework is being used. There is only one exception
and that is for the list-of-tests print out option, since no tests are
run in that case.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 25 ++++++++++++------------
 1 file changed, 13 insertions(+), 12 deletions(-)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 38ec66292e03..7c19aef1dcc9 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -810,7 +810,7 @@ static void pkt_print_data(u32 *data, u32 cnt)
 
 		seqnum = ntohl(*data) & 0xffff;
 		pkt_nb = ntohl(*data) >> 16;
-		fprintf(stdout, "%u:%u ", pkt_nb, seqnum);
+		ksft_print_msg("%u:%u ", pkt_nb, seqnum);
 		data++;
 	}
 }
@@ -822,13 +822,13 @@ static void pkt_dump(void *pkt, u32 len, bool eth_header)
 
 	if (eth_header) {
 		/*extract L2 frame */
-		fprintf(stdout, "DEBUG>> L2: dst mac: ");
+		ksft_print_msg("DEBUG>> L2: dst mac: ");
 		for (i = 0; i < ETH_ALEN; i++)
-			fprintf(stdout, "%02X", ethhdr->h_dest[i]);
+			ksft_print_msg("%02X", ethhdr->h_dest[i]);
 
-		fprintf(stdout, "\nDEBUG>> L2: src mac: ");
+		ksft_print_msg("\nDEBUG>> L2: src mac: ");
 		for (i = 0; i < ETH_ALEN; i++)
-			fprintf(stdout, "%02X", ethhdr->h_source[i]);
+			ksft_print_msg("%02X", ethhdr->h_source[i]);
 
 		data = pkt + PKT_HDR_SIZE;
 	} else {
@@ -836,15 +836,15 @@ static void pkt_dump(void *pkt, u32 len, bool eth_header)
 	}
 
 	/*extract L5 frame */
-	fprintf(stdout, "\nDEBUG>> L5: seqnum: ");
+	ksft_print_msg("\nDEBUG>> L5: seqnum: ");
 	pkt_print_data(data, PKT_DUMP_NB_TO_PRINT);
-	fprintf(stdout, "....");
+	ksft_print_msg("....");
 	if (len > PKT_DUMP_NB_TO_PRINT * sizeof(u32)) {
-		fprintf(stdout, "\n.... ");
+		ksft_print_msg("\n.... ");
 		pkt_print_data(data + len / sizeof(u32) - PKT_DUMP_NB_TO_PRINT,
 			       PKT_DUMP_NB_TO_PRINT);
 	}
-	fprintf(stdout, "\n---------------------------------------\n");
+	ksft_print_msg("\n---------------------------------------\n");
 }
 
 static bool is_offset_correct(struct xsk_umem_info *umem, struct pkt *pkt, u64 addr)
@@ -1555,7 +1555,8 @@ static void *worker_testapp_validate_rx(void *arg)
 		xsk_clear_xskmap(ifobject->xskmap);
 		err = xsk_update_xskmap(ifobject->xskmap, ifobject->xsk->xsk);
 		if (err) {
-			printf("Error: Failed to update xskmap, error %s\n", strerror(-err));
+			ksft_print_msg("Error: Failed to update xskmap, error %s\n",
+				       strerror(-err));
 			exit_with_error(-err);
 		}
 	}
@@ -1619,7 +1620,7 @@ static void xsk_reattach_xdp(struct ifobject *ifobj, struct bpf_program *xdp_pro
 	xsk_detach_xdp_program(ifobj->ifindex, mode_to_xdp_flags(ifobj->mode));
 	err = xsk_attach_xdp_program(xdp_prog, ifobj->ifindex, mode_to_xdp_flags(mode));
 	if (err) {
-		printf("Error attaching XDP program\n");
+		ksft_print_msg("Error attaching XDP program\n");
 		exit_with_error(-err);
 	}
 
@@ -2106,7 +2107,7 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
 
 	err = xsk_load_xdp_programs(ifobj);
 	if (err) {
-		printf("Error loading XDP program\n");
+		ksft_print_msg("Error loading XDP program\n");
 		exit_with_error(err);
 	}
 
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH bpf-next 09/10] selftests/xsk: fail single test instead of all tests
  2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
                   ` (7 preceding siblings ...)
  2023-08-09 12:43 ` [PATCH bpf-next 08/10] selftests/xsk: use ksft_print_msg uniformly Magnus Karlsson
@ 2023-08-09 12:43 ` Magnus Karlsson
  2023-08-09 12:43 ` [PATCH bpf-next 10/10] selftests/xsk: display command line options with -h Magnus Karlsson
  9 siblings, 0 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa

From: Magnus Karlsson <magnus.karlsson@intel.com>

In a number of places at en error, exit_with_error() is called that
terminates the whole test suite. This is not always desirable as it
would be more logical to only fail that test and then go along with
the other ones. So change this in a number of places in which I though
it would be more logical to just fail the test in question. Examples
of this are in code that is only used by a single test.

Also delete a pointless if-statement in receive_pkts() that has an
exit_with_error() in it. It can never occur since the return value is
an unisnged and the test is for less than zero.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xskxceiver.c | 70 ++++++++++++++++--------
 1 file changed, 46 insertions(+), 24 deletions(-)

diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 7c19aef1dcc9..ffa8905a66f1 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -949,36 +949,42 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len)
 	return true;
 }
 
-static void kick_tx(struct xsk_socket_info *xsk)
+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 (ret >= 0)
-		return;
+		return TEST_PASS;
 	if (errno == ENOBUFS || errno == EAGAIN || errno == EBUSY || errno == ENETDOWN) {
 		usleep(100);
-		return;
+		return TEST_PASS;
 	}
-	exit_with_error(errno);
+	return TEST_FAILURE;
 }
 
-static void kick_rx(struct xsk_socket_info *xsk)
+static int kick_rx(struct xsk_socket_info *xsk)
 {
 	int ret;
 
 	ret = recvfrom(xsk_socket__fd(xsk->xsk), NULL, 0, MSG_DONTWAIT, NULL, NULL);
 	if (ret < 0)
-		exit_with_error(errno);
+		return TEST_FAILURE;
+
+	return TEST_PASS;
 }
 
 static int complete_pkts(struct xsk_socket_info *xsk, int batch_size)
 {
 	unsigned int rcvd;
 	u32 idx;
+	int ret;
 
-	if (xsk_ring_prod__needs_wakeup(&xsk->tx))
-		kick_tx(xsk);
+	if (xsk_ring_prod__needs_wakeup(&xsk->tx)) {
+		ret = kick_tx(xsk);
+		if (ret)
+			return TEST_FAILURE;
+	}
 
 	rcvd = xsk_ring_cons__peek(&xsk->umem->cq, batch_size, &idx);
 	if (rcvd) {
@@ -1026,11 +1032,14 @@ static int receive_pkts(struct test_spec *test, struct pollfd *fds)
 			return TEST_FAILURE;
 		}
 
-		kick_rx(xsk);
+		ret = kick_rx(xsk);
+		if (ret)
+			return TEST_FAILURE;
+
 		if (ifobj->use_poll) {
 			ret = poll(fds, 1, POLL_TMOUT);
 			if (ret < 0)
-				exit_with_error(errno);
+				return TEST_FAILURE;
 
 			if (!ret) {
 				if (!is_umem_valid(test->ifobj_tx))
@@ -1051,12 +1060,10 @@ static int receive_pkts(struct test_spec *test, struct pollfd *fds)
 		if (ifobj->use_fill_ring) {
 			ret = xsk_ring_prod__reserve(&umem->fq, rcvd, &idx_fq);
 			while (ret != rcvd) {
-				if (ret < 0)
-					exit_with_error(-ret);
 				if (xsk_ring_prod__needs_wakeup(&umem->fq)) {
 					ret = poll(fds, 1, POLL_TMOUT);
 					if (ret < 0)
-						exit_with_error(errno);
+						return TEST_FAILURE;
 				}
 				ret = xsk_ring_prod__reserve(&umem->fq, rcvd, &idx_fq);
 			}
@@ -1140,7 +1147,9 @@ static int __send_pkts(struct ifobject *ifobject, struct pollfd *fds, bool timeo
 	buffer_len = pkt_get_buffer_len(umem, pkt_stream->max_pkt_len);
 	/* pkts_in_flight might be negative if many invalid packets are sent */
 	if (pkts_in_flight >= (int)((umem_size(umem) - BATCH_SIZE * buffer_len) / buffer_len)) {
-		kick_tx(xsk);
+		ret = kick_tx(xsk);
+		if (ret)
+			return TEST_FAILURE;
 		return TEST_CONTINUE;
 	}
 
@@ -1323,7 +1332,9 @@ static int validate_rx_dropped(struct ifobject *ifobject)
 	struct xdp_statistics stats;
 	int err;
 
-	kick_rx(ifobject->xsk);
+	err = kick_rx(ifobject->xsk);
+	if (err)
+		return TEST_FAILURE;
 
 	err = get_xsk_stats(xsk, &stats);
 	if (err)
@@ -1349,7 +1360,9 @@ static int validate_rx_full(struct ifobject *ifobject)
 	int err;
 
 	usleep(1000);
-	kick_rx(ifobject->xsk);
+	err = kick_rx(ifobject->xsk);
+	if (err)
+		return TEST_FAILURE;
 
 	err = get_xsk_stats(xsk, &stats);
 	if (err)
@@ -1368,7 +1381,9 @@ static int validate_fill_empty(struct ifobject *ifobject)
 	int err;
 
 	usleep(1000);
-	kick_rx(ifobject->xsk);
+	err = kick_rx(ifobject->xsk);
+	if (err)
+		return TEST_FAILURE;
 
 	err = get_xsk_stats(xsk, &stats);
 	if (err)
@@ -1777,7 +1792,7 @@ static int testapp_bidirectional(struct test_spec *test)
 	return res;
 }
 
-static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj_rx)
+static int swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj_rx)
 {
 	int ret;
 
@@ -1788,7 +1803,9 @@ static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj
 
 	ret = xsk_update_xskmap(ifobj_rx->xskmap, ifobj_rx->xsk->xsk);
 	if (ret)
-		exit_with_error(errno);
+		return TEST_FAILURE;
+
+	return TEST_PASS;
 }
 
 static int testapp_xdp_prog_cleanup(struct test_spec *test)
@@ -1798,7 +1815,8 @@ static int testapp_xdp_prog_cleanup(struct test_spec *test)
 	if (testapp_validate_traffic(test))
 		return TEST_FAILURE;
 
-	swap_xsk_resources(test->ifobj_tx, test->ifobj_rx);
+	if (swap_xsk_resources(test->ifobj_tx, test->ifobj_rx))
+		return TEST_FAILURE;
 	return testapp_validate_traffic(test);
 }
 
@@ -1999,11 +2017,15 @@ static int testapp_xdp_metadata_copy(struct test_spec *test)
 	test->ifobj_rx->use_metadata = true;
 
 	data_map = bpf_object__find_map_by_name(skel_rx->obj, "xsk_xdp_.bss");
-	if (!data_map || !bpf_map__is_internal(data_map))
-		exit_with_error(ENOMEM);
+	if (!data_map || !bpf_map__is_internal(data_map)) {
+		ksft_print_msg("Error: could not find bss section of XDP program\n");
+		return TEST_FAILURE;
+	}
 
-	if (bpf_map_update_elem(bpf_map__fd(data_map), &key, &count, BPF_ANY))
-		exit_with_error(errno);
+	if (bpf_map_update_elem(bpf_map__fd(data_map), &key, &count, BPF_ANY)) {
+		ksft_print_msg("Error: could not update count element\n");
+		return TEST_FAILURE;
+	}
 
 	return testapp_validate_traffic(test);
 }
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* [PATCH bpf-next 10/10] selftests/xsk: display command line options with -h
  2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
                   ` (8 preceding siblings ...)
  2023-08-09 12:43 ` [PATCH bpf-next 09/10] selftests/xsk: fail single test instead of all tests Magnus Karlsson
@ 2023-08-09 12:43 ` Magnus Karlsson
  2023-08-10 12:19   ` Przemek Kitszel
  2023-08-22 13:02   ` Maciej Fijalkowski
  9 siblings, 2 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-09 12:43 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
	haoluo, jolsa

From: Magnus Karlsson <magnus.karlsson@intel.com>

Add the -h option to display all available command line options
available for test_xsk.sh and xskxceiver.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 94b4b86d5239..baaeb016d699 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -79,12 +79,15 @@
 #
 # Run a specific test from the test suite
 #   sudo ./test_xsk.sh -t TEST_NAME
+#
+# Display the available command line options
+#   sudo ./test_xsk.sh -h
 
 . xsk_prereqs.sh
 
 ETH=""
 
-while getopts "vi:dm:lt:" flag
+while getopts "vi:dm:lt:h" flag
 do
 	case "${flag}" in
 		v) verbose=1;;
@@ -93,6 +96,7 @@ do
 		m) MODE=${OPTARG};;
 		l) list=1;;
 		t) TEST=${OPTARG};;
+		h) help=1;;
 	esac
 done
 
@@ -140,6 +144,11 @@ setup_vethPairs() {
 	ip link set ${VETH0} up
 }
 
+if [[ $help -eq 1 ]]; then
+	./${XSKOBJ}
+        exit
+fi
+
 if [ ! -z $ETH ]; then
 	VETH0=${ETH}
 	VETH1=${ETH}
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 10/10] selftests/xsk: display command line options with -h
  2023-08-09 12:43 ` [PATCH bpf-next 10/10] selftests/xsk: display command line options with -h Magnus Karlsson
@ 2023-08-10 12:19   ` Przemek Kitszel
  2023-08-10 12:41     ` Magnus Karlsson
  2023-08-22 13:02   ` Maciej Fijalkowski
  1 sibling, 1 reply; 22+ messages in thread
From: Przemek Kitszel @ 2023-08-10 12:19 UTC (permalink / raw)
  To: Magnus Karlsson, Karlsson, Magnus, bjorn@kernel.org,
	ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
	Fijalkowski, Maciej, bpf@vger.kernel.org, yhs@fb.com,
	andrii@kernel.org, martin.lau@linux.dev, song@kernel.org,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@google.com,
	haoluo@google.com, jolsa@kernel.org

On 8/9/23 14:43, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Add the -h option to display all available command line options
> available for test_xsk.sh and xskxceiver.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>   tools/testing/selftests/bpf/test_xsk.sh | 11 ++++++++++-
>   1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> index 94b4b86d5239..baaeb016d699 100755
> --- a/tools/testing/selftests/bpf/test_xsk.sh
> +++ b/tools/testing/selftests/bpf/test_xsk.sh
> @@ -79,12 +79,15 @@
>   #
>   # Run a specific test from the test suite
>   #   sudo ./test_xsk.sh -t TEST_NAME
> +#
> +# Display the available command line options
> +#   sudo ./test_xsk.sh -h

any "help" / "list" commands (that do nothing but print) should be (able 
ot) execute/d without `sudo`.
Removing `sudo` part from the doc here would make it clear to reader too.

>   
>   . xsk_prereqs.sh
>   
>   ETH=""
>   
> -while getopts "vi:dm:lt:" flag
> +while getopts "vi:dm:lt:h" flag
>   do
>   	case "${flag}" in
>   		v) verbose=1;;
> @@ -93,6 +96,7 @@ do
>   		m) MODE=${OPTARG};;
>   		l) list=1;;
>   		t) TEST=${OPTARG};;
> +		h) help=1;;
>   	esac
>   done
>   
> @@ -140,6 +144,11 @@ setup_vethPairs() {
>   	ip link set ${VETH0} up
>   }
>   
> +if [[ $help -eq 1 ]]; then
> +	./${XSKOBJ}
> +        exit
> +fi
> +
>   if [ ! -z $ETH ]; then
>   	VETH0=${ETH}
>   	VETH1=${ETH}


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 10/10] selftests/xsk: display command line options with -h
  2023-08-10 12:19   ` Przemek Kitszel
@ 2023-08-10 12:41     ` Magnus Karlsson
  0 siblings, 0 replies; 22+ messages in thread
From: Magnus Karlsson @ 2023-08-10 12:41 UTC (permalink / raw)
  To: Przemek Kitszel
  Cc: Karlsson, Magnus, bjorn@kernel.org, ast@kernel.org,
	daniel@iogearbox.net, netdev@vger.kernel.org, Fijalkowski, Maciej,
	bpf@vger.kernel.org, yhs@fb.com, andrii@kernel.org,
	martin.lau@linux.dev, song@kernel.org, john.fastabend@gmail.com,
	kpsingh@kernel.org, sdf@google.com, haoluo@google.com,
	jolsa@kernel.org

On Thu, 10 Aug 2023 at 14:19, Przemek Kitszel
<przemyslaw.kitszel@intel.com> wrote:
>
> On 8/9/23 14:43, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Add the -h option to display all available command line options
> > available for test_xsk.sh and xskxceiver.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >   tools/testing/selftests/bpf/test_xsk.sh | 11 ++++++++++-
> >   1 file changed, 10 insertions(+), 1 deletion(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > index 94b4b86d5239..baaeb016d699 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -79,12 +79,15 @@
> >   #
> >   # Run a specific test from the test suite
> >   #   sudo ./test_xsk.sh -t TEST_NAME
> > +#
> > +# Display the available command line options
> > +#   sudo ./test_xsk.sh -h
>
> any "help" / "list" commands (that do nothing but print) should be (able
> ot) execute/d without `sudo`.
> Removing `sudo` part from the doc here would make it clear to reader too.

You are correct. Will remove the "sudo" here as it is not needed.
Would be nice if we could execute the list option (-l) without sudo
rights too since it does not involve creating any xsk sockets. Will
fix that in patch 6.

> >
> >   . xsk_prereqs.sh
> >
> >   ETH=""
> >
> > -while getopts "vi:dm:lt:" flag
> > +while getopts "vi:dm:lt:h" flag
> >   do
> >       case "${flag}" in
> >               v) verbose=1;;
> > @@ -93,6 +96,7 @@ do
> >               m) MODE=${OPTARG};;
> >               l) list=1;;
> >               t) TEST=${OPTARG};;
> > +             h) help=1;;
> >       esac
> >   done
> >
> > @@ -140,6 +144,11 @@ setup_vethPairs() {
> >       ip link set ${VETH0} up
> >   }
> >
> > +if [[ $help -eq 1 ]]; then
> > +     ./${XSKOBJ}
> > +        exit
> > +fi
> > +
> >   if [ ! -z $ETH ]; then
> >       VETH0=${ETH}
> >       VETH1=${ETH}
>

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: [PATCH bpf-next 10/10] selftests/xsk: display command line options with -h
  2023-08-09 12:43 ` [PATCH bpf-next 10/10] selftests/xsk: display command line options with -h Magnus Karlsson
  2023-08-10 12:19   ` Przemek Kitszel
@ 2023-08-22 13:02   ` Maciej Fijalkowski
  1 sibling, 0 replies; 22+ messages in thread
From: Maciej Fijalkowski @ 2023-08-22 13:02 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, bpf, yhs, andrii,
	martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa

On Wed, Aug 09, 2023 at 02:43:43PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Add the -h option to display all available command line options
> available for test_xsk.sh and xskxceiver.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/test_xsk.sh | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> index 94b4b86d5239..baaeb016d699 100755
> --- a/tools/testing/selftests/bpf/test_xsk.sh
> +++ b/tools/testing/selftests/bpf/test_xsk.sh
> @@ -79,12 +79,15 @@
>  #
>  # Run a specific test from the test suite
>  #   sudo ./test_xsk.sh -t TEST_NAME
> +#
> +# Display the available command line options
> +#   sudo ./test_xsk.sh -h
>  
>  . xsk_prereqs.sh
>  
>  ETH=""
>  
> -while getopts "vi:dm:lt:" flag
> +while getopts "vi:dm:lt:h" flag
>  do
>  	case "${flag}" in
>  		v) verbose=1;;
> @@ -93,6 +96,7 @@ do
>  		m) MODE=${OPTARG};;
>  		l) list=1;;
>  		t) TEST=${OPTARG};;
> +		h) help=1;;
>  	esac
>  done
>  
> @@ -140,6 +144,11 @@ setup_vethPairs() {
>  	ip link set ${VETH0} up
>  }
>  
> +if [[ $help -eq 1 ]]; then
> +	./${XSKOBJ}
> +        exit
> +fi

is there anything that stops from having the list of test output before
all of the validation below (check that we are root, veth support etc) ?

I would like us to have a case 'h' within parse_command_line() though.

> +
>  if [ ! -z $ETH ]; then
>  	VETH0=${ETH}
>  	VETH1=${ETH}
> -- 
> 2.34.1
> 

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2023-08-22 13:52 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-08-09 12:43 [PATCH bpf-next 00/10] seltests/xsk: various improvements to xskxceiver Magnus Karlsson
2023-08-09 12:43 ` [PATCH bpf-next 01/10] selftests/xsk: print per packet info in verbose mode Magnus Karlsson
2023-08-09 12:43 ` [PATCH bpf-next 02/10] selftests/xsk: add timeout for Tx thread Magnus Karlsson
2023-08-09 12:43 ` [PATCH bpf-next 03/10] selftests/xsk: add option to only run tests in a single mode Magnus Karlsson
2023-08-10 12:14   ` Przemek Kitszel
2023-08-10 12:36     ` Magnus Karlsson
2023-08-09 12:43 ` [PATCH bpf-next 04/10] selftests/xsk: move all tests to separate functions Magnus Karlsson
2023-08-09 12:43 ` [PATCH bpf-next 05/10] selftests/xsk: declare test names in struct Magnus Karlsson
2023-08-10 12:15   ` Przemek Kitszel
2023-08-10 12:36     ` Magnus Karlsson
2023-08-22 12:28   ` Maciej Fijalkowski
2023-08-09 12:43 ` [PATCH bpf-next 06/10] selftests/xsk: add option that lists all tests Magnus Karlsson
2023-08-22 12:37   ` Maciej Fijalkowski
2023-08-22 13:52     ` Magnus Karlsson
2023-08-09 12:43 ` [PATCH bpf-next 07/10] selftests/xsk: add option to run single test Magnus Karlsson
2023-08-22 12:50   ` Maciej Fijalkowski
2023-08-09 12:43 ` [PATCH bpf-next 08/10] selftests/xsk: use ksft_print_msg uniformly Magnus Karlsson
2023-08-09 12:43 ` [PATCH bpf-next 09/10] selftests/xsk: fail single test instead of all tests Magnus Karlsson
2023-08-09 12:43 ` [PATCH bpf-next 10/10] selftests/xsk: display command line options with -h Magnus Karlsson
2023-08-10 12:19   ` Przemek Kitszel
2023-08-10 12:41     ` Magnus Karlsson
2023-08-22 13:02   ` Maciej Fijalkowski

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).