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

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

[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

11: Introduce the environment variable XSKTEST_ETH to be able to set
    the network interface to be used.

v1 -> v2:

* Introduce XSKTEST_MODE env variable to be able to set the mode to
  use [Przemyslaw]
* Introduce XSKTEST_ETH env variable to be able to set the ethernet
  interface to use by introducing a new patch (#11) [Magnus]
* Fixed spelling error in patch #5 [Przemyslaw, Maciej]
* Fixed confusing documentation in patch #10  [Przemyslaw]
* The -l option can now be used without being root [Magnus, Maciej]
* Fixed documentation error in patch #7 [Maciej]
* Added error handling to the -t option [Maciej]
* -h now displayed as an option [Maciej]

Thanks: Magnus

Magnus Karlsson (11):
  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
  selftests/xsk: introduce XSKTEST_ETH environment variable

 tools/testing/selftests/bpf/test_xsk.sh    |  60 ++-
 tools/testing/selftests/bpf/xsk_prereqs.sh |  10 +-
 tools/testing/selftests/bpf/xskxceiver.c   | 536 ++++++++++++---------
 tools/testing/selftests/bpf/xskxceiver.h   |  44 +-
 4 files changed, 377 insertions(+), 273 deletions(-)


base-commit: f3bdb54f09ab4cdbca48bf7befa8997cadd8d6a1
--
2.34.1

[PATCH bpf-next v2 01/11] selftests/xsk: print per packet info in verbose mode

[Magnus Karlsson]

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

[PATCH bpf-next v2 02/11] selftests/xsk: add timeout for Tx thread

[Magnus Karlsson]

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

[PATCH bpf-next v2 03/11] selftests/xsk: add option to only run tests in a single mode

[Magnus Karlsson]

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..5ae2b3c27e21 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -73,17 +73,21 @@
 #
 # Run test suite for physical device in loopback mode
 #   sudo ./test_xsk.sh -i IFACE
+#
+# Run test suite in a specific mode only [skb,drv,zc]
+#   sudo ./test_xsk.sh -m MODE
 
 . xsk_prereqs.sh
 
 ETH=""
 
-while getopts "vi:d" flag
+while getopts "vi:dm:" flag
 do
 	case "${flag}" in
 		v) verbose=1;;
 		d) debug=1;;
 		i) ETH=${OPTARG};;
+		m) XSKTEST_MODE=${OPTARG};;
 	esac
 done
 
@@ -153,6 +157,10 @@ if [[ $verbose -eq 1 ]]; then
 	ARGS+="-v "
 fi
 
+if [ -n "$XSKTEST_MODE" ]; then
+	ARGS+="-m ${XSKTEST_MODE} "
+fi
+
 retval=$?
 test_status $retval "${TEST_NAME}"
 
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 514fe994e02b..9f79c2b6aa97 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -107,6 +107,9 @@
 static const char *MAC1 = "\x00\x0A\x56\x9E\xEE\x62";
 static const char *MAC2 = "\x00\x0A\x56\x9E\xEE\x61";
 
