netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf-next 0/4] selftests/bpf: xsk improvements and new stats tests
@ 2021-02-17 16:02 Ciara Loftus
  2021-02-17 16:02 ` [PATCH bpf-next 1/4] selftest/bpf: make xsk tests less verbose Ciara Loftus
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Ciara Loftus @ 2021-02-17 16:02 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua; +Cc: Ciara Loftus

This series attempts to improve the xsk selftest framework by:
1. making the default output less verbose
2. adding an optional verbose flag to both the test_xsk.sh script and xdpxceiver app.
3. adding a 'debug' flag to the test_xsk.sh script which enables debug mode in the app.
4. changing how tests are launched - now they are launched from the xdpxceiver app
instead of the script.

Once the improvements are made, a new set of tests are added which test the xsk
statistics.

The output of the test script now looks like:

./test_xsk.sh
PREREQUISITES: [ PASS ]
1..10
ok 1 PASS: SKB NOPOLL 
ok 2 PASS: SKB POLL 
ok 3 PASS: SKB NOPOLL Socket Teardown
ok 4 PASS: SKB NOPOLL Bi-directional Sockets
ok 5 PASS: SKB NOPOLL Stats
ok 6 PASS: DRV NOPOLL 
ok 7 PASS: DRV POLL 
ok 8 PASS: DRV NOPOLL Socket Teardown
ok 9 PASS: DRV NOPOLL Bi-directional Sockets
ok 10 PASS: DRV NOPOLL Stats
# Totals: pass:10 fail:0 xfail:0 xpass:0 skip:0 error:0
XSK KSELFTESTS: [ PASS ]

This series applies on commit b646acd5eb48ec49ef90404336d7e8ee502ecd05


Ciara Loftus (3):
  selftests/bpf: expose debug arg to shell script for xsk tests
  selftests/bpf: restructure xsk selftests
  selftests/bpf: introduce xsk statistics tests

Magnus Karlsson (1):
  selftest/bpf: make xsk tests less verbose

 tools/testing/selftests/bpf/test_xsk.sh    | 129 ++------
 tools/testing/selftests/bpf/xdpxceiver.c   | 350 +++++++++++++++------
 tools/testing/selftests/bpf/xdpxceiver.h   |  53 +++-
 tools/testing/selftests/bpf/xsk_prereqs.sh |  30 +-
 4 files changed, 309 insertions(+), 253 deletions(-)

-- 
2.17.1


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

* [PATCH bpf-next 1/4] selftest/bpf: make xsk tests less verbose
  2021-02-17 16:02 [PATCH bpf-next 0/4] selftests/bpf: xsk improvements and new stats tests Ciara Loftus
@ 2021-02-17 16:02 ` Ciara Loftus
  2021-02-22 11:58   ` Maciej Fijalkowski
  2021-02-17 16:02 ` [PATCH bpf-next 2/4] selftests/bpf: expose debug arg to shell script for xsk tests Ciara Loftus
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 13+ messages in thread
From: Ciara Loftus @ 2021-02-17 16:02 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua; +Cc: Ciara Loftus

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

Make the xsk tests less verbose by only printing the
essentials. Currently, it is hard to see if the tests passed or not
due to all the printouts. Move the extra printouts to a verbose
option, if further debugging is needed when a problem arises.

To run the xsk tests with verbose output:
./test_xsk.sh -v

Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh    | 24 ++++++++++++-----
 tools/testing/selftests/bpf/xdpxceiver.c   | 31 +++++++++++++---------
 tools/testing/selftests/bpf/xdpxceiver.h   | 13 +++++----
 tools/testing/selftests/bpf/xsk_prereqs.sh |  9 +++----
 4 files changed, 47 insertions(+), 30 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 88a7483eaae4..91127a5be90d 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -74,10 +74,11 @@
 
 . xsk_prereqs.sh
 
-while getopts c flag
+while getopts "cv" flag
 do
 	case "${flag}" in
 		c) colorconsole=1;;
+		v) verbose=1;;
 	esac
 done
 
@@ -95,13 +96,17 @@ NS1=af_xdp${VETH1_POSTFIX}
 MTU=1500
 
 setup_vethPairs() {
-	echo "setting up ${VETH0}: namespace: ${NS0}"
+	if [[ $verbose -eq 1 ]]; then
+	        echo "setting up ${VETH0}: namespace: ${NS0}"
+	fi
 	ip netns add ${NS1}
 	ip link add ${VETH0} type veth peer name ${VETH1}
 	if [ -f /proc/net/if_inet6 ]; then
 		echo 1 > /proc/sys/net/ipv6/conf/${VETH0}/disable_ipv6
 	fi
-	echo "setting up ${VETH1}: namespace: ${NS1}"
+	if [[ $verbose -eq 1 ]]; then
+	        echo "setting up ${VETH1}: namespace: ${NS1}"
+	fi
 	ip link set ${VETH1} netns ${NS1}
 	ip netns exec ${NS1} ip link set ${VETH1} mtu ${MTU}
 	ip link set ${VETH0} mtu ${MTU}
@@ -125,7 +130,10 @@ echo "${VETH0}:${VETH1},${NS1}" > ${SPECFILE}
 
 validate_veth_spec_file
 
-echo "Spec file created: ${SPECFILE}"
+if [[ $verbose -eq 1 ]]; then
+        echo "Spec file created: ${SPECFILE}"
+	VERBOSE_ARG="-v"
+fi
 
 test_status $retval "${TEST_NAME}"
 
@@ -136,12 +144,16 @@ statusList=()
 ### TEST 1
 TEST_NAME="XSK KSELFTEST FRAMEWORK"
 
-echo "Switching interfaces [${VETH0}, ${VETH1}] to XDP Generic mode"
+if [[ $verbose -eq 1 ]]; then
+        echo "Switching interfaces [${VETH0}, ${VETH1}] to XDP Generic mode"
+fi
 vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
 
 retval=$?
 if [ $retval -eq 0 ]; then
-	echo "Switching interfaces [${VETH0}, ${VETH1}] to XDP Native mode"
+        if [[ $verbose -eq 1 ]]; then
+	        echo "Switching interfaces [${VETH0}, ${VETH1}] to XDP Native mode"
+	fi
 	vethXDPnative ${VETH0} ${VETH1} ${NS1}
 fi
 
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index f4a96d5ff524..8af746c9a6b6 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -341,6 +341,7 @@ static struct option long_options[] = {
 	{"tear-down", no_argument, 0, 'T'},
 	{"bidi", optional_argument, 0, 'B'},
 	{"debug", optional_argument, 0, 'D'},
+	{"verbose", no_argument, 0, 'v'},
 	{"tx-pkt-count", optional_argument, 0, 'C'},
 	{0, 0, 0, 0}
 };
@@ -359,6 +360,7 @@ static void usage(const char *prog)
 	    "  -T, --tear-down      Tear down sockets by repeatedly recreating them\n"
 	    "  -B, --bidi           Bi-directional sockets test\n"
 	    "  -D, --debug          Debug mode - dump packets L2 - L5\n"
+	    "  -v, --verbose        Verbose output\n"
 	    "  -C, --tx-pkt-count=n Number of packets to send\n";
 	ksft_print_msg(str, prog);
 }
@@ -392,7 +394,7 @@ static void *nsswitchthread(void *args)
 			ksft_test_result_fail("ERROR: [%s] interface \"%s\" does not exist\n",
 					      __func__, ifdict[targs->idx]->ifname);
 		} else {
-			ksft_print_msg("Interface found: %s\n", ifdict[targs->idx]->ifname);
+			print_verbose("Interface found: %s\n", ifdict[targs->idx]->ifname);
 			targs->retptr = true;
 		}
 	}
@@ -422,7 +424,7 @@ static int validate_interfaces(void)
 			pthread_join(ns_thread, NULL);
 
 			if (targs->retptr)
-				ksft_print_msg("NS switched: %s\n", ifdict[i]->nsname);
+				print_verbose("NS switched: %s\n", ifdict[i]->nsname);
 
 			free(targs);
 		} else {
@@ -432,7 +434,7 @@ static int validate_interfaces(void)
 				    ("ERROR: interface \"%s\" does not exist\n", ifdict[i]->ifname);
 				ret = false;
 			} else {
-				ksft_print_msg("Interface found: %s\n", ifdict[i]->ifname);
+				print_verbose("Interface found: %s\n", ifdict[i]->ifname);
 			}
 		}
 	}
@@ -446,7 +448,7 @@ static void parse_command_line(int argc, char **argv)
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "i:q:pSNcTBDC:", long_options, &option_index);
+		c = getopt_long(argc, argv, "i:q:pSNcTBDC:v", long_options, &option_index);
 
 		if (c == -1)
 			break;
@@ -497,6 +499,9 @@ static void parse_command_line(int argc, char **argv)
 		case 'C':
 			opt_pkt_count = atoi(optarg);
 			break;
+		case 'v':
+			opt_verbose = 1;
+			break;
 		default:
 			usage(basename(argv[0]));
 			ksft_exit_xfail();
@@ -714,7 +719,7 @@ static void worker_pkt_dump(void)
 		int payload = *((uint32_t *)(pkt_buf[iter]->payload + PKT_HDR_SIZE));
 
 		if (payload == EOT) {
-			ksft_print_msg("End-of-transmission frame received\n");
+			print_verbose("End-of-transmission frame received\n");
 			fprintf(stdout, "---------------------------------------\n");
 			break;
 		}
@@ -746,7 +751,7 @@ static void worker_pkt_validate(void)
 			}
 
 			if (payloadseqnum == EOT) {
-				ksft_print_msg("End-of-transmission frame received: PASS\n");
+				print_verbose("End-of-transmission frame received: PASS\n");
 				sigvar = 1;
 				break;
 			}
@@ -836,7 +841,7 @@ static void *worker_testapp_validate(void *arg)
 			usleep(USLEEP_MAX);
 		}
 
-		ksft_print_msg("Interface [%s] vector [Tx]\n", ifobject->ifname);
+		print_verbose("Interface [%s] vector [Tx]\n", ifobject->ifname);
 		for (int i = 0; i < num_frames; i++) {
 			/*send EOT frame */
 			if (i == (num_frames - 1))
@@ -850,7 +855,7 @@ static void *worker_testapp_validate(void *arg)
 			gen_eth_frame(ifobject->umem, i * XSK_UMEM__DEFAULT_FRAME_SIZE);
 		}
 
-		ksft_print_msg("Sending %d packets on interface %s\n",
+		print_verbose("Sending %d packets on interface %s\n",
 			       (opt_pkt_count - 1), ifobject->ifname);
 		tx_only_all(ifobject);
 	} else if (ifobject->fv.vector == rx) {
@@ -860,7 +865,7 @@ static void *worker_testapp_validate(void *arg)
 		if (!bidi_pass)
 			thread_common_ops(ifobject, bufs, &sync_mutex_tx, &spinning_rx);
 
-		ksft_print_msg("Interface [%s] vector [Rx]\n", ifobject->ifname);
+		print_verbose("Interface [%s] vector [Rx]\n", ifobject->ifname);
 		xsk_populate_fill_ring(ifobject->umem);
 
 		TAILQ_INIT(&head);
@@ -890,11 +895,11 @@ static void *worker_testapp_validate(void *arg)
 				break;
 		}
 
-		ksft_print_msg("Received %d packets on interface %s\n",
+		print_verbose("Received %d packets on interface %s\n",
 			       pkt_counter, ifobject->ifname);
 
 		if (opt_teardown)
-			ksft_print_msg("Destroying socket\n");
+			print_verbose("Destroying socket\n");
 	}
 
 	if (!opt_bidi || bidi_pass) {
@@ -914,7 +919,7 @@ static void testapp_validate(void)
 	if (opt_bidi && bidi_pass) {
 		pthread_init_mutex();
 		if (!switching_notify) {
-			ksft_print_msg("Switching Tx/Rx vectors\n");
+			print_verbose("Switching Tx/Rx vectors\n");
 			switching_notify++;
 		}
 	}
@@ -974,7 +979,7 @@ static void testapp_sockets(void)
 		pkt_counter = 0;
 		prev_pkt = -1;
 		sigvar = 0;
-		ksft_print_msg("Creating socket\n");
+		print_verbose("Creating socket\n");
 		testapp_validate();
 		opt_bidi ? bidi_pass++ : bidi_pass;
 	}
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 0e9f9b7e61c2..f66f399dfb2d 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -42,6 +42,8 @@
 #define POLL_TMOUT 1000
 #define NEED_WAKEUP true
 
+#define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); } while (0)
+
 typedef __u32 u32;
 typedef __u16 u16;
 typedef __u8 u8;
@@ -51,11 +53,11 @@ enum TESTS {
 	ORDER_CONTENT_VALIDATE_XDP_DRV = 1,
 };
 
-u8 uut;
-u8 debug_pkt_dump;
-u32 num_frames;
-u8 switching_notify;
-u8 bidi_pass;
+static u8 uut;
+static u8 debug_pkt_dump;
+static u32 num_frames;
+static u8 switching_notify;
+static u8 bidi_pass;
 
 static u32 opt_xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int opt_queue;
