All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests
@ 2021-09-01 10:47 Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 01/20] selftests: xsk: simplify xsk and umem arrays Magnus Karlsson
                   ` (19 more replies)
  0 siblings, 20 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: Magnus Karlsson, jonathan.lemon, ciara.loftus, bpf, yhs, andrii

This patch set facilitates adding new tests as well as describing
existing ones in the xsk selftests suite and adds 3 new test suites at
the end. The idea is to isolate the run-time that executes the test
from the actual implementation of the test. Today, implementing a test
amounts to adding test specific if-statements all around the run-time,
which is not scalable or amenable for reuse. This patch set instead
introduces a test specification that is the only thing that a test
fills in. The run-time then gets this specification and acts upon it
completely unaware of what test it is executing. This way, we can get
rid of all test specific if-statements from the run-time and the
implementation of the test can be contained in a single function. This
hopefully makes it easier to add tests and for users to understand
what the test accomplishes.

As a recap of what the run-time does: each test is based on the
run-time launching two threads and connecting a veth link between the
two threads. Each thread opens an AF_XDP socket on that veth interface
and one of them sends traffic that the other one receives and
validates. Each thread has its own umem. Note that this behavior is
not changed by this patch set.

A test specification consists of several items. Most importantly:

* Two packet streams. One for Tx thread that specifies what traffic to
  send and one for the Rx thread that specifies what that thread
  should receive. If it receives exactly what is specified, the test
  passes, otherwise it fails. A packet stream can also specify what
  buffers in the umem that should be used by the Rx and Tx threads.

* What kind of AF_XDP sockets it should create and bind to what
  interfaces

* How many times it should repeat the socket creation and destruction

* The name of the test

The interface for the test spec is the following:

void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
                    struct ifobject *ifobj_rx, enum test_mode mode);

/* Reset everything but the interface specifications and the mode */
void test_spec_reset(struct test_spec *test);

void test_spec_set_name(struct test_spec *test, const char *name);


Packet streams have the following interfaces:

struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)

struct pkt *pkt_stream_get_next_rx_pkt(struct pkt_stream *pkt_stream)

struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem,
                                       u32 nb_pkts, u32 pkt_len);

void pkt_stream_delete(struct pkt_stream *pkt_stream);

struct pkt_stream *pkt_stream_clone(struct xsk_umem_info *umem,
                                    struct pkt_stream *pkt_stream);

/* Replaces all packets in the stream*/
void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len);

/* Replaces every other packet in the stream */
void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 offset);

/* For creating custom made packet streams */
void pkt_stream_set(struct test_spec *test, struct pkt *pkts, u32 nb_pkts);

/* Restores the default packet stream */
void pkt_stream_restore_default(struct test_spec *test);


A test can then then in the most basic case described like this
(provided the test specification has been created before calling the
function):

static bool testapp_aligned(struct test_spec *test)
{
        test_spec_set_name(test, "RUN_TO_COMPLETION");
        testapp_validate_traffic(test);
}

Running the same test in unaligned mode would then look like this:

static bool testapp_unaligned(struct test_spec *test)
{
        if (!hugepages_present(test->ifobj_tx)) {
                ksft_test_result_skip("No 2M huge pages present.\n");
                return false;
        }

        test_spec_set_name(test, "UNALIGNED_MODE");
        test->ifobj_tx->umem->unaligned_mode = true;
        test->ifobj_rx->umem->unaligned_mode = true;
        /* Let half of the packets straddle a buffer boundrary */
        pkt_stream_replace_half(test, PKT_SIZE,
                                XSK_UMEM__DEFAULT_FRAME_SIZE - 32);
	/* Populate fill ring with addresses in the packet stream */
        test->ifobj_rx->pkt_stream->use_addr_for_fill = true;
        testapp_validate_traffic(test);

        pkt_stream_restore_default(test);
	return true;
}

3 of the last 4 patches in the set add 3 new test suites, one for
unaligned mode, one for testing the rejection of tricky invalid
descriptors plus the acceptance of some valid ones in the Tx ring, and
one for testing 2K frame sizes (the default is 4K).

What is left to do for follow-up patches:

* Convert the statistics tests to the new framework.

* Implement a way of registering new tests without having the enum
  test_type. Once this has been done (together with the previous
  bullet), all the test types can be dropped from the header
  file. This means that we should be able to add tests by just writing
  a single function with a new test specification, which is one of the
  goals.

* Introduce functions for manipulating parts of the test or interface
  spec instead of direct manipulations such as
  test->ifobj_rx->pkt_stream->use_addr_for_fill = true; which is kind
  of awkward.

* Move the run-time and its interface to its own .c and .h files. Then
  we can have all the tests in a separate file.

* Better error reporting if a test fails. Today it does not state what
  test fails and might not continue execute the rest of the tests due
  to this failure. Failures are not propagated upwards through the
  functions so a failed test will also be a passed test, which messes
  up the stats counting. This needs to be changed.

* Add option to run specific test instead of all of them

* Introduce pacing of sent packets so that they are never dropped by
  the receiver even if it is stalled for some reason. If you run the
  current tests on a heavily loaded system, they might fail in SKB
  mode due to packets being dropped by the driver on Tx. Though I have
  never seen it, it might happen.

Thanks: Magnus

Magnus Karlsson (20):
  selftests: xsk: simplify xsk and umem arrays
  selftests: xsk: introduce type for thread function
  selftests: xsk: introduce test specifications
  selftests: xsk: move num_frames and frame_headroom to xsk_umem_info
  selftests: xsk: move rxqsize into xsk_socket_info
  selftests: xsk: make frame_size configurable
  selftests: xsx: introduce test name in test spec
  selftests: xsk: add use_poll to ifobject
  selftests: xsk: introduce rx_on and tx_on in ifobject
  selftests: xsk: replace second_step global variable
  selftests: xsk: specify number of sockets to create
  selftests: xsk: make xdp_flags and bind_flags local
  selftests: xsx: make pthreads local scope
  selftests: xsk: eliminate MAX_SOCKS define
  selftests: xsk: allow for invalid packets
  selftests: xsk: introduce replacing the default packet stream
  selftests: xsk: add test for unaligned mode
  selftests: xsk: eliminate test specific if-statement in test runner
  selftests: xsk: add tests for invalid xsk descriptors
  selftests: xsk: add tests for 2K frame size

 tools/testing/selftests/bpf/xdpxceiver.c | 870 +++++++++++++++--------
 tools/testing/selftests/bpf/xdpxceiver.h |  66 +-
 2 files changed, 606 insertions(+), 330 deletions(-)


base-commit: 9e9fb7655ed585da8f468e29221f0ba194a5f613
--
2.29.0

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

* [PATCH bpf-next 01/20] selftests: xsk: simplify xsk and umem arrays
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-03 17:53   ` John Fastabend
  2021-09-01 10:47 ` [PATCH bpf-next 02/20] selftests: xsk: introduce type for thread function Magnus Karlsson
                   ` (18 subsequent siblings)
  19 siblings, 1 reply; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Simplify the xsk_info and umem_info allocation by allocating them
upfront in an array, instead of allocating an array of pointers to
future creations of these. Allocating them upfront also has the
advantage that configuration information can be stored in these
structures instead of relying on global variables. With the previous
structure, xsk_info and umem_info were created too late to be able to
store most configuration information. This will be used to eliminate
most global variables in later patches in this series.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index f53ce2683f8d..9639d8da516d 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -235,7 +235,7 @@ static void gen_udp_csum(struct udphdr *udp_hdr, struct iphdr *ip_hdr)
 	    udp_csum(ip_hdr->saddr, ip_hdr->daddr, UDP_PKT_SIZE, IPPROTO_UDP, (u16 *)udp_hdr);
 }
 
-static void xsk_configure_umem(struct ifobject *data, void *buffer, u64 size, int idx)
+static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size, int idx)
 {
 	struct xsk_umem_config cfg = {
 		.fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
@@ -244,21 +244,15 @@ static void xsk_configure_umem(struct ifobject *data, void *buffer, u64 size, in
 		.frame_headroom = frame_headroom,
 		.flags = XSK_UMEM__DEFAULT_FLAGS
 	};
-	struct xsk_umem_info *umem;
 	int ret;
 
-	umem = calloc(1, sizeof(struct xsk_umem_info));
-	if (!umem)
-		exit_with_error(errno);
-
 	ret = xsk_umem__create(&umem->umem, buffer, size,
 			       &umem->fq, &umem->cq, &cfg);
 	if (ret)
-		exit_with_error(-ret);
+		return ret;
 
 	umem->buffer = buffer;
-
-	data->umem_arr[idx] = umem;
+	return 0;
 }
 
 static void xsk_populate_fill_ring(struct xsk_umem_info *umem)
@@ -274,19 +268,14 @@ static void xsk_populate_fill_ring(struct xsk_umem_info *umem)
 	xsk_ring_prod__submit(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS);
 }
 
-static int xsk_configure_socket(struct ifobject *ifobject, int idx)
+static int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_info *umem,
+				struct ifobject *ifobject, u32 qid)
 {
 	struct xsk_socket_config cfg;
-	struct xsk_socket_info *xsk;
 	struct xsk_ring_cons *rxr;
 	struct xsk_ring_prod *txr;
-	int ret;
 
-	xsk = calloc(1, sizeof(struct xsk_socket_info));
-	if (!xsk)
-		exit_with_error(errno);
-
-	xsk->umem = ifobject->umem;
+	xsk->umem = umem;
 	cfg.rx_size = rxqsize;
 	cfg.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
 	cfg.libbpf_flags = 0;
@@ -301,14 +290,7 @@ static int xsk_configure_socket(struct ifobject *ifobject, int idx)
 		txr = &xsk->tx;
 	}
 
-	ret = xsk_socket__create(&xsk->xsk, ifobject->ifname, idx,
-				 ifobject->umem->umem, rxr, txr, &cfg);
-	if (ret)
-		return 1;
-
-	ifobject->xsk_arr[idx] = xsk;
-
-	return 0;
+	return xsk_socket__create(&xsk->xsk, ifobject->ifname, qid, umem->umem, rxr, txr, &cfg);
 }
 
 static struct option long_options[] = {
@@ -756,8 +738,7 @@ static void thread_common_ops(struct ifobject *ifobject, void *bufs)
 	u64 umem_sz = num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE;
 	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
 	size_t mmap_sz = umem_sz;
-	int ctr = 0;
-	int ret;
+	int ctr = 0, ret;
 
 	ifobject->ns_fd = switch_namespace(ifobject->nsname);
 
@@ -769,31 +750,34 @@ static void thread_common_ops(struct ifobject *ifobject, void *bufs)
 		exit_with_error(errno);
 
 	while (ctr++ < SOCK_RECONF_CTR) {
-		xsk_configure_umem(ifobject, bufs, umem_sz, 0);
-		ifobject->umem = ifobject->umem_arr[0];
-		ret = xsk_configure_socket(ifobject, 0);
+		ret = xsk_configure_umem(&ifobject->umem_arr[0], bufs, umem_sz, 0);
+		if (ret)
+			exit_with_error(-ret);
+
+		ret = xsk_configure_socket(&ifobject->xsk_arr[0], &ifobject->umem_arr[0],
+					   ifobject, 0);
 		if (!ret)
 			break;
 
 		/* Retry Create Socket if it fails as xsk_socket__create() is asynchronous */
-		usleep(USLEEP_MAX);
 		if (ctr >= SOCK_RECONF_CTR)
 			exit_with_error(-ret);
+		usleep(USLEEP_MAX);
 	}
 
-	ifobject->umem = ifobject->umem_arr[0];
-	ifobject->xsk = ifobject->xsk_arr[0];
-
 	if (test_type == TEST_TYPE_BPF_RES) {
-		xsk_configure_umem(ifobject, (u8 *)bufs + umem_sz, umem_sz, 1);
-		ifobject->umem = ifobject->umem_arr[1];
-		ret = xsk_configure_socket(ifobject, 1);
+		ret = xsk_configure_umem(&ifobject->umem_arr[1], (u8 *)bufs + umem_sz, umem_sz, 1);
+		if (ret)
+			exit_with_error(-ret);
+
+		ret = xsk_configure_socket(&ifobject->xsk_arr[1], &ifobject->umem_arr[1],
+					   ifobject, 1);
+		if (ret)
+			exit_with_error(-ret);
 	}
 
-	ifobject->umem = ifobject->umem_arr[0];
-	ifobject->xsk = ifobject->xsk_arr[0];
-	print_verbose("Interface [%s] vector [%s]\n",
-		      ifobject->ifname, ifobject->fv.vector == tx ? "Tx" : "Rx");
+	ifobject->umem = &ifobject->umem_arr[0];
+	ifobject->xsk = &ifobject->xsk_arr[0];
 }
 
 static bool testapp_is_test_two_stepped(void)
@@ -941,10 +925,10 @@ static void swap_xsk_res(void)
 	xsk_umem__delete(ifdict_tx->umem->umem);
 	xsk_socket__delete(ifdict_rx->xsk->xsk);
 	xsk_umem__delete(ifdict_rx->umem->umem);
-	ifdict_tx->umem = ifdict_tx->umem_arr[1];
-	ifdict_tx->xsk = ifdict_tx->xsk_arr[1];
-	ifdict_rx->umem = ifdict_rx->umem_arr[1];
-	ifdict_rx->xsk = ifdict_rx->xsk_arr[1];
+	ifdict_tx->umem = &ifdict_tx->umem_arr[1];
+	ifdict_tx->xsk = &ifdict_tx->xsk_arr[1];
+	ifdict_rx->umem = &ifdict_rx->umem_arr[1];
+	ifdict_rx->xsk = &ifdict_rx->xsk_arr[1];
 }
 
 static void testapp_bpf_res(void)
@@ -1071,11 +1055,11 @@ static struct ifobject *ifobject_create(void)
 	if (!ifobj)
 		return NULL;
 
-	ifobj->xsk_arr = calloc(2, sizeof(struct xsk_socket_info *));
+	ifobj->xsk_arr = calloc(MAX_SOCKETS, sizeof(*ifobj->xsk_arr));
 	if (!ifobj->xsk_arr)
 		goto out_xsk_arr;
 
-	ifobj->umem_arr = calloc(2, sizeof(struct xsk_umem_info *));
+	ifobj->umem_arr = calloc(MAX_SOCKETS, sizeof(*ifobj->umem_arr));
 	if (!ifobj->umem_arr)
 		goto out_umem_arr;
 
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 7e49b9fbe25e..de80516ac6c2 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -21,6 +21,7 @@
 #define MAX_INTERFACE_NAME_CHARS 7
 #define MAX_INTERFACES_NAMESPACE_CHARS 10
 #define MAX_SOCKS 1
+#define MAX_SOCKETS 2
 #define MAX_TEARDOWN_ITER 10
 #define MAX_BIDI_ITER 2
 #define MAX_BPF_ITER 2
@@ -119,9 +120,9 @@ struct ifobject {
 	char ifname[MAX_INTERFACE_NAME_CHARS];
 	char nsname[MAX_INTERFACES_NAMESPACE_CHARS];
 	struct xsk_socket_info *xsk;
-	struct xsk_socket_info **xsk_arr;
-	struct xsk_umem_info **umem_arr;
+	struct xsk_socket_info *xsk_arr;
 	struct xsk_umem_info *umem;
+	struct xsk_umem_info *umem_arr;
 	void *(*func_ptr)(void *arg);
 	struct flow_vector fv;
 	struct pkt_stream *pkt_stream;
-- 
2.29.0


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

* [PATCH bpf-next 02/20] selftests: xsk: introduce type for thread function
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 01/20] selftests: xsk: simplify xsk and umem arrays Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-03 17:56   ` John Fastabend
  2021-09-01 10:47 ` [PATCH bpf-next 03/20] selftests: xsk: introduce test specifications Magnus Karlsson
                   ` (17 subsequent siblings)
  19 siblings, 1 reply; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Introduce a typedef of the thread function so this can be passed to
init_iface() in order to simplify that function.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 9639d8da516d..edf5b6cc6998 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -974,10 +974,9 @@ static void testapp_stats(void)
 	print_ksft_result();
 }
 
-static void init_iface(struct ifobject *ifobj, const char *dst_mac,
-		       const char *src_mac, const char *dst_ip,
-		       const char *src_ip, const u16 dst_port,
-		       const u16 src_port, enum fvector vector)
+static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
+		       const char *dst_ip, const char *src_ip, const u16 dst_port,
+		       const u16 src_port, enum fvector vector, thread_func_t func_ptr)
 {
 	struct in_addr ip;
 
@@ -993,15 +992,13 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac,
 	ifobj->dst_port = dst_port;
 	ifobj->src_port = src_port;
 
-	if (vector == tx) {
-		ifobj->fv.vector = tx;
-		ifobj->func_ptr = worker_testapp_validate_tx;
+	if (vector == tx)
 		ifdict_tx = ifobj;
-	} else {
-		ifobj->fv.vector = rx;
-		ifobj->func_ptr = worker_testapp_validate_rx;
+	else
 		ifdict_rx = ifobj;
-	}
+
+	ifobj->fv.vector = vector;
+	ifobj->func_ptr = func_ptr;
 }
 
 static void run_pkt_test(int mode, int type)
@@ -1097,8 +1094,10 @@ int main(int argc, char **argv)
 
 	parse_command_line(argc, argv);
 
-	init_iface(ifdict[tx], MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2, tx);
-	init_iface(ifdict[rx], MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1, rx);
+	init_iface(ifdict[tx], MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2, tx,
+		   worker_testapp_validate_tx);
+	init_iface(ifdict[rx], MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1, rx,
+		   worker_testapp_validate_rx);
 
 	ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
 
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index de80516ac6c2..799d524eb425 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -116,6 +116,8 @@ struct pkt_stream {
 	struct pkt *pkts;
 };
 
+typedef void *(*thread_func_t)(void *arg);
+
 struct ifobject {
 	char ifname[MAX_INTERFACE_NAME_CHARS];
 	char nsname[MAX_INTERFACES_NAMESPACE_CHARS];
@@ -123,8 +125,8 @@ struct ifobject {
 	struct xsk_socket_info *xsk_arr;
 	struct xsk_umem_info *umem;
 	struct xsk_umem_info *umem_arr;
-	void *(*func_ptr)(void *arg);
 	struct flow_vector fv;
+	thread_func_t func_ptr;
 	struct pkt_stream *pkt_stream;
 	int ns_fd;
 	u32 dst_ip;
-- 
2.29.0


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

* [PATCH bpf-next 03/20] selftests: xsk: introduce test specifications
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 01/20] selftests: xsk: simplify xsk and umem arrays Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 02/20] selftests: xsk: introduce type for thread function Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 04/20] selftests: xsk: move num_frames and frame_headroom to xsk_umem_info Magnus Karlsson
                   ` (16 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Introduce a test specification to be able to concisely describe a
test. Currently, a test is implemented by sprinkling test specific if
statements here and there, which is not scalable or easy to
understand. The end goal with this patch set is to come to the point
in which a test is completely specified by a test specification that
can easily be constructed in a single function so that new tests can
be added without too much trouble. This test specification will be run
by a test runner that has no idea about tests. It just executes the
what test specification states.

This patch introduces the test specification and, as a start, puts the
two interface objects in there, one containing the packet stream to be
sent and the other one the packet stream that is supposed to be
received for a test to pass. The global variables containing these can
then be eliminated. The following patches will convert each existing
test into a test specification and add the needed fields into it and
the functionality in the test runner that act on the test
specification. At the end, the test runner should contain no test
specific code and each test should be described in a single simple
function.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index edf5b6cc6998..a6bcc7453860 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -336,45 +336,43 @@ static int switch_namespace(const char *nsname)
 	return nsfd;
 }
 
-static int validate_interfaces(void)
+static bool validate_interface(struct ifobject *ifobj)
 {
-	bool ret = true;
-
-	for (int i = 0; i < MAX_INTERFACES; i++) {
-		if (!strcmp(ifdict[i]->ifname, "")) {
-			ret = false;
-			ksft_test_result_fail("ERROR: interfaces: -i <int>,<ns> -i <int>,<ns>.");
-		}
-	}
-	return ret;
+	if (!strcmp(ifobj->ifname, ""))
+		return false;
+	return true;
 }
 
-static void parse_command_line(int argc, char **argv)
+static void parse_command_line(struct test_spec *test, int argc, char **argv)
 {
-	int option_index, interface_index = 0, c;
+	struct ifobject *ifobj;
+	u32 interface_nb = 0;
+	int option_index, c;
 
 	opterr = 0;
 
 	for (;;) {
-		c = getopt_long(argc, argv, "i:Dv", long_options, &option_index);
+		char *sptr, *token;
 
+		c = getopt_long(argc, argv, "i:Dv", long_options, &option_index);
 		if (c == -1)
 			break;
 
 		switch (c) {
 		case 'i':
-			if (interface_index == MAX_INTERFACES)
+			if (interface_nb == 0)
+				ifobj = test->ifobj_tx;
+			else if (interface_nb == 1)
+				ifobj = test->ifobj_rx;
+			else
 				break;
-			char *sptr, *token;
 
 			sptr = strndupa(optarg, strlen(optarg));
-			memcpy(ifdict[interface_index]->ifname,
-			       strsep(&sptr, ","), MAX_INTERFACE_NAME_CHARS);
+			memcpy(ifobj->ifname, strsep(&sptr, ","), MAX_INTERFACE_NAME_CHARS);
 			token = strsep(&sptr, ",");
 			if (token)
-				memcpy(ifdict[interface_index]->nsname, token,
-				       MAX_INTERFACES_NAMESPACE_CHARS);
-			interface_index++;
+				memcpy(ifobj->nsname, token, MAX_INTERFACES_NAMESPACE_CHARS);
+			interface_nb++;
 			break;
 		case 'D':
 			opt_pkt_dump = true;
@@ -387,11 +385,44 @@ static void parse_command_line(int argc, char **argv)
 			ksft_exit_xfail();
 		}
 	}
+}
 
-	if (!validate_interfaces()) {
-		usage(basename(argv[0]));
-		ksft_exit_xfail();
+static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
+			     struct ifobject *ifobj_rx)
+{
+	u32 i, j;
+
+	for (i = 0; i < MAX_INTERFACES; i++) {
+		struct ifobject *ifobj = i ? ifobj_rx : ifobj_tx;
+
+		ifobj->umem = &ifobj->umem_arr[0];
+		ifobj->xsk = &ifobj->xsk_arr[0];
+
+		if (i == tx)
+			ifobj->fv.vector = tx;
+		else
+			ifobj->fv.vector = rx;
+
+		for (j = 0; j < MAX_SOCKETS; j++) {
+			memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j]));
+			memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j]));
+		}
 	}