+static bool opt_verbose;
+static enum test_mode opt_mode = TEST_MODE_ALL;
+
 static void __exit_with_error(int error, const char *file, const char *func, int line)
 {
 	ksft_test_result_fail("[%s:%s:%i]: ERROR: %d/\"%s\"\n", file, func, line, error,
@@ -310,17 +313,19 @@ static struct option long_options[] = {
 	{"interface", required_argument, 0, 'i'},
 	{"busy-poll", no_argument, 0, 'b'},
 	{"verbose", no_argument, 0, 'v'},
+	{"mode", required_argument, 0, 'm'},
 	{0, 0, 0, 0}
 };
 
 static void usage(const char *prog)
 {
 	const char *str =
-		"  Usage: %s [OPTIONS]\n"
+		"  Usage: xskxceiver [OPTIONS]\n"
 		"  Options:\n"
 		"  -i, --interface      Use interface\n"
 		"  -v, --verbose        Verbose output\n"
-		"  -b, --busy-poll      Enable busy poll\n";
+		"  -b, --busy-poll      Enable busy poll\n"
+		"  -m, --mode           Run only mode skb, drv, or zc\n";
 
 	ksft_print_msg(str, prog);
 }
@@ -342,7 +347,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "i:vb", long_options, &option_index);
+		c = getopt_long(argc, argv, "i:vbm:", long_options, &option_index);
 		if (c == -1)
 			break;
 
@@ -371,6 +376,21 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 			ifobj_tx->busy_poll = true;
 			ifobj_rx->busy_poll = true;
 			break;
+		case 'm':
+			if (!strncmp("skb", optarg, min_t(size_t, strlen(optarg),
+							  strlen("skb")))) {
+				opt_mode = TEST_MODE_SKB;
+			} else if (!strncmp("drv", optarg, min_t(size_t, strlen(optarg),
+								 strlen("drv")))) {
+				opt_mode = TEST_MODE_DRV;
+			} else if (!strncmp("zc", optarg, min_t(size_t, strlen(optarg),
+								strlen("zc")))) {
+				opt_mode = TEST_MODE_ZC;
+			} else {
+				usage(basename(argv[0]));
+				ksft_exit_xfail();
+			}
+			break;
 		default:
 			usage(basename(argv[0]));
 			ksft_exit_xfail();
@@ -2365,9 +2385,15 @@ int main(int argc, char **argv)
 	test.tx_pkt_stream_default = tx_pkt_stream_default;
 	test.rx_pkt_stream_default = rx_pkt_stream_default;
 
-	ksft_set_plan(modes * TEST_TYPE_MAX);
+	if (opt_mode == TEST_MODE_ALL)
+		ksft_set_plan(modes * TEST_TYPE_MAX);
+	else
+		ksft_set_plan(TEST_TYPE_MAX);
 
 	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

Re: [PATCH bpf-next v2 03/11] selftests/xsk: add option to only run tests in a single mode

[Maciej Fijalkowski]

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

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

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

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

Re: [PATCH bpf-next v2 03/11] selftests/xsk: add option to only run tests in a single mode

[Magnus Karlsson]

On Fri, 25 Aug 2023 at 14:02, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Thu, Aug 24, 2023 at 02:28:45PM +0200, 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..5ae2b3c27e21 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -73,17 +73,21 @@
> >  #
> >  # Run test suite for physical device in loopback mode
> >  #   sudo ./test_xsk.sh -i IFACE
> > +#
> > +# Run test suite in a specific mode only [skb,drv,zc]
> > +#   sudo ./test_xsk.sh -m MODE
> >
> >  . xsk_prereqs.sh
> >
> >  ETH=""
> >
> > -while getopts "vi:d" flag
> > +while getopts "vi:dm:" flag
> >  do
> >       case "${flag}" in
> >               v) verbose=1;;
> >               d) debug=1;;
> >               i) ETH=${OPTARG};;
> > +             m) XSKTEST_MODE=${OPTARG};;
> >       esac
> >  done
> >
> > @@ -153,6 +157,10 @@ if [[ $verbose -eq 1 ]]; then
> >       ARGS+="-v "
> >  fi
> >
> > +if [ -n "$XSKTEST_MODE" ]; then
> > +     ARGS+="-m ${XSKTEST_MODE} "
> > +fi
> > +
> >  retval=$?
> >  test_status $retval "${TEST_NAME}"
> >
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index 514fe994e02b..9f79c2b6aa97 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -107,6 +107,9 @@
> >  static const char *MAC1 = "\x00\x0A\x56\x9E\xEE\x62";
> >  static const char *MAC2 = "\x00\x0A\x56\x9E\xEE\x61";
> >
> > +static bool opt_verbose;
> > +static enum test_mode opt_mode = TEST_MODE_ALL;
> > +
> >  static void __exit_with_error(int error, const char *file, const char *func, int line)
> >  {
> >       ksft_test_result_fail("[%s:%s:%i]: ERROR: %d/\"%s\"\n", file, func, line, error,
> > @@ -310,17 +313,19 @@ static struct option long_options[] = {
> >       {"interface", required_argument, 0, 'i'},
> >       {"busy-poll", no_argument, 0, 'b'},
> >       {"verbose", no_argument, 0, 'v'},
> > +     {"mode", required_argument, 0, 'm'},
> >       {0, 0, 0, 0}
> >  };
> >
> >  static void usage(const char *prog)
> >  {
> >       const char *str =
> > -             "  Usage: %s [OPTIONS]\n"
> > +             "  Usage: xskxceiver [OPTIONS]\n"
> >               "  Options:\n"
> >               "  -i, --interface      Use interface\n"
> >               "  -v, --verbose        Verbose output\n"
> > -             "  -b, --busy-poll      Enable busy poll\n";
> > +             "  -b, --busy-poll      Enable busy poll\n"
> > +             "  -m, --mode           Run only mode skb, drv, or zc\n";
> >
> >       ksft_print_msg(str, prog);
> >  }
> > @@ -342,7 +347,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >       opterr = 0;
> >
> >       for (;;) {
> > -             c = getopt_long(argc, argv, "i:vb", long_options, &option_index);
> > +             c = getopt_long(argc, argv, "i:vbm:", long_options, &option_index);
> >               if (c == -1)
> >                       break;
> >
> > @@ -371,6 +376,21 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >                       ifobj_tx->busy_poll = true;
> >                       ifobj_rx->busy_poll = true;
> >                       break;
> > +             case 'm':
> > +                     if (!strncmp("skb", optarg, min_t(size_t, strlen(optarg),
> > +                                                       strlen("skb")))) {
> > +                             opt_mode = TEST_MODE_SKB;
> > +                     } else if (!strncmp("drv", optarg, min_t(size_t, strlen(optarg),
> > +                                                              strlen("drv")))) {
> > +                             opt_mode = TEST_MODE_DRV;
> > +                     } else if (!strncmp("zc", optarg, min_t(size_t, strlen(optarg),
> > +                                                             strlen("zc")))) {
> > +                             opt_mode = TEST_MODE_ZC;
> > +                     } else {
> > +                             usage(basename(argv[0]));
> > +                             ksft_exit_xfail();
> > +                     }
> > +                     break;
> >               default:
> >                       usage(basename(argv[0]));
> >                       ksft_exit_xfail();
> > @@ -2365,9 +2385,15 @@ int main(int argc, char **argv)
> >       test.tx_pkt_stream_default = tx_pkt_stream_default;
> >       test.rx_pkt_stream_default = rx_pkt_stream_default;
> >
> > -     ksft_set_plan(modes * TEST_TYPE_MAX);
> > +     if (opt_mode == TEST_MODE_ALL)
> > +             ksft_set_plan(modes * TEST_TYPE_MAX);
> > +     else
> > +             ksft_set_plan(TEST_TYPE_MAX);
>
> what will happen if i run zc mode for a device that does not support it?
> what will happen if i run xdp mode for a device that does not support it?
>
> I know we will do nothing and exit gracefully, but i am wondering if
> xskxceiver should catch it.

Hmm, yes it would be nicer just to get a message that zc or drv is not
supported than that it does nothing. I can add two if statements for
this if you like? We do have the information already at this point.

> >
> >       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
> >

[PATCH bpf-next v2 04/11] selftests/xsk: move all tests to separate functions

[Magnus Karlsson]

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

[PATCH bpf-next v2 05/11] selftests/xsk: declare test names in struct

[Magnus Karlsson]

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 patch, the names of them were 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

Re: [PATCH bpf-next v2 05/11] selftests/xsk: declare test names in struct

[Maciej Fijalkowski]

On Thu, Aug 24, 2023 at 02:28:47PM +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 patch, the names of them were 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

(...)

>  		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},
> +};

To others, I was asking on v1 to move this to a header file but i missed
somehow that we also set .test_func here so this would require us to
un-static each and every function which would in turn cause the diffstat
to grow significantly. IOW let us address that later.

> +
>  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
> 

[PATCH bpf-next v2 06/11] selftests/xsk: add option that lists all tests

[Magnus Karlsson]

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    | 15 +++++++++++++-
 tools/testing/selftests/bpf/xsk_prereqs.sh | 10 +++++----
 tools/testing/selftests/bpf/xskxceiver.c   | 24 ++++++++++++++++++++--
 3 files changed, 42 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 5ae2b3c27e21..489101922984 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -76,18 +76,22 @@
 #
 # Run test suite in a specific mode only [skb,drv,zc]
 #   sudo ./test_xsk.sh -m MODE
+#
+# List available tests
+#   ./test_xsk.sh -l
 
 . xsk_prereqs.sh
 
 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) XSKTEST_MODE=${OPTARG};;
+		l) list=1;;
 	esac
 done
 
@@ -135,6 +139,11 @@ setup_vethPairs() {
 	ip link set ${VETH0} up
 }
 
+if [[ $list -eq 1 ]]; then
+        ./${XSKOBJ} -l
+        exit
+fi
+
 if [ ! -z $ETH ]; then
 	VETH0=${ETH}
 	VETH1=${ETH}