@@ -64,6 +66,7 @@ static int opt_poll;
 static int opt_teardown;
 static int opt_bidi;
 static u32 opt_xdp_bind_flags = XDP_USE_NEED_WAKEUP;
+static u8 opt_verbose;
 static u8 pkt_data[XSK_UMEM__DEFAULT_FRAME_SIZE];
 static u32 pkt_counter;
 static u32 prev_pkt = -1;
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index 9d54c4645127..ef8c5b31f4b6 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -82,24 +82,21 @@ clear_configs()
 {
 	if [ $(ip netns show | grep $3 &>/dev/null; echo $?;) == 0 ]; then
 		[ $(ip netns exec $3 ip link show $2 &>/dev/null; echo $?;) == 0 ] &&
-			{ echo "removing link $1:$2"; ip netns exec $3 ip link del $2; }
-		echo "removing ns $3"
+			{ ip netns exec $3 ip link del $2; }
 		ip netns del $3
 	fi
 	#Once we delete a veth pair node, the entire veth pair is removed,
 	#this is just to be cautious just incase the NS does not exist then
 	#veth node inside NS won't get removed so we explicitly remove it
 	[ $(ip link show $1 &>/dev/null; echo $?;) == 0 ] &&
-		{ echo "removing link $1"; ip link del $1; }
+		{ ip link del $1; }
 	if [ -f ${SPECFILE} ]; then
-		echo "removing spec file:" ${SPECFILE}
 		rm -f ${SPECFILE}
 	fi
 }
 
 cleanup_exit()
 {
-	echo "cleaning up..."
 	clear_configs $1 $2 $3
 }
 
@@ -131,5 +128,5 @@ execxdpxceiver()
 			copy[$index]=${!current}
 		done
 
-	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]} -C ${NUMPKTS}
+	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]} -C ${NUMPKTS} ${VERBOSE_ARG}
 }
-- 
2.17.1


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