+
+	test->ifobj_tx = ifobj_tx;
+	test->ifobj_rx = ifobj_rx;
+}
+
+static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
+			   struct ifobject *ifobj_rx)
+{
+	memset(test, 0, sizeof(*test));
+	__test_spec_init(test, ifobj_tx, ifobj_rx);
+}
+
+static void test_spec_reset(struct test_spec *test)
+{
+	__test_spec_init(test, test->ifobj_tx, test->ifobj_rx);
 }
 
 static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
@@ -842,8 +873,10 @@ static void *worker_testapp_validate_rx(void *arg)
 	pthread_exit(NULL);
 }
 
-static void testapp_validate(void)
+static void testapp_validate_traffic(struct test_spec *test)
 {
+	struct ifobject *ifobj_tx = test->ifobj_tx;
+	struct ifobject *ifobj_rx = test->ifobj_rx;
 	bool bidi = test_type == TEST_TYPE_BIDI;
 	bool bpf = test_type == TEST_TYPE_BPF_RES;
 	struct pkt_stream *pkt_stream;
@@ -855,18 +888,18 @@ static void testapp_validate(void)
 		pkt_stream = pkt_stream_generate(DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
 	else
 		pkt_stream = pkt_stream_generate(DEFAULT_PKT_CNT, PKT_SIZE);
-	ifdict_tx->pkt_stream = pkt_stream;
-	ifdict_rx->pkt_stream = pkt_stream;
+	ifobj_tx->pkt_stream = pkt_stream;
+	ifobj_rx->pkt_stream = pkt_stream;
 
 	/*Spawn RX thread */
-	pthread_create(&t0, NULL, ifdict_rx->func_ptr, ifdict_rx);
+	pthread_create(&t0, NULL, ifobj_rx->func_ptr, ifobj_rx);
 
 	pthread_barrier_wait(&barr);
 	if (pthread_barrier_destroy(&barr))
 		exit_with_error(errno);
 
 	/*Spawn TX thread */
-	pthread_create(&t1, NULL, ifdict_tx->func_ptr, ifdict_tx);
+	pthread_create(&t1, NULL, ifobj_tx->func_ptr, ifobj_tx);
 
 	pthread_join(t1, NULL);
 	pthread_join(t0, NULL);
@@ -875,80 +908,82 @@ static void testapp_validate(void)
 		print_ksft_result();
 }
 
-static void testapp_teardown(void)
+static void testapp_teardown(struct test_spec *test)
 {
 	int i;
 
 	for (i = 0; i < MAX_TEARDOWN_ITER; i++) {
-		print_verbose("Creating socket\n");
-		testapp_validate();
+		testapp_validate_traffic(test);
+		test_spec_reset(test);
 	}
 
 	print_ksft_result();
 }
 
-static void swap_vectors(struct ifobject *ifobj1, struct ifobject *ifobj2)
+static void swap_directions(struct ifobject **ifobj1, struct ifobject **ifobj2)
 {
-	void *(*tmp_func_ptr)(void *) = ifobj1->func_ptr;
-	enum fvector tmp_vector = ifobj1->fv.vector;
+	thread_func_t tmp_func_ptr = (*ifobj1)->func_ptr;
+	enum fvector tmp_vector = (*ifobj1)->fv.vector;
+	struct ifobject *tmp_ifobj = (*ifobj1);
 
-	ifobj1->func_ptr = ifobj2->func_ptr;
-	ifobj1->fv.vector = ifobj2->fv.vector;
+	(*ifobj1)->func_ptr = (*ifobj2)->func_ptr;
+	(*ifobj1)->fv.vector = (*ifobj2)->fv.vector;
 
-	ifobj2->func_ptr = tmp_func_ptr;
-	ifobj2->fv.vector = tmp_vector;
+	(*ifobj2)->func_ptr = tmp_func_ptr;
+	(*ifobj2)->fv.vector = tmp_vector;
 
-	ifdict_tx = ifobj1;
-	ifdict_rx = ifobj2;
+	*ifobj1 = *ifobj2;
+	*ifobj2 = tmp_ifobj;
 }
 
-static void testapp_bidi(void)
+static void testapp_bidi(struct test_spec *test)
 {
 	for (int i = 0; i < MAX_BIDI_ITER; i++) {
 		print_verbose("Creating socket\n");
-		testapp_validate();
+		testapp_validate_traffic(test);
 		if (!second_step) {
 			print_verbose("Switching Tx/Rx vectors\n");
-			swap_vectors(ifdict[1], ifdict[0]);
+			swap_directions(&test->ifobj_rx, &test->ifobj_tx);
 		}
 		second_step = true;
 	}
 
-	swap_vectors(ifdict[0], ifdict[1]);
+	swap_directions(&test->ifobj_rx, &test->ifobj_tx);
 
 	print_ksft_result();
 }
 
-static void swap_xsk_res(void)
+static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj_rx)
 {
-	xsk_socket__delete(ifdict_tx->xsk->xsk);
-	xsk_umem__delete(ifdict_tx->umem->umem);
-	xsk_socket__delete(ifdict_rx->xsk->xsk);
-	xsk_umem__delete(ifdict_rx->umem->umem);
-	ifdict_tx->umem = &ifdict_tx->umem_arr[1];
-	ifdict_tx->xsk = &ifdict_tx->xsk_arr[1];
-	ifdict_rx->umem = &ifdict_rx->umem_arr[1];
-	ifdict_rx->xsk = &ifdict_rx->xsk_arr[1];
+	xsk_socket__delete(ifobj_tx->xsk->xsk);
+	xsk_umem__delete(ifobj_tx->umem->umem);
+	xsk_socket__delete(ifobj_rx->xsk->xsk);
+	xsk_umem__delete(ifobj_rx->umem->umem);
+	ifobj_tx->umem = &ifobj_tx->umem_arr[1];
+	ifobj_tx->xsk = &ifobj_tx->xsk_arr[1];
+	ifobj_rx->umem = &ifobj_rx->umem_arr[1];
+	ifobj_rx->xsk = &ifobj_rx->xsk_arr[1];
 }
 
-static void testapp_bpf_res(void)
+static void testapp_bpf_res(struct test_spec *test)
 {
 	int i;
 
 	for (i = 0; i < MAX_BPF_ITER; i++) {
 		print_verbose("Creating socket\n");
-		testapp_validate();
+		testapp_validate_traffic(test);
 		if (!second_step)
-			swap_xsk_res();
+			swap_xsk_resources(test->ifobj_tx, test->ifobj_rx);
 		second_step = true;
 	}
 
 	print_ksft_result();
 }
 
-static void testapp_stats(void)
+static void testapp_stats(struct test_spec *test)
 {
 	for (int i = 0; i < STAT_TEST_TYPE_MAX; i++) {
+		test_spec_reset(test);
 		stat_test_type = i;
 
 		/* reset defaults */
@@ -968,7 +1003,7 @@ static void testapp_stats(void)
 		default:
 			break;
 		}
-		testapp_validate();
+		testapp_validate_traffic(test);
 	}
 
 	print_ksft_result();
@@ -992,16 +1027,11 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
 	ifobj->dst_port = dst_port;
 	ifobj->src_port = src_port;
 
-	if (vector == tx)
-		ifdict_tx = ifobj;
-	else
-		ifdict_rx = ifobj;
-
 	ifobj->fv.vector = vector;
 	ifobj->func_ptr = func_ptr;
 }
 
-static void run_pkt_test(int mode, int type)
+static void run_pkt_test(struct test_spec *test, int mode, int type)
 {
 	test_type = type;
 
@@ -1027,19 +1057,19 @@ static void run_pkt_test(int mode, int type)
 
 	switch (test_type) {
 	case TEST_TYPE_STATS:
-		testapp_stats();
+		testapp_stats(test);
 		break;
 	case TEST_TYPE_TEARDOWN:
-		testapp_teardown();
+		testapp_teardown(test);
 		break;
 	case TEST_TYPE_BIDI:
-		testapp_bidi();
+		testapp_bidi(test);
 		break;
 	case TEST_TYPE_BPF_RES:
-		testapp_bpf_res();
+		testapp_bpf_res(test);
 		break;
 	default:
-		testapp_validate();
+		testapp_validate_traffic(test);
 		break;
 	}
 }
@@ -1079,36 +1109,47 @@ static void ifobject_delete(struct ifobject *ifobj)
 int main(int argc, char **argv)
 {
 	struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
-	int i, j;
+	struct ifobject *ifobj_tx, *ifobj_rx;
+	struct test_spec test;
+	u32 i, j;
 
 	if (setrlimit(RLIMIT_MEMLOCK, &_rlim))
 		exit_with_error(errno);
 
-	for (i = 0; i < MAX_INTERFACES; i++) {
-		ifdict[i] = ifobject_create();
-		if (!ifdict[i])
-			exit_with_error(ENOMEM);
-	}
+	ifobj_tx = ifobject_create();
+	if (!ifobj_tx)
+		exit_with_error(ENOMEM);
+	ifobj_rx = ifobject_create();
+	if (!ifobj_rx)
+		exit_with_error(ENOMEM);
+
+	test_spec_init(&test, ifobj_tx, ifobj_rx);
 
 	setlocale(LC_ALL, "");
 
-	parse_command_line(argc, argv);
+	parse_command_line(&test, argc, argv);
+
+	if (!validate_interface(ifobj_tx) || !validate_interface(ifobj_rx)) {
+		usage(basename(argv[0]));
+		ksft_exit_xfail();
+	}
 
-	init_iface(ifdict[tx], MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2, tx,
+	init_iface(ifobj_tx, MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2, tx,
 		   worker_testapp_validate_tx);
-	init_iface(ifdict[rx], MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1, rx,
+	init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1, rx,
 		   worker_testapp_validate_rx);
 
 	ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
 
 	for (i = 0; i < TEST_MODE_MAX; i++)
 		for (j = 0; j < TEST_TYPE_MAX; j++) {
-			run_pkt_test(i, j);
+			test_spec_init(&test, ifobj_tx, ifobj_rx);
+			run_pkt_test(&test, i, j);
 			usleep(USLEEP_MAX);
 		}
 
-	for (i = 0; i < MAX_INTERFACES; i++)
-		ifobject_delete(ifdict[i]);
+	ifobject_delete(ifobj_tx);
+	ifobject_delete(ifobj_rx);
 
 	ksft_exit_pass();
 	return 0;
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 799d524eb425..e279aa893438 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -137,9 +137,10 @@ struct ifobject {
 	u8 src_mac[ETH_ALEN];
 };
 
-static struct ifobject *ifdict[MAX_INTERFACES];
-static struct ifobject *ifdict_rx;
-static struct ifobject *ifdict_tx;
+struct test_spec {
+	struct ifobject *ifobj_tx;
+	struct ifobject *ifobj_rx;
+};
 
 /*threads*/
 pthread_barrier_t barr;
-- 
2.29.0


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

* [PATCH bpf-next 04/20] selftests: xsk: move num_frames and frame_headroom to xsk_umem_info
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (2 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 03/20] selftests: xsk: introduce test specifications Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 05/20] selftests: xsk: move rxqsize into xsk_socket_info Magnus Karlsson
                   ` (15 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Move the global variables num_frames and frame_headroom to struct
xsk_umem_info. They describe properties of the umem so no reason for
them to be global.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index a6bcc7453860..56ee03fda2b3 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -241,7 +241,7 @@ static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size
 		.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,
+		.frame_headroom = umem->frame_headroom,
 		.flags = XSK_UMEM__DEFAULT_FLAGS
 	};
 	int ret;
@@ -406,6 +406,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 		for (j = 0; j < MAX_SOCKETS; j++) {
 			memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j]));
 			memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j]));
+			ifobj->umem_arr[j].num_frames = DEFAULT_PKT_CNT / 4;
 		}
 	}
 
@@ -433,7 +434,7 @@ static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
 	return &pkt_stream->pkts[pkt_nb];
 }
 
-static struct pkt_stream *pkt_stream_generate(u32 nb_pkts, u32 pkt_len)
+static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
 {
 	struct pkt_stream *pkt_stream;
 	u32 i;
@@ -448,7 +449,7 @@ static struct pkt_stream *pkt_stream_generate(u32 nb_pkts, u32 pkt_len)
 
 	pkt_stream->nb_pkts = nb_pkts;
 	for (i = 0; i < nb_pkts; i++) {
-		pkt_stream->pkts[i].addr = (i % num_frames) * XSK_UMEM__DEFAULT_FRAME_SIZE;
+		pkt_stream->pkts[i].addr = (i % umem->num_frames) * XSK_UMEM__DEFAULT_FRAME_SIZE;
 		pkt_stream->pkts[i].len = pkt_len;
 		pkt_stream->pkts[i].payload = i;
 	}
@@ -766,7 +767,7 @@ static void tx_stats_validate(struct ifobject *ifobject)
 
 static void thread_common_ops(struct ifobject *ifobject, void *bufs)
 {
-	u64 umem_sz = num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE;
+	u64 umem_sz = ifobject->umem->num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE;
 	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
 	size_t mmap_sz = umem_sz;
 	int ctr = 0, ret;
@@ -885,9 +886,10 @@ static void testapp_validate_traffic(struct test_spec *test)
 		exit_with_error(errno);
 
 	if (stat_test_type == STAT_TEST_TX_INVALID)
-		pkt_stream = pkt_stream_generate(DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
+		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT,
+						 XSK_UMEM__INVALID_FRAME_SIZE);
 	else
-		pkt_stream = pkt_stream_generate(DEFAULT_PKT_CNT, PKT_SIZE);
+		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
 	ifobj_tx->pkt_stream = pkt_stream;
 	ifobj_rx->pkt_stream = pkt_stream;
 
@@ -988,12 +990,11 @@ static void testapp_stats(struct test_spec *test)
 
 		/* 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;
+			test->ifobj_rx->umem->frame_headroom = XSK_UMEM__DEFAULT_FRAME_SIZE -
+				XDP_PACKET_HEADROOM - 1;
 			break;
 		case STAT_TEST_RX_FULL:
 			rxqsize = RX_FULL_RXQSIZE;
@@ -1040,7 +1041,6 @@ static void run_pkt_test(struct test_spec *test, int mode, int type)
 	second_step = 0;
 	stat_test_type = -1;
 	rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
-	frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
 
 	configured_mode = mode;
 
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index e279aa893438..0d93a9e6c4f3 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -71,7 +71,6 @@ enum stat_test_type {
 
 static int configured_mode;
 static bool opt_pkt_dump;
-static u32 num_frames = DEFAULT_PKT_CNT / 4;
 static bool second_step;
 static int test_type;
 
@@ -81,12 +80,13 @@ static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static u32 xdp_bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
 static int stat_test_type;
 static u32 rxqsize;
-static u32 frame_headroom;
 
 struct xsk_umem_info {
 	struct xsk_ring_prod fq;
 	struct xsk_ring_cons cq;
 	struct xsk_umem *umem;
+	u32 num_frames;
+	u32 frame_headroom;
 	void *buffer;
 };
 
-- 
2.29.0


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

* [PATCH bpf-next 05/20] selftests: xsk: move rxqsize into xsk_socket_info
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (3 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 04/20] selftests: xsk: move num_frames and frame_headroom to xsk_umem_info Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 06/20] selftests: xsk: make frame_size configurable Magnus Karlsson
                   ` (14 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Move the global variable rxqsize to struct xsk_socket_info as it
describes the size of a ring in that struct. By default, it is set to
the size dictated by libbpf.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 56ee03fda2b3..28bf62c56190 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -276,7 +276,7 @@ static int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_inf
 	struct xsk_ring_prod *txr;
 
 	xsk->umem = umem;
-	cfg.rx_size = rxqsize;
+	cfg.rx_size = xsk->rxqsize;
 	cfg.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
 	cfg.libbpf_flags = 0;
 	cfg.xdp_flags = xdp_flags;
@@ -407,6 +407,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 			memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j]));
 			memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j]));
 			ifobj->umem_arr[j].num_frames = DEFAULT_PKT_CNT / 4;
+			ifobj->xsk_arr[j].rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
 		}
 	}
 
@@ -988,16 +989,13 @@ static void testapp_stats(struct test_spec *test)
 		test_spec_reset(test);
 		stat_test_type = i;
 
-		/* reset defaults */
-		rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
-
 		switch (stat_test_type) {
 		case STAT_TEST_RX_DROPPED:
 			test->ifobj_rx->umem->frame_headroom = XSK_UMEM__DEFAULT_FRAME_SIZE -
 				XDP_PACKET_HEADROOM - 1;
 			break;
 		case STAT_TEST_RX_FULL:
-			rxqsize = RX_FULL_RXQSIZE;
+			test->ifobj_rx->xsk->rxqsize = RX_FULL_RXQSIZE;
 			break;
 		case STAT_TEST_TX_INVALID:
 			continue;
@@ -1040,7 +1038,6 @@ static void run_pkt_test(struct test_spec *test, int mode, int type)
 	xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 	second_step = 0;
 	stat_test_type = -1;
-	rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
 
 	configured_mode = mode;
 
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 0d93a9e6c4f3..09e4e015b1bf 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -79,7 +79,6 @@ static bool opt_verbose;
 static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 static u32 xdp_bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
 static int stat_test_type;
-static u32 rxqsize;
 
 struct xsk_umem_info {
 	struct xsk_ring_prod fq;
@@ -96,6 +95,7 @@ struct xsk_socket_info {
 	struct xsk_umem_info *umem;
 	struct xsk_socket *xsk;
 	u32 outstanding_tx;
+	u32 rxqsize;
 };
 
 struct flow_vector {
-- 
2.29.0


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

* [PATCH bpf-next 06/20] selftests: xsk: make frame_size configurable
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (4 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 05/20] selftests: xsk: move rxqsize into xsk_socket_info Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 07/20] selftests: xsx: introduce test name in test spec Magnus Karlsson
                   ` (13 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Make the frame size configurable instead of it being hard coded to a
default. This is a property of the umem and will make it possible to
implement tests for different umem frame sizes in a later patch.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 28bf62c56190..79cf082a7581 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -240,7 +240,7 @@ static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size
 	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_size = umem->frame_size,
 		.frame_headroom = umem->frame_headroom,
 		.flags = XSK_UMEM__DEFAULT_FLAGS
 	};
@@ -264,7 +264,7 @@ static void xsk_populate_fill_ring(struct xsk_umem_info *umem)
 	if (ret != XSK_RING_PROD__DEFAULT_NUM_DESCS)
 		exit_with_error(-ret);
 	for (i = 0; i < XSK_RING_PROD__DEFAULT_NUM_DESCS; i++)
-		*xsk_ring_prod__fill_addr(&umem->fq, idx++) = i * XSK_UMEM__DEFAULT_FRAME_SIZE;
+		*xsk_ring_prod__fill_addr(&umem->fq, idx++) = i * umem->frame_size;
 	xsk_ring_prod__submit(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS);
 }
 
@@ -407,6 +407,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 			memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j]));
 			memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j]));
 			ifobj->umem_arr[j].num_frames = DEFAULT_PKT_CNT / 4;
+			ifobj->umem_arr[j].frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
 			ifobj->xsk_arr[j].rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
 		}
 	}
@@ -450,7 +451,7 @@ 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) * XSK_UMEM__DEFAULT_FRAME_SIZE;
+		pkt_stream->pkts[i].addr = (i % umem->num_frames) * umem->frame_size;
 		pkt_stream->pkts[i].len = pkt_len;
 		pkt_stream->pkts[i].payload = i;
 	}
@@ -768,7 +769,7 @@ static void tx_stats_validate(struct ifobject *ifobject)
 
 static void thread_common_ops(struct ifobject *ifobject, void *bufs)
 {
-	u64 umem_sz = ifobject->umem->num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE;
+	u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
 	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
 	size_t mmap_sz = umem_sz;
 	int ctr = 0, ret;
@@ -991,7 +992,7 @@ static void testapp_stats(struct test_spec *test)
 
 		switch (stat_test_type) {
 		case STAT_TEST_RX_DROPPED:
-			test->ifobj_rx->umem->frame_headroom = XSK_UMEM__DEFAULT_FRAME_SIZE -
+			test->ifobj_rx->umem->frame_headroom = test->ifobj_rx->umem->frame_size -
 				XDP_PACKET_HEADROOM - 1;
 			break;
 		case STAT_TEST_RX_FULL:
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 09e4e015b1bf..bfd14190abfc 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -87,6 +87,7 @@ struct xsk_umem_info {
 	u32 num_frames;
 	u32 frame_headroom;
 	void *buffer;
+	u32 frame_size;
 };
 
 struct xsk_socket_info {
-- 
2.29.0


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

* [PATCH bpf-next 07/20] selftests: xsx: introduce test name in test spec
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (5 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 06/20] selftests: xsk: make frame_size configurable Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 08/20] selftests: xsk: add use_poll to ifobject Magnus Karlsson
                   ` (12 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Introduce the test name in the test specification. This so we can set
the name locally in the test function and simplify the logic for
printing out test results.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 79cf082a7581..8ef58081d4d2 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -112,13 +112,9 @@ 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%s%s\n", configured_mode ? "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_STATS ? "Stats" : "",\
-			       test_type == TEST_TYPE_BPF_RES ? "BPF RES" : ""))
+#define print_ksft_result(test)\
+	(ksft_test_result_pass("PASS: %s %s\n", configured_mode ? "DRV" : "SKB", \
+			       (test)->name))
 
 static void memset32_htonl(void *dest, u32 val, u32 size)
 {
@@ -428,6 +424,11 @@ static void test_spec_reset(struct test_spec *test)
 	__test_spec_init(test, test->ifobj_tx, test->ifobj_rx);
 }
 
+static void test_spec_set_name(struct test_spec *test, const char *name)
+{
+	strncpy(test->name, name, MAX_TEST_NAME_SIZE);
+}
+
 static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
 {
 	if (pkt_nb >= pkt_stream->nb_pkts)
@@ -880,8 +881,6 @@ static void testapp_validate_traffic(struct test_spec *test)
 {
 	struct ifobject *ifobj_tx = test->ifobj_tx;
 	struct ifobject *ifobj_rx = test->ifobj_rx;
-	bool bidi = test_type == TEST_TYPE_BIDI;
-	bool bpf = test_type == TEST_TYPE_BPF_RES;
 	struct pkt_stream *pkt_stream;
 
 	if (pthread_barrier_init(&barr, NULL, 2))
@@ -907,21 +906,17 @@ static void testapp_validate_traffic(struct test_spec *test)
 
 	pthread_join(t1, NULL);
 	pthread_join(t0, NULL);
-
-	if (!(test_type == TEST_TYPE_TEARDOWN) && !bidi && !bpf && !(test_type == TEST_TYPE_STATS))
-		print_ksft_result();
 }
 
 static void testapp_teardown(struct test_spec *test)
 {
 	int i;
 
+	test_spec_set_name(test, "TEARDOWN");
 	for (i = 0; i < MAX_TEARDOWN_ITER; i++) {
 		testapp_validate_traffic(test);
 		test_spec_reset(test);
 	}
-
-	print_ksft_result();
 }
 
 static void swap_directions(struct ifobject **ifobj1, struct ifobject **ifobj2)
@@ -942,6 +937,7 @@ static void swap_directions(struct ifobject **ifobj1, struct ifobject **ifobj2)
 
 static void testapp_bidi(struct test_spec *test)
 {
+	test_spec_set_name(test, "BIDIRECTIONAL");
 	for (int i = 0; i < MAX_BIDI_ITER; i++) {
 		print_verbose("Creating socket\n");
 		testapp_validate_traffic(test);
@@ -953,8 +949,6 @@ static void testapp_bidi(struct test_spec *test)
 	}
 
 	swap_directions(&test->ifobj_rx, &test->ifobj_tx);
-
-	print_ksft_result();
 }
 
 static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj_rx)
@@ -973,6 +967,7 @@ static void testapp_bpf_res(struct test_spec *test)
 {
 	int i;
 
+	test_spec_set_name(test, "BPF_RES");
 	for (i = 0; i < MAX_BPF_ITER; i++) {
 		print_verbose("Creating socket\n");
 		testapp_validate_traffic(test);
@@ -980,8 +975,6 @@ static void testapp_bpf_res(struct test_spec *test)
 			swap_xsk_resources(test->ifobj_tx, test->ifobj_rx);
 		second_step = true;
 	}
-
-	print_ksft_result();
 }
 
 static void testapp_stats(struct test_spec *test)
@@ -992,21 +985,28 @@ static void testapp_stats(struct test_spec *test)
 
 		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;
 			break;
 		case STAT_TEST_RX_FULL:
+			test_spec_set_name(test, "STAT_RX_FULL");
 			test->ifobj_rx->xsk->rxqsize = RX_FULL_RXQSIZE;
 			break;
 		case STAT_TEST_TX_INVALID:
+			test_spec_set_name(test, "STAT_TX_INVALID");
 			continue;
+		case STAT_TEST_RX_FILL_EMPTY:
+			test_spec_set_name(test, "STAT_RX_FILL_EMPTY");
+			break;
 		default:
 			break;
 		}
 		testapp_validate_traffic(test);
 	}
 