@@ -183,6 +192,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

[PATCH bpf-next v2 07/11] selftests/xsk: add option to run single test

[Magnus Karlsson]

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 skb mode only

./test_xsh.sh -t 21 -m skb

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

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 489101922984..b7186ae48497 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -79,12 +79,15 @@
 #
 # List available tests
 #   ./test_xsk.sh -l
+#
+# 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;;
@@ -92,6 +95,7 @@ do
 		i) ETH=${OPTARG};;
 		m) XSKTEST_MODE=${OPTARG};;
 		l) list=1;;
+		t) XSKTEST_TEST=${OPTARG};;
 	esac
 done
 
@@ -170,6 +174,10 @@ if [ -n "$XSKTEST_MODE" ]; then
 	ARGS+="-m ${XSKTEST_MODE} "
 fi
 
+if [ -n "$XSKTEST_TEST" ]; then
+	ARGS+="-t ${XSKTEST_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..6eca5f95a3e0 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,10 +317,11 @@ static struct option long_options[] = {
 	{"verbose", no_argument, 0, 'v'},
 	{"mode", required_argument, 0, 'm'},
 	{"list", no_argument, 0, 'l'},
+	{"test", required_argument, 0, 't'},
 	{0, 0, 0, 0}
 };
 
-static void usage(const char *prog)
+static void print_usage(char **argv)
 {
 	const char *str =
 		"  Usage: xskxceiver [OPTIONS]\n"
@@ -328,9 +330,11 @@ 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);
+	ksft_print_msg(str, basename(argv[0]));
+	ksft_exit_xfail();
 }
 
 static bool validate_interface(struct ifobject *ifobj)
@@ -350,7 +354,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;
 
@@ -390,16 +394,20 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 								strlen("zc")))) {
 				opt_mode = TEST_MODE_ZC;
 			} else {
-				usage(basename(argv[0]));
-				ksft_exit_xfail();
+				print_usage(argv);
 			}
 			break;
 		case 'l':
 			opt_print_tests = true;
 			break;
+		case 't':
+			errno = 0;
+			opt_run_test = strtol(optarg, NULL, 0);
+			if (errno)
+				print_usage(argv);
+			break;
 		default:
-			usage(basename(argv[0]));
-			ksft_exit_xfail();
+			print_usage(argv);
 		}
 	}
 }
@@ -2330,8 +2338,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,15 +2361,17 @@ 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;
 	ifobj_rx->shared_umem = shared_netdev;
 
-	if (!validate_interface(ifobj_tx) || !validate_interface(ifobj_rx)) {
-		usage(basename(argv[0]));
-		ksft_exit_xfail();
-	}
+	if (!validate_interface(ifobj_tx) || !validate_interface(ifobj_rx))
+		print_usage(argv);
 
 	if (is_xdp_supported(ifobj_tx->ifindex)) {
 		modes++;
@@ -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

Re: [PATCH bpf-next v2 07/11] selftests/xsk: add option to run single test

[Maciej Fijalkowski]

On Thu, Aug 24, 2023 at 02:28:49PM +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 skb mode only
> 
> ./test_xsh.sh -t 21 -m skb
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/test_xsk.sh  | 10 +++-
>  tools/testing/selftests/bpf/xskxceiver.c | 58 ++++++++++++++++--------
>  tools/testing/selftests/bpf/xskxceiver.h |  3 ++
>  3 files changed, 52 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> index 489101922984..b7186ae48497 100755
> --- a/tools/testing/selftests/bpf/test_xsk.sh
> +++ b/tools/testing/selftests/bpf/test_xsk.sh
> @@ -79,12 +79,15 @@
>  #
>  # List available tests
>  #   ./test_xsk.sh -l
> +#
> +# 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;;
> @@ -92,6 +95,7 @@ do
>  		i) ETH=${OPTARG};;
>  		m) XSKTEST_MODE=${OPTARG};;
>  		l) list=1;;
> +		t) XSKTEST_TEST=${OPTARG};;
>  	esac
>  done
>  
> @@ -170,6 +174,10 @@ if [ -n "$XSKTEST_MODE" ]; then
>  	ARGS+="-m ${XSKTEST_MODE} "
>  fi
>  
> +if [ -n "$XSKTEST_TEST" ]; then
> +	ARGS+="-t ${XSKTEST_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..6eca5f95a3e0 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,10 +317,11 @@ static struct option long_options[] = {
>  	{"verbose", no_argument, 0, 'v'},
>  	{"mode", required_argument, 0, 'm'},
>  	{"list", no_argument, 0, 'l'},
> +	{"test", required_argument, 0, 't'},
>  	{0, 0, 0, 0}
>  };
>  
> -static void usage(const char *prog)
> +static void print_usage(char **argv)
>  {
>  	const char *str =
>  		"  Usage: xskxceiver [OPTIONS]\n"
> @@ -328,9 +330,11 @@ 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);
> +	ksft_print_msg(str, basename(argv[0]));
> +	ksft_exit_xfail();
>  }
>  
>  static bool validate_interface(struct ifobject *ifobj)
> @@ -350,7 +354,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;
>  
> @@ -390,16 +394,20 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  								strlen("zc")))) {
>  				opt_mode = TEST_MODE_ZC;
>  			} else {
> -				usage(basename(argv[0]));
> -				ksft_exit_xfail();
> +				print_usage(argv);
>  			}
>  			break;
>  		case 'l':
>  			opt_print_tests = true;
>  			break;
> +		case 't':
> +			errno = 0;
> +			opt_run_test = strtol(optarg, NULL, 0);
> +			if (errno)
> +				print_usage(argv);

aren't you missing ksft_exit_xfail(); here?

> +			break;
>  		default:
> -			usage(basename(argv[0]));
> -			ksft_exit_xfail();
> +			print_usage(argv);
>  		}
>  	}
>  }
> @@ -2330,8 +2338,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,15 +2361,17 @@ 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;
>  	ifobj_rx->shared_umem = shared_netdev;
>  
> -	if (!validate_interface(ifobj_tx) || !validate_interface(ifobj_rx)) {
> -		usage(basename(argv[0]));
> -		ksft_exit_xfail();
> -	}
> +	if (!validate_interface(ifobj_tx) || !validate_interface(ifobj_rx))
> +		print_usage(argv);
>  
>  	if (is_xdp_supported(ifobj_tx->ifindex)) {
>  		modes++;
> @@ -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);

