netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes
@ 2022-05-10 11:55 Magnus Karlsson
  2022-05-10 11:55 ` [PATCH bpf-next 1/9] selftests: xsk: cleanup bash scripts Magnus Karlsson
                   ` (11 more replies)
  0 siblings, 12 replies; 13+ messages in thread
From: Magnus Karlsson @ 2022-05-10 11:55 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh
  Cc: Magnus Karlsson, jonathan.lemon, bpf

This patch set adds busy-poll testing to the xsk selftests. It runs
exactly the same tests as with regular softirq processing, but with
busy-poll enabled. I have also included a number of fixes to the
selftests that have been bugging me for a while or was discovered
while implementing the busy-poll support. In summary these are:

* Fix the error reporting of failed tests. Each failed test used to be
  reported as both failed and passed, messing up things.

* Added a summary test printout at the end of the test suite so that
  users do not have to scroll up and look at the result of both the
  softirq run and the busy_poll run.

* Added a timeout to the tests, so that if a test locks up, we report
  a fail and still get to run all the other tests.

* Made the stats test just look and feel like all the other
  tests. Makes the code simpler and the test reporting more
  consistent. These are the 3 last commits.

* Replaced zero length packets with packets of 64 byte length. This so
  that some of the tests will pass after commit 726e2c5929de84 ("veth:
  Ensure eth header is in skb's linear part").

* Added clean-up of the veth pair when terminating the test run.

* Some smaller clean-ups of unused stuff.

Note, to pass the busy-poll tests commit 8de8b71b787f ("xsk: Fix
l2fwd for copy mode + busy poll combo") need to be present. It is
present in bpf but not yet in bpf-next.

Thanks: Magnus

Magnus Karlsson (9):
  selftests: xsk: cleanup bash scripts
  selftests: xsk: do not send zero-length packets
  selftests: xsk: run all tests for busy-poll
  selftests: xsk: fix reporting of failed tests
  selftests: xsk: add timeout to tests
  selftests: xsk: cleanup veth pair at ctrl-c
  selftests: xsk: introduce validation functions
  selftests: xsk: make the stats tests normal tests
  selftests: xsk: make stat tests not spin on getsockopt

 tools/testing/selftests/bpf/test_xsk.sh    |  53 +-
 tools/testing/selftests/bpf/xdpxceiver.c   | 547 ++++++++++++++-------
 tools/testing/selftests/bpf/xdpxceiver.h   |  42 +-
 tools/testing/selftests/bpf/xsk_prereqs.sh |  47 +-
 4 files changed, 439 insertions(+), 250 deletions(-)


base-commit: 43bf087848ab796fab93c9b4de59a7ed70aab94a
--
2.34.1

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

* [PATCH bpf-next 1/9] selftests: xsk: cleanup bash scripts
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
@ 2022-05-10 11:55 ` Magnus Karlsson
  2022-05-10 11:55 ` [PATCH bpf-next 2/9] selftests: xsk: do not send zero-length packets Magnus Karlsson
                   ` (10 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Magnus Karlsson @ 2022-05-10 11:55 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh
  Cc: jonathan.lemon, bpf

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

Remove the spec-file that is not used any longer from the shell
scripts. Also remove an unused option.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh    |  8 +-------
 tools/testing/selftests/bpf/xsk_prereqs.sh | 11 -----------
 2 files changed, 1 insertion(+), 18 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index cd7bf32e6a17..2bd12c16fbb7 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -43,7 +43,6 @@
 #   ** veth<xxxx> in root namespace
 #   ** veth<yyyy> in af_xdp<xxxx> namespace
 #   ** namespace af_xdp<xxxx>
-#   * create a spec file veth.spec that includes this run-time configuration
 #   *** xxxx and yyyy are randomly generated 4 digit numbers used to avoid
 #       conflict with any existing interface
 #   * tests the veth and xsk layers of the topology
@@ -77,7 +76,7 @@
 
 . xsk_prereqs.sh
 
-while getopts "cvD" flag
+while getopts "vD" flag
 do
 	case "${flag}" in
 		v) verbose=1;;
@@ -130,12 +129,7 @@ if [ $retval -ne 0 ]; then
 	exit $retval
 fi
 
-echo "${VETH0}:${VETH1},${NS1}" > ${SPECFILE}
-
-validate_veth_spec_file
-
 if [[ $verbose -eq 1 ]]; then
-        echo "Spec file created: ${SPECFILE}"
 	VERBOSE_ARG="-v"
 fi
 
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index bf29d2549bee..7606d59b06bd 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -8,7 +8,6 @@ ksft_xfail=2
 ksft_xpass=3
 ksft_skip=4
 
-SPECFILE=veth.spec
 XSKOBJ=xdpxceiver
 
 validate_root_exec()
@@ -34,13 +33,6 @@ validate_veth_support()
 	fi
 }
 
-validate_veth_spec_file()
-{
-	if [ ! -f ${SPECFILE} ]; then
-		test_exit $ksft_skip 1
-	fi
-}
-
 test_status()
 {
 	statusval=$1
@@ -74,9 +66,6 @@ clear_configs()
 	#veth node inside NS won't get removed so we explicitly remove it
 	[ $(ip link show $1 &>/dev/null; echo $?;) == 0 ] &&
 		{ ip link del $1; }
-	if [ -f ${SPECFILE} ]; then
-		rm -f ${SPECFILE}
-	fi
 }
 
 cleanup_exit()
-- 
2.34.1


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

* [PATCH bpf-next 2/9] selftests: xsk: do not send zero-length packets
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
  2022-05-10 11:55 ` [PATCH bpf-next 1/9] selftests: xsk: cleanup bash scripts Magnus Karlsson
@ 2022-05-10 11:55 ` Magnus Karlsson
  2022-05-10 11:55 ` [PATCH bpf-next 3/9] selftests: xsk: run all tests for busy-poll Magnus Karlsson
                   ` (9 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Magnus Karlsson @ 2022-05-10 11:55 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh
  Cc: jonathan.lemon, bpf

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

Do not try to send packets of zero length since they are dropped by
veth after commit 726e2c5929de84 ("veth: Ensure eth header is in skb's
linear part"). Replace these two packets with packets of length 60 so
that they are not dropped.

Also clean up the confusing naming. MIN_PKT_SIZE was really
MIN_ETH_PKT_SIZE and PKT_SIZE was both MIN_ETH_SIZE and the default
packet size called just PKT_SIZE. Make it consistent by using the
right define in the right place.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 14 +++++++-------
 tools/testing/selftests/bpf/xdpxceiver.h |  5 +++--
 2 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index cfcb031323c5..218f20f135c9 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -575,7 +575,7 @@ static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
 
 	if (!pkt)
 		return NULL;
-	if (!pkt->valid || pkt->len < PKT_SIZE)
+	if (!pkt->valid || pkt->len < MIN_PKT_SIZE)
 		return pkt;
 
 	data = xsk_umem__get_data(ifobject->umem->buffer, pkt->addr);
@@ -677,8 +677,8 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len)
 		return false;
 	}
 
-	if (len < PKT_SIZE) {
-		/*Do not try to verify packets that are smaller than minimum size. */
+	if (len < MIN_PKT_SIZE || pkt->len < MIN_PKT_SIZE) {
+		/* Do not try to verify packets that are smaller than minimum size. */
 		return true;
 	}
 
@@ -1282,10 +1282,10 @@ static void testapp_single_pkt(struct test_spec *test)
 static void testapp_invalid_desc(struct test_spec *test)
 {
 	struct pkt pkts[] = {
-		/* Zero packet length at address zero allowed */
-		{0, 0, 0, true},
-		/* Zero packet length allowed */
-		{0x1000, 0, 0, true},
+		/* Zero packet address allowed */
+		{0, PKT_SIZE, 0, true},
+		/* Allowed packet */
+		{0x1000, PKT_SIZE, 0, true},
 		/* Straddling the start of umem */
 		{-2, PKT_SIZE, 0, false},
 		/* Packet too large */
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 62a3e6388632..37ab549ce5fe 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -25,9 +25,10 @@
 #define MAX_TEARDOWN_ITER 10
 #define PKT_HDR_SIZE (sizeof(struct ethhdr) + sizeof(struct iphdr) + \
 			sizeof(struct udphdr))
-#define MIN_PKT_SIZE 64
+#define MIN_ETH_PKT_SIZE 64
 #define ETH_FCS_SIZE 4
-#define PKT_SIZE (MIN_PKT_SIZE - ETH_FCS_SIZE)
+#define MIN_PKT_SIZE (MIN_ETH_PKT_SIZE - ETH_FCS_SIZE)
+#define PKT_SIZE (MIN_PKT_SIZE)
 #define IP_PKT_SIZE (PKT_SIZE - sizeof(struct ethhdr))
 #define IP_PKT_VER 0x4
 #define IP_PKT_TOS 0x9
-- 
2.34.1


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

* [PATCH bpf-next 3/9] selftests: xsk: run all tests for busy-poll
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
  2022-05-10 11:55 ` [PATCH bpf-next 1/9] selftests: xsk: cleanup bash scripts Magnus Karlsson
  2022-05-10 11:55 ` [PATCH bpf-next 2/9] selftests: xsk: do not send zero-length packets Magnus Karlsson
@ 2022-05-10 11:55 ` Magnus Karlsson
  2022-05-10 11:55 ` [PATCH bpf-next 4/9] selftests: xsk: fix reporting of failed tests Magnus Karlsson
                   ` (8 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Magnus Karlsson @ 2022-05-10 11:55 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh
  Cc: jonathan.lemon, bpf

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

Execute all xsk selftests for busy-poll mode too. Currently they were
only run for the standard interrupt driven softirq mode. Replace the
unused option queue-id with the new option busy-poll.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh    | 25 +++++++-
 tools/testing/selftests/bpf/xdpxceiver.c   | 68 ++++++++++++++++++----
 tools/testing/selftests/bpf/xdpxceiver.h   |  9 +++
 tools/testing/selftests/bpf/xsk_prereqs.sh |  6 +-
 4 files changed, 94 insertions(+), 14 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 2bd12c16fbb7..7989a9376f0e 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -109,6 +109,14 @@ setup_vethPairs() {
 	if [[ $verbose -eq 1 ]]; then
 	        echo "setting up ${VETH1}: namespace: ${NS1}"
 	fi
+
+	if [[ $busy_poll -eq 1 ]]; then
+	        echo 2 > /sys/class/net/${VETH0}/napi_defer_hard_irqs
+		echo 200000 > /sys/class/net/${VETH0}/gro_flush_timeout
+		echo 2 > /sys/class/net/${VETH1}/napi_defer_hard_irqs
+		echo 200000 > /sys/class/net/${VETH1}/gro_flush_timeout
+	fi
+
 	ip link set ${VETH1} netns ${NS1}
 	ip netns exec ${NS1} ip link set ${VETH1} mtu ${MTU}
 	ip link set ${VETH0} mtu ${MTU}
@@ -130,11 +138,11 @@ if [ $retval -ne 0 ]; then
 fi
 
 if [[ $verbose -eq 1 ]]; then
-	VERBOSE_ARG="-v"
+	ARGS+="-v "
 fi
 
 if [[ $dump_pkts -eq 1 ]]; then
-	DUMP_PKTS_ARG="-D"
+	ARGS="-D "
 fi
 
 test_status $retval "${TEST_NAME}"
@@ -143,8 +151,19 @@ test_status $retval "${TEST_NAME}"
 
 statusList=()
 
-TEST_NAME="XSK KSELFTESTS"
+TEST_NAME="XSK_SELFTESTS_SOFTIRQ"
+
+execxdpxceiver
+
+retval=$?
+test_status $retval "${TEST_NAME}"
+statusList+=($retval)
 
+cleanup_exit ${VETH0} ${VETH1} ${NS1}
+TEST_NAME="XSK_SELFTESTS_BUSY_POLL"
+busy_poll=1
+
+setup_vethPairs
 execxdpxceiver
 
 retval=$?
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 218f20f135c9..6efac9e35c2e 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -90,6 +90,7 @@
 #include <string.h>
 #include <stddef.h>
 #include <sys/mman.h>
+#include <sys/socket.h>
 #include <sys/types.h>
 #include <sys/queue.h>
 #include <time.h>
@@ -122,9 +123,11 @@ static void __exit_with_error(int error, const char *file, const char *func, int
 #define exit_with_error(error) __exit_with_error(error, __FILE__, __func__, __LINE__)
 
 #define mode_string(test) (test)->ifobj_tx->xdp_flags & XDP_FLAGS_SKB_MODE ? "SKB" : "DRV"
+#define busy_poll_string(test) (test)->ifobj_tx->busy_poll ? "BUSY-POLL " : ""
 
 #define print_ksft_result(test)						\
-	(ksft_test_result_pass("PASS: %s %s\n", mode_string(test), (test)->name))
+	(ksft_test_result_pass("PASS: %s %s%s\n", mode_string(test), busy_poll_string(test), \
+			       (test)->name))
 
 static void memset32_htonl(void *dest, u32 val, u32 size)
 {
@@ -264,6 +267,26 @@ static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size
 	return 0;
 }
 
+static void enable_busy_poll(struct xsk_socket_info *xsk)
+{
+	int sock_opt;
+
+	sock_opt = 1;
+	if (setsockopt(xsk_socket__fd(xsk->xsk), SOL_SOCKET, SO_PREFER_BUSY_POLL,
+		       (void *)&sock_opt, sizeof(sock_opt)) < 0)
+		exit_with_error(errno);
+
+	sock_opt = 20;
+	if (setsockopt(xsk_socket__fd(xsk->xsk), SOL_SOCKET, SO_BUSY_POLL,
+		       (void *)&sock_opt, sizeof(sock_opt)) < 0)
+		exit_with_error(errno);
+
+	sock_opt = BATCH_SIZE;
+	if (setsockopt(xsk_socket__fd(xsk->xsk), SOL_SOCKET, SO_BUSY_POLL_BUDGET,
+		       (void *)&sock_opt, sizeof(sock_opt)) < 0)
+		exit_with_error(errno);
+}
+
 static int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_info *umem,
 				struct ifobject *ifobject, bool shared)
 {
@@ -287,8 +310,8 @@ static int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_inf
 
 static struct option long_options[] = {
 	{"interface", required_argument, 0, 'i'},
-	{"queue", optional_argument, 0, 'q'},
-	{"dump-pkts", optional_argument, 0, 'D'},
+	{"busy-poll", no_argument, 0, 'b'},
+	{"dump-pkts", no_argument, 0, 'D'},
 	{"verbose", no_argument, 0, 'v'},
 	{0, 0, 0, 0}
 };
@@ -299,9 +322,9 @@ static void usage(const char *prog)
 		"  Usage: %s [OPTIONS]\n"
 		"  Options:\n"
 		"  -i, --interface      Use interface\n"
-		"  -q, --queue=n        Use queue n (default 0)\n"
 		"  -D, --dump-pkts      Dump packets L2 - L5\n"
-		"  -v, --verbose        Verbose output\n";
+		"  -v, --verbose        Verbose output\n"
+		"  -b, --busy-poll      Enable busy poll\n";
 
 	ksft_print_msg(str, prog);
 }
@@ -347,7 +370,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 	for (;;) {
 		char *sptr, *token;
 
-		c = getopt_long(argc, argv, "i:Dv", long_options, &option_index);
+		c = getopt_long(argc, argv, "i:Dvb", long_options, &option_index);
 		if (c == -1)
 			break;
 
@@ -373,6 +396,10 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
 		case 'v':
 			opt_verbose = true;
 			break;
+		case 'b':
+			ifobj_tx->busy_poll = true;
+			ifobj_rx->busy_poll = true;
+			break;
 		default:
 			usage(basename(argv[0]));
 			ksft_exit_xfail();
@@ -716,11 +743,24 @@ static void 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 || errno == ENOBUFS || errno == EAGAIN || errno == EBUSY || errno == ENETDOWN)
+	if (ret >= 0)
+		return;
+	if (errno == ENOBUFS || errno == EAGAIN || errno == EBUSY || errno == ENETDOWN) {
+		usleep(100);
 		return;
+	}
 	exit_with_error(errno);
 }
 
+static void 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);
+}
+
 static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
 {
 	unsigned int rcvd;
@@ -745,15 +785,18 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
 	}
 }
 