-	print_ksft_result();
+	/* To only see the whole stat set being completed unless an individual test fails. */
+	test_spec_set_name(test, "STATS");
 }
 
 static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
@@ -1066,10 +1066,19 @@ static void run_pkt_test(struct test_spec *test, int mode, int type)
 	case TEST_TYPE_BPF_RES:
 		testapp_bpf_res(test);
 		break;
-	default:
+	case TEST_TYPE_NOPOLL:
+		test_spec_set_name(test, "RUN_TO_COMPLETION");
+		testapp_validate_traffic(test);
+		break;
+	case TEST_TYPE_POLL:
+		test_spec_set_name(test, "POLL");
 		testapp_validate_traffic(test);
 		break;
+	default:
+		break;
 	}
+
+	print_ksft_result(test);
 }
 
 static struct ifobject *ifobject_create(void)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index bfd14190abfc..15eab31b3b32 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -22,6 +22,7 @@
 #define MAX_INTERFACES_NAMESPACE_CHARS 10
 #define MAX_SOCKS 1
 #define MAX_SOCKETS 2
+#define MAX_TEST_NAME_SIZE 32
 #define MAX_TEARDOWN_ITER 10
 #define MAX_BIDI_ITER 2
 #define MAX_BPF_ITER 2
@@ -141,6 +142,7 @@ struct ifobject {
 struct test_spec {
 	struct ifobject *ifobj_tx;
 	struct ifobject *ifobj_rx;
+	char name[MAX_TEST_NAME_SIZE];
 };
 
 /*threads*/
-- 
2.29.0


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

* [PATCH bpf-next 08/20] selftests: xsk: add use_poll to ifobject
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (6 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 07/20] selftests: xsx: introduce test name in test spec Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 09/20] selftests: xsk: introduce rx_on and tx_on in ifobject Magnus Karlsson
                   ` (11 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Add a use_poll option to the ifobject so that we do not need to use a
test specific if-statement in the test runner.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 8ef58081d4d2..9a98c45933c5 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -393,6 +393,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 
 		ifobj->umem = &ifobj->umem_arr[0];
 		ifobj->xsk = &ifobj->xsk_arr[0];
+		ifobj->use_poll = false;
 
 		if (i == tx)
 			ifobj->fv.vector = tx;
@@ -684,7 +685,7 @@ static void send_pkts(struct ifobject *ifobject)
 	while (pkt_cnt < ifobject->pkt_stream->nb_pkts) {
 		u32 sent;
 
-		if (test_type == TEST_TYPE_POLL) {
+		if (ifobject->use_poll) {
 			int ret;
 
 			ret = poll(fds, 1, POLL_TMOUT);
@@ -1071,6 +1072,8 @@ static void run_pkt_test(struct test_spec *test, int mode, int type)
 		testapp_validate_traffic(test);
 		break;
 	case TEST_TYPE_POLL:
+		test->ifobj_tx->use_poll = true;
+		test->ifobj_rx->use_poll = true;
 		test_spec_set_name(test, "POLL");
 		testapp_validate_traffic(test);
 		break;
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 15eab31b3b32..e02a4dd71bfb 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -135,6 +135,7 @@ struct ifobject {
 	u32 src_ip;
 	u16 src_port;
 	u16 dst_port;
+	bool use_poll;
 	u8 dst_mac[ETH_ALEN];
 	u8 src_mac[ETH_ALEN];
 };
-- 
2.29.0


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

* [PATCH bpf-next 09/20] selftests: xsk: introduce rx_on and tx_on in ifobject
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (7 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 08/20] selftests: xsk: add use_poll to ifobject Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 10/20] selftests: xsk: replace second_step global variable Magnus Karlsson
                   ` (10 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Introduce rx_on and tx_on in the ifobject so that we can describe if
the thread should create a socket with only tx, rx, or both. This
eliminates some test specific if statements from the code. We can also
eliminate the flow vector structure now as this is fully specified
by the tx_on and rx_on variables.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 9a98c45933c5..a896d5845c0e 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -278,14 +278,8 @@ static int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_inf
 	cfg.xdp_flags = xdp_flags;
 	cfg.bind_flags = xdp_bind_flags;
 
-	if (test_type != TEST_TYPE_BIDI) {
-		rxr = (ifobject->fv.vector == rx) ? &xsk->rx : NULL;
-		txr = (ifobject->fv.vector == tx) ? &xsk->tx : NULL;
-	} else {
-		rxr = &xsk->rx;
-		txr = &xsk->tx;
-	}
-
+	txr = ifobject->tx_on ? &xsk->tx : NULL;
+	rxr = ifobject->rx_on ? &xsk->rx : NULL;
 	return xsk_socket__create(&xsk->xsk, ifobject->ifname, qid, umem->umem, rxr, txr, &cfg);
 }
 
@@ -395,10 +389,13 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 		ifobj->xsk = &ifobj->xsk_arr[0];
 		ifobj->use_poll = false;
 
-		if (i == tx)
-			ifobj->fv.vector = tx;
-		else
-			ifobj->fv.vector = rx;
+		if (i == 0) {
+			ifobj->rx_on = false;
+			ifobj->tx_on = true;
+		} else {
+			ifobj->rx_on = true;
+			ifobj->tx_on = false;
+		}
 
 		for (j = 0; j < MAX_SOCKETS; j++) {
 			memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j]));
@@ -923,14 +920,10 @@ static void testapp_teardown(struct test_spec *test)
 static void swap_directions(struct ifobject **ifobj1, struct ifobject **ifobj2)
 {
 	thread_func_t tmp_func_ptr = (*ifobj1)->func_ptr;
-	enum fvector tmp_vector = (*ifobj1)->fv.vector;
 	struct ifobject *tmp_ifobj = (*ifobj1);
 
 	(*ifobj1)->func_ptr = (*ifobj2)->func_ptr;
-	(*ifobj1)->fv.vector = (*ifobj2)->fv.vector;
-
 	(*ifobj2)->func_ptr = tmp_func_ptr;
-	(*ifobj2)->fv.vector = tmp_vector;
 
 	*ifobj1 = *ifobj2;
 	*ifobj2 = tmp_ifobj;
@@ -939,6 +932,8 @@ static void swap_directions(struct ifobject **ifobj1, struct ifobject **ifobj2)
 static void testapp_bidi(struct test_spec *test)
 {
 	test_spec_set_name(test, "BIDIRECTIONAL");
+	test->ifobj_tx->rx_on = true;
+	test->ifobj_rx->tx_on = true;
 	for (int i = 0; i < MAX_BIDI_ITER; i++) {
 		print_verbose("Creating socket\n");
 		testapp_validate_traffic(test);
@@ -1012,7 +1007,7 @@ static void testapp_stats(struct test_spec *test)
 
 static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
 		       const char *dst_ip, const char *src_ip, const u16 dst_port,
-		       const u16 src_port, enum fvector vector, thread_func_t func_ptr)
+		       const u16 src_port, thread_func_t func_ptr)
 {
 	struct in_addr ip;
 
@@ -1028,7 +1023,6 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
 	ifobj->dst_port = dst_port;
 	ifobj->src_port = src_port;
 
-	ifobj->fv.vector = vector;
 	ifobj->func_ptr = func_ptr;
 }
 
@@ -1144,9 +1138,9 @@ int main(int argc, char **argv)
 		ksft_exit_xfail();
 	}
 
-	init_iface(ifobj_tx, MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2, tx,
+	init_iface(ifobj_tx, MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2,
 		   worker_testapp_validate_tx);
-	init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1, rx,
+	init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
 		   worker_testapp_validate_rx);
 
 	ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index e02a4dd71bfb..03ff52897d7b 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -100,13 +100,6 @@ struct xsk_socket_info {
 	u32 rxqsize;
 };
 
-struct flow_vector {
-	enum fvector {
-		tx,
-		rx,
-	} vector;
-};
-
 struct pkt {
 	u64 addr;
 	u32 len;
@@ -127,7 +120,6 @@ struct ifobject {
 	struct xsk_socket_info *xsk_arr;
 	struct xsk_umem_info *umem;
 	struct xsk_umem_info *umem_arr;
-	struct flow_vector fv;
 	thread_func_t func_ptr;
 	struct pkt_stream *pkt_stream;
 	int ns_fd;
@@ -135,6 +127,8 @@ struct ifobject {
 	u32 src_ip;
 	u16 src_port;
 	u16 dst_port;
+	bool tx_on;
+	bool rx_on;
 	bool use_poll;
 	u8 dst_mac[ETH_ALEN];
 	u8 src_mac[ETH_ALEN];
-- 
2.29.0


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

* [PATCH bpf-next 10/20] selftests: xsk: replace second_step global variable
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (8 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 09/20] selftests: xsk: introduce rx_on and tx_on in ifobject Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 11/20] selftests: xsk: specify number of sockets to create Magnus Karlsson
                   ` (9 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Replace the second_step global variable with a test specification
variable called total_steps that a test can be set to indicate how
many times the packet stream should be sent without reinitializing any
sockets. This eliminates test specific code in the test runner around
the bidirectional test.

The total_steps variable is 1 by default as most tests only need a
single round of packets.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index a896d5845c0e..0a3e28c9e2a9 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -408,6 +408,8 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 
 	test->ifobj_tx = ifobj_tx;
 	test->ifobj_rx = ifobj_rx;
+	test->current_step = 0;
+	test->total_steps = 1;
 }
 
 static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
@@ -713,7 +715,7 @@ 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: [%s] getsockopt(XDP_STATISTICS) error %u %s\n",
+		ksft_test_result_fail("ERROR Rx: [%s] getsockopt(XDP_STATISTICS) error %u %s\n",
 				      __func__, -err, strerror(-err));
 		return true;
 	}
@@ -754,7 +756,7 @@ 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: [%s] getsockopt(XDP_STATISTICS) error %u %s\n",
+		ksft_test_result_fail("ERROR Tx: [%s] getsockopt(XDP_STATISTICS) error %u %s\n",
 				      __func__, -err, strerror(-err));
 		return;
 	}
@@ -766,12 +768,13 @@ static void tx_stats_validate(struct ifobject *ifobject)
 			      __func__, stats.tx_invalid_descs, ifobject->pkt_stream->nb_pkts);
 }
 
-static void thread_common_ops(struct ifobject *ifobject, void *bufs)
+static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
 {
 	u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
 	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
 	size_t mmap_sz = umem_sz;
 	int ctr = 0, ret;
+	void *bufs;
 
 	ifobject->ns_fd = switch_namespace(ifobject->nsname);
 
@@ -813,26 +816,19 @@ static void thread_common_ops(struct ifobject *ifobject, void *bufs)
 	ifobject->xsk = &ifobject->xsk_arr[0];
 }
 
-static bool testapp_is_test_two_stepped(void)
-{
-	return (test_type != TEST_TYPE_BIDI && test_type != TEST_TYPE_BPF_RES) || second_step;
-}
-
 static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
 {
-	if (testapp_is_test_two_stepped()) {
-		xsk_socket__delete(ifobj->xsk->xsk);
-		(void)xsk_umem__delete(ifobj->umem->umem);
-	}
+	xsk_socket__delete(ifobj->xsk->xsk);
+	xsk_umem__delete(ifobj->umem->umem);
 }
 
 static void *worker_testapp_validate_tx(void *arg)
 {
-	struct ifobject *ifobject = (struct ifobject *)arg;
-	void *bufs = NULL;
+	struct test_spec *test = (struct test_spec *)arg;
+	struct ifobject *ifobject = test->ifobj_tx;
 
-	if (!second_step)
-		thread_common_ops(ifobject, bufs);
+	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);
@@ -841,18 +837,19 @@ static void *worker_testapp_validate_tx(void *arg)
 	if (stat_test_type == STAT_TEST_TX_INVALID)
 		tx_stats_validate(ifobject);
 
-	testapp_cleanup_xsk_res(ifobject);
+	if (test->total_steps == test->current_step)
+		testapp_cleanup_xsk_res(ifobject);
 	pthread_exit(NULL);
 }
 
 static void *worker_testapp_validate_rx(void *arg)
 {
-	struct ifobject *ifobject = (struct ifobject *)arg;
+	struct test_spec *test = (struct test_spec *)arg;
+	struct ifobject *ifobject = test->ifobj_rx;
 	struct pollfd fds[MAX_SOCKS] = { };
-	void *bufs = NULL;
 
-	if (!second_step)
-		thread_common_ops(ifobject, bufs);
+	if (test->current_step == 1)
+		thread_common_ops(test, ifobject);
 
 	if (stat_test_type != STAT_TEST_RX_FILL_EMPTY)
 		xsk_populate_fill_ring(ifobject->umem);
@@ -871,7 +868,8 @@ static void *worker_testapp_validate_rx(void *arg)
 	if (test_type == TEST_TYPE_TEARDOWN)
 		print_verbose("Destroying socket\n");
 
-	testapp_cleanup_xsk_res(ifobject);
+	if (test->total_steps == test->current_step)
+		testapp_cleanup_xsk_res(ifobject);
 	pthread_exit(NULL);
 }
 
@@ -891,16 +889,17 @@ static void testapp_validate_traffic(struct test_spec *test)
 		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
 	ifobj_tx->pkt_stream = pkt_stream;
 	ifobj_rx->pkt_stream = pkt_stream;
+	test->current_step++;
 
 	/*Spawn RX thread */
-	pthread_create(&t0, NULL, ifobj_rx->func_ptr, ifobj_rx);
+	pthread_create(&t0, NULL, ifobj_rx->func_ptr, test);
 
 	pthread_barrier_wait(&barr);
 	if (pthread_barrier_destroy(&barr))
 		exit_with_error(errno);
 
 	/*Spawn TX thread */
-	pthread_create(&t1, NULL, ifobj_tx->func_ptr, ifobj_tx);
+	pthread_create(&t1, NULL, ifobj_tx->func_ptr, test);
 
 	pthread_join(t1, NULL);
 	pthread_join(t0, NULL);
@@ -934,15 +933,12 @@ static void testapp_bidi(struct test_spec *test)
 	test_spec_set_name(test, "BIDIRECTIONAL");
 	test->ifobj_tx->rx_on = true;
 	test->ifobj_rx->tx_on = true;
-	for (int i = 0; i < MAX_BIDI_ITER; i++) {
-		print_verbose("Creating socket\n");
-		testapp_validate_traffic(test);
-		if (!second_step) {
-			print_verbose("Switching Tx/Rx vectors\n");
-			swap_directions(&test->ifobj_rx, &test->ifobj_tx);
-		}
-		second_step = true;
-	}
+	test->total_steps = 2;
+	testapp_validate_traffic(test);
+
+	print_verbose("Switching Tx/Rx vectors\n");
+	swap_directions(&test->ifobj_rx, &test->ifobj_tx);
+	testapp_validate_traffic(test);
 
 	swap_directions(&test->ifobj_rx, &test->ifobj_tx);
 }
@@ -961,16 +957,12 @@ static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj
 
 static void testapp_bpf_res(struct test_spec *test)
 {
-	int i;
-
 	test_spec_set_name(test, "BPF_RES");
-	for (i = 0; i < MAX_BPF_ITER; i++) {
-		print_verbose("Creating socket\n");
-		testapp_validate_traffic(test);
-		if (!second_step)
-			swap_xsk_resources(test->ifobj_tx, test->ifobj_rx);
-		second_step = true;
-	}
+	test->total_steps = 2;
+	testapp_validate_traffic(test);
+
+	swap_xsk_resources(test->ifobj_tx, test->ifobj_rx);
+	testapp_validate_traffic(test);
 }
 
 static void testapp_stats(struct test_spec *test)
@@ -1032,7 +1024,6 @@ static void run_pkt_test(struct test_spec *test, int mode, int type)
 
 	/* reset defaults after potential previous test */
 	xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
-	second_step = 0;
 	stat_test_type = -1;
 
 	configured_mode = mode;
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 03ff52897d7b..ea505a4cb8c0 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -24,7 +24,6 @@
 #define MAX_SOCKETS 2
 #define MAX_TEST_NAME_SIZE 32
 #define MAX_TEARDOWN_ITER 10
-#define MAX_BIDI_ITER 2
 #define MAX_BPF_ITER 2
 #define PKT_HDR_SIZE (sizeof(struct ethhdr) + sizeof(struct iphdr) + \
 			sizeof(struct udphdr))
@@ -72,7 +71,6 @@ enum stat_test_type {
 
 static int configured_mode;
 static bool opt_pkt_dump;
-static bool second_step;
 static int test_type;
 
 static bool opt_verbose;
@@ -137,6 +135,8 @@ struct ifobject {
 struct test_spec {
 	struct ifobject *ifobj_tx;
 	struct ifobject *ifobj_rx;
+	u16 total_steps;
+	u16 current_step;
 	char name[MAX_TEST_NAME_SIZE];
 };
 
-- 
2.29.0


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

* [PATCH bpf-next 11/20] selftests: xsk: specify number of sockets to create
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (9 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 10/20] selftests: xsk: replace second_step global variable Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 12/20] selftests: xsk: make xdp_flags and bind_flags local Magnus Karlsson
                   ` (8 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Add the ability in the test specification to specify numbers of
sockets to create. The default is one socket. This is then used to
remove test specific if-statements around the bpf_res tests.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 0a3e28c9e2a9..06fa767191af 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -231,7 +231,7 @@ static void gen_udp_csum(struct udphdr *udp_hdr, struct iphdr *ip_hdr)
 	    udp_csum(ip_hdr->saddr, ip_hdr->daddr, UDP_PKT_SIZE, IPPROTO_UDP, (u16 *)udp_hdr);
 }
 
-static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size, int idx)
+static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size)
 {
 	struct xsk_umem_config cfg = {
 		.fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
@@ -410,6 +410,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 	test->ifobj_rx = ifobj_rx;
 	test->current_step = 0;
 	test->total_steps = 1;
+	test->nb_sockets = 1;
 }
 
 static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
@@ -770,46 +771,37 @@ static void tx_stats_validate(struct ifobject *ifobject)
 
 static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
 {
-	u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
-	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
-	size_t mmap_sz = umem_sz;
-	int ctr = 0, ret;
-	void *bufs;
+	u32 i;
 
 	ifobject->ns_fd = switch_namespace(ifobject->nsname);
 
-	if (test_type == TEST_TYPE_BPF_RES)
-		mmap_sz *= 2;
-
-	bufs = mmap(NULL, mmap_sz, PROT_READ | PROT_WRITE, mmap_flags, -1, 0);
-	if (bufs == MAP_FAILED)
-		exit_with_error(errno);
+	for (i = 0; i < test->nb_sockets; i++) {
+		u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
+		int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
+		u32 ctr = 0;
+		void *bufs;
 
-	while (ctr++ < SOCK_RECONF_CTR) {
-		ret = xsk_configure_umem(&ifobject->umem_arr[0], bufs, umem_sz, 0);
-		if (ret)
-			exit_with_error(-ret);
+		bufs = mmap(NULL, umem_sz, PROT_READ | PROT_WRITE, mmap_flags, -1, 0);
+		if (bufs == MAP_FAILED)
+			exit_with_error(errno);
 
-		ret = xsk_configure_socket(&ifobject->xsk_arr[0], &ifobject->umem_arr[0],
-					   ifobject, 0);
-		if (!ret)
-			break;
+		while (ctr++ < SOCK_RECONF_CTR) {
+			int ret;
 
-		/* Retry Create Socket if it fails as xsk_socket__create() is asynchronous */
-		if (ctr >= SOCK_RECONF_CTR)
-			exit_with_error(-ret);
-		usleep(USLEEP_MAX);
-	}
+			ret = xsk_configure_umem(&ifobject->umem_arr[i], bufs, umem_sz);
+			if (ret)
+				exit_with_error(-ret);
 
-	if (test_type == TEST_TYPE_BPF_RES) {
-		ret = xsk_configure_umem(&ifobject->umem_arr[1], (u8 *)bufs + umem_sz, umem_sz, 1);
-		if (ret)
-			exit_with_error(-ret);
+			ret = xsk_configure_socket(&ifobject->xsk_arr[i], &ifobject->umem_arr[i],
+						   ifobject, i);
+			if (!ret)
+				break;
 
-		ret = xsk_configure_socket(&ifobject->xsk_arr[1], &ifobject->umem_arr[1],
-					   ifobject, 1);
-		if (ret)
-			exit_with_error(-ret);
+			/* Retry if it fails as xsk_socket__create() is asynchronous */
+			if (ctr >= SOCK_RECONF_CTR)
+				exit_with_error(-ret);
+			usleep(USLEEP_MAX);
+		}
 	}
 
 	ifobject->umem = &ifobject->umem_arr[0];
@@ -959,6 +951,7 @@ 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);
 
 	swap_xsk_resources(test->ifobj_tx, test->ifobj_rx);
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index ea505a4cb8c0..c09b73fd9878 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -24,7 +24,6 @@
 #define MAX_SOCKETS 2
 #define MAX_TEST_NAME_SIZE 32
 #define MAX_TEARDOWN_ITER 10
-#define MAX_BPF_ITER 2
 #define PKT_HDR_SIZE (sizeof(struct ethhdr) + sizeof(struct iphdr) + \
 			sizeof(struct udphdr))
 #define MIN_PKT_SIZE 64
@@ -137,6 +136,7 @@ struct test_spec {
 	struct ifobject *ifobj_rx;
 	u16 total_steps;
 	u16 current_step;
+	u16 nb_sockets;
 	char name[MAX_TEST_NAME_SIZE];
 };
 
-- 
2.29.0


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

* [PATCH bpf-next 12/20] selftests: xsk: make xdp_flags and bind_flags local
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (10 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 11/20] selftests: xsk: specify number of sockets to create Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 13/20] selftests: xsx: make pthreads local scope Magnus Karlsson
                   ` (7 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Make xdp_flags and bind_flags local instead of global by moving them
into the interface object. These flags decide if the socket should be
created in SKB mode or in DRV mode and therefore they are sticky and
will survive a test_spec_reset. Since every test is first run in SKB
mode then in DRV mode, this change only happens once. With this
change, the configured_mode global variable can also be
erradicated. The first test_spec_init() also becomes superfluous and
can be eliminated.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 06fa767191af..3a1afece7c2c 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -112,9 +112,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(test)\
-	(ksft_test_result_pass("PASS: %s %s\n", configured_mode ? "DRV" : "SKB", \
-			       (test)->name))
+#define mode_string(test) (test)->ifobj_tx->xdp_flags & XDP_FLAGS_SKB_MODE ? "SKB" : "DRV"
+
+#define print_ksft_result(test)						\
+	(ksft_test_result_pass("PASS: %s %s\n", mode_string(test), (test)->name))
 
 static void memset32_htonl(void *dest, u32 val, u32 size)
 {
@@ -275,8 +276,8 @@ static int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_inf
 	cfg.rx_size = xsk->rxqsize;
 	cfg.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
 	cfg.libbpf_flags = 0;
-	cfg.xdp_flags = xdp_flags;
-	cfg.bind_flags = xdp_bind_flags;
+	cfg.xdp_flags = ifobject->xdp_flags;
+	cfg.bind_flags = ifobject->bind_flags;
 
 	txr = ifobject->tx_on ? &xsk->tx : NULL;
 	rxr = ifobject->rx_on ? &xsk->rx : NULL;
@@ -333,7 +334,8 @@ static bool validate_interface(struct ifobject *ifobj)
 	return true;
 }
 
-static void parse_command_line(struct test_spec *test, int argc, char **argv)
+static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj_rx, int argc,
+			       char **argv)
 {
 	struct ifobject *ifobj;
 	u32 interface_nb = 0;
@@ -351,9 +353,9 @@ static void parse_command_line(struct test_spec *test, int argc, char **argv)
 		switch (c) {
 		case 'i':
 			if (interface_nb == 0)
-				ifobj = test->ifobj_tx;
+				ifobj = ifobj_tx;
 			else if (interface_nb == 1)
-				ifobj = test->ifobj_rx;
+				ifobj = ifobj_rx;
 			else
 				break;
 
@@ -414,9 +416,24 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 }
 
 static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
-			   struct ifobject *ifobj_rx)
+			   struct ifobject *ifobj_rx, enum test_mode mode)
 {
+	u32 i;
+
 	memset(test, 0, sizeof(*test));
+
+	for (i = 0; i < MAX_INTERFACES; i++) {
+		struct ifobject *ifobj = i ? ifobj_rx : ifobj_tx;
+
+		ifobj->xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
+		if (mode == TEST_MODE_SKB)
+			ifobj->xdp_flags |= XDP_FLAGS_SKB_MODE;
+		else
+			ifobj->xdp_flags |= XDP_FLAGS_DRV_MODE;
+
+		ifobj->bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
+	}
+
 	__test_spec_init(test, ifobj_tx, ifobj_rx);
 }
 
@@ -1011,27 +1028,13 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
 	ifobj->func_ptr = func_ptr;
 }
 
-static void run_pkt_test(struct test_spec *test, int mode, int type)
+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 */
-	xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
 	stat_test_type = -1;
 
-	configured_mode = mode;
-
-	switch (mode) {
-	case (TEST_MODE_SKB):
-		xdp_flags |= XDP_FLAGS_SKB_MODE;
-		break;
-	case (TEST_MODE_DRV):
-		xdp_flags |= XDP_FLAGS_DRV_MODE;
-		break;
-	default:
-		break;
-	}
-
 	switch (test_type) {
 	case TEST_TYPE_STATS:
 		testapp_stats(test);
@@ -1111,11 +1114,9 @@ int main(int argc, char **argv)
 	if (!ifobj_rx)
 		exit_with_error(ENOMEM);
 
-	test_spec_init(&test, ifobj_tx, ifobj_rx);
-
 	setlocale(LC_ALL, "");
 
-	parse_command_line(&test, argc, argv);
+	parse_command_line(ifobj_tx, ifobj_rx, argc, argv);
 
 	if (!validate_interface(ifobj_tx) || !validate_interface(ifobj_rx)) {
 		usage(basename(argv[0]));
@@ -1131,7 +1132,7 @@ int main(int argc, char **argv)
 
 	for (i = 0; i < TEST_MODE_MAX; i++)
 		for (j = 0; j < TEST_TYPE_MAX; j++) {
-			test_spec_init(&test, ifobj_tx, ifobj_rx);
+			test_spec_init(&test, ifobj_tx, ifobj_rx, i);
 			run_pkt_test(&test, i, j);
 			usleep(USLEEP_MAX);
 		}
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index c09b73fd9878..7ed16128f2ad 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -68,14 +68,10 @@ enum stat_test_type {
 	STAT_TEST_TYPE_MAX
 };
 
-static int configured_mode;
 static bool opt_pkt_dump;
 static int test_type;
 
 static bool opt_verbose;
-
-static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
-static u32 xdp_bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
 static int stat_test_type;
 
 struct xsk_umem_info {
@@ -122,6 +118,8 @@ struct ifobject {
 	int ns_fd;
 	u32 dst_ip;
 	u32 src_ip;
+	u32 xdp_flags;
+	u32 bind_flags;
 	u16 src_port;
 	u16 dst_port;
 	bool tx_on;
-- 
2.29.0


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

* [PATCH bpf-next 13/20] selftests: xsx: make pthreads local scope
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (11 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 12/20] selftests: xsk: make xdp_flags and bind_flags local Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 14/20] selftests: xsk: eliminate MAX_SOCKS define Magnus Karlsson
                   ` (6 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Make the pthread_t variables local scope instead of global. No reason
for them to be global.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 3a1afece7c2c..5ea78c503741 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -887,6 +887,7 @@ static void testapp_validate_traffic(struct test_spec *test)
 	struct ifobject *ifobj_tx = test->ifobj_tx;
 	struct ifobject *ifobj_rx = test->ifobj_rx;
 	struct pkt_stream *pkt_stream;
+	pthread_t t0, t1;
 
 	if (pthread_barrier_init(&barr, NULL, 2))
 		exit_with_error(errno);
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 7ed16128f2ad..34ae4e4ea4ac 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -138,8 +138,6 @@ struct test_spec {
 	char name[MAX_TEST_NAME_SIZE];
 };
 
-/*threads*/
 pthread_barrier_t barr;
-pthread_t t0, t1;
 
 #endif				/* XDPXCEIVER_H */
-- 
2.29.0


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

* [PATCH bpf-next 14/20] selftests: xsk: eliminate MAX_SOCKS define
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (12 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 13/20] selftests: xsx: make pthreads local scope Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 15/20] selftests: xsk: allow for invalid packets Magnus Karlsson
                   ` (5 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Remove the MAX_SOCKS define as it always will be one for the forseable
future and the code does not work for any other case anyway.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 5ea78c503741..0fb5cae974de 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -693,11 +693,11 @@ static void wait_for_tx_completion(struct xsk_socket_info *xsk)
 
 static void send_pkts(struct ifobject *ifobject)
 {
-	struct pollfd fds[MAX_SOCKS] = { };
+	struct pollfd fds = { };
 	u32 pkt_cnt = 0;
 
-	fds[0].fd = xsk_socket__fd(ifobject->xsk->xsk);
-	fds[0].events = POLLOUT;
+	fds.fd = xsk_socket__fd(ifobject->xsk->xsk);
+	fds.events = POLLOUT;
 
 	while (pkt_cnt < ifobject->pkt_stream->nb_pkts) {
 		u32 sent;
@@ -705,11 +705,11 @@ static void send_pkts(struct ifobject *ifobject)
 		if (ifobject->use_poll) {
 			int ret;
 
-			ret = poll(fds, 1, POLL_TMOUT);
+			ret = poll(&fds, 1, POLL_TMOUT);
 			if (ret <= 0)
 				continue;
 
-			if (!(fds[0].revents & POLLOUT))
+			if (!(fds.revents & POLLOUT))
 				continue;
 		}
 
@@ -855,7 +855,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[MAX_SOCKS] = { };
+	struct pollfd fds = { };
 
 	if (test->current_step == 1)
 		thread_common_ops(test, ifobject);
@@ -863,8 +863,8 @@ static void *worker_testapp_validate_rx(void *arg)
 	if (stat_test_type != STAT_TEST_RX_FILL_EMPTY)
 		xsk_populate_fill_ring(ifobject->umem);
 
-	fds[0].fd = xsk_socket__fd(ifobject->xsk->xsk);
-	fds[0].events = POLLIN;
+	fds.fd = xsk_socket__fd(ifobject->xsk->xsk);
+	fds.events = POLLIN;
 
 	pthread_barrier_wait(&barr);
 
@@ -872,7 +872,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->pkt_stream, ifobject->xsk, &fds);
 
 	if (test_type == TEST_TYPE_TEARDOWN)
 		print_verbose("Destroying socket\n");
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 34ae4e4ea4ac..1e9a563380c8 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -20,7 +20,6 @@
 #define MAX_INTERFACES 2
 #define MAX_INTERFACE_NAME_CHARS 7
 #define MAX_INTERFACES_NAMESPACE_CHARS 10
-#define MAX_SOCKS 1
 #define MAX_SOCKETS 2
 #define MAX_TEST_NAME_SIZE 32
 #define MAX_TEARDOWN_ITER 10
-- 
2.29.0


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

* [PATCH bpf-next 15/20] selftests: xsk: allow for invalid packets
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (13 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 14/20] selftests: xsk: eliminate MAX_SOCKS define Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 16/20] selftests: xsk: introduce replacing the default packet stream Magnus Karlsson
                   ` (4 subsequent siblings)
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Allow for invalid packets to be sent. These are verified by the Rx
thread not to be received. Or put in another way, if they are
received, the test will fail. This feature will be used to eliminate
an if statement for a stats test and will also be used by other tests
in later patches. The previous code could only deal with valid
packets.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 0fb5cae974de..09d2854c10e6 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -473,6 +473,11 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
 		pkt_stream->pkts[i].addr = (i % umem->num_frames) * umem->frame_size;
 		pkt_stream->pkts[i].len = 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;
