* [PATCH bpf-next v2 01/16] selftests: xsk: remove color mode
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 02/16] selftests: xsk: remove the num_tx_packets option Magnus Karlsson
` (15 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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 color mode since it does not add any value and having less code
means less maintenance which is a good thing.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/test_xsk.sh | 10 +++-----
tools/testing/selftests/bpf/xsk_prereqs.sh | 27 +++++-----------------
2 files changed, 9 insertions(+), 28 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index 46633a3bfb0b..cd7bf32e6a17 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -63,14 +63,11 @@
# ----------------
# Must run with CAP_NET_ADMIN capability.
#
-# Run (full color-coded output):
-# sudo ./test_xsk.sh -c
+# Run:
+# sudo ./test_xsk.sh
#
# If running from kselftests:
-# sudo make colorconsole=1 run_tests
-#
-# Run (full output without color-coding):
-# sudo ./test_xsk.sh
+# sudo make run_tests
#
# Run with verbose output:
# sudo ./test_xsk.sh -v
@@ -83,7 +80,6 @@
while getopts "cvD" flag
do
case "${flag}" in
- c) colorconsole=1;;
v) verbose=1;;
D) dump_pkts=1;;
esac
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index dac1c5f78752..8fe022a4dbfa 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -8,11 +8,6 @@ ksft_xfail=2
ksft_xpass=3
ksft_skip=4
-GREEN='\033[0;92m'
-YELLOW='\033[0;93m'
-RED='\033[0;31m'
-NC='\033[0m'
-STACK_LIM=131072
SPECFILE=veth.spec
XSKOBJ=xdpxceiver
NUMPKTS=10000
@@ -50,22 +45,12 @@ validate_veth_spec_file()
test_status()
{
statusval=$1
- if [ -n "${colorconsole+set}" ]; then
- if [ $statusval -eq 2 ]; then
- echo -e "${YELLOW}$2${NC}: [ ${RED}FAIL${NC} ]"
- elif [ $statusval -eq 1 ]; then
- echo -e "${YELLOW}$2${NC}: [ ${RED}SKIPPED${NC} ]"
- elif [ $statusval -eq 0 ]; then
- echo -e "${YELLOW}$2${NC}: [ ${GREEN}PASS${NC} ]"
- fi
- else
- if [ $statusval -eq 2 ]; then
- echo -e "$2: [ FAIL ]"
- elif [ $statusval -eq 1 ]; then
- echo -e "$2: [ SKIPPED ]"
- elif [ $statusval -eq 0 ]; then
- echo -e "$2: [ PASS ]"
- fi
+ if [ $statusval -eq 2 ]; then
+ echo -e "$2: [ FAIL ]"
+ elif [ $statusval -eq 1 ]; then
+ echo -e "$2: [ SKIPPED ]"
+ elif [ $statusval -eq 0 ]; then
+ echo -e "$2: [ PASS ]"
fi
}
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 02/16] selftests: xsk: remove the num_tx_packets option
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 01/16] selftests: xsk: remove color mode Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 03/16] selftests: xsk: remove unused variables Magnus Karlsson
` (14 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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 number of tx packet option as this should be decided by the
test itself. Also change the number of packets to be sent to 4096
speeding up the execution.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 33 +++++++---------------
tools/testing/selftests/bpf/xdpxceiver.h | 4 +--
tools/testing/selftests/bpf/xsk_prereqs.sh | 3 +-
3 files changed, 13 insertions(+), 27 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 1135fb980814..1b0efe566278 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -333,20 +333,19 @@ static struct option long_options[] = {
{"queue", optional_argument, 0, 'q'},
{"dump-pkts", optional_argument, 0, 'D'},
{"verbose", no_argument, 0, 'v'},
- {"tx-pkt-count", optional_argument, 0, 'C'},
{0, 0, 0, 0}
};
static void usage(const char *prog)
{
const char *str =
- " Usage: %s [OPTIONS]\n"
- " Options:\n"
- " -i, --interface Use interface\n"
- " -q, --queue=n Use queue n (default 0)\n"
- " -D, --dump-pkts Dump packets L2 - L5\n"
- " -v, --verbose Verbose output\n"
- " -C, --tx-pkt-count=n Number of packets to send\n";
+ " Usage: %s [OPTIONS]\n"
+ " Options:\n"
+ " -i, --interface Use interface\n"
+ " -q, --queue=n Use queue n (default 0)\n"
+ " -D, --dump-pkts Dump packets L2 - L5\n"
+ " -v, --verbose Verbose output\n";
+
ksft_print_msg(str, prog);
}
@@ -392,7 +391,7 @@ static void parse_command_line(int argc, char **argv)
opterr = 0;
for (;;) {
- c = getopt_long(argc, argv, "i:DC:v", long_options, &option_index);
+ c = getopt_long(argc, argv, "i:Dv", long_options, &option_index);
if (c == -1)
break;
@@ -415,9 +414,6 @@ static void parse_command_line(int argc, char **argv)
case 'D':
debug_pkt_dump = 1;
break;
- case 'C':
- opt_pkt_count = atoi(optarg);
- break;
case 'v':
opt_verbose = 1;
break;
@@ -427,11 +423,6 @@ static void parse_command_line(int argc, char **argv)
}
}
- if (!opt_pkt_count) {
- print_verbose("No tx-pkt-count specified, using default %u\n", DEFAULT_PKT_CNT);
- opt_pkt_count = DEFAULT_PKT_CNT;
- }
-
if (!validate_interfaces()) {
usage(basename(argv[0]));
ksft_exit_xfail();
@@ -554,9 +545,6 @@ static void tx_only(struct xsk_socket_info *xsk, u32 *frameptr, int batch_size)
static int get_batch_size(int pkt_cnt)
{
- if (!opt_pkt_count)
- return BATCH_SIZE;
-
if (pkt_cnt + BATCH_SIZE <= opt_pkt_count)
return BATCH_SIZE;
@@ -586,7 +574,7 @@ static void tx_only_all(struct ifobject *ifobject)
fds[0].fd = xsk_socket__fd(ifobject->xsk->xsk);
fds[0].events = POLLOUT;
- while ((opt_pkt_count && pkt_cnt < opt_pkt_count) || !opt_pkt_count) {
+ while (pkt_cnt < opt_pkt_count) {
int batch_size = get_batch_size(pkt_cnt);
if (test_type == TEST_TYPE_POLL) {
@@ -602,8 +590,7 @@ static void tx_only_all(struct ifobject *ifobject)
pkt_cnt += batch_size;
}
- if (opt_pkt_count)
- complete_tx_only_all(ifobject);
+ complete_tx_only_all(ifobject);
}
static void worker_pkt_dump(void)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 6c428b276ab6..4ce5a18b32e7 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -39,7 +39,7 @@
#define SOCK_RECONF_CTR 10
#define BATCH_SIZE 64
#define POLL_TMOUT 1000
-#define DEFAULT_PKT_CNT 10000
+#define DEFAULT_PKT_CNT (4 * 1024)
#define RX_FULL_RXQSIZE 32
#define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); } while (0)
@@ -79,7 +79,7 @@ static u32 num_frames;
static bool second_step;
static int test_type;
-static int opt_pkt_count;
+static u32 opt_pkt_count = DEFAULT_PKT_CNT;
static u8 opt_verbose;
static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index 8fe022a4dbfa..bf29d2549bee 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -10,7 +10,6 @@ ksft_skip=4
SPECFILE=veth.spec
XSKOBJ=xdpxceiver
-NUMPKTS=10000
validate_root_exec()
{
@@ -92,5 +91,5 @@ validate_ip_utility()
execxdpxceiver()
{
- ./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} -C ${NUMPKTS} ${VERBOSE_ARG} ${DUMP_PKTS_ARG}
+ ./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${VERBOSE_ARG} ${DUMP_PKTS_ARG}
}
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 03/16] selftests: xsk: remove unused variables
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 01/16] selftests: xsk: remove color mode Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 02/16] selftests: xsk: remove the num_tx_packets option Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 04/16] selftests: xsk: return correct error codes Magnus Karlsson
` (13 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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 unused variables and typedefs. The *_npkts variables are
incremented but never used.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 3 ---
tools/testing/selftests/bpf/xdpxceiver.h | 8 --------
2 files changed, 11 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 1b0efe566278..4d8ee636fc24 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -70,7 +70,6 @@
#include <errno.h>
#include <getopt.h>
#include <asm/barrier.h>
-typedef __u16 __sum16;
#include <linux/if_link.h>
#include <linux/if_ether.h>
#include <linux/ip.h>
@@ -454,7 +453,6 @@ static void complete_tx_only(struct xsk_socket_info *xsk, int batch_size)
if (rcvd) {
xsk_ring_cons__release(&xsk->umem->cq, rcvd);
xsk->outstanding_tx -= rcvd;
- xsk->tx_npkts += rcvd;
}
}
@@ -512,7 +510,6 @@ static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
xsk_ring_prod__submit(&xsk->umem->fq, rcvd);
xsk_ring_cons__release(&xsk->rx, rcvd);
- xsk->rx_npkts += rcvd;
}
static void tx_only(struct xsk_socket_info *xsk, u32 *frameptr, int batch_size)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 4ce5a18b32e7..02b7d0d6f45d 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -44,10 +44,6 @@
#define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); } while (0)
-typedef __u32 u32;
-typedef __u16 u16;
-typedef __u8 u8;
-
enum TEST_MODES {
TEST_MODE_UNCONFIGURED = -1,
TEST_MODE_SKB,
@@ -104,10 +100,6 @@ struct xsk_socket_info {
struct xsk_ring_prod tx;
struct xsk_umem_info *umem;
struct xsk_socket *xsk;
- unsigned long rx_npkts;
- unsigned long tx_npkts;
- unsigned long prev_rx_npkts;
- unsigned long prev_tx_npkts;
u32 outstanding_tx;
};
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 04/16] selftests: xsk: return correct error codes
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (2 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 03/16] selftests: xsk: remove unused variables Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 05/16] selftests: xsk: simplify the retry code Magnus Karlsson
` (12 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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>
Return the correct error codes so they can be printed correctly.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 4d8ee636fc24..f221bc5dae17 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -270,7 +270,7 @@ static void xsk_configure_umem(struct ifobject *data, void *buffer, int idx)
ret = xsk_umem__create(&umem->umem, buffer, size,
&umem->fq, &umem->cq, &cfg);
if (ret)
- exit_with_error(ret);
+ exit_with_error(-ret);
umem->buffer = buffer;
@@ -284,7 +284,7 @@ static void xsk_populate_fill_ring(struct xsk_umem_info *umem)
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);
+ 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__submit(&umem->fq, XSK_RING_PROD__DEFAULT_NUM_DESCS);
@@ -467,7 +467,7 @@ static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
ret = poll(fds, 1, POLL_TMOUT);
if (ret < 0)
- exit_with_error(ret);
+ exit_with_error(-ret);
}
return;
}
@@ -475,11 +475,11 @@ static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
while (ret != rcvd) {
if (ret < 0)
- exit_with_error(ret);
+ exit_with_error(-ret);
if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
ret = poll(fds, 1, POLL_TMOUT);
if (ret < 0)
- exit_with_error(ret);
+ exit_with_error(-ret);
}
ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
}
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 05/16] selftests: xsk: simplify the retry code
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (3 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 04/16] selftests: xsk: return correct error codes Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 06/16] selftests: xsk: remove end-of-test packet Magnus Karlsson
` (11 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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 retry code and make it more efficient by waiting first,
instead of trying immediately which always fails due to the
asynchronous nature of xsk socket close. Also decrease the wait time
to significantly lower the run-time of the test suite.
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, 11 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index f221bc5dae17..b7d193a96083 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -745,24 +745,19 @@ static void thread_common_ops(struct ifobject *ifobject, void *bufs)
if (bufs == MAP_FAILED)
exit_with_error(errno);
- xsk_configure_umem(ifobject, bufs, 0);
- ifobject->umem = ifobject->umem_arr[0];
- ret = xsk_configure_socket(ifobject, 0);
-
- /* Retry Create Socket if it fails as xsk_socket__create()
- * is asynchronous
- */
- while (ret && ctr < SOCK_RECONF_CTR) {
+ while (ctr++ < SOCK_RECONF_CTR) {
xsk_configure_umem(ifobject, bufs, 0);
ifobject->umem = ifobject->umem_arr[0];
ret = xsk_configure_socket(ifobject, 0);
+ if (!ret)
+ break;
+
+ /* Retry Create Socket if it fails as xsk_socket__create() is asynchronous */
usleep(USLEEP_MAX);
- ctr++;
+ if (ctr >= SOCK_RECONF_CTR)
+ exit_with_error(-ret);
}
- if (ctr >= SOCK_RECONF_CTR)
- exit_with_error(ret);
-
ifobject->umem = ifobject->umem_arr[0];
ifobject->xsk = ifobject->xsk_arr[0];
@@ -1125,8 +1120,10 @@ int main(int argc, char **argv)
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++)
+ for (j = 0; j < TEST_TYPE_MAX; j++) {
run_pkt_test(i, j);
+ usleep(USLEEP_MAX);
+ }
}
cleanup:
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 02b7d0d6f45d..1c94230c351a 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -35,7 +35,7 @@
#define UDP_PKT_SIZE (IP_PKT_SIZE - sizeof(struct iphdr))
#define UDP_PKT_DATA_SIZE (UDP_PKT_SIZE - sizeof(struct udphdr))
#define EOT (-1)
-#define USLEEP_MAX 200000
+#define USLEEP_MAX 10000
#define SOCK_RECONF_CTR 10
#define BATCH_SIZE 64
#define POLL_TMOUT 1000
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 06/16] selftests: xsk: remove end-of-test packet
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (4 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 05/16] selftests: xsk: simplify the retry code Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 07/16] selftests: xsk: disassociate umem size with packets sent Magnus Karlsson
` (10 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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>
Get rid of the end-of-test packet and just count the number of packets
received and quit when the expected number as been
received. Simplifies the code.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 42 +++++++-----------------
tools/testing/selftests/bpf/xdpxceiver.h | 2 --
2 files changed, 12 insertions(+), 32 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index b7d193a96083..b0fee71355bf 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -600,7 +600,7 @@ static void worker_pkt_dump(void)
void *ptr;
fprintf(stdout, "---------------------------------------\n");
- for (int iter = 0; iter < num_frames - 1; iter++) {
+ for (int iter = 0; iter < num_frames; iter++) {
ptr = pkt_buf[iter]->payload;
ethhdr = ptr;
iphdr = ptr + sizeof(*ethhdr);
@@ -627,11 +627,6 @@ static void worker_pkt_dump(void)
/*extract L5 frame */
payload = *((uint32_t *)(ptr + PKT_HDR_SIZE));
- if (payload == EOT) {
- print_verbose("End-of-transmission frame received\n");
- fprintf(stdout, "---------------------------------------\n");
- break;
- }
fprintf(stdout, "DEBUG>> L5: payload: %d\n", payload);
fprintf(stdout, "---------------------------------------\n");
}
@@ -694,28 +689,24 @@ static void worker_pkt_validate(void)
/*do not increment pktcounter if !(tos=0x9 and ipv4) */
if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
payloadseqnum = *((uint32_t *)(pkt_node_rx_q->pkt_frame + PKT_HDR_SIZE));
- if (debug_pkt_dump && payloadseqnum != EOT) {
+ if (debug_pkt_dump) {
pkt_obj = malloc(sizeof(*pkt_obj));
pkt_obj->payload = malloc(PKT_SIZE);
memcpy(pkt_obj->payload, pkt_node_rx_q->pkt_frame, PKT_SIZE);
pkt_buf[payloadseqnum] = pkt_obj;
}
- if (payloadseqnum == EOT) {
- print_verbose("End-of-transmission frame received: PASS\n");
- sigvar = 1;
- break;
- }
-
- if (prev_pkt + 1 != payloadseqnum) {
+ if (pkt_counter % num_frames != payloadseqnum) {
ksft_test_result_fail
- ("ERROR: [%s] prev_pkt [%d], payloadseqnum [%d]\n",
- __func__, prev_pkt, payloadseqnum);
+ ("ERROR: [%s] expected counter [%d], payloadseqnum [%d]\n",
+ __func__, pkt_counter, payloadseqnum);
ksft_exit_xfail();
}
- prev_pkt = payloadseqnum;
- pkt_counter++;
+ if (++pkt_counter == opt_pkt_count) {
+ sigvar = 1;
+ break;
+ }
} else {
ksft_print_msg("Invalid frame received: ");
ksft_print_msg("[IP_PKT_VER: %02X], [IP_PKT_TOS: %02X]\n", iphdr->version,
@@ -800,11 +791,7 @@ static void *worker_testapp_validate_tx(void *arg)
thread_common_ops(ifobject, bufs);
for (int i = 0; i < num_frames; i++) {
- /*send EOT frame */
- if (i == (num_frames - 1))
- data.seqnum = -1;
- else
- data.seqnum = i;
+ data.seqnum = i;
gen_udp_hdr(&data, ifobject, udp_hdr);
gen_ip_hdr(ifobject, ip_hdr);
gen_udp_csum(udp_hdr, ip_hdr);
@@ -812,8 +799,7 @@ static void *worker_testapp_validate_tx(void *arg)
gen_eth_frame(ifobject->umem, i * XSK_UMEM__DEFAULT_FRAME_SIZE);
}
- print_verbose("Sending %d packets on interface %s\n",
- (opt_pkt_count - 1), ifobject->ifname);
+ print_verbose("Sending %d packets on interface %s\n", opt_pkt_count, ifobject->ifname);
tx_only_all(ifobject);
testapp_cleanup_xsk_res(ifobject);
@@ -888,7 +874,7 @@ static void testapp_validate(void)
if (debug_pkt_dump && test_type != TEST_TYPE_STATS) {
worker_pkt_dump();
- for (int iter = 0; iter < num_frames - 1; iter++) {
+ for (int iter = 0; iter < num_frames; iter++) {
free(pkt_buf[iter]->payload);
free(pkt_buf[iter]);
}
@@ -905,7 +891,6 @@ static void testapp_teardown(void)
for (i = 0; i < MAX_TEARDOWN_ITER; i++) {
pkt_counter = 0;
- prev_pkt = -1;
sigvar = 0;
print_verbose("Creating socket\n");
testapp_validate();
@@ -933,7 +918,6 @@ static void testapp_bidi(void)
{
for (int i = 0; i < MAX_BIDI_ITER; i++) {
pkt_counter = 0;
- prev_pkt = -1;
sigvar = 0;
print_verbose("Creating socket\n");
testapp_validate();
@@ -967,7 +951,6 @@ static void testapp_bpf_res(void)
for (i = 0; i < MAX_BPF_ITER; i++) {
pkt_counter = 0;
- prev_pkt = -1;
sigvar = 0;
print_verbose("Creating socket\n");
testapp_validate();
@@ -1043,7 +1026,6 @@ static void run_pkt_test(int mode, int type)
xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
pkt_counter = 0;
second_step = 0;
- prev_pkt = -1;
sigvar = 0;
stat_test_type = -1;
rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 1c94230c351a..a4371d9e2798 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -34,7 +34,6 @@
#define IP_PKT_TOS 0x9
#define UDP_PKT_SIZE (IP_PKT_SIZE - sizeof(struct iphdr))
#define UDP_PKT_DATA_SIZE (UDP_PKT_SIZE - sizeof(struct udphdr))
-#define EOT (-1)
#define USLEEP_MAX 10000
#define SOCK_RECONF_CTR 10
#define BATCH_SIZE 64
@@ -82,7 +81,6 @@ static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
static u32 xdp_bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
static u8 pkt_data[XSK_UMEM__DEFAULT_FRAME_SIZE];
static u32 pkt_counter;
-static long prev_pkt = -1;
static int sigvar;
static int stat_test_type;
static u32 rxqsize;
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 07/16] selftests: xsk: disassociate umem size with packets sent
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (5 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 06/16] selftests: xsk: remove end-of-test packet Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 08/16] selftests: xsk: rename worker_* functions that are not thread entry points Magnus Karlsson
` (9 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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>
Disassociate the number of packets sent with the number of buffers in
the umem. This so we can loop over the umem to test more things. Set
the size of the umem to be a multiple of 2M. A requirement for huge
pages that are needed in unaligned mode.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 18 ++++++++----------
tools/testing/selftests/bpf/xdpxceiver.h | 2 +-
2 files changed, 9 insertions(+), 11 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index b0fee71355bf..ebed88c13509 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -250,7 +250,7 @@ static void gen_eth_frame(struct xsk_umem_info *umem, u64 addr)
memcpy(xsk_umem__get_data(umem->buffer, addr), pkt_data, PKT_SIZE);
}
-static void xsk_configure_umem(struct ifobject *data, void *buffer, int idx)
+static void xsk_configure_umem(struct ifobject *data, void *buffer, u64 size, int idx)
{
struct xsk_umem_config cfg = {
.fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
@@ -259,7 +259,6 @@ static void xsk_configure_umem(struct ifobject *data, void *buffer, int idx)
.frame_headroom = frame_headroom,
.flags = XSK_UMEM__DEFAULT_FLAGS
};
- int size = num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE;
struct xsk_umem_info *umem;
int ret;
@@ -722,22 +721,23 @@ static void worker_pkt_validate(void)
static void thread_common_ops(struct ifobject *ifobject, void *bufs)
{
- int umem_sz = num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE;
+ 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;
ifobject->ns_fd = switch_namespace(ifobject->nsname);
if (test_type == TEST_TYPE_BPF_RES)
- umem_sz *= 2;
+ mmap_sz *= 2;
- bufs = mmap(NULL, umem_sz,
- PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+ bufs = mmap(NULL, mmap_sz, PROT_READ | PROT_WRITE, mmap_flags, -1, 0);
if (bufs == MAP_FAILED)
exit_with_error(errno);
while (ctr++ < SOCK_RECONF_CTR) {
- xsk_configure_umem(ifobject, bufs, 0);
+ xsk_configure_umem(ifobject, bufs, umem_sz, 0);
ifobject->umem = ifobject->umem_arr[0];
ret = xsk_configure_socket(ifobject, 0);
if (!ret)
@@ -753,7 +753,7 @@ static void thread_common_ops(struct ifobject *ifobject, void *bufs)
ifobject->xsk = ifobject->xsk_arr[0];
if (test_type == TEST_TYPE_BPF_RES) {
- xsk_configure_umem(ifobject, (u8 *)bufs + (umem_sz / 2), 1);
+ xsk_configure_umem(ifobject, (u8 *)bufs + umem_sz, umem_sz, 1);
ifobject->umem = ifobject->umem_arr[1];
ret = xsk_configure_socket(ifobject, 1);
}
@@ -1094,8 +1094,6 @@ int main(int argc, char **argv)
parse_command_line(argc, argv);
- num_frames = ++opt_pkt_count;
-
init_iface(ifdict[0], MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2, tx);
init_iface(ifdict[1], MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1, rx);
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index a4371d9e2798..131bd998e374 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -70,7 +70,7 @@ enum STAT_TEST_TYPES {
static int configured_mode = TEST_MODE_UNCONFIGURED;
static u8 debug_pkt_dump;
-static u32 num_frames;
+static u32 num_frames = DEFAULT_PKT_CNT / 4;
static bool second_step;
static int test_type;
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 08/16] selftests: xsk: rename worker_* functions that are not thread entry points
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (6 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 07/16] selftests: xsk: disassociate umem size with packets sent Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 09/16] selftests: xsk: simplify packet validation in xsk tests Magnus Karlsson
` (8 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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>
Rename worker_* functions that are not thread entry points to
something else. This was confusing. Now only thread entry points are
worker_something.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index ebed88c13509..17956fdeb49e 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -589,7 +589,7 @@ static void tx_only_all(struct ifobject *ifobject)
complete_tx_only_all(ifobject);
}
-static void worker_pkt_dump(void)
+static void pkt_dump(void)
{
struct ethhdr *ethhdr;
struct iphdr *iphdr;
@@ -631,7 +631,7 @@ static void worker_pkt_dump(void)
}
}
-static void worker_stats_validate(struct ifobject *ifobject)
+static void stats_validate(struct ifobject *ifobject)
{
struct xdp_statistics stats;
socklen_t optlen;
@@ -673,7 +673,7 @@ static void worker_stats_validate(struct ifobject *ifobject)
}
}
-static void worker_pkt_validate(void)
+static void pkt_validate(void)
{
u32 payloadseqnum = -2;
struct iphdr *iphdr;
@@ -833,9 +833,9 @@ static void *worker_testapp_validate_rx(void *arg)
while (1) {
if (test_type != TEST_TYPE_STATS) {
rx_pkt(ifobject->xsk, fds);
- worker_pkt_validate();
+ pkt_validate();
} else {
- worker_stats_validate(ifobject);
+ stats_validate(ifobject);
}
if (sigvar)
break;
@@ -873,7 +873,7 @@ static void testapp_validate(void)
pthread_join(t0, NULL);
if (debug_pkt_dump && test_type != TEST_TYPE_STATS) {
- worker_pkt_dump();
+ pkt_dump();
for (int iter = 0; iter < num_frames; iter++) {
free(pkt_buf[iter]->payload);
free(pkt_buf[iter]);
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 09/16] selftests: xsk: simplify packet validation in xsk tests
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (7 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 08/16] selftests: xsk: rename worker_* functions that are not thread entry points Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 10/16] selftests: xsk: validate tx stats on tx thread Magnus Karlsson
` (7 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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 packet validation in the xsk selftests by performing it at
once for every packet. The current code performed this per batch and
did this on copied packet data. Make it simpler and faster by
validating it at once and on the umem packet data thus skipping the
copy and the memory allocation for the temprary buffer.
The optional packet dump feature is also simplified in the same
manner. Memory allocation and copying is removed and the dump is
performed directly on the umem data.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 182 ++++++++---------------
tools/testing/selftests/bpf/xdpxceiver.h | 14 --
2 files changed, 65 insertions(+), 131 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 17956fdeb49e..fe3d281a0575 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -427,6 +427,70 @@ static void parse_command_line(int argc, char **argv)
}
}
+static void pkt_dump(void *pkt, u32 len)
+{
+ char s[INET_ADDRSTRLEN];
+ struct ethhdr *ethhdr;
+ struct udphdr *udphdr;
+ struct iphdr *iphdr;
+ int payload, i;
+
+ ethhdr = pkt;
+ iphdr = pkt + sizeof(*ethhdr);
+ udphdr = pkt + sizeof(*ethhdr) + sizeof(*iphdr);
+
+ /*extract L2 frame */
+ fprintf(stdout, "DEBUG>> L2: dst mac: ");
+ for (i = 0; i < ETH_ALEN; i++)
+ fprintf(stdout, "%02X", ethhdr->h_dest[i]);
+
+ fprintf(stdout, "\nDEBUG>> L2: src mac: ");
+ for (i = 0; i < ETH_ALEN; i++)
+ fprintf(stdout, "%02X", ethhdr->h_source[i]);
+
+ /*extract L3 frame */
+ fprintf(stdout, "\nDEBUG>> L3: ip_hdr->ihl: %02X\n", iphdr->ihl);
+ fprintf(stdout, "DEBUG>> L3: ip_hdr->saddr: %s\n",
+ inet_ntop(AF_INET, &iphdr->saddr, s, sizeof(s)));
+ fprintf(stdout, "DEBUG>> L3: ip_hdr->daddr: %s\n",
+ inet_ntop(AF_INET, &iphdr->daddr, s, sizeof(s)));
+ /*extract L4 frame */
+ fprintf(stdout, "DEBUG>> L4: udp_hdr->src: %d\n", ntohs(udphdr->source));
+ fprintf(stdout, "DEBUG>> L4: udp_hdr->dst: %d\n", ntohs(udphdr->dest));
+ /*extract L5 frame */
+ payload = *((uint32_t *)(pkt + PKT_HDR_SIZE));
+
+ fprintf(stdout, "DEBUG>> L5: payload: %d\n", payload);
+ fprintf(stdout, "---------------------------------------\n");
+}
+
+static void pkt_validate(void *pkt)
+{
+ struct iphdr *iphdr = (struct iphdr *)(pkt + sizeof(struct ethhdr));
+
+ /*do not increment pktcounter if !(tos=0x9 and ipv4) */
+ if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
+ u32 payloadseqnum = *((uint32_t *)(pkt + PKT_HDR_SIZE));
+
+ if (debug_pkt_dump && test_type != TEST_TYPE_STATS)
+ pkt_dump(pkt, PKT_SIZE);
+
+ if (pkt_counter % num_frames != payloadseqnum) {
+ ksft_test_result_fail
+ ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
+ __func__, pkt_counter, payloadseqnum);
+ ksft_exit_xfail();
+ }
+
+ if (++pkt_counter == opt_pkt_count)
+ sigvar = 1;
+ } else {
+ ksft_print_msg("Invalid frame received: ");
+ ksft_print_msg("[IP_PKT_VER: %02X], [IP_PKT_TOS: %02X]\n", iphdr->version,
+ iphdr->tos);
+ }
+}
+
static void kick_tx(struct xsk_socket_info *xsk)
{
int ret;
@@ -491,18 +555,7 @@ static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
orig = xsk_umem__extract_addr(addr);
addr = xsk_umem__add_offset_to_addr(addr);
- pkt_node_rx = malloc(sizeof(struct pkt) + PKT_SIZE);
- if (!pkt_node_rx)
- exit_with_error(errno);
-
- pkt_node_rx->pkt_frame = malloc(PKT_SIZE);
- if (!pkt_node_rx->pkt_frame)
- exit_with_error(errno);
-
- memcpy(pkt_node_rx->pkt_frame, xsk_umem__get_data(xsk->umem->buffer, addr),
- PKT_SIZE);
-
- TAILQ_INSERT_HEAD(&head, pkt_node_rx, pkt_nodes);
+ pkt_validate(xsk_umem__get_data(xsk->umem->buffer, addr));
*xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
}
@@ -589,48 +642,6 @@ static void tx_only_all(struct ifobject *ifobject)
complete_tx_only_all(ifobject);
}
-static void pkt_dump(void)
-{
- struct ethhdr *ethhdr;
- struct iphdr *iphdr;
- struct udphdr *udphdr;
- char s[128];
- int payload;
- void *ptr;
-
- fprintf(stdout, "---------------------------------------\n");
- for (int iter = 0; iter < num_frames; iter++) {
- ptr = pkt_buf[iter]->payload;
- ethhdr = ptr;
- iphdr = ptr + sizeof(*ethhdr);
- udphdr = ptr + sizeof(*ethhdr) + sizeof(*iphdr);
-
- /*extract L2 frame */
- fprintf(stdout, "DEBUG>> L2: dst mac: ");
- for (int i = 0; i < ETH_ALEN; i++)
- fprintf(stdout, "%02X", ethhdr->h_dest[i]);
-
- fprintf(stdout, "\nDEBUG>> L2: src mac: ");
- for (int i = 0; i < ETH_ALEN; i++)
- fprintf(stdout, "%02X", ethhdr->h_source[i]);
-
- /*extract L3 frame */
- fprintf(stdout, "\nDEBUG>> L3: ip_hdr->ihl: %02X\n", iphdr->ihl);
- fprintf(stdout, "DEBUG>> L3: ip_hdr->saddr: %s\n",
- inet_ntop(AF_INET, &iphdr->saddr, s, sizeof(s)));
- fprintf(stdout, "DEBUG>> L3: ip_hdr->daddr: %s\n",
- inet_ntop(AF_INET, &iphdr->daddr, s, sizeof(s)));
- /*extract L4 frame */
- fprintf(stdout, "DEBUG>> L4: udp_hdr->src: %d\n", ntohs(udphdr->source));
- fprintf(stdout, "DEBUG>> L4: udp_hdr->dst: %d\n", ntohs(udphdr->dest));
- /*extract L5 frame */
- payload = *((uint32_t *)(ptr + PKT_HDR_SIZE));
-
- fprintf(stdout, "DEBUG>> L5: payload: %d\n", payload);
- fprintf(stdout, "---------------------------------------\n");
- }
-}
-
static void stats_validate(struct ifobject *ifobject)
{
struct xdp_statistics stats;
@@ -673,52 +684,6 @@ static void stats_validate(struct ifobject *ifobject)
}
}
-static void pkt_validate(void)
-{
- u32 payloadseqnum = -2;
- struct iphdr *iphdr;
-
- while (1) {
- pkt_node_rx_q = TAILQ_LAST(&head, head_s);
- if (!pkt_node_rx_q)
- break;
-
- iphdr = (struct iphdr *)(pkt_node_rx_q->pkt_frame + sizeof(struct ethhdr));
-
- /*do not increment pktcounter if !(tos=0x9 and ipv4) */
- if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
- payloadseqnum = *((uint32_t *)(pkt_node_rx_q->pkt_frame + PKT_HDR_SIZE));
- if (debug_pkt_dump) {
- pkt_obj = malloc(sizeof(*pkt_obj));
- pkt_obj->payload = malloc(PKT_SIZE);
- memcpy(pkt_obj->payload, pkt_node_rx_q->pkt_frame, PKT_SIZE);
- pkt_buf[payloadseqnum] = pkt_obj;
- }
-
- if (pkt_counter % num_frames != payloadseqnum) {
- ksft_test_result_fail
- ("ERROR: [%s] expected counter [%d], payloadseqnum [%d]\n",
- __func__, pkt_counter, payloadseqnum);
- ksft_exit_xfail();
- }
-
- if (++pkt_counter == opt_pkt_count) {
- sigvar = 1;
- break;
- }
- } else {
- ksft_print_msg("Invalid frame received: ");
- ksft_print_msg("[IP_PKT_VER: %02X], [IP_PKT_TOS: %02X]\n", iphdr->version,
- iphdr->tos);
- }
-
- TAILQ_REMOVE(&head, pkt_node_rx_q, pkt_nodes);
- free(pkt_node_rx_q->pkt_frame);
- free(pkt_node_rx_q);
- pkt_node_rx_q = NULL;
- }
-}
-
static void thread_common_ops(struct ifobject *ifobject, void *bufs)
{
u64 umem_sz = num_frames * XSK_UMEM__DEFAULT_FRAME_SIZE;
@@ -818,13 +783,6 @@ static void *worker_testapp_validate_rx(void *arg)
if (stat_test_type != STAT_TEST_RX_FILL_EMPTY)
xsk_populate_fill_ring(ifobject->umem);
- TAILQ_INIT(&head);
- if (debug_pkt_dump) {
- pkt_buf = calloc(num_frames, sizeof(*pkt_buf));
- if (!pkt_buf)
- exit_with_error(errno);
- }
-
fds[0].fd = xsk_socket__fd(ifobject->xsk->xsk);
fds[0].events = POLLIN;
@@ -833,7 +791,6 @@ static void *worker_testapp_validate_rx(void *arg)
while (1) {
if (test_type != TEST_TYPE_STATS) {
rx_pkt(ifobject->xsk, fds);
- pkt_validate();
} else {
stats_validate(ifobject);
}
@@ -872,15 +829,6 @@ static void testapp_validate(void)
pthread_join(t1, NULL);
pthread_join(t0, NULL);
- if (debug_pkt_dump && test_type != TEST_TYPE_STATS) {
- pkt_dump();
- for (int iter = 0; iter < num_frames; iter++) {
- free(pkt_buf[iter]->payload);
- free(pkt_buf[iter]);
- }
- free(pkt_buf);
- }
-
if (!(test_type == TEST_TYPE_TEARDOWN) && !bidi && !bpf && !(test_type == TEST_TYPE_STATS))
print_ksft_result();
}
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 131bd998e374..0fb657b505ae 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -139,18 +139,4 @@ static struct ifobject *ifdict_tx;
pthread_barrier_t barr;
pthread_t t0, t1;
-TAILQ_HEAD(head_s, pkt) head = TAILQ_HEAD_INITIALIZER(head);
-struct head_s *head_p;
-struct pkt {
- char *pkt_frame;
-
- TAILQ_ENTRY(pkt) pkt_nodes;
-} *pkt_node_rx, *pkt_node_rx_q;
-
-struct pkt_frame {
- char *payload;
-} *pkt_obj;
-
-struct pkt_frame **pkt_buf;
-
#endif /* XDPXCEIVER_H */
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 10/16] selftests: xsk: validate tx stats on tx thread
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (8 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 09/16] selftests: xsk: simplify packet validation in xsk tests Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-19 9:26 ` Maciej Fijalkowski
2021-08-17 9:27 ` [PATCH bpf-next v2 11/16] selftests: xsk: decrease batch size Magnus Karlsson
` (6 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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>
Validate the tx stats on the Tx thread instead of the Rx
tread. Depending on your settings, you might not be allowed to query
the statistics of a socket you do not own, so better to do this on the
correct thread to start with.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 55 ++++++++++++++++++------
1 file changed, 41 insertions(+), 14 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index fe3d281a0575..8ff24472ef1e 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -642,23 +642,22 @@ static void tx_only_all(struct ifobject *ifobject)
complete_tx_only_all(ifobject);
}
-static void stats_validate(struct ifobject *ifobject)
+static bool rx_stats_are_valid(struct ifobject *ifobject)
{
+ u32 xsk_stat = 0, expected_stat = opt_pkt_count;
+ struct xsk_socket *xsk = ifobject->xsk->xsk;
+ int fd = xsk_socket__fd(xsk);
struct xdp_statistics stats;
socklen_t optlen;
int err;
- struct xsk_socket *xsk = stat_test_type == STAT_TEST_TX_INVALID ?
- ifdict[!ifobject->ifdict_index]->xsk->xsk :
- ifobject->xsk->xsk;
- int fd = xsk_socket__fd(xsk);
- unsigned long xsk_stat = 0, expected_stat = opt_pkt_count;
-
- sigvar = 0;
optlen = sizeof(stats);
err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, &stats, &optlen);
- if (err)
- return;
+ if (err) {
+ ksft_test_result_fail("ERROR: [%s] getsockopt(XDP_STATISTICS) error %u %s\n",
+ __func__, -err, strerror(-err));
+ return true;
+ }
if (optlen == sizeof(struct xdp_statistics)) {
switch (stat_test_type) {
@@ -666,8 +665,7 @@ static void stats_validate(struct ifobject *ifobject)
xsk_stat = stats.rx_dropped;
break;
case STAT_TEST_TX_INVALID:
- xsk_stat = stats.tx_invalid_descs;
- break;
+ return true;
case STAT_TEST_RX_FULL:
xsk_stat = stats.rx_ring_full;
expected_stat -= RX_FULL_RXQSIZE;
@@ -680,8 +678,33 @@ static void stats_validate(struct ifobject *ifobject)
}
if (xsk_stat == expected_stat)
- sigvar = 1;
+ return true;
+ }
+
+ return false;
+}
+
+static void tx_stats_validate(struct ifobject *ifobject)
+{
+ struct xsk_socket *xsk = ifobject->xsk->xsk;
+ int fd = xsk_socket__fd(xsk);
+ struct xdp_statistics stats;
+ socklen_t optlen;
+ int err;
+
+ 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",
+ __func__, -err, strerror(-err));
+ return;
}
+
+ if (stats.tx_invalid_descs == opt_pkt_count)
+ return;
+
+ ksft_test_result_fail("ERROR: [%s] tx_invalid_descs incorrect. Got [%u] expected [%u]\n",
+ __func__, stats.tx_invalid_descs, opt_pkt_count);
}
static void thread_common_ops(struct ifobject *ifobject, void *bufs)
@@ -767,6 +790,9 @@ static void *worker_testapp_validate_tx(void *arg)
print_verbose("Sending %d packets on interface %s\n", opt_pkt_count, ifobject->ifname);
tx_only_all(ifobject);
+ if (stat_test_type == STAT_TEST_TX_INVALID)
+ tx_stats_validate(ifobject);
+
testapp_cleanup_xsk_res(ifobject);
pthread_exit(NULL);
}
@@ -792,7 +818,8 @@ static void *worker_testapp_validate_rx(void *arg)
if (test_type != TEST_TYPE_STATS) {
rx_pkt(ifobject->xsk, fds);
} else {
- stats_validate(ifobject);
+ if (rx_stats_are_valid(ifobject))
+ break;
}
if (sigvar)
break;
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next v2 10/16] selftests: xsk: validate tx stats on tx thread
2021-08-17 9:27 ` [PATCH bpf-next v2 10/16] selftests: xsk: validate tx stats on tx thread Magnus Karlsson
@ 2021-08-19 9:26 ` Maciej Fijalkowski
2021-08-19 9:52 ` Magnus Karlsson
0 siblings, 1 reply; 24+ messages in thread
From: Maciej Fijalkowski @ 2021-08-19 9:26 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
ciara.loftus, bpf, yhs, andrii
On Tue, Aug 17, 2021 at 11:27:23AM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Validate the tx stats on the Tx thread instead of the Rx
> tread. Depending on your settings, you might not be allowed to query
> the statistics of a socket you do not own, so better to do this on the
> correct thread to start with.
>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
> tools/testing/selftests/bpf/xdpxceiver.c | 55 ++++++++++++++++++------
> 1 file changed, 41 insertions(+), 14 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index fe3d281a0575..8ff24472ef1e 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -642,23 +642,22 @@ static void tx_only_all(struct ifobject *ifobject)
> complete_tx_only_all(ifobject);
> }
>
> -static void stats_validate(struct ifobject *ifobject)
> +static bool rx_stats_are_valid(struct ifobject *ifobject)
> {
> + u32 xsk_stat = 0, expected_stat = opt_pkt_count;
> + struct xsk_socket *xsk = ifobject->xsk->xsk;
> + int fd = xsk_socket__fd(xsk);
> struct xdp_statistics stats;
> socklen_t optlen;
> int err;
> - struct xsk_socket *xsk = stat_test_type == STAT_TEST_TX_INVALID ?
> - ifdict[!ifobject->ifdict_index]->xsk->xsk :
> - ifobject->xsk->xsk;
> - int fd = xsk_socket__fd(xsk);
> - unsigned long xsk_stat = 0, expected_stat = opt_pkt_count;
> -
> - sigvar = 0;
>
> optlen = sizeof(stats);
> err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, &stats, &optlen);
> - if (err)
> - return;
> + if (err) {
> + ksft_test_result_fail("ERROR: [%s] getsockopt(XDP_STATISTICS) error %u %s\n",
> + __func__, -err, strerror(-err));
> + return true;
Can we invert the logic or change the name of the func?
Returning 'true' for error case is a bit confusing given the name of func
is blah_are_valid, no? If there was an error then I'd return false.
OTOH we're testing faulty socket situations in here, but error from
getsockopt does not mean that stats were valid.
> + }
>
> if (optlen == sizeof(struct xdp_statistics)) {
> switch (stat_test_type) {
> @@ -666,8 +665,7 @@ static void stats_validate(struct ifobject *ifobject)
> xsk_stat = stats.rx_dropped;
> break;
> case STAT_TEST_TX_INVALID:
> - xsk_stat = stats.tx_invalid_descs;
> - break;
> + return true;
> case STAT_TEST_RX_FULL:
> xsk_stat = stats.rx_ring_full;
> expected_stat -= RX_FULL_RXQSIZE;
> @@ -680,8 +678,33 @@ static void stats_validate(struct ifobject *ifobject)
> }
>
> if (xsk_stat == expected_stat)
> - sigvar = 1;
> + return true;
> + }
> +
> + return false;
> +}
> +
> +static void tx_stats_validate(struct ifobject *ifobject)
> +{
> + struct xsk_socket *xsk = ifobject->xsk->xsk;
> + int fd = xsk_socket__fd(xsk);
> + struct xdp_statistics stats;
> + socklen_t optlen;
> + int err;
> +
> + 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",
> + __func__, -err, strerror(-err));
> + return;
> }
> +
> + if (stats.tx_invalid_descs == opt_pkt_count)
> + return;
> +
> + ksft_test_result_fail("ERROR: [%s] tx_invalid_descs incorrect. Got [%u] expected [%u]\n",
> + __func__, stats.tx_invalid_descs, opt_pkt_count);
> }
>
> static void thread_common_ops(struct ifobject *ifobject, void *bufs)
> @@ -767,6 +790,9 @@ static void *worker_testapp_validate_tx(void *arg)
> print_verbose("Sending %d packets on interface %s\n", opt_pkt_count, ifobject->ifname);
> tx_only_all(ifobject);
>
> + if (stat_test_type == STAT_TEST_TX_INVALID)
> + tx_stats_validate(ifobject);
> +
> testapp_cleanup_xsk_res(ifobject);
> pthread_exit(NULL);
> }
> @@ -792,7 +818,8 @@ static void *worker_testapp_validate_rx(void *arg)
> if (test_type != TEST_TYPE_STATS) {
> rx_pkt(ifobject->xsk, fds);
> } else {
> - stats_validate(ifobject);
> + if (rx_stats_are_valid(ifobject))
> + break;
> }
> if (sigvar)
> break;
> --
> 2.29.0
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next v2 10/16] selftests: xsk: validate tx stats on tx thread
2021-08-19 9:26 ` Maciej Fijalkowski
@ 2021-08-19 9:52 ` Magnus Karlsson
0 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-19 9:52 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 Thu, Aug 19, 2021 at 11:41 AM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Tue, Aug 17, 2021 at 11:27:23AM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Validate the tx stats on the Tx thread instead of the Rx
> > tread. Depending on your settings, you might not be allowed to query
> > the statistics of a socket you do not own, so better to do this on the
> > correct thread to start with.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> > tools/testing/selftests/bpf/xdpxceiver.c | 55 ++++++++++++++++++------
> > 1 file changed, 41 insertions(+), 14 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> > index fe3d281a0575..8ff24472ef1e 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.c
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> > @@ -642,23 +642,22 @@ static void tx_only_all(struct ifobject *ifobject)
> > complete_tx_only_all(ifobject);
> > }
> >
> > -static void stats_validate(struct ifobject *ifobject)
> > +static bool rx_stats_are_valid(struct ifobject *ifobject)
> > {
> > + u32 xsk_stat = 0, expected_stat = opt_pkt_count;
> > + struct xsk_socket *xsk = ifobject->xsk->xsk;
> > + int fd = xsk_socket__fd(xsk);
> > struct xdp_statistics stats;
> > socklen_t optlen;
> > int err;
> > - struct xsk_socket *xsk = stat_test_type == STAT_TEST_TX_INVALID ?
> > - ifdict[!ifobject->ifdict_index]->xsk->xsk :
> > - ifobject->xsk->xsk;
> > - int fd = xsk_socket__fd(xsk);
> > - unsigned long xsk_stat = 0, expected_stat = opt_pkt_count;
> > -
> > - sigvar = 0;
> >
> > optlen = sizeof(stats);
> > err = getsockopt(fd, SOL_XDP, XDP_STATISTICS, &stats, &optlen);
> > - if (err)
> > - return;
> > + if (err) {
> > + ksft_test_result_fail("ERROR: [%s] getsockopt(XDP_STATISTICS) error %u %s\n",
> > + __func__, -err, strerror(-err));
> > + return true;
>
> Can we invert the logic or change the name of the func?
> Returning 'true' for error case is a bit confusing given the name of func
> is blah_are_valid, no? If there was an error then I'd return false.
>
> OTOH we're testing faulty socket situations in here, but error from
> getsockopt does not mean that stats were valid.
Yes, this is not that clear. We want the loop on the higher level to
quit, therefore we return true when there is an error. A problem with
the stats tests is that they do not terminate when the stats are
wrong, only when they pass. Once I get the second patch set accepted
(in some form), we should rewrite these stats tests in that new
framework so that they will terminate even when the stats are wrong.
This whole problem will likely disappear at that point. But I will
scratch my head and try to make it better in this patch.
> > + }
> >
> > if (optlen == sizeof(struct xdp_statistics)) {
> > switch (stat_test_type) {
> > @@ -666,8 +665,7 @@ static void stats_validate(struct ifobject *ifobject)
> > xsk_stat = stats.rx_dropped;
> > break;
> > case STAT_TEST_TX_INVALID:
> > - xsk_stat = stats.tx_invalid_descs;
> > - break;
> > + return true;
> > case STAT_TEST_RX_FULL:
> > xsk_stat = stats.rx_ring_full;
> > expected_stat -= RX_FULL_RXQSIZE;
> > @@ -680,8 +678,33 @@ static void stats_validate(struct ifobject *ifobject)
> > }
> >
> > if (xsk_stat == expected_stat)
> > - sigvar = 1;
> > + return true;
> > + }
> > +
> > + return false;
> > +}
> > +
> > +static void tx_stats_validate(struct ifobject *ifobject)
> > +{
> > + struct xsk_socket *xsk = ifobject->xsk->xsk;
> > + int fd = xsk_socket__fd(xsk);
> > + struct xdp_statistics stats;
> > + socklen_t optlen;
> > + int err;
> > +
> > + 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",
> > + __func__, -err, strerror(-err));
> > + return;
> > }
> > +
> > + if (stats.tx_invalid_descs == opt_pkt_count)
> > + return;
> > +
> > + ksft_test_result_fail("ERROR: [%s] tx_invalid_descs incorrect. Got [%u] expected [%u]\n",
> > + __func__, stats.tx_invalid_descs, opt_pkt_count);
> > }
> >
> > static void thread_common_ops(struct ifobject *ifobject, void *bufs)
> > @@ -767,6 +790,9 @@ static void *worker_testapp_validate_tx(void *arg)
> > print_verbose("Sending %d packets on interface %s\n", opt_pkt_count, ifobject->ifname);
> > tx_only_all(ifobject);
> >
> > + if (stat_test_type == STAT_TEST_TX_INVALID)
> > + tx_stats_validate(ifobject);
> > +
> > testapp_cleanup_xsk_res(ifobject);
> > pthread_exit(NULL);
> > }
> > @@ -792,7 +818,8 @@ static void *worker_testapp_validate_rx(void *arg)
> > if (test_type != TEST_TYPE_STATS) {
> > rx_pkt(ifobject->xsk, fds);
> > } else {
> > - stats_validate(ifobject);
> > + if (rx_stats_are_valid(ifobject))
> > + break;
> > }
> > if (sigvar)
> > break;
> > --
> > 2.29.0
> >
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf-next v2 11/16] selftests: xsk: decrease batch size
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (9 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 10/16] selftests: xsk: validate tx stats on tx thread Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 12/16] selftests: xsk: remove cleanup at end of program Magnus Karlsson
` (5 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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>
Decrease the batch size from 64 to 8 to avoid potentially overflowing
some buffers in the skb case that leads to dropped packets we cannot
control (and thus the tests may generate false negatives).
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 0fb657b505ae..1c5457e9f1d6 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -36,7 +36,7 @@
#define UDP_PKT_DATA_SIZE (UDP_PKT_SIZE - sizeof(struct udphdr))
#define USLEEP_MAX 10000
#define SOCK_RECONF_CTR 10
-#define BATCH_SIZE 64
+#define BATCH_SIZE 8
#define POLL_TMOUT 1000
#define DEFAULT_PKT_CNT (4 * 1024)
#define RX_FULL_RXQSIZE 32
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 12/16] selftests: xsk: remove cleanup at end of program
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (10 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 11/16] selftests: xsk: decrease batch size Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-19 9:28 ` Maciej Fijalkowski
2021-08-17 9:27 ` [PATCH bpf-next v2 13/16] selftests: xsk: generate packet directly in umem Magnus Karlsson
` (4 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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 cleanup right before the program/process exits as this will
trigger the cleanup without us having to write or maintain any
code. The application is not a library, so let us benefit from that.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 29 +++++-------------------
1 file changed, 6 insertions(+), 23 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 8ff24472ef1e..c1bb03e0ca07 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -1041,28 +1041,24 @@ static void run_pkt_test(int mode, int type)
int main(int argc, char **argv)
{
struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
- bool failure = false;
int i, j;
if (setrlimit(RLIMIT_MEMLOCK, &_rlim))
exit_with_error(errno);
- for (int i = 0; i < MAX_INTERFACES; i++) {
+ for (i = 0; i < MAX_INTERFACES; i++) {
ifdict[i] = malloc(sizeof(struct ifobject));
if (!ifdict[i])
exit_with_error(errno);
ifdict[i]->ifdict_index = i;
ifdict[i]->xsk_arr = calloc(2, sizeof(struct xsk_socket_info *));
- if (!ifdict[i]->xsk_arr) {
- failure = true;
- goto cleanup;
- }
+ if (!ifdict[i]->xsk_arr)
+ exit_with_error(errno);
+
ifdict[i]->umem_arr = calloc(2, sizeof(struct xsk_umem_info *));
- if (!ifdict[i]->umem_arr) {
- failure = true;
- goto cleanup;
- }
+ if (!ifdict[i]->umem_arr)
+ exit_with_error(errno);
}
setlocale(LC_ALL, "");
@@ -1081,19 +1077,6 @@ int main(int argc, char **argv)
}
}
-cleanup:
- for (int i = 0; i < MAX_INTERFACES; i++) {
- if (ifdict[i]->ns_fd != -1)
- close(ifdict[i]->ns_fd);
- free(ifdict[i]->xsk_arr);
- free(ifdict[i]->umem_arr);
- free(ifdict[i]);
- }
-
- if (failure)
- exit_with_error(errno);
-
ksft_exit_pass();
-
return 0;
}
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next v2 12/16] selftests: xsk: remove cleanup at end of program
2021-08-17 9:27 ` [PATCH bpf-next v2 12/16] selftests: xsk: remove cleanup at end of program Magnus Karlsson
@ 2021-08-19 9:28 ` Maciej Fijalkowski
2021-08-19 10:02 ` Magnus Karlsson
0 siblings, 1 reply; 24+ messages in thread
From: Maciej Fijalkowski @ 2021-08-19 9:28 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
ciara.loftus, bpf, yhs, andrii
On Tue, Aug 17, 2021 at 11:27:25AM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Remove the cleanup right before the program/process exits as this will
> trigger the cleanup without us having to write or maintain any
> code. The application is not a library, so let us benefit from that.
Not a fan of that, I'd rather keep things tidy, but you're right that
dropping this logic won't hurt us.
>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
> tools/testing/selftests/bpf/xdpxceiver.c | 29 +++++-------------------
> 1 file changed, 6 insertions(+), 23 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index 8ff24472ef1e..c1bb03e0ca07 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -1041,28 +1041,24 @@ static void run_pkt_test(int mode, int type)
> int main(int argc, char **argv)
> {
> struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
> - bool failure = false;
> int i, j;
>
> if (setrlimit(RLIMIT_MEMLOCK, &_rlim))
> exit_with_error(errno);
>
> - for (int i = 0; i < MAX_INTERFACES; i++) {
> + for (i = 0; i < MAX_INTERFACES; i++) {
> ifdict[i] = malloc(sizeof(struct ifobject));
> if (!ifdict[i])
> exit_with_error(errno);
>
> ifdict[i]->ifdict_index = i;
> ifdict[i]->xsk_arr = calloc(2, sizeof(struct xsk_socket_info *));
> - if (!ifdict[i]->xsk_arr) {
> - failure = true;
> - goto cleanup;
> - }
> + if (!ifdict[i]->xsk_arr)
> + exit_with_error(errno);
> +
> ifdict[i]->umem_arr = calloc(2, sizeof(struct xsk_umem_info *));
> - if (!ifdict[i]->umem_arr) {
> - failure = true;
> - goto cleanup;
> - }
> + if (!ifdict[i]->umem_arr)
> + exit_with_error(errno);
> }
>
> setlocale(LC_ALL, "");
> @@ -1081,19 +1077,6 @@ int main(int argc, char **argv)
> }
> }
>
> -cleanup:
> - for (int i = 0; i < MAX_INTERFACES; i++) {
> - if (ifdict[i]->ns_fd != -1)
> - close(ifdict[i]->ns_fd);
> - free(ifdict[i]->xsk_arr);
> - free(ifdict[i]->umem_arr);
> - free(ifdict[i]);
> - }
> -
> - if (failure)
> - exit_with_error(errno);
> -
> ksft_exit_pass();
> -
> return 0;
> }
> --
> 2.29.0
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next v2 12/16] selftests: xsk: remove cleanup at end of program
2021-08-19 9:28 ` Maciej Fijalkowski
@ 2021-08-19 10:02 ` Magnus Karlsson
0 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-19 10: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 Thu, Aug 19, 2021 at 11:43 AM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Tue, Aug 17, 2021 at 11:27:25AM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Remove the cleanup right before the program/process exits as this will
> > trigger the cleanup without us having to write or maintain any
> > code. The application is not a library, so let us benefit from that.
>
> Not a fan of that, I'd rather keep things tidy, but you're right that
> dropping this logic won't hurt us.
My strategy here was that all functions should clean up themselves and
be tidy, the exception to that being the main function as if it
exists, the program is gone and libc will clean up the allocations for
us. Maybe a bit pragmatic, but I do prefer less code in this case. On
the other hand,
I do not have any strong objections to keeping the code. Just think it
is unnecessary. But if we hide the allocations in a function, then I
would need to deallocate them later for it to look clean (according to
the above logic). Maybe that will improve readabilty. Will try it out.
/Magnus
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> > tools/testing/selftests/bpf/xdpxceiver.c | 29 +++++-------------------
> > 1 file changed, 6 insertions(+), 23 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> > index 8ff24472ef1e..c1bb03e0ca07 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.c
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> > @@ -1041,28 +1041,24 @@ static void run_pkt_test(int mode, int type)
> > int main(int argc, char **argv)
> > {
> > struct rlimit _rlim = { RLIM_INFINITY, RLIM_INFINITY };
> > - bool failure = false;
> > int i, j;
> >
> > if (setrlimit(RLIMIT_MEMLOCK, &_rlim))
> > exit_with_error(errno);
> >
> > - for (int i = 0; i < MAX_INTERFACES; i++) {
> > + for (i = 0; i < MAX_INTERFACES; i++) {
> > ifdict[i] = malloc(sizeof(struct ifobject));
> > if (!ifdict[i])
> > exit_with_error(errno);
> >
> > ifdict[i]->ifdict_index = i;
> > ifdict[i]->xsk_arr = calloc(2, sizeof(struct xsk_socket_info *));
> > - if (!ifdict[i]->xsk_arr) {
> > - failure = true;
> > - goto cleanup;
> > - }
> > + if (!ifdict[i]->xsk_arr)
> > + exit_with_error(errno);
> > +
> > ifdict[i]->umem_arr = calloc(2, sizeof(struct xsk_umem_info *));
> > - if (!ifdict[i]->umem_arr) {
> > - failure = true;
> > - goto cleanup;
> > - }
> > + if (!ifdict[i]->umem_arr)
> > + exit_with_error(errno);
> > }
> >
> > setlocale(LC_ALL, "");
> > @@ -1081,19 +1077,6 @@ int main(int argc, char **argv)
> > }
> > }
> >
> > -cleanup:
> > - for (int i = 0; i < MAX_INTERFACES; i++) {
> > - if (ifdict[i]->ns_fd != -1)
> > - close(ifdict[i]->ns_fd);
> > - free(ifdict[i]->xsk_arr);
> > - free(ifdict[i]->umem_arr);
> > - free(ifdict[i]);
> > - }
> > -
> > - if (failure)
> > - exit_with_error(errno);
> > -
> > ksft_exit_pass();
> > -
> > return 0;
> > }
> > --
> > 2.29.0
> >
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf-next v2 13/16] selftests: xsk: generate packet directly in umem
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (11 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 12/16] selftests: xsk: remove cleanup at end of program Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 14/16] selftests: xsk: generate packets from specification Magnus Karlsson
` (3 subsequent siblings)
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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>
Generate the packet directly in the umem instead of in a temporary
buffer that is copied out. Simplifies the code and improves
performance.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 70 +++++++++++-------------
tools/testing/selftests/bpf/xdpxceiver.h | 5 --
2 files changed, 32 insertions(+), 43 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index c1bb03e0ca07..a62e5ebb0f2c 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -125,7 +125,7 @@ static void __exit_with_error(int error, const char *file, const char *func, int
test_type == TEST_TYPE_STATS ? "Stats" : "",\
test_type == TEST_TYPE_BPF_RES ? "BPF RES" : ""))
-static void *memset32_htonl(void *dest, u32 val, u32 size)
+static void memset32_htonl(void *dest, u32 val, u32 size)
{
u32 *ptr = (u32 *)dest;
int i;
@@ -134,11 +134,6 @@ static void *memset32_htonl(void *dest, u32 val, u32 size)
for (i = 0; i < (size & (~0x3)); i += 4)
ptr[i >> 2] = val;
-
- for (; i < size; i++)
- ((char *)dest)[i] = ((char *)&val)[i & 3];
-
- return dest;
}
/*
@@ -229,13 +224,13 @@ static void gen_ip_hdr(struct ifobject *ifobject, struct iphdr *ip_hdr)
ip_hdr->check = 0;
}
-static void gen_udp_hdr(struct generic_data *data, struct ifobject *ifobject,
+static void gen_udp_hdr(u32 payload, void *pkt, struct ifobject *ifobject,
struct udphdr *udp_hdr)
{
udp_hdr->source = htons(ifobject->src_port);
udp_hdr->dest = htons(ifobject->dst_port);
udp_hdr->len = htons(UDP_PKT_SIZE);
- memset32_htonl(pkt_data + PKT_HDR_SIZE, htonl(data->seqnum), UDP_PKT_DATA_SIZE);
+ memset32_htonl(pkt + PKT_HDR_SIZE, payload, UDP_PKT_DATA_SIZE);
}
static void gen_udp_csum(struct udphdr *udp_hdr, struct iphdr *ip_hdr)
@@ -245,11 +240,6 @@ 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 gen_eth_frame(struct xsk_umem_info *umem, u64 addr)
-{
- memcpy(xsk_umem__get_data(umem->buffer, addr), pkt_data, PKT_SIZE);
-}
-
static void xsk_configure_umem(struct ifobject *data, void *buffer, u64 size, int idx)
{
struct xsk_umem_config cfg = {
@@ -427,6 +417,20 @@ static void parse_command_line(int argc, char **argv)
}
}
+static void pkt_generate(struct ifobject *ifobject, u32 pkt_nb, u64 addr)
+{
+ void *data = xsk_umem__get_data(ifobject->umem->buffer, addr);
+ struct udphdr *udp_hdr =
+ (struct udphdr *)(data + sizeof(struct ethhdr) + sizeof(struct iphdr));
+ struct iphdr *ip_hdr = (struct iphdr *)(data + sizeof(struct ethhdr));
+ struct ethhdr *eth_hdr = (struct ethhdr *)data;
+
+ gen_udp_hdr(pkt_nb, data, ifobject, udp_hdr);
+ gen_ip_hdr(ifobject, ip_hdr);
+ gen_udp_csum(udp_hdr, ip_hdr);
+ gen_eth_hdr(ifobject, eth_hdr);
+}
+
static void pkt_dump(void *pkt, u32 len)
{
char s[INET_ADDRSTRLEN];
@@ -464,22 +468,23 @@ static void pkt_dump(void *pkt, u32 len)
fprintf(stdout, "---------------------------------------\n");
}
-static void pkt_validate(void *pkt)
+static void pkt_validate(void *buffer, u64 addr)
{
- struct iphdr *iphdr = (struct iphdr *)(pkt + sizeof(struct ethhdr));
+ void *data = xsk_umem__get_data(buffer, addr);
+ struct iphdr *iphdr = (struct iphdr *)(data + sizeof(struct ethhdr));
- /*do not increment pktcounter if !(tos=0x9 and ipv4) */
if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
- u32 payloadseqnum = *((uint32_t *)(pkt + PKT_HDR_SIZE));
+ u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
+ u32 expected_seqnum = pkt_counter % num_frames;
if (debug_pkt_dump && test_type != TEST_TYPE_STATS)
- pkt_dump(pkt, PKT_SIZE);
+ pkt_dump(data, PKT_SIZE);
- if (pkt_counter % num_frames != payloadseqnum) {
+ if (expected_seqnum != seqnum) {
ksft_test_result_fail
("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
- __func__, pkt_counter, payloadseqnum);
- ksft_exit_xfail();
+ __func__, expected_seqnum, seqnum);
+ sigvar = 1;
}
if (++pkt_counter == opt_pkt_count)
@@ -488,6 +493,7 @@ static void pkt_validate(void *pkt)
ksft_print_msg("Invalid frame received: ");
ksft_print_msg("[IP_PKT_VER: %02X], [IP_PKT_TOS: %02X]\n", iphdr->version,
iphdr->tos);
+ sigvar = 1;
}
}
@@ -555,7 +561,7 @@ static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
orig = xsk_umem__extract_addr(addr);
addr = xsk_umem__add_offset_to_addr(addr);
- pkt_validate(xsk_umem__get_data(xsk->umem->buffer, addr));
+ pkt_validate(xsk->umem->buffer, addr);
*xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
}
@@ -564,8 +570,9 @@ static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
xsk_ring_cons__release(&xsk->rx, rcvd);
}
-static void tx_only(struct xsk_socket_info *xsk, u32 *frameptr, int batch_size)
+static void tx_only(struct ifobject *ifobject, u32 *frameptr, int batch_size)
{
+ struct xsk_socket_info *xsk = ifobject->xsk;
u32 idx = 0;
unsigned int i;
bool tx_invalid_test = stat_test_type == STAT_TEST_TX_INVALID;
@@ -579,6 +586,7 @@ static void tx_only(struct xsk_socket_info *xsk, u32 *frameptr, int batch_size)
tx_desc->addr = (*frameptr + i) << XSK_UMEM__DEFAULT_FRAME_SHIFT;
tx_desc->len = len;
+ pkt_generate(ifobject, *frameptr + i, tx_desc->addr);
}
xsk_ring_prod__submit(&xsk->tx, batch_size);
@@ -635,7 +643,7 @@ static void tx_only_all(struct ifobject *ifobject)
continue;
}
- tx_only(ifobject->xsk, &frame_nb, batch_size);
+ tx_only(ifobject, &frame_nb, batch_size);
pkt_cnt += batch_size;
}
@@ -767,26 +775,12 @@ static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
static void *worker_testapp_validate_tx(void *arg)
{
- struct udphdr *udp_hdr =
- (struct udphdr *)(pkt_data + sizeof(struct ethhdr) + sizeof(struct iphdr));
- struct iphdr *ip_hdr = (struct iphdr *)(pkt_data + sizeof(struct ethhdr));
- struct ethhdr *eth_hdr = (struct ethhdr *)pkt_data;
struct ifobject *ifobject = (struct ifobject *)arg;
- struct generic_data data;
void *bufs = NULL;
if (!second_step)
thread_common_ops(ifobject, bufs);
- for (int i = 0; i < num_frames; i++) {
- data.seqnum = i;
- gen_udp_hdr(&data, ifobject, udp_hdr);
- gen_ip_hdr(ifobject, ip_hdr);
- gen_udp_csum(udp_hdr, ip_hdr);
- gen_eth_hdr(ifobject, eth_hdr);
- gen_eth_frame(ifobject->umem, i * XSK_UMEM__DEFAULT_FRAME_SIZE);
- }
-
print_verbose("Sending %d packets on interface %s\n", opt_pkt_count, ifobject->ifname);
tx_only_all(ifobject);
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 1c5457e9f1d6..b3087270d837 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -79,7 +79,6 @@ static u8 opt_verbose;
static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
static u32 xdp_bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
-static u8 pkt_data[XSK_UMEM__DEFAULT_FRAME_SIZE];
static u32 pkt_counter;
static int sigvar;
static int stat_test_type;
@@ -108,10 +107,6 @@ struct flow_vector {
} vector;
};
-struct generic_data {
- u32 seqnum;
-};
-
struct ifobject {
char ifname[MAX_INTERFACE_NAME_CHARS];
char nsname[MAX_INTERFACES_NAMESPACE_CHARS];
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 14/16] selftests: xsk: generate packets from specification
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (12 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 13/16] selftests: xsk: generate packet directly in umem Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-19 10:10 ` Maciej Fijalkowski
2021-08-17 9:27 ` [PATCH bpf-next v2 15/16] selftests: xsk: make enums lower case Magnus Karlsson
` (2 subsequent siblings)
16 siblings, 1 reply; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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>
Generate packets from a specification instead of something hard
coded. The idea is that a test generates one or more packet
specifications and provides it/them to both Tx and Rx. The Tx thread
will generate from this specification and Rx will validate that it
receives what is in the specification. The specification can be the
same on both ends, meaning that everything that was sent should be
received, or different which means that Rx will only receive part of
the sent packets.
Currently, the packet specification is the same for both Rx and Tx and
the same for each test. This will change in later patches as features
and tests are added.
The data path functions are also renamed to better reflect what
actions they are performing after introducing this feature.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 254 +++++++++++++----------
tools/testing/selftests/bpf/xdpxceiver.h | 15 +-
2 files changed, 152 insertions(+), 117 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index a62e5ebb0f2c..4160ba5f50a3 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -417,18 +417,52 @@ static void parse_command_line(int argc, char **argv)
}
}
-static void pkt_generate(struct ifobject *ifobject, u32 pkt_nb, u64 addr)
+static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
{
- void *data = xsk_umem__get_data(ifobject->umem->buffer, addr);
+ return &pkt_stream->pkts[pkt_nb];
+}
+
+static struct pkt_stream *pkt_stream_generate(u32 nb_pkts, u32 pkt_len)
+{
+ struct pkt_stream *pkt_stream;
+ u32 i;
+
+ pkt_stream = malloc(sizeof(*pkt_stream));
+ if (!pkt_stream)
+ exit_with_error(ENOMEM);
+
+ pkt_stream->pkts = calloc(DEFAULT_PKT_CNT, sizeof(*pkt_stream->pkts));
+ if (!pkt_stream->pkts)
+ exit_with_error(ENOMEM);
+
+ 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].len = pkt_len;
+ pkt_stream->pkts[i].payload = i;
+ }
+
+ return 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);
+ void *data = xsk_umem__get_data(ifobject->umem->buffer, pkt->addr);
struct udphdr *udp_hdr =
(struct udphdr *)(data + sizeof(struct ethhdr) + sizeof(struct iphdr));
struct iphdr *ip_hdr = (struct iphdr *)(data + sizeof(struct ethhdr));
struct ethhdr *eth_hdr = (struct ethhdr *)data;
+ if (pkt_nb >= ifobject->pkt_stream->nb_pkts)
+ return NULL;
+
gen_udp_hdr(pkt_nb, data, ifobject, udp_hdr);
gen_ip_hdr(ifobject, ip_hdr);
gen_udp_csum(udp_hdr, ip_hdr);
gen_eth_hdr(ifobject, eth_hdr);
+
+ return pkt;
}
static void pkt_dump(void *pkt, u32 len)
@@ -468,33 +502,38 @@ static void pkt_dump(void *pkt, u32 len)
fprintf(stdout, "---------------------------------------\n");
}
-static void pkt_validate(void *buffer, u64 addr)
+static bool pkt_validate(struct pkt *pkt, void *buffer, const struct xdp_desc *desc)
{
- void *data = xsk_umem__get_data(buffer, addr);
+ void *data = xsk_umem__get_data(buffer, desc->addr);
struct iphdr *iphdr = (struct iphdr *)(data + sizeof(struct ethhdr));
if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
- u32 expected_seqnum = pkt_counter % num_frames;
if (debug_pkt_dump && test_type != TEST_TYPE_STATS)
pkt_dump(data, PKT_SIZE);
- if (expected_seqnum != seqnum) {
+ if (pkt->len != desc->len) {
ksft_test_result_fail
- ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
- __func__, expected_seqnum, seqnum);
- sigvar = 1;
+ ("ERROR: [%s] expected length [%d], got length [%d]\n",
+ __func__, pkt->len, desc->len);
+ return false;
}
- if (++pkt_counter == opt_pkt_count)
- sigvar = 1;
+ if (pkt->payload != seqnum) {
+ ksft_test_result_fail
+ ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
+ __func__, pkt->payload, seqnum);
+ return false;
+ }
} else {
ksft_print_msg("Invalid frame received: ");
ksft_print_msg("[IP_PKT_VER: %02X], [IP_PKT_TOS: %02X]\n", iphdr->version,
iphdr->tos);
- sigvar = 1;
+ return false;
}
+
+ return true;
}
static void kick_tx(struct xsk_socket_info *xsk)
@@ -507,7 +546,7 @@ static void kick_tx(struct xsk_socket_info *xsk)
exit_with_error(errno);
}
-static void complete_tx_only(struct xsk_socket_info *xsk, int batch_size)
+static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
{
unsigned int rcvd;
u32 idx;
@@ -525,116 +564,106 @@ static void complete_tx_only(struct xsk_socket_info *xsk, int batch_size)
}
}
-static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
+static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *xsk,
+ struct pollfd *fds)
{
- unsigned int rcvd, i;
- u32 idx_rx = 0, idx_fq = 0;
+ u32 idx_rx = 0, idx_fq = 0, rcvd, i, pkt_count = 0;
int ret;
- rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
- if (!rcvd) {
- if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
- ret = poll(fds, 1, POLL_TMOUT);
- if (ret < 0)
- exit_with_error(-ret);
+ while (1) {
+ rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
+ if (!rcvd) {
+ if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
+ ret = poll(fds, 1, POLL_TMOUT);
+ if (ret < 0)
+ exit_with_error(-ret);
+ }
+ continue;
}
- return;
- }
- ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
- while (ret != rcvd) {
- if (ret < 0)
- exit_with_error(-ret);
- if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
- ret = poll(fds, 1, POLL_TMOUT);
+ ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
+ while (ret != rcvd) {
if (ret < 0)
exit_with_error(-ret);
+ if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
+ ret = poll(fds, 1, POLL_TMOUT);
+ if (ret < 0)
+ exit_with_error(-ret);
+ }
+ ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
}
- ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
- }
- for (i = 0; i < rcvd; i++) {
- u64 addr, orig;
+ for (i = 0; i < rcvd; i++) {
+ const struct xdp_desc *desc = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
+ u64 addr = desc->addr, orig;
+
+ orig = xsk_umem__extract_addr(addr);
+ addr = xsk_umem__add_offset_to_addr(addr);
+ if (!pkt_validate(pkt_stream_get_pkt(pkt_stream, pkt_count++),
+ xsk->umem->buffer, desc))
+ return;
- addr = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx)->addr;
- xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
- orig = xsk_umem__extract_addr(addr);
+ *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
+ }
- addr = xsk_umem__add_offset_to_addr(addr);
- pkt_validate(xsk->umem->buffer, addr);
+ xsk_ring_prod__submit(&xsk->umem->fq, rcvd);
+ xsk_ring_cons__release(&xsk->rx, rcvd);
- *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
+ if (pkt_count >= pkt_stream->nb_pkts)
+ return;
}
-
- xsk_ring_prod__submit(&xsk->umem->fq, rcvd);
- xsk_ring_cons__release(&xsk->rx, rcvd);
}
-static void tx_only(struct ifobject *ifobject, u32 *frameptr, int batch_size)
+static u32 __send_pkts(struct ifobject *ifobject, u32 pkt_nb)
{
struct xsk_socket_info *xsk = ifobject->xsk;
- u32 idx = 0;
- unsigned int i;
- bool tx_invalid_test = stat_test_type == STAT_TEST_TX_INVALID;
- u32 len = tx_invalid_test ? XSK_UMEM__DEFAULT_FRAME_SIZE + 1 : PKT_SIZE;
+ u32 i, idx;
- while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) < batch_size)
- complete_tx_only(xsk, batch_size);
+ while (xsk_ring_prod__reserve(&xsk->tx, BATCH_SIZE, &idx) < BATCH_SIZE)
+ complete_pkts(xsk, BATCH_SIZE);
- for (i = 0; i < batch_size; i++) {
+ for (i = 0; i < BATCH_SIZE; i++) {
struct xdp_desc *tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx + i);
+ struct pkt *pkt = pkt_generate(ifobject, pkt_nb);
- tx_desc->addr = (*frameptr + i) << XSK_UMEM__DEFAULT_FRAME_SHIFT;
- tx_desc->len = len;
- pkt_generate(ifobject, *frameptr + i, tx_desc->addr);
- }
+ if (!pkt)
+ break;
- xsk_ring_prod__submit(&xsk->tx, batch_size);
- if (!tx_invalid_test) {
- xsk->outstanding_tx += batch_size;
- } else if (xsk_ring_prod__needs_wakeup(&xsk->tx)) {
- kick_tx(xsk);
+ tx_desc->addr = pkt->addr;
+ tx_desc->len = pkt->len;
+ pkt_nb++;
}
- *frameptr += batch_size;
- *frameptr %= num_frames;
- complete_tx_only(xsk, batch_size);
-}
-static int get_batch_size(int pkt_cnt)
-{
- if (pkt_cnt + BATCH_SIZE <= opt_pkt_count)
- return BATCH_SIZE;
+ 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);
- return opt_pkt_count - pkt_cnt;
+ return i;
}
-static void complete_tx_only_all(struct ifobject *ifobject)
+static void wait_for_tx_completion(struct xsk_socket_info *xsk)
{
- bool pending;
-
- do {
- pending = false;
- if (ifobject->xsk->outstanding_tx) {
- complete_tx_only(ifobject->xsk, BATCH_SIZE);
- pending = !!ifobject->xsk->outstanding_tx;
- }
- } while (pending);
+ while (xsk->outstanding_tx)
+ complete_pkts(xsk, BATCH_SIZE);
}
-static void tx_only_all(struct ifobject *ifobject)
+static void send_pkts(struct ifobject *ifobject)
{
struct pollfd fds[MAX_SOCKS] = { };
- u32 frame_nb = 0;
- int pkt_cnt = 0;
- int ret;
+ u32 pkt_cnt = 0;
fds[0].fd = xsk_socket__fd(ifobject->xsk->xsk);
fds[0].events = POLLOUT;
- while (pkt_cnt < opt_pkt_count) {
- int batch_size = get_batch_size(pkt_cnt);
+ while (pkt_cnt < ifobject->pkt_stream->nb_pkts) {
+ u32 sent;
if (test_type == TEST_TYPE_POLL) {
+ int ret;
+
ret = poll(fds, 1, POLL_TMOUT);
if (ret <= 0)
continue;
@@ -643,16 +672,16 @@ static void tx_only_all(struct ifobject *ifobject)
continue;
}
- tx_only(ifobject, &frame_nb, batch_size);
- pkt_cnt += batch_size;
+ sent = __send_pkts(ifobject, pkt_cnt);
+ pkt_cnt += sent;
}
- complete_tx_only_all(ifobject);
+ wait_for_tx_completion(ifobject->xsk);
}
static bool rx_stats_are_valid(struct ifobject *ifobject)
{
- u32 xsk_stat = 0, expected_stat = opt_pkt_count;
+ u32 xsk_stat = 0, expected_stat = ifobject->pkt_stream->nb_pkts;
struct xsk_socket *xsk = ifobject->xsk->xsk;
int fd = xsk_socket__fd(xsk);
struct xdp_statistics stats;
@@ -708,11 +737,11 @@ static void tx_stats_validate(struct ifobject *ifobject)
return;
}
- if (stats.tx_invalid_descs == opt_pkt_count)
+ if (stats.tx_invalid_descs == ifobject->pkt_stream->nb_pkts)
return;
ksft_test_result_fail("ERROR: [%s] tx_invalid_descs incorrect. Got [%u] expected [%u]\n",
- __func__, stats.tx_invalid_descs, opt_pkt_count);
+ __func__, stats.tx_invalid_descs, ifobject->pkt_stream->nb_pkts);
}
static void thread_common_ops(struct ifobject *ifobject, void *bufs)
@@ -781,8 +810,9 @@ static void *worker_testapp_validate_tx(void *arg)
if (!second_step)
thread_common_ops(ifobject, bufs);
- print_verbose("Sending %d packets on interface %s\n", opt_pkt_count, ifobject->ifname);
- tx_only_all(ifobject);
+ print_verbose("Sending %d packets on interface %s\n", ifobject->pkt_stream->nb_pkts,
+ ifobject->ifname);
+ send_pkts(ifobject);
if (stat_test_type == STAT_TEST_TX_INVALID)
tx_stats_validate(ifobject);
@@ -808,19 +838,11 @@ static void *worker_testapp_validate_rx(void *arg)
pthread_barrier_wait(&barr);
- while (1) {
- if (test_type != TEST_TYPE_STATS) {
- rx_pkt(ifobject->xsk, fds);
- } else {
- if (rx_stats_are_valid(ifobject))
- break;
- }
- if (sigvar)
- break;
- }
-
- print_verbose("Received %d packets on interface %s\n",
- pkt_counter, ifobject->ifname);
+ if (test_type == TEST_TYPE_STATS)
+ while (!rx_stats_are_valid(ifobject))
+ continue;
+ else
+ receive_pkts(ifobject->pkt_stream, ifobject->xsk, fds);
if (test_type == TEST_TYPE_TEARDOWN)
print_verbose("Destroying socket\n");
@@ -833,10 +855,20 @@ static void testapp_validate(void)
{
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))
exit_with_error(errno);
+ if (stat_test_type == STAT_TEST_TX_INVALID) {
+ pkt_stream = pkt_stream_generate(DEFAULT_PKT_CNT,
+ XSK_UMEM__DEFAULT_FRAME_SIZE + 1);
+ } else {
+ pkt_stream = pkt_stream_generate(DEFAULT_PKT_CNT, PKT_SIZE);
+ }
+ ifdict_tx->pkt_stream = pkt_stream;
+ ifdict_rx->pkt_stream = pkt_stream;
+
/*Spawn RX thread */
pthread_create(&t0, NULL, ifdict_rx->func_ptr, ifdict_rx);
@@ -859,8 +891,6 @@ static void testapp_teardown(void)
int i;
for (i = 0; i < MAX_TEARDOWN_ITER; i++) {
- pkt_counter = 0;
- sigvar = 0;
print_verbose("Creating socket\n");
testapp_validate();
}
@@ -886,8 +916,6 @@ static void swap_vectors(struct ifobject *ifobj1, struct ifobject *ifobj2)
static void testapp_bidi(void)
{
for (int i = 0; i < MAX_BIDI_ITER; i++) {
- pkt_counter = 0;
- sigvar = 0;
print_verbose("Creating socket\n");
testapp_validate();
if (!second_step) {
@@ -919,8 +947,6 @@ static void testapp_bpf_res(void)
int i;
for (i = 0; i < MAX_BPF_ITER; i++) {
- pkt_counter = 0;
- sigvar = 0;
print_verbose("Creating socket\n");
testapp_validate();
if (!second_step)
@@ -948,6 +974,8 @@ static void testapp_stats(void)
case STAT_TEST_RX_FULL:
rxqsize = RX_FULL_RXQSIZE;
break;
+ case STAT_TEST_TX_INVALID:
+ continue;
default:
break;
}
@@ -993,9 +1021,7 @@ static void run_pkt_test(int mode, int type)
/* reset defaults after potential previous test */
xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
- pkt_counter = 0;
second_step = 0;
- sigvar = 0;
stat_test_type = -1;
rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index b3087270d837..f5b46cd3d8df 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -74,13 +74,10 @@ static u32 num_frames = DEFAULT_PKT_CNT / 4;
static bool second_step;
static int test_type;
-static u32 opt_pkt_count = DEFAULT_PKT_CNT;
static u8 opt_verbose;
static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
static u32 xdp_bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
-static u32 pkt_counter;
-static int sigvar;
static int stat_test_type;
static u32 rxqsize;
static u32 frame_headroom;
@@ -107,6 +104,17 @@ struct flow_vector {
} vector;
};
+struct pkt {
+ u64 addr;
+ u32 len;
+ u32 payload;
+};
+
+struct pkt_stream {
+ u32 nb_pkts;
+ struct pkt *pkts;
+};
+
struct ifobject {
char ifname[MAX_INTERFACE_NAME_CHARS];
char nsname[MAX_INTERFACES_NAMESPACE_CHARS];
@@ -116,6 +124,7 @@ struct ifobject {
struct xsk_umem_info *umem;
void *(*func_ptr)(void *arg);
struct flow_vector fv;
+ struct pkt_stream *pkt_stream;
int ns_fd;
int ifdict_index;
u32 dst_ip;
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next v2 14/16] selftests: xsk: generate packets from specification
2021-08-17 9:27 ` [PATCH bpf-next v2 14/16] selftests: xsk: generate packets from specification Magnus Karlsson
@ 2021-08-19 10:10 ` Maciej Fijalkowski
2021-08-19 10:38 ` Magnus Karlsson
0 siblings, 1 reply; 24+ messages in thread
From: Maciej Fijalkowski @ 2021-08-19 10:10 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
ciara.loftus, bpf, yhs, andrii
On Tue, Aug 17, 2021 at 11:27:27AM +0200, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Generate packets from a specification instead of something hard
> coded. The idea is that a test generates one or more packet
> specifications and provides it/them to both Tx and Rx. The Tx thread
> will generate from this specification and Rx will validate that it
> receives what is in the specification. The specification can be the
> same on both ends, meaning that everything that was sent should be
> received, or different which means that Rx will only receive part of
> the sent packets.
>
> Currently, the packet specification is the same for both Rx and Tx and
> the same for each test. This will change in later patches as features
> and tests are added.
You probably meant 'later work' by itself, by saying 'later patches' I
would expect it to be in this same set. But that's not a big deal to me.
>
> The data path functions are also renamed to better reflect what
> actions they are performing after introducing this feature.
>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
> tools/testing/selftests/bpf/xdpxceiver.c | 254 +++++++++++++----------
> tools/testing/selftests/bpf/xdpxceiver.h | 15 +-
> 2 files changed, 152 insertions(+), 117 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index a62e5ebb0f2c..4160ba5f50a3 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -417,18 +417,52 @@ static void parse_command_line(int argc, char **argv)
> }
> }
>
> -static void pkt_generate(struct ifobject *ifobject, u32 pkt_nb, u64 addr)
> +static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
> {
> - void *data = xsk_umem__get_data(ifobject->umem->buffer, addr);
> + return &pkt_stream->pkts[pkt_nb];
> +}
> +
> +static struct pkt_stream *pkt_stream_generate(u32 nb_pkts, u32 pkt_len)
> +{
> + struct pkt_stream *pkt_stream;
> + u32 i;
> +
> + pkt_stream = malloc(sizeof(*pkt_stream));
> + if (!pkt_stream)
> + exit_with_error(ENOMEM);
> +
> + pkt_stream->pkts = calloc(DEFAULT_PKT_CNT, sizeof(*pkt_stream->pkts));
Why not nb_pkts as a first arg?
> + if (!pkt_stream->pkts)
> + exit_with_error(ENOMEM);
> +
> + 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].len = pkt_len;
> + pkt_stream->pkts[i].payload = i;
> + }
> +
> + return 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);
> + void *data = xsk_umem__get_data(ifobject->umem->buffer, pkt->addr);
> struct udphdr *udp_hdr =
> (struct udphdr *)(data + sizeof(struct ethhdr) + sizeof(struct iphdr));
> struct iphdr *ip_hdr = (struct iphdr *)(data + sizeof(struct ethhdr));
> struct ethhdr *eth_hdr = (struct ethhdr *)data;
>
> + if (pkt_nb >= ifobject->pkt_stream->nb_pkts)
> + return NULL;
shouldn't this check be within pkt_stream_get_pkt ?
> +
> gen_udp_hdr(pkt_nb, data, ifobject, udp_hdr);
> gen_ip_hdr(ifobject, ip_hdr);
> gen_udp_csum(udp_hdr, ip_hdr);
> gen_eth_hdr(ifobject, eth_hdr);
> +
> + return pkt;
> }
>
> static void pkt_dump(void *pkt, u32 len)
> @@ -468,33 +502,38 @@ static void pkt_dump(void *pkt, u32 len)
> fprintf(stdout, "---------------------------------------\n");
> }
>
> -static void pkt_validate(void *buffer, u64 addr)
> +static bool pkt_validate(struct pkt *pkt, void *buffer, const struct xdp_desc *desc)
Now maybe a better name (because of bool) would be is_pkt_valid ?
> {
> - void *data = xsk_umem__get_data(buffer, addr);
> + void *data = xsk_umem__get_data(buffer, desc->addr);
> struct iphdr *iphdr = (struct iphdr *)(data + sizeof(struct ethhdr));
>
> if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
> u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
> - u32 expected_seqnum = pkt_counter % num_frames;
>
> if (debug_pkt_dump && test_type != TEST_TYPE_STATS)
> pkt_dump(data, PKT_SIZE);
>
> - if (expected_seqnum != seqnum) {
> + if (pkt->len != desc->len) {
> ksft_test_result_fail
> - ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
> - __func__, expected_seqnum, seqnum);
> - sigvar = 1;
> + ("ERROR: [%s] expected length [%d], got length [%d]\n",
> + __func__, pkt->len, desc->len);
> + return false;
> }
>
> - if (++pkt_counter == opt_pkt_count)
> - sigvar = 1;
> + if (pkt->payload != seqnum) {
> + ksft_test_result_fail
> + ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
> + __func__, pkt->payload, seqnum);
> + return false;
> + }
> } else {
> ksft_print_msg("Invalid frame received: ");
> ksft_print_msg("[IP_PKT_VER: %02X], [IP_PKT_TOS: %02X]\n", iphdr->version,
> iphdr->tos);
> - sigvar = 1;
> + return false;
> }
> +
> + return true;
> }
>
> static void kick_tx(struct xsk_socket_info *xsk)
> @@ -507,7 +546,7 @@ static void kick_tx(struct xsk_socket_info *xsk)
> exit_with_error(errno);
> }
>
> -static void complete_tx_only(struct xsk_socket_info *xsk, int batch_size)
> +static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
> {
> unsigned int rcvd;
> u32 idx;
> @@ -525,116 +564,106 @@ static void complete_tx_only(struct xsk_socket_info *xsk, int batch_size)
> }
> }
>
> -static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
> +static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *xsk,
> + struct pollfd *fds)
> {
> - unsigned int rcvd, i;
> - u32 idx_rx = 0, idx_fq = 0;
> + u32 idx_rx = 0, idx_fq = 0, rcvd, i, pkt_count = 0;
> int ret;
>
> - rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
> - if (!rcvd) {
> - if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
> - ret = poll(fds, 1, POLL_TMOUT);
> - if (ret < 0)
> - exit_with_error(-ret);
> + while (1) {
> + rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
> + if (!rcvd) {
> + if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
> + ret = poll(fds, 1, POLL_TMOUT);
> + if (ret < 0)
> + exit_with_error(-ret);
> + }
> + continue;
> }
> - return;
> - }
>
> - ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
> - while (ret != rcvd) {
> - if (ret < 0)
> - exit_with_error(-ret);
> - if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
> - ret = poll(fds, 1, POLL_TMOUT);
> + ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
> + while (ret != rcvd) {
> if (ret < 0)
> exit_with_error(-ret);
> + if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
> + ret = poll(fds, 1, POLL_TMOUT);
> + if (ret < 0)
> + exit_with_error(-ret);
> + }
> + ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
> }
> - ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
> - }
>
> - for (i = 0; i < rcvd; i++) {
> - u64 addr, orig;
> + for (i = 0; i < rcvd; i++) {
> + const struct xdp_desc *desc = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
> + u64 addr = desc->addr, orig;
> +
> + orig = xsk_umem__extract_addr(addr);
> + addr = xsk_umem__add_offset_to_addr(addr);
> + if (!pkt_validate(pkt_stream_get_pkt(pkt_stream, pkt_count++),
> + xsk->umem->buffer, desc))
> + return;
>
> - addr = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx)->addr;
> - xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
> - orig = xsk_umem__extract_addr(addr);
> + *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
> + }
>
> - addr = xsk_umem__add_offset_to_addr(addr);
> - pkt_validate(xsk->umem->buffer, addr);
> + xsk_ring_prod__submit(&xsk->umem->fq, rcvd);
> + xsk_ring_cons__release(&xsk->rx, rcvd);
>
> - *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
> + if (pkt_count >= pkt_stream->nb_pkts)
> + return;
> }
> -
> - xsk_ring_prod__submit(&xsk->umem->fq, rcvd);
> - xsk_ring_cons__release(&xsk->rx, rcvd);
> }
>
> -static void tx_only(struct ifobject *ifobject, u32 *frameptr, int batch_size)
> +static u32 __send_pkts(struct ifobject *ifobject, u32 pkt_nb)
> {
> struct xsk_socket_info *xsk = ifobject->xsk;
> - u32 idx = 0;
> - unsigned int i;
> - bool tx_invalid_test = stat_test_type == STAT_TEST_TX_INVALID;
> - u32 len = tx_invalid_test ? XSK_UMEM__DEFAULT_FRAME_SIZE + 1 : PKT_SIZE;
> + u32 i, idx;
>
> - while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) < batch_size)
> - complete_tx_only(xsk, batch_size);
> + while (xsk_ring_prod__reserve(&xsk->tx, BATCH_SIZE, &idx) < BATCH_SIZE)
> + complete_pkts(xsk, BATCH_SIZE);
>
> - for (i = 0; i < batch_size; i++) {
> + for (i = 0; i < BATCH_SIZE; i++) {
> struct xdp_desc *tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx + i);
> + struct pkt *pkt = pkt_generate(ifobject, pkt_nb);
>
> - tx_desc->addr = (*frameptr + i) << XSK_UMEM__DEFAULT_FRAME_SHIFT;
> - tx_desc->len = len;
> - pkt_generate(ifobject, *frameptr + i, tx_desc->addr);
> - }
> + if (!pkt)
> + break;
>
> - xsk_ring_prod__submit(&xsk->tx, batch_size);
> - if (!tx_invalid_test) {
> - xsk->outstanding_tx += batch_size;
> - } else if (xsk_ring_prod__needs_wakeup(&xsk->tx)) {
> - kick_tx(xsk);
> + tx_desc->addr = pkt->addr;
> + tx_desc->len = pkt->len;
> + pkt_nb++;
> }
> - *frameptr += batch_size;
> - *frameptr %= num_frames;
> - complete_tx_only(xsk, batch_size);
> -}
>
> -static int get_batch_size(int pkt_cnt)
> -{
> - if (pkt_cnt + BATCH_SIZE <= opt_pkt_count)
> - return BATCH_SIZE;
> + 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);
>
> - return opt_pkt_count - pkt_cnt;
> + return i;
> }
>
> -static void complete_tx_only_all(struct ifobject *ifobject)
> +static void wait_for_tx_completion(struct xsk_socket_info *xsk)
> {
> - bool pending;
> -
> - do {
> - pending = false;
> - if (ifobject->xsk->outstanding_tx) {
> - complete_tx_only(ifobject->xsk, BATCH_SIZE);
> - pending = !!ifobject->xsk->outstanding_tx;
> - }
> - } while (pending);
> + while (xsk->outstanding_tx)
> + complete_pkts(xsk, BATCH_SIZE);
> }
>
> -static void tx_only_all(struct ifobject *ifobject)
> +static void send_pkts(struct ifobject *ifobject)
> {
> struct pollfd fds[MAX_SOCKS] = { };
> - u32 frame_nb = 0;
> - int pkt_cnt = 0;
> - int ret;
> + u32 pkt_cnt = 0;
>
> fds[0].fd = xsk_socket__fd(ifobject->xsk->xsk);
> fds[0].events = POLLOUT;
>
> - while (pkt_cnt < opt_pkt_count) {
> - int batch_size = get_batch_size(pkt_cnt);
> + while (pkt_cnt < ifobject->pkt_stream->nb_pkts) {
> + u32 sent;
>
> if (test_type == TEST_TYPE_POLL) {
> + int ret;
> +
> ret = poll(fds, 1, POLL_TMOUT);
> if (ret <= 0)
> continue;
> @@ -643,16 +672,16 @@ static void tx_only_all(struct ifobject *ifobject)
> continue;
> }
>
> - tx_only(ifobject, &frame_nb, batch_size);
> - pkt_cnt += batch_size;
> + sent = __send_pkts(ifobject, pkt_cnt);
> + pkt_cnt += sent;
> }
>
> - complete_tx_only_all(ifobject);
> + wait_for_tx_completion(ifobject->xsk);
> }
>
> static bool rx_stats_are_valid(struct ifobject *ifobject)
> {
> - u32 xsk_stat = 0, expected_stat = opt_pkt_count;
> + u32 xsk_stat = 0, expected_stat = ifobject->pkt_stream->nb_pkts;
> struct xsk_socket *xsk = ifobject->xsk->xsk;
> int fd = xsk_socket__fd(xsk);
> struct xdp_statistics stats;
> @@ -708,11 +737,11 @@ static void tx_stats_validate(struct ifobject *ifobject)
> return;
> }
>
> - if (stats.tx_invalid_descs == opt_pkt_count)
> + if (stats.tx_invalid_descs == ifobject->pkt_stream->nb_pkts)
> return;
>
> ksft_test_result_fail("ERROR: [%s] tx_invalid_descs incorrect. Got [%u] expected [%u]\n",
> - __func__, stats.tx_invalid_descs, opt_pkt_count);
> + __func__, stats.tx_invalid_descs, ifobject->pkt_stream->nb_pkts);
> }
>
> static void thread_common_ops(struct ifobject *ifobject, void *bufs)
> @@ -781,8 +810,9 @@ static void *worker_testapp_validate_tx(void *arg)
> if (!second_step)
> thread_common_ops(ifobject, bufs);
>
> - print_verbose("Sending %d packets on interface %s\n", opt_pkt_count, ifobject->ifname);
> - tx_only_all(ifobject);
> + print_verbose("Sending %d packets on interface %s\n", ifobject->pkt_stream->nb_pkts,
> + ifobject->ifname);
> + send_pkts(ifobject);
>
> if (stat_test_type == STAT_TEST_TX_INVALID)
> tx_stats_validate(ifobject);
> @@ -808,19 +838,11 @@ static void *worker_testapp_validate_rx(void *arg)
>
> pthread_barrier_wait(&barr);
>
> - while (1) {
> - if (test_type != TEST_TYPE_STATS) {
> - rx_pkt(ifobject->xsk, fds);
> - } else {
> - if (rx_stats_are_valid(ifobject))
> - break;
> - }
> - if (sigvar)
> - break;
> - }
> -
> - print_verbose("Received %d packets on interface %s\n",
> - pkt_counter, ifobject->ifname);
> + if (test_type == TEST_TYPE_STATS)
> + while (!rx_stats_are_valid(ifobject))
> + continue;
> + else
> + receive_pkts(ifobject->pkt_stream, ifobject->xsk, fds);
>
> if (test_type == TEST_TYPE_TEARDOWN)
> print_verbose("Destroying socket\n");
> @@ -833,10 +855,20 @@ static void testapp_validate(void)
> {
> 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))
> exit_with_error(errno);
>
> + if (stat_test_type == STAT_TEST_TX_INVALID) {
> + pkt_stream = pkt_stream_generate(DEFAULT_PKT_CNT,
> + XSK_UMEM__DEFAULT_FRAME_SIZE + 1);
nit: maybe add:
#define XSK_UMEM__INVALID_FRAME_SIZE (XSK_UMEM__DEFAULT_FRAME_SIZE + 1)
and use it above. Again, not a big deal.
Also wondering if we imagine having a different frame size in umem than
4k? Then it would make sense to pass it as a size. Currently
pkt_stream_generate has hard coded XSK_UMEM__DEFAULT_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;
> +
> /*Spawn RX thread */
> pthread_create(&t0, NULL, ifdict_rx->func_ptr, ifdict_rx);
>
> @@ -859,8 +891,6 @@ static void testapp_teardown(void)
> int i;
>
> for (i = 0; i < MAX_TEARDOWN_ITER; i++) {
> - pkt_counter = 0;
> - sigvar = 0;
> print_verbose("Creating socket\n");
> testapp_validate();
> }
> @@ -886,8 +916,6 @@ static void swap_vectors(struct ifobject *ifobj1, struct ifobject *ifobj2)
> static void testapp_bidi(void)
> {
> for (int i = 0; i < MAX_BIDI_ITER; i++) {
> - pkt_counter = 0;
> - sigvar = 0;
> print_verbose("Creating socket\n");
> testapp_validate();
> if (!second_step) {
> @@ -919,8 +947,6 @@ static void testapp_bpf_res(void)
> int i;
>
> for (i = 0; i < MAX_BPF_ITER; i++) {
> - pkt_counter = 0;
> - sigvar = 0;
> print_verbose("Creating socket\n");
> testapp_validate();
> if (!second_step)
> @@ -948,6 +974,8 @@ static void testapp_stats(void)
> case STAT_TEST_RX_FULL:
> rxqsize = RX_FULL_RXQSIZE;
> break;
> + case STAT_TEST_TX_INVALID:
> + continue;
> default:
> break;
> }
> @@ -993,9 +1021,7 @@ static void run_pkt_test(int mode, int type)
>
> /* reset defaults after potential previous test */
> xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> - pkt_counter = 0;
> second_step = 0;
> - sigvar = 0;
> stat_test_type = -1;
> rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> index b3087270d837..f5b46cd3d8df 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.h
> +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> @@ -74,13 +74,10 @@ static u32 num_frames = DEFAULT_PKT_CNT / 4;
> static bool second_step;
> static int test_type;
>
> -static u32 opt_pkt_count = DEFAULT_PKT_CNT;
> static u8 opt_verbose;
>
> static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> static u32 xdp_bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
> -static u32 pkt_counter;
> -static int sigvar;
> static int stat_test_type;
> static u32 rxqsize;
> static u32 frame_headroom;
> @@ -107,6 +104,17 @@ struct flow_vector {
> } vector;
> };
>
> +struct pkt {
> + u64 addr;
> + u32 len;
> + u32 payload;
> +};
> +
> +struct pkt_stream {
> + u32 nb_pkts;
> + struct pkt *pkts;
> +};
> +
> struct ifobject {
> char ifname[MAX_INTERFACE_NAME_CHARS];
> char nsname[MAX_INTERFACES_NAMESPACE_CHARS];
> @@ -116,6 +124,7 @@ struct ifobject {
> struct xsk_umem_info *umem;
> void *(*func_ptr)(void *arg);
> struct flow_vector fv;
> + struct pkt_stream *pkt_stream;
> int ns_fd;
> int ifdict_index;
> u32 dst_ip;
> --
> 2.29.0
>
^ permalink raw reply [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next v2 14/16] selftests: xsk: generate packets from specification
2021-08-19 10:10 ` Maciej Fijalkowski
@ 2021-08-19 10:38 ` Magnus Karlsson
0 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-19 10:38 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 Thu, Aug 19, 2021 at 12:26 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Tue, Aug 17, 2021 at 11:27:27AM +0200, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Generate packets from a specification instead of something hard
> > coded. The idea is that a test generates one or more packet
> > specifications and provides it/them to both Tx and Rx. The Tx thread
> > will generate from this specification and Rx will validate that it
> > receives what is in the specification. The specification can be the
> > same on both ends, meaning that everything that was sent should be
> > received, or different which means that Rx will only receive part of
> > the sent packets.
> >
> > Currently, the packet specification is the same for both Rx and Tx and
> > the same for each test. This will change in later patches as features
> > and tests are added.
>
> You probably meant 'later work' by itself, by saying 'later patches' I
> would expect it to be in this same set. But that's not a big deal to me.
There is going to be a v3 so might as well fix this too :-).
> >
> > The data path functions are also renamed to better reflect what
> > actions they are performing after introducing this feature.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> > tools/testing/selftests/bpf/xdpxceiver.c | 254 +++++++++++++----------
> > tools/testing/selftests/bpf/xdpxceiver.h | 15 +-
> > 2 files changed, 152 insertions(+), 117 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> > index a62e5ebb0f2c..4160ba5f50a3 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.c
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> > @@ -417,18 +417,52 @@ static void parse_command_line(int argc, char **argv)
> > }
> > }
> >
> > -static void pkt_generate(struct ifobject *ifobject, u32 pkt_nb, u64 addr)
> > +static struct pkt *pkt_stream_get_pkt(struct pkt_stream *pkt_stream, u32 pkt_nb)
> > {
> > - void *data = xsk_umem__get_data(ifobject->umem->buffer, addr);
> > + return &pkt_stream->pkts[pkt_nb];
> > +}
> > +
> > +static struct pkt_stream *pkt_stream_generate(u32 nb_pkts, u32 pkt_len)
> > +{
> > + struct pkt_stream *pkt_stream;
> > + u32 i;
> > +
> > + pkt_stream = malloc(sizeof(*pkt_stream));
> > + if (!pkt_stream)
> > + exit_with_error(ENOMEM);
> > +
> > + pkt_stream->pkts = calloc(DEFAULT_PKT_CNT, sizeof(*pkt_stream->pkts));
>
> Why not nb_pkts as a first arg?
Good catch. Will fix!
> > + if (!pkt_stream->pkts)
> > + exit_with_error(ENOMEM);
> > +
> > + 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].len = pkt_len;
> > + pkt_stream->pkts[i].payload = i;
> > + }
> > +
> > + return 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);
> > + void *data = xsk_umem__get_data(ifobject->umem->buffer, pkt->addr);
> > struct udphdr *udp_hdr =
> > (struct udphdr *)(data + sizeof(struct ethhdr) + sizeof(struct iphdr));
> > struct iphdr *ip_hdr = (struct iphdr *)(data + sizeof(struct ethhdr));
> > struct ethhdr *eth_hdr = (struct ethhdr *)data;
> >
> > + if (pkt_nb >= ifobject->pkt_stream->nb_pkts)
> > + return NULL;
>
> shouldn't this check be within pkt_stream_get_pkt ?
This might make things easier to understand in other parts. Will try
it out. Requires changing the code that uses pkt_stream_get_pkt.
> > +
> > gen_udp_hdr(pkt_nb, data, ifobject, udp_hdr);
> > gen_ip_hdr(ifobject, ip_hdr);
> > gen_udp_csum(udp_hdr, ip_hdr);
> > gen_eth_hdr(ifobject, eth_hdr);
> > +
> > + return pkt;
> > }
> >
> > static void pkt_dump(void *pkt, u32 len)
> > @@ -468,33 +502,38 @@ static void pkt_dump(void *pkt, u32 len)
> > fprintf(stdout, "---------------------------------------\n");
> > }
> >
> > -static void pkt_validate(void *buffer, u64 addr)
> > +static bool pkt_validate(struct pkt *pkt, void *buffer, const struct xdp_desc *desc)
>
> Now maybe a better name (because of bool) would be is_pkt_valid ?
True. Will fix.
> > {
> > - void *data = xsk_umem__get_data(buffer, addr);
> > + void *data = xsk_umem__get_data(buffer, desc->addr);
> > struct iphdr *iphdr = (struct iphdr *)(data + sizeof(struct ethhdr));
> >
> > if (iphdr->version == IP_PKT_VER && iphdr->tos == IP_PKT_TOS) {
> > u32 seqnum = ntohl(*((u32 *)(data + PKT_HDR_SIZE)));
> > - u32 expected_seqnum = pkt_counter % num_frames;
> >
> > if (debug_pkt_dump && test_type != TEST_TYPE_STATS)
> > pkt_dump(data, PKT_SIZE);
> >
> > - if (expected_seqnum != seqnum) {
> > + if (pkt->len != desc->len) {
> > ksft_test_result_fail
> > - ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
> > - __func__, expected_seqnum, seqnum);
> > - sigvar = 1;
> > + ("ERROR: [%s] expected length [%d], got length [%d]\n",
> > + __func__, pkt->len, desc->len);
> > + return false;
> > }
> >
> > - if (++pkt_counter == opt_pkt_count)
> > - sigvar = 1;
> > + if (pkt->payload != seqnum) {
> > + ksft_test_result_fail
> > + ("ERROR: [%s] expected seqnum [%d], got seqnum [%d]\n",
> > + __func__, pkt->payload, seqnum);
> > + return false;
> > + }
> > } else {
> > ksft_print_msg("Invalid frame received: ");
> > ksft_print_msg("[IP_PKT_VER: %02X], [IP_PKT_TOS: %02X]\n", iphdr->version,
> > iphdr->tos);
> > - sigvar = 1;
> > + return false;
> > }
> > +
> > + return true;
> > }
> >
> > static void kick_tx(struct xsk_socket_info *xsk)
> > @@ -507,7 +546,7 @@ static void kick_tx(struct xsk_socket_info *xsk)
> > exit_with_error(errno);
> > }
> >
> > -static void complete_tx_only(struct xsk_socket_info *xsk, int batch_size)
> > +static void complete_pkts(struct xsk_socket_info *xsk, int batch_size)
> > {
> > unsigned int rcvd;
> > u32 idx;
> > @@ -525,116 +564,106 @@ static void complete_tx_only(struct xsk_socket_info *xsk, int batch_size)
> > }
> > }
> >
> > -static void rx_pkt(struct xsk_socket_info *xsk, struct pollfd *fds)
> > +static void receive_pkts(struct pkt_stream *pkt_stream, struct xsk_socket_info *xsk,
> > + struct pollfd *fds)
> > {
> > - unsigned int rcvd, i;
> > - u32 idx_rx = 0, idx_fq = 0;
> > + u32 idx_rx = 0, idx_fq = 0, rcvd, i, pkt_count = 0;
> > int ret;
> >
> > - rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
> > - if (!rcvd) {
> > - if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
> > - ret = poll(fds, 1, POLL_TMOUT);
> > - if (ret < 0)
> > - exit_with_error(-ret);
> > + while (1) {
> > + rcvd = xsk_ring_cons__peek(&xsk->rx, BATCH_SIZE, &idx_rx);
> > + if (!rcvd) {
> > + if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
> > + ret = poll(fds, 1, POLL_TMOUT);
> > + if (ret < 0)
> > + exit_with_error(-ret);
> > + }
> > + continue;
> > }
> > - return;
> > - }
> >
> > - ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
> > - while (ret != rcvd) {
> > - if (ret < 0)
> > - exit_with_error(-ret);
> > - if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
> > - ret = poll(fds, 1, POLL_TMOUT);
> > + ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
> > + while (ret != rcvd) {
> > if (ret < 0)
> > exit_with_error(-ret);
> > + if (xsk_ring_prod__needs_wakeup(&xsk->umem->fq)) {
> > + ret = poll(fds, 1, POLL_TMOUT);
> > + if (ret < 0)
> > + exit_with_error(-ret);
> > + }
> > + ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
> > }
> > - ret = xsk_ring_prod__reserve(&xsk->umem->fq, rcvd, &idx_fq);
> > - }
> >
> > - for (i = 0; i < rcvd; i++) {
> > - u64 addr, orig;
> > + for (i = 0; i < rcvd; i++) {
> > + const struct xdp_desc *desc = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
> > + u64 addr = desc->addr, orig;
> > +
> > + orig = xsk_umem__extract_addr(addr);
> > + addr = xsk_umem__add_offset_to_addr(addr);
> > + if (!pkt_validate(pkt_stream_get_pkt(pkt_stream, pkt_count++),
> > + xsk->umem->buffer, desc))
> > + return;
> >
> > - addr = xsk_ring_cons__rx_desc(&xsk->rx, idx_rx)->addr;
> > - xsk_ring_cons__rx_desc(&xsk->rx, idx_rx++);
> > - orig = xsk_umem__extract_addr(addr);
> > + *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
> > + }
> >
> > - addr = xsk_umem__add_offset_to_addr(addr);
> > - pkt_validate(xsk->umem->buffer, addr);
> > + xsk_ring_prod__submit(&xsk->umem->fq, rcvd);
> > + xsk_ring_cons__release(&xsk->rx, rcvd);
> >
> > - *xsk_ring_prod__fill_addr(&xsk->umem->fq, idx_fq++) = orig;
> > + if (pkt_count >= pkt_stream->nb_pkts)
> > + return;
> > }
> > -
> > - xsk_ring_prod__submit(&xsk->umem->fq, rcvd);
> > - xsk_ring_cons__release(&xsk->rx, rcvd);
> > }
> >
> > -static void tx_only(struct ifobject *ifobject, u32 *frameptr, int batch_size)
> > +static u32 __send_pkts(struct ifobject *ifobject, u32 pkt_nb)
> > {
> > struct xsk_socket_info *xsk = ifobject->xsk;
> > - u32 idx = 0;
> > - unsigned int i;
> > - bool tx_invalid_test = stat_test_type == STAT_TEST_TX_INVALID;
> > - u32 len = tx_invalid_test ? XSK_UMEM__DEFAULT_FRAME_SIZE + 1 : PKT_SIZE;
> > + u32 i, idx;
> >
> > - while (xsk_ring_prod__reserve(&xsk->tx, batch_size, &idx) < batch_size)
> > - complete_tx_only(xsk, batch_size);
> > + while (xsk_ring_prod__reserve(&xsk->tx, BATCH_SIZE, &idx) < BATCH_SIZE)
> > + complete_pkts(xsk, BATCH_SIZE);
> >
> > - for (i = 0; i < batch_size; i++) {
> > + for (i = 0; i < BATCH_SIZE; i++) {
> > struct xdp_desc *tx_desc = xsk_ring_prod__tx_desc(&xsk->tx, idx + i);
> > + struct pkt *pkt = pkt_generate(ifobject, pkt_nb);
> >
> > - tx_desc->addr = (*frameptr + i) << XSK_UMEM__DEFAULT_FRAME_SHIFT;
> > - tx_desc->len = len;
> > - pkt_generate(ifobject, *frameptr + i, tx_desc->addr);
> > - }
> > + if (!pkt)
> > + break;
> >
> > - xsk_ring_prod__submit(&xsk->tx, batch_size);
> > - if (!tx_invalid_test) {
> > - xsk->outstanding_tx += batch_size;
> > - } else if (xsk_ring_prod__needs_wakeup(&xsk->tx)) {
> > - kick_tx(xsk);
> > + tx_desc->addr = pkt->addr;
> > + tx_desc->len = pkt->len;
> > + pkt_nb++;
> > }
> > - *frameptr += batch_size;
> > - *frameptr %= num_frames;
> > - complete_tx_only(xsk, batch_size);
> > -}
> >
> > -static int get_batch_size(int pkt_cnt)
> > -{
> > - if (pkt_cnt + BATCH_SIZE <= opt_pkt_count)
> > - return BATCH_SIZE;
> > + 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);
> >
> > - return opt_pkt_count - pkt_cnt;
> > + return i;
> > }
> >
> > -static void complete_tx_only_all(struct ifobject *ifobject)
> > +static void wait_for_tx_completion(struct xsk_socket_info *xsk)
> > {
> > - bool pending;
> > -
> > - do {
> > - pending = false;
> > - if (ifobject->xsk->outstanding_tx) {
> > - complete_tx_only(ifobject->xsk, BATCH_SIZE);
> > - pending = !!ifobject->xsk->outstanding_tx;
> > - }
> > - } while (pending);
> > + while (xsk->outstanding_tx)
> > + complete_pkts(xsk, BATCH_SIZE);
> > }
> >
> > -static void tx_only_all(struct ifobject *ifobject)
> > +static void send_pkts(struct ifobject *ifobject)
> > {
> > struct pollfd fds[MAX_SOCKS] = { };
> > - u32 frame_nb = 0;
> > - int pkt_cnt = 0;
> > - int ret;
> > + u32 pkt_cnt = 0;
> >
> > fds[0].fd = xsk_socket__fd(ifobject->xsk->xsk);
> > fds[0].events = POLLOUT;
> >
> > - while (pkt_cnt < opt_pkt_count) {
> > - int batch_size = get_batch_size(pkt_cnt);
> > + while (pkt_cnt < ifobject->pkt_stream->nb_pkts) {
> > + u32 sent;
> >
> > if (test_type == TEST_TYPE_POLL) {
> > + int ret;
> > +
> > ret = poll(fds, 1, POLL_TMOUT);
> > if (ret <= 0)
> > continue;
> > @@ -643,16 +672,16 @@ static void tx_only_all(struct ifobject *ifobject)
> > continue;
> > }
> >
> > - tx_only(ifobject, &frame_nb, batch_size);
> > - pkt_cnt += batch_size;
> > + sent = __send_pkts(ifobject, pkt_cnt);
> > + pkt_cnt += sent;
> > }
> >
> > - complete_tx_only_all(ifobject);
> > + wait_for_tx_completion(ifobject->xsk);
> > }
> >
> > static bool rx_stats_are_valid(struct ifobject *ifobject)
> > {
> > - u32 xsk_stat = 0, expected_stat = opt_pkt_count;
> > + u32 xsk_stat = 0, expected_stat = ifobject->pkt_stream->nb_pkts;
> > struct xsk_socket *xsk = ifobject->xsk->xsk;
> > int fd = xsk_socket__fd(xsk);
> > struct xdp_statistics stats;
> > @@ -708,11 +737,11 @@ static void tx_stats_validate(struct ifobject *ifobject)
> > return;
> > }
> >
> > - if (stats.tx_invalid_descs == opt_pkt_count)
> > + if (stats.tx_invalid_descs == ifobject->pkt_stream->nb_pkts)
> > return;
> >
> > ksft_test_result_fail("ERROR: [%s] tx_invalid_descs incorrect. Got [%u] expected [%u]\n",
> > - __func__, stats.tx_invalid_descs, opt_pkt_count);
> > + __func__, stats.tx_invalid_descs, ifobject->pkt_stream->nb_pkts);
> > }
> >
> > static void thread_common_ops(struct ifobject *ifobject, void *bufs)
> > @@ -781,8 +810,9 @@ static void *worker_testapp_validate_tx(void *arg)
> > if (!second_step)
> > thread_common_ops(ifobject, bufs);
> >
> > - print_verbose("Sending %d packets on interface %s\n", opt_pkt_count, ifobject->ifname);
> > - tx_only_all(ifobject);
> > + print_verbose("Sending %d packets on interface %s\n", ifobject->pkt_stream->nb_pkts,
> > + ifobject->ifname);
> > + send_pkts(ifobject);
> >
> > if (stat_test_type == STAT_TEST_TX_INVALID)
> > tx_stats_validate(ifobject);
> > @@ -808,19 +838,11 @@ static void *worker_testapp_validate_rx(void *arg)
> >
> > pthread_barrier_wait(&barr);
> >
> > - while (1) {
> > - if (test_type != TEST_TYPE_STATS) {
> > - rx_pkt(ifobject->xsk, fds);
> > - } else {
> > - if (rx_stats_are_valid(ifobject))
> > - break;
> > - }
> > - if (sigvar)
> > - break;
> > - }
> > -
> > - print_verbose("Received %d packets on interface %s\n",
> > - pkt_counter, ifobject->ifname);
> > + if (test_type == TEST_TYPE_STATS)
> > + while (!rx_stats_are_valid(ifobject))
> > + continue;
> > + else
> > + receive_pkts(ifobject->pkt_stream, ifobject->xsk, fds);
> >
> > if (test_type == TEST_TYPE_TEARDOWN)
> > print_verbose("Destroying socket\n");
> > @@ -833,10 +855,20 @@ static void testapp_validate(void)
> > {
> > 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))
> > exit_with_error(errno);
> >
> > + if (stat_test_type == STAT_TEST_TX_INVALID) {
> > + pkt_stream = pkt_stream_generate(DEFAULT_PKT_CNT,
> > + XSK_UMEM__DEFAULT_FRAME_SIZE + 1);
>
> nit: maybe add:
> #define XSK_UMEM__INVALID_FRAME_SIZE (XSK_UMEM__DEFAULT_FRAME_SIZE + 1)
> and use it above. Again, not a big deal.
Good idea.
> Also wondering if we imagine having a different frame size in umem than
> 4k? Then it would make sense to pass it as a size. Currently
> pkt_stream_generate has hard coded XSK_UMEM__DEFAULT_FRAME_SIZE.
Yes, I see that we would want to test that too in the future. But let
that future patch set fix that.
> > + } else {
> > + pkt_stream = pkt_stream_generate(DEFAULT_PKT_CNT, PKT_SIZE);
> > + }
> > + ifdict_tx->pkt_stream = pkt_stream;
> > + ifdict_rx->pkt_stream = pkt_stream;
> > +
> > /*Spawn RX thread */
> > pthread_create(&t0, NULL, ifdict_rx->func_ptr, ifdict_rx);
> >
> > @@ -859,8 +891,6 @@ static void testapp_teardown(void)
> > int i;
> >
> > for (i = 0; i < MAX_TEARDOWN_ITER; i++) {
> > - pkt_counter = 0;
> > - sigvar = 0;
> > print_verbose("Creating socket\n");
> > testapp_validate();
> > }
> > @@ -886,8 +916,6 @@ static void swap_vectors(struct ifobject *ifobj1, struct ifobject *ifobj2)
> > static void testapp_bidi(void)
> > {
> > for (int i = 0; i < MAX_BIDI_ITER; i++) {
> > - pkt_counter = 0;
> > - sigvar = 0;
> > print_verbose("Creating socket\n");
> > testapp_validate();
> > if (!second_step) {
> > @@ -919,8 +947,6 @@ static void testapp_bpf_res(void)
> > int i;
> >
> > for (i = 0; i < MAX_BPF_ITER; i++) {
> > - pkt_counter = 0;
> > - sigvar = 0;
> > print_verbose("Creating socket\n");
> > testapp_validate();
> > if (!second_step)
> > @@ -948,6 +974,8 @@ static void testapp_stats(void)
> > case STAT_TEST_RX_FULL:
> > rxqsize = RX_FULL_RXQSIZE;
> > break;
> > + case STAT_TEST_TX_INVALID:
> > + continue;
> > default:
> > break;
> > }
> > @@ -993,9 +1021,7 @@ static void run_pkt_test(int mode, int type)
> >
> > /* reset defaults after potential previous test */
> > xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> > - pkt_counter = 0;
> > second_step = 0;
> > - sigvar = 0;
> > stat_test_type = -1;
> > rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> > frame_headroom = XSK_UMEM__DEFAULT_FRAME_HEADROOM;
> > diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> > index b3087270d837..f5b46cd3d8df 100644
> > --- a/tools/testing/selftests/bpf/xdpxceiver.h
> > +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> > @@ -74,13 +74,10 @@ static u32 num_frames = DEFAULT_PKT_CNT / 4;
> > static bool second_step;
> > static int test_type;
> >
> > -static u32 opt_pkt_count = DEFAULT_PKT_CNT;
> > static u8 opt_verbose;
> >
> > static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> > static u32 xdp_bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
> > -static u32 pkt_counter;
> > -static int sigvar;
> > static int stat_test_type;
> > static u32 rxqsize;
> > static u32 frame_headroom;
> > @@ -107,6 +104,17 @@ struct flow_vector {
> > } vector;
> > };
> >
> > +struct pkt {
> > + u64 addr;
> > + u32 len;
> > + u32 payload;
> > +};
> > +
> > +struct pkt_stream {
> > + u32 nb_pkts;
> > + struct pkt *pkts;
> > +};
> > +
> > struct ifobject {
> > char ifname[MAX_INTERFACE_NAME_CHARS];
> > char nsname[MAX_INTERFACES_NAMESPACE_CHARS];
> > @@ -116,6 +124,7 @@ struct ifobject {
> > struct xsk_umem_info *umem;
> > void *(*func_ptr)(void *arg);
> > struct flow_vector fv;
> > + struct pkt_stream *pkt_stream;
> > int ns_fd;
> > int ifdict_index;
> > u32 dst_ip;
> > --
> > 2.29.0
> >
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH bpf-next v2 15/16] selftests: xsk: make enums lower case
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (13 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 14/16] selftests: xsk: generate packets from specification Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-17 9:27 ` [PATCH bpf-next v2 16/16] selftests: xsk: preface options with opt Magnus Karlsson
2021-08-19 10:13 ` [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Maciej Fijalkowski
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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 enums lower case as that is the standard. Also drop the
unnecessary TEST_MODE_UNCONFIGURED mode.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 11 +++--------
tools/testing/selftests/bpf/xdpxceiver.h | 9 ++++-----
2 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 4160ba5f50a3..3c41dc23b97e 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -105,14 +105,9 @@ static const u16 UDP_PORT2 = 2121;
static void __exit_with_error(int error, const char *file, const char *func, int line)
{
- if (configured_mode == TEST_MODE_UNCONFIGURED) {
- ksft_exit_fail_msg
- ("[%s:%s:%i]: ERROR: %d/\"%s\"\n", file, func, line, error, strerror(error));
- } else {
- ksft_test_result_fail
- ("[%s:%s:%i]: ERROR: %d/\"%s\"\n", file, func, line, error, strerror(error));
- ksft_exit_xfail();
- }
+ ksft_test_result_fail("[%s:%s:%i]: ERROR: %d/\"%s\"\n", file, func, line, error,
+ strerror(error));
+ ksft_exit_xfail();
}
#define exit_with_error(error) __exit_with_error(error, __FILE__, __func__, __LINE__)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index f5b46cd3d8df..309a580fd1c5 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -43,14 +43,13 @@
#define print_verbose(x...) do { if (opt_verbose) ksft_print_msg(x); } while (0)
-enum TEST_MODES {
- TEST_MODE_UNCONFIGURED = -1,
+enum test_mode {
TEST_MODE_SKB,
TEST_MODE_DRV,
TEST_MODE_MAX
};
-enum TEST_TYPES {
+enum test_type {
TEST_TYPE_NOPOLL,
TEST_TYPE_POLL,
TEST_TYPE_TEARDOWN,
@@ -60,7 +59,7 @@ enum TEST_TYPES {
TEST_TYPE_MAX
};
-enum STAT_TEST_TYPES {
+enum stat_test_type {
STAT_TEST_RX_DROPPED,
STAT_TEST_TX_INVALID,
STAT_TEST_RX_FULL,
@@ -68,7 +67,7 @@ enum STAT_TEST_TYPES {
STAT_TEST_TYPE_MAX
};
-static int configured_mode = TEST_MODE_UNCONFIGURED;
+static int configured_mode;
static u8 debug_pkt_dump;
static u32 num_frames = DEFAULT_PKT_CNT / 4;
static bool second_step;
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* [PATCH bpf-next v2 16/16] selftests: xsk: preface options with opt
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (14 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 15/16] selftests: xsk: make enums lower case Magnus Karlsson
@ 2021-08-17 9:27 ` Magnus Karlsson
2021-08-19 10:13 ` [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Maciej Fijalkowski
16 siblings, 0 replies; 24+ messages in thread
From: Magnus Karlsson @ 2021-08-17 9:27 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>
Preface all options with opt_ and make them booleans.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xdpxceiver.c | 6 +++---
tools/testing/selftests/bpf/xdpxceiver.h | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
index 3c41dc23b97e..2c1da060f832 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.c
+++ b/tools/testing/selftests/bpf/xdpxceiver.c
@@ -395,10 +395,10 @@ static void parse_command_line(int argc, char **argv)
interface_index++;
break;
case 'D':
- debug_pkt_dump = 1;
+ opt_pkt_dump = true;
break;
case 'v':
- opt_verbose = 1;
+ opt_verbose = true;
break;
default:
usage(basename(argv[0]));
@@ -505,7 +505,7 @@ static bool pkt_validate(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 (debug_pkt_dump && test_type != TEST_TYPE_STATS)
+ if (opt_pkt_dump && test_type != TEST_TYPE_STATS)
pkt_dump(data, PKT_SIZE);
if (pkt->len != desc->len) {
diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
index 309a580fd1c5..664b74ac2233 100644
--- a/tools/testing/selftests/bpf/xdpxceiver.h
+++ b/tools/testing/selftests/bpf/xdpxceiver.h
@@ -68,12 +68,12 @@ enum stat_test_type {
};
static int configured_mode;
-static u8 debug_pkt_dump;
+static bool opt_pkt_dump;
static u32 num_frames = DEFAULT_PKT_CNT / 4;
static bool second_step;
static int test_type;
-static u8 opt_verbose;
+static bool opt_verbose;
static u32 xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
static u32 xdp_bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
--
2.29.0
^ permalink raw reply related [flat|nested] 24+ messages in thread* Re: [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications
2021-08-17 9:27 [PATCH bpf-next v2 00/16] selftests: xsk: various simplifications Magnus Karlsson
` (15 preceding siblings ...)
2021-08-17 9:27 ` [PATCH bpf-next v2 16/16] selftests: xsk: preface options with opt Magnus Karlsson
@ 2021-08-19 10:13 ` Maciej Fijalkowski
16 siblings, 0 replies; 24+ messages in thread
From: Maciej Fijalkowski @ 2021-08-19 10:13 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, jonathan.lemon,
ciara.loftus, bpf, yhs, andrii
On Tue, Aug 17, 2021 at 11:27:13AM +0200, Magnus Karlsson wrote:
> This patch set mainly contains various simplifications to the xsk
> selftests. The only exception is the introduction of packet streams
> that describes what the Tx process should send and what the Rx process
> should receive. If it receives anything else, the test fails. This
> mechanism can be used to produce tests were all packets are not
> received by the Rx thread or modified in some way. An example of this
> is if an XDP program does XDP_PASS on some of the packets.
>
> This patch set will be followed by another patch set that implements a
> new structure that will facilitate adding new tests. A couple of new
> tests will also be included in that patch set.
>
> v1 -> v2:
>
> * Dropped the patch with per process limit changes as it is not needed
> [Yonghong]
> * Improved the commit message of patch 1 [Yonghong]
> * Fixed a spelling error in patch 9
>
> Thanks: Magnus
This is awesome work. Lots of simplification and the pkt stream generation
makes sense to me.
I had little comments on patch 14.
Besides that:
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>
> Magnus Karlsson (16):
> selftests: xsk: remove color mode
> selftests: xsk: remove the num_tx_packets option
> selftests: xsk: remove unused variables
> selftests: xsk: return correct error codes
> selftests: xsk: simplify the retry code
> selftests: xsk: remove end-of-test packet
> selftests: xsk: disassociate umem size with packets sent
> selftests: xsk: rename worker_* functions that are not thread entry
> points
> selftests: xsk: simplify packet validation in xsk tests
> selftests: xsk: validate tx stats on tx thread
> selftests: xsk: decrease batch size
> selftests: xsk: remove cleanup at end of program
> selftests: xsk: generate packet directly in umem
> selftests: xsk: generate packets from specification
> selftests: xsk: make enums lower case
> selftests: xsk: preface options with opt
>
> tools/testing/selftests/bpf/test_xsk.sh | 10 +-
> tools/testing/selftests/bpf/xdpxceiver.c | 626 +++++++++------------
> tools/testing/selftests/bpf/xdpxceiver.h | 61 +-
> tools/testing/selftests/bpf/xsk_prereqs.sh | 30 +-
> 4 files changed, 310 insertions(+), 417 deletions(-)
>
>
> base-commit: 3c3bd542ffbb2ac09631313ede46ae66660ae550
> --
> 2.29.0
^ permalink raw reply [flat|nested] 24+ messages in thread