-static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *xsk,
-			 struct pollfd *fds)
+static void receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
 {
+	struct pkt_stream *pkt_stream = ifobj->pkt_stream;
 	struct pkt *pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
+	struct xsk_socket_info *xsk = ifobj->xsk;
 	struct xsk_umem_info *umem = xsk->umem;
 	u32 idx_rx = 0, idx_fq = 0, rcvd, i;
 	int ret;
 
 	while (pkt) {
+		kick_rx(xsk);
+
 		rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
 		if (!rcvd) {
 			if (xsk_ring_prod__needs_wakeup(&umem->fq)) {
@@ -890,6 +933,8 @@ static bool rx_stats_are_valid(struct ifobject *ifobject)
 	socklen_t optlen;
 	int err;
 
+	kick_rx(ifobject->xsk);
+
 	optlen = sizeof(stats);
 	err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, &stats, &optlen);
 	if (err) {
@@ -984,6 +1029,9 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
 				exit_with_error(-ret);
 			usleep(USLEEP_MAX);
 		}
+
+		if (ifobject->busy_poll)
+			enable_busy_poll(&ifobject->xsk_arr[i]);
 	}
 
 	ifobject->xsk = &ifobject->xsk_arr[0];
@@ -1083,7 +1131,7 @@ static void *worker_testapp_validate_rx(void *arg)
 		while (!rx_stats_are_valid(ifobject))
 			continue;
 	else
-		receive_pkts(ifobject->pkt_stream, ifobject->xsk, &fds);
+		receive_pkts(ifobject, &fds);
 
 	if (test->total_steps == test->current_step)
 		testapp_cleanup_xsk_res(ifobject);
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 37ab549ce5fe..0eea0e1495ba 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -17,6 +17,14 @@
 #define PF_XDP AF_XDP
 #endif
 
+#ifndef SO_BUSY_POLL_BUDGET
+#define SO_BUSY_POLL_BUDGET 70
+#endif
+
+#ifndef SO_PREFER_BUSY_POLL
+#define SO_PREFER_BUSY_POLL 69
+#endif
+
 #define MAX_INTERFACES 2
 #define MAX_INTERFACE_NAME_CHARS 7
 #define MAX_INTERFACES_NAMESPACE_CHARS 10
@@ -139,6 +147,7 @@ struct ifobject {
 	bool tx_on;
 	bool rx_on;
 	bool use_poll;
+	bool busy_poll;
 	bool pacing_on;
 	u8 dst_mac[ETH_ALEN];
 	u8 src_mac[ETH_ALEN];
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index 7606d59b06bd..8b77d4c78aba 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -80,5 +80,9 @@ validate_ip_utility()
 
 execxdpxceiver()
 {
-	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${VERBOSE_ARG} ${DUMP_PKTS_ARG}
+        if [[ $busy_poll -eq 1 ]]; then
+	        ARGS+="-b "
+	fi
+
+	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${ARGS}
 }
-- 
2.34.1


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

* [PATCH bpf-next 4/9] selftests: xsk: fix reporting of failed tests
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
                   ` (2 preceding siblings ...)
  2022-05-10 11:55 ` [PATCH bpf-next 3/9] selftests: xsk: run all tests for busy-poll Magnus Karlsson
@ 2022-05-10 11:55 ` Magnus Karlsson
  2022-05-10 11:56 ` [PATCH bpf-next 5/9] selftests: xsk: add timeout to tests Magnus Karlsson
                   ` (7 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Magnus Karlsson @ 2022-05-10 11:55 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh
  Cc: jonathan.lemon, bpf

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

Fix the reporting of failed tests as it was broken in several
ways. First, a failed test was reported as both failed and passed
messing up the count. Second, tests were not aborted after a failure
and could generate more "failures" messing up the count even
more. Third, the failure reporting from the application to the shell
script was wrong. It always reported pass. And finally, the handling
of the failures in the launch script was not correct.

Correct all this by propagating the failure up through the function
calls to a calling function that can abort the test. A receiver or
sender thread will mark the new variable in the test spec called fail,
if a test has failed. This is then picked up by the main thread when
everyone else has exited and this is then marked and propagated up to
the calling script.

Also add a summary function in the calling script so that a user
does not have to go through the sub tests to see if something has
failed.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh    |  23 ++-
 tools/testing/selftests/bpf/xdpxceiver.c   | 173 +++++++++++++--------
 tools/testing/selftests/bpf/xdpxceiver.h   |   4 +
 tools/testing/selftests/bpf/xsk_prereqs.sh |  30 ++--
 4 files changed, 141 insertions(+), 89 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 7989a9376f0e..d06215ee843d 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -87,7 +87,7 @@ done
 TEST_NAME="PREREQUISITES"
 
 URANDOM=/dev/urandom
-[ ! -e "${URANDOM}" ] && { echo "${URANDOM} not found. Skipping tests."; test_exit 1 1; }
+[ ! -e "${URANDOM}" ] && { echo "${URANDOM} not found. Skipping tests."; test_exit $ksft_fail; }
 
 VETH0_POSTFIX=$(cat ${URANDOM} | tr -dc '0-9' | fold -w 256 | head -n 1 | head --bytes 4)
 VETH0=ve${VETH0_POSTFIX}
@@ -155,10 +155,6 @@ TEST_NAME="XSK_SELFTESTS_SOFTIRQ"
 
 execxdpxceiver
 
-retval=$?
-test_status $retval "${TEST_NAME}"
-statusList+=($retval)
-
 cleanup_exit ${VETH0} ${VETH1} ${NS1}
 TEST_NAME="XSK_SELFTESTS_BUSY_POLL"
 busy_poll=1
@@ -166,19 +162,20 @@ busy_poll=1
 setup_vethPairs
 execxdpxceiver
 
-retval=$?
-test_status $retval "${TEST_NAME}"
-statusList+=($retval)
-
 ## END TESTS
 
 cleanup_exit ${VETH0} ${VETH1} ${NS1}
 
-for _status in "${statusList[@]}"
+failures=0
+echo -e "\nSummary:"
+for i in "${!statusList[@]}"
 do
-	if [ $_status -ne 0 ]; then
-		test_exit $ksft_fail 0
+	if [ ${statusList[$i]} -ne 0 ]; then
+	        test_status ${statusList[$i]} ${nameList[$i]}
+		failures=1
 	fi
 done
 
-test_exit $ksft_pass 0
+if [ $failures -eq 0 ]; then
+        echo "All tests successful!"
+fi
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 6efac9e35c2e..ebbab8f967c1 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -125,9 +125,15 @@ static void __exit_with_error(int error, const char *file, const char *func, int
 #define mode_string(test) (test)->ifobj_tx->xdp_flags & XDP_FLAGS_SKB_MODE ? "SKB" : "DRV"
 #define busy_poll_string(test) (test)->ifobj_tx->busy_poll ? "BUSY-POLL " : ""
 
-#define print_ksft_result(test)						\
-	(ksft_test_result_pass("PASS: %s %s%s\n", mode_string(test), busy_poll_string(test), \
-			       (test)->name))
+static void report_failure(struct test_spec *test)
+{
+	if (test->fail)
+		return;
+
+	ksft_test_result_fail("FAIL: %s %s%s\n", mode_string(test), busy_poll_string(test),
+			      test->name);
+	test->fail = true;
+}
 
 static void memset32_htonl(void *dest, u32 val, u32 size)
 {
@@ -443,6 +449,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 	test->current_step = 0;
 	test->total_steps = 1;
 	test->nb_sockets = 1;
+	test->fail = false;
 }
 
 static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
@@ -689,8 +696,7 @@ static bool is_offset_correct(struct xsk_umem_info *umem, struct pkt_stream *pkt
 	if (offset == expected_offset)
 		return true;
 
-	ksft_test_result_fail("ERROR: [%s] expected [%u], got [%u]\n", __func__, expected_offset,
-			      offset);
+	ksft_print_msg("[%s] expected [%u], got [%u]\n", __func__, expected_offset, offset);
 	return false;
 }
 
@@ -700,7 +706,7 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len)
 	struct iphdr *iphdr = (struct iphdr *)(data + sizeof(struct ethhdr));
 
 	if (!pkt) {
-		ksft_test_result_fail("ERROR: [%s] too many packets received\n", __func__);
+		ksft_print_msg("[%s] too many packets received\n", __func__);
 		return false;
 	}
 
@@ -710,9 +716,8 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len)
 	}
 
 	if (pkt->len != len) {
-		ksft_test_result_fail
-			("ERROR: [%s] expected length [%d], got length [%d]\n",
-			 __func__, pkt->len, len);
+		ksft_print_msg("[%s] expected length [%d], got length [%d]\n",
+			       __func__, pkt->len, len);
 		return false;
 	}
 
@@ -723,9 +728,8 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len)
 			pkt_dump(data, PKT_SIZE);
 
 		if (pkt->payload != seqnum) {
-			ksft_test_result_fail
-				("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
-					__func__, pkt->payload, seqnum);
+			ksft_print_msg("[%s] expected seqnum [%d], got seqnum [%d]\n",
+				       __func__, pkt->payload, seqnum);
 			return false;
 		}
 	} else {
@@ -761,7 +765,7 @@ static void kick_rx(struct xsk_socket_info *xsk)
 		exit_with_error(errno);
 }
 
-static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
+static int complete_pkts(struct xsk_socket_info *xsk, int batch_size)
 {
 	unsigned int rcvd;
 	u32 idx;
@@ -774,18 +778,19 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
 		if (rcvd > xsk->outstanding_tx) {
 			u64 addr = *xsk_ring_cons__comp_addr(&xsk->umem->cq, idx + rcvd - 1);
 
-			ksft_test_result_fail("ERROR: [%s] Too many packets completed\n",
-					      __func__);
+			ksft_print_msg("[%s] Too many packets completed\n", __func__);
 			ksft_print_msg("Last completion address: %llx\n", addr);
-			return;
+			return TEST_FAILURE;
 		}
 
 		xsk_ring_cons__release(&xsk->umem->cq, rcvd);
 		xsk->outstanding_tx -= rcvd;
 	}
+
+	return TEST_PASS;
 }
 
-static void receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
+static int receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
 {
 	struct pkt_stream *pkt_stream = ifobj->pkt_stream;
 	struct pkt *pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
@@ -824,20 +829,19 @@ static void receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
 			u64 addr = desc->addr, orig;
 
 			if (!pkt) {
-				ksft_test_result_fail("ERROR: [%s] Received too many packets.\n",
-						      __func__);
+				ksft_print_msg("[%s] Received too many packets.\n",
+					       __func__);
 				ksft_print_msg("Last packet has addr: %llx len: %u\n",
 					       addr, desc->len);
-				return;
+				return TEST_FAILURE;
 			}
 
 			orig = xsk_umem__extract_addr(addr);
 			addr = xsk_umem__add_offset_to_addr(addr);
 
-			if (!is_pkt_valid(pkt, umem->buffer, addr, desc->len))
-				return;
-			if (!is_offset_correct(umem, pkt_stream, addr, pkt->addr))
-				return;
+			if (!is_pkt_valid(pkt, umem->buffer, addr, desc->len) ||
+			    !is_offset_correct(umem, pkt_stream, addr, pkt->addr))
+				return TEST_FAILURE;
 
 			*xsk_ring_prod__fill_addr(&umem->fq, idx_fq++) = orig;
 			pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
@@ -852,9 +856,11 @@ static void receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
 			pthread_cond_signal(&pacing_cond);
 		pthread_mutex_unlock(&pacing_mutex);
 	}
+
+	return TEST_PASS;
 }
 
-static u32 __send_pkts(struct ifobject *ifobject, u32 pkt_nb)
+static int __send_pkts(struct ifobject *ifobject, u32 *pkt_nb)
 {
 	struct xsk_socket_info *xsk = ifobject->xsk;
 	u32 i, idx, valid_pkts = 0;
@@ -864,14 +870,14 @@ static u32 __send_pkts(struct ifobject *ifobject, u32 pkt_nb)
 
 	for (i = 0; i < BATCH_SIZE; i++) {
 		struct xdp_desc *tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx + i);
-		struct pkt *pkt = pkt_generate(ifobject, pkt_nb);
+		struct pkt *pkt = pkt_generate(ifobject, *pkt_nb);
 
 		if (!pkt)
 			break;
 
 		tx_desc->addr = pkt->addr;
 		tx_desc->len = pkt->len;
-		pkt_nb++;
+		(*pkt_nb)++;
 		if (pkt->valid)
 			valid_pkts++;
 	}
@@ -886,10 +892,11 @@ static u32 __send_pkts(struct ifobject *ifobject, u32 pkt_nb)
 
 	xsk_ring_prod__submit(&xsk->tx, i);
 	xsk->outstanding_tx += valid_pkts;
-	complete_pkts(xsk, i);
+	if (complete_pkts(xsk, i))
+		return TEST_FAILURE;
 
 	usleep(10);
-	return i;
+	return TEST_PASS;
 }
 
 static void wait_for_tx_completion(struct xsk_socket_info *xsk)
@@ -898,7 +905,7 @@ static void wait_for_tx_completion(struct xsk_socket_info *xsk)
 		complete_pkts(xsk, BATCH_SIZE);
 }
 
-static void send_pkts(struct ifobject *ifobject)
+static int send_pkts(struct test_spec *test, struct ifobject *ifobject)
 {
 	struct pollfd fds = { };
 	u32 pkt_cnt = 0;
@@ -907,6 +914,8 @@ static void send_pkts(struct ifobject *ifobject)
 	fds.events = POLLOUT;
 
 	while (pkt_cnt < ifobject->pkt_stream->nb_pkts) {
+		int err;
+
 		if (ifobject->use_poll) {
 			int ret;
 
@@ -918,13 +927,16 @@ static void send_pkts(struct ifobject *ifobject)
 				continue;
 		}
 
-		pkt_cnt += __send_pkts(ifobject, pkt_cnt);
+		err = __send_pkts(ifobject, &pkt_cnt);
+		if (err || test->fail)
+			return TEST_FAILURE;
 	}
 
 	wait_for_tx_completion(ifobject->xsk);
+	return TEST_PASS;
 }
 
-static bool rx_stats_are_valid(struct ifobject *ifobject)
+static int rx_stats_validate(struct ifobject *ifobject)
 {
 	u32 xsk_stat = 0, expected_stat = ifobject->pkt_stream->nb_pkts;
 	struct xsk_socket *xsk = ifobject->xsk->xsk;
@@ -938,9 +950,9 @@ static bool rx_stats_are_valid(struct ifobject *ifobject)
 	optlen = sizeof(stats);
 	err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, &stats, &optlen);
 	if (err) {
-		ksft_test_result_fail("ERROR Rx: [%s] getsockopt(XDP_STATISTICS) error %u %s\n",
-				      __func__, -err, strerror(-err));
-		return true;
+		ksft_print_msg("[%s] getsockopt(XDP_STATISTICS) error %u %s\n",
+			       __func__, -err, strerror(-err));
+		return TEST_FAILURE;
 	}
 
 	if (optlen == sizeof(struct xdp_statistics)) {
@@ -965,13 +977,13 @@ static bool rx_stats_are_valid(struct ifobject *ifobject)
 		}
 
 		if (xsk_stat == expected_stat)
-			return true;
+			return TEST_PASS;
 	}
 
-	return false;
+	return TEST_CONTINUE;
 }
 
-static void tx_stats_validate(struct ifobject *ifobject)
+static int tx_stats_validate(struct ifobject *ifobject)
 {
 	struct xsk_socket *xsk = ifobject->xsk->xsk;
 	int fd = xsk_socket__fd(xsk);
@@ -982,16 +994,18 @@ static void tx_stats_validate(struct ifobject *ifobject)
 	optlen = sizeof(stats);
 	err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, &stats, &optlen);
 	if (err) {
-		ksft_test_result_fail("ERROR Tx: [%s] getsockopt(XDP_STATISTICS) error %u %s\n",
-				      __func__, -err, strerror(-err));
-		return;
+		ksft_print_msg("[%s] getsockopt(XDP_STATISTICS) error %u %s\n",
+			       __func__, -err, strerror(-err));
+		return TEST_FAILURE;
 	}
 
-	if (stats.tx_invalid_descs == ifobject->pkt_stream->nb_pkts)
-		return;
+	if (stats.tx_invalid_descs != ifobject->pkt_stream->nb_pkts) {
+		ksft_print_msg("[%s] tx_invalid_descs incorrect. Got [%u] expected [%u]\n",
+			       __func__, stats.tx_invalid_descs, ifobject->pkt_stream->nb_pkts);
+		return TEST_FAILURE;
+	}
 
-	ksft_test_result_fail("ERROR: [%s] tx_invalid_descs incorrect. Got [%u] expected [%u]\n",
-			      __func__, stats.tx_invalid_descs, ifobject->pkt_stream->nb_pkts);
+	return TEST_PASS;
 }
 
 static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
@@ -1064,18 +1078,26 @@ static void *worker_testapp_validate_tx(void *arg)
 {
 	struct test_spec *test = (struct test_spec *)arg;
 	struct ifobject *ifobject = test->ifobj_tx;
+	int err;
 
 	if (test->current_step == 1)
 		thread_common_ops(test, ifobject);
 
 	print_verbose("Sending %d packets on interface %s\n", ifobject->pkt_stream->nb_pkts,
 		      ifobject->ifname);
-	send_pkts(ifobject);
+	err = send_pkts(test, ifobject);
+	if (err) {
+		report_failure(test);
+		goto out;
+	}
 
-	if (stat_test_type == STAT_TEST_TX_INVALID)
-		tx_stats_validate(ifobject);
+	if (stat_test_type == STAT_TEST_TX_INVALID) {
+		err = tx_stats_validate(ifobject);
+		report_failure(test);
+	}
 
-	if (test->total_steps == test->current_step)
+out:
+	if (test->total_steps == test->current_step || err)
 		testapp_cleanup_xsk_res(ifobject);
 	pthread_exit(NULL);
 }
@@ -1116,6 +1138,7 @@ static void *worker_testapp_validate_rx(void *arg)
 	struct test_spec *test = (struct test_spec *)arg;
 	struct ifobject *ifobject = test->ifobj_rx;
 	struct pollfd fds = { };
+	int err;
 
 	if (test->current_step == 1)
 		thread_common_ops(test, ifobject);
@@ -1127,18 +1150,27 @@ static void *worker_testapp_validate_rx(void *arg)
 
 	pthread_barrier_wait(&barr);
 
-	if (test_type == TEST_TYPE_STATS)
-		while (!rx_stats_are_valid(ifobject))
-			continue;
-	else
-		receive_pkts(ifobject, &fds);
+	if (test_type == TEST_TYPE_STATS) {
+		do {
+			err = rx_stats_validate(ifobject);
+		} while (err == TEST_CONTINUE);
+	} else {
+		err = receive_pkts(ifobject, &fds);
+	}
+
+	if (err) {
+		report_failure(test);
+		pthread_mutex_lock(&pacing_mutex);
+		pthread_cond_signal(&pacing_cond);
+		pthread_mutex_unlock(&pacing_mutex);
+	}
 
-	if (test->total_steps == test->current_step)
+	if (test->total_steps == test->current_step || err)
 		testapp_cleanup_xsk_res(ifobject);
 	pthread_exit(NULL);
 }
 
-static void testapp_validate_traffic(struct test_spec *test)
+static int testapp_validate_traffic(struct test_spec *test)
 {
 	struct ifobject *ifobj_tx = test->ifobj_tx;
 	struct ifobject *ifobj_rx = test->ifobj_rx;
@@ -1163,6 +1195,8 @@ static void testapp_validate_traffic(struct test_spec *test)
 
 	pthread_join(t1, NULL);
 	pthread_join(t0, NULL);
+
+	return !!test->fail;
 }
 
 static void testapp_teardown(struct test_spec *test)
@@ -1171,7 +1205,8 @@ static void testapp_teardown(struct test_spec *test)
 
 	test_spec_set_name(test, "TEARDOWN");
 	for (i = 0; i < MAX_TEARDOWN_ITER; i++) {
-		testapp_validate_traffic(test);
+		if (testapp_validate_traffic(test))
+			return;
 		test_spec_reset(test);
 	}
 }
@@ -1194,7 +1229,8 @@ static void testapp_bidi(struct test_spec *test)
 	test->ifobj_tx->rx_on = true;
 	test->ifobj_rx->tx_on = true;
 	test->total_steps = 2;
-	testapp_validate_traffic(test);
+	if (testapp_validate_traffic(test))
+		return;
 
 	print_verbose("Switching Tx/Rx vectors\n");
 	swap_directions(&test->ifobj_rx, &test->ifobj_tx);
@@ -1222,7 +1258,8 @@ static void testapp_bpf_res(struct test_spec *test)
 	test_spec_set_name(test, "BPF_RES");
 	test->total_steps = 2;
 	test->nb_sockets = 2;
-	testapp_validate_traffic(test);
+	if (testapp_validate_traffic(test))
+		return;
 
 	swap_xsk_resources(test->ifobj_tx, test->ifobj_rx);
 	testapp_validate_traffic(test);
@@ -1278,6 +1315,9 @@ static void testapp_stats(struct test_spec *test)
 		default:
 			break;
 		}
+
+		if (test->fail)
+			break;
 	}
 
 	/* To only see the whole stat set being completed unless an individual test fails. */
@@ -1458,7 +1498,9 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
 		break;
 	}
 
-	print_ksft_result(test);
+	if (!test->fail)
+		ksft_test_result_pass("PASS: %s %s%s\n", mode_string(test), busy_poll_string(test),
+				      test->name);
 }
 
 static struct ifobject *ifobject_create(void)
@@ -1497,8 +1539,8 @@ int main(int argc, char **argv)
 {
 	struct pkt_stream *pkt_stream_default;
 	struct ifobject *ifobj_tx, *ifobj_rx;
+	u32 i, j, failed_tests = 0;
 	struct test_spec test;
-	u32 i, j;
 
 	/* Use libbpf 1.0 API mode */
 	libbpf_set_strict_mode(LIBBPF_STRICT_ALL);
@@ -1537,12 +1579,17 @@ int main(int argc, char **argv)
 			test_spec_init(&test, ifobj_tx, ifobj_rx, i);
 			run_pkt_test(&test, i, j);
 			usleep(USLEEP_MAX);
+
+			if (test.fail)
+				failed_tests++;
 		}
 
 	pkt_stream_delete(pkt_stream_default);
 	ifobject_delete(ifobj_tx);
 	ifobject_delete(ifobj_rx);
 
-	ksft_exit_pass();
-	return 0;
+	if (failed_tests)
+		ksft_exit_fail();
+	else
+		ksft_exit_pass();
 }
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 0eea0e1495ba..7c6bf5ed594d 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -25,6 +25,9 @@
 #define SO_PREFER_BUSY_POLL 69
 #endif
 
+#define TEST_PASS 0
+#define TEST_FAILURE -1
+#define TEST_CONTINUE -2
 #define MAX_INTERFACES 2
 #define MAX_INTERFACE_NAME_CHARS 7
 #define MAX_INTERFACES_NAMESPACE_CHARS 10
@@ -160,6 +163,7 @@ struct test_spec {
 	u16 total_steps;
 	u16 current_step;
 	u16 nb_sockets;
+	bool fail;
 	char name[MAX_TEST_NAME_SIZE];
 };
 
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index 8b77d4c78aba..684e813803ec 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -15,7 +15,7 @@ validate_root_exec()
 	msg="skip all tests:"
 	if [ $UID != 0 ]; then
 		echo $msg must be run as root >&2
-		test_exit $ksft_fail 2
+		test_exit $ksft_fail
 	else
 		return $ksft_pass
 	fi
@@ -26,7 +26,7 @@ validate_veth_support()
 	msg="skip all tests:"
 	if [ $(ip link add $1 type veth 2>/dev/null; echo $?;) != 0 ]; then
 		echo $msg veth kernel support not available >&2
-		test_exit $ksft_skip 1
+		test_exit $ksft_skip
 	else
 		ip link del $1
 		return $ksft_pass
@@ -36,22 +36,21 @@ validate_veth_support()
 test_status()
 {
 	statusval=$1
-	if [ $statusval -eq 2 ]; then
-		echo -e "$2: [ FAIL ]"
-	elif [ $statusval -eq 1 ]; then
-		echo -e "$2: [ SKIPPED ]"
-	elif [ $statusval -eq 0 ]; then
-		echo -e "$2: [ PASS ]"
+	if [ $statusval -eq $ksft_fail ]; then
+		echo "$2: [ FAIL ]"
+	elif [ $statusval -eq $ksft_skip ]; then
+		echo "$2: [ SKIPPED ]"
+	elif [ $statusval -eq $ksft_pass ]; then
+		echo "$2: [ PASS ]"
 	fi
 }
 
 test_exit()
 {
-	retval=$1
-	if [ $2 -ne 0 ]; then
-		test_status $2 $(basename $0)
+	if [ $1 -ne 0 ]; then
+		test_status $1 $(basename $0)
 	fi
-	exit $retval
+	exit 1
 }
 
 clear_configs()
@@ -75,7 +74,7 @@ cleanup_exit()
 
 validate_ip_utility()
 {
-	[ ! $(type -P ip) ] && { echo "'ip' not found. Skipping tests."; test_exit $ksft_skip 1; }
+	[ ! $(type -P ip) ] && { echo "'ip' not found. Skipping tests."; test_exit $ksft_skip; }
 }
 
 execxdpxceiver()
@@ -85,4 +84,9 @@ execxdpxceiver()
 	fi
 
 	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${ARGS}
+
+	retval=$?
+	test_status $retval "${TEST_NAME}"
+	statusList+=($retval)
+	nameList+=(${TEST_NAME})
 }
-- 
2.34.1


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

* [PATCH bpf-next 5/9] selftests: xsk: add timeout to tests
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
                   ` (3 preceding siblings ...)
  2022-05-10 11:55 ` [PATCH bpf-next 4/9] selftests: xsk: fix reporting of failed tests Magnus Karlsson
@ 2022-05-10 11:56 ` Magnus Karlsson
  2022-05-10 11:56 ` [PATCH bpf-next 6/9] selftests: xsk: cleanup veth pair at ctrl-c Magnus Karlsson
                   ` (6 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Magnus Karlsson @ 2022-05-10 11:56 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh
  Cc: jonathan.lemon, bpf

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

Add a timeout to the tests so that if all packets have not been
received within 3 seconds, fail the ongoing test. Hinders a test from
dead-locking if there is something wrong.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 15 +++++++++++++++
 tools/testing/selftests/bpf/xdpxceiver.h |  1 +
 2 files changed, 16 insertions(+)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index ebbab8f967c1..dc21951a1b0a 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -91,6 +91,7 @@
 #include <stddef.h>
 #include <sys/mman.h>
 #include <sys/socket.h>
+#include <sys/time.h>
 #include <sys/types.h>
 #include <sys/queue.h>
 #include <time.h>
@@ -792,6 +793,7 @@ static int complete_pkts(struct xsk_socket_info *xsk, int batch_size)
 
 static int receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
 {
+	struct timeval tv_end, tv_now, tv_timeout = {RECV_TMOUT, 0};
 	struct pkt_stream *pkt_stream = ifobj->pkt_stream;
 	struct pkt *pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
 	struct xsk_socket_info *xsk = ifobj->xsk;
@@ -799,7 +801,20 @@ static int receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
 	u32 idx_rx = 0, idx_fq = 0, rcvd, i;
 	int ret;
 
+	ret = gettimeofday(&tv_now, NULL);
+	if (ret)
+		exit_with_error(errno);
+	timeradd(&tv_now, &tv_timeout, &tv_end);
+
 	while (pkt) {
+		ret = gettimeofday(&tv_now, NULL);
+		if (ret)
+			exit_with_error(errno);
+		if (timercmp(&tv_now, &tv_end, >)) {
+			ksft_print_msg("ERROR: [%s] Receive loop timed out\n", __func__);
+			return TEST_FAILURE;
+		}
+
 		kick_rx(xsk);
 
 		rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 7c6bf5ed594d..79ba344d2765 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -49,6 +49,7 @@
 #define SOCK_RECONF_CTR 10
 #define BATCH_SIZE 64
 #define POLL_TMOUT 1000
+#define RECV_TMOUT 3
 #define DEFAULT_PKT_CNT (4 * 1024)
 #define DEFAULT_UMEM_BUFFERS (DEFAULT_PKT_CNT / 4)
 #define UMEM_SIZE (DEFAULT_UMEM_BUFFERS * XSK_UMEM__DEFAULT_FRAME_SIZE)
-- 
2.34.1


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

* [PATCH bpf-next 6/9] selftests: xsk: cleanup veth pair at ctrl-c
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
                   ` (4 preceding siblings ...)
  2022-05-10 11:56 ` [PATCH bpf-next 5/9] selftests: xsk: add timeout to tests Magnus Karlsson
@ 2022-05-10 11:56 ` Magnus Karlsson
  2022-05-10 11:56 ` [PATCH bpf-next 7/9] selftests: xsk: introduce validation functions Magnus Karlsson
                   ` (5 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Magnus Karlsson @ 2022-05-10 11:56 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh
  Cc: jonathan.lemon, bpf

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

Remove the veth pair when the tests are aborted by pressing
ctrl-c. Currently in this situation, the veth pair is left on the
system polluting the netdev space.

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

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index d06215ee843d..567500299231 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -97,6 +97,13 @@ NS0=root
 NS1=af_xdp${VETH1_POSTFIX}
 MTU=1500
 
+trap ctrl_c INT
+
+function ctrl_c() {
+        cleanup_exit ${VETH0} ${VETH1} ${NS1}
+	exit 1
+}
+
 setup_vethPairs() {
 	if [[ $verbose -eq 1 ]]; then
 	        echo "setting up ${VETH0}: namespace: ${NS0}"
-- 
2.34.1


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

* [PATCH bpf-next 7/9] selftests: xsk: introduce validation functions
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
                   ` (5 preceding siblings ...)
  2022-05-10 11:56 ` [PATCH bpf-next 6/9] selftests: xsk: cleanup veth pair at ctrl-c Magnus Karlsson
@ 2022-05-10 11:56 ` Magnus Karlsson
  2022-05-10 11:56 ` [PATCH bpf-next 8/9] selftests: xsk: make the stats tests normal tests Magnus Karlsson
                   ` (4 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Magnus Karlsson @ 2022-05-10 11:56 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh
  Cc: jonathan.lemon, bpf

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

Introduce validation functions that can be optionally called by the Rx
and Tx threads. These are then used to replace the Rx and Tx stats
dispatchers. This so that we in the next commit can make the stats
tests proper normal tests and not be some special case, as today.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 117 +++++++++++++++--------
 tools/testing/selftests/bpf/xdpxceiver.h |   3 +
 2 files changed, 82 insertions(+), 38 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index dc21951a1b0a..3eef29cacf94 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -426,6 +426,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 		ifobj->use_poll = false;
 		ifobj->pacing_on = true;
 		ifobj->pkt_stream = test->pkt_stream_default;
+		ifobj->validation_func = NULL;
 
 		if (i == 0) {
 			ifobj->rx_on = false;
@@ -951,54 +952,90 @@ static int send_pkts(struct test_spec *test, struct ifobject *ifobject)
 	return TEST_PASS;
 }
 
-static int rx_stats_validate(struct ifobject *ifobject)
+static int get_xsk_stats(struct xsk_socket *xsk, struct xdp_statistics *stats)
+{
+	int fd = xsk_socket__fd(xsk), err;
+	socklen_t optlen, expected_len;
+
+	optlen = sizeof(*stats);
+	err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, stats, &optlen);
+	if (err) {
+		ksft_print_msg("[%s] getsockopt(XDP_STATISTICS) error %u %s\n",
+			       __func__, -err, strerror(-err));
+		return TEST_FAILURE;
+	}
+
+	expected_len = sizeof(struct xdp_statistics);
+	if (optlen != expected_len) {
+		ksft_print_msg("[%s] getsockopt optlen error. Expected: %u got: %u\n",
+			       __func__, expected_len, optlen);
+		return TEST_FAILURE;
+	}
+
+	return TEST_PASS;
+}
+
+static int validate_rx_dropped(struct ifobject *ifobject)
 {
-	u32 xsk_stat = 0, expected_stat = ifobject->pkt_stream->nb_pkts;
 	struct xsk_socket *xsk = ifobject->xsk->xsk;
-	int fd = xsk_socket__fd(xsk);
 	struct xdp_statistics stats;
-	socklen_t optlen;
 	int err;
 
 	kick_rx(ifobject->xsk);
 
-	optlen = sizeof(stats);
-	err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, &stats, &optlen);
-	if (err) {
-		ksft_print_msg("[%s] getsockopt(XDP_STATISTICS) error %u %s\n",
-			       __func__, -err, strerror(-err));
+	err = get_xsk_stats(xsk, &stats);
+	if (err)
 		return TEST_FAILURE;
-	}
 
-	if (optlen == sizeof(struct xdp_statistics)) {
-		switch (stat_test_type) {
-		case STAT_TEST_RX_DROPPED:
-			xsk_stat = stats.rx_dropped;
-			break;
-		case STAT_TEST_TX_INVALID:
-			return true;
-		case STAT_TEST_RX_FULL:
-			xsk_stat = stats.rx_ring_full;
-			if (ifobject->umem->num_frames < XSK_RING_PROD__DEFAULT_NUM_DESCS)
-				expected_stat = ifobject->umem->num_frames - RX_FULL_RXQSIZE;
-			else
-				expected_stat = XSK_RING_PROD__DEFAULT_NUM_DESCS - RX_FULL_RXQSIZE;
-			break;
-		case STAT_TEST_RX_FILL_EMPTY:
-			xsk_stat = stats.rx_fill_ring_empty_descs;
-			break;
-		default:
-			break;
-		}
+	if (stats.rx_dropped == ifobject->pkt_stream->nb_pkts)
+		return TEST_PASS;
 
-		if (xsk_stat == expected_stat)
-			return TEST_PASS;
-	}
+	return TEST_CONTINUE;
+}
+
+static int validate_rx_full(struct ifobject *ifobject)
+{
+	struct xsk_socket *xsk = ifobject->xsk->xsk;
+	struct xdp_statistics stats;
+	u32 expected_stat;
+	int err;
+
+	kick_rx(ifobject->xsk);
+
+	err = get_xsk_stats(xsk, &stats);
+	if (err)
+		return TEST_FAILURE;
+
+	if (ifobject->umem->num_frames < XSK_RING_PROD__DEFAULT_NUM_DESCS)
+		expected_stat = ifobject->umem->num_frames - RX_FULL_RXQSIZE;
+	else
+		expected_stat = XSK_RING_PROD__DEFAULT_NUM_DESCS - RX_FULL_RXQSIZE;
+
+	if (stats.rx_ring_full == expected_stat)
+		return TEST_PASS;
+
+	return TEST_CONTINUE;
+}
+
+static int validate_fill_empty(struct ifobject *ifobject)
+{
+	struct xsk_socket *xsk = ifobject->xsk->xsk;
+	struct xdp_statistics stats;
+	int err;
+
+	kick_rx(ifobject->xsk);
+
+	err = get_xsk_stats(xsk, &stats);
+	if (err)
+		return TEST_FAILURE;
+
+	if (stats.rx_fill_ring_empty_descs == ifobject->pkt_stream->nb_pkts)
+		return TEST_PASS;
 
 	return TEST_CONTINUE;
 }
 
-static int tx_stats_validate(struct ifobject *ifobject)
+static int validate_tx_invalid_descs(struct ifobject *ifobject)
 {
 	struct xsk_socket *xsk = ifobject->xsk->xsk;
 	int fd = xsk_socket__fd(xsk);
@@ -1106,8 +1143,8 @@ static void *worker_testapp_validate_tx(void *arg)
 		goto out;
 	}
 
-	if (stat_test_type == STAT_TEST_TX_INVALID) {
-		err = tx_stats_validate(ifobject);
+	if (ifobject->validation_func) {
+		err = ifobject->validation_func(ifobject);
 		report_failure(test);
 	}
 
@@ -1165,9 +1202,9 @@ static void *worker_testapp_validate_rx(void *arg)
 
 	pthread_barrier_wait(&barr);
 
-	if (test_type == TEST_TYPE_STATS) {
+	if (ifobject->validation_func) {
 		do {
-			err = rx_stats_validate(ifobject);
+			err = ifobject->validation_func(ifobject);
 		} while (err == TEST_CONTINUE);
 	} else {
 		err = receive_pkts(ifobject, &fds);
@@ -1302,16 +1339,19 @@ static void testapp_stats(struct test_spec *test)
 			test_spec_set_name(test, "STAT_RX_DROPPED");
 			test->ifobj_rx->umem->frame_headroom = test->ifobj_rx->umem->frame_size -
 				XDP_PACKET_HEADROOM - 1;
+			test->ifobj_rx->validation_func = validate_rx_dropped;
 			testapp_validate_traffic(test);
 			break;
 		case STAT_TEST_RX_FULL:
 			test_spec_set_name(test, "STAT_RX_FULL");
 			test->ifobj_rx->xsk->rxqsize = RX_FULL_RXQSIZE;
+			test->ifobj_rx->validation_func = validate_rx_full;
 			testapp_validate_traffic(test);
 			break;
 		case STAT_TEST_TX_INVALID:
 			test_spec_set_name(test, "STAT_TX_INVALID");
 			pkt_stream_replace(test, DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
+			test->ifobj_tx->validation_func = validate_tx_invalid_descs;
 			testapp_validate_traffic(test);
 
 			pkt_stream_restore_default(test);
@@ -1323,6 +1363,7 @@ static void testapp_stats(struct test_spec *test)
 			if (!test->ifobj_rx->pkt_stream)
 				exit_with_error(ENOMEM);
 			test->ifobj_rx->pkt_stream->use_addr_for_fill = true;
+			test->ifobj_rx->validation_func = validate_fill_empty;
 			testapp_validate_traffic(test);
 
 			pkt_stream_restore_default(test);
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 79ba344d2765..f271c7b35a2c 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -130,6 +130,8 @@ struct pkt_stream {
 	bool use_addr_for_fill;
 };
 
+struct ifobject;
+typedef int (*validation_func_t)(struct ifobject *ifobj);
 typedef void *(*thread_func_t)(void *arg);
 
 struct ifobject {
@@ -139,6 +141,7 @@ struct ifobject {
 	struct xsk_socket_info *xsk_arr;
 	struct xsk_umem_info *umem;
 	thread_func_t func_ptr;
+	validation_func_t validation_func;
 	struct pkt_stream *pkt_stream;
 	int ns_fd;
 	int xsk_map_fd;
-- 
2.34.1


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

* [PATCH bpf-next 8/9] selftests: xsk: make the stats tests normal tests
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
                   ` (6 preceding siblings ...)
  2022-05-10 11:56 ` [PATCH bpf-next 7/9] selftests: xsk: introduce validation functions Magnus Karlsson
@ 2022-05-10 11:56 ` Magnus Karlsson
  2022-05-10 11:56 ` [PATCH bpf-next 9/9] selftests: xsk: make stat tests not spin on getsockopt Magnus Karlsson
                   ` (3 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Magnus Karlsson @ 2022-05-10 11:56 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh
  Cc: jonathan.lemon, bpf

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

Make the stats tests look and feel just like normal tests instead of
bunched under the umbrella of TEST_STATS. This means we will always
run each of them even if one fails. Also gets rid of some special case
code.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 106 +++++++++++------------
 tools/testing/selftests/bpf/xdpxceiver.h |  16 +---
 2 files changed, 53 insertions(+), 69 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 3eef29cacf94..a75af0ea19a3 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -1324,60 +1324,48 @@ static void testapp_headroom(struct test_spec *test)
 	testapp_validate_traffic(test);
 }
 
-static void testapp_stats(struct test_spec *test)
+static void testapp_stats_rx_dropped(struct test_spec *test)
 {
-	int i;
+	test_spec_set_name(test, "STAT_RX_DROPPED");
+	test->ifobj_tx->pacing_on = false;
+	test->ifobj_rx->umem->frame_headroom = test->ifobj_rx->umem->frame_size -
+		XDP_PACKET_HEADROOM - 1;
+	test->ifobj_rx->validation_func = validate_rx_dropped;
+	testapp_validate_traffic(test);
+}
 
-	for (i = 0; i < STAT_TEST_TYPE_MAX; i++) {
-		test_spec_reset(test);
-		stat_test_type = i;
-		/* No or few packets will be received so cannot pace packets */
-		test->ifobj_tx->pacing_on = false;
-
-		switch (stat_test_type) {
-		case STAT_TEST_RX_DROPPED:
-			test_spec_set_name(test, "STAT_RX_DROPPED");
-			test->ifobj_rx->umem->frame_headroom = test->ifobj_rx->umem->frame_size -
-				XDP_PACKET_HEADROOM - 1;
-			test->ifobj_rx->validation_func = validate_rx_dropped;
-			testapp_validate_traffic(test);
-			break;
-		case STAT_TEST_RX_FULL:
-			test_spec_set_name(test, "STAT_RX_FULL");
-			test->ifobj_rx->xsk->rxqsize = RX_FULL_RXQSIZE;
-			test->ifobj_rx->validation_func = validate_rx_full;
-			testapp_validate_traffic(test);
-			break;
-		case STAT_TEST_TX_INVALID:
-			test_spec_set_name(test, "STAT_TX_INVALID");
-			pkt_stream_replace(test, DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
-			test->ifobj_tx->validation_func = validate_tx_invalid_descs;
-			testapp_validate_traffic(test);
+static void testapp_stats_tx_invalid_descs(struct test_spec *test)
+{
+	test_spec_set_name(test, "STAT_TX_INVALID");
+	test->ifobj_tx->pacing_on = false;
+	pkt_stream_replace(test, DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
+	test->ifobj_tx->validation_func = validate_tx_invalid_descs;
+	testapp_validate_traffic(test);
 
-			pkt_stream_restore_default(test);
-			break;
-		case STAT_TEST_RX_FILL_EMPTY:
-			test_spec_set_name(test, "STAT_RX_FILL_EMPTY");
-			test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem, 0,
-									 MIN_PKT_SIZE);
-			if (!test->ifobj_rx->pkt_stream)
-				exit_with_error(ENOMEM);
-			test->ifobj_rx->pkt_stream->use_addr_for_fill = true;
-			test->ifobj_rx->validation_func = validate_fill_empty;
-			testapp_validate_traffic(test);
-
-			pkt_stream_restore_default(test);
-			break;
-		default:
-			break;
-		}
+	pkt_stream_restore_default(test);
+}
 
-		if (test->fail)
-			break;
-	}
+static void testapp_stats_rx_full(struct test_spec *test)
+{
+	test_spec_set_name(test, "STAT_RX_FULL");
+	test->ifobj_tx->pacing_on = false;
+	test->ifobj_rx->xsk->rxqsize = RX_FULL_RXQSIZE;
+	test->ifobj_rx->validation_func = validate_rx_full;
+	testapp_validate_traffic(test);
+}
 
-	/* To only see the whole stat set being completed unless an individual test fails. */
-	test_spec_set_name(test, "STATS");
+static void testapp_stats_fill_empty(struct test_spec *test)
+{
+	test_spec_set_name(test, "STAT_RX_FILL_EMPTY");
+	test->ifobj_tx->pacing_on = false;
+	test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem, 0, MIN_PKT_SIZE);
+	if (!test->ifobj_rx->pkt_stream)
+		exit_with_error(ENOMEM);
+	test->ifobj_rx->pkt_stream->use_addr_for_fill = true;
+	test->ifobj_rx->validation_func = validate_fill_empty;
+	testapp_validate_traffic(test);
+
+	pkt_stream_restore_default(test);
 }
 
 /* Simple test */
@@ -1482,14 +1470,18 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
 
 static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
 {
-	test_type = type;
-
-	/* reset defaults after potential previous test */
-	stat_test_type = -1;
-
-	switch (test_type) {
-	case TEST_TYPE_STATS:
-		testapp_stats(test);
+	switch (type) {
+	case TEST_TYPE_STATS_RX_DROPPED:
+		testapp_stats_rx_dropped(test);
+		break;
+	case TEST_TYPE_STATS_TX_INVALID_DESCS:
+		testapp_stats_tx_invalid_descs(test);
+		break;
+	case TEST_TYPE_STATS_RX_FULL:
+		testapp_stats_rx_full(test);
+		break;
+	case TEST_TYPE_STATS_FILL_EMPTY:
+		testapp_stats_fill_empty(test);
 		break;
 	case TEST_TYPE_TEARDOWN:
 		testapp_teardown(test);
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index f271c7b35a2c..bf18a95be48c 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -77,24 +77,16 @@ enum test_type {
 	TEST_TYPE_HEADROOM,
 	TEST_TYPE_TEARDOWN,
 	TEST_TYPE_BIDI,
-	TEST_TYPE_STATS,
+	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_MAX
 };
 
-enum stat_test_type {
-	STAT_TEST_RX_DROPPED,
-	STAT_TEST_TX_INVALID,
-	STAT_TEST_RX_FULL,
-	STAT_TEST_RX_FILL_EMPTY,
-	STAT_TEST_TYPE_MAX
-};
-
 static bool opt_pkt_dump;
-static int test_type;
-
 static bool opt_verbose;
-static int stat_test_type;
 
 struct xsk_umem_info {
 	struct xsk_ring_prod fq;
-- 
2.34.1


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

* [PATCH bpf-next 9/9] selftests: xsk: make stat tests not spin on getsockopt
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
                   ` (7 preceding siblings ...)
  2022-05-10 11:56 ` [PATCH bpf-next 8/9] selftests: xsk: make the stats tests normal tests Magnus Karlsson
@ 2022-05-10 11:56 ` Magnus Karlsson
  2022-05-10 17:07 ` [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Alexei Starovoitov
                   ` (2 subsequent siblings)
  11 siblings, 0 replies; 13+ messages in thread
From: Magnus Karlsson @ 2022-05-10 11:56 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh
  Cc: jonathan.lemon, bpf

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

Convert the stats tests from spinning on the getsockopt to just check
getsockopt once when the Rx thread has received all the packets. The
actual completion of receiving the last packet forms a natural point
in time when the receiver is ready to call the getsockopt to check the
stats. In the previous version , we just span on the getsockopt until
we received the right answer. This could be forever or just getting
the "correct" answer by shear luck.

The pacing_on variable can now be dropped since all test can now
handle pacing properly.

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 170 +++++++++++++----------
 tools/testing/selftests/bpf/xdpxceiver.h |   6 +-
 2 files changed, 99 insertions(+), 77 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index a75af0ea19a3..e5992a6b5e09 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -424,7 +424,8 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 
 		ifobj->xsk = &ifobj->xsk_arr[0];
 		ifobj->use_poll = false;
-		ifobj->pacing_on = true;
+		ifobj->use_fill_ring = true;
+		ifobj->release_rx = true;
 		ifobj->pkt_stream = test->pkt_stream_default;
 		ifobj->validation_func = NULL;
 
@@ -503,9 +504,10 @@ static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
 	return &pkt_stream->pkts[pkt_nb];
 }
 
-static struct pkt *pkt_stream_get_next_rx_pkt(struct pkt_stream *pkt_stream)
+static struct pkt *pkt_stream_get_next_rx_pkt(struct pkt_stream *pkt_stream, u32 *pkts_sent)
 {
 	while (pkt_stream->rx_pkt_nb < pkt_stream->nb_pkts) {
+		(*pkts_sent)++;
 		if (pkt_stream->pkts[pkt_stream->rx_pkt_nb].valid)
 			return &pkt_stream->pkts[pkt_stream->rx_pkt_nb++];
 		pkt_stream->rx_pkt_nb++;
@@ -521,10 +523,16 @@ static void pkt_stream_delete(struct pkt_stream *pkt_stream)
 
 static void pkt_stream_restore_default(struct test_spec *test)
 {
-	if (test->ifobj_tx->pkt_stream != test->pkt_stream_default) {
+	struct pkt_stream *tx_pkt_stream = test->ifobj_tx->pkt_stream;
+
+	if (tx_pkt_stream != test->pkt_stream_default) {
 		pkt_stream_delete(test->ifobj_tx->pkt_stream);
 		test->ifobj_tx->pkt_stream = test->pkt_stream_default;
 	}
+
+	if (test->ifobj_rx->pkt_stream != test->pkt_stream_default &&
+	    test->ifobj_rx->pkt_stream != tx_pkt_stream)
+		pkt_stream_delete(test->ifobj_rx->pkt_stream);
 	test->ifobj_rx->pkt_stream = test->pkt_stream_default;
 }
 
@@ -546,6 +554,16 @@ static struct pkt_stream *__pkt_stream_alloc(u32 nb_pkts)
 	return pkt_stream;
 }
 
+static void pkt_set(struct xsk_umem_info *umem, struct pkt *pkt, u64 addr, u32 len)
+{
+	pkt->addr = addr;
+	pkt->len = len;
+	if (len > umem->frame_size - XDP_PACKET_HEADROOM - MIN_PKT_SIZE * 2 - umem->frame_headroom)
+		pkt->valid = false;
+	else
+		pkt->valid = true;
+}
+
 static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
 {
 	struct pkt_stream *pkt_stream;
@@ -557,14 +575,9 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
 
 	pkt_stream->nb_pkts = nb_pkts;
 	for (i = 0; i < nb_pkts; i++) {
-		pkt_stream->pkts[i].addr = (i % umem->num_frames) * umem->frame_size;
-		pkt_stream->pkts[i].len = pkt_len;
+		pkt_set(umem, &pkt_stream->pkts[i], (i % umem->num_frames) * umem->frame_size,
+			pkt_len);
 		pkt_stream->pkts[i].payload = i;
-
-		if (pkt_len > umem->frame_size)
-			pkt_stream->pkts[i].valid = false;
-		else
-			pkt_stream->pkts[i].valid = true;
 	}
 
 	return pkt_stream;
@@ -592,15 +605,27 @@ static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, int off
 	u32 i;
 
 	pkt_stream = pkt_stream_clone(umem, test->pkt_stream_default);
-	for (i = 1; i < test->pkt_stream_default->nb_pkts; i += 2) {
-		pkt_stream->pkts[i].addr = (i % umem->num_frames) * umem->frame_size + offset;
-		pkt_stream->pkts[i].len = pkt_len;
-	}
+	for (i = 1; i < test->pkt_stream_default->nb_pkts; i += 2)
+		pkt_set(umem, &pkt_stream->pkts[i],
+			(i % umem->num_frames) * umem->frame_size + offset, pkt_len);
 
 	test->ifobj_tx->pkt_stream = pkt_stream;
 	test->ifobj_rx->pkt_stream = pkt_stream;
 }
 
+static void pkt_stream_receive_half(struct test_spec *test)
+{
+	struct xsk_umem_info *umem = test->ifobj_rx->umem;
+	struct pkt_stream *pkt_stream = test->ifobj_tx->pkt_stream;
+	u32 i;
+
+	test->ifobj_rx->pkt_stream = pkt_stream_generate(umem, pkt_stream->nb_pkts,
+							 pkt_stream->pkts[0].len);
+	pkt_stream = test->ifobj_rx->pkt_stream;
+	for (i = 1; i < pkt_stream->nb_pkts; i += 2)
+		pkt_stream->pkts[i].valid = false;
+}
+
 static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
 {
 	struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb);
@@ -795,11 +820,11 @@ static int complete_pkts(struct xsk_socket_info *xsk, int batch_size)
 static int receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
 {
 	struct timeval tv_end, tv_now, tv_timeout = {RECV_TMOUT, 0};
+	u32 idx_rx = 0, idx_fq = 0, rcvd, i, pkts_sent = 0;
 	struct pkt_stream *pkt_stream = ifobj->pkt_stream;
-	struct pkt *pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
 	struct xsk_socket_info *xsk = ifobj->xsk;
 	struct xsk_umem_info *umem = xsk->umem;
-	u32 idx_rx = 0, idx_fq = 0, rcvd, i;
+	struct pkt *pkt;
 	int ret;
 
 	ret = gettimeofday(&tv_now, NULL);
@@ -807,6 +832,7 @@ static int receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
 		exit_with_error(errno);
 	timeradd(&tv_now, &tv_timeout, &tv_end);
 
+	pkt = pkt_stream_get_next_rx_pkt(pkt_stream, &pkts_sent);
 	while (pkt) {
 		ret = gettimeofday(&tv_now, NULL);
 		if (ret)
@@ -828,30 +854,24 @@ static int receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
 			continue;
 		}
 
-		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 (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(-ret);
+				}
+				ret = xsk_ring_prod__reserve(&umem->fq, rcvd, &idx_fq);
 			}
-			ret = xsk_ring_prod__reserve(&umem->fq, rcvd, &idx_fq);
 		}
 
 		for (i = 0; i < rcvd; i++) {
 			const struct xdp_desc *desc = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
 			u64 addr = desc->addr, orig;
 
-			if (!pkt) {
-				ksft_print_msg("[%s] Received too many packets.\n",
-					       __func__);
-				ksft_print_msg("Last packet has addr: %llx len: %u\n",
-					       addr, desc->len);
-				return TEST_FAILURE;
-			}
-
 			orig = xsk_umem__extract_addr(addr);
 			addr = xsk_umem__add_offset_to_addr(addr);
 
@@ -859,18 +879,22 @@ static int receive_pkts(struct ifobject *ifobj, struct pollfd *fds)
 			    !is_offset_correct(umem, pkt_stream, addr, pkt->addr))
 				return TEST_FAILURE;
 
-			*xsk_ring_prod__fill_addr(&umem->fq, idx_fq++) = orig;
-			pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
+			if (ifobj->use_fill_ring)
+				*xsk_ring_prod__fill_addr(&umem->fq, idx_fq++) = orig;
+			pkt = pkt_stream_get_next_rx_pkt(pkt_stream, &pkts_sent);
 		}
 
-		xsk_ring_prod__submit(&umem->fq, rcvd);
-		xsk_ring_cons__release(&xsk->rx, rcvd);
+		if (ifobj->use_fill_ring)
+			xsk_ring_prod__submit(&umem->fq, rcvd);
+		if (ifobj->release_rx)
+			xsk_ring_cons__release(&xsk->rx, rcvd);
 
 		pthread_mutex_lock(&pacing_mutex);
-		pkts_in_flight -= rcvd;
+		pkts_in_flight -= pkts_sent;
 		if (pkts_in_flight < umem->num_frames)
 			pthread_cond_signal(&pacing_cond);
 		pthread_mutex_unlock(&pacing_mutex);
+		pkts_sent = 0;
 	}
 
 	return TEST_PASS;
@@ -900,7 +924,8 @@ static int __send_pkts(struct ifobject *ifobject, u32 *pkt_nb)
 
 	pthread_mutex_lock(&pacing_mutex);
 	pkts_in_flight += valid_pkts;
-	if (ifobject->pacing_on && pkts_in_flight >= ifobject->umem->num_frames - BATCH_SIZE) {
+	/* pkts_in_flight might be negative if many invalid packets are sent */
+	if (pkts_in_flight >= (int)(ifobject->umem->num_frames - BATCH_SIZE)) {
 		kick_tx(xsk);
 		pthread_cond_wait(&pacing_cond, &pacing_mutex);
 	}
@@ -987,34 +1012,29 @@ static int validate_rx_dropped(struct ifobject *ifobject)
 	if (err)
 		return TEST_FAILURE;
 
-	if (stats.rx_dropped == ifobject->pkt_stream->nb_pkts)
+	if (stats.rx_dropped == ifobject->pkt_stream->nb_pkts / 2)
 		return TEST_PASS;
 
-	return TEST_CONTINUE;
+	return TEST_FAILURE;
 }
 
 static int validate_rx_full(struct ifobject *ifobject)
 {
 	struct xsk_socket *xsk = ifobject->xsk->xsk;
 	struct xdp_statistics stats;
-	u32 expected_stat;
 	int err;
 
+	usleep(1000);
 	kick_rx(ifobject->xsk);
 
 	err = get_xsk_stats(xsk, &stats);
 	if (err)
 		return TEST_FAILURE;
 
-	if (ifobject->umem->num_frames < XSK_RING_PROD__DEFAULT_NUM_DESCS)
-		expected_stat = ifobject->umem->num_frames - RX_FULL_RXQSIZE;
-	else
-		expected_stat = XSK_RING_PROD__DEFAULT_NUM_DESCS - RX_FULL_RXQSIZE;
-
-	if (stats.rx_ring_full == expected_stat)
+	if (stats.rx_ring_full)
 		return TEST_PASS;
 
-	return TEST_CONTINUE;
+	return TEST_FAILURE;
 }
 
 static int validate_fill_empty(struct ifobject *ifobject)
@@ -1023,16 +1043,17 @@ static int validate_fill_empty(struct ifobject *ifobject)
 	struct xdp_statistics stats;
 	int err;
 
+	usleep(1000);
 	kick_rx(ifobject->xsk);
 
 	err = get_xsk_stats(xsk, &stats);
 	if (err)
 		return TEST_FAILURE;
 
-	if (stats.rx_fill_ring_empty_descs == ifobject->pkt_stream->nb_pkts)
+	if (stats.rx_fill_ring_empty_descs)
 		return TEST_PASS;
 
-	return TEST_CONTINUE;
+	return TEST_FAILURE;
 }
 
 static int validate_tx_invalid_descs(struct ifobject *ifobject)
@@ -1051,7 +1072,7 @@ static int validate_tx_invalid_descs(struct ifobject *ifobject)
 		return TEST_FAILURE;
 	}
 
-	if (stats.tx_invalid_descs != ifobject->pkt_stream->nb_pkts) {
+	if (stats.tx_invalid_descs != ifobject->pkt_stream->nb_pkts / 2) {
 		ksft_print_msg("[%s] tx_invalid_descs incorrect. Got [%u] expected [%u]\n",
 			       __func__, stats.tx_invalid_descs, ifobject->pkt_stream->nb_pkts);
 		return TEST_FAILURE;
@@ -1138,17 +1159,12 @@ static void *worker_testapp_validate_tx(void *arg)
 	print_verbose("Sending %d packets on interface %s\n", ifobject->pkt_stream->nb_pkts,
 		      ifobject->ifname);
 	err = send_pkts(test, ifobject);
-	if (err) {
-		report_failure(test);
-		goto out;
-	}
 
-	if (ifobject->validation_func) {
+	if (!err && ifobject->validation_func)
 		err = ifobject->validation_func(ifobject);
+	if (err)
 		report_failure(test);
-	}
 
-out:
 	if (test->total_steps == test->current_step || err)
 		testapp_cleanup_xsk_res(ifobject);
 	pthread_exit(NULL);
@@ -1202,14 +1218,10 @@ static void *worker_testapp_validate_rx(void *arg)
 
 	pthread_barrier_wait(&barr);
 
-	if (ifobject->validation_func) {
-		do {
-			err = ifobject->validation_func(ifobject);
-		} while (err == TEST_CONTINUE);
-	} else {
-		err = receive_pkts(ifobject, &fds);
-	}
+	err = receive_pkts(ifobject, &fds);
 
+	if (!err && ifobject->validation_func)
+		err = ifobject->validation_func(ifobject);
 	if (err) {
 		report_failure(test);
 		pthread_mutex_lock(&pacing_mutex);
@@ -1327,9 +1339,10 @@ static void testapp_headroom(struct test_spec *test)
 static void testapp_stats_rx_dropped(struct test_spec *test)
 {
 	test_spec_set_name(test, "STAT_RX_DROPPED");
-	test->ifobj_tx->pacing_on = false;
 	test->ifobj_rx->umem->frame_headroom = test->ifobj_rx->umem->frame_size -
-		XDP_PACKET_HEADROOM - 1;
+		XDP_PACKET_HEADROOM - MIN_PKT_SIZE * 3;
+	pkt_stream_replace_half(test, MIN_PKT_SIZE * 4, 0);
+	pkt_stream_receive_half(test);
 	test->ifobj_rx->validation_func = validate_rx_dropped;
 	testapp_validate_traffic(test);
 }
@@ -1337,8 +1350,7 @@ static void testapp_stats_rx_dropped(struct test_spec *test)
 static void testapp_stats_tx_invalid_descs(struct test_spec *test)
 {
 	test_spec_set_name(test, "STAT_TX_INVALID");
-	test->ifobj_tx->pacing_on = false;
-	pkt_stream_replace(test, DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
+	pkt_stream_replace_half(test, XSK_UMEM__INVALID_FRAME_SIZE, 0);
 	test->ifobj_tx->validation_func = validate_tx_invalid_descs;
 	testapp_validate_traffic(test);
 
@@ -1348,20 +1360,30 @@ static void testapp_stats_tx_invalid_descs(struct test_spec *test)
 static void testapp_stats_rx_full(struct test_spec *test)
 {
 	test_spec_set_name(test, "STAT_RX_FULL");
-	test->ifobj_tx->pacing_on = false;
-	test->ifobj_rx->xsk->rxqsize = RX_FULL_RXQSIZE;
+	pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, PKT_SIZE);
+	test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
+							 DEFAULT_UMEM_BUFFERS, PKT_SIZE);
+	if (!test->ifobj_rx->pkt_stream)
+		exit_with_error(ENOMEM);
+
+	test->ifobj_rx->xsk->rxqsize = DEFAULT_UMEM_BUFFERS;
+	test->ifobj_rx->release_rx = false;
 	test->ifobj_rx->validation_func = validate_rx_full;
 	testapp_validate_traffic(test);
+
+	pkt_stream_restore_default(test);
 }
 
 static void testapp_stats_fill_empty(struct test_spec *test)
 {
 	test_spec_set_name(test, "STAT_RX_FILL_EMPTY");
-	test->ifobj_tx->pacing_on = false;
-	test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem, 0, MIN_PKT_SIZE);
+	pkt_stream_replace(test, DEFAULT_UMEM_BUFFERS + DEFAULT_UMEM_BUFFERS / 2, PKT_SIZE);
+	test->ifobj_rx->pkt_stream = pkt_stream_generate(test->ifobj_rx->umem,
+							 DEFAULT_UMEM_BUFFERS, PKT_SIZE);
 	if (!test->ifobj_rx->pkt_stream)
 		exit_with_error(ENOMEM);
-	test->ifobj_rx->pkt_stream->use_addr_for_fill = true;
+
+	test->ifobj_rx->use_fill_ring = false;
 	test->ifobj_rx->validation_func = validate_fill_empty;
 	testapp_validate_traffic(test);
 
@@ -1504,7 +1526,7 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
 		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);
+		pkt_stream_replace(test, DEFAULT_PKT_CNT, PKT_SIZE);
 		testapp_validate_traffic(test);
 
 		pkt_stream_restore_default(test);
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index bf18a95be48c..8f672b0fe0e1 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -27,7 +27,6 @@
 
 #define TEST_PASS 0
 #define TEST_FAILURE -1
-#define TEST_CONTINUE -2
 #define MAX_INTERFACES 2
 #define MAX_INTERFACE_NAME_CHARS 7
 #define MAX_INTERFACES_NAMESPACE_CHARS 10
@@ -147,7 +146,8 @@ struct ifobject {
 	bool rx_on;
 	bool use_poll;
 	bool busy_poll;
-	bool pacing_on;
+	bool use_fill_ring;
+	bool release_rx;
 	u8 dst_mac[ETH_ALEN];
 	u8 src_mac[ETH_ALEN];
 };
@@ -167,6 +167,6 @@ pthread_barrier_t barr;
 pthread_mutex_t pacing_mutex = PTHREAD_MUTEX_INITIALIZER;
 pthread_cond_t pacing_cond = PTHREAD_COND_INITIALIZER;
 
-u32 pkts_in_flight;
+int pkts_in_flight;
 
 #endif				/* XDPXCEIVER_H */
-- 
2.34.1


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

* Re: [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
                   ` (8 preceding siblings ...)
  2022-05-10 11:56 ` [PATCH bpf-next 9/9] selftests: xsk: make stat tests not spin on getsockopt Magnus Karlsson
@ 2022-05-10 17:07 ` Alexei Starovoitov
  2022-05-11 14:08 ` Björn Töpel
  2022-05-11 15:10 ` patchwork-bot+netdevbpf
  11 siblings, 0 replies; 13+ messages in thread
From: Alexei Starovoitov @ 2022-05-10 17:07 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh,
	jonathan.lemon, bpf

On Tue, May 10, 2022 at 01:55:55PM +0200, Magnus Karlsson wrote:
> This patch set adds busy-poll testing to the xsk selftests. It runs
> exactly the same tests as with regular softirq processing, but with
> busy-poll enabled. I have also included a number of fixes to the
> selftests that have been bugging me for a while or was discovered
> while implementing the busy-poll support. In summary these are:
> 
> * Fix the error reporting of failed tests. Each failed test used to be
>   reported as both failed and passed, messing up things.
> 
> * Added a summary test printout at the end of the test suite so that
>   users do not have to scroll up and look at the result of both the
>   softirq run and the busy_poll run.
> 
> * Added a timeout to the tests, so that if a test locks up, we report
>   a fail and still get to run all the other tests.
> 
> * Made the stats test just look and feel like all the other
>   tests. Makes the code simpler and the test reporting more
>   consistent. These are the 3 last commits.
> 
> * Replaced zero length packets with packets of 64 byte length. This so
>   that some of the tests will pass after commit 726e2c5929de84 ("veth:
>   Ensure eth header is in skb's linear part").
> 
> * Added clean-up of the veth pair when terminating the test run.
> 
> * Some smaller clean-ups of unused stuff.

Sounds like a good set of improvements and fixes.

Bjorn,
please review.

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

* Re: [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
                   ` (9 preceding siblings ...)
  2022-05-10 17:07 ` [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Alexei Starovoitov
@ 2022-05-11 14:08 ` Björn Töpel
  2022-05-11 15:10 ` patchwork-bot+netdevbpf
  11 siblings, 0 replies; 13+ messages in thread
From: Björn Töpel @ 2022-05-11 14:08 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: Karlsson, Magnus, Alexei Starovoitov, Daniel Borkmann, Netdev,
	Fijalkowski, Maciej, Yonghong Song, Andrii Nakryiko,
	Martin KaFai Lau, Song Liu, John Fastabend, KP Singh,
	Jonathan Lemon, bpf

On Tue, 10 May 2022 at 13:56, Magnus Karlsson <magnus.karlsson@gmail.com> wrote:
>
> This patch set adds busy-poll testing to the xsk selftests. It runs
> exactly the same tests as with regular softirq processing, but with
> busy-poll enabled. I have also included a number of fixes to the
> selftests that have been bugging me for a while or was discovered
> while implementing the busy-poll support. In summary these are:
>

Nice additions, and nice cleanups. Awesome, Magnus!

Acked-by: Björn Töpel <bjorn@kernel.org>

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

* Re: [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes
  2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
                   ` (10 preceding siblings ...)
  2022-05-11 14:08 ` Björn Töpel
@ 2022-05-11 15:10 ` patchwork-bot+netdevbpf
  11 siblings, 0 replies; 13+ messages in thread
From: patchwork-bot+netdevbpf @ 2022-05-11 15:10 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
	yhs, andrii, kafai, songliubraving, john.fastabend, kpsingh,
	jonathan.lemon, bpf

Hello:

This series was applied to bpf/bpf-next.git (master)
by Alexei Starovoitov <ast@kernel.org>:

On Tue, 10 May 2022 13:55:55 +0200 you wrote:
> This patch set adds busy-poll testing to the xsk selftests. It runs
> exactly the same tests as with regular softirq processing, but with
> busy-poll enabled. I have also included a number of fixes to the
> selftests that have been bugging me for a while or was discovered
> while implementing the busy-poll support. In summary these are:
> 
> * Fix the error reporting of failed tests. Each failed test used to be
>   reported as both failed and passed, messing up things.
> 
> [...]

Here is the summary with links:
  - [bpf-next,1/9] selftests: xsk: cleanup bash scripts
    https://git.kernel.org/bpf/bpf-next/c/685e64a3c91d
  - [bpf-next,2/9] selftests: xsk: do not send zero-length packets
    https://git.kernel.org/bpf/bpf-next/c/f3e619bb34d3
  - [bpf-next,3/9] selftests: xsk: run all tests for busy-poll
    https://git.kernel.org/bpf/bpf-next/c/f90062b53229
  - [bpf-next,4/9] selftests: xsk: fix reporting of failed tests
    https://git.kernel.org/bpf/bpf-next/c/895b62eed2ab
  - [bpf-next,5/9] selftests: xsk: add timeout to tests
    https://git.kernel.org/bpf/bpf-next/c/db1bd7a99454
  - [bpf-next,6/9] selftests: xsk: cleanup veth pair at ctrl-c
    https://git.kernel.org/bpf/bpf-next/c/d41cb6c47474
  - [bpf-next,7/9] selftests: xsk: introduce validation functions
    https://git.kernel.org/bpf/bpf-next/c/76c576638f5d
  - [bpf-next,8/9] selftests: xsk: make the stats tests normal tests
    https://git.kernel.org/bpf/bpf-next/c/4fec7028ffea
  - [bpf-next,9/9] selftests: xsk: make stat tests not spin on getsockopt
    https://git.kernel.org/bpf/bpf-next/c/27e934bec35b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



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

end of thread, other threads:[~2022-05-11 15:10 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-05-10 11:55 [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Magnus Karlsson
2022-05-10 11:55 ` [PATCH bpf-next 1/9] selftests: xsk: cleanup bash scripts Magnus Karlsson
2022-05-10 11:55 ` [PATCH bpf-next 2/9] selftests: xsk: do not send zero-length packets Magnus Karlsson
2022-05-10 11:55 ` [PATCH bpf-next 3/9] selftests: xsk: run all tests for busy-poll Magnus Karlsson
2022-05-10 11:55 ` [PATCH bpf-next 4/9] selftests: xsk: fix reporting of failed tests Magnus Karlsson
2022-05-10 11:56 ` [PATCH bpf-next 5/9] selftests: xsk: add timeout to tests Magnus Karlsson
2022-05-10 11:56 ` [PATCH bpf-next 6/9] selftests: xsk: cleanup veth pair at ctrl-c Magnus Karlsson
2022-05-10 11:56 ` [PATCH bpf-next 7/9] selftests: xsk: introduce validation functions Magnus Karlsson
2022-05-10 11:56 ` [PATCH bpf-next 8/9] selftests: xsk: make the stats tests normal tests Magnus Karlsson
2022-05-10 11:56 ` [PATCH bpf-next 9/9] selftests: xsk: make stat tests not spin on getsockopt Magnus Karlsson
2022-05-10 17:07 ` [PATCH bpf-next 0/9] selftests: xsk: add busy-poll testing plus various fixes Alexei Starovoitov
2022-05-11 14:08 ` Björn Töpel
2022-05-11 15:10 ` patchwork-bot+netdevbpf

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