@@ -658,7 +663,7 @@ static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *
 static u32 __send_pkts(struct ifobject *ifobject, u32 pkt_nb)
 {
 	struct xsk_socket_info *xsk = ifobject->xsk;
-	u32 i, idx;
+	u32 i, idx, valid_pkts = 0;
 
 	while (xsk_ring_prod__reserve(&xsk->tx, BATCH_SIZE, &idx) < BATCH_SIZE)
 		complete_pkts(xsk, BATCH_SIZE);
@@ -673,14 +678,13 @@ static u32 __send_pkts(struct ifobject *ifobject, u32 pkt_nb)
 		tx_desc->addr = pkt->addr;
 		tx_desc->len = pkt->len;
 		pkt_nb++;
+		if (pkt->valid)
+			valid_pkts++;
 	}
 
 	xsk_ring_prod__submit(&xsk->tx, i);
-	if (stat_test_type != STAT_TEST_TX_INVALID)
-		xsk->outstanding_tx += i;
-	else if (xsk_ring_prod__needs_wakeup(&xsk->tx))
-		kick_tx(xsk);
-	complete_pkts(xsk, i);
+	xsk->outstanding_tx += valid_pkts;
+	complete_pkts(xsk, BATCH_SIZE);
 
 	return i;
 }
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 1e9a563380c8..c5baa7c5f560 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -96,6 +96,7 @@ struct pkt {
 	u64 addr;
 	u32 len;
 	u32 payload;
+	bool valid;
 };
 
 struct pkt_stream {
-- 
2.29.0


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

* [PATCH bpf-next 16/20] selftests: xsk: introduce replacing the default packet stream
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (14 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 15/20] selftests: xsk: allow for invalid packets Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-03 15:04   ` Maciej Fijalkowski
  2021-09-01 10:47 ` [PATCH bpf-next 17/20] selftests: xsk: add test for unaligned mode Magnus Karlsson
                   ` (3 subsequent siblings)
  19 siblings, 1 reply; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Introduce the concept of a default packet stream that is the set of
packets sent by most tests. Then add the ability to replace it for a
test that would like to send or receive something else through the use
of the function pkt_stream_replace() and then restored with
pkt_stream_restore_default(). These are then used to convert the
STAT_TX_INVALID_TEST to use these new APIs.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 09d2854c10e6..d4aad4833754 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -390,6 +390,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 		ifobj->umem = &ifobj->umem_arr[0];
 		ifobj->xsk = &ifobj->xsk_arr[0];
 		ifobj->use_poll = false;
+		ifobj->pkt_stream = test->pkt_stream_default;
 
 		if (i == 0) {
 			ifobj->rx_on = false;
@@ -418,9 +419,12 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 			   struct ifobject *ifobj_rx, enum test_mode mode)
 {
+	struct pkt_stream *pkt_stream;
 	u32 i;
 
+	pkt_stream = test->pkt_stream_default;
 	memset(test, 0, sizeof(*test));
+	test->pkt_stream_default = pkt_stream;
 
 	for (i = 0; i < MAX_INTERFACES; i++) {
 		struct ifobject *ifobj = i ? ifobj_rx : ifobj_tx;
@@ -455,6 +459,19 @@ static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
 	return &pkt_stream->pkts[pkt_nb];
 }
 
+static void pkt_stream_delete(struct pkt_stream *pkt_stream)
+{
+	free(pkt_stream->pkts);
+	free(pkt_stream);
+}
+
+static void pkt_stream_restore_default(struct test_spec *test)
+{
+	pkt_stream_delete(test->ifobj_tx->pkt_stream);
+	test->ifobj_tx->pkt_stream = test->pkt_stream_default;
+	test->ifobj_rx->pkt_stream = test->pkt_stream_default;
+}
+
 static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
 {
 	struct pkt_stream *pkt_stream;
@@ -483,6 +500,17 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
 	return pkt_stream;
 }
 
+static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
+{
+	struct pkt_stream *pkt_stream;
+
+	pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
+	test->ifobj_tx->pkt_stream = pkt_stream;
+	test->ifobj_rx->pkt_stream = pkt_stream;
+
+	pkt_stream_delete(pkt_stream);
+}
+
 static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
 {
 	struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb);
@@ -557,7 +585,7 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *d
 	if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
 		u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
 
-		if (opt_pkt_dump && test_type != TEST_TYPE_STATS)
+		if (opt_pkt_dump)
 			pkt_dump(data, PKT_SIZE);
 
 		if (pkt->len != desc->len) {
@@ -598,9 +626,6 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
 	unsigned int rcvd;
 	u32 idx;
 
-	if (!xsk->outstanding_tx)
-		return;
-
 	if (xsk_ring_prod__needs_wakeup(&xsk->tx))
 		kick_tx(xsk);
 
@@ -831,6 +856,7 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
 
 static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
 {
+	print_verbose("Destroying socket\n");
 	xsk_socket__delete(ifobj->xsk->xsk);
 	xsk_umem__delete(ifobj->umem->umem);
 }
@@ -878,9 +904,6 @@ static void *worker_testapp_validate_rx(void *arg)
 	else
 		receive_pkts(ifobject->pkt_stream, ifobject->xsk, &fds);
 
-	if (test_type == TEST_TYPE_TEARDOWN)
-		print_verbose("Destroying socket\n");
-
 	if (test->total_steps == test->current_step)
 		testapp_cleanup_xsk_res(ifobject);
 	pthread_exit(NULL);
@@ -890,19 +913,11 @@ static void testapp_validate_traffic(struct test_spec *test)
 {
 	struct ifobject *ifobj_tx = test->ifobj_tx;
 	struct ifobject *ifobj_rx = test->ifobj_rx;
-	struct pkt_stream *pkt_stream;
 	pthread_t t0, t1;
 
 	if (pthread_barrier_init(&barr, NULL, 2))
 		exit_with_error(errno);
 
-	if (stat_test_type == STAT_TEST_TX_INVALID)
-		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT,
-						 XSK_UMEM__INVALID_FRAME_SIZE);
-	else
-		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
-	ifobj_tx->pkt_stream = pkt_stream;
-	ifobj_rx->pkt_stream = pkt_stream;
 	test->current_step++;
 
 	/*Spawn RX thread */
@@ -982,7 +997,9 @@ static void testapp_bpf_res(struct test_spec *test)
 
 static void testapp_stats(struct test_spec *test)
 {
-	for (int i = 0; i < STAT_TEST_TYPE_MAX; i++) {
+	int i;
+
+	for (i = 0; i < STAT_TEST_TYPE_MAX; i++) {
 		test_spec_reset(test);
 		stat_test_type = i;
 
@@ -991,21 +1008,27 @@ 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;
+			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;
+			testapp_validate_traffic(test);
 			break;
 		case STAT_TEST_TX_INVALID:
 			test_spec_set_name(test, "STAT_TX_INVALID");
-			continue;
+			pkt_stream_replace(test, DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
+			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");
+			testapp_validate_traffic(test);
 			break;
 		default:
 			break;
 		}
-		testapp_validate_traffic(test);
 	}
 
 	/* To only see the whole stat set being completed unless an individual test fails. */
@@ -1106,6 +1129,7 @@ int main(int argc, char **argv)
 {
 	struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
 	struct ifobject *ifobj_tx, *ifobj_rx;
+	struct pkt_stream *pkt_stream_default;
 	struct test_spec test;
 	u32 i, j;
 
@@ -1133,6 +1157,12 @@ int main(int argc, char **argv)
 	init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
 		   worker_testapp_validate_rx);
 
+	test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
+	pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
+	if (!pkt_stream_default)
+		exit_with_error(ENOMEM);
+	test.pkt_stream_default = pkt_stream_default;
+
 	ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
 
 	for (i = 0; i < TEST_MODE_MAX; i++)
@@ -1142,6 +1172,7 @@ int main(int argc, char **argv)
 			usleep(USLEEP_MAX);
 		}
 
+	pkt_stream_delete(pkt_stream_default);
 	ifobject_delete(ifobj_tx);
 	ifobject_delete(ifobj_rx);
 
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index c5baa7c5f560..e27fe348ae50 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -132,6 +132,7 @@ struct ifobject {
 struct test_spec {
 	struct ifobject *ifobj_tx;
 	struct ifobject *ifobj_rx;
+	struct pkt_stream *pkt_stream_default;
 	u16 total_steps;
 	u16 current_step;
 	u16 nb_sockets;
-- 
2.29.0


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

* [PATCH bpf-next 17/20] selftests: xsk: add test for unaligned mode
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (15 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 16/20] selftests: xsk: introduce replacing the default packet stream Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-03 15:38   ` Maciej Fijalkowski
  2021-09-01 10:47 ` [PATCH bpf-next 18/20] selftests: xsk: eliminate test specific if-statement in test runner Magnus Karlsson
                   ` (2 subsequent siblings)
  19 siblings, 1 reply; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Add a test for unaligned mode in which packet buffers can be placed
anywhere within the umem. Some packets are made to straddle page
boundraries in order to check for correctness. On the Tx side, buffers
are now allocated according to the addresses found in the packet
stream. Thus, the placement of buffers can be controlled with the
boolean use_addr_for_fill in the packet stream.

One new pkt_stream insterface is introduced: pkt_stream_replace_half()
that replaces every other packet in the default packet stream with the
specified new packet.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index d4aad4833754..a24068993cc3 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -19,7 +19,7 @@
  * Virtual Ethernet interfaces.
  *
  * For each mode, the following tests are run:
- *    a. nopoll - soft-irq processing
+ *    a. nopoll - soft-irq processing in run-to-completion mode
  *    b. poll - using poll() syscall
  *    c. Socket Teardown
  *       Create a Tx and a Rx socket, Tx from one socket, Rx on another. Destroy
@@ -45,6 +45,7 @@
  *       Configure sockets at indexes 0 and 1, run a traffic on queue ids 0,
  *       then remove xsk sockets from queue 0 on both veth interfaces and
  *       finally run a traffic on queues ids 1
+ *    g. unaligned mode
  *
  * Total tests: 12
  *
@@ -243,6 +244,9 @@ static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size
 	};
 	int ret;
 
+	if (umem->unaligned_mode)
+		cfg.flags |= XDP_UMEM_UNALIGNED_CHUNK_FLAG;
+
 	ret = xsk_umem__create(&umem->umem, buffer, size,
 			       &umem->fq, &umem->cq, &cfg);
 	if (ret)
@@ -252,19 +256,6 @@ static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size
 	return 0;
 }
 
-static void xsk_populate_fill_ring(struct xsk_umem_info *umem)
-{
-	int ret, i;
-	u32 idx = 0;
-
-	ret = xsk_ring_prod__reserve(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS, &idx);
-	if (ret != XSK_RING_PROD__DEFAULT_NUM_DESCS)
-		exit_with_error(-ret);
-	for (i = 0; i < XSK_RING_PROD__DEFAULT_NUM_DESCS; i++)
-		*xsk_ring_prod__fill_addr(&umem->fq, idx++) = i * umem->frame_size;
-	xsk_ring_prod__submit(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS);
-}
-
 static int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_info *umem,
 				struct ifobject *ifobject, u32 qid)
 {
@@ -487,7 +478,8 @@ 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].addr = (i % umem->num_frames) * umem->frame_size +
+			DEFAULT_OFFSET;
 		pkt_stream->pkts[i].len = pkt_len;
 		pkt_stream->pkts[i].payload = i;
 