* [PATCH bpf-next 2/4] selftests/bpf: expose debug arg to shell script for xsk tests
  2021-02-17 16:02 [PATCH bpf-next 0/4] selftests/bpf: xsk improvements and new stats tests Ciara Loftus
  2021-02-17 16:02 ` [PATCH bpf-next 1/4] selftest/bpf: make xsk tests less verbose Ciara Loftus
@ 2021-02-17 16:02 ` Ciara Loftus
  2021-02-19 13:12   ` Magnus Karlsson
  2021-02-17 16:02 ` [PATCH bpf-next 3/4] selftests/bpf: restructure xsk selftests Ciara Loftus
  2021-02-17 16:02 ` [PATCH bpf-next 4/4] selftests/bpf: introduce xsk statistics tests Ciara Loftus
  3 siblings, 1 reply; 13+ messages in thread
From: Ciara Loftus @ 2021-02-17 16:02 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua; +Cc: Ciara Loftus

Launching xdpxceiver with -D enables debug mode. Make it possible
to pass this flag to the app via the test_xsk.sh shell script like
so:

./test_xsk.sh -d

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh    | 7 ++++++-
 tools/testing/selftests/bpf/xsk_prereqs.sh | 3 ++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 91127a5be90d..a72f8ed2932d 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -74,11 +74,12 @@
 
 . xsk_prereqs.sh
 
-while getopts "cv" flag
+while getopts "cvd" flag
 do
 	case "${flag}" in
 		c) colorconsole=1;;
 		v) verbose=1;;
+		d) debug=1;;
 	esac
 done
 
@@ -135,6 +136,10 @@ if [[ $verbose -eq 1 ]]; then
 	VERBOSE_ARG="-v"
 fi
 
+if [[ $debug -eq 1 ]]; then
+	DEBUG_ARG="-D"
+fi
+
 test_status $retval "${TEST_NAME}"
 
 ## START TESTS
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index ef8c5b31f4b6..d95018051fcc 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -128,5 +128,6 @@ execxdpxceiver()
 			copy[$index]=${!current}
 		done
 
-	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]} -C ${NUMPKTS} ${VERBOSE_ARG}
+	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]} -C ${NUMPKTS} ${VERBOSE_ARG} \
+		${DEBUG_ARG}
 }
-- 
2.17.1


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

* [PATCH bpf-next 3/4] selftests/bpf: restructure xsk selftests
  2021-02-17 16:02 [PATCH bpf-next 0/4] selftests/bpf: xsk improvements and new stats tests Ciara Loftus
  2021-02-17 16:02 ` [PATCH bpf-next 1/4] selftest/bpf: make xsk tests less verbose Ciara Loftus
  2021-02-17 16:02 ` [PATCH bpf-next 2/4] selftests/bpf: expose debug arg to shell script for xsk tests Ciara Loftus
@ 2021-02-17 16:02 ` Ciara Loftus
  2021-02-18  9:33   ` Björn Töpel
  2021-02-22 11:23   ` Maciej Fijalkowski
  2021-02-17 16:02 ` [PATCH bpf-next 4/4] selftests/bpf: introduce xsk statistics tests Ciara Loftus
  3 siblings, 2 replies; 13+ messages in thread
From: Ciara Loftus @ 2021-02-17 16:02 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua; +Cc: Ciara Loftus

Prior to this commit individual xsk tests were launched from the
shell script 'test_xsk.sh'. When adding a new test type, two new test
configurations had to be added to this file - one for each of the
supported XDP 'modes' (skb or drv). Should zero copy support be added to
the xsk selftest framework in the future, three new test configurations
would need to be added for each new test type. Each new test type also
typically requires new CLI arguments for the xdpxceiver program.

This commit aims to reduce the overhead of adding new tests, by launching
the test configurations from within the xdpxceiver program itself, using
simple loops. Every test is run every time the C program is executed. Many
of the CLI arguments can be removed as a result.

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 tools/testing/selftests/bpf/test_xsk.sh    | 112 +-----------
 tools/testing/selftests/bpf/xdpxceiver.c   | 199 ++++++++++++---------
 tools/testing/selftests/bpf/xdpxceiver.h   |  27 ++-
 tools/testing/selftests/bpf/xsk_prereqs.sh |  24 +--
 4 files changed, 139 insertions(+), 223 deletions(-)

diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index a72f8ed2932d..ae2936e0208f 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -146,117 +146,9 @@ test_status $retval "${TEST_NAME}"
 
 statusList=()
 
-### TEST 1
-TEST_NAME="XSK KSELFTEST FRAMEWORK"
+TEST_NAME="XSK KSELFTESTS"
 
-if [[ $verbose -eq 1 ]]; then
-        echo "Switching interfaces [${VETH0}, ${VETH1}] to XDP Generic mode"
-fi
-vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
-
-retval=$?
-if [ $retval -eq 0 ]; then
-        if [[ $verbose -eq 1 ]]; then
-	        echo "Switching interfaces [${VETH0}, ${VETH1}] to XDP Native mode"
-	fi
-	vethXDPnative ${VETH0} ${VETH1} ${NS1}
-fi
-
-retval=$?
-test_status $retval "${TEST_NAME}"
-statusList+=($retval)
-
-### TEST 2
-TEST_NAME="SKB NOPOLL"
-
-vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
-
-params=("-S")
-execxdpxceiver params
-
-retval=$?
-test_status $retval "${TEST_NAME}"
-statusList+=($retval)
-
-### TEST 3
-TEST_NAME="SKB POLL"
-
-vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
-
-params=("-S" "-p")
-execxdpxceiver params
-
-retval=$?
-test_status $retval "${TEST_NAME}"
-statusList+=($retval)
-
-### TEST 4
-TEST_NAME="DRV NOPOLL"
-
-vethXDPnative ${VETH0} ${VETH1} ${NS1}
-
-params=("-N")
-execxdpxceiver params
-
-retval=$?
-test_status $retval "${TEST_NAME}"
-statusList+=($retval)
-
-### TEST 5
-TEST_NAME="DRV POLL"
-
-vethXDPnative ${VETH0} ${VETH1} ${NS1}
-
-params=("-N" "-p")
-execxdpxceiver params
-
-retval=$?
-test_status $retval "${TEST_NAME}"
-statusList+=($retval)
-
-### TEST 6
-TEST_NAME="SKB SOCKET TEARDOWN"
-
-vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
-
-params=("-S" "-T")
-execxdpxceiver params
-
-retval=$?
-test_status $retval "${TEST_NAME}"
-statusList+=($retval)
-
-### TEST 7
-TEST_NAME="DRV SOCKET TEARDOWN"
-
-vethXDPnative ${VETH0} ${VETH1} ${NS1}
-
-params=("-N" "-T")
-execxdpxceiver params
-
-retval=$?
-test_status $retval "${TEST_NAME}"
-statusList+=($retval)
-
-### TEST 8
-TEST_NAME="SKB BIDIRECTIONAL SOCKETS"
-
-vethXDPgeneric ${VETH0} ${VETH1} ${NS1}
-
-params=("-S" "-B")
-execxdpxceiver params
-
-retval=$?
-test_status $retval "${TEST_NAME}"
-statusList+=($retval)
-
-### TEST 9
-TEST_NAME="DRV BIDIRECTIONAL SOCKETS"
-
-vethXDPnative ${VETH0} ${VETH1} ${NS1}
-
-params=("-N" "-B")
-execxdpxceiver params
+execxdpxceiver
 
 retval=$?
 test_status $retval "${TEST_NAME}"
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 8af746c9a6b6..7cb4a13597d0 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -18,12 +18,7 @@
  * These selftests test AF_XDP SKB and Native/DRV modes using veth
  * Virtual Ethernet interfaces.
  *
- * The following tests are run:
- *
- * 1. AF_XDP SKB mode
- *    Generic mode XDP is driver independent, used when the driver does
- *    not have support for XDP. Works on any netdevice using sockets and
- *    generic XDP path. XDP hook from netif_receive_skb().
+ * For each mode, the following tests are run:
  *    a. nopoll - soft-irq processing
  *    b. poll - using poll() syscall
  *    c. Socket Teardown
@@ -34,17 +29,6 @@
  *       completion rings on each socket, tx/rx in both directions. Only nopoll
  *       mode is used
  *
- * 2. AF_XDP DRV/Native mode
- *    Works on any netdevice with XDP_REDIRECT support, driver dependent. Processes
- *    packets before SKB allocation. Provides better performance than SKB. Driver
- *    hook available just after DMA of buffer descriptor.
- *    a. nopoll
- *    b. poll
- *    c. Socket Teardown
- *    d. Bi-directional sockets
- *    - Only copy mode is supported because veth does not currently support
- *      zero-copy mode
- *
  * Total tests: 8
  *
  * Flow:
@@ -106,9 +90,10 @@ 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 print_ksft_result(void)\
-	(ksft_test_result_pass("PASS: %s %s %s%s\n", uut ? "DRV" : "SKB", opt_poll ? "POLL" :\
-			       "NOPOLL", opt_teardown ? "Socket Teardown" : "",\
-			       opt_bidi ? "Bi-directional Sockets" : ""))
+	(ksft_test_result_pass("PASS: %s %s %s%s\n", uut ? "DRV" : "SKB",\
+			       test_type == TEST_TYPE_POLL ? "POLL" : "NOPOLL",\
+			       test_type == TEST_TYPE_TEARDOWN ? "Socket Teardown" : "",\
+			       test_type == TEST_TYPE_BIDI ? "Bi-directional Sockets" : ""))
 
 static void pthread_init_mutex(void)
 {
@@ -311,10 +296,10 @@ static int xsk_configure_socket(struct ifobject *ifobject)
 	cfg.rx_size = XSK_RING_CONS__DEFAULT_NUM_DESCS;
 	cfg.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
 	cfg.libbpf_flags = 0;
-	cfg.xdp_flags = opt_xdp_flags;
-	cfg.bind_flags = opt_xdp_bind_flags;
+	cfg.xdp_flags = xdp_flags;
+	cfg.bind_flags = xdp_bind_flags;
 
-	if (!opt_bidi) {
+	if (test_type != TEST_TYPE_BIDI) {
 		rxr = (ifobject->fv.vector == rx) ? &ifobject->xsk->rx : NULL;
 		txr = (ifobject->fv.vector == tx) ? &ifobject->xsk->tx : NULL;
 	} else {
@@ -334,12 +319,6 @@ static int xsk_configure_socket(struct ifobject *ifobject)
 static struct option long_options[] = {
 	{"interface", required_argument, 0, 'i'},
 	{"queue", optional_argument, 0, 'q'},
-	{"poll", no_argument, 0, 'p'},
-	{"xdp-skb", no_argument, 0, 'S'},
-	{"xdp-native", no_argument, 0, 'N'},
-	{"copy", no_argument, 0, 'c'},
-	{"tear-down", no_argument, 0, 'T'},
-	{"bidi", optional_argument, 0, 'B'},
 	{"debug", optional_argument, 0, 'D'},
 	{"verbose", no_argument, 0, 'v'},
 	{"tx-pkt-count", optional_argument, 0, 'C'},
@@ -353,12 +332,6 @@ static void usage(const char *prog)
 	    "  Options:\n"
 	    "  -i, --interface      Use interface\n"
 	    "  -q, --queue=n        Use queue n (default 0)\n"
-	    "  -p, --poll           Use poll syscall\n"
-	    "  -S, --xdp-skb=n      Use XDP SKB mode\n"
-	    "  -N, --xdp-native=n   Enforce XDP DRV (native) mode\n"
-	    "  -c, --copy           Force copy mode\n"
-	    "  -T, --tear-down      Tear down sockets by repeatedly recreating them\n"
-	    "  -B, --bidi           Bi-directional sockets test\n"
 	    "  -D, --debug          Debug mode - dump packets L2 - L5\n"
 	    "  -v, --verbose        Verbose output\n"
 	    "  -C, --tx-pkt-count=n Number of packets to send\n";
@@ -448,7 +421,7 @@ static void parse_command_line(int argc, char **argv)
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "i:q:pSNcTBDC:v", long_options, &option_index);
+		c = getopt_long(argc, argv, "i:q:DC:v", long_options, &option_index);
 
 		if (c == -1)
 			break;
@@ -471,28 +444,6 @@ static void parse_command_line(int argc, char **argv)
 		case 'q':
 			opt_queue = atoi(optarg);
 			break;
-		case 'p':
-			opt_poll = 1;
-			break;
-		case 'S':
-			opt_xdp_flags |= XDP_FLAGS_SKB_MODE;
-			opt_xdp_bind_flags |= XDP_COPY;
-			uut = ORDER_CONTENT_VALIDATE_XDP_SKB;
-			break;
-		case 'N':
-			opt_xdp_flags |= XDP_FLAGS_DRV_MODE;
-			opt_xdp_bind_flags |= XDP_COPY;
-			uut = ORDER_CONTENT_VALIDATE_XDP_DRV;
-			break;
-		case 'c':
-			opt_xdp_bind_flags |= XDP_COPY;
-			break;
-		case 'T':
-			opt_teardown = 1;
-			break;
-		case 'B':
-			opt_bidi = 1;
-			break;
 		case 'D':
 			debug_pkt_dump = 1;
 			break;
@@ -659,7 +610,7 @@ static void tx_only_all(struct ifobject *ifobject)
 	while ((opt_pkt_count && pkt_cnt < opt_pkt_count) || !opt_pkt_count) {
 		int batch_size = get_batch_size(pkt_cnt);
 
-		if (opt_poll) {
+		if (test_type == TEST_TYPE_POLL) {
 			ret = poll(fds, 1, POLL_TMOUT);
 			if (ret <= 0)
 				continue;
@@ -883,7 +834,7 @@ static void *worker_testapp_validate(void *arg)
 		pthread_mutex_unlock(&sync_mutex);
 
 		while (1) {
-			if (opt_poll) {
+			if (test_type == TEST_TYPE_POLL) {
 				ret = poll(fds, 1, POLL_TMOUT);
 				if (ret <= 0)
 					continue;
@@ -898,11 +849,11 @@ static void *worker_testapp_validate(void *arg)
 		print_verbose("Received %d packets on interface %s\n",
 			       pkt_counter, ifobject->ifname);
 
-		if (opt_teardown)
+		if (test_type == TEST_TYPE_TEARDOWN)
 			print_verbose("Destroying socket\n");
 	}
 
-	if (!opt_bidi || bidi_pass) {
+	if ((test_type != TEST_TYPE_BIDI) || bidi_pass) {
 		xsk_socket__delete(ifobject->xsk->xsk);
 		(void)xsk_umem__delete(ifobject->umem->umem);
 	}
@@ -912,11 +863,12 @@ static void *worker_testapp_validate(void *arg)
 static void testapp_validate(void)
 {
 	struct timespec max_wait = { 0, 0 };
+	bool bidi = test_type == TEST_TYPE_BIDI;
 
 	pthread_attr_init(&attr);
 	pthread_attr_setstacksize(&attr, THREAD_STACK);
 
-	if (opt_bidi && bidi_pass) {
+	if ((test_type == TEST_TYPE_BIDI) && bidi_pass) {
 		pthread_init_mutex();
 		if (!switching_notify) {
 			print_verbose("Switching Tx/Rx vectors\n");
@@ -927,10 +879,10 @@ static void testapp_validate(void)
 	pthread_mutex_lock(&sync_mutex);
 
 	/*Spawn RX thread */
-	if (!opt_bidi || !bidi_pass) {
+	if (!bidi || !bidi_pass) {
 		if (pthread_create(&t0, &attr, worker_testapp_validate, ifdict[1]))
 			exit_with_error(errno);
-	} else if (opt_bidi && bidi_pass) {
+	} else if (bidi && bidi_pass) {
 		/*switch Tx/Rx vectors */
 		ifdict[0]->fv.vector = rx;
 		if (pthread_create(&t0, &attr, worker_testapp_validate, ifdict[0]))
@@ -947,10 +899,10 @@ static void testapp_validate(void)
 	pthread_mutex_unlock(&sync_mutex);
 
 	/*Spawn TX thread */
-	if (!opt_bidi || !bidi_pass) {
+	if (!bidi || !bidi_pass) {
 		if (pthread_create(&t1, &attr, worker_testapp_validate, ifdict[0]))
 			exit_with_error(errno);
-	} else if (opt_bidi && bidi_pass) {
+	} else if (bidi && bidi_pass) {
 		/*switch Tx/Rx vectors */
 		ifdict[1]->fv.vector = tx;
 		if (pthread_create(&t1, &attr, worker_testapp_validate, ifdict[1]))
@@ -969,19 +921,20 @@ static void testapp_validate(void)
 		free(pkt_buf);
 	}
 
-	if (!opt_teardown && !opt_bidi)
+	if (!(test_type == TEST_TYPE_TEARDOWN) && !bidi)
 		print_ksft_result();
 }
 
 static void testapp_sockets(void)
 {
-	for (int i = 0; i < (opt_teardown ? MAX_TEARDOWN_ITER : MAX_BIDI_ITER); i++) {
+	for (int i = 0; i < ((test_type == TEST_TYPE_TEARDOWN) ? MAX_TEARDOWN_ITER : MAX_BIDI_ITER);
+	     i++) {
 		pkt_counter = 0;
 		prev_pkt = -1;
 		sigvar = 0;
 		print_verbose("Creating socket\n");
 		testapp_validate();
-		opt_bidi ? bidi_pass++ : bidi_pass;
+		test_type == TEST_TYPE_BIDI ? bidi_pass++ : bidi_pass;
 	}
 
 	print_ksft_result();
@@ -1008,6 +961,94 @@ static void init_iface_config(struct ifaceconfigobj *ifaceconfig)
 	ifdict[1]->src_port = ifaceconfig->dst_port;
 }
 
+static int configure_skb(void)
+{
+
+	char cmd[80];
+
+	snprintf(cmd, sizeof(cmd), "ip link set dev %s xdpdrv off", ifdict[0]->ifname);
+	if (system(cmd)) {
+		ksft_test_result_fail("Failed to configure native mode on iface %s\n",
+						ifdict[0]->ifname);
+		return -1;
+	}
+	snprintf(cmd, sizeof(cmd), "ip netns exec %s ip link set dev %s xdpdrv off",
+					ifdict[1]->nsname, ifdict[1]->ifname);
+	if (system(cmd)) {
+		ksft_test_result_fail("Failed to configure native mode on iface/ns %s\n",
+						ifdict[1]->ifname, ifdict[1]->nsname);
+		return -1;
+	}
+
+	cur_mode = TEST_MODE_SKB;
+
+	return 0;
+
+}
+
+static int configure_drv(void)
+{
+	char cmd[80];
+
+	snprintf(cmd, sizeof(cmd), "ip link set dev %s xdpgeneric off", ifdict[0]->ifname);
+	if (system(cmd)) {
+		ksft_test_result_fail("Failed to configure native mode on iface %s\n",
+						ifdict[0]->ifname);
+		return -1;
+	}
+	snprintf(cmd, sizeof(cmd), "ip netns exec %s ip link set dev %s xdpgeneric off",
+					ifdict[1]->nsname, ifdict[1]->ifname);
+	if (system(cmd)) {
+		ksft_test_result_fail("Failed to configure native mode on iface/ns %s\n",
+						ifdict[1]->ifname, ifdict[1]->nsname);
+		return -1;
+	}
+
+	cur_mode = TEST_MODE_DRV;
+
+	return 0;
+}
+
+static void run_pkt_test(int mode, int type)
+{
+	test_type = type;
+
+	/* reset defaults after potential previous test */
+	xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+	pkt_counter = 0;
+	switching_notify = 0;
+	bidi_pass = 0;
+	prev_pkt = -1;
+	ifdict[0]->fv.vector = tx;
+	ifdict[1]->fv.vector = rx;
+
+	switch (mode) {
+	case (TEST_MODE_SKB):
+		if (cur_mode != TEST_MODE_SKB)
+			configure_skb();
+		xdp_flags |= XDP_FLAGS_SKB_MODE;
+		uut = TEST_MODE_SKB;
+		break;
+	case (TEST_MODE_DRV):
+		if (cur_mode != TEST_MODE_DRV)
+			configure_drv();
+		xdp_flags |= XDP_FLAGS_DRV_MODE;
+		uut = TEST_MODE_DRV;
+		break;
+	default:
+		break;
+	}
+
+	pthread_init_mutex();
+
+	if ((test_type != TEST_TYPE_TEARDOWN) && (test_type != TEST_TYPE_BIDI))
+		testapp_validate();
+	else
+		testapp_sockets();
+
+	pthread_destroy_mutex();
+}
+
 int main(int argc, char **argv)
 {
 	struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
@@ -1021,6 +1062,7 @@ int main(int argc, char **argv)
 	const char *IP2 = "192.168.100.161";
 	u16 UDP_DST_PORT = 2020;
 	u16 UDP_SRC_PORT = 2121;
+	int i, j;
 
 	ifaceconfig = malloc(sizeof(struct ifaceconfigobj));
 	memcpy(ifaceconfig->dst_mac, MAC1, ETH_ALEN);
@@ -1046,24 +1088,19 @@ int main(int argc, char **argv)
 
 	init_iface_config(ifaceconfig);
 
-	pthread_init_mutex();
+	ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
 
-	ksft_set_plan(1);
+	configure_skb();
+	cur_mode = TEST_MODE_SKB;
 
-	if (!opt_teardown && !opt_bidi) {
-		testapp_validate();
-	} else if (opt_teardown && opt_bidi) {
-		ksft_test_result_fail("ERROR: parameters -T and -B cannot be used together\n");
-		ksft_exit_xfail();
-	} else {
-		testapp_sockets();
+	for (i = 0; i < TEST_MODE_MAX; i++) {
+		for (j = 0; j < TEST_TYPE_MAX; j++)
+			run_pkt_test(i, j);
 	}
 
 	for (int i = 0; i < MAX_INTERFACES; i++)
 		free(ifdict[i]);
 
-	pthread_destroy_mutex();
-
 	ksft_exit_pass();
 
 	return 0;
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index f66f399dfb2d..1127a396d5d0 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -48,9 +48,18 @@ typedef __u32 u32;
 typedef __u16 u16;
 typedef __u8 u8;
 
-enum TESTS {
-	ORDER_CONTENT_VALIDATE_XDP_SKB = 0,
-	ORDER_CONTENT_VALIDATE_XDP_DRV = 1,
+enum TEST_MODES {
+	TEST_MODE_SKB,
+	TEST_MODE_DRV,
+	TEST_MODE_MAX
+};
+
+enum TEST_TYPES {
+	TEST_TYPE_NOPOLL,
+	TEST_TYPE_POLL,
+	TEST_TYPE_TEARDOWN,
+	TEST_TYPE_BIDI,
+	TEST_TYPE_MAX
 };
 
 static u8 uut;
@@ -58,18 +67,18 @@ static u8 debug_pkt_dump;
 static u32 num_frames;
 static u8 switching_notify;
 static u8 bidi_pass;
+static int cur_mode;
+static int test_type;
 
-static u32 opt_xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static int opt_queue;
 static int opt_pkt_count;
-static int opt_poll;
-static int opt_teardown;
-static int opt_bidi;
-static u32 opt_xdp_bind_flags = XDP_USE_NEED_WAKEUP;
 static u8 opt_verbose;
+
+static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+static u32 xdp_bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
 static u8 pkt_data[XSK_UMEM__DEFAULT_FRAME_SIZE];
 static u32 pkt_counter;
-static u32 prev_pkt = -1;
+static long prev_pkt = -1;
 static int sigvar;
 
 struct xsk_umem_info {
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index d95018051fcc..2d0fc9b7fe34 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -105,29 +105,7 @@ validate_ip_utility()
 	[ ! $(type -P ip) ] && { echo "'ip' not found. Skipping tests."; test_exit $ksft_skip 1; }
 }
 
-vethXDPgeneric()
-{
-	ip link set dev $1 xdpdrv off
-	ip netns exec $3 ip link set dev $2 xdpdrv off
-}
-
-vethXDPnative()
-{
-	ip link set dev $1 xdpgeneric off
-	ip netns exec $3 ip link set dev $2 xdpgeneric off
-}
-
 execxdpxceiver()
 {
-	local -a 'paramkeys=("${!'"$1"'[@]}")' copy
-	paramkeysstr=${paramkeys[*]}
-
-	for index in $paramkeysstr;
-		do
-			current=$1"[$index]"
-			copy[$index]=${!current}
-		done
-
-	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]} -C ${NUMPKTS} ${VERBOSE_ARG} \
-		${DEBUG_ARG}
+	./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} -C ${NUMPKTS} ${VERBOSE_ARG} ${DEBUG_ARG}
 }
-- 
2.17.1


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

* [PATCH bpf-next 4/4] selftests/bpf: introduce xsk statistics tests
  2021-02-17 16:02 [PATCH bpf-next 0/4] selftests/bpf: xsk improvements and new stats tests Ciara Loftus
                   ` (2 preceding siblings ...)
  2021-02-17 16:02 ` [PATCH bpf-next 3/4] selftests/bpf: restructure xsk selftests Ciara Loftus
@ 2021-02-17 16:02 ` Ciara Loftus
  2021-02-22 11:56   ` Maciej Fijalkowski
  3 siblings, 1 reply; 13+ messages in thread
From: Ciara Loftus @ 2021-02-17 16:02 UTC (permalink / raw)
  To: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua; +Cc: Ciara Loftus

This commit introduces a range of tests to the xsk testsuite
for validating xsk statistics.

A new test type called 'stats' is added. Within it there are
four sub-tests which test the following statistics:
1. rx dropped
2. tx invalid
3. rx ring full
4. fill queue empty

Each test configures a scenario which should trigger the given
error statistic. The test passes if the statistic is successfully
incremented.

Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
---
 tools/testing/selftests/bpf/xdpxceiver.c | 130 ++++++++++++++++++++---
 tools/testing/selftests/bpf/xdpxceiver.h |  13 +++
 2 files changed, 130 insertions(+), 13 deletions(-)

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 7cb4a13597d0..4647c89b2019 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -28,8 +28,11 @@
  *       Configure sockets as bi-directional tx/rx sockets, sets up fill and
  *       completion rings on each socket, tx/rx in both directions. Only nopoll
  *       mode is used
+ *    e. Statistics
+ *       Trigger some error conditions and ensure that the appropriate statistics
+ *       are incremented.
  *
- * Total tests: 8
+ * Total tests: 10
  *
  * Flow:
  * -----
@@ -90,10 +93,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 print_ksft_result(void)\
-	(ksft_test_result_pass("PASS: %s %s %s%s\n", uut ? "DRV" : "SKB",\
+	(ksft_test_result_pass("PASS: %s %s %s%s%s\n", uut ? "DRV" : "SKB",\
 			       test_type == TEST_TYPE_POLL ? "POLL" : "NOPOLL",\
 			       test_type == TEST_TYPE_TEARDOWN ? "Socket Teardown" : "",\
-			       test_type == TEST_TYPE_BIDI ? "Bi-directional Sockets" : ""))
+			       test_type == TEST_TYPE_BIDI ? "Bi-directional Sockets" : "",\
+			       test_type == TEST_TYPE_STATS ? "Stats" : ""))
 
 static void pthread_init_mutex(void)
 {
@@ -255,13 +259,20 @@ static void gen_eth_frame(struct xsk_umem_info *umem, u64 addr)
 static void xsk_configure_umem(struct ifobject *data, void *buffer, u64 size)
 {
 	int ret;
+	struct xsk_umem_config cfg = {
+		.fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
+		.comp_size = XSK_RING_CONS__DEFAULT_NUM_DESCS,
+		.frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE,
+		.frame_headroom = frame_headroom,
+		.flags = XSK_UMEM__DEFAULT_FLAGS
+	};
 
 	data->umem = calloc(1, sizeof(struct xsk_umem_info));
 	if (!data->umem)
 		exit_with_error(errno);
 
 	ret = xsk_umem__create(&data->umem->umem, buffer, size,
-			       &data->umem->fq, &data->umem->cq, NULL);
+			       &data->umem->fq, &data->umem->cq, &cfg);
 	if (ret)
 		exit_with_error(ret);
 
@@ -293,7 +304,7 @@ static int xsk_configure_socket(struct ifobject *ifobject)
 		exit_with_error(errno);
 
 	ifobject->xsk->umem = ifobject->umem;
-	cfg.rx_size = XSK_RING_CONS__DEFAULT_NUM_DESCS;
+	cfg.rx_size = rxqsize;
 	cfg.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
 	cfg.libbpf_flags = 0;
 	cfg.xdp_flags = xdp_flags;
@@ -555,6 +566,8 @@ static void tx_only(struct xsk_socket_info *xsk, u32 *frameptr, int batch_size)
 {
 	u32 idx;
 	unsigned int i;
+	bool tx_invalid_test = stat_test_type == STAT_TEST_TX_INVALID;
+	u32 len = tx_invalid_test ? XSK_UMEM__DEFAULT_FRAME_SIZE + 1 : PKT_SIZE;
 
 	while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) < batch_size)
 		complete_tx_only(xsk, batch_size);
@@ -563,11 +576,16 @@ static void tx_only(struct xsk_socket_info *xsk, u32 *frameptr, int batch_size)
 		struct xdp_desc *tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx + i);
 
 		tx_desc->addr = (*frameptr + i) << XSK_UMEM__DEFAULT_FRAME_SHIFT;
-		tx_desc->len = PKT_SIZE;
+		tx_desc->len = len;
 	}
 
 	xsk_ring_prod__submit(&xsk->tx, batch_size);
-	xsk->outstanding_tx += batch_size;
+	if (!tx_invalid_test) {
+		xsk->outstanding_tx += batch_size;
+	} else {
+		if (!NEED_WAKEUP || xsk_ring_prod__needs_wakeup(&xsk->tx))
+			kick_tx(xsk);
+	}
 	*frameptr += batch_size;
 	*frameptr %= num_frames;
 	complete_tx_only(xsk, batch_size);
@@ -679,6 +697,48 @@ static void worker_pkt_dump(void)
 	}
 }
 
+static void worker_stats_validate(struct ifobject *ifobject)
+{
+	struct xdp_statistics stats;
+	socklen_t optlen;
+	int err;
+	struct xsk_socket *xsk = stat_test_type == STAT_TEST_TX_INVALID ?
+							ifdict[!ifobject->ifdict_index]->xsk->xsk :
+							ifobject->xsk->xsk;
+	int fd = xsk_socket__fd(xsk);
+	unsigned long xsk_stat = 0, expected_stat = opt_pkt_count;
+
+	sigvar = 0;
+
+	optlen = sizeof(stats);
+	err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, &stats, &optlen);
+	if (err)
+		return;
+
+	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:
+			xsk_stat = stats.tx_invalid_descs;
+			break;
+		case STAT_TEST_RX_FULL:
+			xsk_stat = stats.rx_ring_full;
+			expected_stat -= RX_FULL_RXQSIZE;
+			break;
+		case STAT_TEST_RX_FILL_EMPTY:
+			xsk_stat = stats.rx_fill_ring_empty_descs;
+			break;
+		default:
+			break;
+		}
+
+		if (xsk_stat == expected_stat)
+			sigvar = 1;
+	}
+}
+
 static void worker_pkt_validate(void)
 {
 	u32 payloadseqnum = -2;
@@ -817,7 +877,8 @@ static void *worker_testapp_validate(void *arg)
 			thread_common_ops(ifobject, bufs, &sync_mutex_tx, &spinning_rx);
 
 		print_verbose("Interface [%s] vector [Rx]\n", ifobject->ifname);
-		xsk_populate_fill_ring(ifobject->umem);
+		if (stat_test_type != STAT_TEST_RX_FILL_EMPTY)
+			xsk_populate_fill_ring(ifobject->umem);
 
 		TAILQ_INIT(&head);
 		if (debug_pkt_dump) {
@@ -839,15 +900,21 @@ static void *worker_testapp_validate(void *arg)
 				if (ret <= 0)
 					continue;
 			}
-			rx_pkt(ifobject->xsk, fds);
-			worker_pkt_validate();
+
+			if (test_type != TEST_TYPE_STATS) {
+				rx_pkt(ifobject->xsk, fds);
+				worker_pkt_validate();
+			} else {
+				worker_stats_validate(ifobject);
+			}
 
 			if (sigvar)
 				break;
 		}
 
-		print_verbose("Received %d packets on interface %s\n",
-			       pkt_counter, ifobject->ifname);
+		if (test_type != TEST_TYPE_STATS)
+			print_verbose("Received %d packets on interface %s\n",
+				pkt_counter, ifobject->ifname);
 
 		if (test_type == TEST_TYPE_TEARDOWN)
 			print_verbose("Destroying socket\n");
@@ -921,7 +988,7 @@ static void testapp_validate(void)
 		free(pkt_buf);
 	}
 
-	if (!(test_type == TEST_TYPE_TEARDOWN) && !bidi)
+	if (!(test_type == TEST_TYPE_TEARDOWN) && !bidi && !(test_type == TEST_TYPE_STATS))
 		print_ksft_result();
 }
 
@@ -940,6 +1007,34 @@ static void testapp_sockets(void)
 	print_ksft_result();
 }
 
+static void testapp_stats(void)
+{
+	for (int i = 0; i < STAT_TEST_TYPE_MAX; i++) {
+		stat_test_type = i;
+
+		/* reset defaults */
+		rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
+		frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
+
+		switch (stat_test_type) {
+		case STAT_TEST_RX_DROPPED:
+			frame_headroom = XSK_UMEM__DEFAULT_FRAME_SIZE -
+						XDP_PACKET_HEADROOM - 1;
+			break;
+		case STAT_TEST_RX_FULL:
+			rxqsize = RX_FULL_RXQSIZE;
+			break;
+		default:
+			break;
+		}
+		pthread_init_mutex();
+		testapp_validate();
+		pthread_destroy_mutex();
+	}
+
+	print_ksft_result();
+}
+
 static void init_iface_config(struct ifaceconfigobj *ifaceconfig)
 {
 	/*Init interface0 */
@@ -1021,6 +1116,10 @@ static void run_pkt_test(int mode, int type)
 	prev_pkt = -1;
 	ifdict[0]->fv.vector = tx;
 	ifdict[1]->fv.vector = rx;
+	sigvar = 0;
+	stat_test_type = -1;
+	rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
+	frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
 
 	switch (mode) {
 	case (TEST_MODE_SKB):
@@ -1039,6 +1138,11 @@ static void run_pkt_test(int mode, int type)
 		break;
 	}
 
+	if (test_type == TEST_TYPE_STATS) {
+		testapp_stats();
+		return;
+	}
+
 	pthread_init_mutex();
 
 	if ((test_type != TEST_TYPE_TEARDOWN) && (test_type != TEST_TYPE_BIDI))
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 1127a396d5d0..4d0a80dbfef0 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -41,6 +41,7 @@
 #define BATCH_SIZE 64
 #define POLL_TMOUT 1000
 #define NEED_WAKEUP true
+#define RX_FULL_RXQSIZE 32
 
 #define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); } while (0)
 
@@ -59,9 +60,18 @@ enum TEST_TYPES {
 	TEST_TYPE_POLL,
 	TEST_TYPE_TEARDOWN,
 	TEST_TYPE_BIDI,
+	TEST_TYPE_STATS,
 	TEST_TYPE_MAX
 };
 
+enum STAT_TEST_TYPES {
+	STAT_TEST_RX_DROPPED,
+	STAT_TEST_TX_INVALID,
+	STAT_TEST_RX_FULL,
+	STAT_TEST_RX_FILL_EMPTY,
+	STAT_TEST_TYPE_MAX
+};
+
 static u8 uut;
 static u8 debug_pkt_dump;
 static u32 num_frames;
@@ -80,6 +90,9 @@ static u8 pkt_data[XSK_UMEM__DEFAULT_FRAME_SIZE];
 static u32 pkt_counter;
 static long prev_pkt = -1;
 static int sigvar;
+static int stat_test_type;
+static u32 rxqsize;
+static u32 frame_headroom;
 
 struct xsk_umem_info {
 	struct xsk_ring_prod fq;
-- 
2.17.1


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

* Re: [PATCH bpf-next 3/4] selftests/bpf: restructure xsk selftests
  2021-02-17 16:02 ` [PATCH bpf-next 3/4] selftests/bpf: restructure xsk selftests Ciara Loftus