Could we simplify this and avoid branching just for getting the index of
test to run with something like:

		for (curr_test = opt_run_test; j = 0; j < nb_tests; j++, curr_test++) {

branch vs additional variable, up to you but repeated code caught my eye

>  
>  			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
> 

Re: [PATCH bpf-next v2 07/11] selftests/xsk: add option to run single test

[Magnus Karlsson]

On Fri, 25 Aug 2023 at 14:45, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Thu, Aug 24, 2023 at 02:28:49PM +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 skb mode only
> >
> > ./test_xsh.sh -t 21 -m skb
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >  tools/testing/selftests/bpf/test_xsk.sh  | 10 +++-
> >  tools/testing/selftests/bpf/xskxceiver.c | 58 ++++++++++++++++--------
> >  tools/testing/selftests/bpf/xskxceiver.h |  3 ++
> >  3 files changed, 52 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > index 489101922984..b7186ae48497 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -79,12 +79,15 @@
> >  #
> >  # List available tests
> >  #   ./test_xsk.sh -l
> > +#
> > +# 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;;
> > @@ -92,6 +95,7 @@ do
> >               i) ETH=${OPTARG};;
> >               m) XSKTEST_MODE=${OPTARG};;
> >               l) list=1;;
> > +             t) XSKTEST_TEST=${OPTARG};;
> >       esac
> >  done
> >
> > @@ -170,6 +174,10 @@ if [ -n "$XSKTEST_MODE" ]; then
> >       ARGS+="-m ${XSKTEST_MODE} "
> >  fi
> >
> > +if [ -n "$XSKTEST_TEST" ]; then
> > +     ARGS+="-t ${XSKTEST_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..6eca5f95a3e0 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,10 +317,11 @@ static struct option long_options[] = {
> >       {"verbose", no_argument, 0, 'v'},
> >       {"mode", required_argument, 0, 'm'},
> >       {"list", no_argument, 0, 'l'},
> > +     {"test", required_argument, 0, 't'},
> >       {0, 0, 0, 0}
> >  };
> >
> > -static void usage(const char *prog)
> > +static void print_usage(char **argv)
> >  {
> >       const char *str =
> >               "  Usage: xskxceiver [OPTIONS]\n"
> > @@ -328,9 +330,11 @@ 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);
> > +     ksft_print_msg(str, basename(argv[0]));
> > +     ksft_exit_xfail();
> >  }
> >
> >  static bool validate_interface(struct ifobject *ifobj)
> > @@ -350,7 +354,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;
> >
> > @@ -390,16 +394,20 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >                                                               strlen("zc")))) {
> >                               opt_mode = TEST_MODE_ZC;
> >                       } else {
> > -                             usage(basename(argv[0]));
> > -                             ksft_exit_xfail();
> > +                             print_usage(argv);
> >                       }
> >                       break;
> >               case 'l':
> >                       opt_print_tests = true;
> >                       break;
> > +             case 't':
> > +                     errno = 0;
> > +                     opt_run_test = strtol(optarg, NULL, 0);
> > +                     if (errno)
> > +                             print_usage(argv);
>
> aren't you missing ksft_exit_xfail(); here?

It is in print_usage() instead since all calls to print_usage() ends that way.

> > +                     break;
> >               default:
> > -                     usage(basename(argv[0]));
> > -                     ksft_exit_xfail();
> > +                     print_usage(argv);
> >               }
> >       }
> >  }
> > @@ -2330,8 +2338,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,15 +2361,17 @@ 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;
> >       ifobj_rx->shared_umem = shared_netdev;
> >
> > -     if (!validate_interface(ifobj_tx) || !validate_interface(ifobj_rx)) {
> > -             usage(basename(argv[0]));
> > -             ksft_exit_xfail();
> > -     }
> > +     if (!validate_interface(ifobj_tx) || !validate_interface(ifobj_rx))
> > +             print_usage(argv);
> >
> >       if (is_xdp_supported(ifobj_tx->ifindex)) {
> >               modes++;
> > @@ -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);
>
> Could we simplify this and avoid branching just for getting the index of
> test to run with something like:
>
>                 for (curr_test = opt_run_test; j = 0; j < nb_tests; j++, curr_test++) {
>
> branch vs additional variable, up to you but repeated code caught my eye

Would be nicer without repeated code for sure. I will try to simplify it.

> >
> >                       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
> >

[PATCH bpf-next v2 08/11] selftests/xsk: use ksft_print_msg uniformly

[Magnus Karlsson]

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 6eca5f95a3e0..2cb5fdf0188e 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -812,7 +812,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++;
 	}
 }
@@ -824,13 +824,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 {
@@ -838,15 +838,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)
@@ -1557,7 +1557,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);
 		}
 	}
@@ -1621,7 +1622,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);
 	}
 
@@ -2108,7 +2109,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

[PATCH bpf-next v2 09/11] selftests/xsk: fail single test instead of all tests

[Magnus Karlsson]

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 2cb5fdf0188e..19db9a827c30 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -951,36 +951,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) {
@@ -1028,11 +1034,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))
@@ -1053,12 +1062,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);
 			}
@@ -1142,7 +1149,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;
 	}
 
@@ -1325,7 +1334,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)
@@ -1351,7 +1362,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)
@@ -1370,7 +1383,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)
@@ -1779,7 +1794,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;
 
@@ -1790,7 +1805,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)
@@ -1800,7 +1817,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);
 }
 
@@ -2001,11 +2019,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

Re: [PATCH bpf-next v2 09/11] selftests/xsk: fail single test instead of all tests

[Maciej Fijalkowski]

On Thu, Aug 24, 2023 at 02:28:51PM +0200, Magnus Karlsson wrote:
> 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

nit: s/though/thought

> 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(-)

[PATCH bpf-next v2 10/11] selftests/xsk: display command line options with -h

[Magnus Karlsson]

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 ++++++++++-
 tools/testing/selftests/bpf/xskxceiver.c |  5 ++++-
 2 files changed, 14 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index b7186ae48497..9ec718043c1a 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -82,12 +82,15 @@
 #
 # Run a specific test from the test suite
 #   sudo ./test_xsk.sh -t TEST_NAME
+#
+# Display the available command line options
+#   ./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;;
@@ -96,6 +99,7 @@ do
 		m) XSKTEST_MODE=${OPTARG};;
 		l) list=1;;
 		t) XSKTEST_TEST=${OPTARG};;
+		h) help=1;;
 	esac
 done
 
@@ -148,6 +152,11 @@ if [[ $list -eq 1 ]]; then
         exit
 fi
 