@@ -500,6 +492,12 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
 	return pkt_stream;
 }
 
+static struct pkt_stream *pkt_stream_clone(struct xsk_umem_info *umem,
+					   struct pkt_stream *pkt_stream)
+{
+	return pkt_stream_generate(umem, pkt_stream->nb_pkts, pkt_stream->pkts[0].len);
+}
+
 static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
 {
 	struct pkt_stream *pkt_stream;
@@ -507,8 +505,22 @@ static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
 	pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
 	test->ifobj_tx->pkt_stream = pkt_stream;
 	test->ifobj_rx->pkt_stream = pkt_stream;
+}
 
-	pkt_stream_delete(pkt_stream);
+static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 offset)
+{
+	struct xsk_umem_info *umem = test->ifobj_tx->umem;
+	struct pkt_stream *pkt_stream;
+	u32 i;
+
+	pkt_stream = pkt_stream_clone(umem, test->pkt_stream_default);
+	for (i = 0; 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;
+	}
+
+	test->ifobj_tx->pkt_stream = pkt_stream;
+	test->ifobj_rx->pkt_stream = pkt_stream;
 }
 
 static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
@@ -572,9 +584,9 @@ static void pkt_dump(void *pkt, u32 len)
 	fprintf(stdout, "---------------------------------------\n");
 }
 