@ 2021-02-18  9:33   ` Björn Töpel
  2021-02-22 11:23   ` Maciej Fijalkowski
  1 sibling, 0 replies; 13+ messages in thread
From: Björn Töpel @ 2021-02-18  9:33 UTC (permalink / raw)
  To: Ciara Loftus; +Cc: Netdev, bpf, Karlsson, Magnus, Weqaar Janjua

On Wed, 17 Feb 2021 at 17:33, Ciara Loftus <ciara.loftus@intel.com> wrote:
>
> Prior to this commit individual xsk tests were launched from the
> shell script 'test_xsk.sh'. When adding a new test type, two new test
> configurations had to be added to this file - one for each of the
> supported XDP 'modes' (skb or drv). Should zero copy support be added to
> the xsk selftest framework in the future, three new test configurations
> would need to be added for each new test type. Each new test type also
> typically requires new CLI arguments for the xdpxceiver program.
>
> This commit aims to reduce the overhead of adding new tests, by launching
> the test configurations from within the xdpxceiver program itself, using
> simple loops. Every test is run every time the C program is executed. Many
> of the CLI arguments can be removed as a result.
>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
>  tools/testing/selftests/bpf/test_xsk.sh    | 112 +-----------
>  tools/testing/selftests/bpf/xdpxceiver.c   | 199 ++++++++++++---------
>  tools/testing/selftests/bpf/xdpxceiver.h   |  27 ++-
>  tools/testing/selftests/bpf/xsk_prereqs.sh |  24 +--
>  4 files changed, 139 insertions(+), 223 deletions(-)
>