+if [[ $help -eq 1 ]]; then
+	./${XSKOBJ}
+        exit
+fi
+
 if [ ! -z $ETH ]; then
 	VETH0=${ETH}
 	VETH1=${ETH}
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 19db9a827c30..9feb476d647f 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -318,6 +318,7 @@ static struct option long_options[] = {
 	{"mode", required_argument, 0, 'm'},
 	{"list", no_argument, 0, 'l'},
 	{"test", required_argument, 0, 't'},
+	{"help", no_argument, 0, 'h'},
 	{0, 0, 0, 0}
 };
 
@@ -331,7 +332,8 @@ static void print_usage(char **argv)
 		"  -b, --busy-poll      Enable busy poll\n"
 		"  -m, --mode           Run only mode skb, drv, or zc\n"
 		"  -l, --list           List all available tests\n"
-		"  -t, --test           Run a specific test. Enter number from -l option.\n";
+		"  -t, --test           Run a specific test. Enter number from -l option.\n"
+		"  -h, --help           Display this help and exit\n";
 
 	ksft_print_msg(str, basename(argv[0]));
 	ksft_exit_xfail();
@@ -406,6 +408,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 			if (errno)
 				print_usage(argv);
 			break;
+		case 'h':
 		default:
 			print_usage(argv);
 		}
-- 
2.34.1

Re: [PATCH bpf-next v2 10/11] selftests/xsk: display command line options with -h

[Maciej Fijalkowski]

On Thu, Aug 24, 2023 at 02:28:52PM +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 ++++++++++-
>  tools/testing/selftests/bpf/xskxceiver.c |  5 ++++-
>  2 files changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> index b7186ae48497..9ec718043c1a 100755
> --- a/tools/testing/selftests/bpf/test_xsk.sh
> +++ b/tools/testing/selftests/bpf/test_xsk.sh
> @@ -82,12 +82,15 @@
>  #
>  # Run a specific test from the test suite
>  #   sudo ./test_xsk.sh -t TEST_NAME
> +#
> +# Display the available command line options
> +#   ./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;;
> @@ -96,6 +99,7 @@ do
>  		m) XSKTEST_MODE=${OPTARG};;
>  		l) list=1;;
>  		t) XSKTEST_TEST=${OPTARG};;
> +		h) help=1;;
>  	esac
>  done
>  
> @@ -148,6 +152,11 @@ if [[ $list -eq 1 ]]; then
>          exit
>  fi
>  
> +if [[ $help -eq 1 ]]; then
> +	./${XSKOBJ}
> +        exit
> +fi
> +
>  if [ ! -z $ETH ]; then
>  	VETH0=${ETH}
>  	VETH1=${ETH}
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 19db9a827c30..9feb476d647f 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -318,6 +318,7 @@ static struct option long_options[] = {
>  	{"mode", required_argument, 0, 'm'},
>  	{"list", no_argument, 0, 'l'},
>  	{"test", required_argument, 0, 't'},
> +	{"help", no_argument, 0, 'h'},
>  	{0, 0, 0, 0}
>  };
>  
> @@ -331,7 +332,8 @@ static void print_usage(char **argv)
>  		"  -b, --busy-poll      Enable busy poll\n"
>  		"  -m, --mode           Run only mode skb, drv, or zc\n"
>  		"  -l, --list           List all available tests\n"
> -		"  -t, --test           Run a specific test. Enter number from -l option.\n";
> +		"  -t, --test           Run a specific test. Enter number from -l option.\n"
> +		"  -h, --help           Display this help and exit\n";
>  
>  	ksft_print_msg(str, basename(argv[0]));
>  	ksft_exit_xfail();
> @@ -406,6 +408,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>  			if (errno)
>  				print_usage(argv);
>  			break;
> +		case 'h':

do you need 'fallthrough' here?

>  		default:
>  			print_usage(argv);
>  		}
> -- 
> 2.34.1
> 

Re: [PATCH bpf-next v2 10/11] selftests/xsk: display command line options with -h

[Magnus Karlsson]

On Fri, 25 Aug 2023 at 14:50, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Thu, Aug 24, 2023 at 02:28:52PM +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 ++++++++++-
> >  tools/testing/selftests/bpf/xskxceiver.c |  5 ++++-
> >  2 files changed, 14 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > index b7186ae48497..9ec718043c1a 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -82,12 +82,15 @@
> >  #
> >  # Run a specific test from the test suite
> >  #   sudo ./test_xsk.sh -t TEST_NAME
> > +#
> > +# Display the available command line options
> > +#   ./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;;
> > @@ -96,6 +99,7 @@ do
> >               m) XSKTEST_MODE=${OPTARG};;
> >               l) list=1;;
> >               t) XSKTEST_TEST=${OPTARG};;
> > +             h) help=1;;
> >       esac
> >  done
> >
> > @@ -148,6 +152,11 @@ if [[ $list -eq 1 ]]; then
> >          exit
> >  fi
> >
> > +if [[ $help -eq 1 ]]; then
> > +     ./${XSKOBJ}
> > +        exit
> > +fi
> > +
> >  if [ ! -z $ETH ]; then
> >       VETH0=${ETH}
> >       VETH1=${ETH}
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index 19db9a827c30..9feb476d647f 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -318,6 +318,7 @@ static struct option long_options[] = {
> >       {"mode", required_argument, 0, 'm'},
> >       {"list", no_argument, 0, 'l'},
> >       {"test", required_argument, 0, 't'},
> > +     {"help", no_argument, 0, 'h'},
> >       {0, 0, 0, 0}
> >  };
> >
> > @@ -331,7 +332,8 @@ static void print_usage(char **argv)
> >               "  -b, --busy-poll      Enable busy poll\n"
> >               "  -m, --mode           Run only mode skb, drv, or zc\n"
> >               "  -l, --list           List all available tests\n"
> > -             "  -t, --test           Run a specific test. Enter number from -l option.\n";
> > +             "  -t, --test           Run a specific test. Enter number from -l option.\n"
> > +             "  -h, --help           Display this help and exit\n";
> >
> >       ksft_print_msg(str, basename(argv[0]));
> >       ksft_exit_xfail();
> > @@ -406,6 +408,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >                       if (errno)
> >                               print_usage(argv);
> >                       break;
> > +             case 'h':
>
> do you need 'fallthrough' here?

Did not get any complaints from checkpatch, so do not know since it is
a case without any content on its own. I would say it is obvious that
it is "falling through" in this case :-). But I do not know what the
rule is.

> >               default:
> >                       print_usage(argv);
> >               }
> > --
> > 2.34.1
> >

Re: [PATCH bpf-next v2 10/11] selftests/xsk: display command line options with -h

