* [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs
@ 2023-01-04 12:17 Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 01/15] selftests/xsk: print correct payload for packet dump Magnus Karlsson
` (15 more replies)
0 siblings, 16 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: Magnus Karlsson, jonathan.lemon
This is a patch set of various performance improvements, fixes and the
introduction of more than one XDP program to the xsk selftests
framework so we can test more things in the future such as upcoming
multi-buffer and metadata support for AF_XDP. The new programs just
reuses the framework that all the other eBPF selftests use. The new
feature is used to implement one new test that does XDP_DROP on every
other packet. More tests using this will be added in future commits.
Contents:
* The run-time of the test suite is cut by 10x when executing the
tests on a real NIC, by only attaching the XDP program once per mode
tested, instead of once per test program.
* Over 700 lines of code have been removed. The xsk.c control file was
moved straight over from libbpf when the xsk support was deprecated
there. As it is now not used as library code that has to work with
all kinds of versions of Linux, a lot of code could be dropped or
simplified.
* Add a new command line option "-d" that can be used when a test
fails and you want to debug it with gdb or some other debugger. The
option creates the two veth netdevs and prints them to the screen
without deleting them afterwards. This way these veth netdevs can be
used when running xskxceiver in a debugger.
* Implemented the possibility to load external XDP programs so we can
have more than the default one. This feature is used to implement a
test where every other packet is dropped. Good exercise for the
recycling mechanism of the xsk buffer pool used in zero-copy mode.
* Various clean-ups and small fixes in patches 1 to 5. None of these
fixes has any impact on the correct execution of the tests when they
pass, though they can be irritating when a test fails. IMHO, they do
not need to go to bpf as they will not fix anything there. The first
version of patches 1, 2, and 4 where previously sent to bpf, but has
now been included here.
v1 -> v2:
* Fixed spelling error in commit message of patch #6 [Björn]
* Added explanation on why it is safe to use C11 atomics in patch #7
[Daniel]
* Put all XDP programs in the same file so that adding more XDP
programs to xskxceiver.c becomes more scalable in patches #11 and
#12 [Maciej]
* Removed more dead code in patch #8 [Maciej]
* Removed stale %s specifier in error print, patch #9 [Maciej]
* Changed name of XDP_CONSUMES_SOME_PACKETS to XDP_DROP_HALF to
hopefully make it clearer [Maciej]
* ifobj_rx and ifobj_tx name changes in patch #13 [Maciej]
* Simplified XDP attachment code in patch #15 [Maciej]
Patches:
1-5: Small fixes and clean-ups
6: New convenient debug option when using a debugger such as gdb
7-8: Removal of unnecessary code
9: Add the ability to load external XDP programs
10-11: Removal of more unnecessary code
12: Implement a new test where every other packet is XDP_DROP:ed
13: Unify the thread dispatching code
14-15: Simplify the way tests are written when using custom packet_streams
or custom XDP programs
Thanks: Magnus
Magnus Karlsson (15):
selftests/xsk: print correct payload for packet dump
selftests/xsk: do not close unused file descriptors
selftests/xsk: submit correct number of frames in populate_fill_ring
selftests/xsk: print correct error codes when exiting
selftests/xsk: remove unused variable outstanding_tx
selftests/xsk: add debug option for creating netdevs
selftests/xsk: replace asm acquire/release implementations
selftests/xsk: remove namespaces
selftests/xsk: load and attach XDP program only once per mode
selftests/xsk: remove unnecessary code in control path
selftests/xsk: get rid of built-in XDP program
selftests/xsk: add test when some packets are XDP_DROPed
selftests/xsk: merge dual and single thread dispatchers
selftests/xsk: automatically restore packet stream
selftests/xsk: automatically switch XDP programs
tools/testing/selftests/bpf/Makefile | 2 +-
.../selftests/bpf/progs/xsk_xdp_progs.c | 30 +
tools/testing/selftests/bpf/test_xsk.sh | 42 +-
tools/testing/selftests/bpf/xsk.c | 674 +-----------------
tools/testing/selftests/bpf/xsk.h | 97 +--
tools/testing/selftests/bpf/xsk_prereqs.sh | 12 +-
tools/testing/selftests/bpf/xskxceiver.c | 382 +++++-----
tools/testing/selftests/bpf/xskxceiver.h | 17 +-
8 files changed, 308 insertions(+), 948 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
base-commit: bb5747cfbc4b7fe29621ca6cd4a695d2723bf2e8
--
2.34.1
^ permalink raw reply [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 01/15] selftests/xsk: print correct payload for packet dump
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 02/15] selftests/xsk: do not close unused file descriptors Magnus Karlsson
` (14 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Print the correct payload when the packet dump option is selected. The
network to host conversion was forgotten and the payload was
erronously declared to be an int instead of an unsigned int.
Fixes: facb7cb2e909 ("selftests/bpf: Xsk selftests - SKB POLL, NOPOLL")
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xskxceiver.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 162d3a516f2c..2ff43b22180f 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -767,7 +767,7 @@ static void pkt_dump(void *pkt, u32 len)
struct ethhdr *ethhdr;
struct udphdr *udphdr;
struct iphdr *iphdr;
- int payload, i;
+ u32 payload, i;
ethhdr = pkt;
iphdr = pkt + sizeof(*ethhdr);
@@ -792,7 +792,7 @@ static void pkt_dump(void *pkt, u32 len)
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));
+ payload = ntohl(*((u32 *)(pkt + PKT_HDR_SIZE)));
fprintf(stdout, "DEBUG>> L5: payload: %d\n", payload);
fprintf(stdout, "---------------------------------------\n");
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 02/15] selftests/xsk: do not close unused file descriptors
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 01/15] selftests/xsk: print correct payload for packet dump Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 03/15] selftests/xsk: submit correct number of frames in populate_fill_ring Magnus Karlsson
` (13 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Do not close descriptors that have never been used. File descriptor
fields that are not in use are erroneously marked with the number 0,
which is a valid fd. Mark unused fds with -1 instead and do not close
these when deleting the socket.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
---
tools/testing/selftests/bpf/xsk.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/xsk.c b/tools/testing/selftests/bpf/xsk.c
index 39d349509ba4..5e4a6552ed37 100644
--- a/tools/testing/selftests/bpf/xsk.c
+++ b/tools/testing/selftests/bpf/xsk.c
@@ -35,6 +35,8 @@
#include "xsk.h"
#include "bpf_util.h"
+#define FD_NOT_USED (-1)
+
#ifndef SOL_XDP
#define SOL_XDP 283
#endif
@@ -583,6 +585,9 @@ static void xsk_delete_bpf_maps(struct xsk_socket *xsk)
{
struct xsk_ctx *ctx = xsk->ctx;
+ if (ctx->xsks_map_fd == FD_NOT_USED)
+ return;
+
bpf_map_delete_elem(ctx->xsks_map_fd, &ctx->queue_id);
close(ctx->xsks_map_fd);
}
@@ -941,6 +946,9 @@ static struct xsk_ctx *xsk_create_ctx(struct xsk_socket *xsk,
ctx->umem = umem;
ctx->queue_id = queue_id;
bpf_strlcpy(ctx->ifname, ifname, IFNAMSIZ);
+ ctx->prog_fd = FD_NOT_USED;
+ ctx->link_fd = FD_NOT_USED;
+ ctx->xsks_map_fd = FD_NOT_USED;
ctx->fill = fill;
ctx->comp = comp;
@@ -1221,8 +1229,9 @@ void xsk_socket__delete(struct xsk_socket *xsk)
if (ctx->refcount == 1) {
xsk_delete_bpf_maps(xsk);
- close(ctx->prog_fd);
- if (ctx->has_bpf_link)
+ if (ctx->prog_fd != FD_NOT_USED)
+ close(ctx->prog_fd);
+ if (ctx->has_bpf_link && ctx->link_fd != FD_NOT_USED)
close(ctx->link_fd);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 03/15] selftests/xsk: submit correct number of frames in populate_fill_ring
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 01/15] selftests/xsk: print correct payload for packet dump Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 02/15] selftests/xsk: do not close unused file descriptors Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-09 23:01 ` Maciej Fijalkowski
2023-01-04 12:17 ` [PATCH bpf-next v2 04/15] selftests/xsk: print correct error codes when exiting Magnus Karlsson
` (12 subsequent siblings)
15 siblings, 1 reply; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Submit the correct number of frames in the function
xsk_populate_fill_ring(). For the tests that set the flag
use_addr_for_fill, uninitialized buffers were sent to the fill ring
following the correct ones. This has no impact on the tests, since
they only use the ones that were initialized. But for correctnes, this
should be fixed.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xskxceiver.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 2ff43b22180f..a239e975ab66 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1272,7 +1272,7 @@ static void xsk_populate_fill_ring(struct xsk_umem_info *umem, struct pkt_stream
*xsk_ring_prod__fill_addr(&umem->fq, idx++) = addr;
}
- xsk_ring_prod__submit(&umem->fq, buffers_to_fill);
+ xsk_ring_prod__submit(&umem->fq, i);
}
static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 04/15] selftests/xsk: print correct error codes when exiting
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (2 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 03/15] selftests/xsk: submit correct number of frames in populate_fill_ring Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 05/15] selftests/xsk: remove unused variable outstanding_tx Magnus Karlsson
` (11 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Print the correct error codes when exiting the test suite due to some
terminal error. Some of these had a switched sign and some of them
printed zero instead of errno.
Fixes: facb7cb2e909 ("selftests/bpf: Xsk selftests - SKB POLL, NOPOLL")
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xskxceiver.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index a239e975ab66..72578cebfbf7 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -350,7 +350,7 @@ static bool ifobj_zc_avail(struct ifobject *ifobject)
umem = calloc(1, sizeof(struct xsk_umem_info));
if (!umem) {
munmap(bufs, umem_sz);
- exit_with_error(-ENOMEM);
+ exit_with_error(ENOMEM);
}
umem->frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
ret = xsk_configure_umem(umem, bufs, umem_sz);
@@ -936,7 +936,7 @@ static int receive_pkts(struct test_spec *test, struct pollfd *fds)
if (ifobj->use_poll) {
ret = poll(fds, 1, POLL_TMOUT);
if (ret < 0)
- exit_with_error(-ret);
+ exit_with_error(errno);
if (!ret) {
if (!is_umem_valid(test->ifobj_tx))
@@ -963,7 +963,7 @@ static int receive_pkts(struct test_spec *test, struct pollfd *fds)
if (xsk_ring_prod__needs_wakeup(&umem->fq)) {
ret = poll(fds, 1, POLL_TMOUT);
if (ret < 0)
- exit_with_error(-ret);
+ exit_with_error(errno);
}
ret = xsk_ring_prod__reserve(&umem->fq, rcvd, &idx_fq);
}
@@ -1015,7 +1015,7 @@ static int __send_pkts(struct ifobject *ifobject, u32 *pkt_nb, struct pollfd *fd
if (timeout) {
if (ret < 0) {
ksft_print_msg("ERROR: [%s] Poll error %d\n",
- __func__, ret);
+ __func__, errno);
return TEST_FAILURE;
}
if (ret == 0)
@@ -1024,7 +1024,7 @@ static int __send_pkts(struct ifobject *ifobject, u32 *pkt_nb, struct pollfd *fd
}
if (ret <= 0) {
ksft_print_msg("ERROR: [%s] Poll error %d\n",
- __func__, ret);
+ __func__, errno);
return TEST_FAILURE;
}
}
@@ -1323,18 +1323,18 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
if (ifobject->xdp_flags & XDP_FLAGS_SKB_MODE) {
if (opts.attach_mode != XDP_ATTACHED_SKB) {
ksft_print_msg("ERROR: [%s] XDP prog not in SKB mode\n");
- exit_with_error(-EINVAL);
+ exit_with_error(EINVAL);
}
} else if (ifobject->xdp_flags & XDP_FLAGS_DRV_MODE) {
if (opts.attach_mode != XDP_ATTACHED_DRV) {
ksft_print_msg("ERROR: [%s] XDP prog not in DRV mode\n");
- exit_with_error(-EINVAL);
+ exit_with_error(EINVAL);
}
}
ret = xsk_socket__update_xskmap(ifobject->xsk->xsk, ifobject->xsk_map_fd);
if (ret)
- exit_with_error(-ret);
+ exit_with_error(errno);
}
static void *worker_testapp_validate_tx(void *arg)
@@ -1541,7 +1541,7 @@ static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj
ret = xsk_socket__update_xskmap(ifobj_rx->xsk->xsk, ifobj_rx->xsk_map_fd);
if (ret)
- exit_with_error(-ret);
+ exit_with_error(errno);
}
static void testapp_bpf_res(struct test_spec *test)
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 05/15] selftests/xsk: remove unused variable outstanding_tx
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (3 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 04/15] selftests/xsk: print correct error codes when exiting Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 06/15] selftests/xsk: add debug option for creating netdevs Magnus Karlsson
` (10 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Remove the unused variable outstanding_tx.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xsk.c | 2 --
1 file changed, 2 deletions(-)
diff --git a/tools/testing/selftests/bpf/xsk.c b/tools/testing/selftests/bpf/xsk.c
index 5e4a6552ed37..b166edfff86d 100644
--- a/tools/testing/selftests/bpf/xsk.c
+++ b/tools/testing/selftests/bpf/xsk.c
@@ -86,7 +86,6 @@ struct xsk_ctx {
struct xsk_socket {
struct xsk_ring_cons *rx;
struct xsk_ring_prod *tx;
- __u64 outstanding_tx;
struct xsk_ctx *ctx;
struct xsk_socket_config config;
int fd;
@@ -1021,7 +1020,6 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
if (err)
goto out_xsk_alloc;
- xsk->outstanding_tx = 0;
ifindex = if_nametoindex(ifname);
if (!ifindex) {
err = -errno;
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 06/15] selftests/xsk: add debug option for creating netdevs
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (4 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 05/15] selftests/xsk: remove unused variable outstanding_tx Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 07/15] selftests/xsk: replace asm acquire/release implementations Magnus Karlsson
` (9 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Add a new option to the test_xsk.sh script that only creates the two
veth netdevs and the extra namespace, then exits without running any
tests. The failed test can then be executed in the debugger without
having to create the netdevs and namespace manually. For ease-of-use,
the veth netdevs to use are printed so they can be copied into the
debugger.
Here is an example how to use it:
> sudo ./test_xsk.sh -d
veth10 veth11
> gdb xskxceiver
In gdb:
run -i veth10 -i veth11
And now the test cases can be debugged with gdb.
If you want to debug the test suite on a real NIC in loopback mode,
there is no need to use this feature as you already know the netdev of
your NIC.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/test_xsk.sh | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index d821fd098504..cb315d85148b 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -74,6 +74,9 @@
# Run and dump packet contents:
# sudo ./test_xsk.sh -D
#
+# Set up veth interfaces and leave them up so xskxceiver can be launched in a debugger:
+# sudo ./test_xsk.sh -d
+#
# Run test suite for physical device in loopback mode
# sudo ./test_xsk.sh -i IFACE
@@ -81,11 +84,12 @@
ETH=""
-while getopts "vDi:" flag
+while getopts "vDi:d" flag
do
case "${flag}" in
v) verbose=1;;
D) dump_pkts=1;;
+ d) debug=1;;
i) ETH=${OPTARG};;
esac
done
@@ -174,6 +178,11 @@ statusList=()
TEST_NAME="XSK_SELFTESTS_${VETH0}_SOFTIRQ"
+if [[ $debug -eq 1 ]]; then
+ echo "-i" ${VETH0} "-i" ${VETH1},${NS1}
+ exit
+fi
+
exec_xskxceiver
if [ -z $ETH ]; then
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 07/15] selftests/xsk: replace asm acquire/release implementations
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (5 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 06/15] selftests/xsk: add debug option for creating netdevs Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 08/15] selftests/xsk: remove namespaces Magnus Karlsson
` (8 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Replace our own homegrown assembly store/release and load/acquire
implementations with the HW agnositic atomic APIs C11 offers. This to
make the code more portable, easier to read, and reduce the
maintenance burden.
The original code used load-acquire and store-release barriers
hand-coded in assembly. Since C11, these kind of operations are
offered as built-ins in gcc and llvm. The load-acquire operation
prevents hoisting of non-atomic memory operations to before this
operation and it corresponds to the __ATOMIC_ACQUIRE operation in the
built-in atomics. The store-release operation prevents hoisting of
non-atomic memory operations to after this operation and it
corresponds to the __ATOMIC_RELEASE operation in the built-in atomics.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xsk.h | 80 ++-----------------------------
1 file changed, 4 insertions(+), 76 deletions(-)
diff --git a/tools/testing/selftests/bpf/xsk.h b/tools/testing/selftests/bpf/xsk.h
index 997723b0bfb2..24ee765aded3 100644
--- a/tools/testing/selftests/bpf/xsk.h
+++ b/tools/testing/selftests/bpf/xsk.h
@@ -23,77 +23,6 @@
extern "C" {
#endif
-/* This whole API has been deprecated and moved to libxdp that can be found at
- * https://github.com/xdp-project/xdp-tools. The APIs are exactly the same so
- * it should just be linking with libxdp instead of libbpf for this set of
- * functionality. If not, please submit a bug report on the aforementioned page.
- */
-
-/* Load-Acquire Store-Release barriers used by the XDP socket
- * library. The following macros should *NOT* be considered part of
- * the xsk.h API, and is subject to change anytime.
- *
- * LIBRARY INTERNAL
- */
-
-#define __XSK_READ_ONCE(x) (*(volatile typeof(x) *)&x)
-#define __XSK_WRITE_ONCE(x, v) (*(volatile typeof(x) *)&x) = (v)
-
-#if defined(__i386__) || defined(__x86_64__)
-# define libbpf_smp_store_release(p, v) \
- do { \
- asm volatile("" : : : "memory"); \
- __XSK_WRITE_ONCE(*p, v); \
- } while (0)
-# define libbpf_smp_load_acquire(p) \
- ({ \
- typeof(*p) ___p1 = __XSK_READ_ONCE(*p); \
- asm volatile("" : : : "memory"); \
- ___p1; \
- })
-#elif defined(__aarch64__)
-# define libbpf_smp_store_release(p, v) \
- asm volatile ("stlr %w1, %0" : "=Q" (*p) : "r" (v) : "memory")
-# define libbpf_smp_load_acquire(p) \
- ({ \
- typeof(*p) ___p1; \
- asm volatile ("ldar %w0, %1" \
- : "=r" (___p1) : "Q" (*p) : "memory"); \
- ___p1; \
- })
-#elif defined(__riscv)
-# define libbpf_smp_store_release(p, v) \
- do { \
- asm volatile ("fence rw,w" : : : "memory"); \
- __XSK_WRITE_ONCE(*p, v); \
- } while (0)
-# define libbpf_smp_load_acquire(p) \
- ({ \
- typeof(*p) ___p1 = __XSK_READ_ONCE(*p); \
- asm volatile ("fence r,rw" : : : "memory"); \
- ___p1; \
- })
-#endif
-
-#ifndef libbpf_smp_store_release
-#define libbpf_smp_store_release(p, v) \
- do { \
- __sync_synchronize(); \
- __XSK_WRITE_ONCE(*p, v); \
- } while (0)
-#endif
-
-#ifndef libbpf_smp_load_acquire
-#define libbpf_smp_load_acquire(p) \
- ({ \
- typeof(*p) ___p1 = __XSK_READ_ONCE(*p); \
- __sync_synchronize(); \
- ___p1; \
- })
-#endif
-
-/* LIBRARY INTERNAL -- END */
-
/* Do not access these members directly. Use the functions below. */
#define DEFINE_XSK_RING(name) \
struct name { \
@@ -168,7 +97,7 @@ static inline __u32 xsk_prod_nb_free(struct xsk_ring_prod *r, __u32 nb)
* this function. Without this optimization it whould have been
* free_entries = r->cached_prod - r->cached_cons + r->size.
*/
- r->cached_cons = libbpf_smp_load_acquire(r->consumer);
+ r->cached_cons = __atomic_load_n(r->consumer, __ATOMIC_ACQUIRE);
r->cached_cons += r->size;
return r->cached_cons - r->cached_prod;
@@ -179,7 +108,7 @@ static inline __u32 xsk_cons_nb_avail(struct xsk_ring_cons *r, __u32 nb)
__u32 entries = r->cached_prod - r->cached_cons;
if (entries == 0) {
- r->cached_prod = libbpf_smp_load_acquire(r->producer);
+ r->cached_prod = __atomic_load_n(r->producer, __ATOMIC_ACQUIRE);
entries = r->cached_prod - r->cached_cons;
}
@@ -202,7 +131,7 @@ static inline void xsk_ring_prod__submit(struct xsk_ring_prod *prod, __u32 nb)
/* Make sure everything has been written to the ring before indicating
* this to the kernel by writing the producer pointer.
*/
- libbpf_smp_store_release(prod->producer, *prod->producer + nb);
+ __atomic_store_n(prod->producer, *prod->producer + nb, __ATOMIC_RELEASE);
}
static inline __u32 xsk_ring_cons__peek(struct xsk_ring_cons *cons, __u32 nb, __u32 *idx)
@@ -227,8 +156,7 @@ static inline void xsk_ring_cons__release(struct xsk_ring_cons *cons, __u32 nb)
/* Make sure data has been read before indicating we are done
* with the entries by updating the consumer pointer.
*/
- libbpf_smp_store_release(cons->consumer, *cons->consumer + nb);
-
+ __atomic_store_n(cons->consumer, *cons->consumer + nb, __ATOMIC_RELEASE);
}
static inline void *xsk_umem__get_data(void *umem_area, __u64 addr)
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 08/15] selftests/xsk: remove namespaces
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (6 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 07/15] selftests/xsk: replace asm acquire/release implementations Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 09/15] selftests/xsk: load and attach XDP program only once per mode Magnus Karlsson
` (7 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Remove the namespaces used as they fill no function. This will
simplify the code for speeding up the tests in the following commits.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/test_xsk.sh | 33 ++++++----------
tools/testing/selftests/bpf/xsk_prereqs.sh | 12 +-----
tools/testing/selftests/bpf/xskxceiver.c | 46 +++-------------------
tools/testing/selftests/bpf/xskxceiver.h | 3 --
4 files changed, 20 insertions(+), 74 deletions(-)
diff --git a/tools/testing/selftests/bpf/test_xsk.sh b/tools/testing/selftests/bpf/test_xsk.sh
index cb315d85148b..b077cf58f825 100755
--- a/tools/testing/selftests/bpf/test_xsk.sh
+++ b/tools/testing/selftests/bpf/test_xsk.sh
@@ -24,8 +24,6 @@
# ----------- | ----------
# | vethX | --------- | vethY |
# ----------- peer ----------
-# | | |
-# namespaceX | namespaceY
#
# AF_XDP is an address family optimized for high performance packet processing,
# it is XDP’s user-space interface.
@@ -39,10 +37,9 @@
# Prerequisites setup by script:
#
# Set up veth interfaces as per the topology shown ^^:
-# * setup two veth interfaces and one namespace
-# ** veth<xxxx> in root namespace
-# ** veth<yyyy> in af_xdp<xxxx> namespace
-# ** namespace af_xdp<xxxx>
+# * setup two veth interfaces
+# ** veth<xxxx>
+# ** veth<yyyy>
# *** xxxx and yyyy are randomly generated 4 digit numbers used to avoid
# conflict with any existing interface
# * tests the veth and xsk layers of the topology
@@ -103,28 +100,25 @@ VETH0_POSTFIX=$(cat ${URANDOM} | tr -dc '0-9' | fold -w 256 | head -n 1 | head -
VETH0=ve${VETH0_POSTFIX}
VETH1_POSTFIX=$(cat ${URANDOM} | tr -dc '0-9' | fold -w 256 | head -n 1 | head --bytes 4)
VETH1=ve${VETH1_POSTFIX}
-NS0=root
-NS1=af_xdp${VETH1_POSTFIX}
MTU=1500
trap ctrl_c INT
function ctrl_c() {
- cleanup_exit ${VETH0} ${VETH1} ${NS1}
+ cleanup_exit ${VETH0} ${VETH1}
exit 1
}
setup_vethPairs() {
if [[ $verbose -eq 1 ]]; then
- echo "setting up ${VETH0}: namespace: ${NS0}"
+ echo "setting up ${VETH0}"
fi
- ip netns add ${NS1}
ip link add ${VETH0} numtxqueues 4 numrxqueues 4 type veth peer name ${VETH1} numtxqueues 4 numrxqueues 4
if [ -f /proc/net/if_inet6 ]; then
echo 1 > /proc/sys/net/ipv6/conf/${VETH0}/disable_ipv6
fi
if [[ $verbose -eq 1 ]]; then
- echo "setting up ${VETH1}: namespace: ${NS1}"
+ echo "setting up ${VETH1}"
fi
if [[ $busy_poll -eq 1 ]]; then
@@ -134,18 +128,15 @@ setup_vethPairs() {
echo 200000 > /sys/class/net/${VETH1}/gro_flush_timeout
fi
- ip link set ${VETH1} netns ${NS1}
- ip netns exec ${NS1} ip link set ${VETH1} mtu ${MTU}
+ ip link set ${VETH1} mtu ${MTU}
ip link set ${VETH0} mtu ${MTU}
- ip netns exec ${NS1} ip link set ${VETH1} up
- ip netns exec ${NS1} ip link set dev lo up
+ ip link set ${VETH1} up
ip link set ${VETH0} up
}
if [ ! -z $ETH ]; then
VETH0=${ETH}
VETH1=${ETH}
- NS1=""
else
validate_root_exec
validate_veth_support ${VETH0}
@@ -155,7 +146,7 @@ else
retval=$?
if [ $retval -ne 0 ]; then
test_status $retval "${TEST_NAME}"
- cleanup_exit ${VETH0} ${VETH1} ${NS1}
+ cleanup_exit ${VETH0} ${VETH1}
exit $retval
fi
fi
@@ -179,14 +170,14 @@ statusList=()
TEST_NAME="XSK_SELFTESTS_${VETH0}_SOFTIRQ"
if [[ $debug -eq 1 ]]; then
- echo "-i" ${VETH0} "-i" ${VETH1},${NS1}
+ echo "-i" ${VETH0} "-i" ${VETH1}
exit
fi
exec_xskxceiver
if [ -z $ETH ]; then
- cleanup_exit ${VETH0} ${VETH1} ${NS1}
+ cleanup_exit ${VETH0} ${VETH1}
fi
TEST_NAME="XSK_SELFTESTS_${VETH0}_BUSY_POLL"
busy_poll=1
@@ -199,7 +190,7 @@ exec_xskxceiver
## END TESTS
if [ -z $ETH ]; then
- cleanup_exit ${VETH0} ${VETH1} ${NS1}
+ cleanup_exit ${VETH0} ${VETH1}
fi
failures=0
diff --git a/tools/testing/selftests/bpf/xsk_prereqs.sh b/tools/testing/selftests/bpf/xsk_prereqs.sh
index a0b71723a818..ae697a10a056 100755
--- a/tools/testing/selftests/bpf/xsk_prereqs.sh
+++ b/tools/testing/selftests/bpf/xsk_prereqs.sh
@@ -55,21 +55,13 @@ test_exit()
clear_configs()
{
- if [ $(ip netns show | grep $3 &>/dev/null; echo $?;) == 0 ]; then
- [ $(ip netns exec $3 ip link show $2 &>/dev/null; echo $?;) == 0 ] &&
- { ip netns exec $3 ip link del $2; }
- ip netns del $3
- fi
- #Once we delete a veth pair node, the entire veth pair is removed,
- #this is just to be cautious just incase the NS does not exist then
- #veth node inside NS won't get removed so we explicitly remove it
[ $(ip link show $1 &>/dev/null; echo $?;) == 0 ] &&
{ ip link del $1; }
}
cleanup_exit()
{
- clear_configs $1 $2 $3
+ clear_configs $1 $2
}
validate_ip_utility()
@@ -83,7 +75,7 @@ exec_xskxceiver()
ARGS+="-b "
fi
- ./${XSKOBJ} -i ${VETH0} -i ${VETH1},${NS1} ${ARGS}
+ ./${XSKOBJ} -i ${VETH0} -i ${VETH1} ${ARGS}
retval=$?
test_status $retval "${TEST_NAME}"
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 72578cebfbf7..4c8f32e1c431 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -55,12 +55,11 @@
* Flow:
* -----
* - Single process spawns two threads: Tx and Rx
- * - Each of these two threads attach to a veth interface within their assigned
- * namespaces
- * - Each thread Creates one AF_XDP socket connected to a unique umem for each
+ * - Each of these two threads attach to a veth interface
+ * - Each thread creates one AF_XDP socket connected to a unique umem for each
* veth interface
- * - Tx thread Transmits 10k packets from veth<xxxx> to veth<yyyy>
- * - Rx thread verifies if all 10k packets were received and delivered in-order,
+ * - Tx thread Transmits a number of packets from veth<xxxx> to veth<yyyy>
+ * - Rx thread verifies if all packets were received and delivered in-order,
* and have the right content
*
* Enable/disable packet dump mode:
@@ -399,28 +398,6 @@ static void usage(const char *prog)
ksft_print_msg(str, prog);
}
-static int switch_namespace(const char *nsname)
-{
- char fqns[26] = "/var/run/netns/";
- int nsfd;
-
- if (!nsname || strlen(nsname) == 0)
- return -1;
-
- strncat(fqns, nsname, sizeof(fqns) - strlen(fqns) - 1);
- nsfd = open(fqns, O_RDONLY);
-
- if (nsfd == -1)
- exit_with_error(errno);
-
- if (setns(nsfd, 0) == -1)
- exit_with_error(errno);
-
- print_verbose("NS switched: %s\n", nsname);
-
- return nsfd;
-}
-
static bool validate_interface(struct ifobject *ifobj)
{
if (!strcmp(ifobj->ifname, ""))
@@ -438,8 +415,6 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
opterr = 0;
for (;;) {
- char *sptr, *token;
-
c = getopt_long(argc, argv, "i:Dvb", long_options, &option_index);
if (c == -1)
break;
@@ -453,11 +428,8 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
else
break;
- sptr = strndupa(optarg, strlen(optarg));
- memcpy(ifobj->ifname, strsep(&sptr, ","), MAX_INTERFACE_NAME_CHARS);
- token = strsep(&sptr, ",");
- if (token)
- memcpy(ifobj->nsname, token, MAX_INTERFACES_NAMESPACE_CHARS);
+ memcpy(ifobj->ifname, optarg,
+ min_t(size_t, MAX_INTERFACE_NAME_CHARS, strlen(optarg)));
interface_nb++;
break;
case 'D':
@@ -1283,8 +1255,6 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
int ret, ifindex;
void *bufs;
- ifobject->ns_fd = switch_namespace(ifobject->nsname);
-
if (ifobject->umem->unaligned_mode)
mmap_flags |= MAP_HUGETLB;
@@ -1843,8 +1813,6 @@ static struct ifobject *ifobject_create(void)
if (!ifobj->umem)
goto out_umem;
- ifobj->ns_fd = -1;
-
return ifobj;
out_umem:
@@ -1856,8 +1824,6 @@ static struct ifobject *ifobject_create(void)
static void ifobject_delete(struct ifobject *ifobj)
{
- if (ifobj->ns_fd != -1)
- close(ifobj->ns_fd);
free(ifobj->umem);
free(ifobj->xsk_arr);
free(ifobj);
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index edb76d2def9f..dcb908f5bb4c 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -30,7 +30,6 @@
#define TEST_CONTINUE 1
#define MAX_INTERFACES 2
#define MAX_INTERFACE_NAME_CHARS 16
-#define MAX_INTERFACES_NAMESPACE_CHARS 16
#define MAX_SOCKETS 2
#define MAX_TEST_NAME_SIZE 32
#define MAX_TEARDOWN_ITER 10
@@ -133,14 +132,12 @@ typedef void *(*thread_func_t)(void *arg);
struct ifobject {
char ifname[MAX_INTERFACE_NAME_CHARS];
- char nsname[MAX_INTERFACES_NAMESPACE_CHARS];
struct xsk_socket_info *xsk;
struct xsk_socket_info *xsk_arr;
struct xsk_umem_info *umem;
thread_func_t func_ptr;
validation_func_t validation_func;
struct pkt_stream *pkt_stream;
- int ns_fd;
int xsk_map_fd;
u32 dst_ip;
u32 src_ip;
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 09/15] selftests/xsk: load and attach XDP program only once per mode
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (7 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 08/15] selftests/xsk: remove namespaces Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 10/15] selftests/xsk: remove unnecessary code in control path Magnus Karlsson
` (6 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Load and attach the XDP program only once per XDP mode that is being
executed. Today, the XDP program is loaded and attached for every
test, then unloaded, which takes a long time on real NICs, since they
have to reconfigure their HW, in contrast to veth. The test suite now
completes in 21 seconds, instead of 207 seconds previously on my
machine. This is a speed-up of around 10x.
This is accomplished by moving the XDP loading from the worker threads
to the main thread and replacing the XDP loading interfaces of xsk.c
that was taken from the xsk support in libbpf, with something more
explicit that is more useful for these tests. Instead, the relevant
file descriptors and ifindexes are just passed down to the new
functions.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xsk.c | 92 +++++++++-----
tools/testing/selftests/bpf/xsk.h | 7 +-
tools/testing/selftests/bpf/xskxceiver.c | 147 ++++++++++++++---------
tools/testing/selftests/bpf/xskxceiver.h | 3 +
4 files changed, 162 insertions(+), 87 deletions(-)
diff --git a/tools/testing/selftests/bpf/xsk.c b/tools/testing/selftests/bpf/xsk.c
index b166edfff86d..1dd953541812 100644
--- a/tools/testing/selftests/bpf/xsk.c
+++ b/tools/testing/selftests/bpf/xsk.c
@@ -51,6 +51,8 @@
#define pr_warn(fmt, ...) fprintf(stderr, fmt, ##__VA_ARGS__)
+#define XSKMAP_SIZE 1
+
enum xsk_prog {
XSK_PROG_FALLBACK,
XSK_PROG_REDIRECT_FLAGS,
@@ -387,10 +389,9 @@ static enum xsk_prog get_xsk_prog(void)
return detected;
}
-static int xsk_load_xdp_prog(struct xsk_socket *xsk)
+static int __xsk_load_xdp_prog(int xsk_map_fd)
{
static const int log_buf_size = 16 * 1024;
- struct xsk_ctx *ctx = xsk->ctx;
char log_buf[log_buf_size];
int prog_fd;
@@ -418,7 +419,7 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk)
/* *(u32 *)(r10 - 4) = r2 */
BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_2, -4),
/* r1 = xskmap[] */
- BPF_LD_MAP_FD(BPF_REG_1, ctx->xsks_map_fd),
+ BPF_LD_MAP_FD(BPF_REG_1, xsk_map_fd),
/* r3 = XDP_PASS */
BPF_MOV64_IMM(BPF_REG_3, 2),
/* call bpf_redirect_map */
@@ -430,7 +431,7 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk)
/* r2 += -4 */
BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4),
/* r1 = xskmap[] */
- BPF_LD_MAP_FD(BPF_REG_1, ctx->xsks_map_fd),
+ BPF_LD_MAP_FD(BPF_REG_1, xsk_map_fd),
/* call bpf_map_lookup_elem */
BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
/* r1 = r0 */
@@ -442,7 +443,7 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk)
/* r2 = *(u32 *)(r10 - 4) */
BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_10, -4),
/* r1 = xskmap[] */
- BPF_LD_MAP_FD(BPF_REG_1, ctx->xsks_map_fd),
+ BPF_LD_MAP_FD(BPF_REG_1, xsk_map_fd),
/* r3 = 0 */
BPF_MOV64_IMM(BPF_REG_3, 0),
/* call bpf_redirect_map */
@@ -461,7 +462,7 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk)
/* r2 = *(u32 *)(r1 + 16) */
BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, 16),
/* r1 = xskmap[] */
- BPF_LD_MAP_FD(BPF_REG_1, ctx->xsks_map_fd),
+ BPF_LD_MAP_FD(BPF_REG_1, xsk_map_fd),
/* r3 = XDP_PASS */
BPF_MOV64_IMM(BPF_REG_3, 2),
/* call bpf_redirect_map */
@@ -480,13 +481,40 @@ static int xsk_load_xdp_prog(struct xsk_socket *xsk)
prog_fd = bpf_prog_load(BPF_PROG_TYPE_XDP, NULL, "LGPL-2.1 or BSD-2-Clause",
progs[option], insns_cnt[option], &opts);
- if (prog_fd < 0) {
+ if (prog_fd < 0)
pr_warn("BPF log buffer:\n%s", log_buf);
- return prog_fd;
+
+ return prog_fd;
+}
+
+int xsk_attach_xdp_program(int ifindex, int prog_fd, u32 xdp_flags)
+{
+ DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
+ __u32 prog_id = 0;
+ int link_fd;
+ int err;
+
+ err = bpf_xdp_query_id(ifindex, xdp_flags, &prog_id);
+ if (err) {
+ pr_warn("getting XDP prog id failed\n");
+ return err;
}
- ctx->prog_fd = prog_fd;
- return 0;
+ /* If there's a netlink-based XDP prog loaded on interface, bail out
+ * and ask user to do the removal by himself
+ */
+ if (prog_id) {
+ pr_warn("Netlink-based XDP prog detected, please unload it in order to launch AF_XDP prog\n");
+ return -EINVAL;
+ }
+
+ opts.flags = xdp_flags & ~(XDP_FLAGS_UPDATE_IF_NOEXIST | XDP_FLAGS_REPLACE);
+
+ link_fd = bpf_link_create(prog_fd, ifindex, BPF_XDP, &opts);
+ if (link_fd < 0)
+ pr_warn("bpf_link_create failed: %s\n", strerror(errno));
+
+ return link_fd;
}
static int xsk_create_bpf_link(struct xsk_socket *xsk)
@@ -775,7 +803,7 @@ static int xsk_init_xdp_res(struct xsk_socket *xsk,
if (err)
return err;
- err = xsk_load_xdp_prog(xsk);
+ err = __xsk_load_xdp_prog(*xsks_map_fd);
if (err)
goto err_load_xdp_prog;
@@ -871,6 +899,22 @@ int xsk_setup_xdp_prog_xsk(struct xsk_socket *xsk, int *xsks_map_fd)
return __xsk_setup_xdp_prog(xsk, xsks_map_fd);
}
+int xsk_load_xdp_program(int *xsk_map_fd, int *prog_fd)
+{
+ *xsk_map_fd = bpf_map_create(BPF_MAP_TYPE_XSKMAP, "xsks_map", sizeof(int), sizeof(int),
+ XSKMAP_SIZE, NULL);
+ if (*xsk_map_fd < 0)
+ return *xsk_map_fd;
+
+ *prog_fd = __xsk_load_xdp_prog(*xsk_map_fd);
+ if (*prog_fd < 0) {
+ close(*xsk_map_fd);
+ return *prog_fd;
+ }
+
+ return 0;
+}
+
static struct xsk_ctx *xsk_get_ctx(struct xsk_umem *umem, int ifindex,
__u32 queue_id)
{
@@ -917,7 +961,7 @@ static void xsk_put_ctx(struct xsk_ctx *ctx, bool unmap)
static struct xsk_ctx *xsk_create_ctx(struct xsk_socket *xsk,
struct xsk_umem *umem, int ifindex,
- const char *ifname, __u32 queue_id,
+ __u32 queue_id,
struct xsk_ring_prod *fill,
struct xsk_ring_cons *comp)
{
@@ -944,7 +988,6 @@ static struct xsk_ctx *xsk_create_ctx(struct xsk_socket *xsk,
ctx->refcount = 1;
ctx->umem = umem;
ctx->queue_id = queue_id;
- bpf_strlcpy(ctx->ifname, ifname, IFNAMSIZ);
ctx->prog_fd = FD_NOT_USED;
ctx->link_fd = FD_NOT_USED;
ctx->xsks_map_fd = FD_NOT_USED;
@@ -991,7 +1034,7 @@ int xsk_setup_xdp_prog(int ifindex, int *xsks_map_fd)
}
int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
- const char *ifname,
+ int ifindex,
__u32 queue_id, struct xsk_umem *umem,
struct xsk_ring_cons *rx,
struct xsk_ring_prod *tx,
@@ -1005,7 +1048,7 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
struct xdp_mmap_offsets off;
struct xsk_socket *xsk;
struct xsk_ctx *ctx;
- int err, ifindex;
+ int err;
if (!umem || !xsk_ptr || !(rx || tx))
return -EFAULT;
@@ -1020,12 +1063,6 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
if (err)
goto out_xsk_alloc;
- ifindex = if_nametoindex(ifname);
- if (!ifindex) {
- err = -errno;
- goto out_xsk_alloc;
- }
-
if (umem->refcount++ > 0) {
xsk->fd = socket(AF_XDP, SOCK_RAW | SOCK_CLOEXEC, 0);
if (xsk->fd < 0) {
@@ -1045,8 +1082,7 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
goto out_socket;
}
- ctx = xsk_create_ctx(xsk, umem, ifindex, ifname, queue_id,
- fill, comp);
+ ctx = xsk_create_ctx(xsk, umem, ifindex, queue_id, fill, comp);
if (!ctx) {
err = -ENOMEM;
goto out_socket;
@@ -1144,12 +1180,6 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
goto out_mmap_tx;
}
- if (!(xsk->config.libbpf_flags & XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)) {
- err = __xsk_setup_xdp_prog(xsk, NULL);
- if (err)
- goto out_mmap_tx;
- }
-
*xsk_ptr = xsk;
umem->fill_save = NULL;
umem->comp_save = NULL;
@@ -1173,7 +1203,7 @@ int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
return err;
}
-int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
+int xsk_socket__create(struct xsk_socket **xsk_ptr, int ifindex,
__u32 queue_id, struct xsk_umem *umem,
struct xsk_ring_cons *rx, struct xsk_ring_prod *tx,
const struct xsk_socket_config *usr_config)
@@ -1181,7 +1211,7 @@ int xsk_socket__create(struct xsk_socket **xsk_ptr, const char *ifname,
if (!umem)
return -EFAULT;
- return xsk_socket__create_shared(xsk_ptr, ifname, queue_id, umem,
+ return xsk_socket__create_shared(xsk_ptr, ifindex, queue_id, umem,
rx, tx, umem->fill_save,
umem->comp_save, usr_config);
}
diff --git a/tools/testing/selftests/bpf/xsk.h b/tools/testing/selftests/bpf/xsk.h
index 24ee765aded3..7a5aeacd261b 100644
--- a/tools/testing/selftests/bpf/xsk.h
+++ b/tools/testing/selftests/bpf/xsk.h
@@ -204,6 +204,9 @@ int xsk_socket__update_xskmap(struct xsk_socket *xsk, int xsks_map_fd);
/* Flags for the libbpf_flags field. */
#define XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD (1 << 0)
+int xsk_load_xdp_program(int *xsk_map_fd, int *prog_fd);
+int xsk_attach_xdp_program(int ifindex, int prog_fd, u32 xdp_flags);
+
struct xsk_socket_config {
__u32 rx_size;
__u32 tx_size;
@@ -219,13 +222,13 @@ int xsk_umem__create(struct xsk_umem **umem,
struct xsk_ring_cons *comp,
const struct xsk_umem_config *config);
int xsk_socket__create(struct xsk_socket **xsk,
- const char *ifname, __u32 queue_id,
+ int ifindex, __u32 queue_id,
struct xsk_umem *umem,
struct xsk_ring_cons *rx,
struct xsk_ring_prod *tx,
const struct xsk_socket_config *config);
int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
- const char *ifname,
+ int ifindex,
__u32 queue_id, struct xsk_umem *umem,
struct xsk_ring_cons *rx,
struct xsk_ring_prod *tx,
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 4c8f32e1c431..0c0974c209cd 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -268,6 +268,11 @@ 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 u32 mode_to_xdp_flags(enum test_mode mode)
+{
+ return (mode == TEST_MODE_SKB) ? XDP_FLAGS_SKB_MODE : XDP_FLAGS_DRV_MODE;
+}
+
static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size)
{
struct xsk_umem_config cfg = {
@@ -329,7 +334,7 @@ static int __xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_i
txr = ifobject->tx_on ? &xsk->tx : NULL;
rxr = ifobject->rx_on ? &xsk->rx : NULL;
- return xsk_socket__create(&xsk->xsk, ifobject->ifname, 0, umem->umem, rxr, txr, &cfg);
+ return xsk_socket__create(&xsk->xsk, ifobject->ifindex, 0, umem->umem, rxr, txr, &cfg);
}
static bool ifobj_zc_avail(struct ifobject *ifobject)
@@ -359,8 +364,7 @@ static bool ifobj_zc_avail(struct ifobject *ifobject)
xsk = calloc(1, sizeof(struct xsk_socket_info));
if (!xsk)
goto out;
- ifobject->xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
- ifobject->xdp_flags |= XDP_FLAGS_DRV_MODE;
+ ifobject->xdp_flags = XDP_FLAGS_DRV_MODE;
ifobject->bind_flags = XDP_USE_NEED_WAKEUP | XDP_ZEROCOPY;
ifobject->rx_on = true;
xsk->rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
@@ -430,6 +434,11 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
memcpy(ifobj->ifname, optarg,
min_t(size_t, MAX_INTERFACE_NAME_CHARS, strlen(optarg)));
+
+ ifobj->ifindex = if_nametoindex(ifobj->ifname);
+ if (!ifobj->ifindex)
+ exit_with_error(errno);
+
interface_nb++;
break;
case 'D':
@@ -510,12 +519,6 @@ static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
for (i = 0; i < MAX_INTERFACES; i++) {
struct ifobject *ifobj = i ? ifobj_rx : ifobj_tx;
- ifobj->xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
- if (mode == TEST_MODE_SKB)
- ifobj->xdp_flags |= XDP_FLAGS_SKB_MODE;
- else
- ifobj->xdp_flags |= XDP_FLAGS_DRV_MODE;
-
ifobj->bind_flags = XDP_USE_NEED_WAKEUP;
if (mode == TEST_MODE_ZC)
ifobj->bind_flags |= XDP_ZEROCOPY;
@@ -1252,7 +1255,8 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
LIBBPF_OPTS(bpf_xdp_query_opts, opts);
- int ret, ifindex;
+ u32 queue_id = 0;
+ int ret, fd;
void *bufs;
if (ifobject->umem->unaligned_mode)
@@ -1278,31 +1282,8 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
if (!ifobject->rx_on)
return;
- ifindex = if_nametoindex(ifobject->ifname);
- if (!ifindex)
- exit_with_error(errno);
-
- ret = xsk_setup_xdp_prog_xsk(ifobject->xsk->xsk, &ifobject->xsk_map_fd);
- if (ret)
- exit_with_error(-ret);
-
- ret = bpf_xdp_query(ifindex, ifobject->xdp_flags, &opts);
- if (ret)
- exit_with_error(-ret);
-
- if (ifobject->xdp_flags & XDP_FLAGS_SKB_MODE) {
- if (opts.attach_mode != XDP_ATTACHED_SKB) {
- ksft_print_msg("ERROR: [%s] XDP prog not in SKB mode\n");
- exit_with_error(EINVAL);
- }
- } else if (ifobject->xdp_flags & XDP_FLAGS_DRV_MODE) {
- if (opts.attach_mode != XDP_ATTACHED_DRV) {
- ksft_print_msg("ERROR: [%s] XDP prog not in DRV mode\n");
- exit_with_error(EINVAL);
- }
- }
-
- ret = xsk_socket__update_xskmap(ifobject->xsk->xsk, ifobject->xsk_map_fd);
+ fd = xsk_socket__fd(ifobject->xsk->xsk);
+ ret = bpf_map_update_elem(ifobject->xsk_map_fd, &queue_id, &fd, 0);
if (ret)
exit_with_error(errno);
}
@@ -1336,15 +1317,19 @@ static void *worker_testapp_validate_rx(void *arg)
{
struct test_spec *test = (struct test_spec *)arg;
struct ifobject *ifobject = test->ifobj_rx;
+ int id = 0, err, fd = xsk_socket__fd(ifobject->xsk->xsk);
struct pollfd fds = { };
- int id = 0;
- int err;
+ u32 queue_id = 0;
if (test->current_step == 1) {
thread_common_ops(test, ifobject);
} else {
bpf_map_delete_elem(ifobject->xsk_map_fd, &id);
- xsk_socket__update_xskmap(ifobject->xsk->xsk, ifobject->xsk_map_fd);
+ err = bpf_map_update_elem(ifobject->xsk_map_fd, &queue_id, &fd, 0);
+ if (err) {
+ printf("Error: Failed to update xskmap, error %s\n", strerror(err));
+ exit_with_error(err);
+ }
}
fds.fd = xsk_socket__fd(ifobject->xsk->xsk);
@@ -1413,7 +1398,10 @@ static int testapp_validate_traffic_single_thread(struct test_spec *test, struct
pthread_join(t0, NULL);
if (test->total_steps == test->current_step || test->fail) {
+ u32 queue_id = 0;
+
xsk_socket__delete(ifobj->xsk->xsk);
+ bpf_map_delete_elem(ifobj->xsk_map_fd, &queue_id);
testapp_clean_xsk_umem(ifobj);
}
@@ -1502,14 +1490,14 @@ static void testapp_bidi(struct test_spec *test)
static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj_rx)
{
- int ret;
+ int ret, queue_id = 0, fd = xsk_socket__fd(ifobj_rx->xsk->xsk);
xsk_socket__delete(ifobj_tx->xsk->xsk);
xsk_socket__delete(ifobj_rx->xsk->xsk);
ifobj_tx->xsk = &ifobj_tx->xsk_arr[1];
ifobj_rx->xsk = &ifobj_rx->xsk_arr[1];
- ret = xsk_socket__update_xskmap(ifobj_rx->xsk->xsk, ifobj_rx->xsk_map_fd);
+ ret = bpf_map_update_elem(ifobj_rx->xsk_map_fd, &queue_id, &fd, 0);
if (ret)
exit_with_error(errno);
}
@@ -1673,8 +1661,9 @@ static void testapp_invalid_desc(struct test_spec *test)
static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
const char *dst_ip, const char *src_ip, const u16 dst_port,
- const u16 src_port, thread_func_t func_ptr)
+ const u16 src_port, thread_func_t func_ptr, bool load_xdp)
{
+ int xsk_map_fd, prog_fd, err;
struct in_addr ip;
memcpy(ifobj->dst_mac, dst_mac, ETH_ALEN);
@@ -1690,6 +1679,24 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
ifobj->src_port = src_port;
ifobj->func_ptr = func_ptr;
+
+ if (!load_xdp)
+ return;
+
+ err = xsk_load_xdp_program(&xsk_map_fd, &prog_fd);
+ if (err) {
+ printf("Error loading XDP program\n");
+ exit_with_error(err);
+ }
+
+ ifobj->xsk_map_fd = xsk_map_fd;
+ ifobj->prog_fd = prog_fd;
+ ifobj->xdp_flags = mode_to_xdp_flags(TEST_MODE_SKB);
+ ifobj->link_fd = xsk_attach_xdp_program(ifobj->ifindex, prog_fd, ifobj->xdp_flags);
+ if (ifobj->link_fd < 0) {
+ printf("Error attaching XDP program\n");
+ exit_with_error(ifobj->link_fd);
+ }
}
static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
@@ -1824,12 +1831,15 @@ static struct ifobject *ifobject_create(void)
static void ifobject_delete(struct ifobject *ifobj)
{
+ close(ifobj->prog_fd);
+ close(ifobj->xsk_map_fd);
+
free(ifobj->umem);
free(ifobj->xsk_arr);
free(ifobj);
}
-static bool is_xdp_supported(struct ifobject *ifobject)
+static bool is_xdp_supported(int ifindex)
{
int flags = XDP_FLAGS_DRV_MODE;
@@ -1838,7 +1848,6 @@ static bool is_xdp_supported(struct ifobject *ifobject)
BPF_MOV64_IMM(BPF_REG_0, XDP_PASS),
BPF_EXIT_INSN()
};
- int ifindex = if_nametoindex(ifobject->ifname);
int prog_fd, insn_cnt = ARRAY_SIZE(insns);
int err;
@@ -1858,6 +1867,29 @@ static bool is_xdp_supported(struct ifobject *ifobject)
return true;
}
+static void change_to_drv_mode(struct ifobject *ifobj)
+{
+ LIBBPF_OPTS(bpf_xdp_query_opts, opts);
+ int ret;
+
+ close(ifobj->link_fd);
+ ifobj->link_fd = xsk_attach_xdp_program(ifobj->ifindex, ifobj->prog_fd,
+ XDP_FLAGS_DRV_MODE);
+ if (ifobj->link_fd < 0) {
+ ksft_print_msg("Error attaching XDP program\n");
+ exit_with_error(-ifobj->link_fd);
+ }
+
+ ret = bpf_xdp_query(ifobj->ifindex, XDP_FLAGS_DRV_MODE, &opts);
+ if (ret)
+ exit_with_error(errno);
+
+ if (opts.attach_mode != XDP_ATTACHED_DRV) {
+ ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
+ exit_with_error(EINVAL);
+ }
+}
+
int main(int argc, char **argv)
{
struct pkt_stream *rx_pkt_stream_default;
@@ -1866,7 +1898,7 @@ int main(int argc, char **argv)
int modes = TEST_MODE_SKB + 1;
u32 i, j, failed_tests = 0;
struct test_spec test;
- bool shared_umem;
+ bool shared_netdev;
/* Use libbpf 1.0 API mode */
libbpf_set_strict_mode(LIBBPF_STRICT_ALL);
@@ -1881,27 +1913,27 @@ int main(int argc, char **argv)
setlocale(LC_ALL, "");
parse_command_line(ifobj_tx, ifobj_rx, argc, argv);
- shared_umem = !strcmp(ifobj_tx->ifname, ifobj_rx->ifname);
- ifobj_tx->shared_umem = shared_umem;
- ifobj_rx->shared_umem = shared_umem;
+ shared_netdev = (ifobj_tx->ifindex == ifobj_rx->ifindex);
+ ifobj_tx->shared_umem = shared_netdev;
+ ifobj_rx->shared_umem = shared_netdev;
if (!validate_interface(ifobj_tx) || !validate_interface(ifobj_rx)) {
usage(basename(argv[0]));
ksft_exit_xfail();
}
- init_iface(ifobj_tx, MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2,
- worker_testapp_validate_tx);
- init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
- worker_testapp_validate_rx);
-
- if (is_xdp_supported(ifobj_tx)) {
+ if (is_xdp_supported(ifobj_tx->ifindex)) {
modes++;
if (ifobj_zc_avail(ifobj_tx))
modes++;
}
+ init_iface(ifobj_rx, MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2,
+ worker_testapp_validate_rx, true);
+ init_iface(ifobj_tx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
+ worker_testapp_validate_tx, !shared_netdev);
+
test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
rx_pkt_stream_default = pkt_stream_generate(ifobj_rx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
@@ -1912,7 +1944,13 @@ int main(int argc, char **argv)
ksft_set_plan(modes * TEST_TYPE_MAX);
- for (i = 0; i < modes; i++)
+ for (i = 0; i < modes; i++) {
+ if (i == TEST_MODE_DRV) {
+ change_to_drv_mode(ifobj_rx);
+ if (!shared_netdev)
+ change_to_drv_mode(ifobj_tx);
+ }
+
for (j = 0; j < TEST_TYPE_MAX; j++) {
test_spec_init(&test, ifobj_tx, ifobj_rx, i);
run_pkt_test(&test, i, j);
@@ -1921,6 +1959,7 @@ int main(int argc, char **argv)
if (test.fail)
failed_tests++;
}
+ }
pkt_stream_delete(tx_pkt_stream_default);
pkt_stream_delete(rx_pkt_stream_default);
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index dcb908f5bb4c..b2ba877b1966 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -139,6 +139,9 @@ struct ifobject {
validation_func_t validation_func;
struct pkt_stream *pkt_stream;
int xsk_map_fd;
+ int prog_fd;
+ int link_fd;
+ int ifindex;
u32 dst_ip;
u32 src_ip;
u32 xdp_flags;
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 10/15] selftests/xsk: remove unnecessary code in control path
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (8 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 09/15] selftests/xsk: load and attach XDP program only once per mode Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program Magnus Karlsson
` (5 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Remove unnecessary code in the control path. This is located in the
file xsk.c that was moved from libbpf when the xsk support there was
deprecated. Some of the code there is not needed anymore as the
selftests are only guaranteed to run on the kernel it is shipped
with. Therefore, all the code that has to deal with compatibility of
older kernels can be dropped and also any other function that is not
of any use for the tests.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xsk.c | 617 +----------------------
tools/testing/selftests/bpf/xsk.h | 9 -
tools/testing/selftests/bpf/xskxceiver.c | 8 -
3 files changed, 3 insertions(+), 631 deletions(-)
diff --git a/tools/testing/selftests/bpf/xsk.c b/tools/testing/selftests/bpf/xsk.c
index 1dd953541812..9ed31d280e48 100644
--- a/tools/testing/selftests/bpf/xsk.c
+++ b/tools/testing/selftests/bpf/xsk.c
@@ -35,8 +35,6 @@
#include "xsk.h"
#include "bpf_util.h"
-#define FD_NOT_USED (-1)
-
#ifndef SOL_XDP
#define SOL_XDP 283
#endif
@@ -53,11 +51,6 @@
#define XSKMAP_SIZE 1
-enum xsk_prog {
- XSK_PROG_FALLBACK,
- XSK_PROG_REDIRECT_FLAGS,
-};
-
struct xsk_umem {
struct xsk_ring_prod *fill_save;
struct xsk_ring_cons *comp_save;
@@ -78,11 +71,6 @@ struct xsk_ctx {
int refcount;
int ifindex;
struct list_head list;
- int prog_fd;
- int link_fd;
- int xsks_map_fd;
- char ifname[IFNAMSIZ];
- bool has_bpf_link;
};
struct xsk_socket {
@@ -93,27 +81,6 @@ struct xsk_socket {
int fd;
};
-struct xsk_nl_info {
- bool xdp_prog_attached;
- int ifindex;
- int fd;
-};
-
-/* Up until and including Linux 5.3 */
-struct xdp_ring_offset_v1 {
- __u64 producer;
- __u64 consumer;
- __u64 desc;
-};
-
-/* Up until and including Linux 5.3 */
-struct xdp_mmap_offsets_v1 {
- struct xdp_ring_offset_v1 rx;
- struct xdp_ring_offset_v1 tx;
- struct xdp_ring_offset_v1 fr;
- struct xdp_ring_offset_v1 cr;
-};
-
int xsk_umem__fd(const struct xsk_umem *umem)
{
return umem ? umem->fd : -EINVAL;
@@ -156,55 +123,17 @@ static int xsk_set_xdp_socket_config(struct xsk_socket_config *cfg,
if (!usr_cfg) {
cfg->rx_size = XSK_RING_CONS__DEFAULT_NUM_DESCS;
cfg->tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
- cfg->libbpf_flags = 0;
- cfg->xdp_flags = 0;
cfg->bind_flags = 0;
return 0;
}
- if (usr_cfg->libbpf_flags & ~XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD)
- return -EINVAL;
-
cfg->rx_size = usr_cfg->rx_size;
cfg->tx_size = usr_cfg->tx_size;
- cfg->libbpf_flags = usr_cfg->libbpf_flags;
- cfg->xdp_flags = usr_cfg->xdp_flags;
cfg->bind_flags = usr_cfg->bind_flags;
return 0;
}
-static void xsk_mmap_offsets_v1(struct xdp_mmap_offsets *off)
-{
- struct xdp_mmap_offsets_v1 off_v1;
-
- /* getsockopt on a kernel <= 5.3 has no flags fields.
- * Copy over the offsets to the correct places in the >=5.4 format
- * and put the flags where they would have been on that kernel.
- */
- memcpy(&off_v1, off, sizeof(off_v1));
-
- off->rx.producer = off_v1.rx.producer;
- off->rx.consumer = off_v1.rx.consumer;
- off->rx.desc = off_v1.rx.desc;
- off->rx.flags = off_v1.rx.consumer + sizeof(__u32);
-
- off->tx.producer = off_v1.tx.producer;
- off->tx.consumer = off_v1.tx.consumer;
- off->tx.desc = off_v1.tx.desc;
- off->tx.flags = off_v1.tx.consumer + sizeof(__u32);
-
- off->fr.producer = off_v1.fr.producer;
- off->fr.consumer = off_v1.fr.consumer;
- off->fr.desc = off_v1.fr.desc;
- off->fr.flags = off_v1.fr.consumer + sizeof(__u32);
-
- off->cr.producer = off_v1.cr.producer;
- off->cr.consumer = off_v1.cr.consumer;
- off->cr.desc = off_v1.cr.desc;
- off->cr.flags = off_v1.cr.consumer + sizeof(__u32);
-}
-
static int xsk_get_mmap_offsets(int fd, struct xdp_mmap_offsets *off)
{
socklen_t optlen;
@@ -218,11 +147,6 @@ static int xsk_get_mmap_offsets(int fd, struct xdp_mmap_offsets *off)
if (optlen == sizeof(*off))
return 0;
- if (optlen == sizeof(struct xdp_mmap_offsets_v1)) {
- xsk_mmap_offsets_v1(off);
- return 0;
- }
-
return -EINVAL;
}
@@ -343,122 +267,19 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area,
return err;
}
-struct xsk_umem_config_v1 {
- __u32 fill_size;
- __u32 comp_size;
- __u32 frame_size;
- __u32 frame_headroom;
-};
-
-static enum xsk_prog get_xsk_prog(void)
-{
- enum xsk_prog detected = XSK_PROG_FALLBACK;
- char data_in = 0, data_out;
- struct bpf_insn insns[] = {
- BPF_LD_MAP_FD(BPF_REG_1, 0),
- BPF_MOV64_IMM(BPF_REG_2, 0),
- BPF_MOV64_IMM(BPF_REG_3, XDP_PASS),
- BPF_EMIT_CALL(BPF_FUNC_redirect_map),
- BPF_EXIT_INSN(),
- };
- LIBBPF_OPTS(bpf_test_run_opts, opts,
- .data_in = &data_in,
- .data_size_in = 1,
- .data_out = &data_out,
- );
-
- int prog_fd, map_fd, ret, insn_cnt = ARRAY_SIZE(insns);
-
- map_fd = bpf_map_create(BPF_MAP_TYPE_XSKMAP, NULL, sizeof(int), sizeof(int), 1, NULL);
- if (map_fd < 0)
- return detected;
-
- insns[0].imm = map_fd;
-
- prog_fd = bpf_prog_load(BPF_PROG_TYPE_XDP, NULL, "GPL", insns, insn_cnt, NULL);
- if (prog_fd < 0) {
- close(map_fd);
- return detected;
- }
-
- ret = bpf_prog_test_run_opts(prog_fd, &opts);
- if (!ret && opts.retval == XDP_PASS)
- detected = XSK_PROG_REDIRECT_FLAGS;
- close(prog_fd);
- close(map_fd);
- return detected;
-}
-
static int __xsk_load_xdp_prog(int xsk_map_fd)
{
static const int log_buf_size = 16 * 1024;
char log_buf[log_buf_size];
int prog_fd;
- /* This is the fallback C-program:
- * SEC("xdp_sock") int xdp_sock_prog(struct xdp_md *ctx)
- * {
- * int ret, index = ctx->rx_queue_index;
- *
- * // A set entry here means that the correspnding queue_id
- * // has an active AF_XDP socket bound to it.
- * ret = bpf_redirect_map(&xsks_map, index, XDP_PASS);
- * if (ret > 0)
- * return ret;
- *
- * // Fallback for pre-5.3 kernels, not supporting default
- * // action in the flags parameter.
- * if (bpf_map_lookup_elem(&xsks_map, &index))
- * return bpf_redirect_map(&xsks_map, index, 0);
- * return XDP_PASS;
- * }
- */
- struct bpf_insn prog[] = {
- /* r2 = *(u32 *)(r1 + 16) */
- BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, 16),
- /* *(u32 *)(r10 - 4) = r2 */
- BPF_STX_MEM(BPF_W, BPF_REG_10, BPF_REG_2, -4),
- /* r1 = xskmap[] */
- BPF_LD_MAP_FD(BPF_REG_1, xsk_map_fd),
- /* r3 = XDP_PASS */
- BPF_MOV64_IMM(BPF_REG_3, 2),
- /* call bpf_redirect_map */
- BPF_EMIT_CALL(BPF_FUNC_redirect_map),
- /* if w0 != 0 goto pc+13 */
- BPF_JMP32_IMM(BPF_JSGT, BPF_REG_0, 0, 13),
- /* r2 = r10 */
- BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
- /* r2 += -4 */
- BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -4),
- /* r1 = xskmap[] */
- BPF_LD_MAP_FD(BPF_REG_1, xsk_map_fd),
- /* call bpf_map_lookup_elem */
- BPF_EMIT_CALL(BPF_FUNC_map_lookup_elem),
- /* r1 = r0 */
- BPF_MOV64_REG(BPF_REG_1, BPF_REG_0),
- /* r0 = XDP_PASS */
- BPF_MOV64_IMM(BPF_REG_0, 2),
- /* if r1 == 0 goto pc+5 */
- BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 0, 5),
- /* r2 = *(u32 *)(r10 - 4) */
- BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_10, -4),
- /* r1 = xskmap[] */
- BPF_LD_MAP_FD(BPF_REG_1, xsk_map_fd),
- /* r3 = 0 */
- BPF_MOV64_IMM(BPF_REG_3, 0),
- /* call bpf_redirect_map */
- BPF_EMIT_CALL(BPF_FUNC_redirect_map),
- /* The jumps are to this instruction */
- BPF_EXIT_INSN(),
- };
-
/* This is the post-5.3 kernel C-program:
* SEC("xdp_sock") int xdp_sock_prog(struct xdp_md *ctx)
* {
* return bpf_redirect_map(&xsks_map, ctx->rx_queue_index, XDP_PASS);
* }
*/
- struct bpf_insn prog_redirect_flags[] = {
+ struct bpf_insn prog[] = {
/* r2 = *(u32 *)(r1 + 16) */
BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, 16),
/* r1 = xskmap[] */
@@ -469,18 +290,14 @@ static int __xsk_load_xdp_prog(int xsk_map_fd)
BPF_EMIT_CALL(BPF_FUNC_redirect_map),
BPF_EXIT_INSN(),
};
- size_t insns_cnt[] = {ARRAY_SIZE(prog),
- ARRAY_SIZE(prog_redirect_flags),
- };
- struct bpf_insn *progs[] = {prog, prog_redirect_flags};
- enum xsk_prog option = get_xsk_prog();
+ size_t insns_cnt = ARRAY_SIZE(prog);
LIBBPF_OPTS(bpf_prog_load_opts, opts,
.log_buf = log_buf,
.log_size = log_buf_size,
);
prog_fd = bpf_prog_load(BPF_PROG_TYPE_XDP, NULL, "LGPL-2.1 or BSD-2-Clause",
- progs[option], insns_cnt[option], &opts);
+ prog, insns_cnt, &opts);
if (prog_fd < 0)
pr_warn("BPF log buffer:\n%s", log_buf);
@@ -517,388 +334,6 @@ int xsk_attach_xdp_program(int ifindex, int prog_fd, u32 xdp_flags)
return link_fd;
}
-static int xsk_create_bpf_link(struct xsk_socket *xsk)
-{
- DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
- struct xsk_ctx *ctx = xsk->ctx;
- __u32 prog_id = 0;
- int link_fd;
- int err;
-
- err = bpf_xdp_query_id(ctx->ifindex, xsk->config.xdp_flags, &prog_id);
- if (err) {
- pr_warn("getting XDP prog id failed\n");
- return err;
- }
-
- /* if there's a netlink-based XDP prog loaded on interface, bail out
- * and ask user to do the removal by himself
- */
- if (prog_id) {
- pr_warn("Netlink-based XDP prog detected, please unload it in order to launch AF_XDP prog\n");
- return -EINVAL;
- }
-
- opts.flags = xsk->config.xdp_flags & ~(XDP_FLAGS_UPDATE_IF_NOEXIST | XDP_FLAGS_REPLACE);
-
- link_fd = bpf_link_create(ctx->prog_fd, ctx->ifindex, BPF_XDP, &opts);
- if (link_fd < 0) {
- pr_warn("bpf_link_create failed: %s\n", strerror(errno));
- return link_fd;
- }
-
- ctx->link_fd = link_fd;
- return 0;
-}
-
-static int xsk_get_max_queues(struct xsk_socket *xsk)
-{
- struct ethtool_channels channels = { .cmd = ETHTOOL_GCHANNELS };
- struct xsk_ctx *ctx = xsk->ctx;
- struct ifreq ifr = {};
- int fd, err, ret;
-
- fd = socket(AF_LOCAL, SOCK_DGRAM | SOCK_CLOEXEC, 0);
- if (fd < 0)
- return -errno;
-
- ifr.ifr_data = (void *)&channels;
- bpf_strlcpy(ifr.ifr_name, ctx->ifname, IFNAMSIZ);
- err = ioctl(fd, SIOCETHTOOL, &ifr);
- if (err && errno != EOPNOTSUPP) {
- ret = -errno;
- goto out;
- }
-
- if (err) {
- /* If the device says it has no channels, then all traffic
- * is sent to a single stream, so max queues = 1.
- */
- ret = 1;
- } else {
- /* Take the max of rx, tx, combined. Drivers return
- * the number of channels in different ways.
- */
- ret = max(channels.max_rx, channels.max_tx);
- ret = max(ret, (int)channels.max_combined);
- }
-
-out:
- close(fd);
- return ret;
-}
-
-static int xsk_create_bpf_maps(struct xsk_socket *xsk)
-{
- struct xsk_ctx *ctx = xsk->ctx;
- int max_queues;
- int fd;
-
- max_queues = xsk_get_max_queues(xsk);
- if (max_queues < 0)
- return max_queues;
-
- fd = bpf_map_create(BPF_MAP_TYPE_XSKMAP, "xsks_map",
- sizeof(int), sizeof(int), max_queues, NULL);
- if (fd < 0)
- return fd;
-
- ctx->xsks_map_fd = fd;
-
- return 0;
-}
-
-static void xsk_delete_bpf_maps(struct xsk_socket *xsk)
-{
- struct xsk_ctx *ctx = xsk->ctx;
-
- if (ctx->xsks_map_fd == FD_NOT_USED)
- return;
-
- bpf_map_delete_elem(ctx->xsks_map_fd, &ctx->queue_id);
- close(ctx->xsks_map_fd);
-}
-
-static int xsk_lookup_bpf_maps(struct xsk_socket *xsk)
-{
- __u32 i, *map_ids, num_maps, prog_len = sizeof(struct bpf_prog_info);
- __u32 map_len = sizeof(struct bpf_map_info);
- struct bpf_prog_info prog_info = {};
- struct xsk_ctx *ctx = xsk->ctx;
- struct bpf_map_info map_info;
- int fd, err;
-
- err = bpf_obj_get_info_by_fd(ctx->prog_fd, &prog_info, &prog_len);
- if (err)
- return err;
-
- num_maps = prog_info.nr_map_ids;
-
- map_ids = calloc(prog_info.nr_map_ids, sizeof(*map_ids));
- if (!map_ids)
- return -ENOMEM;
-
- memset(&prog_info, 0, prog_len);
- prog_info.nr_map_ids = num_maps;
- prog_info.map_ids = (__u64)(unsigned long)map_ids;
-
- err = bpf_obj_get_info_by_fd(ctx->prog_fd, &prog_info, &prog_len);
- if (err)
- goto out_map_ids;
-
- ctx->xsks_map_fd = -1;
-
- for (i = 0; i < prog_info.nr_map_ids; i++) {
- fd = bpf_map_get_fd_by_id(map_ids[i]);
- if (fd < 0)
- continue;
-
- memset(&map_info, 0, map_len);
- err = bpf_obj_get_info_by_fd(fd, &map_info, &map_len);
- if (err) {
- close(fd);
- continue;
- }
-
- if (!strncmp(map_info.name, "xsks_map", sizeof(map_info.name))) {
- ctx->xsks_map_fd = fd;
- break;
- }
-
- close(fd);
- }
-
- if (ctx->xsks_map_fd == -1)
- err = -ENOENT;
-
-out_map_ids:
- free(map_ids);
- return err;
-}
-
-static int xsk_set_bpf_maps(struct xsk_socket *xsk)
-{
- struct xsk_ctx *ctx = xsk->ctx;
-
- return bpf_map_update_elem(ctx->xsks_map_fd, &ctx->queue_id,
- &xsk->fd, 0);
-}
-
-static int xsk_link_lookup(int ifindex, __u32 *prog_id, int *link_fd)
-{
- struct bpf_link_info link_info;
- __u32 link_len;
- __u32 id = 0;
- int err;
- int fd;
-
- while (true) {
- err = bpf_link_get_next_id(id, &id);
- if (err) {
- if (errno == ENOENT) {
- err = 0;
- break;
- }
- pr_warn("can't get next link: %s\n", strerror(errno));
- break;
- }
-
- fd = bpf_link_get_fd_by_id(id);
- if (fd < 0) {
- if (errno == ENOENT)
- continue;
- pr_warn("can't get link by id (%u): %s\n", id, strerror(errno));
- err = -errno;
- break;
- }
-
- link_len = sizeof(struct bpf_link_info);
- memset(&link_info, 0, link_len);
- err = bpf_obj_get_info_by_fd(fd, &link_info, &link_len);
- if (err) {
- pr_warn("can't get link info: %s\n", strerror(errno));
- close(fd);
- break;
- }
- if (link_info.type == BPF_LINK_TYPE_XDP) {
- if (link_info.xdp.ifindex == ifindex) {
- *link_fd = fd;
- if (prog_id)
- *prog_id = link_info.prog_id;
- break;
- }
- }
- close(fd);
- }
-
- return err;
-}
-
-static bool xsk_probe_bpf_link(void)
-{
- LIBBPF_OPTS(bpf_link_create_opts, opts, .flags = XDP_FLAGS_SKB_MODE);
- struct bpf_insn insns[2] = {
- BPF_MOV64_IMM(BPF_REG_0, XDP_PASS),
- BPF_EXIT_INSN()
- };
- int prog_fd, link_fd = -1, insn_cnt = ARRAY_SIZE(insns);
- int ifindex_lo = 1;
- bool ret = false;
- int err;
-
- err = xsk_link_lookup(ifindex_lo, NULL, &link_fd);
- if (err)
- return ret;
-
- if (link_fd >= 0)
- return true;
-
- prog_fd = bpf_prog_load(BPF_PROG_TYPE_XDP, NULL, "GPL", insns, insn_cnt, NULL);
- if (prog_fd < 0)
- return ret;
-
- link_fd = bpf_link_create(prog_fd, ifindex_lo, BPF_XDP, &opts);
- close(prog_fd);
-
- if (link_fd >= 0) {
- ret = true;
- close(link_fd);
- }
-
- return ret;
-}
-
-static int xsk_create_xsk_struct(int ifindex, struct xsk_socket *xsk)
-{
- char ifname[IFNAMSIZ];
- struct xsk_ctx *ctx;
- char *interface;
-
- ctx = calloc(1, sizeof(*ctx));
- if (!ctx)
- return -ENOMEM;
-
- interface = if_indextoname(ifindex, &ifname[0]);
- if (!interface) {
- free(ctx);
- return -errno;
- }
-
- ctx->ifindex = ifindex;
- bpf_strlcpy(ctx->ifname, ifname, IFNAMSIZ);
-
- xsk->ctx = ctx;
- xsk->ctx->has_bpf_link = xsk_probe_bpf_link();
-
- return 0;
-}
-
-static int xsk_init_xdp_res(struct xsk_socket *xsk,
- int *xsks_map_fd)
-{
- struct xsk_ctx *ctx = xsk->ctx;
- int err;
-
- err = xsk_create_bpf_maps(xsk);
- if (err)
- return err;
-
- err = __xsk_load_xdp_prog(*xsks_map_fd);
- if (err)
- goto err_load_xdp_prog;
-
- if (ctx->has_bpf_link)
- err = xsk_create_bpf_link(xsk);
- else
- err = bpf_xdp_attach(xsk->ctx->ifindex, ctx->prog_fd,
- xsk->config.xdp_flags, NULL);
-
- if (err)
- goto err_attach_xdp_prog;
-
- if (!xsk->rx)
- return err;
-
- err = xsk_set_bpf_maps(xsk);
- if (err)
- goto err_set_bpf_maps;
-
- return err;
-
-err_set_bpf_maps:
- if (ctx->has_bpf_link)
- close(ctx->link_fd);
- else
- bpf_xdp_detach(ctx->ifindex, 0, NULL);
-err_attach_xdp_prog:
- close(ctx->prog_fd);
-err_load_xdp_prog:
- xsk_delete_bpf_maps(xsk);
- return err;
-}
-
-static int xsk_lookup_xdp_res(struct xsk_socket *xsk, int *xsks_map_fd, int prog_id)
-{
- struct xsk_ctx *ctx = xsk->ctx;
- int err;
-
- ctx->prog_fd = bpf_prog_get_fd_by_id(prog_id);
- if (ctx->prog_fd < 0) {
- err = -errno;
- goto err_prog_fd;
- }
- err = xsk_lookup_bpf_maps(xsk);
- if (err)
- goto err_lookup_maps;
-
- if (!xsk->rx)
- return err;
-
- err = xsk_set_bpf_maps(xsk);
- if (err)
- goto err_set_maps;
-
- return err;
-
-err_set_maps:
- close(ctx->xsks_map_fd);
-err_lookup_maps:
- close(ctx->prog_fd);
-err_prog_fd:
- if (ctx->has_bpf_link)
- close(ctx->link_fd);
- return err;
-}
-
-static int __xsk_setup_xdp_prog(struct xsk_socket *_xdp, int *xsks_map_fd)
-{
- struct xsk_socket *xsk = _xdp;
- struct xsk_ctx *ctx = xsk->ctx;
- __u32 prog_id = 0;
- int err;
-
- if (ctx->has_bpf_link)
- err = xsk_link_lookup(ctx->ifindex, &prog_id, &ctx->link_fd);
- else
- err = bpf_xdp_query_id(ctx->ifindex, xsk->config.xdp_flags, &prog_id);
-
- if (err)
- return err;
-
- err = !prog_id ? xsk_init_xdp_res(xsk, xsks_map_fd) :
- xsk_lookup_xdp_res(xsk, xsks_map_fd, prog_id);
-
- if (!err && xsks_map_fd)
- *xsks_map_fd = ctx->xsks_map_fd;
-
- return err;
-}
-
-int xsk_setup_xdp_prog_xsk(struct xsk_socket *xsk, int *xsks_map_fd)
-{
- return __xsk_setup_xdp_prog(xsk, xsks_map_fd);
-}
-
int xsk_load_xdp_program(int *xsk_map_fd, int *prog_fd)
{
*xsk_map_fd = bpf_map_create(BPF_MAP_TYPE_XSKMAP, "xsks_map", sizeof(int), sizeof(int),
@@ -988,51 +423,13 @@ static struct xsk_ctx *xsk_create_ctx(struct xsk_socket *xsk,
ctx->refcount = 1;
ctx->umem = umem;
ctx->queue_id = queue_id;
- ctx->prog_fd = FD_NOT_USED;
- ctx->link_fd = FD_NOT_USED;
- ctx->xsks_map_fd = FD_NOT_USED;
ctx->fill = fill;
ctx->comp = comp;
list_add(&ctx->list, &umem->ctx_list);
- ctx->has_bpf_link = xsk_probe_bpf_link();
return ctx;
}
-static void xsk_destroy_xsk_struct(struct xsk_socket *xsk)
-{
- free(xsk->ctx);
- free(xsk);
-}
-
-int xsk_socket__update_xskmap(struct xsk_socket *xsk, int fd)
-{
- xsk->ctx->xsks_map_fd = fd;
- return xsk_set_bpf_maps(xsk);
-}
-
-int xsk_setup_xdp_prog(int ifindex, int *xsks_map_fd)
-{
- struct xsk_socket *xsk;
- int res;
-
- xsk = calloc(1, sizeof(*xsk));
- if (!xsk)
- return -ENOMEM;
-
- res = xsk_create_xsk_struct(ifindex, xsk);
- if (res) {
- free(xsk);
- return -EINVAL;
- }
-
- res = __xsk_setup_xdp_prog(xsk, xsks_map_fd);
-
- xsk_destroy_xsk_struct(xsk);
-
- return res;
-}
-
int xsk_socket__create_shared(struct xsk_socket **xsk_ptr,
int ifindex,
__u32 queue_id, struct xsk_umem *umem,
@@ -1255,14 +652,6 @@ void xsk_socket__delete(struct xsk_socket *xsk)
ctx = xsk->ctx;
umem = ctx->umem;
- if (ctx->refcount == 1) {
- xsk_delete_bpf_maps(xsk);
- if (ctx->prog_fd != FD_NOT_USED)
- close(ctx->prog_fd);
- if (ctx->has_bpf_link && ctx->link_fd != FD_NOT_USED)
- close(ctx->link_fd);
- }
-
xsk_put_ctx(ctx, true);
err = xsk_get_mmap_offsets(xsk->fd, &off);
diff --git a/tools/testing/selftests/bpf/xsk.h b/tools/testing/selftests/bpf/xsk.h
index 7a5aeacd261b..bd5b55ad9f8a 100644
--- a/tools/testing/selftests/bpf/xsk.h
+++ b/tools/testing/selftests/bpf/xsk.h
@@ -197,21 +197,12 @@ struct xsk_umem_config {
__u32 flags;
};
-int xsk_setup_xdp_prog_xsk(struct xsk_socket *xsk, int *xsks_map_fd);
-int xsk_setup_xdp_prog(int ifindex, int *xsks_map_fd);
-int xsk_socket__update_xskmap(struct xsk_socket *xsk, int xsks_map_fd);
-
-/* Flags for the libbpf_flags field. */
-#define XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD (1 << 0)
-
int xsk_load_xdp_program(int *xsk_map_fd, int *prog_fd);
int xsk_attach_xdp_program(int ifindex, int prog_fd, u32 xdp_flags);
struct xsk_socket_config {
__u32 rx_size;
__u32 tx_size;
- __u32 libbpf_flags;
- __u32 xdp_flags;
__u16 bind_flags;
};
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 0c0974c209cd..693f8a63f718 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -102,12 +102,6 @@
#include <linux/filter.h>
#include "../kselftest.h"
-/* AF_XDP APIs were moved into libxdp and marked as deprecated in libbpf.
- * Until xskxceiver is either moved or re-writed into libxdp, suppress
- * deprecation warnings in this file
- */
-#pragma GCC diagnostic ignored "-Wdeprecated-declarations"
-
static const char *MAC1 = "\x00\x0A\x56\x9E\xEE\x62";
static const char *MAC2 = "\x00\x0A\x56\x9E\xEE\x61";
static const char *IP1 = "192.168.100.162";
@@ -326,8 +320,6 @@ static int __xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_i
xsk->umem = umem;
cfg.rx_size = xsk->rxqsize;
cfg.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
- cfg.libbpf_flags = XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD;
- cfg.xdp_flags = ifobject->xdp_flags;
cfg.bind_flags = ifobject->bind_flags;
if (shared)
cfg.bind_flags |= XDP_SHARED_UMEM;
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (9 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 10/15] selftests/xsk: remove unnecessary code in control path Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 18:14 ` David Vernet
2023-01-04 23:15 ` Andrii Nakryiko
2023-01-04 12:17 ` [PATCH bpf-next v2 12/15] selftests/xsk: add test when some packets are XDP_DROPed Magnus Karlsson
` (4 subsequent siblings)
15 siblings, 2 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Get rid of the built-in XDP program that was part of the old libbpf
code in xsk.c and replace it with an eBPF program build using the
framework by all the other bpf selftests. This will form the base for
adding more programs in later commits.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/Makefile | 2 +-
.../selftests/bpf/progs/xsk_xdp_progs.c | 19 ++++
tools/testing/selftests/bpf/xsk.c | 88 ++++---------------
tools/testing/selftests/bpf/xsk.h | 6 +-
tools/testing/selftests/bpf/xskxceiver.c | 72 ++++++++-------
tools/testing/selftests/bpf/xskxceiver.h | 7 +-
6 files changed, 88 insertions(+), 106 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 205e8c3c346a..a0193a8f9da6 100644
--- a/tools/testing/selftests/bpf/Makefile
+++ b/tools/testing/selftests/bpf/Makefile
@@ -240,7 +240,7 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
$(OUTPUT)/test_maps: $(TESTING_HELPERS)
$(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
$(OUTPUT)/xsk.o: $(BPFOBJ)
-$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o
+$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h
BPFTOOL ?= $(DEFAULT_BPFTOOL)
$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
new file mode 100644
index 000000000000..698176882ac6
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2022 Intel */
+
+#include <linux/bpf.h>
+#include <bpf/bpf_helpers.h>
+
+struct {
+ __uint(type, BPF_MAP_TYPE_XSKMAP);
+ __uint(max_entries, 1);
+ __uint(key_size, sizeof(int));
+ __uint(value_size, sizeof(int));
+} xsk SEC(".maps");
+
+SEC("xdp") int xsk_def_prog(struct xdp_md *xdp)
+{
+ return bpf_redirect_map(&xsk, 0, XDP_DROP);
+}
+
+char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/xsk.c b/tools/testing/selftests/bpf/xsk.c
index 9ed31d280e48..dc6b47280ec4 100644
--- a/tools/testing/selftests/bpf/xsk.c
+++ b/tools/testing/selftests/bpf/xsk.c
@@ -267,87 +267,37 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area,
return err;
}
-static int __xsk_load_xdp_prog(int xsk_map_fd)
+int xsk_attach_xdp_program(struct bpf_program *prog, int ifindex, u32 xdp_flags)
{
- static const int log_buf_size = 16 * 1024;
- char log_buf[log_buf_size];
int prog_fd;
- /* This is the post-5.3 kernel C-program:
- * SEC("xdp_sock") int xdp_sock_prog(struct xdp_md *ctx)
- * {
- * return bpf_redirect_map(&xsks_map, ctx->rx_queue_index, XDP_PASS);
- * }
- */
- struct bpf_insn prog[] = {
- /* r2 = *(u32 *)(r1 + 16) */
- BPF_LDX_MEM(BPF_W, BPF_REG_2, BPF_REG_1, 16),
- /* r1 = xskmap[] */
- BPF_LD_MAP_FD(BPF_REG_1, xsk_map_fd),
- /* r3 = XDP_PASS */
- BPF_MOV64_IMM(BPF_REG_3, 2),
- /* call bpf_redirect_map */
- BPF_EMIT_CALL(BPF_FUNC_redirect_map),
- BPF_EXIT_INSN(),
- };
- size_t insns_cnt = ARRAY_SIZE(prog);
- LIBBPF_OPTS(bpf_prog_load_opts, opts,
- .log_buf = log_buf,
- .log_size = log_buf_size,
- );
-
- prog_fd = bpf_prog_load(BPF_PROG_TYPE_XDP, NULL, "LGPL-2.1 or BSD-2-Clause",
- prog, insns_cnt, &opts);
- if (prog_fd < 0)
- pr_warn("BPF log buffer:\n%s", log_buf);
-
- return prog_fd;
+ prog_fd = bpf_program__fd(prog);
+ return bpf_xdp_attach(ifindex, prog_fd, xdp_flags, NULL);
}
-int xsk_attach_xdp_program(int ifindex, int prog_fd, u32 xdp_flags)
+void xsk_detach_xdp_program(int ifindex, u32 xdp_flags)
{
- DECLARE_LIBBPF_OPTS(bpf_link_create_opts, opts);
- __u32 prog_id = 0;
- int link_fd;
- int err;
-
- err = bpf_xdp_query_id(ifindex, xdp_flags, &prog_id);
- if (err) {
- pr_warn("getting XDP prog id failed\n");
- return err;
- }
-
- /* If there's a netlink-based XDP prog loaded on interface, bail out
- * and ask user to do the removal by himself
- */
- if (prog_id) {
- pr_warn("Netlink-based XDP prog detected, please unload it in order to launch AF_XDP prog\n");
- return -EINVAL;
- }
-
- opts.flags = xdp_flags & ~(XDP_FLAGS_UPDATE_IF_NOEXIST | XDP_FLAGS_REPLACE);
+ bpf_xdp_detach(ifindex, xdp_flags, NULL);
+}
- link_fd = bpf_link_create(prog_fd, ifindex, BPF_XDP, &opts);
- if (link_fd < 0)
- pr_warn("bpf_link_create failed: %s\n", strerror(errno));
+void xsk_clear_xskmap(struct bpf_map *map)
+{
+ u32 index = 0;
+ int map_fd;
- return link_fd;
+ map_fd = bpf_map__fd(map);
+ bpf_map_delete_elem(map_fd, &index);
}
-int xsk_load_xdp_program(int *xsk_map_fd, int *prog_fd)
+int xsk_update_xskmap(struct bpf_map *map, struct xsk_socket *xsk)
{
- *xsk_map_fd = bpf_map_create(BPF_MAP_TYPE_XSKMAP, "xsks_map", sizeof(int), sizeof(int),
- XSKMAP_SIZE, NULL);
- if (*xsk_map_fd < 0)
- return *xsk_map_fd;
-
- *prog_fd = __xsk_load_xdp_prog(*xsk_map_fd);
- if (*prog_fd < 0) {
- close(*xsk_map_fd);
- return *prog_fd;
- }
+ int map_fd, sock_fd;
+ u32 index = 0;
- return 0;
+ map_fd = bpf_map__fd(map);
+ sock_fd = xsk_socket__fd(xsk);
+
+ return bpf_map_update_elem(map_fd, &index, &sock_fd, 0);
}
static struct xsk_ctx *xsk_get_ctx(struct xsk_umem *umem, int ifindex,
diff --git a/tools/testing/selftests/bpf/xsk.h b/tools/testing/selftests/bpf/xsk.h
index bd5b55ad9f8a..5624d31b8db7 100644
--- a/tools/testing/selftests/bpf/xsk.h
+++ b/tools/testing/selftests/bpf/xsk.h
@@ -197,8 +197,10 @@ struct xsk_umem_config {
__u32 flags;
};
-int xsk_load_xdp_program(int *xsk_map_fd, int *prog_fd);
-int xsk_attach_xdp_program(int ifindex, int prog_fd, u32 xdp_flags);
+int xsk_attach_xdp_program(struct bpf_program *prog, int ifindex, u32 xdp_flags);
+void xsk_detach_xdp_program(int ifindex, u32 xdp_flags);
+int xsk_update_xskmap(struct bpf_map *map, struct xsk_socket *xsk);
+void xsk_clear_xskmap(struct bpf_map *map);
struct xsk_socket_config {
__u32 rx_size;
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 693f8a63f718..d69100267f70 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1207,7 +1207,7 @@ static void thread_common_ops_tx(struct test_spec *test, struct ifobject *ifobje
{
xsk_configure_socket(test, ifobject, test->ifobj_rx->umem, true);
ifobject->xsk = &ifobject->xsk_arr[0];
- ifobject->xsk_map_fd = test->ifobj_rx->xsk_map_fd;
+ ifobject->xskmap = test->ifobj_rx->xskmap;
memcpy(ifobject->umem, test->ifobj_rx->umem, sizeof(struct xsk_umem_info));
}
@@ -1247,9 +1247,8 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
LIBBPF_OPTS(bpf_xdp_query_opts, opts);
- u32 queue_id = 0;
- int ret, fd;
void *bufs;
+ int ret;
if (ifobject->umem->unaligned_mode)
mmap_flags |= MAP_HUGETLB;
@@ -1274,8 +1273,7 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
if (!ifobject->rx_on)
return;
- fd = xsk_socket__fd(ifobject->xsk->xsk);
- ret = bpf_map_update_elem(ifobject->xsk_map_fd, &queue_id, &fd, 0);
+ ret = xsk_update_xskmap(ifobject->xskmap, ifobject->xsk->xsk);
if (ret)
exit_with_error(errno);
}
@@ -1309,18 +1307,17 @@ static void *worker_testapp_validate_rx(void *arg)
{
struct test_spec *test = (struct test_spec *)arg;
struct ifobject *ifobject = test->ifobj_rx;
- int id = 0, err, fd = xsk_socket__fd(ifobject->xsk->xsk);
struct pollfd fds = { };
- u32 queue_id = 0;
+ int err;
if (test->current_step == 1) {
thread_common_ops(test, ifobject);
} else {
- bpf_map_delete_elem(ifobject->xsk_map_fd, &id);
- err = bpf_map_update_elem(ifobject->xsk_map_fd, &queue_id, &fd, 0);
+ xsk_clear_xskmap(ifobject->xskmap);
+ err = xsk_update_xskmap(ifobject->xskmap, ifobject->xsk->xsk);
if (err) {
- printf("Error: Failed to update xskmap, error %s\n", strerror(err));
- exit_with_error(err);
+ printf("Error: Failed to update xskmap, error %s\n", strerror(-err));
+ exit_with_error(-err);
}
}
@@ -1390,10 +1387,8 @@ static int testapp_validate_traffic_single_thread(struct test_spec *test, struct
pthread_join(t0, NULL);
if (test->total_steps == test->current_step || test->fail) {
- u32 queue_id = 0;
-
xsk_socket__delete(ifobj->xsk->xsk);
- bpf_map_delete_elem(ifobj->xsk_map_fd, &queue_id);
+ xsk_clear_xskmap(ifobj->xskmap);
testapp_clean_xsk_umem(ifobj);
}
@@ -1482,14 +1477,14 @@ static void testapp_bidi(struct test_spec *test)
static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj_rx)
{
- int ret, queue_id = 0, fd = xsk_socket__fd(ifobj_rx->xsk->xsk);
+ int ret;
xsk_socket__delete(ifobj_tx->xsk->xsk);
xsk_socket__delete(ifobj_rx->xsk->xsk);
ifobj_tx->xsk = &ifobj_tx->xsk_arr[1];
ifobj_rx->xsk = &ifobj_rx->xsk_arr[1];
- ret = bpf_map_update_elem(ifobj_rx->xsk_map_fd, &queue_id, &fd, 0);
+ ret = xsk_update_xskmap(ifobj_rx->xskmap, ifobj_rx->xsk->xsk);
if (ret)
exit_with_error(errno);
}
@@ -1651,12 +1646,26 @@ static void testapp_invalid_desc(struct test_spec *test)
pkt_stream_restore_default(test);
}
+static int xsk_load_xdp_programs(struct ifobject *ifobj)
+{
+ ifobj->xdp_progs = xsk_xdp_progs__open_and_load();
+ if (libbpf_get_error(ifobj->xdp_progs))
+ return libbpf_get_error(ifobj->xdp_progs);
+
+ return 0;
+}
+
+static void xsk_unload_xdp_programs(struct ifobject *ifobj)
+{
+ xsk_xdp_progs__destroy(ifobj->xdp_progs);
+}
+
static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
const char *dst_ip, const char *src_ip, const u16 dst_port,
const u16 src_port, thread_func_t func_ptr, bool load_xdp)
{
- int xsk_map_fd, prog_fd, err;
struct in_addr ip;
+ int err;
memcpy(ifobj->dst_mac, dst_mac, ETH_ALEN);
memcpy(ifobj->src_mac, src_mac, ETH_ALEN);
@@ -1675,20 +1684,20 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
if (!load_xdp)
return;
- err = xsk_load_xdp_program(&xsk_map_fd, &prog_fd);
+ err = xsk_load_xdp_programs(ifobj);
if (err) {
printf("Error loading XDP program\n");
exit_with_error(err);
}
- ifobj->xsk_map_fd = xsk_map_fd;
- ifobj->prog_fd = prog_fd;
ifobj->xdp_flags = mode_to_xdp_flags(TEST_MODE_SKB);
- ifobj->link_fd = xsk_attach_xdp_program(ifobj->ifindex, prog_fd, ifobj->xdp_flags);
- if (ifobj->link_fd < 0) {
+ err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
+ ifobj->xdp_flags);
+ if (err) {
printf("Error attaching XDP program\n");
- exit_with_error(ifobj->link_fd);
+ exit_with_error(-err);
}
+ ifobj->xskmap = ifobj->xdp_progs->maps.xsk;
}
static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
@@ -1823,9 +1832,6 @@ static struct ifobject *ifobject_create(void)
static void ifobject_delete(struct ifobject *ifobj)
{
- close(ifobj->prog_fd);
- close(ifobj->xsk_map_fd);
-
free(ifobj->umem);
free(ifobj->xsk_arr);
free(ifobj);
@@ -1864,13 +1870,15 @@ static void change_to_drv_mode(struct ifobject *ifobj)
LIBBPF_OPTS(bpf_xdp_query_opts, opts);
int ret;
- close(ifobj->link_fd);
- ifobj->link_fd = xsk_attach_xdp_program(ifobj->ifindex, ifobj->prog_fd,
- XDP_FLAGS_DRV_MODE);
- if (ifobj->link_fd < 0) {
+ xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
+ ifobj->xdp_flags = XDP_FLAGS_DRV_MODE;
+ ret = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
+ ifobj->xdp_flags);
+ if (ret) {
ksft_print_msg("Error attaching XDP program\n");
- exit_with_error(-ifobj->link_fd);
+ exit_with_error(-ret);
}
+ ifobj->xskmap = ifobj->xdp_progs->maps.xsk;
ret = bpf_xdp_query(ifobj->ifindex, XDP_FLAGS_DRV_MODE, &opts);
if (ret)
@@ -1955,6 +1963,8 @@ int main(int argc, char **argv)
pkt_stream_delete(tx_pkt_stream_default);
pkt_stream_delete(rx_pkt_stream_default);
+ xsk_unload_xdp_programs(ifobj_tx);
+ xsk_unload_xdp_programs(ifobj_rx);
ifobject_delete(ifobj_tx);
ifobject_delete(ifobj_rx);
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index b2ba877b1966..70b3e5d1d40c 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -5,6 +5,8 @@
#ifndef XSKXCEIVER_H_
#define XSKXCEIVER_H_
+#include "xsk_xdp_progs.skel.h"
+
#ifndef SOL_XDP
#define SOL_XDP 283
#endif
@@ -138,9 +140,8 @@ struct ifobject {
thread_func_t func_ptr;
validation_func_t validation_func;
struct pkt_stream *pkt_stream;
- int xsk_map_fd;
- int prog_fd;
- int link_fd;
+ struct xsk_xdp_progs *xdp_progs;
+ struct bpf_map *xskmap;
int ifindex;
u32 dst_ip;
u32 src_ip;
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 12/15] selftests/xsk: add test when some packets are XDP_DROPed
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (10 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 13/15] selftests/xsk: merge dual and single thread dispatchers Magnus Karlsson
` (3 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Add a new test where some of the packets are not passed to the AF_XDP
socket and instead get a XDP_DROP verdict. This is important as it
tests the recycling mechanism of the buffer pool. If a packet is not
sent to the AF_XDP socket, the buffer the packet resides in is instead
recycled so it can be used again without the round-trip to user
space. The test introduces a new XDP program that drops every other
packet.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
.../selftests/bpf/progs/xsk_xdp_progs.c | 11 +++++++
tools/testing/selftests/bpf/xskxceiver.c | 31 +++++++++++++++++++
tools/testing/selftests/bpf/xskxceiver.h | 1 +
3 files changed, 43 insertions(+)
diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
index 698176882ac6..744a01d0e57d 100644
--- a/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
+++ b/tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
@@ -11,9 +11,20 @@ struct {
__uint(value_size, sizeof(int));
} xsk SEC(".maps");
+static unsigned int idx;
+
SEC("xdp") int xsk_def_prog(struct xdp_md *xdp)
{
return bpf_redirect_map(&xsk, 0, XDP_DROP);
}
+SEC("xdp") int xsk_xdp_drop(struct xdp_md *xdp)
+{
+ /* Drop every other packet */
+ if (idx++ % 2)
+ return XDP_DROP;
+
+ return bpf_redirect_map(&xsk, 0, XDP_DROP);
+}
+
char _license[] SEC("license") = "GPL";
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index d69100267f70..a33f11b4c598 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1646,6 +1646,34 @@ static void testapp_invalid_desc(struct test_spec *test)
pkt_stream_restore_default(test);
}
+static void testapp_xdp_drop(struct test_spec *test)
+{
+ struct ifobject *ifobj = test->ifobj_rx;
+ int err;
+
+ test_spec_set_name(test, "XDP_DROP_HALF");
+ xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
+ err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_xdp_drop, ifobj->ifindex,
+ ifobj->xdp_flags);
+ if (err) {
+ printf("Error attaching XDP_DROP program\n");
+ test->fail = true;
+ return;
+ }
+
+ pkt_stream_receive_half(test);
+ testapp_validate_traffic(test);
+
+ pkt_stream_restore_default(test);
+ xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
+ err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
+ ifobj->xdp_flags);
+ if (err) {
+ printf("Error restoring default XDP program\n");
+ exit_with_error(-err);
+ }
+}
+
static int xsk_load_xdp_programs(struct ifobject *ifobj)
{
ifobj->xdp_progs = xsk_xdp_progs__open_and_load();
@@ -1796,6 +1824,9 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
case TEST_TYPE_HEADROOM:
testapp_headroom(test);
break;
+ case TEST_TYPE_XDP_DROP_HALF:
+ testapp_xdp_drop(test);
+ break;
default:
break;
}
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index 70b3e5d1d40c..a4daa5134fc0 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -87,6 +87,7 @@ enum test_type {
TEST_TYPE_STATS_RX_FULL,
TEST_TYPE_STATS_FILL_EMPTY,
TEST_TYPE_BPF_RES,
+ TEST_TYPE_XDP_DROP_HALF,
TEST_TYPE_MAX
};
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 13/15] selftests/xsk: merge dual and single thread dispatchers
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (11 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 12/15] selftests/xsk: add test when some packets are XDP_DROPed Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 14/15] selftests/xsk: automatically restore packet stream Magnus Karlsson
` (2 subsequent siblings)
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Make the thread dispatching code common by unifying the dual and
single thread dispatcher code. This so we do not have to add code in
two places in upcoming commits.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xskxceiver.c | 120 ++++++++++-------------
1 file changed, 54 insertions(+), 66 deletions(-)
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index a33f11b4c598..11e4f29d40f7 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1356,83 +1356,59 @@ static void handler(int signum)
pthread_exit(NULL);
}
-static int testapp_validate_traffic_single_thread(struct test_spec *test, struct ifobject *ifobj,
- enum test_type type)
+static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *ifobj1,
+ struct ifobject *ifobj2)
{
- bool old_shared_umem = ifobj->shared_umem;
- pthread_t t0;
+ pthread_t t0, t1;
- if (pthread_barrier_init(&barr, NULL, 2))
- exit_with_error(errno);
+ if (ifobj2)
+ if (pthread_barrier_init(&barr, NULL, 2))
+ exit_with_error(errno);
test->current_step++;
- if (type == TEST_TYPE_POLL_RXQ_TMOUT)
- pkt_stream_reset(ifobj->pkt_stream);
+ pkt_stream_reset(ifobj1->pkt_stream);
pkts_in_flight = 0;
- test->ifobj_rx->shared_umem = false;
- test->ifobj_tx->shared_umem = false;
-
signal(SIGUSR1, handler);
- /* Spawn thread */
- pthread_create(&t0, NULL, ifobj->func_ptr, test);
+ /*Spawn RX thread */
+ pthread_create(&t0, NULL, ifobj1->func_ptr, test);
- if (type != TEST_TYPE_POLL_TXQ_TMOUT)
+ if (ifobj2) {
pthread_barrier_wait(&barr);
+ if (pthread_barrier_destroy(&barr))
+ exit_with_error(errno);
- if (pthread_barrier_destroy(&barr))
- exit_with_error(errno);
+ /*Spawn TX thread */
+ pthread_create(&t1, NULL, ifobj2->func_ptr, test);
- pthread_kill(t0, SIGUSR1);
- pthread_join(t0, NULL);
+ pthread_join(t1, NULL);
+ }
+
+ if (!ifobj2)
+ pthread_kill(t0, SIGUSR1);
+ else
+ pthread_join(t0, NULL);
if (test->total_steps == test->current_step || test->fail) {
- xsk_socket__delete(ifobj->xsk->xsk);
- xsk_clear_xskmap(ifobj->xskmap);
- testapp_clean_xsk_umem(ifobj);
+ if (ifobj2)
+ xsk_socket__delete(ifobj2->xsk->xsk);
+ xsk_socket__delete(ifobj1->xsk->xsk);
+ testapp_clean_xsk_umem(ifobj1);
+ if (ifobj2 && !ifobj2->shared_umem)
+ testapp_clean_xsk_umem(ifobj2);
}
- test->ifobj_rx->shared_umem = old_shared_umem;
- test->ifobj_tx->shared_umem = old_shared_umem;
-
return !!test->fail;
}
static int testapp_validate_traffic(struct test_spec *test)
{
- struct ifobject *ifobj_tx = test->ifobj_tx;
- struct ifobject *ifobj_rx = test->ifobj_rx;
- pthread_t t0, t1;
-
- if (pthread_barrier_init(&barr, NULL, 2))
- exit_with_error(errno);
-
- test->current_step++;
- pkt_stream_reset(ifobj_rx->pkt_stream);
- pkts_in_flight = 0;
-
- /*Spawn RX thread */
- pthread_create(&t0, NULL, ifobj_rx->func_ptr, test);
-
- pthread_barrier_wait(&barr);
- if (pthread_barrier_destroy(&barr))
- exit_with_error(errno);
-
- /*Spawn TX thread */
- pthread_create(&t1, NULL, ifobj_tx->func_ptr, test);
-
- pthread_join(t1, NULL);
- pthread_join(t0, NULL);
-
- if (test->total_steps == test->current_step || test->fail) {
- xsk_socket__delete(ifobj_tx->xsk->xsk);
- xsk_socket__delete(ifobj_rx->xsk->xsk);
- testapp_clean_xsk_umem(ifobj_rx);
- if (!ifobj_tx->shared_umem)
- testapp_clean_xsk_umem(ifobj_tx);
- }
+ return __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
+}
- return !!test->fail;
+static int testapp_validate_traffic_single_thread(struct test_spec *test, struct ifobject *ifobj)
+{
+ return __testapp_validate_traffic(test, ifobj, NULL);
}
static void testapp_teardown(struct test_spec *test)
@@ -1674,6 +1650,26 @@ static void testapp_xdp_drop(struct test_spec *test)
}
}
+static void testapp_poll_txq_tmout(struct test_spec *test)
+{
+ test_spec_set_name(test, "POLL_TXQ_FULL");
+
+ test->ifobj_tx->use_poll = true;
+ /* create invalid frame by set umem frame_size and pkt length equal to 2048 */
+ test->ifobj_tx->umem->frame_size = 2048;
+ pkt_stream_replace(test, 2 * DEFAULT_PKT_CNT, 2048);
+ testapp_validate_traffic_single_thread(test, test->ifobj_tx);
+
+ pkt_stream_restore_default(test);
+}
+
+static void testapp_poll_rxq_tmout(struct test_spec *test)
+{
+ test_spec_set_name(test, "POLL_RXQ_EMPTY");
+ test->ifobj_rx->use_poll = true;
+ testapp_validate_traffic_single_thread(test, test->ifobj_rx);
+}
+
static int xsk_load_xdp_programs(struct ifobject *ifobj)
{
ifobj->xdp_progs = xsk_xdp_progs__open_and_load();
@@ -1784,18 +1780,10 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
testapp_validate_traffic(test);
break;
case TEST_TYPE_POLL_TXQ_TMOUT:
- test_spec_set_name(test, "POLL_TXQ_FULL");
- test->ifobj_tx->use_poll = true;
- /* create invalid frame by set umem frame_size and pkt length equal to 2048 */
- test->ifobj_tx->umem->frame_size = 2048;
- pkt_stream_replace(test, 2 * DEFAULT_PKT_CNT, 2048);
- testapp_validate_traffic_single_thread(test, test->ifobj_tx, type);
- pkt_stream_restore_default(test);
+ testapp_poll_txq_tmout(test);
break;
case TEST_TYPE_POLL_RXQ_TMOUT:
- test_spec_set_name(test, "POLL_RXQ_EMPTY");
- test->ifobj_rx->use_poll = true;
- testapp_validate_traffic_single_thread(test, test->ifobj_rx, type);
+ testapp_poll_rxq_tmout(test);
break;
case TEST_TYPE_ALIGNED_INV_DESC:
test_spec_set_name(test, "ALIGNED_INV_DESC");
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 14/15] selftests/xsk: automatically restore packet stream
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (12 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 13/15] selftests/xsk: merge dual and single thread dispatchers Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 15/15] selftests/xsk: automatically switch XDP programs Magnus Karlsson
2023-01-10 12:00 ` [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new " Maciej Fijalkowski
15 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Automatically restore the default packet stream if needed at the end
of each test. This so that test writers do not forget to do this.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xskxceiver.c | 15 +--------------
1 file changed, 1 insertion(+), 14 deletions(-)
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 11e4f29d40f7..66863504c76a 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1501,8 +1501,6 @@ static void testapp_stats_tx_invalid_descs(struct test_spec *test)
pkt_stream_replace_half(test, XSK_UMEM__INVALID_FRAME_SIZE, 0);
test->ifobj_tx->validation_func = validate_tx_invalid_descs;
testapp_validate_traffic(test);
-
- pkt_stream_restore_default(test);
}
static void testapp_stats_rx_full(struct test_spec *test)
@@ -1518,8 +1516,6 @@ static void testapp_stats_rx_full(struct test_spec *test)
test->ifobj_rx->release_rx = false;
test->ifobj_rx->validation_func = validate_rx_full;
testapp_validate_traffic(test);
-
- pkt_stream_restore_default(test);
}
static void testapp_stats_fill_empty(struct test_spec *test)
@@ -1534,8 +1530,6 @@ static void testapp_stats_fill_empty(struct test_spec *test)
test->ifobj_rx->use_fill_ring = false;
test->ifobj_rx->validation_func = validate_fill_empty;
testapp_validate_traffic(test);
-
- pkt_stream_restore_default(test);
}
/* Simple test */
@@ -1568,7 +1562,6 @@ static bool testapp_unaligned(struct test_spec *test)
test->ifobj_rx->pkt_stream->use_addr_for_fill = true;
testapp_validate_traffic(test);
- pkt_stream_restore_default(test);
return true;
}
@@ -1578,7 +1571,6 @@ static void testapp_single_pkt(struct test_spec *test)
pkt_stream_generate_custom(test, pkts, ARRAY_SIZE(pkts));
testapp_validate_traffic(test);
- pkt_stream_restore_default(test);
}
static void testapp_invalid_desc(struct test_spec *test)
@@ -1619,7 +1611,6 @@ static void testapp_invalid_desc(struct test_spec *test)
pkt_stream_generate_custom(test, pkts, ARRAY_SIZE(pkts));
testapp_validate_traffic(test);
- pkt_stream_restore_default(test);
}
static void testapp_xdp_drop(struct test_spec *test)
@@ -1640,7 +1631,6 @@ static void testapp_xdp_drop(struct test_spec *test)
pkt_stream_receive_half(test);
testapp_validate_traffic(test);
- pkt_stream_restore_default(test);
xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
ifobj->xdp_flags);
@@ -1659,8 +1649,6 @@ static void testapp_poll_txq_tmout(struct test_spec *test)
test->ifobj_tx->umem->frame_size = 2048;
pkt_stream_replace(test, 2 * DEFAULT_PKT_CNT, 2048);
testapp_validate_traffic_single_thread(test, test->ifobj_tx);
-
- pkt_stream_restore_default(test);
}
static void testapp_poll_rxq_tmout(struct test_spec *test)
@@ -1766,8 +1754,6 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
test->ifobj_rx->umem->frame_size = 2048;
pkt_stream_replace(test, DEFAULT_PKT_CNT, PKT_SIZE);
testapp_validate_traffic(test);
-
- pkt_stream_restore_default(test);
break;
case TEST_TYPE_RX_POLL:
test->ifobj_rx->use_poll = true;
@@ -1822,6 +1808,7 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
if (!test->fail)
ksft_test_result_pass("PASS: %s %s%s\n", mode_string(test), busy_poll_string(test),
test->name);
+ pkt_stream_restore_default(test);
}
static struct ifobject *ifobject_create(void)
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* [PATCH bpf-next v2 15/15] selftests/xsk: automatically switch XDP programs
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (13 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 14/15] selftests/xsk: automatically restore packet stream Magnus Karlsson
@ 2023-01-04 12:17 ` Magnus Karlsson
2023-01-10 11:56 ` Maciej Fijalkowski
2023-01-10 12:00 ` [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new " Maciej Fijalkowski
15 siblings, 1 reply; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-04 12:17 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Implement automatic switching of XDP programs and execution modes if
needed by a test. This makes it much simpler to write a test as it
only has to say what XDP program it needs if it is not the default
one. This also makes it possible to remove the initial explicit
attachment of the XDP program as well as the explicit mode switch in
the code. These are now handled by the same code that just checks if a
switch is necessary, so no special cases are needed.
The default XDP program for all tests is one that sends all packets to
the AF_XDP socket. If you need another one, please use the new
function test_spec_set_xdp_prog() to specify what XDP programs and
maps to use for this test.
Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
---
tools/testing/selftests/bpf/xsk.c | 14 +++
tools/testing/selftests/bpf/xsk.h | 1 +
tools/testing/selftests/bpf/xskxceiver.c | 137 ++++++++++++-----------
tools/testing/selftests/bpf/xskxceiver.h | 7 +-
4 files changed, 91 insertions(+), 68 deletions(-)
diff --git a/tools/testing/selftests/bpf/xsk.c b/tools/testing/selftests/bpf/xsk.c
index dc6b47280ec4..d9d44a29c7cc 100644
--- a/tools/testing/selftests/bpf/xsk.c
+++ b/tools/testing/selftests/bpf/xsk.c
@@ -267,6 +267,20 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area,
return err;
}
+bool xsk_is_in_drv_mode(u32 ifindex)
+{
+ LIBBPF_OPTS(bpf_xdp_query_opts, opts);
+ int ret;
+
+ ret = bpf_xdp_query(ifindex, XDP_FLAGS_DRV_MODE, &opts);
+ if (ret) {
+ printf("DRV mode query returned error %s\n", strerror(errno));
+ return false;
+ }
+
+ return opts.attach_mode == XDP_ATTACHED_DRV;
+}
+
int xsk_attach_xdp_program(struct bpf_program *prog, int ifindex, u32 xdp_flags)
{
int prog_fd;
diff --git a/tools/testing/selftests/bpf/xsk.h b/tools/testing/selftests/bpf/xsk.h
index 5624d31b8db7..3cb9d69589b8 100644
--- a/tools/testing/selftests/bpf/xsk.h
+++ b/tools/testing/selftests/bpf/xsk.h
@@ -201,6 +201,7 @@ int xsk_attach_xdp_program(struct bpf_program *prog, int ifindex, u32 xdp_flags)
void xsk_detach_xdp_program(int ifindex, u32 xdp_flags);
int xsk_update_xskmap(struct bpf_map *map, struct xsk_socket *xsk);
void xsk_clear_xskmap(struct bpf_map *map);
+bool xsk_is_in_drv_mode(u32 ifindex);
struct xsk_socket_config {
__u32 rx_size;
diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
index 66863504c76a..9af0f8240a59 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -96,6 +96,8 @@
#include <time.h>
#include <unistd.h>
#include <stdatomic.h>
+
+#include "xsk_xdp_progs.skel.h"
#include "xsk.h"
#include "xskxceiver.h"
#include <bpf/bpf.h>
@@ -356,7 +358,6 @@ static bool ifobj_zc_avail(struct ifobject *ifobject)
xsk = calloc(1, sizeof(struct xsk_socket_info));
if (!xsk)
goto out;
- ifobject->xdp_flags = XDP_FLAGS_DRV_MODE;
ifobject->bind_flags = XDP_USE_NEED_WAKEUP | XDP_ZEROCOPY;
ifobject->rx_on = true;
xsk->rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
@@ -493,6 +494,10 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
test->total_steps = 1;
test->nb_sockets = 1;
test->fail = false;
+ test->xdp_prog_rx = ifobj_rx->xdp_progs->progs.xsk_def_prog;
+ test->xskmap_rx = ifobj_rx->xdp_progs->maps.xsk;
+ test->xdp_prog_tx = ifobj_tx->xdp_progs->progs.xsk_def_prog;
+ test->xskmap_tx = ifobj_tx->xdp_progs->maps.xsk;
}
static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
@@ -532,6 +537,16 @@ static void test_spec_set_name(struct test_spec *test, const char *name)
strncpy(test->name, name, MAX_TEST_NAME_SIZE);
}
+static void test_spec_set_xdp_prog(struct test_spec *test, struct bpf_program *xdp_prog_rx,
+ struct bpf_program *xdp_prog_tx, struct bpf_map *xskmap_rx,
+ struct bpf_map *xskmap_tx)
+{
+ test->xdp_prog_rx = xdp_prog_rx;
+ test->xdp_prog_tx = xdp_prog_tx;
+ test->xskmap_rx = xskmap_rx;
+ test->xskmap_tx = xskmap_tx;
+}
+
static void pkt_stream_reset(struct pkt_stream *pkt_stream)
{
if (pkt_stream)
@@ -1356,6 +1371,47 @@ static void handler(int signum)
pthread_exit(NULL);
}
+static bool xdp_prog_changed(struct test_spec *test, struct ifobject *ifobj)
+{
+ return ifobj->xdp_prog != test->xdp_prog_rx || ifobj->mode != test->mode;
+}
+
+static void xsk_reattach_xdp(struct ifobject *ifobj, struct bpf_program *xdp_prog,
+ struct bpf_map *xskmap, enum test_mode mode)
+{
+ int err;
+
+ xsk_detach_xdp_program(ifobj->ifindex, mode_to_xdp_flags(ifobj->mode));
+ err = xsk_attach_xdp_program(xdp_prog, ifobj->ifindex, mode_to_xdp_flags(mode));
+ if (err) {
+ printf("Error attaching XDP program\n");
+ exit_with_error(-err);
+ }
+
+ if (ifobj->mode != mode && (mode == TEST_MODE_DRV || mode == TEST_MODE_ZC))
+ if (!xsk_is_in_drv_mode(ifobj->ifindex)) {
+ ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
+ exit_with_error(EINVAL);
+ }
+
+ ifobj->xdp_prog = xdp_prog;
+ ifobj->xskmap = xskmap;
+ ifobj->mode = mode;
+}
+
+static void xsk_attach_xdp_progs(struct test_spec *test, struct ifobject *ifobj_rx,
+ struct ifobject *ifobj_tx)
+{
+ if (xdp_prog_changed(test, ifobj_rx))
+ xsk_reattach_xdp(ifobj_rx, test->xdp_prog_rx, test->xskmap_rx, test->mode);
+
+ if (!ifobj_tx || ifobj_tx->shared_umem)
+ return;
+
+ if (xdp_prog_changed(test, ifobj_tx))
+ xsk_reattach_xdp(ifobj_tx, test->xdp_prog_tx, test->xskmap_tx, test->mode);
+}
+
static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *ifobj1,
struct ifobject *ifobj2)
{
@@ -1403,7 +1459,11 @@ static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *i
static int testapp_validate_traffic(struct test_spec *test)
{
- return __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
+ struct ifobject *ifobj_rx = test->ifobj_rx;
+ struct ifobject *ifobj_tx = test->ifobj_tx;
+
+ xsk_attach_xdp_progs(test, ifobj_rx, ifobj_tx);
+ return __testapp_validate_traffic(test, ifobj_rx, ifobj_tx);
}
static int testapp_validate_traffic_single_thread(struct test_spec *test, struct ifobject *ifobj)
@@ -1446,7 +1506,7 @@ static void testapp_bidi(struct test_spec *test)
print_verbose("Switching Tx/Rx vectors\n");
swap_directions(&test->ifobj_rx, &test->ifobj_tx);
- testapp_validate_traffic(test);
+ __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
swap_directions(&test->ifobj_rx, &test->ifobj_tx);
}
@@ -1615,29 +1675,15 @@ static void testapp_invalid_desc(struct test_spec *test)
static void testapp_xdp_drop(struct test_spec *test)
{
- struct ifobject *ifobj = test->ifobj_rx;
- int err;
+ struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
+ struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
test_spec_set_name(test, "XDP_DROP_HALF");
- xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
- err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_xdp_drop, ifobj->ifindex,
- ifobj->xdp_flags);
- if (err) {
- printf("Error attaching XDP_DROP program\n");
- test->fail = true;
- return;
- }
+ test_spec_set_xdp_prog(test, skel_rx->progs.xsk_xdp_drop, skel_tx->progs.xsk_xdp_drop,
+ skel_rx->maps.xsk, skel_tx->maps.xsk);
pkt_stream_receive_half(test);
testapp_validate_traffic(test);
-
- xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
- err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
- ifobj->xdp_flags);
- if (err) {
- printf("Error restoring default XDP program\n");
- exit_with_error(-err);
- }
}
static void testapp_poll_txq_tmout(struct test_spec *test)
@@ -1674,7 +1720,7 @@ static void xsk_unload_xdp_programs(struct ifobject *ifobj)
static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
const char *dst_ip, const char *src_ip, const u16 dst_port,
- const u16 src_port, thread_func_t func_ptr, bool load_xdp)
+ const u16 src_port, thread_func_t func_ptr)
{
struct in_addr ip;
int err;
@@ -1693,23 +1739,11 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
ifobj->func_ptr = func_ptr;
- if (!load_xdp)
- return;
-
err = xsk_load_xdp_programs(ifobj);
if (err) {
printf("Error loading XDP program\n");
exit_with_error(err);
}
-
- ifobj->xdp_flags = mode_to_xdp_flags(TEST_MODE_SKB);
- err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
- ifobj->xdp_flags);
- if (err) {
- printf("Error attaching XDP program\n");
- exit_with_error(-err);
- }
- ifobj->xskmap = ifobj->xdp_progs->maps.xsk;
}
static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
@@ -1871,31 +1905,6 @@ static bool is_xdp_supported(int ifindex)
return true;
}
-static void change_to_drv_mode(struct ifobject *ifobj)
-{
- LIBBPF_OPTS(bpf_xdp_query_opts, opts);
- int ret;
-
- xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
- ifobj->xdp_flags = XDP_FLAGS_DRV_MODE;
- ret = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
- ifobj->xdp_flags);
- if (ret) {
- ksft_print_msg("Error attaching XDP program\n");
- exit_with_error(-ret);
- }
- ifobj->xskmap = ifobj->xdp_progs->maps.xsk;
-
- ret = bpf_xdp_query(ifobj->ifindex, XDP_FLAGS_DRV_MODE, &opts);
- if (ret)
- exit_with_error(errno);
-
- if (opts.attach_mode != XDP_ATTACHED_DRV) {
- ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
- exit_with_error(EINVAL);
- }
-}
-
int main(int argc, char **argv)
{
struct pkt_stream *rx_pkt_stream_default;
@@ -1936,9 +1945,9 @@ int main(int argc, char **argv)
}
init_iface(ifobj_rx, MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2,
- worker_testapp_validate_rx, true);
+ worker_testapp_validate_rx);
init_iface(ifobj_tx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
- worker_testapp_validate_tx, !shared_netdev);
+ worker_testapp_validate_tx);
test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
@@ -1951,12 +1960,6 @@ int main(int argc, char **argv)
ksft_set_plan(modes * TEST_TYPE_MAX);
for (i = 0; i < modes; i++) {
- if (i == TEST_MODE_DRV) {
- change_to_drv_mode(ifobj_rx);
- if (!shared_netdev)
- change_to_drv_mode(ifobj_tx);
- }
-
for (j = 0; j < TEST_TYPE_MAX; j++) {
test_spec_init(&test, ifobj_tx, ifobj_rx, i);
run_pkt_test(&test, i, j);
diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
index a4daa5134fc0..3e8ec7d8ec32 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -143,10 +143,11 @@ struct ifobject {
struct pkt_stream *pkt_stream;
struct xsk_xdp_progs *xdp_progs;
struct bpf_map *xskmap;
+ struct bpf_program *xdp_prog;
+ enum test_mode mode;
int ifindex;
u32 dst_ip;
u32 src_ip;
- u32 xdp_flags;
u32 bind_flags;
u16 src_port;
u16 dst_port;
@@ -166,6 +167,10 @@ struct test_spec {
struct ifobject *ifobj_rx;
struct pkt_stream *tx_pkt_stream_default;
struct pkt_stream *rx_pkt_stream_default;
+ struct bpf_program *xdp_prog_rx;
+ struct bpf_program *xdp_prog_tx;
+ struct bpf_map *xskmap_rx;
+ struct bpf_map *xskmap_tx;
u16 total_steps;
u16 current_step;
u16 nb_sockets;
--
2.34.1
^ permalink raw reply related [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program
2023-01-04 12:17 ` [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program Magnus Karlsson
@ 2023-01-04 18:14 ` David Vernet
2023-01-04 18:19 ` Stanislav Fomichev
2023-01-05 8:38 ` Magnus Karlsson
2023-01-04 23:15 ` Andrii Nakryiko
1 sibling, 2 replies; 29+ messages in thread
From: David Vernet @ 2023-01-04 18:14 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar, jonathan.lemon
On Wed, Jan 04, 2023 at 01:17:40PM +0100, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Get rid of the built-in XDP program that was part of the old libbpf
> code in xsk.c and replace it with an eBPF program build using the
> framework by all the other bpf selftests. This will form the base for
> adding more programs in later commits.
>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
> tools/testing/selftests/bpf/Makefile | 2 +-
> .../selftests/bpf/progs/xsk_xdp_progs.c | 19 ++++
> tools/testing/selftests/bpf/xsk.c | 88 ++++---------------
> tools/testing/selftests/bpf/xsk.h | 6 +-
> tools/testing/selftests/bpf/xskxceiver.c | 72 ++++++++-------
> tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> 6 files changed, 88 insertions(+), 106 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 205e8c3c346a..a0193a8f9da6 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -240,7 +240,7 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
> $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
> $(OUTPUT)/xsk.o: $(BPFOBJ)
> -$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o
> +$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h
Hi Magnus,
This seems to break the selftests build for clang:
$ pwd
<redacted>/bpf-next/tools/testing/selftests/bpf
$ make LLVM=1 CC=clang
MKDIR libbpf
HOSTCC /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep.o
HOSTLD /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep-in.o
LINK /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep
...
GEN-SKEL [test_progs-no_alu32] test_static_linked.skel.h
LINK-BPF [test_progs-no_alu32] test_usdt.bpf.o
GEN-SKEL [test_progs-no_alu32] linked_vars.skel.h
GEN-SKEL [test_progs-no_alu32] linked_funcs.skel.h
EXT-COPY [test_progs-no_alu32] urandom_read bpf_testmod.ko liburandom_read.so xdp_synproxy sign-file ima_setup.sh verify_sig_setup.sh btf_dump_test_case_bitfields.c btf_dump_test_case_multidim.c btf_dump_test_case_namespacing.c btf_dump_test_case_ordering.c btf_dump_test_case_packing.c btf_dump_test_case_padding.c btf_dump_test_case_syntax.c
GEN-SKEL [test_progs-no_alu32] linked_maps.skel.h
GEN-SKEL [test_progs-no_alu32] test_subskeleton.skel.h
BINARY xskxceiver
BINARY bench
GEN-SKEL [test_progs-no_alu32] test_subskeleton_lib.skel.h
GEN-SKEL [test_progs-no_alu32] test_usdt.skel.h
clang-15: error: cannot specify -o when generating multiple output files
make: *** [Makefile:171: /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xskxceiver] Error 1
make: *** Waiting for unfinished jobs....
make[1]: Nothing to be done for 'docs'.
$
It's also broken on CI: https://github.com/kernel-patches/bpf/actions/runs/3837984934/jobs/6533917001
Could you please look into this?
Thanks,
David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program
2023-01-04 18:14 ` David Vernet
@ 2023-01-04 18:19 ` Stanislav Fomichev
2023-01-04 19:13 ` David Vernet
2023-01-05 8:38 ` Magnus Karlsson
1 sibling, 1 reply; 29+ messages in thread
From: Stanislav Fomichev @ 2023-01-04 18:19 UTC (permalink / raw)
To: David Vernet
Cc: Magnus Karlsson, magnus.karlsson, bjorn, ast, daniel, netdev,
maciej.fijalkowski, bpf, yhs, andrii, martin.lau, song,
john.fastabend, kpsingh, haoluo, jolsa, tirthendu.sarkar,
jonathan.lemon
On Wed, Jan 4, 2023 at 10:14 AM David Vernet <void@manifault.com> wrote:
>
> On Wed, Jan 04, 2023 at 01:17:40PM +0100, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Get rid of the built-in XDP program that was part of the old libbpf
> > code in xsk.c and replace it with an eBPF program build using the
> > framework by all the other bpf selftests. This will form the base for
> > adding more programs in later commits.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 2 +-
> > .../selftests/bpf/progs/xsk_xdp_progs.c | 19 ++++
> > tools/testing/selftests/bpf/xsk.c | 88 ++++---------------
> > tools/testing/selftests/bpf/xsk.h | 6 +-
> > tools/testing/selftests/bpf/xskxceiver.c | 72 ++++++++-------
> > tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> > 6 files changed, 88 insertions(+), 106 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 205e8c3c346a..a0193a8f9da6 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -240,7 +240,7 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
> > $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> > $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
> > $(OUTPUT)/xsk.o: $(BPFOBJ)
> > -$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o
> > +$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h
>
> Hi Magnus,
>
> This seems to break the selftests build for clang:
>
> $ pwd
> <redacted>/bpf-next/tools/testing/selftests/bpf
>
> $ make LLVM=1 CC=clang
> MKDIR libbpf
> HOSTCC /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep.o
> HOSTLD /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep-in.o
> LINK /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep
>
> ...
>
> GEN-SKEL [test_progs-no_alu32] test_static_linked.skel.h
> LINK-BPF [test_progs-no_alu32] test_usdt.bpf.o
> GEN-SKEL [test_progs-no_alu32] linked_vars.skel.h
> GEN-SKEL [test_progs-no_alu32] linked_funcs.skel.h
> EXT-COPY [test_progs-no_alu32] urandom_read bpf_testmod.ko liburandom_read.so xdp_synproxy sign-file ima_setup.sh verify_sig_setup.sh btf_dump_test_case_bitfields.c btf_dump_test_case_multidim.c btf_dump_test_case_namespacing.c btf_dump_test_case_ordering.c btf_dump_test_case_packing.c btf_dump_test_case_padding.c btf_dump_test_case_syntax.c
> GEN-SKEL [test_progs-no_alu32] linked_maps.skel.h
> GEN-SKEL [test_progs-no_alu32] test_subskeleton.skel.h
> BINARY xskxceiver
> BINARY bench
> GEN-SKEL [test_progs-no_alu32] test_subskeleton_lib.skel.h
> GEN-SKEL [test_progs-no_alu32] test_usdt.skel.h
> clang-15: error: cannot specify -o when generating multiple output files
> make: *** [Makefile:171: /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xskxceiver] Error 1
> make: *** Waiting for unfinished jobs....
> make[1]: Nothing to be done for 'docs'.
> $
>
> It's also broken on CI: https://github.com/kernel-patches/bpf/actions/runs/3837984934/jobs/6533917001
>
> Could you please look into this?
Ugh, that's the same issue I'm getting for my xdp_hw_metadata binary.
And I'm still at loss on how to reproduce it locally. (I've tried 'apt
install clang-16 in ubuntu latest docker container' and it's still
fine).
Any pointers on how to debug those github actions locally?
> Thanks,
> David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program
2023-01-04 18:19 ` Stanislav Fomichev
@ 2023-01-04 19:13 ` David Vernet
2023-01-04 19:46 ` Stanislav Fomichev
0 siblings, 1 reply; 29+ messages in thread
From: David Vernet @ 2023-01-04 19:13 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Magnus Karlsson, magnus.karlsson, bjorn, ast, daniel, netdev,
maciej.fijalkowski, bpf, yhs, andrii, martin.lau, song,
john.fastabend, kpsingh, haoluo, jolsa, tirthendu.sarkar,
jonathan.lemon
On Wed, Jan 04, 2023 at 10:19:37AM -0800, Stanislav Fomichev wrote:
> On Wed, Jan 4, 2023 at 10:14 AM David Vernet <void@manifault.com> wrote:
> >
> > On Wed, Jan 04, 2023 at 01:17:40PM +0100, Magnus Karlsson wrote:
> > > From: Magnus Karlsson <magnus.karlsson@intel.com>
> > >
> > > Get rid of the built-in XDP program that was part of the old libbpf
> > > code in xsk.c and replace it with an eBPF program build using the
> > > framework by all the other bpf selftests. This will form the base for
> > > adding more programs in later commits.
> > >
> > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > > ---
> > > tools/testing/selftests/bpf/Makefile | 2 +-
> > > .../selftests/bpf/progs/xsk_xdp_progs.c | 19 ++++
> > > tools/testing/selftests/bpf/xsk.c | 88 ++++---------------
> > > tools/testing/selftests/bpf/xsk.h | 6 +-
> > > tools/testing/selftests/bpf/xskxceiver.c | 72 ++++++++-------
> > > tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> > > 6 files changed, 88 insertions(+), 106 deletions(-)
> > > create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index 205e8c3c346a..a0193a8f9da6 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -240,7 +240,7 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
> > > $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> > > $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
> > > $(OUTPUT)/xsk.o: $(BPFOBJ)
> > > -$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o
> > > +$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h
> >
> > Hi Magnus,
> >
> > This seems to break the selftests build for clang:
> >
> > $ pwd
> > <redacted>/bpf-next/tools/testing/selftests/bpf
> >
> > $ make LLVM=1 CC=clang
> > MKDIR libbpf
> > HOSTCC /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep.o
> > HOSTLD /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep-in.o
> > LINK /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep
> >
> > ...
> >
> > GEN-SKEL [test_progs-no_alu32] test_static_linked.skel.h
> > LINK-BPF [test_progs-no_alu32] test_usdt.bpf.o
> > GEN-SKEL [test_progs-no_alu32] linked_vars.skel.h
> > GEN-SKEL [test_progs-no_alu32] linked_funcs.skel.h
> > EXT-COPY [test_progs-no_alu32] urandom_read bpf_testmod.ko liburandom_read.so xdp_synproxy sign-file ima_setup.sh verify_sig_setup.sh btf_dump_test_case_bitfields.c btf_dump_test_case_multidim.c btf_dump_test_case_namespacing.c btf_dump_test_case_ordering.c btf_dump_test_case_packing.c btf_dump_test_case_padding.c btf_dump_test_case_syntax.c
> > GEN-SKEL [test_progs-no_alu32] linked_maps.skel.h
> > GEN-SKEL [test_progs-no_alu32] test_subskeleton.skel.h
> > BINARY xskxceiver
> > BINARY bench
> > GEN-SKEL [test_progs-no_alu32] test_subskeleton_lib.skel.h
> > GEN-SKEL [test_progs-no_alu32] test_usdt.skel.h
> > clang-15: error: cannot specify -o when generating multiple output files
> > make: *** [Makefile:171: /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xskxceiver] Error 1
> > make: *** Waiting for unfinished jobs....
> > make[1]: Nothing to be done for 'docs'.
> > $
> >
> > It's also broken on CI: https://github.com/kernel-patches/bpf/actions/runs/3837984934/jobs/6533917001
> >
> > Could you please look into this?
>
> Ugh, that's the same issue I'm getting for my xdp_hw_metadata binary.
> And I'm still at loss on how to reproduce it locally. (I've tried 'apt
> install clang-16 in ubuntu latest docker container' and it's still
> fine).
I was able to reproduce this issue locally:
[void@maniforge bpf]$ make -j LLVM=1 CC=clang
GEN-SKEL [test_progs] pyperf600.skel.h
GEN-SKEL [test_progs] test_verif_scale2.skel.h
LINK-BPF [test_progs] test_static_linked.bpf.o
LINK-BPF [test_progs] linked_funcs.bpf.o
...
LINK-BPF [test_progs-no_alu32] test_usdt.bpf.o
EXT-COPY [test_progs-no_alu32] urandom_read bpf_testmod.ko liburandom_read.so xdp_synproxy sign-file ima_setup.sh verify_sig_setup.sh btf_dump_test_case_bitfields.c btf_dump_test_case_multidim.c btf_dump_test_case_namespacing.c btf_dump_test_case_ordering.c btf_dump_test_case_packing.c btf_dump_test_case_padding.c btf_dump_test_case_syntax.c
GEN-SKEL [test_progs-no_alu32] linked_funcs.skel.h
BINARY bench
GEN-SKEL [test_progs-no_alu32] test_subskeleton.skel.h
BINARY xdp_hw_metadata
GEN-SKEL [test_progs-no_alu32] test_subskeleton_lib.skel.h
GEN-SKEL [test_progs-no_alu32] test_usdt.skel.h
clang-15: error: cannot specify -o when generating multiple output files
make: *** [Makefile:171: /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata] Error 1
make: *** Waiting for unfinished jobs....
make[1]: Nothing to be done for 'docs'.
Here's the actual clang command being executed:
[void@maniforge bpf]$ make LLVM=1 --dry-run xdp_hw_metadata
printf ' %-8s%s %s%s\n' "BINARY" "" "xdp_hw_metadata" "";
clang --target=x86_64-linux-gnu -fintegrated-as -g -O0 -rdynamic -Wall -Werror -DHAVE_GENHDR -I/home/void/upstream/bpf-next/tools/testing/selftests/bpf -I/home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/include -I/home/void/upstream/bpf-next/include/generated -I/home/void/upstream/bpf-next/tools/lib -I/home/void/upstream/bpf-next/tools/include -I/home/void/upstream/bpf-next/tools/include/uapi -I/home/void/upstream/bpf-next/tools/testing/selftests/bpf -Wno-unused-command-line-argument -static xdp_hw_metadata.c /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xsk.o /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata.skel.h /home/void/upstream/bpf-next/tools/testing/selftests/bpf/network_helpers.o -lelf -lz -lrt -lpthread -o /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata
and the output using --debug=j
[void@maniforge bpf]$ make LLVM=1 --debug=j xdp_hw_metadata
Putting child 0x55cc78cd6670 (/home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata) PID 693804 on the chain.
Live child 0x55cc78cd6670 (/home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata) PID 693804
BINARY xdp_hw_metadata
Reaping winning child 0x55cc78cd6670 PID 693804
Live child 0x55cc78cd6670 (/home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata) PID 693805
clang-15: error: cannot specify -o when generating multiple output files
Reaping losing child 0x55cc78cd6670 PID 693805
make: *** [Makefile:171: /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata] Error 1
Removing child 0x55cc78cd6670 PID 693805 from chain.
make is taking the xdp_hw_metadata.skel.h file and providing it as an
input to clang. So I believe what's going on here is that the clang
command above is actually creating two output files:
1. xdp_hw_metadata
2. The precompiled header generated from xdp_hw_metadata.skel.h
and the error is clang reasonably saying: "I don't know which output
file you're referring to with -o". I'm surprised that gcc doesn't
complain about this, but I assume that it's doing the far more
non-intuitive thing of first outputting the precompiled header as
xdp_hw_metadata, and then immediately overwriting it with the actual
xdp_hw_metadata binary.
> Any pointers on how to debug those github actions locally?
>
> > Thanks,
> > David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program
2023-01-04 19:13 ` David Vernet
@ 2023-01-04 19:46 ` Stanislav Fomichev
0 siblings, 0 replies; 29+ messages in thread
From: Stanislav Fomichev @ 2023-01-04 19:46 UTC (permalink / raw)
To: David Vernet
Cc: Magnus Karlsson, magnus.karlsson, bjorn, ast, daniel, netdev,
maciej.fijalkowski, bpf, yhs, andrii, martin.lau, song,
john.fastabend, kpsingh, haoluo, jolsa, tirthendu.sarkar,
jonathan.lemon
On Wed, Jan 4, 2023 at 11:14 AM David Vernet <void@manifault.com> wrote:
>
> On Wed, Jan 04, 2023 at 10:19:37AM -0800, Stanislav Fomichev wrote:
> > On Wed, Jan 4, 2023 at 10:14 AM David Vernet <void@manifault.com> wrote:
> > >
> > > On Wed, Jan 04, 2023 at 01:17:40PM +0100, Magnus Karlsson wrote:
> > > > From: Magnus Karlsson <magnus.karlsson@intel.com>
> > > >
> > > > Get rid of the built-in XDP program that was part of the old libbpf
> > > > code in xsk.c and replace it with an eBPF program build using the
> > > > framework by all the other bpf selftests. This will form the base for
> > > > adding more programs in later commits.
> > > >
> > > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > > > ---
> > > > tools/testing/selftests/bpf/Makefile | 2 +-
> > > > .../selftests/bpf/progs/xsk_xdp_progs.c | 19 ++++
> > > > tools/testing/selftests/bpf/xsk.c | 88 ++++---------------
> > > > tools/testing/selftests/bpf/xsk.h | 6 +-
> > > > tools/testing/selftests/bpf/xskxceiver.c | 72 ++++++++-------
> > > > tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> > > > 6 files changed, 88 insertions(+), 106 deletions(-)
> > > > create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> > > >
> > > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > > index 205e8c3c346a..a0193a8f9da6 100644
> > > > --- a/tools/testing/selftests/bpf/Makefile
> > > > +++ b/tools/testing/selftests/bpf/Makefile
> > > > @@ -240,7 +240,7 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
> > > > $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> > > > $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
> > > > $(OUTPUT)/xsk.o: $(BPFOBJ)
> > > > -$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o
> > > > +$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h
> > >
> > > Hi Magnus,
> > >
> > > This seems to break the selftests build for clang:
> > >
> > > $ pwd
> > > <redacted>/bpf-next/tools/testing/selftests/bpf
> > >
> > > $ make LLVM=1 CC=clang
> > > MKDIR libbpf
> > > HOSTCC /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep.o
> > > HOSTLD /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep-in.o
> > > LINK /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep
> > >
> > > ...
> > >
> > > GEN-SKEL [test_progs-no_alu32] test_static_linked.skel.h
> > > LINK-BPF [test_progs-no_alu32] test_usdt.bpf.o
> > > GEN-SKEL [test_progs-no_alu32] linked_vars.skel.h
> > > GEN-SKEL [test_progs-no_alu32] linked_funcs.skel.h
> > > EXT-COPY [test_progs-no_alu32] urandom_read bpf_testmod.ko liburandom_read.so xdp_synproxy sign-file ima_setup.sh verify_sig_setup.sh btf_dump_test_case_bitfields.c btf_dump_test_case_multidim.c btf_dump_test_case_namespacing.c btf_dump_test_case_ordering.c btf_dump_test_case_packing.c btf_dump_test_case_padding.c btf_dump_test_case_syntax.c
> > > GEN-SKEL [test_progs-no_alu32] linked_maps.skel.h
> > > GEN-SKEL [test_progs-no_alu32] test_subskeleton.skel.h
> > > BINARY xskxceiver
> > > BINARY bench
> > > GEN-SKEL [test_progs-no_alu32] test_subskeleton_lib.skel.h
> > > GEN-SKEL [test_progs-no_alu32] test_usdt.skel.h
> > > clang-15: error: cannot specify -o when generating multiple output files
> > > make: *** [Makefile:171: /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xskxceiver] Error 1
> > > make: *** Waiting for unfinished jobs....
> > > make[1]: Nothing to be done for 'docs'.
> > > $
> > >
> > > It's also broken on CI: https://github.com/kernel-patches/bpf/actions/runs/3837984934/jobs/6533917001
> > >
> > > Could you please look into this?
> >
> > Ugh, that's the same issue I'm getting for my xdp_hw_metadata binary.
> > And I'm still at loss on how to reproduce it locally. (I've tried 'apt
> > install clang-16 in ubuntu latest docker container' and it's still
> > fine).
>
> I was able to reproduce this issue locally:
Damn, I was missing LLVM=1, that does make it reproduce for me, thanks!
> [void@maniforge bpf]$ make -j LLVM=1 CC=clang
> GEN-SKEL [test_progs] pyperf600.skel.h
> GEN-SKEL [test_progs] test_verif_scale2.skel.h
> LINK-BPF [test_progs] test_static_linked.bpf.o
> LINK-BPF [test_progs] linked_funcs.bpf.o
>
> ...
>
> LINK-BPF [test_progs-no_alu32] test_usdt.bpf.o
> EXT-COPY [test_progs-no_alu32] urandom_read bpf_testmod.ko liburandom_read.so xdp_synproxy sign-file ima_setup.sh verify_sig_setup.sh btf_dump_test_case_bitfields.c btf_dump_test_case_multidim.c btf_dump_test_case_namespacing.c btf_dump_test_case_ordering.c btf_dump_test_case_packing.c btf_dump_test_case_padding.c btf_dump_test_case_syntax.c
> GEN-SKEL [test_progs-no_alu32] linked_funcs.skel.h
> BINARY bench
> GEN-SKEL [test_progs-no_alu32] test_subskeleton.skel.h
> BINARY xdp_hw_metadata
> GEN-SKEL [test_progs-no_alu32] test_subskeleton_lib.skel.h
> GEN-SKEL [test_progs-no_alu32] test_usdt.skel.h
> clang-15: error: cannot specify -o when generating multiple output files
> make: *** [Makefile:171: /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata] Error 1
> make: *** Waiting for unfinished jobs....
> make[1]: Nothing to be done for 'docs'.
>
> Here's the actual clang command being executed:
>
> [void@maniforge bpf]$ make LLVM=1 --dry-run xdp_hw_metadata
> printf ' %-8s%s %s%s\n' "BINARY" "" "xdp_hw_metadata" "";
> clang --target=x86_64-linux-gnu -fintegrated-as -g -O0 -rdynamic -Wall -Werror -DHAVE_GENHDR -I/home/void/upstream/bpf-next/tools/testing/selftests/bpf -I/home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/include -I/home/void/upstream/bpf-next/include/generated -I/home/void/upstream/bpf-next/tools/lib -I/home/void/upstream/bpf-next/tools/include -I/home/void/upstream/bpf-next/tools/include/uapi -I/home/void/upstream/bpf-next/tools/testing/selftests/bpf -Wno-unused-command-line-argument -static xdp_hw_metadata.c /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/libbpf.a /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xsk.o /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata.skel.h /home/void/upstream/bpf-next/tools/testing/selftests/bpf/network_helpers.o -lelf -lz -lrt -lpthread -o /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata
>
> and the output using --debug=j
>
> [void@maniforge bpf]$ make LLVM=1 --debug=j xdp_hw_metadata
> Putting child 0x55cc78cd6670 (/home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata) PID 693804 on the chain.
> Live child 0x55cc78cd6670 (/home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata) PID 693804
> BINARY xdp_hw_metadata
> Reaping winning child 0x55cc78cd6670 PID 693804
> Live child 0x55cc78cd6670 (/home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata) PID 693805
> clang-15: error: cannot specify -o when generating multiple output files
> Reaping losing child 0x55cc78cd6670 PID 693805
> make: *** [Makefile:171: /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xdp_hw_metadata] Error 1
> Removing child 0x55cc78cd6670 PID 693805 from chain.
>
> make is taking the xdp_hw_metadata.skel.h file and providing it as an
> input to clang. So I believe what's going on here is that the clang
> command above is actually creating two output files:
>
> 1. xdp_hw_metadata
> 2. The precompiled header generated from xdp_hw_metadata.skel.h
>
> and the error is clang reasonably saying: "I don't know which output
> file you're referring to with -o". I'm surprised that gcc doesn't
> complain about this, but I assume that it's doing the far more
> non-intuitive thing of first outputting the precompiled header as
> xdp_hw_metadata, and then immediately overwriting it with the actual
> xdp_hw_metadata binary.
Hm, I'm a bit surprised that clang can't handle .h as an extra input.
GCC seems to be doing the right thing; the headers compile to nothing
-> use that nothing as an extra input and don't complain..
Seems like the way to go is to explicitly have a '$(CC) xxx' rule that
filters it out. Will try.
> > Any pointers on how to debug those github actions locally?
> >
> > > Thanks,
> > > David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program
2023-01-04 12:17 ` [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program Magnus Karlsson
2023-01-04 18:14 ` David Vernet
@ 2023-01-04 23:15 ` Andrii Nakryiko
2023-01-04 23:44 ` Stanislav Fomichev
1 sibling, 1 reply; 29+ messages in thread
From: Andrii Nakryiko @ 2023-01-04 23:15 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar, jonathan.lemon
On Wed, Jan 4, 2023 at 4:19 AM Magnus Karlsson
<magnus.karlsson@gmail.com> wrote:
>
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Get rid of the built-in XDP program that was part of the old libbpf
> code in xsk.c and replace it with an eBPF program build using the
> framework by all the other bpf selftests. This will form the base for
> adding more programs in later commits.
>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
> tools/testing/selftests/bpf/Makefile | 2 +-
> .../selftests/bpf/progs/xsk_xdp_progs.c | 19 ++++
> tools/testing/selftests/bpf/xsk.c | 88 ++++---------------
> tools/testing/selftests/bpf/xsk.h | 6 +-
> tools/testing/selftests/bpf/xskxceiver.c | 72 ++++++++-------
> tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> 6 files changed, 88 insertions(+), 106 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 205e8c3c346a..a0193a8f9da6 100644
> --- a/tools/testing/selftests/bpf/Makefile
> +++ b/tools/testing/selftests/bpf/Makefile
> @@ -240,7 +240,7 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
> $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
> $(OUTPUT)/xsk.o: $(BPFOBJ)
shouldn't $(OUTPUT)/xsk_xdp_progs.skel.h be added as a dependency
here, at .o file?
> -$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o
> +$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h
and not here. Is that why we have this clang compilation failure?
>
> BPFTOOL ?= $(DEFAULT_BPFTOOL)
> $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
[...]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program
2023-01-04 23:15 ` Andrii Nakryiko
@ 2023-01-04 23:44 ` Stanislav Fomichev
2023-01-11 19:20 ` Andrii Nakryiko
0 siblings, 1 reply; 29+ messages in thread
From: Stanislav Fomichev @ 2023-01-04 23:44 UTC (permalink / raw)
To: Andrii Nakryiko
Cc: Magnus Karlsson, magnus.karlsson, bjorn, ast, daniel, netdev,
maciej.fijalkowski, bpf, yhs, andrii, martin.lau, song,
john.fastabend, kpsingh, haoluo, jolsa, tirthendu.sarkar,
jonathan.lemon
On Wed, Jan 4, 2023 at 3:15 PM Andrii Nakryiko
<andrii.nakryiko@gmail.com> wrote:
>
> On Wed, Jan 4, 2023 at 4:19 AM Magnus Karlsson
> <magnus.karlsson@gmail.com> wrote:
> >
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Get rid of the built-in XDP program that was part of the old libbpf
> > code in xsk.c and replace it with an eBPF program build using the
> > framework by all the other bpf selftests. This will form the base for
> > adding more programs in later commits.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 2 +-
> > .../selftests/bpf/progs/xsk_xdp_progs.c | 19 ++++
> > tools/testing/selftests/bpf/xsk.c | 88 ++++---------------
> > tools/testing/selftests/bpf/xsk.h | 6 +-
> > tools/testing/selftests/bpf/xskxceiver.c | 72 ++++++++-------
> > tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> > 6 files changed, 88 insertions(+), 106 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 205e8c3c346a..a0193a8f9da6 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -240,7 +240,7 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
> > $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> > $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
> > $(OUTPUT)/xsk.o: $(BPFOBJ)
>
> shouldn't $(OUTPUT)/xsk_xdp_progs.skel.h be added as a dependency
> here, at .o file?
Not sure we can:
xsk.o is a 'generic' library and xsk_xdp_progs.skel.h is xskxceiver-specific.
I was trying to see how it works for the other cases where we depend
on the headers and saw the following:
$(OUTPUT)/test_verifier: test_verifier.c verifier/tests.h $(BPFOBJ) | $(OUTPUT)
$(call msg,BINARY,,$@)
$(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) $(LDLIBS) -o $@
So at least for test_verifier, we explicitly filter out anything
non-.[aoc]. Presumably because of the same issue?
Should we do the same for xskxceiver? I've sent similar changes for my
xdp_hw_metadata binary about an hour ago..
> > -$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o
> > +$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h
>
> and not here. Is that why we have this clang compilation failure?
>
> >
> > BPFTOOL ?= $(DEFAULT_BPFTOOL)
> > $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
>
> [...]
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program
2023-01-04 18:14 ` David Vernet
2023-01-04 18:19 ` Stanislav Fomichev
@ 2023-01-05 8:38 ` Magnus Karlsson
1 sibling, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-05 8:38 UTC (permalink / raw)
To: David Vernet
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, tirthendu.sarkar, jonathan.lemon
On Wed, Jan 4, 2023 at 7:14 PM David Vernet <void@manifault.com> wrote:
>
> On Wed, Jan 04, 2023 at 01:17:40PM +0100, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Get rid of the built-in XDP program that was part of the old libbpf
> > code in xsk.c and replace it with an eBPF program build using the
> > framework by all the other bpf selftests. This will form the base for
> > adding more programs in later commits.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> > tools/testing/selftests/bpf/Makefile | 2 +-
> > .../selftests/bpf/progs/xsk_xdp_progs.c | 19 ++++
> > tools/testing/selftests/bpf/xsk.c | 88 ++++---------------
> > tools/testing/selftests/bpf/xsk.h | 6 +-
> > tools/testing/selftests/bpf/xskxceiver.c | 72 ++++++++-------
> > tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> > 6 files changed, 88 insertions(+), 106 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 205e8c3c346a..a0193a8f9da6 100644
> > --- a/tools/testing/selftests/bpf/Makefile
> > +++ b/tools/testing/selftests/bpf/Makefile
> > @@ -240,7 +240,7 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
> > $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> > $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
> > $(OUTPUT)/xsk.o: $(BPFOBJ)
> > -$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o
> > +$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h
>
> Hi Magnus,
>
> This seems to break the selftests build for clang:
>
> $ pwd
> <redacted>/bpf-next/tools/testing/selftests/bpf
>
> $ make LLVM=1 CC=clang
> MKDIR libbpf
> HOSTCC /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep.o
> HOSTLD /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep-in.o
> LINK /home/void/upstream/bpf-next/tools/testing/selftests/bpf/tools/build/libbpf/fixdep
>
> ...
>
> GEN-SKEL [test_progs-no_alu32] test_static_linked.skel.h
> LINK-BPF [test_progs-no_alu32] test_usdt.bpf.o
> GEN-SKEL [test_progs-no_alu32] linked_vars.skel.h
> GEN-SKEL [test_progs-no_alu32] linked_funcs.skel.h
> EXT-COPY [test_progs-no_alu32] urandom_read bpf_testmod.ko liburandom_read.so xdp_synproxy sign-file ima_setup.sh verify_sig_setup.sh btf_dump_test_case_bitfields.c btf_dump_test_case_multidim.c btf_dump_test_case_namespacing.c btf_dump_test_case_ordering.c btf_dump_test_case_packing.c btf_dump_test_case_padding.c btf_dump_test_case_syntax.c
> GEN-SKEL [test_progs-no_alu32] linked_maps.skel.h
> GEN-SKEL [test_progs-no_alu32] test_subskeleton.skel.h
> BINARY xskxceiver
> BINARY bench
> GEN-SKEL [test_progs-no_alu32] test_subskeleton_lib.skel.h
> GEN-SKEL [test_progs-no_alu32] test_usdt.skel.h
> clang-15: error: cannot specify -o when generating multiple output files
> make: *** [Makefile:171: /home/void/upstream/bpf-next/tools/testing/selftests/bpf/xskxceiver] Error 1
> make: *** Waiting for unfinished jobs....
> make[1]: Nothing to be done for 'docs'.
> $
>
> It's also broken on CI: https://github.com/kernel-patches/bpf/actions/runs/3837984934/jobs/6533917001
>
> Could you please look into this?
Thanks for spotting this David. Will fix it in the v3.
> Thanks,
> David
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 03/15] selftests/xsk: submit correct number of frames in populate_fill_ring
2023-01-04 12:17 ` [PATCH bpf-next v2 03/15] selftests/xsk: submit correct number of frames in populate_fill_ring Magnus Karlsson
@ 2023-01-09 23:01 ` Maciej Fijalkowski
0 siblings, 0 replies; 29+ messages in thread
From: Maciej Fijalkowski @ 2023-01-09 23:01 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, bpf, yhs, andrii,
martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
tirthendu.sarkar, jonathan.lemon
On Wed, Jan 04, 2023 at 01:17:32PM +0100, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Submit the correct number of frames in the function
> xsk_populate_fill_ring(). For the tests that set the flag
> use_addr_for_fill, uninitialized buffers were sent to the fill ring
> following the correct ones. This has no impact on the tests, since
> they only use the ones that were initialized. But for correctnes, this
tiny nit: missing second 's' on correctness
> should be fixed.
>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
> tools/testing/selftests/bpf/xskxceiver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 2ff43b22180f..a239e975ab66 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -1272,7 +1272,7 @@ static void xsk_populate_fill_ring(struct xsk_umem_info *umem, struct pkt_stream
>
> *xsk_ring_prod__fill_addr(&umem->fq, idx++) = addr;
> }
> - xsk_ring_prod__submit(&umem->fq, buffers_to_fill);
> + xsk_ring_prod__submit(&umem->fq, i);
> }
>
> static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 15/15] selftests/xsk: automatically switch XDP programs
2023-01-04 12:17 ` [PATCH bpf-next v2 15/15] selftests/xsk: automatically switch XDP programs Magnus Karlsson
@ 2023-01-10 11:56 ` Maciej Fijalkowski
2023-01-10 12:08 ` Magnus Karlsson
0 siblings, 1 reply; 29+ messages in thread
From: Maciej Fijalkowski @ 2023-01-10 11:56 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, bpf, yhs, andrii,
martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
tirthendu.sarkar, jonathan.lemon
On Wed, Jan 04, 2023 at 01:17:44PM +0100, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Implement automatic switching of XDP programs and execution modes if
> needed by a test. This makes it much simpler to write a test as it
> only has to say what XDP program it needs if it is not the default
> one. This also makes it possible to remove the initial explicit
> attachment of the XDP program as well as the explicit mode switch in
> the code. These are now handled by the same code that just checks if a
> switch is necessary, so no special cases are needed.
>
> The default XDP program for all tests is one that sends all packets to
> the AF_XDP socket. If you need another one, please use the new
> function test_spec_set_xdp_prog() to specify what XDP programs and
> maps to use for this test.
>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> ---
> tools/testing/selftests/bpf/xsk.c | 14 +++
> tools/testing/selftests/bpf/xsk.h | 1 +
> tools/testing/selftests/bpf/xskxceiver.c | 137 ++++++++++++-----------
> tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> 4 files changed, 91 insertions(+), 68 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xsk.c b/tools/testing/selftests/bpf/xsk.c
> index dc6b47280ec4..d9d44a29c7cc 100644
> --- a/tools/testing/selftests/bpf/xsk.c
> +++ b/tools/testing/selftests/bpf/xsk.c
> @@ -267,6 +267,20 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area,
> return err;
> }
>
> +bool xsk_is_in_drv_mode(u32 ifindex)
> +{
> + LIBBPF_OPTS(bpf_xdp_query_opts, opts);
> + int ret;
> +
> + ret = bpf_xdp_query(ifindex, XDP_FLAGS_DRV_MODE, &opts);
> + if (ret) {
> + printf("DRV mode query returned error %s\n", strerror(errno));
> + return false;
> + }
> +
> + return opts.attach_mode == XDP_ATTACHED_DRV;
> +}
How about making this function more generic since you're adding this to a
"lib" file? you could take on input the mode that you are expecting the
prog being loaded on. Not sure if there will be any future use case for
this, maybe if we would have a support for running a standalone test, not
the whole test suite. I am sort of bothered that things are hard coded in
a way that we expect DRV tests to follow the SKB ones.
That's only a rant though :)
> +
> int xsk_attach_xdp_program(struct bpf_program *prog, int ifindex, u32 xdp_flags)
> {
> int prog_fd;
> diff --git a/tools/testing/selftests/bpf/xsk.h b/tools/testing/selftests/bpf/xsk.h
> index 5624d31b8db7..3cb9d69589b8 100644
> --- a/tools/testing/selftests/bpf/xsk.h
> +++ b/tools/testing/selftests/bpf/xsk.h
> @@ -201,6 +201,7 @@ int xsk_attach_xdp_program(struct bpf_program *prog, int ifindex, u32 xdp_flags)
> void xsk_detach_xdp_program(int ifindex, u32 xdp_flags);
> int xsk_update_xskmap(struct bpf_map *map, struct xsk_socket *xsk);
> void xsk_clear_xskmap(struct bpf_map *map);
> +bool xsk_is_in_drv_mode(u32 ifindex);
>
> struct xsk_socket_config {
> __u32 rx_size;
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 66863504c76a..9af0f8240a59 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -96,6 +96,8 @@
> #include <time.h>
> #include <unistd.h>
> #include <stdatomic.h>
> +
> +#include "xsk_xdp_progs.skel.h"
> #include "xsk.h"
> #include "xskxceiver.h"
> #include <bpf/bpf.h>
> @@ -356,7 +358,6 @@ static bool ifobj_zc_avail(struct ifobject *ifobject)
> xsk = calloc(1, sizeof(struct xsk_socket_info));
> if (!xsk)
> goto out;
> - ifobject->xdp_flags = XDP_FLAGS_DRV_MODE;
> ifobject->bind_flags = XDP_USE_NEED_WAKEUP | XDP_ZEROCOPY;
> ifobject->rx_on = true;
> xsk->rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> @@ -493,6 +494,10 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> test->total_steps = 1;
> test->nb_sockets = 1;
> test->fail = false;
> + test->xdp_prog_rx = ifobj_rx->xdp_progs->progs.xsk_def_prog;
> + test->xskmap_rx = ifobj_rx->xdp_progs->maps.xsk;
> + test->xdp_prog_tx = ifobj_tx->xdp_progs->progs.xsk_def_prog;
> + test->xskmap_tx = ifobj_tx->xdp_progs->maps.xsk;
Is this needed for Tx side? I believe at this point shared_netdev is set,
no? Or is this just a default state.
> }
>
> static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> @@ -532,6 +537,16 @@ static void test_spec_set_name(struct test_spec *test, const char *name)
> strncpy(test->name, name, MAX_TEST_NAME_SIZE);
> }
>
> +static void test_spec_set_xdp_prog(struct test_spec *test, struct bpf_program *xdp_prog_rx,
> + struct bpf_program *xdp_prog_tx, struct bpf_map *xskmap_rx,
> + struct bpf_map *xskmap_tx)
Nit:
static void
test_spec_set_xdp_prog(struct test_spec *test, struct bpf_program *xdp_prog_rx,
struct bpf_program *xdp_prog_tx, struct bpf_map *xskmap_rx,
struct bpf_map *xskmap_tx)
> +{
> + test->xdp_prog_rx = xdp_prog_rx;
> + test->xdp_prog_tx = xdp_prog_tx;
> + test->xskmap_rx = xskmap_rx;
> + test->xskmap_tx = xskmap_tx;
> +}
> +
> static void pkt_stream_reset(struct pkt_stream *pkt_stream)
> {
> if (pkt_stream)
> @@ -1356,6 +1371,47 @@ static void handler(int signum)
> pthread_exit(NULL);
> }
>
> +static bool xdp_prog_changed(struct test_spec *test, struct ifobject *ifobj)
> +{
> + return ifobj->xdp_prog != test->xdp_prog_rx || ifobj->mode != test->mode;
> +}
> +
> +static void xsk_reattach_xdp(struct ifobject *ifobj, struct bpf_program *xdp_prog,
> + struct bpf_map *xskmap, enum test_mode mode)
> +{
> + int err;
> +
> + xsk_detach_xdp_program(ifobj->ifindex, mode_to_xdp_flags(ifobj->mode));
> + err = xsk_attach_xdp_program(xdp_prog, ifobj->ifindex, mode_to_xdp_flags(mode));
> + if (err) {
> + printf("Error attaching XDP program\n");
> + exit_with_error(-err);
> + }
> +
> + if (ifobj->mode != mode && (mode == TEST_MODE_DRV || mode == TEST_MODE_ZC))
> + if (!xsk_is_in_drv_mode(ifobj->ifindex)) {
> + ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
> + exit_with_error(EINVAL);
> + }
> +
> + ifobj->xdp_prog = xdp_prog;
> + ifobj->xskmap = xskmap;
> + ifobj->mode = mode;
> +}
> +
> +static void xsk_attach_xdp_progs(struct test_spec *test, struct ifobject *ifobj_rx,
> + struct ifobject *ifobj_tx)
> +{
> + if (xdp_prog_changed(test, ifobj_rx))
> + xsk_reattach_xdp(ifobj_rx, test->xdp_prog_rx, test->xskmap_rx, test->mode);
> +
> + if (!ifobj_tx || ifobj_tx->shared_umem)
> + return;
> +
> + if (xdp_prog_changed(test, ifobj_tx))
> + xsk_reattach_xdp(ifobj_tx, test->xdp_prog_tx, test->xskmap_tx, test->mode);
> +}
> +
> static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *ifobj1,
> struct ifobject *ifobj2)
> {
> @@ -1403,7 +1459,11 @@ static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *i
>
> static int testapp_validate_traffic(struct test_spec *test)
> {
> - return __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
> + struct ifobject *ifobj_rx = test->ifobj_rx;
> + struct ifobject *ifobj_tx = test->ifobj_tx;
> +
> + xsk_attach_xdp_progs(test, ifobj_rx, ifobj_tx);
> + return __testapp_validate_traffic(test, ifobj_rx, ifobj_tx);
> }
>
> static int testapp_validate_traffic_single_thread(struct test_spec *test, struct ifobject *ifobj)
> @@ -1446,7 +1506,7 @@ static void testapp_bidi(struct test_spec *test)
>
> print_verbose("Switching Tx/Rx vectors\n");
> swap_directions(&test->ifobj_rx, &test->ifobj_tx);
> - testapp_validate_traffic(test);
> + __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
>
> swap_directions(&test->ifobj_rx, &test->ifobj_tx);
> }
> @@ -1615,29 +1675,15 @@ static void testapp_invalid_desc(struct test_spec *test)
>
> static void testapp_xdp_drop(struct test_spec *test)
> {
> - struct ifobject *ifobj = test->ifobj_rx;
> - int err;
> + struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
> + struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
>
> test_spec_set_name(test, "XDP_DROP_HALF");
> - xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
> - err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_xdp_drop, ifobj->ifindex,
> - ifobj->xdp_flags);
> - if (err) {
> - printf("Error attaching XDP_DROP program\n");
> - test->fail = true;
> - return;
> - }
> + test_spec_set_xdp_prog(test, skel_rx->progs.xsk_xdp_drop, skel_tx->progs.xsk_xdp_drop,
> + skel_rx->maps.xsk, skel_tx->maps.xsk);
>
> pkt_stream_receive_half(test);
> testapp_validate_traffic(test);
> -
> - xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
> - err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
> - ifobj->xdp_flags);
> - if (err) {
> - printf("Error restoring default XDP program\n");
> - exit_with_error(-err);
> - }
> }
>
> static void testapp_poll_txq_tmout(struct test_spec *test)
> @@ -1674,7 +1720,7 @@ static void xsk_unload_xdp_programs(struct ifobject *ifobj)
>
> static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
> const char *dst_ip, const char *src_ip, const u16 dst_port,
> - const u16 src_port, thread_func_t func_ptr, bool load_xdp)
> + const u16 src_port, thread_func_t func_ptr)
> {
> struct in_addr ip;
> int err;
> @@ -1693,23 +1739,11 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
>
> ifobj->func_ptr = func_ptr;
>
> - if (!load_xdp)
> - return;
> -
> err = xsk_load_xdp_programs(ifobj);
> if (err) {
> printf("Error loading XDP program\n");
> exit_with_error(err);
> }
> -
> - ifobj->xdp_flags = mode_to_xdp_flags(TEST_MODE_SKB);
> - err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
> - ifobj->xdp_flags);
> - if (err) {
> - printf("Error attaching XDP program\n");
> - exit_with_error(-err);
> - }
> - ifobj->xskmap = ifobj->xdp_progs->maps.xsk;
> }
>
> static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
> @@ -1871,31 +1905,6 @@ static bool is_xdp_supported(int ifindex)
> return true;
> }
>
> -static void change_to_drv_mode(struct ifobject *ifobj)
> -{
> - LIBBPF_OPTS(bpf_xdp_query_opts, opts);
> - int ret;
> -
> - xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
> - ifobj->xdp_flags = XDP_FLAGS_DRV_MODE;
> - ret = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
> - ifobj->xdp_flags);
> - if (ret) {
> - ksft_print_msg("Error attaching XDP program\n");
> - exit_with_error(-ret);
> - }
> - ifobj->xskmap = ifobj->xdp_progs->maps.xsk;
> -
> - ret = bpf_xdp_query(ifobj->ifindex, XDP_FLAGS_DRV_MODE, &opts);
> - if (ret)
> - exit_with_error(errno);
> -
> - if (opts.attach_mode != XDP_ATTACHED_DRV) {
> - ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
> - exit_with_error(EINVAL);
> - }
> -}
> -
> int main(int argc, char **argv)
> {
> struct pkt_stream *rx_pkt_stream_default;
> @@ -1936,9 +1945,9 @@ int main(int argc, char **argv)
> }
>
> init_iface(ifobj_rx, MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2,
> - worker_testapp_validate_rx, true);
> + worker_testapp_validate_rx);
> init_iface(ifobj_tx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
> - worker_testapp_validate_tx, !shared_netdev);
> + worker_testapp_validate_tx);
>
> test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
> tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
> @@ -1951,12 +1960,6 @@ int main(int argc, char **argv)
> ksft_set_plan(modes * TEST_TYPE_MAX);
>
> for (i = 0; i < modes; i++) {
> - if (i == TEST_MODE_DRV) {
> - change_to_drv_mode(ifobj_rx);
> - if (!shared_netdev)
> - change_to_drv_mode(ifobj_tx);
> - }
> -
> for (j = 0; j < TEST_TYPE_MAX; j++) {
> test_spec_init(&test, ifobj_tx, ifobj_rx, i);
> run_pkt_test(&test, i, j);
> diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> index a4daa5134fc0..3e8ec7d8ec32 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.h
> +++ b/tools/testing/selftests/bpf/xskxceiver.h
> @@ -143,10 +143,11 @@ struct ifobject {
> struct pkt_stream *pkt_stream;
> struct xsk_xdp_progs *xdp_progs;
> struct bpf_map *xskmap;
> + struct bpf_program *xdp_prog;
> + enum test_mode mode;
> int ifindex;
> u32 dst_ip;
> u32 src_ip;
> - u32 xdp_flags;
> u32 bind_flags;
> u16 src_port;
> u16 dst_port;
> @@ -166,6 +167,10 @@ struct test_spec {
> struct ifobject *ifobj_rx;
> struct pkt_stream *tx_pkt_stream_default;
> struct pkt_stream *rx_pkt_stream_default;
> + struct bpf_program *xdp_prog_rx;
> + struct bpf_program *xdp_prog_tx;
> + struct bpf_map *xskmap_rx;
> + struct bpf_map *xskmap_tx;
> u16 total_steps;
> u16 current_step;
> u16 nb_sockets;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (14 preceding siblings ...)
2023-01-04 12:17 ` [PATCH bpf-next v2 15/15] selftests/xsk: automatically switch XDP programs Magnus Karlsson
@ 2023-01-10 12:00 ` Maciej Fijalkowski
2023-01-10 12:10 ` Magnus Karlsson
15 siblings, 1 reply; 29+ messages in thread
From: Maciej Fijalkowski @ 2023-01-10 12:00 UTC (permalink / raw)
To: Magnus Karlsson
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, bpf, yhs, andrii,
martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
tirthendu.sarkar, jonathan.lemon
On Wed, Jan 04, 2023 at 01:17:29PM +0100, Magnus Karlsson wrote:
> This is a patch set of various performance improvements, fixes and the
> introduction of more than one XDP program to the xsk selftests
> framework so we can test more things in the future such as upcoming
> multi-buffer and metadata support for AF_XDP. The new programs just
> reuses the framework that all the other eBPF selftests use. The new
> feature is used to implement one new test that does XDP_DROP on every
> other packet. More tests using this will be added in future commits.
>
> Contents:
>
> * The run-time of the test suite is cut by 10x when executing the
> tests on a real NIC, by only attaching the XDP program once per mode
> tested, instead of once per test program.
>
> * Over 700 lines of code have been removed. The xsk.c control file was
> moved straight over from libbpf when the xsk support was deprecated
> there. As it is now not used as library code that has to work with
> all kinds of versions of Linux, a lot of code could be dropped or
> simplified.
>
> * Add a new command line option "-d" that can be used when a test
> fails and you want to debug it with gdb or some other debugger. The
> option creates the two veth netdevs and prints them to the screen
> without deleting them afterwards. This way these veth netdevs can be
> used when running xskxceiver in a debugger.
>
> * Implemented the possibility to load external XDP programs so we can
> have more than the default one. This feature is used to implement a
> test where every other packet is dropped. Good exercise for the
> recycling mechanism of the xsk buffer pool used in zero-copy mode.
>
> * Various clean-ups and small fixes in patches 1 to 5. None of these
> fixes has any impact on the correct execution of the tests when they
> pass, though they can be irritating when a test fails. IMHO, they do
> not need to go to bpf as they will not fix anything there. The first
> version of patches 1, 2, and 4 where previously sent to bpf, but has
> now been included here.
>
> v1 -> v2:
> * Fixed spelling error in commit message of patch #6 [Björn]
> * Added explanation on why it is safe to use C11 atomics in patch #7
> [Daniel]
> * Put all XDP programs in the same file so that adding more XDP
> programs to xskxceiver.c becomes more scalable in patches #11 and
> #12 [Maciej]
> * Removed more dead code in patch #8 [Maciej]
> * Removed stale %s specifier in error print, patch #9 [Maciej]
> * Changed name of XDP_CONSUMES_SOME_PACKETS to XDP_DROP_HALF to
> hopefully make it clearer [Maciej]
> * ifobj_rx and ifobj_tx name changes in patch #13 [Maciej]
> * Simplified XDP attachment code in patch #15 [Maciej]
I had minor comments on last patch which you can take or not.
From my side it is an ack for whole series:
Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
However you probably would like to ping Daniel against patch 7.
Also, usage of printf vs ksft_print_msg seems sort of random throughout
this series but it's not a big deal.
Thanks!
>
> Patches:
> 1-5: Small fixes and clean-ups
> 6: New convenient debug option when using a debugger such as gdb
> 7-8: Removal of unnecessary code
> 9: Add the ability to load external XDP programs
> 10-11: Removal of more unnecessary code
> 12: Implement a new test where every other packet is XDP_DROP:ed
> 13: Unify the thread dispatching code
> 14-15: Simplify the way tests are written when using custom packet_streams
> or custom XDP programs
>
> Thanks: Magnus
>
> Magnus Karlsson (15):
> selftests/xsk: print correct payload for packet dump
> selftests/xsk: do not close unused file descriptors
> selftests/xsk: submit correct number of frames in populate_fill_ring
> selftests/xsk: print correct error codes when exiting
> selftests/xsk: remove unused variable outstanding_tx
> selftests/xsk: add debug option for creating netdevs
> selftests/xsk: replace asm acquire/release implementations
> selftests/xsk: remove namespaces
> selftests/xsk: load and attach XDP program only once per mode
> selftests/xsk: remove unnecessary code in control path
> selftests/xsk: get rid of built-in XDP program
> selftests/xsk: add test when some packets are XDP_DROPed
> selftests/xsk: merge dual and single thread dispatchers
> selftests/xsk: automatically restore packet stream
> selftests/xsk: automatically switch XDP programs
>
> tools/testing/selftests/bpf/Makefile | 2 +-
> .../selftests/bpf/progs/xsk_xdp_progs.c | 30 +
> tools/testing/selftests/bpf/test_xsk.sh | 42 +-
> tools/testing/selftests/bpf/xsk.c | 674 +-----------------
> tools/testing/selftests/bpf/xsk.h | 97 +--
> tools/testing/selftests/bpf/xsk_prereqs.sh | 12 +-
> tools/testing/selftests/bpf/xskxceiver.c | 382 +++++-----
> tools/testing/selftests/bpf/xskxceiver.h | 17 +-
> 8 files changed, 308 insertions(+), 948 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
>
>
> base-commit: bb5747cfbc4b7fe29621ca6cd4a695d2723bf2e8
> --
> 2.34.1
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 15/15] selftests/xsk: automatically switch XDP programs
2023-01-10 11:56 ` Maciej Fijalkowski
@ 2023-01-10 12:08 ` Magnus Karlsson
0 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-10 12:08 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, bpf, yhs, andrii,
martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
tirthendu.sarkar, jonathan.lemon
On Tue, Jan 10, 2023 at 12:57 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Wed, Jan 04, 2023 at 01:17:44PM +0100, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Implement automatic switching of XDP programs and execution modes if
> > needed by a test. This makes it much simpler to write a test as it
> > only has to say what XDP program it needs if it is not the default
> > one. This also makes it possible to remove the initial explicit
> > attachment of the XDP program as well as the explicit mode switch in
> > the code. These are now handled by the same code that just checks if a
> > switch is necessary, so no special cases are needed.
> >
> > The default XDP program for all tests is one that sends all packets to
> > the AF_XDP socket. If you need another one, please use the new
> > function test_spec_set_xdp_prog() to specify what XDP programs and
> > maps to use for this test.
> >
> > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > ---
> > tools/testing/selftests/bpf/xsk.c | 14 +++
> > tools/testing/selftests/bpf/xsk.h | 1 +
> > tools/testing/selftests/bpf/xskxceiver.c | 137 ++++++++++++-----------
> > tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> > 4 files changed, 91 insertions(+), 68 deletions(-)
> >
> > diff --git a/tools/testing/selftests/bpf/xsk.c b/tools/testing/selftests/bpf/xsk.c
> > index dc6b47280ec4..d9d44a29c7cc 100644
> > --- a/tools/testing/selftests/bpf/xsk.c
> > +++ b/tools/testing/selftests/bpf/xsk.c
> > @@ -267,6 +267,20 @@ int xsk_umem__create(struct xsk_umem **umem_ptr, void *umem_area,
> > return err;
> > }
> >
> > +bool xsk_is_in_drv_mode(u32 ifindex)
> > +{
> > + LIBBPF_OPTS(bpf_xdp_query_opts, opts);
> > + int ret;
> > +
> > + ret = bpf_xdp_query(ifindex, XDP_FLAGS_DRV_MODE, &opts);
> > + if (ret) {
> > + printf("DRV mode query returned error %s\n", strerror(errno));
> > + return false;
> > + }
> > +
> > + return opts.attach_mode == XDP_ATTACHED_DRV;
> > +}
>
> How about making this function more generic since you're adding this to a
> "lib" file? you could take on input the mode that you are expecting the
> prog being loaded on. Not sure if there will be any future use case for
> this, maybe if we would have a support for running a standalone test, not
> the whole test suite. I am sort of bothered that things are hard coded in
> a way that we expect DRV tests to follow the SKB ones.
>
> That's only a rant though :)
Sounds like a good idea. Will give it a shot.
> > +
> > int xsk_attach_xdp_program(struct bpf_program *prog, int ifindex, u32 xdp_flags)
> > {
> > int prog_fd;
> > diff --git a/tools/testing/selftests/bpf/xsk.h b/tools/testing/selftests/bpf/xsk.h
> > index 5624d31b8db7..3cb9d69589b8 100644
> > --- a/tools/testing/selftests/bpf/xsk.h
> > +++ b/tools/testing/selftests/bpf/xsk.h
> > @@ -201,6 +201,7 @@ int xsk_attach_xdp_program(struct bpf_program *prog, int ifindex, u32 xdp_flags)
> > void xsk_detach_xdp_program(int ifindex, u32 xdp_flags);
> > int xsk_update_xskmap(struct bpf_map *map, struct xsk_socket *xsk);
> > void xsk_clear_xskmap(struct bpf_map *map);
> > +bool xsk_is_in_drv_mode(u32 ifindex);
> >
> > struct xsk_socket_config {
> > __u32 rx_size;
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index 66863504c76a..9af0f8240a59 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -96,6 +96,8 @@
> > #include <time.h>
> > #include <unistd.h>
> > #include <stdatomic.h>
> > +
> > +#include "xsk_xdp_progs.skel.h"
> > #include "xsk.h"
> > #include "xskxceiver.h"
> > #include <bpf/bpf.h>
> > @@ -356,7 +358,6 @@ static bool ifobj_zc_avail(struct ifobject *ifobject)
> > xsk = calloc(1, sizeof(struct xsk_socket_info));
> > if (!xsk)
> > goto out;
> > - ifobject->xdp_flags = XDP_FLAGS_DRV_MODE;
> > ifobject->bind_flags = XDP_USE_NEED_WAKEUP | XDP_ZEROCOPY;
> > ifobject->rx_on = true;
> > xsk->rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> > @@ -493,6 +494,10 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> > test->total_steps = 1;
> > test->nb_sockets = 1;
> > test->fail = false;
> > + test->xdp_prog_rx = ifobj_rx->xdp_progs->progs.xsk_def_prog;
> > + test->xskmap_rx = ifobj_rx->xdp_progs->maps.xsk;
> > + test->xdp_prog_tx = ifobj_tx->xdp_progs->progs.xsk_def_prog;
> > + test->xskmap_tx = ifobj_tx->xdp_progs->maps.xsk;
>
> Is this needed for Tx side? I believe at this point shared_netdev is set,
> no? Or is this just a default state.
This is the default state. shared_netdev could be either false or true
here. Though shared_netdev is fixed for the execution of the test
suite, shared_umem might change in the future as we add tests for that
too.
> > }
> >
> > static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> > @@ -532,6 +537,16 @@ static void test_spec_set_name(struct test_spec *test, const char *name)
> > strncpy(test->name, name, MAX_TEST_NAME_SIZE);
> > }
> >
> > +static void test_spec_set_xdp_prog(struct test_spec *test, struct bpf_program *xdp_prog_rx,
> > + struct bpf_program *xdp_prog_tx, struct bpf_map *xskmap_rx,
> > + struct bpf_map *xskmap_tx)
>
>
> Nit:
>
> static void
> test_spec_set_xdp_prog(struct test_spec *test, struct bpf_program *xdp_prog_rx,
> struct bpf_program *xdp_prog_tx, struct bpf_map *xskmap_rx,
> struct bpf_map *xskmap_tx)
>
> > +{
> > + test->xdp_prog_rx = xdp_prog_rx;
> > + test->xdp_prog_tx = xdp_prog_tx;
> > + test->xskmap_rx = xskmap_rx;
> > + test->xskmap_tx = xskmap_tx;
> > +}
> > +
> > static void pkt_stream_reset(struct pkt_stream *pkt_stream)
> > {
> > if (pkt_stream)
> > @@ -1356,6 +1371,47 @@ static void handler(int signum)
> > pthread_exit(NULL);
> > }
> >
> > +static bool xdp_prog_changed(struct test_spec *test, struct ifobject *ifobj)
> > +{
> > + return ifobj->xdp_prog != test->xdp_prog_rx || ifobj->mode != test->mode;
> > +}
> > +
> > +static void xsk_reattach_xdp(struct ifobject *ifobj, struct bpf_program *xdp_prog,
> > + struct bpf_map *xskmap, enum test_mode mode)
> > +{
> > + int err;
> > +
> > + xsk_detach_xdp_program(ifobj->ifindex, mode_to_xdp_flags(ifobj->mode));
> > + err = xsk_attach_xdp_program(xdp_prog, ifobj->ifindex, mode_to_xdp_flags(mode));
> > + if (err) {
> > + printf("Error attaching XDP program\n");
> > + exit_with_error(-err);
> > + }
> > +
> > + if (ifobj->mode != mode && (mode == TEST_MODE_DRV || mode == TEST_MODE_ZC))
> > + if (!xsk_is_in_drv_mode(ifobj->ifindex)) {
> > + ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
> > + exit_with_error(EINVAL);
> > + }
> > +
> > + ifobj->xdp_prog = xdp_prog;
> > + ifobj->xskmap = xskmap;
> > + ifobj->mode = mode;
> > +}
> > +
> > +static void xsk_attach_xdp_progs(struct test_spec *test, struct ifobject *ifobj_rx,
> > + struct ifobject *ifobj_tx)
> > +{
> > + if (xdp_prog_changed(test, ifobj_rx))
> > + xsk_reattach_xdp(ifobj_rx, test->xdp_prog_rx, test->xskmap_rx, test->mode);
> > +
> > + if (!ifobj_tx || ifobj_tx->shared_umem)
> > + return;
> > +
> > + if (xdp_prog_changed(test, ifobj_tx))
> > + xsk_reattach_xdp(ifobj_tx, test->xdp_prog_tx, test->xskmap_tx, test->mode);
> > +}
> > +
> > static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *ifobj1,
> > struct ifobject *ifobj2)
> > {
> > @@ -1403,7 +1459,11 @@ static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *i
> >
> > static int testapp_validate_traffic(struct test_spec *test)
> > {
> > - return __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
> > + struct ifobject *ifobj_rx = test->ifobj_rx;
> > + struct ifobject *ifobj_tx = test->ifobj_tx;
> > +
> > + xsk_attach_xdp_progs(test, ifobj_rx, ifobj_tx);
> > + return __testapp_validate_traffic(test, ifobj_rx, ifobj_tx);
> > }
> >
> > static int testapp_validate_traffic_single_thread(struct test_spec *test, struct ifobject *ifobj)
> > @@ -1446,7 +1506,7 @@ static void testapp_bidi(struct test_spec *test)
> >
> > print_verbose("Switching Tx/Rx vectors\n");
> > swap_directions(&test->ifobj_rx, &test->ifobj_tx);
> > - testapp_validate_traffic(test);
> > + __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
> >
> > swap_directions(&test->ifobj_rx, &test->ifobj_tx);
> > }
> > @@ -1615,29 +1675,15 @@ static void testapp_invalid_desc(struct test_spec *test)
> >
> > static void testapp_xdp_drop(struct test_spec *test)
> > {
> > - struct ifobject *ifobj = test->ifobj_rx;
> > - int err;
> > + struct xsk_xdp_progs *skel_rx = test->ifobj_rx->xdp_progs;
> > + struct xsk_xdp_progs *skel_tx = test->ifobj_tx->xdp_progs;
> >
> > test_spec_set_name(test, "XDP_DROP_HALF");
> > - xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
> > - err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_xdp_drop, ifobj->ifindex,
> > - ifobj->xdp_flags);
> > - if (err) {
> > - printf("Error attaching XDP_DROP program\n");
> > - test->fail = true;
> > - return;
> > - }
> > + test_spec_set_xdp_prog(test, skel_rx->progs.xsk_xdp_drop, skel_tx->progs.xsk_xdp_drop,
> > + skel_rx->maps.xsk, skel_tx->maps.xsk);
> >
> > pkt_stream_receive_half(test);
> > testapp_validate_traffic(test);
> > -
> > - xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
> > - err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
> > - ifobj->xdp_flags);
> > - if (err) {
> > - printf("Error restoring default XDP program\n");
> > - exit_with_error(-err);
> > - }
> > }
> >
> > static void testapp_poll_txq_tmout(struct test_spec *test)
> > @@ -1674,7 +1720,7 @@ static void xsk_unload_xdp_programs(struct ifobject *ifobj)
> >
> > static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
> > const char *dst_ip, const char *src_ip, const u16 dst_port,
> > - const u16 src_port, thread_func_t func_ptr, bool load_xdp)
> > + const u16 src_port, thread_func_t func_ptr)
> > {
> > struct in_addr ip;
> > int err;
> > @@ -1693,23 +1739,11 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
> >
> > ifobj->func_ptr = func_ptr;
> >
> > - if (!load_xdp)
> > - return;
> > -
> > err = xsk_load_xdp_programs(ifobj);
> > if (err) {
> > printf("Error loading XDP program\n");
> > exit_with_error(err);
> > }
> > -
> > - ifobj->xdp_flags = mode_to_xdp_flags(TEST_MODE_SKB);
> > - err = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
> > - ifobj->xdp_flags);
> > - if (err) {
> > - printf("Error attaching XDP program\n");
> > - exit_with_error(-err);
> > - }
> > - ifobj->xskmap = ifobj->xdp_progs->maps.xsk;
> > }
> >
> > static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
> > @@ -1871,31 +1905,6 @@ static bool is_xdp_supported(int ifindex)
> > return true;
> > }
> >
> > -static void change_to_drv_mode(struct ifobject *ifobj)
> > -{
> > - LIBBPF_OPTS(bpf_xdp_query_opts, opts);
> > - int ret;
> > -
> > - xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
> > - ifobj->xdp_flags = XDP_FLAGS_DRV_MODE;
> > - ret = xsk_attach_xdp_program(ifobj->xdp_progs->progs.xsk_def_prog, ifobj->ifindex,
> > - ifobj->xdp_flags);
> > - if (ret) {
> > - ksft_print_msg("Error attaching XDP program\n");
> > - exit_with_error(-ret);
> > - }
> > - ifobj->xskmap = ifobj->xdp_progs->maps.xsk;
> > -
> > - ret = bpf_xdp_query(ifobj->ifindex, XDP_FLAGS_DRV_MODE, &opts);
> > - if (ret)
> > - exit_with_error(errno);
> > -
> > - if (opts.attach_mode != XDP_ATTACHED_DRV) {
> > - ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
> > - exit_with_error(EINVAL);
> > - }
> > -}
> > -
> > int main(int argc, char **argv)
> > {
> > struct pkt_stream *rx_pkt_stream_default;
> > @@ -1936,9 +1945,9 @@ int main(int argc, char **argv)
> > }
> >
> > init_iface(ifobj_rx, MAC1, MAC2, IP1, IP2, UDP_PORT1, UDP_PORT2,
> > - worker_testapp_validate_rx, true);
> > + worker_testapp_validate_rx);
> > init_iface(ifobj_tx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
> > - worker_testapp_validate_tx, !shared_netdev);
> > + worker_testapp_validate_tx);
> >
> > test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
> > tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
> > @@ -1951,12 +1960,6 @@ int main(int argc, char **argv)
> > ksft_set_plan(modes * TEST_TYPE_MAX);
> >
> > for (i = 0; i < modes; i++) {
> > - if (i == TEST_MODE_DRV) {
> > - change_to_drv_mode(ifobj_rx);
> > - if (!shared_netdev)
> > - change_to_drv_mode(ifobj_tx);
> > - }
> > -
> > for (j = 0; j < TEST_TYPE_MAX; j++) {
> > test_spec_init(&test, ifobj_tx, ifobj_rx, i);
> > run_pkt_test(&test, i, j);
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.h b/tools/testing/selftests/bpf/xskxceiver.h
> > index a4daa5134fc0..3e8ec7d8ec32 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.h
> > +++ b/tools/testing/selftests/bpf/xskxceiver.h
> > @@ -143,10 +143,11 @@ struct ifobject {
> > struct pkt_stream *pkt_stream;
> > struct xsk_xdp_progs *xdp_progs;
> > struct bpf_map *xskmap;
> > + struct bpf_program *xdp_prog;
> > + enum test_mode mode;
> > int ifindex;
> > u32 dst_ip;
> > u32 src_ip;
> > - u32 xdp_flags;
> > u32 bind_flags;
> > u16 src_port;
> > u16 dst_port;
> > @@ -166,6 +167,10 @@ struct test_spec {
> > struct ifobject *ifobj_rx;
> > struct pkt_stream *tx_pkt_stream_default;
> > struct pkt_stream *rx_pkt_stream_default;
> > + struct bpf_program *xdp_prog_rx;
> > + struct bpf_program *xdp_prog_tx;
> > + struct bpf_map *xskmap_rx;
> > + struct bpf_map *xskmap_tx;
> > u16 total_steps;
> > u16 current_step;
> > u16 nb_sockets;
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs
2023-01-10 12:00 ` [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new " Maciej Fijalkowski
@ 2023-01-10 12:10 ` Magnus Karlsson
0 siblings, 0 replies; 29+ messages in thread
From: Magnus Karlsson @ 2023-01-10 12:10 UTC (permalink / raw)
To: Maciej Fijalkowski
Cc: magnus.karlsson, bjorn, ast, daniel, netdev, bpf, yhs, andrii,
martin.lau, song, john.fastabend, kpsingh, sdf, haoluo, jolsa,
tirthendu.sarkar, jonathan.lemon
On Tue, Jan 10, 2023 at 1:00 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Wed, Jan 04, 2023 at 01:17:29PM +0100, Magnus Karlsson wrote:
> > This is a patch set of various performance improvements, fixes and the
> > introduction of more than one XDP program to the xsk selftests
> > framework so we can test more things in the future such as upcoming
> > multi-buffer and metadata support for AF_XDP. The new programs just
> > reuses the framework that all the other eBPF selftests use. The new
> > feature is used to implement one new test that does XDP_DROP on every
> > other packet. More tests using this will be added in future commits.
> >
> > Contents:
> >
> > * The run-time of the test suite is cut by 10x when executing the
> > tests on a real NIC, by only attaching the XDP program once per mode
> > tested, instead of once per test program.
> >
> > * Over 700 lines of code have been removed. The xsk.c control file was
> > moved straight over from libbpf when the xsk support was deprecated
> > there. As it is now not used as library code that has to work with
> > all kinds of versions of Linux, a lot of code could be dropped or
> > simplified.
> >
> > * Add a new command line option "-d" that can be used when a test
> > fails and you want to debug it with gdb or some other debugger. The
> > option creates the two veth netdevs and prints them to the screen
> > without deleting them afterwards. This way these veth netdevs can be
> > used when running xskxceiver in a debugger.
> >
> > * Implemented the possibility to load external XDP programs so we can
> > have more than the default one. This feature is used to implement a
> > test where every other packet is dropped. Good exercise for the
> > recycling mechanism of the xsk buffer pool used in zero-copy mode.
> >
> > * Various clean-ups and small fixes in patches 1 to 5. None of these
> > fixes has any impact on the correct execution of the tests when they
> > pass, though they can be irritating when a test fails. IMHO, they do
> > not need to go to bpf as they will not fix anything there. The first
> > version of patches 1, 2, and 4 where previously sent to bpf, but has
> > now been included here.
> >
> > v1 -> v2:
> > * Fixed spelling error in commit message of patch #6 [Björn]
> > * Added explanation on why it is safe to use C11 atomics in patch #7
> > [Daniel]
> > * Put all XDP programs in the same file so that adding more XDP
> > programs to xskxceiver.c becomes more scalable in patches #11 and
> > #12 [Maciej]
> > * Removed more dead code in patch #8 [Maciej]
> > * Removed stale %s specifier in error print, patch #9 [Maciej]
> > * Changed name of XDP_CONSUMES_SOME_PACKETS to XDP_DROP_HALF to
> > hopefully make it clearer [Maciej]
> > * ifobj_rx and ifobj_tx name changes in patch #13 [Maciej]
> > * Simplified XDP attachment code in patch #15 [Maciej]
>
> I had minor comments on last patch which you can take or not.
> From my side it is an ack for whole series:
>
> Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
>
> However you probably would like to ping Daniel against patch 7.
> Also, usage of printf vs ksft_print_msg seems sort of random throughout
> this series but it's not a big deal.
Yep, it has been more or less random from day one, unfortunately. I
will look into it for the next patch set of fixes.
> Thanks!
>
> >
> > Patches:
> > 1-5: Small fixes and clean-ups
> > 6: New convenient debug option when using a debugger such as gdb
> > 7-8: Removal of unnecessary code
> > 9: Add the ability to load external XDP programs
> > 10-11: Removal of more unnecessary code
> > 12: Implement a new test where every other packet is XDP_DROP:ed
> > 13: Unify the thread dispatching code
> > 14-15: Simplify the way tests are written when using custom packet_streams
> > or custom XDP programs
> >
> > Thanks: Magnus
> >
> > Magnus Karlsson (15):
> > selftests/xsk: print correct payload for packet dump
> > selftests/xsk: do not close unused file descriptors
> > selftests/xsk: submit correct number of frames in populate_fill_ring
> > selftests/xsk: print correct error codes when exiting
> > selftests/xsk: remove unused variable outstanding_tx
> > selftests/xsk: add debug option for creating netdevs
> > selftests/xsk: replace asm acquire/release implementations
> > selftests/xsk: remove namespaces
> > selftests/xsk: load and attach XDP program only once per mode
> > selftests/xsk: remove unnecessary code in control path
> > selftests/xsk: get rid of built-in XDP program
> > selftests/xsk: add test when some packets are XDP_DROPed
> > selftests/xsk: merge dual and single thread dispatchers
> > selftests/xsk: automatically restore packet stream
> > selftests/xsk: automatically switch XDP programs
> >
> > tools/testing/selftests/bpf/Makefile | 2 +-
> > .../selftests/bpf/progs/xsk_xdp_progs.c | 30 +
> > tools/testing/selftests/bpf/test_xsk.sh | 42 +-
> > tools/testing/selftests/bpf/xsk.c | 674 +-----------------
> > tools/testing/selftests/bpf/xsk.h | 97 +--
> > tools/testing/selftests/bpf/xsk_prereqs.sh | 12 +-
> > tools/testing/selftests/bpf/xskxceiver.c | 382 +++++-----
> > tools/testing/selftests/bpf/xskxceiver.h | 17 +-
> > 8 files changed, 308 insertions(+), 948 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> >
> >
> > base-commit: bb5747cfbc4b7fe29621ca6cd4a695d2723bf2e8
> > --
> > 2.34.1
^ permalink raw reply [flat|nested] 29+ messages in thread
* Re: [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program
2023-01-04 23:44 ` Stanislav Fomichev
@ 2023-01-11 19:20 ` Andrii Nakryiko
0 siblings, 0 replies; 29+ messages in thread
From: Andrii Nakryiko @ 2023-01-11 19:20 UTC (permalink / raw)
To: Stanislav Fomichev
Cc: Magnus Karlsson, magnus.karlsson, bjorn, ast, daniel, netdev,
maciej.fijalkowski, bpf, yhs, andrii, martin.lau, song,
john.fastabend, kpsingh, haoluo, jolsa, tirthendu.sarkar,
jonathan.lemon
On Wed, Jan 4, 2023 at 3:44 PM Stanislav Fomichev <sdf@google.com> wrote:
>
> On Wed, Jan 4, 2023 at 3:15 PM Andrii Nakryiko
> <andrii.nakryiko@gmail.com> wrote:
> >
> > On Wed, Jan 4, 2023 at 4:19 AM Magnus Karlsson
> > <magnus.karlsson@gmail.com> wrote:
> > >
> > > From: Magnus Karlsson <magnus.karlsson@intel.com>
> > >
> > > Get rid of the built-in XDP program that was part of the old libbpf
> > > code in xsk.c and replace it with an eBPF program build using the
> > > framework by all the other bpf selftests. This will form the base for
> > > adding more programs in later commits.
> > >
> > > Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
> > > ---
> > > tools/testing/selftests/bpf/Makefile | 2 +-
> > > .../selftests/bpf/progs/xsk_xdp_progs.c | 19 ++++
> > > tools/testing/selftests/bpf/xsk.c | 88 ++++---------------
> > > tools/testing/selftests/bpf/xsk.h | 6 +-
> > > tools/testing/selftests/bpf/xskxceiver.c | 72 ++++++++-------
> > > tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> > > 6 files changed, 88 insertions(+), 106 deletions(-)
> > > create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_progs.c
> > >
> > > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > > index 205e8c3c346a..a0193a8f9da6 100644
> > > --- a/tools/testing/selftests/bpf/Makefile
> > > +++ b/tools/testing/selftests/bpf/Makefile
> > > @@ -240,7 +240,7 @@ $(OUTPUT)/flow_dissector_load: $(TESTING_HELPERS)
> > > $(OUTPUT)/test_maps: $(TESTING_HELPERS)
> > > $(OUTPUT)/test_verifier: $(TESTING_HELPERS) $(CAP_HELPERS)
> > > $(OUTPUT)/xsk.o: $(BPFOBJ)
> >
> > shouldn't $(OUTPUT)/xsk_xdp_progs.skel.h be added as a dependency
> > here, at .o file?
>
> Not sure we can:
> xsk.o is a 'generic' library and xsk_xdp_progs.skel.h is xskxceiver-specific.
Oh, I'm just not very familiar with xskxceiver. What I meant was that
this skel.h header should be a dependency of an object file that
results from .c file that includes that skel.h header. It seems like
it's a xskxceiver.c -> xskxceiver.o in this case, that's where I'd add
dependency.
But it might not be possible for some reason (as we compile
test_verifier straight from test_verifier.c, bypassing .o creation),
which I might be forgetting.
>
> I was trying to see how it works for the other cases where we depend
> on the headers and saw the following:
>
> $(OUTPUT)/test_verifier: test_verifier.c verifier/tests.h $(BPFOBJ) | $(OUTPUT)
> $(call msg,BINARY,,$@)
> $(Q)$(CC) $(CFLAGS) $(filter %.a %.o %.c,$^) $(LDLIBS) -o $@
>
> So at least for test_verifier, we explicitly filter out anything
> non-.[aoc]. Presumably because of the same issue?
> Should we do the same for xskxceiver? I've sent similar changes for my
> xdp_hw_metadata binary about an hour ago..
We do filter "irrelevant" inputs yes, as a general rule. And we can do
that for other binaries as well, yep.
>
> > > -$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o
> > > +$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_xdp_progs.skel.h
> >
> > and not here. Is that why we have this clang compilation failure?
> >
> > >
> > > BPFTOOL ?= $(DEFAULT_BPFTOOL)
> > > $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> >
> > [...]
^ permalink raw reply [flat|nested] 29+ messages in thread
end of thread, other threads:[~2023-01-11 19:20 UTC | newest]
Thread overview: 29+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-01-04 12:17 [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 01/15] selftests/xsk: print correct payload for packet dump Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 02/15] selftests/xsk: do not close unused file descriptors Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 03/15] selftests/xsk: submit correct number of frames in populate_fill_ring Magnus Karlsson
2023-01-09 23:01 ` Maciej Fijalkowski
2023-01-04 12:17 ` [PATCH bpf-next v2 04/15] selftests/xsk: print correct error codes when exiting Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 05/15] selftests/xsk: remove unused variable outstanding_tx Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 06/15] selftests/xsk: add debug option for creating netdevs Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 07/15] selftests/xsk: replace asm acquire/release implementations Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 08/15] selftests/xsk: remove namespaces Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 09/15] selftests/xsk: load and attach XDP program only once per mode Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 10/15] selftests/xsk: remove unnecessary code in control path Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 11/15] selftests/xsk: get rid of built-in XDP program Magnus Karlsson
2023-01-04 18:14 ` David Vernet
2023-01-04 18:19 ` Stanislav Fomichev
2023-01-04 19:13 ` David Vernet
2023-01-04 19:46 ` Stanislav Fomichev
2023-01-05 8:38 ` Magnus Karlsson
2023-01-04 23:15 ` Andrii Nakryiko
2023-01-04 23:44 ` Stanislav Fomichev
2023-01-11 19:20 ` Andrii Nakryiko
2023-01-04 12:17 ` [PATCH bpf-next v2 12/15] selftests/xsk: add test when some packets are XDP_DROPed Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 13/15] selftests/xsk: merge dual and single thread dispatchers Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 14/15] selftests/xsk: automatically restore packet stream Magnus Karlsson
2023-01-04 12:17 ` [PATCH bpf-next v2 15/15] selftests/xsk: automatically switch XDP programs Magnus Karlsson
2023-01-10 11:56 ` Maciej Fijalkowski
2023-01-10 12:08 ` Magnus Karlsson
2023-01-10 12:00 ` [PATCH bpf-next v2 00/15] selftests/xsk: speed-ups, fixes, and new " Maciej Fijalkowski
2023-01-10 12:10 ` Magnus Karlsson
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).