I like where this is going! Nice!

[...]

> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index 8af746c9a6b6..7cb4a13597d0 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c

[...]

>
> +static int configure_skb(void)
> +{
> +
> +       char cmd[80];
> +
> +       snprintf(cmd, sizeof(cmd), "ip link set dev %s xdpdrv off", ifdict[0]->ifname);
> +       if (system(cmd)) {
> +               ksft_test_result_fail("Failed to configure native mode on iface %s\n",
> +                                               ifdict[0]->ifname);
> +               return -1;
> +       }
> +       snprintf(cmd, sizeof(cmd), "ip netns exec %s ip link set dev %s xdpdrv off",
> +                                       ifdict[1]->nsname, ifdict[1]->ifname);
> +       if (system(cmd)) {
> +               ksft_test_result_fail("Failed to configure native mode on iface/ns %s\n",
> +                                               ifdict[1]->ifname, ifdict[1]->nsname);
> +               return -1;
> +       }
> +
> +       cur_mode = TEST_MODE_SKB;
> +
> +       return 0;
> +
> +}
> +
> +static int configure_drv(void)
> +{
> +       char cmd[80];
> +
> +       snprintf(cmd, sizeof(cmd), "ip link set dev %s xdpgeneric off", ifdict[0]->ifname);
> +       if (system(cmd)) {
> +               ksft_test_result_fail("Failed to configure native mode on iface %s\n",
> +                                               ifdict[0]->ifname);
> +               return -1;
> +       }
> +       snprintf(cmd, sizeof(cmd), "ip netns exec %s ip link set dev %s xdpgeneric off",
> +                                       ifdict[1]->nsname, ifdict[1]->ifname);
> +       if (system(cmd)) {
> +               ksft_test_result_fail("Failed to configure native mode on iface/ns %s\n",
> +                                               ifdict[1]->ifname, ifdict[1]->nsname);
> +               return -1;
> +       }
> +
> +       cur_mode = TEST_MODE_DRV;
> +
> +       return 0;
> +}
> +

We're already using libbpf, so please use the libbpf APIs instead of
calling iproute2.

Björn

[...]

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

* Re: [PATCH bpf-next 2/4] selftests/bpf: expose debug arg to shell script for xsk tests
  2021-02-17 16:02 ` [PATCH bpf-next 2/4] selftests/bpf: expose debug arg to shell script for xsk tests Ciara Loftus
@ 2021-02-19 13:12   ` Magnus Karlsson
  2021-02-22 14:29     ` Loftus, Ciara
  0 siblings, 1 reply; 13+ messages in thread
From: Magnus Karlsson @ 2021-02-19 13:12 UTC (permalink / raw)
  To: Ciara Loftus
  Cc: Network Development, bpf, Karlsson, Magnus,
	Björn Töpel, Weqaar Janjua

On Wed, Feb 17, 2021 at 5:36 PM Ciara Loftus <ciara.loftus@intel.com> wrote:
>
> Launching xdpxceiver with -D enables debug mode. Make it possible

Would be clearer if the option was the same both in the shell and in
the xdpreceiver app, so please pick -d or -D and stick with it. And
how about calling the mode "dump packets" instead of debug, because
that is what it is doing right now?

> to pass this flag to the app via the test_xsk.sh shell script like
> so:
>
> ./test_xsk.sh -d
>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
>  tools/testing/selftests/bpf/test_xsk.sh    | 7 ++++++-
>  tools/testing/selftests/bpf/xsk_prereqs.sh | 3 ++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
> index 91127a5be90d..a72f8ed2932d 100755
> --- a/tools/testing/selftests/bpf/test_xsk.sh
> +++ b/tools/testing/selftests/bpf/test_xsk.sh
> @@ -74,11 +74,12 @@
>
>  . xsk_prereqs.sh
>
> -while getopts "cv" flag
> +while getopts "cvd" flag
>  do
>         case "${flag}" in
>                 c) colorconsole=1;;
>                 v) verbose=1;;
> +               d) debug=1;;
>         esac
>  done
>
> @@ -135,6 +136,10 @@ if [[ $verbose -eq 1 ]]; then
>         VERBOSE_ARG="-v"
>  fi
>
> +if [[ $debug -eq 1 ]]; then
> +       DEBUG_ARG="-D"
> +fi
> +
>  test_status $retval "${TEST_NAME}"
>
>  ## START TESTS
> diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
> index ef8c5b31f4b6..d95018051fcc 100755
> --- a/tools/testing/selftests/bpf/xsk_prereqs.sh
> +++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
> @@ -128,5 +128,6 @@ execxdpxceiver()
>                         copy[$index]=${!current}
>                 done
>
> -       ./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]} -C ${NUMPKTS} ${VERBOSE_ARG}
> +       ./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]} -C ${NUMPKTS} ${VERBOSE_ARG} \
> +               ${DEBUG_ARG}
>  }
> --
> 2.17.1
>

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

* Re: [PATCH bpf-next 3/4] selftests/bpf: restructure xsk selftests
  2021-02-17 16:02 ` [PATCH bpf-next 3/4] selftests/bpf: restructure xsk selftests Ciara Loftus
  2021-02-18  9:33   ` Björn Töpel
@ 2021-02-22 11:23   ` Maciej Fijalkowski
  2021-02-23  8:24     ` Loftus, Ciara
  1 sibling, 1 reply; 13+ messages in thread
From: Maciej Fijalkowski @ 2021-02-22 11:23 UTC (permalink / raw)
  To: Ciara Loftus; +Cc: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua

On Wed, Feb 17, 2021 at 04:02:13PM +0000, Ciara Loftus wrote:
> Prior to this commit individual xsk tests were launched from the
> shell script 'test_xsk.sh'. When adding a new test type, two new test
> configurations had to be added to this file - one for each of the
> supported XDP 'modes' (skb or drv). Should zero copy support be added to
> the xsk selftest framework in the future, three new test configurations
> would need to be added for each new test type. Each new test type also
> typically requires new CLI arguments for the xdpxceiver program.
> 
> This commit aims to reduce the overhead of adding new tests, by launching
> the test configurations from within the xdpxceiver program itself, using
> simple loops. Every test is run every time the C program is executed. Many
> of the CLI arguments can be removed as a result.
> 
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
>  tools/testing/selftests/bpf/test_xsk.sh    | 112 +-----------
>  tools/testing/selftests/bpf/xdpxceiver.c   | 199 ++++++++++++---------
>  tools/testing/selftests/bpf/xdpxceiver.h   |  27 ++-
>  tools/testing/selftests/bpf/xsk_prereqs.sh |  24 +--
>  4 files changed, 139 insertions(+), 223 deletions(-)
> 

Good cleanup! I have a series of fixes/cleanups as well and I need to
introduce a new test over here, so your work makes it easier for me.

One nit below and once you address Bjorn's request, then feel free to add
my:

Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

[...]

> +static int configure_skb(void)
> +{
> +
> +	char cmd[80];
> +
> +	snprintf(cmd, sizeof(cmd), "ip link set dev %s xdpdrv off", ifdict[0]->ifname);
> +	if (system(cmd)) {
> +		ksft_test_result_fail("Failed to configure native mode on iface %s\n",
> +						ifdict[0]->ifname);
> +		return -1;
> +	}
> +	snprintf(cmd, sizeof(cmd), "ip netns exec %s ip link set dev %s xdpdrv off",
> +					ifdict[1]->nsname, ifdict[1]->ifname);
> +	if (system(cmd)) {
> +		ksft_test_result_fail("Failed to configure native mode on iface/ns %s\n",
> +						ifdict[1]->ifname, ifdict[1]->nsname);
> +		return -1;
> +	}
> +
> +	cur_mode = TEST_MODE_SKB;
> +
> +	return 0;
> +
> +}
> +
> +static int configure_drv(void)
> +{
> +	char cmd[80];
> +
> +	snprintf(cmd, sizeof(cmd), "ip link set dev %s xdpgeneric off", ifdict[0]->ifname);
> +	if (system(cmd)) {
> +		ksft_test_result_fail("Failed to configure native mode on iface %s\n",
> +						ifdict[0]->ifname);
> +		return -1;
> +	}
> +	snprintf(cmd, sizeof(cmd), "ip netns exec %s ip link set dev %s xdpgeneric off",
> +					ifdict[1]->nsname, ifdict[1]->ifname);
> +	if (system(cmd)) {
> +		ksft_test_result_fail("Failed to configure native mode on iface/ns %s\n",
> +						ifdict[1]->ifname, ifdict[1]->nsname);
> +		return -1;
> +	}
> +
> +	cur_mode = TEST_MODE_DRV;
> +
> +	return 0;
> +}
> +
> +static void run_pkt_test(int mode, int type)
> +{
> +	test_type = type;
> +
> +	/* reset defaults after potential previous test */
> +	xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> +	pkt_counter = 0;
> +	switching_notify = 0;
> +	bidi_pass = 0;
> +	prev_pkt = -1;
> +	ifdict[0]->fv.vector = tx;
> +	ifdict[1]->fv.vector = rx;
> +
> +	switch (mode) {
> +	case (TEST_MODE_SKB):
> +		if (cur_mode != TEST_MODE_SKB)
> +			configure_skb();

Should you check a return value over here?

> +		xdp_flags |= XDP_FLAGS_SKB_MODE;
> +		uut = TEST_MODE_SKB;
> +		break;
> +	case (TEST_MODE_DRV):
> +		if (cur_mode != TEST_MODE_DRV)
> +			configure_drv();

ditto

> +		xdp_flags |= XDP_FLAGS_DRV_MODE;
> +		uut = TEST_MODE_DRV;
> +		break;
> +	default:
> +		break;
> +	}
> +
> +	pthread_init_mutex();
> +
> +	if ((test_type != TEST_TYPE_TEARDOWN) && (test_type != TEST_TYPE_BIDI))
> +		testapp_validate();
> +	else
> +		testapp_sockets();
> +
> +	pthread_destroy_mutex();
> +}
> +
>  int main(int argc, char **argv)
>  {
>  	struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
> @@ -1021,6 +1062,7 @@ int main(int argc, char **argv)
>  	const char *IP2 = "192.168.100.161";
>  	u16 UDP_DST_PORT = 2020;
>  	u16 UDP_SRC_PORT = 2121;
> +	int i, j;
>  
>  	ifaceconfig = malloc(sizeof(struct ifaceconfigobj));
>  	memcpy(ifaceconfig->dst_mac, MAC1, ETH_ALEN);
> @@ -1046,24 +1088,19 @@ int main(int argc, char **argv)
>  
>  	init_iface_config(ifaceconfig);
>  
> -	pthread_init_mutex();
> +	ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
>  
> -	ksft_set_plan(1);
> +	configure_skb();
> +	cur_mode = TEST_MODE_SKB;
>  
> -	if (!opt_teardown && !opt_bidi) {
> -		testapp_validate();
> -	} else if (opt_teardown && opt_bidi) {
> -		ksft_test_result_fail("ERROR: parameters -T and -B cannot be used together\n");
> -		ksft_exit_xfail();
> -	} else {
> -		testapp_sockets();
> +	for (i = 0; i < TEST_MODE_MAX; i++) {
> +		for (j = 0; j < TEST_TYPE_MAX; j++)
> +			run_pkt_test(i, j);
>  	}
>  
>  	for (int i = 0; i < MAX_INTERFACES; i++)
>  		free(ifdict[i]);
>  
> -	pthread_destroy_mutex();
> -
>  	ksft_exit_pass();
>  
>  	return 0;

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

* Re: [PATCH bpf-next 4/4] selftests/bpf: introduce xsk statistics tests
  2021-02-17 16:02 ` [PATCH bpf-next 4/4] selftests/bpf: introduce xsk statistics tests Ciara Loftus