-static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *desc)
+static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len)
 {
-	void *data = xsk_umem__get_data(buffer, desc->addr);
+	void *data = xsk_umem__get_data(buffer, addr);
 	struct iphdr *iphdr = (struct iphdr *)(data + sizeof(struct ethhdr));
 
 	if (!pkt) {
@@ -588,10 +600,10 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *d
 		if (opt_pkt_dump)
 			pkt_dump(data, PKT_SIZE);
 
-		if (pkt->len != desc->len) {
+		if (pkt->len != len) {
 			ksft_test_result_fail
 				("ERROR: [%s] expected length [%d], got length [%d]\n",
-					__func__, pkt->len, desc->len);
+					__func__, pkt->len, len);
 			return false;
 		}
 
@@ -673,7 +685,7 @@ static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *
 
 			orig = xsk_umem__extract_addr(addr);
 			addr = xsk_umem__add_offset_to_addr(addr);
-			if (!is_pkt_valid(pkt, xsk->umem->buffer, desc))
+			if (!is_pkt_valid(pkt, xsk->umem->buffer, addr, desc->len))
 				return;
 
 			*xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
@@ -817,13 +829,16 @@ static void tx_stats_validate(struct ifobject *ifobject)
 
 static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
 {
+	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
 	u32 i;
 
 	ifobject->ns_fd = switch_namespace(ifobject->nsname);
 
+	if (ifobject->umem->unaligned_mode)
+		mmap_flags |= MAP_HUGETLB;
+
 	for (i = 0; i < test->nb_sockets; i++) {
 		u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
-		int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
 		u32 ctr = 0;
 		void *bufs;
 
@@ -881,6 +896,32 @@ static void *worker_testapp_validate_tx(void *arg)
 	pthread_exit(NULL);
 }
 
+static void xsk_populate_fill_ring(struct xsk_umem_info *umem, struct pkt_stream *pkt_stream)
+{
+	u32 idx = 0, i;
+	int ret;
+
+	ret = xsk_ring_prod__reserve(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS, &idx);
+	if (ret != XSK_RING_PROD__DEFAULT_NUM_DESCS)
+		exit_with_error(ENOSPC);
+	for (i = 0; i < XSK_RING_PROD__DEFAULT_NUM_DESCS; i++) {
+		u64 addr;
+
+		if (pkt_stream->use_addr_for_fill) {
+			struct pkt *pkt = pkt_stream_get_pkt(pkt_stream, i);
+
+			if (!pkt)
+				break;
+			addr = pkt->addr;
+		} else {
+			addr = (i % umem->num_frames) * umem->frame_size + DEFAULT_OFFSET;
+		}
+
+		*xsk_ring_prod__fill_addr(&umem->fq, idx++) = addr;
+	}
+	xsk_ring_prod__submit(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS);
+}
+
 static void *worker_testapp_validate_rx(void *arg)
 {
 	struct test_spec *test = (struct test_spec *)arg;
@@ -891,7 +932,7 @@ static void *worker_testapp_validate_rx(void *arg)
 		thread_common_ops(test, ifobject);
 
 	if (stat_test_type != STAT_TEST_RX_FILL_EMPTY)
-		xsk_populate_fill_ring(ifobject->umem);
+		xsk_populate_fill_ring(ifobject->umem, ifobject->pkt_stream);
 
 	fds.fd = xsk_socket__fd(ifobject->xsk->xsk);
 	fds.events = POLLIN;
@@ -1035,6 +1076,40 @@ static void testapp_stats(struct test_spec *test)
 	test_spec_set_name(test, "STATS");
 }
 
+/* Simple test */
+static bool hugepages_present(struct ifobject *ifobject)
+{
+	const size_t mmap_sz = 2 * ifobject->umem->num_frames * ifobject->umem->frame_size;
+	void *bufs;
+
+	bufs = mmap(NULL, mmap_sz, PROT_READ | PROT_WRITE,
+		    MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE | MAP_HUGETLB, -1, 0);
+	if (bufs == MAP_FAILED)
+		return false;
+
+	munmap(bufs, mmap_sz);
+	return true;
+}
+
+static bool testapp_unaligned(struct test_spec *test)
+{
+	if (!hugepages_present(test->ifobj_tx)) {
+		ksft_test_result_skip("No 2M huge pages present.\n");
+		return false;
+	}
+
+	test_spec_set_name(test, "UNALIGNED_MODE");
+	test->ifobj_tx->umem->unaligned_mode = true;
+	test->ifobj_rx->umem->unaligned_mode = true;
+	/* Let half of the packets straddle a buffer boundrary */
+	pkt_stream_replace_half(test, PKT_SIZE, test->ifobj_tx->umem->frame_size - 32);
+	test->ifobj_rx->pkt_stream->use_addr_for_fill = true;
+	testapp_validate_traffic(test);
+
+	pkt_stream_restore_default(test);
+	return true;
+}
+
 static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
 		       const char *dst_ip, const char *src_ip, const u16 dst_port,
 		       const u16 src_port, thread_func_t func_ptr)
@@ -1086,6 +1161,10 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
 		test_spec_set_name(test, "POLL");
 		testapp_validate_traffic(test);
 		break;
+	case TEST_TYPE_UNALIGNED:
+		if (!testapp_unaligned(test))
+			return;
+		break;
 	default:
 		break;
 	}
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index e27fe348ae50..129801eb013c 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -39,6 +39,7 @@
 #define POLL_TMOUT 1000
 #define DEFAULT_PKT_CNT (4 * 1024)
 #define RX_FULL_RXQSIZE 32
+#define DEFAULT_OFFSET 256
 #define XSK_UMEM__INVALID_FRAME_SIZE (XSK_UMEM__DEFAULT_FRAME_SIZE + 1)
 
 #define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); } while (0)
@@ -52,6 +53,7 @@ enum test_mode {
 enum test_type {
 	TEST_TYPE_NOPOLL,
 	TEST_TYPE_POLL,
+	TEST_TYPE_UNALIGNED,
 	TEST_TYPE_TEARDOWN,
 	TEST_TYPE_BIDI,
 	TEST_TYPE_STATS,
@@ -81,6 +83,7 @@ struct xsk_umem_info {
 	u32 frame_headroom;
 	void *buffer;
 	u32 frame_size;
+	bool unaligned_mode;
 };
 
 struct xsk_socket_info {
@@ -102,6 +105,7 @@ struct pkt {
 struct pkt_stream {
 	u32 nb_pkts;
 	struct pkt *pkts;
+	bool use_addr_for_fill;
 };
 
 typedef void *(*thread_func_t)(void *arg);
-- 
2.29.0


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

* [PATCH bpf-next 18/20] selftests: xsk: eliminate test specific if-statement in test runner
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (16 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 17/20] selftests: xsk: add test for unaligned mode Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 19/20] selftests: xsk: add tests for invalid xsk descriptors Magnus Karlsson
  2021-09-01 10:47 ` [PATCH bpf-next 20/20] selftests: xsk: add tests for 2K frame size Magnus Karlsson
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Eliminate a test specific if-statement for the RX_FILL_EMTPY stats
test that is present in the test runner. We can do this as we now have
the use_addr_for_fill option. Just create and empty Rx packet stream
and indicated that the test runner should use the addresses in that to
populate the fill ring. As there are no packets in the stream, the
fill ring will be empty and we will get the error stats that we want
to test.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index a24068993cc3..d085033afd53 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -458,8 +458,10 @@ static void pkt_stream_delete(struct pkt_stream *pkt_stream)
 
 static void pkt_stream_restore_default(struct test_spec *test)
 {
-	pkt_stream_delete(test->ifobj_tx->pkt_stream);
-	test->ifobj_tx->pkt_stream = test->pkt_stream_default;
+	if (test->ifobj_tx->pkt_stream != test->pkt_stream_default) {
+		pkt_stream_delete(test->ifobj_tx->pkt_stream);
+		test->ifobj_tx->pkt_stream = test->pkt_stream_default;
+	}
 	test->ifobj_rx->pkt_stream = test->pkt_stream_default;
 }
 
@@ -931,8 +933,7 @@ static void *worker_testapp_validate_rx(void *arg)
 	if (test->current_step == 1)
 		thread_common_ops(test, ifobject);
 
-	if (stat_test_type != STAT_TEST_RX_FILL_EMPTY)
-		xsk_populate_fill_ring(ifobject->umem, ifobject->pkt_stream);
+	xsk_populate_fill_ring(ifobject->umem, ifobject->pkt_stream);
 
 	fds.fd = xsk_socket__fd(ifobject->xsk->xsk);
 	fds.events = POLLIN;
@@ -1065,7 +1066,14 @@ static void testapp_stats(struct test_spec *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;
 			testapp_validate_traffic(test);
+
+			pkt_stream_restore_default(test);
 			break;
 		default:
 			break;
-- 
2.29.0


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

* [PATCH bpf-next 19/20] selftests: xsk: add tests for invalid xsk descriptors
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (17 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 18/20] selftests: xsk: eliminate test specific if-statement in test runner Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  2021-09-03 18:58   ` Maciej Fijalkowski
  2021-09-01 10:47 ` [PATCH bpf-next 20/20] selftests: xsk: add tests for 2K frame size Magnus Karlsson
  19 siblings, 1 reply; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Add tests for invalid xsk descriptors in the Tx ring. A number of
handcrafted nasty invalid descriptors are created and submitted to the
tx ring to check that they are validated correctly. Corener case valid
ones are also sent. The tests are run for both aligned and unaligned
mode.

pkt_stream_set() is introduced to be able to create a hand-crafted
packet stream where every single packet is specified in detail.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index d085033afd53..a4f6ce3a6b14 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -46,6 +46,8 @@
  *       then remove xsk sockets from queue 0 on both veth interfaces and
  *       finally run a traffic on queues ids 1
  *    g. unaligned mode
+ *    h. tests for invalid and corner case Tx descriptors so that the correct ones
+ *       are discarded and let through, respectively.
  *
  * Total tests: 12
  *
@@ -394,7 +396,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
 		for (j = 0; j < MAX_SOCKETS; j++) {
 			memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j]));
 			memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j]));
-			ifobj->umem_arr[j].num_frames = DEFAULT_PKT_CNT / 4;
+			ifobj->umem_arr[j].num_frames = DEFAULT_UMEM_BUFFERS;
 			ifobj->umem_arr[j].frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
 			ifobj->xsk_arr[j].rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
 		}
@@ -450,6 +452,16 @@ 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)
+{
+	while (pkt_stream->rx_pkt_nb < pkt_stream->nb_pkts) {
+		if (pkt_stream->pkts[pkt_stream->rx_pkt_nb].valid)
+			return &pkt_stream->pkts[pkt_stream->rx_pkt_nb++];
+		pkt_stream->rx_pkt_nb++;
+	}
+	return NULL;
+}
+
 static void pkt_stream_delete(struct pkt_stream *pkt_stream)
 {
 	free(pkt_stream->pkts);
@@ -465,17 +477,31 @@ static void pkt_stream_restore_default(struct test_spec *test)
 	test->ifobj_rx->pkt_stream = test->pkt_stream_default;
 }
 
-static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
+static struct pkt_stream *__pkt_stream_alloc(u32 nb_pkts)
 {
 	struct pkt_stream *pkt_stream;
-	u32 i;
 
-	pkt_stream = malloc(sizeof(*pkt_stream));
+	pkt_stream = calloc(1, sizeof(*pkt_stream));
 	if (!pkt_stream)
-		exit_with_error(ENOMEM);
+		return NULL;
 
 	pkt_stream->pkts = calloc(nb_pkts, sizeof(*pkt_stream->pkts));
-	if (!pkt_stream->pkts)
+	if (!pkt_stream->pkts) {
+		free(pkt_stream);
+		return NULL;
+	}
+
+	pkt_stream->nb_pkts = nb_pkts;
+	return pkt_stream;
+}
+
+static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
+{
+	struct pkt_stream *pkt_stream;
+	u32 i;
+
+	pkt_stream = __pkt_stream_alloc(nb_pkts);
+	if (!pkt_stream)
 		exit_with_error(ENOMEM);
 
 	pkt_stream->nb_pkts = nb_pkts;
@@ -525,6 +551,26 @@ static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 off
 	test->ifobj_rx->pkt_stream = pkt_stream;
 }
 
+static void pkt_stream_set(struct test_spec *test, struct pkt *pkts, u32 nb_pkts)
+{
+	struct pkt_stream *pkt_stream;
+	u32 i;
+
+	pkt_stream = __pkt_stream_alloc(nb_pkts);
+	if (!pkt_stream)
+		exit_with_error(ENOMEM);
+
+	test->ifobj_tx->pkt_stream = pkt_stream;
+	test->ifobj_rx->pkt_stream = pkt_stream;
+
+	for (i = 0; i < nb_pkts; i++) {
+		pkt_stream->pkts[i].addr = pkts[i].addr;
+		pkt_stream->pkts[i].len = pkts[i].len;
+		pkt_stream->pkts[i].payload = i;
+		pkt_stream->pkts[i].valid = pkts[i].valid;
+	}
+}
+
 static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
 {
 	struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb);
@@ -535,6 +581,8 @@ static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
 
 	if (!pkt)
 		return NULL;
+	if (!pkt->valid || pkt->len < PKT_SIZE)
+		return pkt;
 
 	data = xsk_umem__get_data(ifobject->umem->buffer, pkt->addr);
 	udp_hdr = (struct udphdr *)(data + sizeof(struct ethhdr) + sizeof(struct iphdr));
@@ -596,19 +644,24 @@ 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 minimun size. */
+		return true;
+	}
+
+	if (pkt->len != len) {
+		ksft_test_result_fail
+			("ERROR: [%s] expected length [%d], got length [%d]\n",
+			 __func__, pkt->len, len);
+		return false;
+	}
+
 	if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
 		u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
 
 		if (opt_pkt_dump)
 			pkt_dump(data, PKT_SIZE);
 
-		if (pkt->len != len) {
-			ksft_test_result_fail
-				("ERROR: [%s] expected length [%d], got length [%d]\n",
-					__func__, pkt->len, len);
-			return false;
-		}
-
 		if (pkt->payload != seqnum) {
 			ksft_test_result_fail
 				("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
@@ -645,6 +698,15 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
 
 	rcvd = xsk_ring_cons__peek(&xsk->umem->cq, batch_size, &idx);
 	if (rcvd) {
+		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("Last completion address: %llx\n", addr);
+			return;
+		}
+
 		xsk_ring_cons__release(&xsk->umem->cq, rcvd);
 		xsk->outstanding_tx -= rcvd;
 	}
@@ -653,11 +715,10 @@ 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)
 {
-	u32 idx_rx = 0, idx_fq = 0, rcvd, i, pkt_count = 0;
-	struct pkt *pkt;
+	struct pkt *pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
+	u32 idx_rx = 0, idx_fq = 0, rcvd, i;
 	int ret;
 
-	pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++);
 	while (pkt) {
 		rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
 		if (!rcvd) {
@@ -685,13 +746,21 @@ static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *
 			const struct xdp_desc *desc = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
 			u64 addr = desc->addr, orig;
 
+			if (!pkt) {
+				ksft_test_result_fail("ERROR: [%s] Received too many packets.\n",
+						      __func__);
+				ksft_print_msg("Last packet has addr: %llx len: %u\n",
+					       addr, desc->len);
+				return;
+			}
+
 			orig = xsk_umem__extract_addr(addr);
 			addr = xsk_umem__add_offset_to_addr(addr);
 			if (!is_pkt_valid(pkt, xsk->umem->buffer, addr, desc->len))
 				return;
 
 			*xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
-			pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++);
+			pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
 		}
 
 		xsk_ring_prod__submit(&xsk->umem->fq, rcvd);
@@ -875,6 +944,7 @@ static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
 {
 	print_verbose("Destroying socket\n");
 	xsk_socket__delete(ifobj->xsk->xsk);
+	munmap(ifobj->umem->buffer, ifobj->umem->num_frames * ifobj->umem->frame_size);
 	xsk_umem__delete(ifobj->umem->umem);
 }
 
@@ -1118,6 +1188,33 @@ static bool testapp_unaligned(struct test_spec *test)
 	return true;
 }
 
+static void testapp_inv_desc(struct test_spec *test)
+{
+	struct pkt pkts[] = {{0, 0, 0, true}, /* Zero packet length and zero address allowed */
+		/* Zero packet length allowed */
+			     {0x1000, 0, 0, true},
+		/* Straddling the start of umem */
+			     {-2, PKT_SIZE, 0, false},
+		/* Packet too large */
+			     {0x2000, XSK_UMEM__INVALID_FRAME_SIZE, 0, false},
+		/* After umem ends */
+			     {UMEM_SIZE, PKT_SIZE, 0, false},
+		/* Straddle the end of umem */
+			     {UMEM_SIZE - PKT_SIZE / 2, PKT_SIZE, 0, false},
+		/* Straddle a page boundrary */
+			     {0x3000 - PKT_SIZE / 2, PKT_SIZE, 0, false},
+		/* Valid packet for synch so that something is received */
+			     {0x4000, PKT_SIZE, 0, true}};
+
+	if (test->ifobj_tx->umem->unaligned_mode) {
+		/* Crossing a page boundrary allowed */
+		pkts[6].valid = true;
+	}
+	pkt_stream_set(test, pkts, ARRAY_SIZE(pkts));
+	testapp_validate_traffic(test);
+	pkt_stream_restore_default(test);
+}
+
 static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
 		       const char *dst_ip, const char *src_ip, const u16 dst_port,
 		       const u16 src_port, thread_func_t func_ptr)
@@ -1159,7 +1256,7 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
 	case TEST_TYPE_BPF_RES:
 		testapp_bpf_res(test);
 		break;
-	case TEST_TYPE_NOPOLL:
+	case TEST_TYPE_RUN_TO_COMPLETION:
 		test_spec_set_name(test, "RUN_TO_COMPLETION");
 		testapp_validate_traffic(test);
 		break;
@@ -1169,6 +1266,16 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
 		test_spec_set_name(test, "POLL");
 		testapp_validate_traffic(test);
 		break;
+	case TEST_TYPE_ALIGNED_INV_DESC:
+		test_spec_set_name(test, "ALIGNED_INV_DESC");
+		testapp_inv_desc(test);
+		break;
+	case TEST_TYPE_UNALIGNED_INV_DESC:
+		test_spec_set_name(test, "UNALIGNED_INV_DESC");
+		test->ifobj_tx->umem->unaligned_mode = true;
+		test->ifobj_rx->umem->unaligned_mode = true;
+		testapp_inv_desc(test);
+		break;
 	case TEST_TYPE_UNALIGNED:
 		if (!testapp_unaligned(test))
 			return;
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 129801eb013c..2d9efb89ea28 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -38,6 +38,8 @@
 #define BATCH_SIZE 8
 #define POLL_TMOUT 1000
 #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)
 #define RX_FULL_RXQSIZE 32
 #define DEFAULT_OFFSET 256
 #define XSK_UMEM__INVALID_FRAME_SIZE (XSK_UMEM__DEFAULT_FRAME_SIZE + 1)
@@ -51,9 +53,11 @@ enum test_mode {
 };
 
 enum test_type {
-	TEST_TYPE_NOPOLL,
+	TEST_TYPE_RUN_TO_COMPLETION,
 	TEST_TYPE_POLL,
 	TEST_TYPE_UNALIGNED,
+	TEST_TYPE_ALIGNED_INV_DESC,
+	TEST_TYPE_UNALIGNED_INV_DESC,
 	TEST_TYPE_TEARDOWN,
 	TEST_TYPE_BIDI,
 	TEST_TYPE_STATS,
@@ -104,6 +108,7 @@ struct pkt {
 
 struct pkt_stream {
 	u32 nb_pkts;
+	u32 rx_pkt_nb;
 	struct pkt *pkts;
 	bool use_addr_for_fill;
 };
-- 
2.29.0


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

* [PATCH bpf-next 20/20] selftests: xsk: add tests for 2K frame size
  2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
                   ` (18 preceding siblings ...)
  2021-09-01 10:47 ` [PATCH bpf-next 19/20] selftests: xsk: add tests for invalid xsk descriptors Magnus Karlsson
@ 2021-09-01 10:47 ` Magnus Karlsson
  19 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-01 10:47 UTC (permalink / raw)
  To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

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

Add tests for 2K frame size. Both a standard send and receive test and
one testing for invalid descriptors when the frame size is 2K.

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

diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index a4f6ce3a6b14..8e4cb781a3e3 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -48,6 +48,7 @@
  *    g. unaligned mode
  *    h. tests for invalid and corner case Tx descriptors so that the correct ones
  *       are discarded and let through, respectively.
+ *    i. 2K frame size tests
  *
  * Total tests: 12
  *
@@ -1203,6 +1204,8 @@ static void testapp_inv_desc(struct test_spec *test)
 			     {UMEM_SIZE - PKT_SIZE / 2, PKT_SIZE, 0, false},
 		/* Straddle a page boundrary */
 			     {0x3000 - PKT_SIZE / 2, PKT_SIZE, 0, false},
+		/* Straddle a 2K boundrary */
+			     {0x3800 - PKT_SIZE / 2, PKT_SIZE, 0, true},
 		/* Valid packet for synch so that something is received */
 			     {0x4000, PKT_SIZE, 0, true}};
 
@@ -1210,6 +1213,11 @@ static void testapp_inv_desc(struct test_spec *test)
 		/* Crossing a page boundrary allowed */
 		pkts[6].valid = true;
 	}
+	if (test->ifobj_tx->umem->frame_size == XSK_UMEM__DEFAULT_FRAME_SIZE / 2) {
+		/* Crossing a 2K frame size boundrary not allowed */
+		pkts[7].valid = false;
+	}
+
 	pkt_stream_set(test, pkts, ARRAY_SIZE(pkts));
 	testapp_validate_traffic(test);
 	pkt_stream_restore_default(test);
@@ -1260,6 +1268,15 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
 		test_spec_set_name(test, "RUN_TO_COMPLETION");
 		testapp_validate_traffic(test);
 		break;
+	case TEST_TYPE_RUN_TO_COMPLETION_2K_FRAME:
+		test_spec_set_name(test, "RUN_TO_COMPLETION_2K_FRAME_SIZE");
+		test->ifobj_tx->umem->frame_size = 2048;
+		test->ifobj_rx->umem->frame_size = 2048;
+		pkt_stream_replace(test, DEFAULT_PKT_CNT, MIN_PKT_SIZE);
+		testapp_validate_traffic(test);
+
+		pkt_stream_restore_default(test);
+		break;
 	case TEST_TYPE_POLL:
 		test->ifobj_tx->use_poll = true;
 		test->ifobj_rx->use_poll = true;
@@ -1270,6 +1287,12 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
 		test_spec_set_name(test, "ALIGNED_INV_DESC");
 		testapp_inv_desc(test);
 		break;
+	case TEST_TYPE_ALIGNED_INV_DESC_2K_FRAME:
+		test_spec_set_name(test, "ALIGNED_INV_DESC_2K_FRAME_SIZE");
+		test->ifobj_tx->umem->frame_size = 2048;
+		test->ifobj_rx->umem->frame_size = 2048;
+		testapp_inv_desc(test);
+		break;
 	case TEST_TYPE_UNALIGNED_INV_DESC:
 		test_spec_set_name(test, "UNALIGNED_INV_DESC");
 		test->ifobj_tx->umem->unaligned_mode = true;
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 2d9efb89ea28..5ac4a5e64744 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -54,9 +54,11 @@ enum test_mode {
 
 enum test_type {
 	TEST_TYPE_RUN_TO_COMPLETION,
+	TEST_TYPE_RUN_TO_COMPLETION_2K_FRAME,
 	TEST_TYPE_POLL,
 	TEST_TYPE_UNALIGNED,
 	TEST_TYPE_ALIGNED_INV_DESC,
+	TEST_TYPE_ALIGNED_INV_DESC_2K_FRAME,
 	TEST_TYPE_UNALIGNED_INV_DESC,
 	TEST_TYPE_TEARDOWN,
 	TEST_TYPE_BIDI,
-- 
2.29.0


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

* Re: [PATCH bpf-next 16/20] selftests: xsk: introduce replacing the default packet stream
  2021-09-01 10:47 ` [PATCH bpf-next 16/20] selftests: xsk: introduce replacing the default packet stream Magnus Karlsson
@ 2021-09-03 15:04   ` Maciej Fijalkowski
  2021-09-06  8:00     ` Magnus Karlsson
  0 siblings, 1 reply; 29+ messages in thread
From: Maciej Fijalkowski @ 2021-09-03 15:04 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
	ciara.loftus, bpf, yhs, andrii

On Wed, Sep 01, 2021 at 12:47:28PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Introduce the concept of a default packet stream that is the set of
> packets sent by most tests. Then add the ability to replace it for a
> test that would like to send or receive something else through the use
> of the function pkt_stream_replace() and then restored with
> pkt_stream_restore_default(). These are then used to convert the
> STAT_TX_INVALID_TEST to use these new APIs.

s/STAT_TX_INVALID_TEST/STAT_TEST_TX_INVALID

> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/xdpxceiver.c | 67 +++++++++++++++++-------
>  tools/testing/selftests/bpf/xdpxceiver.h |  1 +
>  2 files changed, 50 insertions(+), 18 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index 09d2854c10e6..d4aad4833754 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -390,6 +390,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  		ifobj->umem = &ifobj->umem_arr[0];
>  		ifobj->xsk = &ifobj->xsk_arr[0];
>  		ifobj->use_poll = false;
> +		ifobj->pkt_stream = test->pkt_stream_default;
>  
>  		if (i == 0) {
>  			ifobj->rx_on = false;
> @@ -418,9 +419,12 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  			   struct ifobject *ifobj_rx, enum test_mode mode)
>  {
> +	struct pkt_stream *pkt_stream;
>  	u32 i;
>  
> +	pkt_stream = test->pkt_stream_default;
>  	memset(test, 0, sizeof(*test));
> +	test->pkt_stream_default = pkt_stream;
>  
>  	for (i = 0; i < MAX_INTERFACES; i++) {
>  		struct ifobject *ifobj = i ? ifobj_rx : ifobj_tx;
> @@ -455,6 +459,19 @@ static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
>  	return &pkt_stream->pkts[pkt_nb];
>  }
>  
> +static void pkt_stream_delete(struct pkt_stream *pkt_stream)
> +{
> +	free(pkt_stream->pkts);
> +	free(pkt_stream);
> +}
> +
> +static void pkt_stream_restore_default(struct test_spec *test)
> +{
> +	pkt_stream_delete(test->ifobj_tx->pkt_stream);

I suppose that streams are the same for both tx and rx ifobjs hence it's
enough to call the delete op a single time.

> +	test->ifobj_tx->pkt_stream = test->pkt_stream_default;
> +	test->ifobj_rx->pkt_stream = test->pkt_stream_default;
> +}
> +
>  static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
>  {
>  	struct pkt_stream *pkt_stream;
> @@ -483,6 +500,17 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
>  	return pkt_stream;
>  }
>  
> +static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
> +{
> +	struct pkt_stream *pkt_stream;
> +
> +	pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
> +	test->ifobj_tx->pkt_stream = pkt_stream;
> +	test->ifobj_rx->pkt_stream = pkt_stream;
> +
> +	pkt_stream_delete(pkt_stream);

Shouldn't this be deleting the stream that got replaced? You're assigning
pkt_stream to ifobjs and then immediately free it.

I'd say that we should drop this call to pkt_stream_delete() in here
unless I'm missing something, pkt_stream_restore_default() will free the
currently assigned pkt stream and bring back the default one to the
ifobjs.

> +}
> +
>  static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
>  {
>  	struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb);
> @@ -557,7 +585,7 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *d
>  	if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
>  		u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
>  
> -		if (opt_pkt_dump && test_type != TEST_TYPE_STATS)
> +		if (opt_pkt_dump)
>  			pkt_dump(data, PKT_SIZE);
>  
>  		if (pkt->len != desc->len) {
> @@ -598,9 +626,6 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
>  	unsigned int rcvd;
>  	u32 idx;
>  
> -	if (!xsk->outstanding_tx)
> -		return;
> -
>  	if (xsk_ring_prod__needs_wakeup(&xsk->tx))
>  		kick_tx(xsk);
>  
> @@ -831,6 +856,7 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
>  
>  static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
>  {
> +	print_verbose("Destroying socket\n");
>  	xsk_socket__delete(ifobj->xsk->xsk);
>  	xsk_umem__delete(ifobj->umem->umem);
>  }
> @@ -878,9 +904,6 @@ static void *worker_testapp_validate_rx(void *arg)
>  	else
>  		receive_pkts(ifobject->pkt_stream, ifobject->xsk, &fds);
>  
> -	if (test_type == TEST_TYPE_TEARDOWN)
> -		print_verbose("Destroying socket\n");
> -
>  	if (test->total_steps == test->current_step)
>  		testapp_cleanup_xsk_res(ifobject);
>  	pthread_exit(NULL);
> @@ -890,19 +913,11 @@ static void testapp_validate_traffic(struct test_spec *test)
>  {
>  	struct ifobject *ifobj_tx = test->ifobj_tx;
>  	struct ifobject *ifobj_rx = test->ifobj_rx;
> -	struct pkt_stream *pkt_stream;
>  	pthread_t t0, t1;
>  
>  	if (pthread_barrier_init(&barr, NULL, 2))
>  		exit_with_error(errno);
>  
> -	if (stat_test_type == STAT_TEST_TX_INVALID)
> -		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT,
> -						 XSK_UMEM__INVALID_FRAME_SIZE);
> -	else
> -		pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
> -	ifobj_tx->pkt_stream = pkt_stream;
> -	ifobj_rx->pkt_stream = pkt_stream;
>  	test->current_step++;
>  
>  	/*Spawn RX thread */
> @@ -982,7 +997,9 @@ static void testapp_bpf_res(struct test_spec *test)
>  
>  static void testapp_stats(struct test_spec *test)
>  {
> -	for (int i = 0; i < STAT_TEST_TYPE_MAX; i++) {
> +	int i;
> +
> +	for (i = 0; i < STAT_TEST_TYPE_MAX; i++) {
>  		test_spec_reset(test);
>  		stat_test_type = i;
>  
> @@ -991,21 +1008,27 @@ 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;
> +			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;
> +			testapp_validate_traffic(test);
>  			break;
>  		case STAT_TEST_TX_INVALID:
>  			test_spec_set_name(test, "STAT_TX_INVALID");
> -			continue;
> +			pkt_stream_replace(test, DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
> +			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");
> +			testapp_validate_traffic(test);
>  			break;
>  		default:
>  			break;
>  		}
> -		testapp_validate_traffic(test);
>  	}
>  
>  	/* To only see the whole stat set being completed unless an individual test fails. */
> @@ -1106,6 +1129,7 @@ int main(int argc, char **argv)
>  {
>  	struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
>  	struct ifobject *ifobj_tx, *ifobj_rx;
> +	struct pkt_stream *pkt_stream_default;

rct broken by a little?

>  	struct test_spec test;
>  	u32 i, j;
>  
> @@ -1133,6 +1157,12 @@ int main(int argc, char **argv)
>  	init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
>  		   worker_testapp_validate_rx);
>  
> +	test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
> +	pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
> +	if (!pkt_stream_default)
> +		exit_with_error(ENOMEM);

I missed this probably while reviewing previous set, but to be consistent
with 083be682d976 ("selftests: xsk: Return correct error codes") this
probably should have -ENOMEM as an arg?

> +	test.pkt_stream_default = pkt_stream_default;
> +
>  	ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
>  
>  	for (i = 0; i < TEST_MODE_MAX; i++)
> @@ -1142,6 +1172,7 @@ int main(int argc, char **argv)
>  			usleep(USLEEP_MAX);
>  		}
>  
> +	pkt_stream_delete(pkt_stream_default);
>  	ifobject_delete(ifobj_tx);
>  	ifobject_delete(ifobj_rx);
>  
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> index c5baa7c5f560..e27fe348ae50 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.h
> +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> @@ -132,6 +132,7 @@ struct ifobject {
>  struct test_spec {
>  	struct ifobject *ifobj_tx;
>  	struct ifobject *ifobj_rx;
> +	struct pkt_stream *pkt_stream_default;
>  	u16 total_steps;
>  	u16 current_step;
>  	u16 nb_sockets;
> -- 
> 2.29.0
> 

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

* Re: [PATCH bpf-next 17/20] selftests: xsk: add test for unaligned mode
  2021-09-01 10:47 ` [PATCH bpf-next 17/20] selftests: xsk: add test for unaligned mode Magnus Karlsson
@ 2021-09-03 15:38   ` Maciej Fijalkowski
  2021-09-06  8:02     ` Magnus Karlsson
  0 siblings, 1 reply; 29+ messages in thread
From: Maciej Fijalkowski @ 2021-09-03 15:38 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
	ciara.loftus, bpf, yhs, andrii

On Wed, Sep 01, 2021 at 12:47:29PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Add a test for unaligned mode in which packet buffers can be placed
> anywhere within the umem. Some packets are made to straddle page
> boundraries in order to check for correctness. On the Tx side, buffers

boundaries

> are now allocated according to the addresses found in the packet
> stream. Thus, the placement of buffers can be controlled with the
> boolean use_addr_for_fill in the packet stream.
> 
> One new pkt_stream insterface is introduced: pkt_stream_replace_half()

interface

> that replaces every other packet in the default packet stream with the
> specified new packet.

Can you describe the introduction of DEFAULT_OFFSET ?

> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/xdpxceiver.c | 125 ++++++++++++++++++-----
>  tools/testing/selftests/bpf/xdpxceiver.h |   4 +
>  2 files changed, 106 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index d4aad4833754..a24068993cc3 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -19,7 +19,7 @@
>   * Virtual Ethernet interfaces.
>   *
>   * For each mode, the following tests are run:
> - *    a. nopoll - soft-irq processing
> + *    a. nopoll - soft-irq processing in run-to-completion mode
>   *    b. poll - using poll() syscall
>   *    c. Socket Teardown
>   *       Create a Tx and a Rx socket, Tx from one socket, Rx on another. Destroy
> @@ -45,6 +45,7 @@
>   *       Configure sockets at indexes 0 and 1, run a traffic on queue ids 0,
>   *       then remove xsk sockets from queue 0 on both veth interfaces and
>   *       finally run a traffic on queues ids 1
> + *    g. unaligned mode
>   *
>   * Total tests: 12
>   *
> @@ -243,6 +244,9 @@ static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size
>  	};
>  	int ret;
>  
> +	if (umem->unaligned_mode)
> +		cfg.flags |= XDP_UMEM_UNALIGNED_CHUNK_FLAG;
> +
>  	ret = xsk_umem__create(&umem->umem, buffer, size,
>  			       &umem->fq, &umem->cq, &cfg);
>  	if (ret)
> @@ -252,19 +256,6 @@ static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size
>  	return 0;
>  }
>  
> -static void xsk_populate_fill_ring(struct xsk_umem_info *umem)
> -{
> -	int ret, i;
> -	u32 idx = 0;
> -
> -	ret = xsk_ring_prod__reserve(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS, &idx);
> -	if (ret != XSK_RING_PROD__DEFAULT_NUM_DESCS)
> -		exit_with_error(-ret);
> -	for (i = 0; i < XSK_RING_PROD__DEFAULT_NUM_DESCS; i++)
> -		*xsk_ring_prod__fill_addr(&umem->fq, idx++) = i * umem->frame_size;
> -	xsk_ring_prod__submit(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS);
> -}
> -
>  static int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_info *umem,
>  				struct ifobject *ifobject, u32 qid)
>  {
> @@ -487,7 +478,8 @@ 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].addr = (i % umem->num_frames) * umem->frame_size +
> +			DEFAULT_OFFSET;
>  		pkt_stream->pkts[i].len = pkt_len;
>  		pkt_stream->pkts[i].payload = i;

Probably we need to init use_addr_for_fill to false by default in here as
pkt_stream is malloc'd.

>  
> @@ -500,6 +492,12 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
>  	return pkt_stream;
>  }
>  
> +static struct pkt_stream *pkt_stream_clone(struct xsk_umem_info *umem,
> +					   struct pkt_stream *pkt_stream)
> +{
> +	return pkt_stream_generate(umem, pkt_stream->nb_pkts, pkt_stream->pkts[0].len);
> +}
> +
>  static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
>  {
>  	struct pkt_stream *pkt_stream;
> @@ -507,8 +505,22 @@ static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
>  	pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
>  	test->ifobj_tx->pkt_stream = pkt_stream;
>  	test->ifobj_rx->pkt_stream = pkt_stream;
> +}
>  
> -	pkt_stream_delete(pkt_stream);
> +static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 offset)
> +{
> +	struct xsk_umem_info *umem = test->ifobj_tx->umem;
> +	struct pkt_stream *pkt_stream;
> +	u32 i;
> +
> +	pkt_stream = pkt_stream_clone(umem, test->pkt_stream_default);
> +	for (i = 0; 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;
> +	}
> +
> +	test->ifobj_tx->pkt_stream = pkt_stream;
> +	test->ifobj_rx->pkt_stream = pkt_stream;
>  }
>  
>  static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
> @@ -572,9 +584,9 @@ static void pkt_dump(void *pkt, u32 len)
>  	fprintf(stdout, "---------------------------------------\n");
>  }
>  
> -static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *desc)
> +static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len)
>  {
> -	void *data = xsk_umem__get_data(buffer, desc->addr);
> +	void *data = xsk_umem__get_data(buffer, addr);
>  	struct iphdr *iphdr = (struct iphdr *)(data + sizeof(struct ethhdr));
>  
>  	if (!pkt) {
> @@ -588,10 +600,10 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *d
>  		if (opt_pkt_dump)
>  			pkt_dump(data, PKT_SIZE);
>  
> -		if (pkt->len != desc->len) {
> +		if (pkt->len != len) {
>  			ksft_test_result_fail
>  				("ERROR: [%s] expected length [%d], got length [%d]\n",
> -					__func__, pkt->len, desc->len);
> +					__func__, pkt->len, len);
>  			return false;
>  		}
>  
> @@ -673,7 +685,7 @@ static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *
>  
>  			orig = xsk_umem__extract_addr(addr);
>  			addr = xsk_umem__add_offset_to_addr(addr);
> -			if (!is_pkt_valid(pkt, xsk->umem->buffer, desc))
> +			if (!is_pkt_valid(pkt, xsk->umem->buffer, addr, desc->len))
>  				return;
>  
>  			*xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
> @@ -817,13 +829,16 @@ static void tx_stats_validate(struct ifobject *ifobject)
>  
>  static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
>  {
> +	int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
>  	u32 i;
>  
>  	ifobject->ns_fd = switch_namespace(ifobject->nsname);
>  
> +	if (ifobject->umem->unaligned_mode)
> +		mmap_flags |= MAP_HUGETLB;
> +
>  	for (i = 0; i < test->nb_sockets; i++) {
>  		u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
> -		int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
>  		u32 ctr = 0;
>  		void *bufs;
>  
> @@ -881,6 +896,32 @@ static void *worker_testapp_validate_tx(void *arg)
>  	pthread_exit(NULL);
>  }
>  
> +static void xsk_populate_fill_ring(struct xsk_umem_info *umem, struct pkt_stream *pkt_stream)
> +{
> +	u32 idx = 0, i;
> +	int ret;
> +
> +	ret = xsk_ring_prod__reserve(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS, &idx);
> +	if (ret != XSK_RING_PROD__DEFAULT_NUM_DESCS)
> +		exit_with_error(ENOSPC);

-ENOSPC?

> +	for (i = 0; i < XSK_RING_PROD__DEFAULT_NUM_DESCS; i++) {
> +		u64 addr;
> +
> +		if (pkt_stream->use_addr_for_fill) {
> +			struct pkt *pkt = pkt_stream_get_pkt(pkt_stream, i);
> +
> +			if (!pkt)
> +				break;
> +			addr = pkt->addr;
> +		} else {
> +			addr = (i % umem->num_frames) * umem->frame_size + DEFAULT_OFFSET;
> +		}
> +
> +		*xsk_ring_prod__fill_addr(&umem->fq, idx++) = addr;
> +	}
> +	xsk_ring_prod__submit(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS);
> +}

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

* RE: [PATCH bpf-next 01/20] selftests: xsk: simplify xsk and umem arrays
  2021-09-01 10:47 ` [PATCH bpf-next 01/20] selftests: xsk: simplify xsk and umem arrays Magnus Karlsson
@ 2021-09-03 17:53   ` John Fastabend
  0 siblings, 0 replies; 29+ messages in thread
From: John Fastabend @ 2021-09-03 17:53 UTC (permalink / raw)
  To: Magnus Karlsson, magnus.karlsson, bjorn, ast, daniel, netdev,
	maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Simplify the xsk_info and umem_info allocation by allocating them
> upfront in an array, instead of allocating an array of pointers to
> future creations of these. Allocating them upfront also has the
> advantage that configuration information can be stored in these
> structures instead of relying on global variables. With the previous
> structure, xsk_info and umem_info were created too late to be able to
> store most configuration information. This will be used to eliminate
> most global variables in later patches in this series.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---

Acked-by: John Fastabend <john.fastabend@gmail.com>

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

* RE: [PATCH bpf-next 02/20] selftests: xsk: introduce type for thread function
  2021-09-01 10:47 ` [PATCH bpf-next 02/20] selftests: xsk: introduce type for thread function Magnus Karlsson
@ 2021-09-03 17:56   ` John Fastabend
  0 siblings, 0 replies; 29+ messages in thread
From: John Fastabend @ 2021-09-03 17:56 UTC (permalink / raw)
  To: Magnus Karlsson, magnus.karlsson, bjorn, ast, daniel, netdev,
	maciej.fijalkowski
  Cc: jonathan.lemon, ciara.loftus, bpf, yhs, andrii

Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Introduce a typedef of the thread function so this can be passed to
> init_iface() in order to simplify that function.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---

Acked-by: John Fastabend <john.fastabend@gmail.com>

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

* Re: [PATCH bpf-next 19/20] selftests: xsk: add tests for invalid xsk descriptors
  2021-09-01 10:47 ` [PATCH bpf-next 19/20] selftests: xsk: add tests for invalid xsk descriptors Magnus Karlsson
@ 2021-09-03 18:58   ` Maciej Fijalkowski
  2021-09-06  8:06     ` Magnus Karlsson
  0 siblings, 1 reply; 29+ messages in thread
From: Maciej Fijalkowski @ 2021-09-03 18:58 UTC (permalink / raw)
  To: Magnus Karlsson
  Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
	ciara.loftus, bpf, yhs, andrii

On Wed, Sep 01, 2021 at 12:47:31PM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
> 
> Add tests for invalid xsk descriptors in the Tx ring. A number of
> handcrafted nasty invalid descriptors are created and submitted to the
> tx ring to check that they are validated correctly. Corener case valid

Corner

> ones are also sent. The tests are run for both aligned and unaligned
> mode.
> 
> pkt_stream_set() is introduced to be able to create a hand-crafted
> packet stream where every single packet is specified in detail.
> 
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
>  tools/testing/selftests/bpf/xdpxceiver.c | 143 ++++++++++++++++++++---
>  tools/testing/selftests/bpf/xdpxceiver.h |   7 +-
>  2 files changed, 131 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index d085033afd53..a4f6ce3a6b14 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -46,6 +46,8 @@
>   *       then remove xsk sockets from queue 0 on both veth interfaces and
>   *       finally run a traffic on queues ids 1
>   *    g. unaligned mode
> + *    h. tests for invalid and corner case Tx descriptors so that the correct ones
> + *       are discarded and let through, respectively.
>   *
>   * Total tests: 12
>   *
> @@ -394,7 +396,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
>  		for (j = 0; j < MAX_SOCKETS; j++) {
>  			memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j]));
>  			memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j]));
> -			ifobj->umem_arr[j].num_frames = DEFAULT_PKT_CNT / 4;
> +			ifobj->umem_arr[j].num_frames = DEFAULT_UMEM_BUFFERS;
>  			ifobj->umem_arr[j].frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
>  			ifobj->xsk_arr[j].rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
>  		}
> @@ -450,6 +452,16 @@ 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)
> +{
> +	while (pkt_stream->rx_pkt_nb < pkt_stream->nb_pkts) {
> +		if (pkt_stream->pkts[pkt_stream->rx_pkt_nb].valid)
> +			return &pkt_stream->pkts[pkt_stream->rx_pkt_nb++];
> +		pkt_stream->rx_pkt_nb++;
> +	}
> +	return NULL;
> +}
> +
>  static void pkt_stream_delete(struct pkt_stream *pkt_stream)
>  {
>  	free(pkt_stream->pkts);
> @@ -465,17 +477,31 @@ static void pkt_stream_restore_default(struct test_spec *test)
>  	test->ifobj_rx->pkt_stream = test->pkt_stream_default;
>  }
>  
> -static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
> +static struct pkt_stream *__pkt_stream_alloc(u32 nb_pkts)
>  {
>  	struct pkt_stream *pkt_stream;
> -	u32 i;
>  
> -	pkt_stream = malloc(sizeof(*pkt_stream));
> +	pkt_stream = calloc(1, sizeof(*pkt_stream));

So with that probably my previous comment about use_addr_for_fill being
not set is not relevant.

>  	if (!pkt_stream)
> -		exit_with_error(ENOMEM);
> +		return NULL;
>  
>  	pkt_stream->pkts = calloc(nb_pkts, sizeof(*pkt_stream->pkts));
> -	if (!pkt_stream->pkts)
> +	if (!pkt_stream->pkts) {
> +		free(pkt_stream);
> +		return NULL;
> +	}
> +
> +	pkt_stream->nb_pkts = nb_pkts;
> +	return pkt_stream;
> +}
> +
> +static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
> +{
> +	struct pkt_stream *pkt_stream;
> +	u32 i;
> +
> +	pkt_stream = __pkt_stream_alloc(nb_pkts);
> +	if (!pkt_stream)
>  		exit_with_error(ENOMEM);
>  
>  	pkt_stream->nb_pkts = nb_pkts;
> @@ -525,6 +551,26 @@ static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 off
>  	test->ifobj_rx->pkt_stream = pkt_stream;
>  }
>  
> +static void pkt_stream_set(struct test_spec *test, struct pkt *pkts, u32 nb_pkts)

This is still a generation of pkt stream, can we name this is as:
pkt_stream_generate_custom or is this too long? WDYT?

> +{
> +	struct pkt_stream *pkt_stream;
> +	u32 i;
> +
> +	pkt_stream = __pkt_stream_alloc(nb_pkts);
> +	if (!pkt_stream)
> +		exit_with_error(ENOMEM);
> +
> +	test->ifobj_tx->pkt_stream = pkt_stream;
> +	test->ifobj_rx->pkt_stream = pkt_stream;
> +
> +	for (i = 0; i < nb_pkts; i++) {
> +		pkt_stream->pkts[i].addr = pkts[i].addr;
> +		pkt_stream->pkts[i].len = pkts[i].len;
> +		pkt_stream->pkts[i].payload = i;
> +		pkt_stream->pkts[i].valid = pkts[i].valid;
> +	}
> +}
> +
>  static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
>  {
>  	struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb);
> @@ -535,6 +581,8 @@ static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
>  
>  	if (!pkt)
>  		return NULL;
> +	if (!pkt->valid || pkt->len < PKT_SIZE)
> +		return pkt;
>  
>  	data = xsk_umem__get_data(ifobject->umem->buffer, pkt->addr);
>  	udp_hdr = (struct udphdr *)(data + sizeof(struct ethhdr) + sizeof(struct iphdr));
> @@ -596,19 +644,24 @@ 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 minimun size. */

minimum

> +		return true;
> +	}
> +
> +	if (pkt->len != len) {
> +		ksft_test_result_fail
> +			("ERROR: [%s] expected length [%d], got length [%d]\n",
> +			 __func__, pkt->len, len);
> +		return false;
> +	}
> +
>  	if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
>  		u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
>  
>  		if (opt_pkt_dump)
>  			pkt_dump(data, PKT_SIZE);
>  
> -		if (pkt->len != len) {
> -			ksft_test_result_fail
> -				("ERROR: [%s] expected length [%d], got length [%d]\n",
> -					__func__, pkt->len, len);
> -			return false;
> -		}
> -
>  		if (pkt->payload != seqnum) {
>  			ksft_test_result_fail
>  				("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
> @@ -645,6 +698,15 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
>  
>  	rcvd = xsk_ring_cons__peek(&xsk->umem->cq, batch_size, &idx);
>  	if (rcvd) {
> +		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("Last completion address: %llx\n", addr);
> +			return;
> +		}
> +
>  		xsk_ring_cons__release(&xsk->umem->cq, rcvd);
>  		xsk->outstanding_tx -= rcvd;
>  	}
> @@ -653,11 +715,10 @@ 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)
>  {
> -	u32 idx_rx = 0, idx_fq = 0, rcvd, i, pkt_count = 0;
> -	struct pkt *pkt;
> +	struct pkt *pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
> +	u32 idx_rx = 0, idx_fq = 0, rcvd, i;
>  	int ret;
>  
> -	pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++);
>  	while (pkt) {
>  		rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
>  		if (!rcvd) {
> @@ -685,13 +746,21 @@ static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *
>  			const struct xdp_desc *desc = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
>  			u64 addr = desc->addr, orig;
>  
> +			if (!pkt) {
> +				ksft_test_result_fail("ERROR: [%s] Received too many packets.\n",
> +						      __func__);
> +				ksft_print_msg("Last packet has addr: %llx len: %u\n",
> +					       addr, desc->len);
> +				return;
> +			}
> +
>  			orig = xsk_umem__extract_addr(addr);
>  			addr = xsk_umem__add_offset_to_addr(addr);
>  			if (!is_pkt_valid(pkt, xsk->umem->buffer, addr, desc->len))
>  				return;
>  
>  			*xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
> -			pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++);
> +			pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
>  		}
>  
>  		xsk_ring_prod__submit(&xsk->umem->fq, rcvd);
> @@ -875,6 +944,7 @@ static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
>  {
>  	print_verbose("Destroying socket\n");
>  	xsk_socket__delete(ifobj->xsk->xsk);
> +	munmap(ifobj->umem->buffer, ifobj->umem->num_frames * ifobj->umem->frame_size);
>  	xsk_umem__delete(ifobj->umem->umem);
>  }
>  
> @@ -1118,6 +1188,33 @@ static bool testapp_unaligned(struct test_spec *test)
>  	return true;
>  }
>  
> +static void testapp_inv_desc(struct test_spec *test)

I'd say that speaking out whole 'invalid' word wouldn't hurt us here.

> +{
> +	struct pkt pkts[] = {{0, 0, 0, true}, /* Zero packet length and zero address allowed */

Above looks a bit weird, maybe this should be formatted same as the lines
below:

	struct pkt pkts[] = {
		/* Zero packet length and zero address allowed */
			     {0, 0, 0, true},

> +		/* Zero packet length allowed */
> +			     {0x1000, 0, 0, true},
> +		/* Straddling the start of umem */
> +			     {-2, PKT_SIZE, 0, false},
> +		/* Packet too large */
> +			     {0x2000, XSK_UMEM__INVALID_FRAME_SIZE, 0, false},
> +		/* After umem ends */
> +			     {UMEM_SIZE, PKT_SIZE, 0, false},
> +		/* Straddle the end of umem */
> +			     {UMEM_SIZE - PKT_SIZE / 2, PKT_SIZE, 0, false},
> +		/* Straddle a page boundrary */
> +			     {0x3000 - PKT_SIZE / 2, PKT_SIZE, 0, false},
> +		/* Valid packet for synch so that something is received */
> +			     {0x4000, PKT_SIZE, 0, true}};
> +
> +	if (test->ifobj_tx->umem->unaligned_mode) {
> +		/* Crossing a page boundrary allowed */
> +		pkts[6].valid = true;
> +	}
> +	pkt_stream_set(test, pkts, ARRAY_SIZE(pkts));
> +	testapp_validate_traffic(test);
> +	pkt_stream_restore_default(test);
> +}
> +
>  static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
>  		       const char *dst_ip, const char *src_ip, const u16 dst_port,
>  		       const u16 src_port, thread_func_t func_ptr)
> @@ -1159,7 +1256,7 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
>  	case TEST_TYPE_BPF_RES:
>  		testapp_bpf_res(test);
>  		break;
> -	case TEST_TYPE_NOPOLL:
> +	case TEST_TYPE_RUN_TO_COMPLETION:

I think that you were updating some comment around it on previous patches
and it probably belongs to this one.

>  		test_spec_set_name(test, "RUN_TO_COMPLETION");
>  		testapp_validate_traffic(test);
>  		break;
> @@ -1169,6 +1266,16 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
>  		test_spec_set_name(test, "POLL");
>  		testapp_validate_traffic(test);
>  		break;
> +	case TEST_TYPE_ALIGNED_INV_DESC:
> +		test_spec_set_name(test, "ALIGNED_INV_DESC");
> +		testapp_inv_desc(test);
> +		break;
> +	case TEST_TYPE_UNALIGNED_INV_DESC:
> +		test_spec_set_name(test, "UNALIGNED_INV_DESC");
> +		test->ifobj_tx->umem->unaligned_mode = true;
> +		test->ifobj_rx->umem->unaligned_mode = true;
> +		testapp_inv_desc(test);
> +		break;
>  	case TEST_TYPE_UNALIGNED:
>  		if (!testapp_unaligned(test))
>  			return;
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> index 129801eb013c..2d9efb89ea28 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.h
> +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> @@ -38,6 +38,8 @@
>  #define BATCH_SIZE 8
>  #define POLL_TMOUT 1000
>  #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)
>  #define RX_FULL_RXQSIZE 32
>  #define DEFAULT_OFFSET 256
>  #define XSK_UMEM__INVALID_FRAME_SIZE (XSK_UMEM__DEFAULT_FRAME_SIZE + 1)
> @@ -51,9 +53,11 @@ enum test_mode {
>  };
>  
>  enum test_type {
> -	TEST_TYPE_NOPOLL,
> +	TEST_TYPE_RUN_TO_COMPLETION,
>  	TEST_TYPE_POLL,
>  	TEST_TYPE_UNALIGNED,
> +	TEST_TYPE_ALIGNED_INV_DESC,
> +	TEST_TYPE_UNALIGNED_INV_DESC,
>  	TEST_TYPE_TEARDOWN,
>  	TEST_TYPE_BIDI,
>  	TEST_TYPE_STATS,
> @@ -104,6 +108,7 @@ struct pkt {
>  
>  struct pkt_stream {
>  	u32 nb_pkts;
> +	u32 rx_pkt_nb;
>  	struct pkt *pkts;
>  	bool use_addr_for_fill;
>  };
> -- 
> 2.29.0
> 

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

* Re: [PATCH bpf-next 16/20] selftests: xsk: introduce replacing the default packet stream
  2021-09-03 15:04   ` Maciej Fijalkowski
@ 2021-09-06  8:00     ` Magnus Karlsson
  0 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-06  8:00 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: Karlsson, Magnus, Björn Töpel, Alexei Starovoitov,
	Daniel Borkmann, Network Development, Jonathan Lemon,
	Ciara Loftus, bpf, Yonghong Song, Andrii Nakryiko

On Fri, Sep 3, 2021 at 3:04 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Wed, Sep 01, 2021 at 12:47:28PM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Introduce the concept of a default packet stream that is the set of
> > packets sent by most tests. Then add the ability to replace it for a
> > test that would like to send or receive something else through the use
> > of the function pkt_stream_replace() and then restored with
> > pkt_stream_restore_default(). These are then used to convert the
> > STAT_TX_INVALID_TEST to use these new APIs.
>
> s/STAT_TX_INVALID_TEST/STAT_TEST_TX_INVALID

Will fix.

> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >  tools/testing/selftests/bpf/xdpxceiver.c | 67 +++++++++++++++++-------
> >  tools/testing/selftests/bpf/xdpxceiver.h |  1 +
> >  2 files changed, 50 insertions(+), 18 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> > index 09d2854c10e6..d4aad4833754 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.c
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> > @@ -390,6 +390,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> >               ifobj->umem = &ifobj->umem_arr[0];
> >               ifobj->xsk = &ifobj->xsk_arr[0];
> >               ifobj->use_poll = false;
> > +             ifobj->pkt_stream = test->pkt_stream_default;
> >
> >               if (i == 0) {
> >                       ifobj->rx_on = false;
> > @@ -418,9 +419,12 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> >  static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> >                          struct ifobject *ifobj_rx, enum test_mode mode)
> >  {
> > +     struct pkt_stream *pkt_stream;
> >       u32 i;
> >
> > +     pkt_stream = test->pkt_stream_default;
> >       memset(test, 0, sizeof(*test));
> > +     test->pkt_stream_default = pkt_stream;
> >
> >       for (i = 0; i < MAX_INTERFACES; i++) {
> >               struct ifobject *ifobj = i ? ifobj_rx : ifobj_tx;
> > @@ -455,6 +459,19 @@ static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
> >       return &pkt_stream->pkts[pkt_nb];
> >  }
> >
> > +static void pkt_stream_delete(struct pkt_stream *pkt_stream)
> > +{
> > +     free(pkt_stream->pkts);
> > +     free(pkt_stream);
> > +}
> > +
> > +static void pkt_stream_restore_default(struct test_spec *test)
> > +{
> > +     pkt_stream_delete(test->ifobj_tx->pkt_stream);
>
> I suppose that streams are the same for both tx and rx ifobjs hence it's
> enough to call the delete op a single time.

At this point in time, yes. But this has to change as we develop more tests.

> > +     test->ifobj_tx->pkt_stream = test->pkt_stream_default;
> > +     test->ifobj_rx->pkt_stream = test->pkt_stream_default;
> > +}
> > +
> >  static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
> >  {
> >       struct pkt_stream *pkt_stream;
> > @@ -483,6 +500,17 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
> >       return pkt_stream;
> >  }
> >
> > +static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
> > +{
> > +     struct pkt_stream *pkt_stream;
> > +
> > +     pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
> > +     test->ifobj_tx->pkt_stream = pkt_stream;
> > +     test->ifobj_rx->pkt_stream = pkt_stream;
> > +
> > +     pkt_stream_delete(pkt_stream);
>
> Shouldn't this be deleting the stream that got replaced? You're assigning
> pkt_stream to ifobjs and then immediately free it.
>
> I'd say that we should drop this call to pkt_stream_delete() in here
> unless I'm missing something, pkt_stream_restore_default() will free the
> currently assigned pkt stream and bring back the default one to the
> ifobjs.

That is indeed a bug and should be removed.

> > +}
> > +
> >  static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
> >  {
> >       struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb);
> > @@ -557,7 +585,7 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *d
> >       if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
> >               u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
> >
> > -             if (opt_pkt_dump && test_type != TEST_TYPE_STATS)
> > +             if (opt_pkt_dump)
> >                       pkt_dump(data, PKT_SIZE);
> >
> >               if (pkt->len != desc->len) {
> > @@ -598,9 +626,6 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
> >       unsigned int rcvd;
> >       u32 idx;
> >
> > -     if (!xsk->outstanding_tx)
> > -             return;
> > -
> >       if (xsk_ring_prod__needs_wakeup(&xsk->tx))
> >               kick_tx(xsk);
> >
> > @@ -831,6 +856,7 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
> >
> >  static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
> >  {
> > +     print_verbose("Destroying socket\n");
> >       xsk_socket__delete(ifobj->xsk->xsk);
> >       xsk_umem__delete(ifobj->umem->umem);
> >  }
> > @@ -878,9 +904,6 @@ static void *worker_testapp_validate_rx(void *arg)
> >       else
> >               receive_pkts(ifobject->pkt_stream, ifobject->xsk, &fds);
> >
> > -     if (test_type == TEST_TYPE_TEARDOWN)
> > -             print_verbose("Destroying socket\n");
> > -
> >       if (test->total_steps == test->current_step)
> >               testapp_cleanup_xsk_res(ifobject);
> >       pthread_exit(NULL);
> > @@ -890,19 +913,11 @@ static void testapp_validate_traffic(struct test_spec *test)
> >  {
> >       struct ifobject *ifobj_tx = test->ifobj_tx;
> >       struct ifobject *ifobj_rx = test->ifobj_rx;
> > -     struct pkt_stream *pkt_stream;
> >       pthread_t t0, t1;
> >
> >       if (pthread_barrier_init(&barr, NULL, 2))
> >               exit_with_error(errno);
> >
> > -     if (stat_test_type == STAT_TEST_TX_INVALID)
> > -             pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT,
> > -                                              XSK_UMEM__INVALID_FRAME_SIZE);
> > -     else
> > -             pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
> > -     ifobj_tx->pkt_stream = pkt_stream;
> > -     ifobj_rx->pkt_stream = pkt_stream;
> >       test->current_step++;
> >
> >       /*Spawn RX thread */
> > @@ -982,7 +997,9 @@ static void testapp_bpf_res(struct test_spec *test)
> >
> >  static void testapp_stats(struct test_spec *test)
> >  {
> > -     for (int i = 0; i < STAT_TEST_TYPE_MAX; i++) {
> > +     int i;
> > +
> > +     for (i = 0; i < STAT_TEST_TYPE_MAX; i++) {
> >               test_spec_reset(test);
> >               stat_test_type = i;
> >
> > @@ -991,21 +1008,27 @@ 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;
> > +                     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;
> > +                     testapp_validate_traffic(test);
> >                       break;
> >               case STAT_TEST_TX_INVALID:
> >                       test_spec_set_name(test, "STAT_TX_INVALID");
> > -                     continue;
> > +                     pkt_stream_replace(test, DEFAULT_PKT_CNT, XSK_UMEM__INVALID_FRAME_SIZE);
> > +                     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");
> > +                     testapp_validate_traffic(test);
> >                       break;
> >               default:
> >                       break;
> >               }
> > -             testapp_validate_traffic(test);
> >       }
> >
> >       /* To only see the whole stat set being completed unless an individual test fails. */
> > @@ -1106,6 +1129,7 @@ int main(int argc, char **argv)
> >  {
> >       struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
> >       struct ifobject *ifobj_tx, *ifobj_rx;
> > +     struct pkt_stream *pkt_stream_default;
>
> rct broken by a little?
>
> >       struct test_spec test;
> >       u32 i, j;
> >
> > @@ -1133,6 +1157,12 @@ int main(int argc, char **argv)
> >       init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
> >                  worker_testapp_validate_rx);
> >
> > +     test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
> > +     pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
> > +     if (!pkt_stream_default)
> > +             exit_with_error(ENOMEM);
>
> I missed this probably while reviewing previous set, but to be consistent
> with 083be682d976 ("selftests: xsk: Return correct error codes") this
> probably should have -ENOMEM as an arg?

Have eliminated exit_with_error from most of the functions I have been
touching, but not from the main function. Want the error to be printed
out before exiting the program, so kept it at this level.

> > +     test.pkt_stream_default = pkt_stream_default;
> > +
> >       ksft_set_plan(TEST_MODE_MAX * TEST_TYPE_MAX);
> >
> >       for (i = 0; i < TEST_MODE_MAX; i++)
> > @@ -1142,6 +1172,7 @@ int main(int argc, char **argv)
> >                       usleep(USLEEP_MAX);
> >               }
> >
> > +     pkt_stream_delete(pkt_stream_default);
> >       ifobject_delete(ifobj_tx);
> >       ifobject_delete(ifobj_rx);
> >
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> > index c5baa7c5f560..e27fe348ae50 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.h
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> > @@ -132,6 +132,7 @@ struct ifobject {
> >  struct test_spec {
> >       struct ifobject *ifobj_tx;
> >       struct ifobject *ifobj_rx;
> > +     struct pkt_stream *pkt_stream_default;
> >       u16 total_steps;
> >       u16 current_step;
> >       u16 nb_sockets;
> > --
> > 2.29.0
> >

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

* Re: [PATCH bpf-next 17/20] selftests: xsk: add test for unaligned mode
  2021-09-03 15:38   ` Maciej Fijalkowski
@ 2021-09-06  8:02     ` Magnus Karlsson
  0 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-06  8:02 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: Karlsson, Magnus, Björn Töpel, Alexei Starovoitov,
	Daniel Borkmann, Network Development, Jonathan Lemon,
	Ciara Loftus, bpf, Yonghong Song, Andrii Nakryiko

On Fri, Sep 3, 2021 at 3:37 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Wed, Sep 01, 2021 at 12:47:29PM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Add a test for unaligned mode in which packet buffers can be placed
> > anywhere within the umem. Some packets are made to straddle page
> > boundraries in order to check for correctness. On the Tx side, buffers
>
> boundaries
>
> > are now allocated according to the addresses found in the packet
> > stream. Thus, the placement of buffers can be controlled with the
> > boolean use_addr_for_fill in the packet stream.
> >
> > One new pkt_stream insterface is introduced: pkt_stream_replace_half()
>
> interface
>
> > that replaces every other packet in the default packet stream with the
> > specified new packet.
>
> Can you describe the introduction of DEFAULT_OFFSET ?

Will fix both.

> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >  tools/testing/selftests/bpf/xdpxceiver.c | 125 ++++++++++++++++++-----
> >  tools/testing/selftests/bpf/xdpxceiver.h |   4 +
> >  2 files changed, 106 insertions(+), 23 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> > index d4aad4833754..a24068993cc3 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.c
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> > @@ -19,7 +19,7 @@
> >   * Virtual Ethernet interfaces.
> >   *
> >   * For each mode, the following tests are run:
> > - *    a. nopoll - soft-irq processing
> > + *    a. nopoll - soft-irq processing in run-to-completion mode
> >   *    b. poll - using poll() syscall
> >   *    c. Socket Teardown
> >   *       Create a Tx and a Rx socket, Tx from one socket, Rx on another. Destroy
> > @@ -45,6 +45,7 @@
> >   *       Configure sockets at indexes 0 and 1, run a traffic on queue ids 0,
> >   *       then remove xsk sockets from queue 0 on both veth interfaces and
> >   *       finally run a traffic on queues ids 1
> > + *    g. unaligned mode
> >   *
> >   * Total tests: 12
> >   *
> > @@ -243,6 +244,9 @@ static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size
> >       };
> >       int ret;
> >
> > +     if (umem->unaligned_mode)
> > +             cfg.flags |= XDP_UMEM_UNALIGNED_CHUNK_FLAG;
> > +
> >       ret = xsk_umem__create(&umem->umem, buffer, size,
> >                              &umem->fq, &umem->cq, &cfg);
> >       if (ret)
> > @@ -252,19 +256,6 @@ static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size
> >       return 0;
> >  }
> >
> > -static void xsk_populate_fill_ring(struct xsk_umem_info *umem)
> > -{
> > -     int ret, i;
> > -     u32 idx = 0;
> > -
> > -     ret = xsk_ring_prod__reserve(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS, &idx);
> > -     if (ret != XSK_RING_PROD__DEFAULT_NUM_DESCS)
> > -             exit_with_error(-ret);
> > -     for (i = 0; i < XSK_RING_PROD__DEFAULT_NUM_DESCS; i++)
> > -             *xsk_ring_prod__fill_addr(&umem->fq, idx++) = i * umem->frame_size;
> > -     xsk_ring_prod__submit(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS);
> > -}
> > -
> >  static int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_info *umem,
> >                               struct ifobject *ifobject, u32 qid)
> >  {
> > @@ -487,7 +478,8 @@ 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].addr = (i % umem->num_frames) * umem->frame_size +
> > +                     DEFAULT_OFFSET;
> >               pkt_stream->pkts[i].len = pkt_len;
> >               pkt_stream->pkts[i].payload = i;
>
> Probably we need to init use_addr_for_fill to false by default in here as
> pkt_stream is malloc'd.

I will use calloc here, instead of in patch #19. Was fixed too late in
the patch set.

> >
> > @@ -500,6 +492,12 @@ static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb
> >       return pkt_stream;
> >  }
> >
> > +static struct pkt_stream *pkt_stream_clone(struct xsk_umem_info *umem,
> > +                                        struct pkt_stream *pkt_stream)
> > +{
> > +     return pkt_stream_generate(umem, pkt_stream->nb_pkts, pkt_stream->pkts[0].len);
> > +}
> > +
> >  static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
> >  {
> >       struct pkt_stream *pkt_stream;
> > @@ -507,8 +505,22 @@ static void pkt_stream_replace(struct test_spec *test, u32 nb_pkts, u32 pkt_len)
> >       pkt_stream = pkt_stream_generate(test->ifobj_tx->umem, nb_pkts, pkt_len);
> >       test->ifobj_tx->pkt_stream = pkt_stream;
> >       test->ifobj_rx->pkt_stream = pkt_stream;
> > +}
> >
> > -     pkt_stream_delete(pkt_stream);
> > +static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 offset)
> > +{
> > +     struct xsk_umem_info *umem = test->ifobj_tx->umem;
> > +     struct pkt_stream *pkt_stream;
> > +     u32 i;
> > +
> > +     pkt_stream = pkt_stream_clone(umem, test->pkt_stream_default);
> > +     for (i = 0; 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;
> > +     }
> > +
> > +     test->ifobj_tx->pkt_stream = pkt_stream;
> > +     test->ifobj_rx->pkt_stream = pkt_stream;
> >  }
> >
> >  static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
> > @@ -572,9 +584,9 @@ static void pkt_dump(void *pkt, u32 len)
> >       fprintf(stdout, "---------------------------------------\n");
> >  }
> >
> > -static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *desc)
> > +static bool is_pkt_valid(struct pkt *pkt, void *buffer, u64 addr, u32 len)
> >  {
> > -     void *data = xsk_umem__get_data(buffer, desc->addr);
> > +     void *data = xsk_umem__get_data(buffer, addr);
> >       struct iphdr *iphdr = (struct iphdr *)(data + sizeof(struct ethhdr));
> >
> >       if (!pkt) {
> > @@ -588,10 +600,10 @@ static bool is_pkt_valid(struct pkt *pkt, void *buffer, const struct xdp_desc *d
> >               if (opt_pkt_dump)
> >                       pkt_dump(data, PKT_SIZE);
> >
> > -             if (pkt->len != desc->len) {
> > +             if (pkt->len != len) {
> >                       ksft_test_result_fail
> >                               ("ERROR: [%s] expected length [%d], got length [%d]\n",
> > -                                     __func__, pkt->len, desc->len);
> > +                                     __func__, pkt->len, len);
> >                       return false;
> >               }
> >
> > @@ -673,7 +685,7 @@ static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *
> >
> >                       orig = xsk_umem__extract_addr(addr);
> >                       addr = xsk_umem__add_offset_to_addr(addr);
> > -                     if (!is_pkt_valid(pkt, xsk->umem->buffer, desc))
> > +                     if (!is_pkt_valid(pkt, xsk->umem->buffer, addr, desc->len))
> >                               return;
> >
> >                       *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
> > @@ -817,13 +829,16 @@ static void tx_stats_validate(struct ifobject *ifobject)
> >
> >  static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
> >  {
> > +     int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
> >       u32 i;
> >
> >       ifobject->ns_fd = switch_namespace(ifobject->nsname);
> >
> > +     if (ifobject->umem->unaligned_mode)
> > +             mmap_flags |= MAP_HUGETLB;
> > +
> >       for (i = 0; i < test->nb_sockets; i++) {
> >               u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
> > -             int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
> >               u32 ctr = 0;
> >               void *bufs;
> >
> > @@ -881,6 +896,32 @@ static void *worker_testapp_validate_tx(void *arg)
> >       pthread_exit(NULL);
> >  }
> >
> > +static void xsk_populate_fill_ring(struct xsk_umem_info *umem, struct pkt_stream *pkt_stream)
> > +{
> > +     u32 idx = 0, i;
> > +     int ret;
> > +
> > +     ret = xsk_ring_prod__reserve(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS, &idx);
> > +     if (ret != XSK_RING_PROD__DEFAULT_NUM_DESCS)
> > +             exit_with_error(ENOSPC);
>
> -ENOSPC?

Without the minus sign is correct here. Though I would prefer to not
use exit_with_error at all except for in the main function, this would
require a lot of surgery and is left as an exercise for later patch
sets.

> > +     for (i = 0; i < XSK_RING_PROD__DEFAULT_NUM_DESCS; i++) {
> > +             u64 addr;
> > +
> > +             if (pkt_stream->use_addr_for_fill) {
> > +                     struct pkt *pkt = pkt_stream_get_pkt(pkt_stream, i);
> > +
> > +                     if (!pkt)
> > +                             break;
> > +                     addr = pkt->addr;
> > +             } else {
> > +                     addr = (i % umem->num_frames) * umem->frame_size + DEFAULT_OFFSET;
> > +             }
> > +
> > +             *xsk_ring_prod__fill_addr(&umem->fq, idx++) = addr;
> > +     }
> > +     xsk_ring_prod__submit(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS);
> > +}

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

* Re: [PATCH bpf-next 19/20] selftests: xsk: add tests for invalid xsk descriptors
  2021-09-03 18:58   ` Maciej Fijalkowski
@ 2021-09-06  8:06     ` Magnus Karlsson
  0 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2021-09-06  8:06 UTC (permalink / raw)
  To: Maciej Fijalkowski
  Cc: Karlsson, Magnus, Björn Töpel, Alexei Starovoitov,
	Daniel Borkmann, Network Development, Jonathan Lemon,
	Ciara Loftus, bpf, Yonghong Song, Andrii Nakryiko

On Fri, Sep 3, 2021 at 6:57 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Wed, Sep 01, 2021 at 12:47:31PM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Add tests for invalid xsk descriptors in the Tx ring. A number of
> > handcrafted nasty invalid descriptors are created and submitted to the
> > tx ring to check that they are validated correctly. Corener case valid
>
> Corner

Will fix.

> > ones are also sent. The tests are run for both aligned and unaligned
> > mode.
> >
> > pkt_stream_set() is introduced to be able to create a hand-crafted
> > packet stream where every single packet is specified in detail.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> >  tools/testing/selftests/bpf/xdpxceiver.c | 143 ++++++++++++++++++++---
> >  tools/testing/selftests/bpf/xdpxceiver.h |   7 +-
> >  2 files changed, 131 insertions(+), 19 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> > index d085033afd53..a4f6ce3a6b14 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.c
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> > @@ -46,6 +46,8 @@
> >   *       then remove xsk sockets from queue 0 on both veth interfaces and
> >   *       finally run a traffic on queues ids 1
> >   *    g. unaligned mode
> > + *    h. tests for invalid and corner case Tx descriptors so that the correct ones
> > + *       are discarded and let through, respectively.
> >   *
> >   * Total tests: 12
> >   *
> > @@ -394,7 +396,7 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> >               for (j = 0; j < MAX_SOCKETS; j++) {
> >                       memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j]));
> >                       memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j]));
> > -                     ifobj->umem_arr[j].num_frames = DEFAULT_PKT_CNT / 4;
> > +                     ifobj->umem_arr[j].num_frames = DEFAULT_UMEM_BUFFERS;
> >                       ifobj->umem_arr[j].frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
> >                       ifobj->xsk_arr[j].rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> >               }
> > @@ -450,6 +452,16 @@ 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)
> > +{
> > +     while (pkt_stream->rx_pkt_nb < pkt_stream->nb_pkts) {
> > +             if (pkt_stream->pkts[pkt_stream->rx_pkt_nb].valid)
> > +                     return &pkt_stream->pkts[pkt_stream->rx_pkt_nb++];
> > +             pkt_stream->rx_pkt_nb++;
> > +     }
> > +     return NULL;
> > +}
> > +
> >  static void pkt_stream_delete(struct pkt_stream *pkt_stream)
> >  {
> >       free(pkt_stream->pkts);
> > @@ -465,17 +477,31 @@ static void pkt_stream_restore_default(struct test_spec *test)
> >       test->ifobj_rx->pkt_stream = test->pkt_stream_default;
> >  }
> >
> > -static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
> > +static struct pkt_stream *__pkt_stream_alloc(u32 nb_pkts)
> >  {
> >       struct pkt_stream *pkt_stream;
> > -     u32 i;
> >
> > -     pkt_stream = malloc(sizeof(*pkt_stream));
> > +     pkt_stream = calloc(1, sizeof(*pkt_stream));
>
> So with that probably my previous comment about use_addr_for_fill being
> not set is not relevant.

Will fix this earlier.

> >       if (!pkt_stream)
> > -             exit_with_error(ENOMEM);
> > +             return NULL;
> >
> >       pkt_stream->pkts = calloc(nb_pkts, sizeof(*pkt_stream->pkts));
> > -     if (!pkt_stream->pkts)
> > +     if (!pkt_stream->pkts) {
> > +             free(pkt_stream);
> > +             return NULL;
> > +     }
> > +
> > +     pkt_stream->nb_pkts = nb_pkts;
> > +     return pkt_stream;
> > +}
> > +
> > +static struct pkt_stream *pkt_stream_generate(struct xsk_umem_info *umem, u32 nb_pkts, u32 pkt_len)
> > +{
> > +     struct pkt_stream *pkt_stream;
> > +     u32 i;
> > +
> > +     pkt_stream = __pkt_stream_alloc(nb_pkts);
> > +     if (!pkt_stream)
> >               exit_with_error(ENOMEM);
> >
> >       pkt_stream->nb_pkts = nb_pkts;
> > @@ -525,6 +551,26 @@ static void pkt_stream_replace_half(struct test_spec *test, u32 pkt_len, u32 off
> >       test->ifobj_rx->pkt_stream = pkt_stream;
> >  }
> >
> > +static void pkt_stream_set(struct test_spec *test, struct pkt *pkts, u32 nb_pkts)
>
> This is still a generation of pkt stream, can we name this is as:
> pkt_stream_generate_custom or is this too long? WDYT?

Sounds good.

> > +{
> > +     struct pkt_stream *pkt_stream;
> > +     u32 i;
> > +
> > +     pkt_stream = __pkt_stream_alloc(nb_pkts);
> > +     if (!pkt_stream)
> > +             exit_with_error(ENOMEM);
> > +
> > +     test->ifobj_tx->pkt_stream = pkt_stream;
> > +     test->ifobj_rx->pkt_stream = pkt_stream;
> > +
> > +     for (i = 0; i < nb_pkts; i++) {
> > +             pkt_stream->pkts[i].addr = pkts[i].addr;
> > +             pkt_stream->pkts[i].len = pkts[i].len;
> > +             pkt_stream->pkts[i].payload = i;
> > +             pkt_stream->pkts[i].valid = pkts[i].valid;
> > +     }
> > +}
> > +
> >  static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
> >  {
> >       struct pkt *pkt = pkt_stream_get_pkt(ifobject->pkt_stream, pkt_nb);
> > @@ -535,6 +581,8 @@ static struct pkt *pkt_generate(struct ifobject *ifobject, u32 pkt_nb)
> >
> >       if (!pkt)
> >               return NULL;
> > +     if (!pkt->valid || pkt->len < PKT_SIZE)
> > +             return pkt;
> >
> >       data = xsk_umem__get_data(ifobject->umem->buffer, pkt->addr);
> >       udp_hdr = (struct udphdr *)(data + sizeof(struct ethhdr) + sizeof(struct iphdr));
> > @@ -596,19 +644,24 @@ 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 minimun size. */
>
> minimum
>
> > +             return true;
> > +     }
> > +
> > +     if (pkt->len != len) {
> > +             ksft_test_result_fail
> > +                     ("ERROR: [%s] expected length [%d], got length [%d]\n",
> > +                      __func__, pkt->len, len);
> > +             return false;
> > +     }
> > +
> >       if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
> >               u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
> >
> >               if (opt_pkt_dump)
> >                       pkt_dump(data, PKT_SIZE);
> >
> > -             if (pkt->len != len) {
> > -                     ksft_test_result_fail
> > -                             ("ERROR: [%s] expected length [%d], got length [%d]\n",
> > -                                     __func__, pkt->len, len);
> > -                     return false;
> > -             }
> > -
> >               if (pkt->payload != seqnum) {
> >                       ksft_test_result_fail
> >                               ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
> > @@ -645,6 +698,15 @@ static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
> >
> >       rcvd = xsk_ring_cons__peek(&xsk->umem->cq, batch_size, &idx);
> >       if (rcvd) {
> > +             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("Last completion address: %llx\n", addr);
> > +                     return;
> > +             }
> > +
> >               xsk_ring_cons__release(&xsk->umem->cq, rcvd);
> >               xsk->outstanding_tx -= rcvd;
> >       }
> > @@ -653,11 +715,10 @@ 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)
> >  {
> > -     u32 idx_rx = 0, idx_fq = 0, rcvd, i, pkt_count = 0;
> > -     struct pkt *pkt;
> > +     struct pkt *pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
> > +     u32 idx_rx = 0, idx_fq = 0, rcvd, i;
> >       int ret;
> >
> > -     pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++);
> >       while (pkt) {
> >               rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
> >               if (!rcvd) {
> > @@ -685,13 +746,21 @@ static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *
> >                       const struct xdp_desc *desc = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
> >                       u64 addr = desc->addr, orig;
> >
> > +                     if (!pkt) {
> > +                             ksft_test_result_fail("ERROR: [%s] Received too many packets.\n",
> > +                                                   __func__);
> > +                             ksft_print_msg("Last packet has addr: %llx len: %u\n",
> > +                                            addr, desc->len);
> > +                             return;
> > +                     }
> > +
> >                       orig = xsk_umem__extract_addr(addr);
> >                       addr = xsk_umem__add_offset_to_addr(addr);
> >                       if (!is_pkt_valid(pkt, xsk->umem->buffer, addr, desc->len))
> >                               return;
> >
> >                       *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
> > -                     pkt = pkt_stream_get_pkt(pkt_stream, pkt_count++);
> > +                     pkt = pkt_stream_get_next_rx_pkt(pkt_stream);
> >               }
> >
> >               xsk_ring_prod__submit(&xsk->umem->fq, rcvd);
> > @@ -875,6 +944,7 @@ static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
> >  {
> >       print_verbose("Destroying socket\n");
> >       xsk_socket__delete(ifobj->xsk->xsk);
> > +     munmap(ifobj->umem->buffer, ifobj->umem->num_frames * ifobj->umem->frame_size);
> >       xsk_umem__delete(ifobj->umem->umem);
> >  }
> >
> > @@ -1118,6 +1188,33 @@ static bool testapp_unaligned(struct test_spec *test)
> >       return true;
> >  }
> >
> > +static void testapp_inv_desc(struct test_spec *test)
>
> I'd say that speaking out whole 'invalid' word wouldn't hurt us here.
>
> > +{
> > +     struct pkt pkts[] = {{0, 0, 0, true}, /* Zero packet length and zero address allowed */
>
> Above looks a bit weird, maybe this should be formatted same as the lines
> below:
>
>         struct pkt pkts[] = {
>                 /* Zero packet length and zero address allowed */
>                              {0, 0, 0, true},
>
> > +             /* Zero packet length allowed */
> > +                          {0x1000, 0, 0, true},
> > +             /* Straddling the start of umem */
> > +                          {-2, PKT_SIZE, 0, false},
> > +             /* Packet too large */
> > +                          {0x2000, XSK_UMEM__INVALID_FRAME_SIZE, 0, false},
> > +             /* After umem ends */
> > +                          {UMEM_SIZE, PKT_SIZE, 0, false},
> > +             /* Straddle the end of umem */
> > +                          {UMEM_SIZE - PKT_SIZE / 2, PKT_SIZE, 0, false},
> > +             /* Straddle a page boundrary */
> > +                          {0x3000 - PKT_SIZE / 2, PKT_SIZE, 0, false},
> > +             /* Valid packet for synch so that something is received */
> > +                          {0x4000, PKT_SIZE, 0, true}};
> > +
> > +     if (test->ifobj_tx->umem->unaligned_mode) {
> > +             /* Crossing a page boundrary allowed */
> > +             pkts[6].valid = true;
> > +     }
> > +     pkt_stream_set(test, pkts, ARRAY_SIZE(pkts));
> > +     testapp_validate_traffic(test);
> > +     pkt_stream_restore_default(test);
> > +}
> > +
> >  static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
> >                      const char *dst_ip, const char *src_ip, const u16 dst_port,
> >                      const u16 src_port, thread_func_t func_ptr)
> > @@ -1159,7 +1256,7 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
> >       case TEST_TYPE_BPF_RES:
> >               testapp_bpf_res(test);
> >               break;
> > -     case TEST_TYPE_NOPOLL:
> > +     case TEST_TYPE_RUN_TO_COMPLETION:
>
> I think that you were updating some comment around it on previous patches
> and it probably belongs to this one.

Will fix plus all the other suggestions above.

Thank you Maciej!

/Magnus

> >               test_spec_set_name(test, "RUN_TO_COMPLETION");
> >               testapp_validate_traffic(test);
> >               break;
> > @@ -1169,6 +1266,16 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
> >               test_spec_set_name(test, "POLL");
> >               testapp_validate_traffic(test);
> >               break;
> > +     case TEST_TYPE_ALIGNED_INV_DESC:
> > +             test_spec_set_name(test, "ALIGNED_INV_DESC");
> > +             testapp_inv_desc(test);
> > +             break;
> > +     case TEST_TYPE_UNALIGNED_INV_DESC:
> > +             test_spec_set_name(test, "UNALIGNED_INV_DESC");
> > +             test->ifobj_tx->umem->unaligned_mode = true;
> > +             test->ifobj_rx->umem->unaligned_mode = true;
> > +             testapp_inv_desc(test);
> > +             break;
> >       case TEST_TYPE_UNALIGNED:
> >               if (!testapp_unaligned(test))
> >                       return;
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> > index 129801eb013c..2d9efb89ea28 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.h
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> > @@ -38,6 +38,8 @@
> >  #define BATCH_SIZE 8
> >  #define POLL_TMOUT 1000
> >  #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)
> >  #define RX_FULL_RXQSIZE 32
> >  #define DEFAULT_OFFSET 256
> >  #define XSK_UMEM__INVALID_FRAME_SIZE (XSK_UMEM__DEFAULT_FRAME_SIZE + 1)
> > @@ -51,9 +53,11 @@ enum test_mode {
> >  };
> >
> >  enum test_type {
> > -     TEST_TYPE_NOPOLL,
> > +     TEST_TYPE_RUN_TO_COMPLETION,
> >       TEST_TYPE_POLL,
> >       TEST_TYPE_UNALIGNED,
> > +     TEST_TYPE_ALIGNED_INV_DESC,
> > +     TEST_TYPE_UNALIGNED_INV_DESC,
> >       TEST_TYPE_TEARDOWN,
> >       TEST_TYPE_BIDI,
> >       TEST_TYPE_STATS,
> > @@ -104,6 +108,7 @@ struct pkt {
> >
> >  struct pkt_stream {
> >       u32 nb_pkts;
> > +     u32 rx_pkt_nb;
> >       struct pkt *pkts;
> >       bool use_addr_for_fill;
> >  };
> > --
> > 2.29.0
> >

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

end of thread, other threads:[~2021-09-06  8:06 UTC | newest]

Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-01 10:47 [PATCH bpf-next 00/20] selftests: xsk: facilitate adding tests Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 01/20] selftests: xsk: simplify xsk and umem arrays Magnus Karlsson
2021-09-03 17:53   ` John Fastabend
2021-09-01 10:47 ` [PATCH bpf-next 02/20] selftests: xsk: introduce type for thread function Magnus Karlsson
2021-09-03 17:56   ` John Fastabend
2021-09-01 10:47 ` [PATCH bpf-next 03/20] selftests: xsk: introduce test specifications Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 04/20] selftests: xsk: move num_frames and frame_headroom to xsk_umem_info Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 05/20] selftests: xsk: move rxqsize into xsk_socket_info Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 06/20] selftests: xsk: make frame_size configurable Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 07/20] selftests: xsx: introduce test name in test spec Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 08/20] selftests: xsk: add use_poll to ifobject Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 09/20] selftests: xsk: introduce rx_on and tx_on in ifobject Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 10/20] selftests: xsk: replace second_step global variable Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 11/20] selftests: xsk: specify number of sockets to create Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 12/20] selftests: xsk: make xdp_flags and bind_flags local Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 13/20] selftests: xsx: make pthreads local scope Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 14/20] selftests: xsk: eliminate MAX_SOCKS define Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 15/20] selftests: xsk: allow for invalid packets Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 16/20] selftests: xsk: introduce replacing the default packet stream Magnus Karlsson
2021-09-03 15:04   ` Maciej Fijalkowski
2021-09-06  8:00     ` Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 17/20] selftests: xsk: add test for unaligned mode Magnus Karlsson
2021-09-03 15:38   ` Maciej Fijalkowski
2021-09-06  8:02     ` Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 18/20] selftests: xsk: eliminate test specific if-statement in test runner Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 19/20] selftests: xsk: add tests for invalid xsk descriptors Magnus Karlsson
2021-09-03 18:58   ` Maciej Fijalkowski
2021-09-06  8:06     ` Magnus Karlsson
2021-09-01 10:47 ` [PATCH bpf-next 20/20] selftests: xsk: add tests for 2K frame size Magnus Karlsson

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.