[Maciej Fijalkowski]

On Fri, Aug 25, 2023 at 02:54:01PM +0200, Magnus Karlsson wrote:
> On Fri, 25 Aug 2023 at 14:50, Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > On Thu, Aug 24, 2023 at 02:28:52PM +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 ++++++++++-
> > >  tools/testing/selftests/bpf/xskxceiver.c |  5 ++++-
> > >  2 files changed, 14 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > > index b7186ae48497..9ec718043c1a 100755
> > > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > > @@ -82,12 +82,15 @@
> > >  #
> > >  # Run a specific test from the test suite
> > >  #   sudo ./test_xsk.sh -t TEST_NAME
> > > +#
> > > +# Display the available command line options
> > > +#   ./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;;
> > > @@ -96,6 +99,7 @@ do
> > >               m) XSKTEST_MODE=${OPTARG};;
> > >               l) list=1;;
> > >               t) XSKTEST_TEST=${OPTARG};;
> > > +             h) help=1;;
> > >       esac
> > >  done
> > >
> > > @@ -148,6 +152,11 @@ if [[ $list -eq 1 ]]; then
> > >          exit
> > >  fi
> > >
> > > +if [[ $help -eq 1 ]]; then
> > > +     ./${XSKOBJ}
> > > +        exit
> > > +fi
> > > +
> > >  if [ ! -z $ETH ]; then
> > >       VETH0=${ETH}
> > >       VETH1=${ETH}
> > > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > > index 19db9a827c30..9feb476d647f 100644
> > > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > > @@ -318,6 +318,7 @@ static struct option long_options[] = {
> > >       {"mode", required_argument, 0, 'm'},
> > >       {"list", no_argument, 0, 'l'},
> > >       {"test", required_argument, 0, 't'},
> > > +     {"help", no_argument, 0, 'h'},
> > >       {0, 0, 0, 0}
> > >  };
> > >
> > > @@ -331,7 +332,8 @@ static void print_usage(char **argv)
> > >               "  -b, --busy-poll      Enable busy poll\n"
> > >               "  -m, --mode           Run only mode skb, drv, or zc\n"
> > >               "  -l, --list           List all available tests\n"
> > > -             "  -t, --test           Run a specific test. Enter number from -l option.\n";
> > > +             "  -t, --test           Run a specific test. Enter number from -l option.\n"
> > > +             "  -h, --help           Display this help and exit\n";
> > >
> > >       ksft_print_msg(str, basename(argv[0]));
> > >       ksft_exit_xfail();
> > > @@ -406,6 +408,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> > >                       if (errno)
> > >                               print_usage(argv);
> > >                       break;
> > > +             case 'h':
> >
> > do you need 'fallthrough' here?
> 
> Did not get any complaints from checkpatch, so do not know since it is
> a case without any content on its own. I would say it is obvious that
> it is "falling through" in this case :-). But I do not know what the
> rule is.

Sorry for the noise it's fine i quickly tested that with
-Wimplicit-fallthrough.

> 
> > >               default:
> > >                       print_usage(argv);
> > >               }
> > > --
> > > 2.34.1
> > >

[PATCH bpf-next v2 11/11] selftests/xsk: introduce XSKTEST_ETH environment variable

[Magnus Karlsson]

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

Introduce the XSKTEST_ETH environment variable to be able to set the
network interface that should be used for testing.

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

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 9ec718043c1a..3e0a2302a185 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -88,14 +88,12 @@
 
 . xsk_prereqs.sh
 
-ETH=""
-
 while getopts "vi:dm:lt:h" flag
 do
 	case "${flag}" in
 		v) verbose=1;;
 		d) debug=1;;
-		i) ETH=${OPTARG};;
+		i) XSKTEST_ETH=${OPTARG};;
 		m) XSKTEST_MODE=${OPTARG};;
 		l) list=1;;
 		t) XSKTEST_TEST=${OPTARG};;
@@ -157,9 +155,9 @@ if [[ $help -eq 1 ]]; then
         exit
 fi
 
-if [ ! -z $ETH ]; then
-	VETH0=${ETH}
-	VETH1=${ETH}
+if [ -n "$XSKTEST_ETH" ]; then
+	VETH0=${XSKTEST_ETH}
+	VETH1=${XSKTEST_ETH}
 else
 	validate_root_exec
 	validate_veth_support ${VETH0}
@@ -203,10 +201,10 @@ fi
 
 exec_xskxceiver
 
-if [ -z $ETH ]; then
+if [ -z $XSKTEST_ETH ]; then
 	cleanup_exit ${VETH0} ${VETH1}
 else
-	cleanup_iface ${ETH} ${MTU}
+	cleanup_iface ${XSKTEST_ETH} ${MTU}
 fi
 
 if [[ $list -eq 1 ]]; then
@@ -216,17 +214,17 @@ fi
 TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
 busy_poll=1
 
-if [ -z $ETH ]; then
+if [ -z $XSKTEST_ETH ]; then
 	setup_vethPairs
 fi
 exec_xskxceiver
 
 ## END TESTS
 
-if [ -z $ETH ]; then
+if [ -z $XSKTEST_ETH ]; then
 	cleanup_exit ${VETH0} ${VETH1}
 else
-	cleanup_iface ${ETH} ${MTU}
+	cleanup_iface ${XSKTEST_ETH} ${MTU}
 fi
 
 failures=0
-- 
2.34.1

Re: [PATCH bpf-next v2 11/11] selftests/xsk: introduce XSKTEST_ETH environment variable

[Maciej Fijalkowski]

On Thu, Aug 24, 2023 at 02:28:53PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Introduce the XSKTEST_ETH environment variable to be able to set the
> network interface that should be used for testing.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/test_xsk.sh | 20 +++++++++-----------
>  1 file changed, 9 insertions(+), 11 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> index 9ec718043c1a..3e0a2302a185 100755
> --- a/tools/testing/selftests/bpf/test_xsk.sh
> +++ b/tools/testing/selftests/bpf/test_xsk.sh
> @@ -88,14 +88,12 @@
>  
>  . xsk_prereqs.sh
>  
> -ETH=""
> -
>  while getopts "vi:dm:lt:h" flag
>  do
>  	case "${flag}" in
>  		v) verbose=1;;
>  		d) debug=1;;
> -		i) ETH=${OPTARG};;
> +		i) XSKTEST_ETH=${OPTARG};;
>  		m) XSKTEST_MODE=${OPTARG};;
>  		l) list=1;;
>  		t) XSKTEST_TEST=${OPTARG};;
> @@ -157,9 +155,9 @@ if [[ $help -eq 1 ]]; then
>          exit
>  fi
>  
> -if [ ! -z $ETH ]; then
> -	VETH0=${ETH}
> -	VETH1=${ETH}
> +if [ -n "$XSKTEST_ETH" ]; then