@ 2021-02-22 11:56   ` Maciej Fijalkowski
  2021-02-22 14:33     ` Loftus, Ciara
  0 siblings, 1 reply; 13+ messages in thread
From: Maciej Fijalkowski @ 2021-02-22 11:56 UTC (permalink / raw)
  To: Ciara Loftus; +Cc: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua

On Wed, Feb 17, 2021 at 04:02:14PM +0000, Ciara Loftus wrote:
> This commit introduces a range of tests to the xsk testsuite
> for validating xsk statistics.
> 
> A new test type called 'stats' is added. Within it there are
> four sub-tests which test the following statistics:
> 1. rx dropped
> 2. tx invalid
> 3. rx ring full
> 4. fill queue empty
> 
> Each test configures a scenario which should trigger the given
> error statistic. The test passes if the statistic is successfully
> incremented.

Have you thought of adding a short description per each sub-test? This
would be helpful if you would mention how each particular is triggered.

Like, reducing the size of XSK Rx ring causes Rx drops and so on.

Reason why I'm asking for that is because personally I feel like 'tx
invalid' is a bit too generic name for a sub-test.

> 
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> ---
>  tools/testing/selftests/bpf/xdpxceiver.c | 130 ++++++++++++++++++++---
>  tools/testing/selftests/bpf/xdpxceiver.h |  13 +++
>  2 files changed, 130 insertions(+), 13 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index 7cb4a13597d0..4647c89b2019 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -28,8 +28,11 @@
>   *       Configure sockets as bi-directional tx/rx sockets, sets up fill and
>   *       completion rings on each socket, tx/rx in both directions. Only nopoll
>   *       mode is used
> + *    e. Statistics
> + *       Trigger some error conditions and ensure that the appropriate statistics
> + *       are incremented.
>   *
> - * Total tests: 8
> + * Total tests: 10
>   *
>   * Flow:
>   * -----
> @@ -90,10 +93,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 print_ksft_result(void)\
> -	(ksft_test_result_pass("PASS: %s %s %s%s\n", uut ? "DRV" : "SKB",\
> +	(ksft_test_result_pass("PASS: %s %s %s%s%s\n", uut ? "DRV" : "SKB",\
>  			       test_type == TEST_TYPE_POLL ? "POLL" : "NOPOLL",\
>  			       test_type == TEST_TYPE_TEARDOWN ? "Socket Teardown" : "",\
> -			       test_type == TEST_TYPE_BIDI ? "Bi-directional Sockets" : ""))
> +			       test_type == TEST_TYPE_BIDI ? "Bi-directional Sockets" : "",\
> +			       test_type == TEST_TYPE_STATS ? "Stats" : ""))
>  
>  static void pthread_init_mutex(void)
>  {
> @@ -255,13 +259,20 @@ static void gen_eth_frame(struct xsk_umem_info *umem, u64 addr)
>  static void xsk_configure_umem(struct ifobject *data, void *buffer, u64 size)
>  {
>  	int ret;
> +	struct xsk_umem_config cfg = {
> +		.fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
> +		.comp_size = XSK_RING_CONS__DEFAULT_NUM_DESCS,
> +		.frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE,
> +		.frame_headroom = frame_headroom,
> +		.flags = XSK_UMEM__DEFAULT_FLAGS
> +	};
>  
>  	data->umem = calloc(1, sizeof(struct xsk_umem_info));
>  	if (!data->umem)
>  		exit_with_error(errno);
>  
>  	ret = xsk_umem__create(&data->umem->umem, buffer, size,
> -			       &data->umem->fq, &data->umem->cq, NULL);
> +			       &data->umem->fq, &data->umem->cq, &cfg);
>  	if (ret)
>  		exit_with_error(ret);
>  
> @@ -293,7 +304,7 @@ static int xsk_configure_socket(struct ifobject *ifobject)
>  		exit_with_error(errno);
>  
>  	ifobject->xsk->umem = ifobject->umem;
> -	cfg.rx_size = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> +	cfg.rx_size = rxqsize;
>  	cfg.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
>  	cfg.libbpf_flags = 0;
>  	cfg.xdp_flags = xdp_flags;
> @@ -555,6 +566,8 @@ static void tx_only(struct xsk_socket_info *xsk, u32 *frameptr, int batch_size)
>  {
>  	u32 idx;
>  	unsigned int i;
> +	bool tx_invalid_test = stat_test_type == STAT_TEST_TX_INVALID;
> +	u32 len = tx_invalid_test ? XSK_UMEM__DEFAULT_FRAME_SIZE + 1 : PKT_SIZE;
>  
>  	while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) < batch_size)
>  		complete_tx_only(xsk, batch_size);
> @@ -563,11 +576,16 @@ static void tx_only(struct xsk_socket_info *xsk, u32 *frameptr, int batch_size)
>  		struct xdp_desc *tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx + i);
>  
>  		tx_desc->addr = (*frameptr + i) << XSK_UMEM__DEFAULT_FRAME_SHIFT;
> -		tx_desc->len = PKT_SIZE;
> +		tx_desc->len = len;
>  	}
>  
>  	xsk_ring_prod__submit(&xsk->tx, batch_size);
> -	xsk->outstanding_tx += batch_size;
> +	if (!tx_invalid_test) {
> +		xsk->outstanding_tx += batch_size;
> +	} else {
> +		if (!NEED_WAKEUP || xsk_ring_prod__needs_wakeup(&xsk->tx))
> +			kick_tx(xsk);
> +	}
>  	*frameptr += batch_size;
>  	*frameptr %= num_frames;
>  	complete_tx_only(xsk, batch_size);
> @@ -679,6 +697,48 @@ static void worker_pkt_dump(void)
>  	}
>  }
>  
> +static void worker_stats_validate(struct ifobject *ifobject)
> +{
> +	struct xdp_statistics stats;
> +	socklen_t optlen;
> +	int err;
> +	struct xsk_socket *xsk = stat_test_type == STAT_TEST_TX_INVALID ?
> +							ifdict[!ifobject->ifdict_index]->xsk->xsk :
> +							ifobject->xsk->xsk;
> +	int fd = xsk_socket__fd(xsk);
> +	unsigned long xsk_stat = 0, expected_stat = opt_pkt_count;
> +
> +	sigvar = 0;
> +
> +	optlen = sizeof(stats);
> +	err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, &stats, &optlen);
> +	if (err)
> +		return;
> +
> +	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:
> +			xsk_stat = stats.tx_invalid_descs;
> +			break;
> +		case STAT_TEST_RX_FULL:
> +			xsk_stat = stats.rx_ring_full;
> +			expected_stat -= RX_FULL_RXQSIZE;
> +			break;
> +		case STAT_TEST_RX_FILL_EMPTY:
> +			xsk_stat = stats.rx_fill_ring_empty_descs;
> +			break;
> +		default:
> +			break;
> +		}
> +
> +		if (xsk_stat == expected_stat)
> +			sigvar = 1;
> +	}
> +}
> +
>  static void worker_pkt_validate(void)
>  {
>  	u32 payloadseqnum = -2;
> @@ -817,7 +877,8 @@ static void *worker_testapp_validate(void *arg)
>  			thread_common_ops(ifobject, bufs, &sync_mutex_tx, &spinning_rx);
>  
>  		print_verbose("Interface [%s] vector [Rx]\n", ifobject->ifname);
> -		xsk_populate_fill_ring(ifobject->umem);
> +		if (stat_test_type != STAT_TEST_RX_FILL_EMPTY)
> +			xsk_populate_fill_ring(ifobject->umem);
>  
>  		TAILQ_INIT(&head);
>  		if (debug_pkt_dump) {
> @@ -839,15 +900,21 @@ static void *worker_testapp_validate(void *arg)
>  				if (ret <= 0)
>  					continue;
>  			}
> -			rx_pkt(ifobject->xsk, fds);
> -			worker_pkt_validate();
> +
> +			if (test_type != TEST_TYPE_STATS) {
> +				rx_pkt(ifobject->xsk, fds);
> +				worker_pkt_validate();
> +			} else {
> +				worker_stats_validate(ifobject);
> +			}
>  
>  			if (sigvar)
>  				break;
>  		}
>  
> -		print_verbose("Received %d packets on interface %s\n",
> -			       pkt_counter, ifobject->ifname);
> +		if (test_type != TEST_TYPE_STATS)
> +			print_verbose("Received %d packets on interface %s\n",
> +				pkt_counter, ifobject->ifname);
>  
>  		if (test_type == TEST_TYPE_TEARDOWN)
>  			print_verbose("Destroying socket\n");
> @@ -921,7 +988,7 @@ static void testapp_validate(void)
>  		free(pkt_buf);
>  	}
>  
> -	if (!(test_type == TEST_TYPE_TEARDOWN) && !bidi)
> +	if (!(test_type == TEST_TYPE_TEARDOWN) && !bidi && !(test_type == TEST_TYPE_STATS))
>  		print_ksft_result();
>  }
>  
> @@ -940,6 +1007,34 @@ static void testapp_sockets(void)
>  	print_ksft_result();
>  }
>  
> +static void testapp_stats(void)
> +{
> +	for (int i = 0; i < STAT_TEST_TYPE_MAX; i++) {
> +		stat_test_type = i;
> +
> +		/* reset defaults */
> +		rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> +		frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
> +
> +		switch (stat_test_type) {
> +		case STAT_TEST_RX_DROPPED:
> +			frame_headroom = XSK_UMEM__DEFAULT_FRAME_SIZE -
> +						XDP_PACKET_HEADROOM - 1;
> +			break;
> +		case STAT_TEST_RX_FULL:
> +			rxqsize = RX_FULL_RXQSIZE;
> +			break;
> +		default:
> +			break;
> +		}
> +		pthread_init_mutex();

Do you really have to init/destroy mutexes per each iteration?

> +		testapp_validate();
> +		pthread_destroy_mutex();
> +	}
> +
> +	print_ksft_result();
> +}
> +
>  static void init_iface_config(struct ifaceconfigobj *ifaceconfig)
>  {
>  	/*Init interface0 */
> @@ -1021,6 +1116,10 @@ static void run_pkt_test(int mode, int type)
>  	prev_pkt = -1;
>  	ifdict[0]->fv.vector = tx;
>  	ifdict[1]->fv.vector = rx;
> +	sigvar = 0;
> +	stat_test_type = -1;
> +	rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> +	frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
>  
>  	switch (mode) {
>  	case (TEST_MODE_SKB):
> @@ -1039,6 +1138,11 @@ static void run_pkt_test(int mode, int type)
>  		break;
>  	}
>  
> +	if (test_type == TEST_TYPE_STATS) {
> +		testapp_stats();
> +		return;
> +	}

Why this can't be a part of if/else if/else branch that below is picking
the test type?

> +
>  	pthread_init_mutex();
>  
>  	if ((test_type != TEST_TYPE_TEARDOWN) && (test_type != TEST_TYPE_BIDI))
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> index 1127a396d5d0..4d0a80dbfef0 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.h
> +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> @@ -41,6 +41,7 @@
>  #define BATCH_SIZE 64
>  #define POLL_TMOUT 1000
>  #define NEED_WAKEUP true
> +#define RX_FULL_RXQSIZE 32
>  
>  #define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); } while (0)
>  
> @@ -59,9 +60,18 @@ enum TEST_TYPES {
>  	TEST_TYPE_POLL,
>  	TEST_TYPE_TEARDOWN,
>  	TEST_TYPE_BIDI,
> +	TEST_TYPE_STATS,
>  	TEST_TYPE_MAX
>  };
>  
> +enum STAT_TEST_TYPES {
> +	STAT_TEST_RX_DROPPED,
> +	STAT_TEST_TX_INVALID,
> +	STAT_TEST_RX_FULL,
> +	STAT_TEST_RX_FILL_EMPTY,
> +	STAT_TEST_TYPE_MAX
> +};
> +
>  static u8 uut;
>  static u8 debug_pkt_dump;
>  static u32 num_frames;
> @@ -80,6 +90,9 @@ static u8 pkt_data[XSK_UMEM__DEFAULT_FRAME_SIZE];
>  static u32 pkt_counter;
>  static long prev_pkt = -1;
>  static int sigvar;
> +static int stat_test_type;
> +static u32 rxqsize;
> +static u32 frame_headroom;
>  
>  struct xsk_umem_info {
>  	struct xsk_ring_prod fq;
> -- 
> 2.17.1
> 

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

* Re: [PATCH bpf-next 1/4] selftest/bpf: make xsk tests less verbose
  2021-02-17 16:02 ` [PATCH bpf-next 1/4] selftest/bpf: make xsk tests less verbose Ciara Loftus
@ 2021-02-22 11:58   ` Maciej Fijalkowski
  0 siblings, 0 replies; 13+ messages in thread
From: Maciej Fijalkowski @ 2021-02-22 11:58 UTC (permalink / raw)
  To: Ciara Loftus; +Cc: netdev, bpf, magnus.karlsson, bjorn, weqaar.a.janjua

On Wed, Feb 17, 2021 at 04:02:11PM +0000, Ciara Loftus wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Make the xsk tests less verbose by only printing the
> essentials. Currently, it is hard to see if the tests passed or not
> due to all the printouts. Move the extra printouts to a verbose
> option, if further debugging is needed when a problem arises.
> 
> To run the xsk tests with verbose output:
> ./test_xsk.sh -v
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>

Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

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

* RE: [PATCH bpf-next 2/4] selftests/bpf: expose debug arg to shell script for xsk tests
  2021-02-19 13:12   ` Magnus Karlsson
@ 2021-02-22 14:29     ` Loftus, Ciara
  0 siblings, 0 replies; 13+ messages in thread
From: Loftus, Ciara @ 2021-02-22 14:29 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: Network Development, bpf, Karlsson, Magnus,
	Björn Töpel, Janjua, Weqaar A

> 
> On Wed, Feb 17, 2021 at 5:36 PM Ciara Loftus <ciara.loftus@intel.com>
> wrote:
> >
> > Launching xdpxceiver with -D enables debug mode. Make it possible
> 
> Would be clearer if the option was the same both in the shell and in
> the xdpreceiver app, so please pick -d or -D and stick with it. And
> how about calling the mode "dump packets" instead of debug, because
> that is what it is doing right now?

+1. Will stick with -D to be consistent with the current interface.
Will make the long opt '--dump_pkts'.

Thanks,
Ciara

> 
> > to pass this flag to the app via the test_xsk.sh shell script like
> > so:
> >
> > ./test_xsk.sh -d
> >
> > Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> > ---
> >  tools/testing/selftests/bpf/test_xsk.sh    | 7 ++++++-
> >  tools/testing/selftests/bpf/xsk_prereqs.sh | 3 ++-
> >  2 files changed, 8 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/test_xsk.sh
> b/tools/testing/selftests/bpf/test_xsk.sh
> > index 91127a5be90d..a72f8ed2932d 100755
> > --- a/tools/testing/selftests/bpf/test_xsk.sh
> > +++ b/tools/testing/selftests/bpf/test_xsk.sh
> > @@ -74,11 +74,12 @@
> >
> >  . xsk_prereqs.sh
> >
> > -while getopts "cv" flag
> > +while getopts "cvd" flag
> >  do
> >         case "${flag}" in
> >                 c) colorconsole=1;;
> >                 v) verbose=1;;
> > +               d) debug=1;;
> >         esac
> >  done
> >
> > @@ -135,6 +136,10 @@ if [[ $verbose -eq 1 ]]; then
> >         VERBOSE_ARG="-v"
> >  fi
> >
> > +if [[ $debug -eq 1 ]]; then
> > +       DEBUG_ARG="-D"
> > +fi
> > +
> >  test_status $retval "${TEST_NAME}"
> >
> >  ## START TESTS
> > diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh
> b/tools/testing/selftests/bpf/xsk_prereqs.sh
> > index ef8c5b31f4b6..d95018051fcc 100755
> > --- a/tools/testing/selftests/bpf/xsk_prereqs.sh
> > +++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
> > @@ -128,5 +128,6 @@ execxdpxceiver()
> >                         copy[$index]=${!current}
> >                 done
> >
> > -       ./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]} -C ${NUMPKTS}
> ${VERBOSE_ARG}
> > +       ./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${copy[*]} -C ${NUMPKTS}
> ${VERBOSE_ARG} \
> > +               ${DEBUG_ARG}
> >  }
> > --
> > 2.17.1
> >

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

* RE: [PATCH bpf-next 4/4] selftests/bpf: introduce xsk statistics tests
  2021-02-22 11:56   ` Maciej Fijalkowski
@ 2021-02-22 14:33     ` Loftus, Ciara
  0 siblings, 0 replies; 13+ messages in thread
From: Loftus, Ciara @ 2021-02-22 14:33 UTC (permalink / raw)
  To: Fijalkowski, Maciej
  Cc: netdev, bpf, Karlsson, Magnus, bjorn, Janjua, Weqaar A

> 
> On Wed, Feb 17, 2021 at 04:02:14PM +0000, Ciara Loftus wrote:
> > This commit introduces a range of tests to the xsk testsuite
> > for validating xsk statistics.
> >
> > A new test type called 'stats' is added. Within it there are
> > four sub-tests which test the following statistics:
> > 1. rx dropped
> > 2. tx invalid
> > 3. rx ring full
> > 4. fill queue empty
> >
> > Each test configures a scenario which should trigger the given
> > error statistic. The test passes if the statistic is successfully
> > incremented.
> 
> Have you thought of adding a short description per each sub-test? This
> would be helpful if you would mention how each particular is triggered.
> 
> Like, reducing the size of XSK Rx ring causes Rx drops and so on.
> 
> Reason why I'm asking for that is because personally I feel like 'tx
> invalid' is a bit too generic name for a sub-test.

+1. I will add a description to both the commit log and inside xdpxceiver.c for good measure.