Sorry - is point of this patch is just to invert the logic and rename the
env var?

> +	VETH0=${XSKTEST_ETH}
> +	VETH1=${XSKTEST_ETH}
>  else
>  	validate_root_exec
>  	validate_veth_support ${VETH0}
> @@ -203,10 +201,10 @@ fi
>  
>  exec_xskxceiver
>  
> -if [ -z $ETH ]; then
> +if [ -z $XSKTEST_ETH ]; then
>  	cleanup_exit ${VETH0} ${VETH1}
>  else
> -	cleanup_iface ${ETH} ${MTU}
> +	cleanup_iface ${XSKTEST_ETH} ${MTU}
>  fi
>  
>  if [[ $list -eq 1 ]]; then
> @@ -216,17 +214,17 @@ fi
>  TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
>  busy_poll=1
>  
> -if [ -z $ETH ]; then
> +if [ -z $XSKTEST_ETH ]; then
>  	setup_vethPairs
>  fi
>  exec_xskxceiver
>  
>  ## END TESTS
>  
> -if [ -z $ETH ]; then
> +if [ -z $XSKTEST_ETH ]; then
>  	cleanup_exit ${VETH0} ${VETH1}
>  else
> -	cleanup_iface ${ETH} ${MTU}
> +	cleanup_iface ${XSKTEST_ETH} ${MTU}
>  fi
>  
>  failures=0
> -- 
> 2.34.1
> 

Re: [PATCH bpf-next v2 11/11] selftests/xsk: introduce XSKTEST_ETH environment variable

[Magnus Karlsson]

On Fri, 25 Aug 2023 at 14:57, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Thu, Aug 24, 2023 at 02:28:53PM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Introduce the XSKTEST_ETH environment variable to be able to set the
> > network interface that should be used for testing.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >  tools/testing/selftests/bpf/test_xsk.sh | 20 +++++++++-----------
> >  1 file changed, 9 insertions(+), 11 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > index 9ec718043c1a..3e0a2302a185 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -88,14 +88,12 @@
> >
> >  . xsk_prereqs.sh
> >
> > -ETH=""
> > -
> >  while getopts "vi:dm:lt:h" flag
> >  do
> >       case "${flag}" in
> >               v) verbose=1;;
> >               d) debug=1;;
> > -             i) ETH=${OPTARG};;
> > +             i) XSKTEST_ETH=${OPTARG};;
> >               m) XSKTEST_MODE=${OPTARG};;
> >               l) list=1;;
> >               t) XSKTEST_TEST=${OPTARG};;
> > @@ -157,9 +155,9 @@ if [[ $help -eq 1 ]]; then
> >          exit
> >  fi
> >
> > -if [ ! -z $ETH ]; then
> > -     VETH0=${ETH}
> > -     VETH1=${ETH}
> > +if [ -n "$XSKTEST_ETH" ]; then
>
> Sorry - is point of this patch is just to invert the logic and rename the
> env var?

The purpose was to make it setable from the outside and give it a name
that is more descriptive and targeted only to xskxceiver.

> > +     VETH0=${XSKTEST_ETH}
> > +     VETH1=${XSKTEST_ETH}
> >  else
> >       validate_root_exec
> >       validate_veth_support ${VETH0}
> > @@ -203,10 +201,10 @@ fi
> >
> >  exec_xskxceiver
> >
> > -if [ -z $ETH ]; then
> > +if [ -z $XSKTEST_ETH ]; then
> >       cleanup_exit ${VETH0} ${VETH1}
> >  else
> > -     cleanup_iface ${ETH} ${MTU}
> > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> >  fi
> >
> >  if [[ $list -eq 1 ]]; then
> > @@ -216,17 +214,17 @@ fi
> >  TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
> >  busy_poll=1
> >
> > -if [ -z $ETH ]; then
> > +if [ -z $XSKTEST_ETH ]; then
> >       setup_vethPairs
> >  fi
> >  exec_xskxceiver
> >
> >  ## END TESTS
> >
> > -if [ -z $ETH ]; then
> > +if [ -z $XSKTEST_ETH ]; then
> >       cleanup_exit ${VETH0} ${VETH1}
> >  else
> > -     cleanup_iface ${ETH} ${MTU}
> > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> >  fi
> >
> >  failures=0
> > --
> > 2.34.1
> >

Re: [PATCH bpf-next v2 11/11] selftests/xsk: introduce XSKTEST_ETH environment variable

[Maciej Fijalkowski]

On Fri, Aug 25, 2023 at 03:03:58PM +0200, Magnus Karlsson wrote:
> On Fri, 25 Aug 2023 at 14:57, Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > On Thu, Aug 24, 2023 at 02:28:53PM +0200, Magnus Karlsson wrote:
> > > From: Magnus Karlsson <magnus.karlsson@intel.com>
> > >
> > > Introduce the XSKTEST_ETH environment variable to be able to set the
> > > network interface that should be used for testing.
> > >
> > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > > ---
> > >  tools/testing/selftests/bpf/test_xsk.sh | 20 +++++++++-----------
> > >  1 file changed, 9 insertions(+), 11 deletions(-)
> > >
> > > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > > index 9ec718043c1a..3e0a2302a185 100755
> > > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > > @@ -88,14 +88,12 @@
> > >
> > >  . xsk_prereqs.sh
> > >
> > > -ETH=""
> > > -
> > >  while getopts "vi:dm:lt:h" flag
> > >  do
> > >       case "${flag}" in
> > >               v) verbose=1;;
> > >               d) debug=1;;
> > > -             i) ETH=${OPTARG};;
> > > +             i) XSKTEST_ETH=${OPTARG};;
> > >               m) XSKTEST_MODE=${OPTARG};;
> > >               l) list=1;;
> > >               t) XSKTEST_TEST=${OPTARG};;
> > > @@ -157,9 +155,9 @@ if [[ $help -eq 1 ]]; then
> > >          exit
> > >  fi
> > >
> > > -if [ ! -z $ETH ]; then
> > > -     VETH0=${ETH}
> > > -     VETH1=${ETH}
> > > +if [ -n "$XSKTEST_ETH" ]; then
> >
> > Sorry - is point of this patch is just to invert the logic and rename the
> > env var?
> 
> The purpose was to make it setable from the outside and give it a name
> that is more descriptive and targeted only to xskxceiver.