> 
> >
> > Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> > ---
> >  tools/testing/selftests/bpf/xdpxceiver.c | 130 ++++++++++++++++++++--
> -
> >  tools/testing/selftests/bpf/xdpxceiver.h |  13 +++
> >  2 files changed, 130 insertions(+), 13 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c
> b/tools/testing/selftests/bpf/xdpxceiver.c
> > index 7cb4a13597d0..4647c89b2019 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.c
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> > @@ -28,8 +28,11 @@
> >   *       Configure sockets as bi-directional tx/rx sockets, sets up fill and
> >   *       completion rings on each socket, tx/rx in both directions. Only nopoll
> >   *       mode is used
> > + *    e. Statistics
> > + *       Trigger some error conditions and ensure that the appropriate
> statistics
> > + *       are incremented.
> >   *
> > - * Total tests: 8
> > + * Total tests: 10
> >   *
> >   * Flow:
> >   * -----
> > @@ -90,10 +93,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 print_ksft_result(void)\
> > -	(ksft_test_result_pass("PASS: %s %s %s%s\n", uut ? "DRV" : "SKB",\
> > +	(ksft_test_result_pass("PASS: %s %s %s%s%s\n", uut ? "DRV" :
> "SKB",\
> >  			       test_type == TEST_TYPE_POLL ? "POLL" :
> "NOPOLL",\
> >  			       test_type == TEST_TYPE_TEARDOWN ? "Socket
> Teardown" : "",\
> > -			       test_type == TEST_TYPE_BIDI ? "Bi-directional
> Sockets" : ""))
> > +			       test_type == TEST_TYPE_BIDI ? "Bi-directional
> Sockets" : "",\
> > +			       test_type == TEST_TYPE_STATS ? "Stats" : ""))
> >
> >  static void pthread_init_mutex(void)
> >  {
> > @@ -255,13 +259,20 @@ static void gen_eth_frame(struct xsk_umem_info
> *umem, u64 addr)
> >  static void xsk_configure_umem(struct ifobject *data, void *buffer, u64
> size)
> >  {
> >  	int ret;
> > +	struct xsk_umem_config cfg = {
> > +		.fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
> > +		.comp_size = XSK_RING_CONS__DEFAULT_NUM_DESCS,
> > +		.frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE,
> > +		.frame_headroom = frame_headroom,
> > +		.flags = XSK_UMEM__DEFAULT_FLAGS
> > +	};
> >
> >  	data->umem = calloc(1, sizeof(struct xsk_umem_info));
> >  	if (!data->umem)
> >  		exit_with_error(errno);
> >
> >  	ret = xsk_umem__create(&data->umem->umem, buffer, size,
> > -			       &data->umem->fq, &data->umem->cq, NULL);
> > +			       &data->umem->fq, &data->umem->cq, &cfg);
> >  	if (ret)
> >  		exit_with_error(ret);
> >
> > @@ -293,7 +304,7 @@ static int xsk_configure_socket(struct ifobject
> *ifobject)
> >  		exit_with_error(errno);
> >
> >  	ifobject->xsk->umem = ifobject->umem;
> > -	cfg.rx_size = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> > +	cfg.rx_size = rxqsize;
> >  	cfg.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
> >  	cfg.libbpf_flags = 0;
> >  	cfg.xdp_flags = xdp_flags;
> > @@ -555,6 +566,8 @@ static void tx_only(struct xsk_socket_info *xsk, u32
> *frameptr, int batch_size)
> >  {
> >  	u32 idx;
> >  	unsigned int i;
> > +	bool tx_invalid_test = stat_test_type == STAT_TEST_TX_INVALID;
> > +	u32 len = tx_invalid_test ? XSK_UMEM__DEFAULT_FRAME_SIZE + 1 :
> PKT_SIZE;
> >
> >  	while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) <
> batch_size)
> >  		complete_tx_only(xsk, batch_size);
> > @@ -563,11 +576,16 @@ static void tx_only(struct xsk_socket_info *xsk,
> u32 *frameptr, int batch_size)
> >  		struct xdp_desc *tx_desc = xsk_ring_prod__tx_desc(&xsk-
> >tx, idx + i);
> >
> >  		tx_desc->addr = (*frameptr + i) <<
> XSK_UMEM__DEFAULT_FRAME_SHIFT;
> > -		tx_desc->len = PKT_SIZE;
> > +		tx_desc->len = len;
> >  	}
> >
> >  	xsk_ring_prod__submit(&xsk->tx, batch_size);
> > -	xsk->outstanding_tx += batch_size;
> > +	if (!tx_invalid_test) {
> > +		xsk->outstanding_tx += batch_size;
> > +	} else {
> > +		if (!NEED_WAKEUP ||
> xsk_ring_prod__needs_wakeup(&xsk->tx))
> > +			kick_tx(xsk);
> > +	}
> >  	*frameptr += batch_size;
> >  	*frameptr %= num_frames;
> >  	complete_tx_only(xsk, batch_size);
> > @@ -679,6 +697,48 @@ static void worker_pkt_dump(void)
> >  	}
> >  }
> >
> > +static void worker_stats_validate(struct ifobject *ifobject)
> > +{
> > +	struct xdp_statistics stats;
> > +	socklen_t optlen;
> > +	int err;
> > +	struct xsk_socket *xsk = stat_test_type == STAT_TEST_TX_INVALID ?
> > +							ifdict[!ifobject-
> >ifdict_index]->xsk->xsk :
> > +							ifobject->xsk->xsk;
> > +	int fd = xsk_socket__fd(xsk);
> > +	unsigned long xsk_stat = 0, expected_stat = opt_pkt_count;
> > +
> > +	sigvar = 0;
> > +
> > +	optlen = sizeof(stats);
> > +	err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, &stats, &optlen);
> > +	if (err)
> > +		return;
> > +
> > +	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:
> > +			xsk_stat = stats.tx_invalid_descs;
> > +			break;
> > +		case STAT_TEST_RX_FULL:
> > +			xsk_stat = stats.rx_ring_full;
> > +			expected_stat -= RX_FULL_RXQSIZE;
> > +			break;
> > +		case STAT_TEST_RX_FILL_EMPTY:
> > +			xsk_stat = stats.rx_fill_ring_empty_descs;
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +
> > +		if (xsk_stat == expected_stat)
> > +			sigvar = 1;
> > +	}
> > +}
> > +
> >  static void worker_pkt_validate(void)
> >  {
> >  	u32 payloadseqnum = -2;
> > @@ -817,7 +877,8 @@ static void *worker_testapp_validate(void *arg)
> >  			thread_common_ops(ifobject, bufs,
> &sync_mutex_tx, &spinning_rx);
> >
> >  		print_verbose("Interface [%s] vector [Rx]\n", ifobject-
> >ifname);
> > -		xsk_populate_fill_ring(ifobject->umem);
> > +		if (stat_test_type != STAT_TEST_RX_FILL_EMPTY)
> > +			xsk_populate_fill_ring(ifobject->umem);
> >
> >  		TAILQ_INIT(&head);
> >  		if (debug_pkt_dump) {
> > @@ -839,15 +900,21 @@ static void *worker_testapp_validate(void *arg)
> >  				if (ret <= 0)
> >  					continue;
> >  			}
> > -			rx_pkt(ifobject->xsk, fds);
> > -			worker_pkt_validate();
> > +
> > +			if (test_type != TEST_TYPE_STATS) {
> > +				rx_pkt(ifobject->xsk, fds);
> > +				worker_pkt_validate();
> > +			} else {
> > +				worker_stats_validate(ifobject);
> > +			}
> >
> >  			if (sigvar)
> >  				break;
> >  		}
> >
> > -		print_verbose("Received %d packets on interface %s\n",
> > -			       pkt_counter, ifobject->ifname);
> > +		if (test_type != TEST_TYPE_STATS)
> > +			print_verbose("Received %d packets on interface
> %s\n",
> > +				pkt_counter, ifobject->ifname);
> >
> >  		if (test_type == TEST_TYPE_TEARDOWN)
> >  			print_verbose("Destroying socket\n");
> > @@ -921,7 +988,7 @@ static void testapp_validate(void)
> >  		free(pkt_buf);
> >  	}
> >
> > -	if (!(test_type == TEST_TYPE_TEARDOWN) && !bidi)
> > +	if (!(test_type == TEST_TYPE_TEARDOWN) && !bidi && !(test_type
> == TEST_TYPE_STATS))
> >  		print_ksft_result();
> >  }
> >
> > @@ -940,6 +1007,34 @@ static void testapp_sockets(void)
> >  	print_ksft_result();
> >  }
> >
> > +static void testapp_stats(void)
> > +{
> > +	for (int i = 0; i < STAT_TEST_TYPE_MAX; i++) {
> > +		stat_test_type = i;
> > +
> > +		/* reset defaults */
> > +		rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> > +		frame_headroom =
> XSK_UMEM__DEFAULT_FRAME_HEADROOM;
> > +
> > +		switch (stat_test_type) {
> > +		case STAT_TEST_RX_DROPPED:
> > +			frame_headroom =
> XSK_UMEM__DEFAULT_FRAME_SIZE -
> > +						XDP_PACKET_HEADROOM -
> 1;
> > +			break;
> > +		case STAT_TEST_RX_FULL:
> > +			rxqsize = RX_FULL_RXQSIZE;
> > +			break;
> > +		default:
> > +			break;
> > +		}
> > +		pthread_init_mutex();
> 
> Do you really have to init/destroy mutexes per each iteration?

Good point. We don't. Will remove in v2.

> 
> > +		testapp_validate();
> > +		pthread_destroy_mutex();
> > +	}
> > +
> > +	print_ksft_result();
> > +}
> > +
> >  static void init_iface_config(struct ifaceconfigobj *ifaceconfig)
> >  {
> >  	/*Init interface0 */
> > @@ -1021,6 +1116,10 @@ static void run_pkt_test(int mode, int type)
> >  	prev_pkt = -1;
> >  	ifdict[0]->fv.vector = tx;
> >  	ifdict[1]->fv.vector = rx;
> > +	sigvar = 0;
> > +	stat_test_type = -1;
> > +	rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> > +	frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
> >
> >  	switch (mode) {
> >  	case (TEST_MODE_SKB):
> > @@ -1039,6 +1138,11 @@ static void run_pkt_test(int mode, int type)
> >  		break;
> >  	}
> >
> > +	if (test_type == TEST_TYPE_STATS) {
> > +		testapp_stats();
> > +		return;
> > +	}
> 
> Why this can't be a part of if/else if/else branch that below is picking
> the test type?

Had this here since we were initializing and destroying the mutexes for each individual stats test.
Since that's no longer needed, this can be brought inline with the if/else branch below.

Thanks for the feedback!
Ciara

> 
> > +
> >  	pthread_init_mutex();
> >
> >  	if ((test_type != TEST_TYPE_TEARDOWN) && (test_type !=
> TEST_TYPE_BIDI))
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.h
> b/tools/testing/selftests/bpf/xdpxceiver.h
> > index 1127a396d5d0..4d0a80dbfef0 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.h
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> > @@ -41,6 +41,7 @@
> >  #define BATCH_SIZE 64
> >  #define POLL_TMOUT 1000
> >  #define NEED_WAKEUP true
> > +#define RX_FULL_RXQSIZE 32
> >
> >  #define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); }
> while (0)
> >
> > @@ -59,9 +60,18 @@ enum TEST_TYPES {
> >  	TEST_TYPE_POLL,
> >  	TEST_TYPE_TEARDOWN,
> >  	TEST_TYPE_BIDI,
> > +	TEST_TYPE_STATS,
> >  	TEST_TYPE_MAX
> >  };
> >
> > +enum STAT_TEST_TYPES {
> > +	STAT_TEST_RX_DROPPED,
> > +	STAT_TEST_TX_INVALID,
> > +	STAT_TEST_RX_FULL,
> > +	STAT_TEST_RX_FILL_EMPTY,
> > +	STAT_TEST_TYPE_MAX
> > +};
> > +
> >  static u8 uut;
> >  static u8 debug_pkt_dump;
> >  static u32 num_frames;
> > @@ -80,6 +90,9 @@ static u8
> pkt_data[XSK_UMEM__DEFAULT_FRAME_SIZE];
> >  static u32 pkt_counter;
> >  static long prev_pkt = -1;
> >  static int sigvar;
> > +static int stat_test_type;
> > +static u32 rxqsize;
> > +static u32 frame_headroom;
> >
> >  struct xsk_umem_info {
> >  	struct xsk_ring_prod fq;
> > --
> > 2.17.1
> >

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

* RE: [PATCH bpf-next 3/4] selftests/bpf: restructure xsk selftests
  2021-02-22 11:23   ` Maciej Fijalkowski
@ 2021-02-23  8:24     ` Loftus, Ciara
  0 siblings, 0 replies; 13+ messages in thread
From: Loftus, Ciara @ 2021-02-23  8:24 UTC (permalink / raw)
  To: Fijalkowski, Maciej, bpf
  Cc: netdev, Karlsson, Magnus, bjorn, Janjua, Weqaar A

> 
> On Wed, Feb 17, 2021 at 04:02:13PM +0000, Ciara Loftus wrote:
> > Prior to this commit individual xsk tests were launched from the
> > shell script 'test_xsk.sh'. When adding a new test type, two new test
> > configurations had to be added to this file - one for each of the
> > supported XDP 'modes' (skb or drv). Should zero copy support be added to
> > the xsk selftest framework in the future, three new test configurations
> > would need to be added for each new test type. Each new test type also
> > typically requires new CLI arguments for the xdpxceiver program.
> >
> > This commit aims to reduce the overhead of adding new tests, by launching
> > the test configurations from within the xdpxceiver program itself, using
> > simple loops. Every test is run every time the C program is executed. Many
> > of the CLI arguments can be removed as a result.
> >
> > Signed-off-by: Ciara Loftus <ciara.loftus@intel.com>
> > ---
> >  tools/testing/selftests/bpf/test_xsk.sh    | 112 +-----------
> >  tools/testing/selftests/bpf/xdpxceiver.c   | 199 ++++++++++++---------
> >  tools/testing/selftests/bpf/xdpxceiver.h   |  27 ++-
> >  tools/testing/selftests/bpf/xsk_prereqs.sh |  24 +--
> >  4 files changed, 139 insertions(+), 223 deletions(-)
> >
> 
> Good cleanup! I have a series of fixes/cleanups as well and I need to
> introduce a new test over here, so your work makes it easier for me.
> 
> One nit below and once you address Bjorn's request, then feel free to add
> my:

Thanks Björn and Maciej for the feedback. Will include your suggestions in the v2.
I discovered some extra things to tweak for the v2 but hope to have it up shortly.

Thanks,
Ciara

> 
> Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> 
> [...]
> 
> > +static int configure_skb(void)
> > +{
> > +
> > +	char cmd[80];
> > +
> > +	snprintf(cmd, sizeof(cmd), "ip link set dev %s xdpdrv off", ifdict[0]-
> >ifname);
> > +	if (system(cmd)) {
> > +		ksft_test_result_fail("Failed to configure native mode on
> iface %s\n",
> > +						ifdict[0]->ifname);
> > +		return -1;
> > +	}
> > +	snprintf(cmd, sizeof(cmd), "ip netns exec %s ip link set dev %s
> xdpdrv off",
> > +					ifdict[1]->nsname, ifdict[1]->ifname);
> > +	if (system(cmd)) {
> > +		ksft_test_result_fail("Failed to configure native mode on
> iface/ns %s\n",
> > +						ifdict[1]->ifname, ifdict[1]-
> >nsname);
> > +		return -1;
> > +	}
> > +
> > +	cur_mode = TEST_MODE_SKB;
> > +
> > +	return 0;
> > +
> > +}
> > +
> > +static int configure_drv(void)
> > +{
> > +	char cmd[80];
> > +
> > +	snprintf(cmd, sizeof(cmd), "ip link set dev %s xdpgeneric off",
> ifdict[0]->ifname);
> > +	if (system(cmd)) {
> > +		ksft_test_result_fail("Failed to configure native mode on
> iface %s\n",
> > +						ifdict[0]->ifname);
> > +		return -1;
> > +	}
> > +	snprintf(cmd, sizeof(cmd), "ip netns exec %s ip link set dev %s
> xdpgeneric off",
> > +					ifdict[1]->nsname, ifdict[1]->ifname);
> > +	if (system(cmd)) {
> > +		ksft_test_result_fail("Failed to configure native mode on
> iface/ns %s\n",
> > +						ifdict[1]->ifname, ifdict[1]-
> >nsname);
> > +		return -1;
> > +	}
> > +
> > +	cur_mode = TEST_MODE_DRV;
> > +
> > +	return 0;
> > +}
> > +
> > +static void run_pkt_test(int mode, int type)
> > +{
> > +	test_type = type;
> > +
> > +	/* reset defaults after potential previous test */
> > +	xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> > +	pkt_counter = 0;
> > +	switching_notify = 0;
> > +	bidi_pass = 0;
> > +	prev_pkt = -1;
> > +	ifdict[0]->fv.vector = tx;
> > +	ifdict[1]->fv.vector = rx;
> > +
> > +	switch (mode) {
> > +	case (TEST_MODE_SKB):
> > +		if (cur_mode != TEST_MODE_SKB)
> > +			configure_skb();
> 
> Should you check a return value over here?
> 
> > +		xdp_flags |= XDP_FLAGS_SKB_MODE;
> > +		uut = TEST_MODE_SKB;
> > +		break;
> > +	case (TEST_MODE_DRV):
> > +		if (cur_mode != TEST_MODE_DRV)
> > +			configure_drv();
> 
> ditto
> 
> > +		xdp_flags |= XDP_FLAGS_DRV_MODE;
> > +		uut = TEST_MODE_DRV;
> > +		break;
> > +	default:
> > +		break;
> > +	}
> > +
> > +	pthread_init_mutex();
> > +
> > +	if ((test_type != TEST_TYPE_TEARDOWN) && (test_type !=
> TEST_TYPE_BIDI))
> > +		testapp_validate();
> > +	else
> > +		testapp_sockets();
> > +
> > +	pthread_destroy_mutex();
> > +}
> > +
> >  int main(int argc, char **argv)
> >  {
> >  	struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
> > @@ -1021,6 +1062,7 @@ int main(int argc, char **argv)
> >  	const char *IP2 = "192.168.100.161";
> >  	u16 UDP_DST_PORT = 2020;
> >  	u16 UDP_SRC_PORT = 2121;
> > +	int i, j;
> >
> >  	ifaceconfig = malloc(sizeof(struct ifaceconfigobj));
> >  	memcpy(ifaceconfig->dst_mac, MAC1, ETH_ALEN);
> > @@ -1046,24 +1088,19 @@ int main(int argc, char **argv)
> >
> >  	init_iface_config(ifaceconfig);
> >
> > -	pthread_init_mutex();
> > +	ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
> >
> > -	ksft_set_plan(1);
> > +	configure_skb();
> > +	cur_mode = TEST_MODE_SKB;
> >
> > -	if (!opt_teardown && !opt_bidi) {
> > -		testapp_validate();
> > -	} else if (opt_teardown && opt_bidi) {
> > -		ksft_test_result_fail("ERROR: parameters -T and -B cannot
> be used together\n");
> > -		ksft_exit_xfail();
> > -	} else {
> > -		testapp_sockets();
> > +	for (i = 0; i < TEST_MODE_MAX; i++) {
> > +		for (j = 0; j < TEST_TYPE_MAX; j++)
> > +			run_pkt_test(i, j);
> >  	}
> >
> >  	for (int i = 0; i < MAX_INTERFACES; i++)
> >  		free(ifdict[i]);
> >
> > -	pthread_destroy_mutex();
> > -
> >  	ksft_exit_pass();
> >
> >  	return 0;

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

end of thread, other threads:[~2021-02-23  8:24 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-02-17 16:02 [PATCH bpf-next 0/4] selftests/bpf: xsk improvements and new stats tests Ciara Loftus
2021-02-17 16:02 ` [PATCH bpf-next 1/4] selftest/bpf: make xsk tests less verbose Ciara Loftus
2021-02-22 11:58   ` Maciej Fijalkowski
2021-02-17 16:02 ` [PATCH bpf-next 2/4] selftests/bpf: expose debug arg to shell script for xsk tests Ciara Loftus
2021-02-19 13:12   ` Magnus Karlsson
2021-02-22 14:29     ` Loftus, Ciara
2021-02-17 16:02 ` [PATCH bpf-next 3/4] selftests/bpf: restructure xsk selftests Ciara Loftus
2021-02-18  9:33   ` Björn Töpel
2021-02-22 11:23   ` Maciej Fijalkowski
2021-02-23  8:24     ` Loftus, Ciara
2021-02-17 16:02 ` [PATCH bpf-next 4/4] selftests/bpf: introduce xsk statistics tests Ciara Loftus
2021-02-22 11:56   ` Maciej Fijalkowski
2021-02-22 14:33     ` Loftus, Ciara

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