and this is accomplished by not having ETH initialized here? What will be
'the outside' ?

Currently I don't see much value within this patch, unless you explain the
need for setting this from outside of this script. Maybe I missed some
discussion from v1. I can live with this variable being ETH, what's more
concerning/confusing to me is that for ZC we have to set VETH0 and VETH1
to ETH and then use that later on.


> 
> > > +     VETH0=${XSKTEST_ETH}
> > > +     VETH1=${XSKTEST_ETH}
> > >  else
> > >       validate_root_exec
> > >       validate_veth_support ${VETH0}
> > > @@ -203,10 +201,10 @@ fi
> > >
> > >  exec_xskxceiver
> > >
> > > -if [ -z $ETH ]; then
> > > +if [ -z $XSKTEST_ETH ]; then
> > >       cleanup_exit ${VETH0} ${VETH1}
> > >  else
> > > -     cleanup_iface ${ETH} ${MTU}
> > > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> > >  fi
> > >
> > >  if [[ $list -eq 1 ]]; then
> > > @@ -216,17 +214,17 @@ fi
> > >  TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
> > >  busy_poll=1
> > >
> > > -if [ -z $ETH ]; then
> > > +if [ -z $XSKTEST_ETH ]; then
> > >       setup_vethPairs
> > >  fi
> > >  exec_xskxceiver
> > >
> > >  ## END TESTS
> > >
> > > -if [ -z $ETH ]; then
> > > +if [ -z $XSKTEST_ETH ]; then
> > >       cleanup_exit ${VETH0} ${VETH1}
> > >  else
> > > -     cleanup_iface ${ETH} ${MTU}
> > > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> > >  fi
> > >
> > >  failures=0
> > > --
> > > 2.34.1
> > >

Re: [PATCH bpf-next v2 11/11] selftests/xsk: introduce XSKTEST_ETH environment variable

[Magnus Karlsson]

On Fri, 25 Aug 2023 at 15:20, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Fri, Aug 25, 2023 at 03:03:58PM +0200, Magnus Karlsson wrote:
> > On Fri, 25 Aug 2023 at 14:57, Maciej Fijalkowski
> > <maciej.fijalkowski@intel.com> wrote:
> > >
> > > On Thu, Aug 24, 2023 at 02:28:53PM +0200, Magnus Karlsson wrote:
> > > > From: Magnus Karlsson <magnus.karlsson@intel.com>
> > > >
> > > > Introduce the XSKTEST_ETH environment variable to be able to set the
> > > > network interface that should be used for testing.
> > > >
> > > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > > > ---
> > > >  tools/testing/selftests/bpf/test_xsk.sh | 20 +++++++++-----------
> > > >  1 file changed, 9 insertions(+), 11 deletions(-)
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> > > > index 9ec718043c1a..3e0a2302a185 100755
> > > > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > > > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > > > @@ -88,14 +88,12 @@
> > > >
> > > >  . xsk_prereqs.sh
> > > >
> > > > -ETH=""
> > > > -
> > > >  while getopts "vi:dm:lt:h" flag
> > > >  do
> > > >       case "${flag}" in
> > > >               v) verbose=1;;
> > > >               d) debug=1;;
> > > > -             i) ETH=${OPTARG};;
> > > > +             i) XSKTEST_ETH=${OPTARG};;
> > > >               m) XSKTEST_MODE=${OPTARG};;
> > > >               l) list=1;;
> > > >               t) XSKTEST_TEST=${OPTARG};;
> > > > @@ -157,9 +155,9 @@ if [[ $help -eq 1 ]]; then
> > > >          exit
> > > >  fi
> > > >
> > > > -if [ ! -z $ETH ]; then
> > > > -     VETH0=${ETH}
> > > > -     VETH1=${ETH}
> > > > +if [ -n "$XSKTEST_ETH" ]; then
> > >
> > > Sorry - is point of this patch is just to invert the logic and rename the
> > > env var?
> >
> > The purpose was to make it setable from the outside and give it a name
> > that is more descriptive and targeted only to xskxceiver.
>
> and this is accomplished by not having ETH initialized here? What will be
> 'the outside' ?
>
> Currently I don't see much value within this patch, unless you explain the
> need for setting this from outside of this script. Maybe I missed some

Outside = from the command line or your environment variables. ETH is
not a good name to have set in your environment variable. Who knows
what it would be used for. with XSKTEST_* at least you know it is used
for the xsk testing.

> discussion from v1. I can live with this variable being ETH, what's more
> concerning/confusing to me is that for ZC we have to set VETH0 and VETH1
> to ETH and then use that later on.

That is in the old code. Nothing I am trying to address here.

If this patch is useful or not, I do not know to be honest. Just added
it because Przemyslaw suggested that we make the mode setable from
your env variables in your shell. Weird to have the mode setable but
not the ethernet interface. So either we drop both and make it
impossible to use env variables, or support setting both from the env
variables. Do not have a strong opinion either way. Maybe this is just
an unnecessary "feature" that will not be used.

>
> >
> > > > +     VETH0=${XSKTEST_ETH}
> > > > +     VETH1=${XSKTEST_ETH}
> > > >  else
> > > >       validate_root_exec
> > > >       validate_veth_support ${VETH0}
> > > > @@ -203,10 +201,10 @@ fi
> > > >
> > > >  exec_xskxceiver
> > > >
> > > > -if [ -z $ETH ]; then
> > > > +if [ -z $XSKTEST_ETH ]; then
> > > >       cleanup_exit ${VETH0} ${VETH1}
> > > >  else
> > > > -     cleanup_iface ${ETH} ${MTU}
> > > > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> > > >  fi
> > > >
> > > >  if [[ $list -eq 1 ]]; then
> > > > @@ -216,17 +214,17 @@ fi
> > > >  TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
> > > >  busy_poll=1
> > > >
> > > > -if [ -z $ETH ]; then
> > > > +if [ -z $XSKTEST_ETH ]; then
> > > >       setup_vethPairs
> > > >  fi
> > > >  exec_xskxceiver
> > > >
> > > >  ## END TESTS
> > > >
> > > > -if [ -z $ETH ]; then
> > > > +if [ -z $XSKTEST_ETH ]; then
> > > >       cleanup_exit ${VETH0} ${VETH1}
> > > >  else
> > > > -     cleanup_iface ${ETH} ${MTU}
> > > > +     cleanup_iface ${XSKTEST_ETH} ${MTU}
> > > >  fi
> > > >
> > > >  failures=0
> > > > --
> > > > 2.34.1
> > > >