* [PATCH bpf-next 01/15] selftests/xsk: print correct payload for packet dump
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 02/15] selftests/xsk: do not close unused file descriptors Magnus Karlsson
` (13 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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] 28+ messages in thread* [PATCH bpf-next 02/15] selftests/xsk: do not close unused file descriptors
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 01/15] selftests/xsk: print correct payload for packet dump Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 03/15] selftests/xsk: submit correct number of frames in populate_fill_ring Magnus Karlsson
` (12 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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] 28+ messages in thread* [PATCH bpf-next 03/15] selftests/xsk: submit correct number of frames in populate_fill_ring
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 01/15] selftests/xsk: print correct payload for packet dump Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 02/15] selftests/xsk: do not close unused file descriptors Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 04/15] selftests/xsk: print correct error codes when exiting Magnus Karlsson
` (11 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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] 28+ messages in thread* [PATCH bpf-next 04/15] selftests/xsk: print correct error codes when exiting
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (2 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 03/15] selftests/xsk: submit correct number of frames in populate_fill_ring Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 05/15] selftests/xsk: remove unused variable outstanding_tx Magnus Karlsson
` (10 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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] 28+ messages in thread* [PATCH bpf-next 05/15] selftests/xsk: remove unused variable outstanding_tx
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (3 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 04/15] selftests/xsk: print correct error codes when exiting Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 06/15] selftests/xsk: add debug option for creating netdevs Magnus Karlsson
` (9 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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] 28+ messages in thread* [PATCH bpf-next 06/15] selftests/xsk: add debug option for creating netdevs
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (4 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 05/15] selftests/xsk: remove unused variable outstanding_tx Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-07 13:39 ` Björn Töpel
2022-12-06 9:08 ` [PATCH bpf-next 07/15] selftests/xsk: get rid of asm store/release implementations Magnus Karlsson
` (8 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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 dugged 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] 28+ messages in thread* Re: [PATCH bpf-next 06/15] selftests/xsk: add debug option for creating netdevs
2022-12-06 9:08 ` [PATCH bpf-next 06/15] selftests/xsk: add debug option for creating netdevs Magnus Karlsson
@ 2022-12-07 13:39 ` Björn Töpel
0 siblings, 0 replies; 28+ messages in thread
From: Björn Töpel @ 2022-12-07 13:39 UTC (permalink / raw)
To: Magnus Karlsson, magnus.karlsson, ast, daniel, netdev,
maciej.fijalkowski, bpf, yhs, andrii, martin.lau, song,
john.fastabend, kpsingh, sdf, haoluo, jolsa
Cc: jonathan.lemon
Magnus Karlsson <magnus.karlsson@gmail.com> writes:
> 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 dugged with gdb.
Nit: "debugged"
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH bpf-next 07/15] selftests/xsk: get rid of asm store/release implementations
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (5 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 06/15] selftests/xsk: add debug option for creating netdevs Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-06 23:48 ` Daniel Borkmann
2022-12-06 9:08 ` [PATCH bpf-next 08/15] selftests/xsk: remove namespaces Magnus Karlsson
` (7 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
Cc: jonathan.lemon
From: Magnus Karlsson <magnus.karlsson@intel.com>
Get rid of our own homegrown assembly store/release and load/acquire
implementations. Use the HW agnositic APIs the compiler offers
instead.
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] 28+ messages in thread* Re: [PATCH bpf-next 07/15] selftests/xsk: get rid of asm store/release implementations
2022-12-06 9:08 ` [PATCH bpf-next 07/15] selftests/xsk: get rid of asm store/release implementations Magnus Karlsson
@ 2022-12-06 23:48 ` Daniel Borkmann
2022-12-07 8:23 ` Magnus Karlsson
0 siblings, 1 reply; 28+ messages in thread
From: Daniel Borkmann @ 2022-12-06 23:48 UTC (permalink / raw)
To: Magnus Karlsson, magnus.karlsson, bjorn, ast, netdev,
maciej.fijalkowski, bpf, yhs, andrii, martin.lau, song,
john.fastabend, kpsingh, sdf, haoluo, jolsa
Cc: jonathan.lemon
On 12/6/22 10:08 AM, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> Get rid of our own homegrown assembly store/release and load/acquire
> implementations. Use the HW agnositic APIs the compiler offers
> instead.
The description is a bit terse. Could you add a bit more context, discussion
or reference on why it's safe to replace them with C11 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)
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH bpf-next 07/15] selftests/xsk: get rid of asm store/release implementations
2022-12-06 23:48 ` Daniel Borkmann
@ 2022-12-07 8:23 ` Magnus Karlsson
0 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-07 8:23 UTC (permalink / raw)
To: Daniel Borkmann
Cc: magnus.karlsson, bjorn, ast, netdev, maciej.fijalkowski, bpf,
yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa, jonathan.lemon
On Wed, Dec 7, 2022 at 12:48 AM Daniel Borkmann <daniel@iogearbox.net> wrote:
>
> On 12/6/22 10:08 AM, Magnus Karlsson wrote:
> > From: Magnus Karlsson <magnus.karlsson@intel.com>
> >
> > Get rid of our own homegrown assembly store/release and load/acquire
> > implementations. Use the HW agnositic APIs the compiler offers
> > instead.
>
> The description is a bit terse. Could you add a bit more context, discussion
> or reference on why it's safe to replace them with C11 atomics?
Will do, though I will hold off on a v2 in case there are further comments.
> > 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)
> >
>
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH bpf-next 08/15] selftests/xsk: remove namespaces
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (6 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 07/15] selftests/xsk: get rid of asm store/release implementations Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-12 14:19 ` Maciej Fijalkowski
2022-12-06 9:08 ` [PATCH bpf-next 09/15] selftests/xsk: load and attach XDP program only once per mode Magnus Karlsson
` (6 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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 | 42 +++-------------------
tools/testing/selftests/bpf/xskxceiver.h | 3 --
4 files changed, 19 insertions(+), 71 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..0aaf2f0a9d75 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,7 +415,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
opterr = 0;
for (;;) {
- char *sptr, *token;
+ char *sptr;
c = getopt_long(argc, argv, "i:Dvb", long_options, &option_index);
if (c == -1)
@@ -455,9 +432,6 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
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);
interface_nb++;
break;
case 'D':
@@ -1283,8 +1257,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 +1815,6 @@ static struct ifobject *ifobject_create(void)
if (!ifobj->umem)
goto out_umem;
- ifobj->ns_fd = -1;
-
return ifobj;
out_umem:
@@ -1856,8 +1826,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] 28+ messages in thread* Re: [PATCH bpf-next 08/15] selftests/xsk: remove namespaces
2022-12-06 9:08 ` [PATCH bpf-next 08/15] selftests/xsk: remove namespaces Magnus Karlsson
@ 2022-12-12 14:19 ` Maciej Fijalkowski
2022-12-14 10:44 ` Magnus Karlsson
0 siblings, 1 reply; 28+ messages in thread
From: Maciej Fijalkowski @ 2022-12-12 14:19 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,
jonathan.lemon
On Tue, Dec 06, 2022 at 10:08:19AM +0100, Magnus Karlsson wrote:
> 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 | 42 +++-------------------
> tools/testing/selftests/bpf/xskxceiver.h | 3 --
> 4 files changed, 19 insertions(+), 71 deletions(-)
>
(...)
> diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> index 72578cebfbf7..0aaf2f0a9d75 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,7 +415,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> opterr = 0;
>
> for (;;) {
> - char *sptr, *token;
> + char *sptr;
>
> c = getopt_long(argc, argv, "i:Dvb", long_options, &option_index);
> if (c == -1)
> @@ -455,9 +432,6 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>
> sptr = strndupa(optarg, strlen(optarg));
Wasn't this strndupa needed only because of strsep usage?
I feel that now you can just memcpy directly from optarg to ifobj->nsname.
> memcpy(ifobj->ifname, strsep(&sptr, ","), MAX_INTERFACE_NAME_CHARS);
> - token = strsep(&sptr, ",");
> - if (token)
> - memcpy(ifobj->nsname, token, MAX_INTERFACES_NAMESPACE_CHARS);
> interface_nb++;
> break;
> case 'D':
> @@ -1283,8 +1257,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 +1815,6 @@ static struct ifobject *ifobject_create(void)
> if (!ifobj->umem)
> goto out_umem;
>
> - ifobj->ns_fd = -1;
> -
> return ifobj;
>
> out_umem:
> @@ -1856,8 +1826,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);
(...)
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH bpf-next 08/15] selftests/xsk: remove namespaces
2022-12-12 14:19 ` Maciej Fijalkowski
@ 2022-12-14 10:44 ` Magnus Karlsson
0 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-14 10:44 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,
jonathan.lemon
On Mon, Dec 12, 2022 at 3:20 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Tue, Dec 06, 2022 at 10:08:19AM +0100, Magnus Karlsson wrote:
> > 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 | 42 +++-------------------
> > tools/testing/selftests/bpf/xskxceiver.h | 3 --
> > 4 files changed, 19 insertions(+), 71 deletions(-)
> >
>
> (...)
>
> > diff --git a/tools/testing/selftests/bpf/xskxceiver.c b/tools/testing/selftests/bpf/xskxceiver.c
> > index 72578cebfbf7..0aaf2f0a9d75 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,7 +415,7 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> > opterr = 0;
> >
> > for (;;) {
> > - char *sptr, *token;
> > + char *sptr;
> >
> > c = getopt_long(argc, argv, "i:Dvb", long_options, &option_index);
> > if (c == -1)
> > @@ -455,9 +432,6 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >
> > sptr = strndupa(optarg, strlen(optarg));
>
> Wasn't this strndupa needed only because of strsep usage?
> I feel that now you can just memcpy directly from optarg to ifobj->nsname.
Will fix.
> > memcpy(ifobj->ifname, strsep(&sptr, ","), MAX_INTERFACE_NAME_CHARS);
> > - token = strsep(&sptr, ",");
> > - if (token)
> > - memcpy(ifobj->nsname, token, MAX_INTERFACES_NAMESPACE_CHARS);
> > interface_nb++;
> > break;
> > case 'D':
> > @@ -1283,8 +1257,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 +1815,6 @@ static struct ifobject *ifobject_create(void)
> > if (!ifobj->umem)
> > goto out_umem;
> >
> > - ifobj->ns_fd = -1;
> > -
> > return ifobj;
> >
> > out_umem:
> > @@ -1856,8 +1826,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);
>
> (...)
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH bpf-next 09/15] selftests/xsk: load and attach XDP program only once per mode
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (7 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 08/15] selftests/xsk: remove namespaces Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-12 15:01 ` Maciej Fijalkowski
2022-12-06 9:08 ` [PATCH bpf-next 10/15] selftests/xsk: remove unnecessary code in control path Magnus Karlsson
` (5 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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 0aaf2f0a9d75..5f22ee88a523 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;
@@ -432,6 +436,11 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
sptr = strndupa(optarg, strlen(optarg));
memcpy(ifobj->ifname, strsep(&sptr, ","), MAX_INTERFACE_NAME_CHARS);
+
+ ifobj->ifindex = if_nametoindex(ifobj->ifname);
+ if (!ifobj->ifindex)
+ exit_with_error(errno);
+
interface_nb++;
break;
case 'D':
@@ -512,12 +521,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;
@@ -1254,7 +1257,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)
@@ -1280,31 +1284,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);
}
@@ -1338,15 +1319,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);
@@ -1415,7 +1400,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);
}
@@ -1504,14 +1492,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);
}
@@ -1675,8 +1663,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);
@@ -1692,6 +1681,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)
@@ -1826,12 +1833,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;
@@ -1840,7 +1850,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;
@@ -1860,6 +1869,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) {
+ printf("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: [%s] XDP prog not in DRV mode\n");
+ exit_with_error(EINVAL);
+ }
+}
+
int main(int argc, char **argv)
{
struct pkt_stream *rx_pkt_stream_default;
@@ -1868,7 +1900,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);
@@ -1883,27 +1915,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);
@@ -1914,7 +1946,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);
@@ -1923,6 +1961,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] 28+ messages in thread* Re: [PATCH bpf-next 09/15] selftests/xsk: load and attach XDP program only once per mode
2022-12-06 9:08 ` [PATCH bpf-next 09/15] selftests/xsk: load and attach XDP program only once per mode Magnus Karlsson
@ 2022-12-12 15:01 ` Maciej Fijalkowski
2022-12-14 10:50 ` Magnus Karlsson
0 siblings, 1 reply; 28+ messages in thread
From: Maciej Fijalkowski @ 2022-12-12 15: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,
jonathan.lemon
On Tue, Dec 06, 2022 at 10:08:20AM +0100, Magnus Karlsson wrote:
> 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);
Next step would be to create the map once per whole test suite and pin it?
:p I sort of wanted to ask if this would make sense, but I would consider
this as over-optimization. Once per mode is more than enough.
> + 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 0aaf2f0a9d75..5f22ee88a523 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;
> @@ -432,6 +436,11 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
>
> sptr = strndupa(optarg, strlen(optarg));
> memcpy(ifobj->ifname, strsep(&sptr, ","), MAX_INTERFACE_NAME_CHARS);
> +
> + ifobj->ifindex = if_nametoindex(ifobj->ifname);
> + if (!ifobj->ifindex)
> + exit_with_error(errno);
> +
> interface_nb++;
> break;
> case 'D':
> @@ -512,12 +521,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;
> @@ -1254,7 +1257,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)
> @@ -1280,31 +1284,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);
> }
> @@ -1338,15 +1319,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;
move up to pretend like we care about RCT?
>
> 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);
> @@ -1415,7 +1400,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);
> }
>
> @@ -1504,14 +1492,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);
> }
> @@ -1675,8 +1663,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);
> @@ -1692,6 +1681,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);
Are you going to have other callsites of mode_to_xdp_flags() ?
Currently there is only this single usage which probably could be replaced
with explicit XDP_FLAGS_SKB_MODE.
> + 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)
> @@ -1826,12 +1833,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;
>
> @@ -1840,7 +1850,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;
>
> @@ -1860,6 +1869,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) {
> + printf("Error attaching XDP program\n");
Nit: ksft_print_msg or we don't care?
> + 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: [%s] XDP prog not in DRV mode\n");
Aren't you missing the thing to print out for %s format specifier?
I thought that ksft_print_msg() is doing something with that under the
hood, but apparently it's not the case.
> + exit_with_error(EINVAL);
> + }
> +}
> +
> int main(int argc, char **argv)
> {
> struct pkt_stream *rx_pkt_stream_default;
> @@ -1868,7 +1900,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);
> @@ -1883,27 +1915,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;
Hmm looks bit odd. I'd keep shared_umem as local var here or update the
name in struct as well.
>
> 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);
> @@ -1914,7 +1946,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);
> @@ -1923,6 +1961,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 [flat|nested] 28+ messages in thread* Re: [PATCH bpf-next 09/15] selftests/xsk: load and attach XDP program only once per mode
2022-12-12 15:01 ` Maciej Fijalkowski
@ 2022-12-14 10:50 ` Magnus Karlsson
0 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-14 10:50 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,
jonathan.lemon
On Mon, Dec 12, 2022 at 4:02 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Tue, Dec 06, 2022 at 10:08:20AM +0100, Magnus Karlsson wrote:
> > 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);
>
> Next step would be to create the map once per whole test suite and pin it?
> :p I sort of wanted to ask if this would make sense, but I would consider
> this as over-optimization. Once per mode is more than enough.
>
> > + 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 0aaf2f0a9d75..5f22ee88a523 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;
> > @@ -432,6 +436,11 @@ static void parse_command_line(struct ifobject *ifobj_tx, struct ifobject *ifobj
> >
> > sptr = strndupa(optarg, strlen(optarg));
> > memcpy(ifobj->ifname, strsep(&sptr, ","), MAX_INTERFACE_NAME_CHARS);
> > +
> > + ifobj->ifindex = if_nametoindex(ifobj->ifname);
> > + if (!ifobj->ifindex)
> > + exit_with_error(errno);
> > +
> > interface_nb++;
> > break;
> > case 'D':
> > @@ -512,12 +521,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;
> > @@ -1254,7 +1257,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)
> > @@ -1280,31 +1284,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);
> > }
> > @@ -1338,15 +1319,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;
>
> move up to pretend like we care about RCT?
Will fix.
> >
> > 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);
> > @@ -1415,7 +1400,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);
> > }
> >
> > @@ -1504,14 +1492,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);
> > }
> > @@ -1675,8 +1663,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);
> > @@ -1692,6 +1681,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);
>
> Are you going to have other callsites of mode_to_xdp_flags() ?
> Currently there is only this single usage which probably could be replaced
> with explicit XDP_FLAGS_SKB_MODE.
Yes, there will be more.
> > + 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)
> > @@ -1826,12 +1833,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;
> >
> > @@ -1840,7 +1850,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;
> >
> > @@ -1860,6 +1869,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) {
> > + printf("Error attaching XDP program\n");
>
> Nit: ksft_print_msg or we don't care?
>
> > + 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: [%s] XDP prog not in DRV mode\n");
>
> Aren't you missing the thing to print out for %s format specifier?
> I thought that ksft_print_msg() is doing something with that under the
> hood, but apparently it's not the case.
Yes, that %s is bonkers and was in the original code I moved here.
Will remove it.
> > + exit_with_error(EINVAL);
> > + }
> > +}
> > +
> > int main(int argc, char **argv)
> > {
> > struct pkt_stream *rx_pkt_stream_default;
> > @@ -1868,7 +1900,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);
> > @@ -1883,27 +1915,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;
>
> Hmm looks bit odd. I'd keep shared_umem as local var here or update the
> name in struct as well.
>
> >
> > 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);
> > @@ -1914,7 +1946,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);
> > @@ -1923,6 +1961,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 [flat|nested] 28+ messages in thread
* [PATCH bpf-next 10/15] selftests/xsk: remove unnecessary code in control path
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (8 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 09/15] selftests/xsk: load and attach XDP program only once per mode Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 11/15] selftests/xsk: get rid of built-in XDP program Magnus Karlsson
` (4 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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 | 2 -
3 files changed, 3 insertions(+), 625 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 5f22ee88a523..81b0a3ca05a6 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -326,8 +326,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] 28+ messages in thread* [PATCH bpf-next 11/15] selftests/xsk: get rid of built-in XDP program
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (9 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 10/15] selftests/xsk: remove unnecessary code in control path Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 12/15] selftests/xsk: add test when some packets are XDP_DROPed Magnus Karlsson
` (3 subsequent siblings)
14 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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_def_prog.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_def_prog.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 6a0f043dc410..42e15b5a34a7 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_def_prog.skel.h
BPFTOOL ?= $(DEFAULT_BPFTOOL)
$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
diff --git a/tools/testing/selftests/bpf/progs/xsk_def_prog.c b/tools/testing/selftests/bpf/progs/xsk_def_prog.c
new file mode 100644
index 000000000000..698176882ac6
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/xsk_def_prog.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 81b0a3ca05a6..0cda4e3f1871 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1215,7 +1215,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));
}
@@ -1255,9 +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);
- u32 queue_id = 0;
- int ret, fd;
void *bufs;
+ int ret;
if (ifobject->umem->unaligned_mode)
mmap_flags |= MAP_HUGETLB;
@@ -1282,8 +1281,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);
}
@@ -1317,18 +1315,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);
}
}
@@ -1398,10 +1395,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);
}
@@ -1490,14 +1485,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);
}
@@ -1659,12 +1654,26 @@ static void testapp_invalid_desc(struct test_spec *test)
pkt_stream_restore_default(test);
}
+static int xsk_load_xdp_program(struct ifobject *ifobj)
+{
+ ifobj->def_prog = xsk_def_prog__open_and_load();
+ if (libbpf_get_error(ifobj->def_prog))
+ return libbpf_get_error(ifobj->def_prog);
+
+ return 0;
+}
+
+static void xsk_unload_xdp_program(struct ifobject *ifobj)
+{
+ xsk_def_prog__destroy(ifobj->def_prog);
+}
+
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);
@@ -1683,20 +1692,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_program(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->def_prog->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->def_prog->maps.xsk;
}
static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
@@ -1831,9 +1840,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);
@@ -1872,13 +1878,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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
+ ifobj->xdp_flags);
+ if (ret) {
printf("Error attaching XDP program\n");
- exit_with_error(-ifobj->link_fd);
+ exit_with_error(-ret);
}
+ ifobj->xskmap = ifobj->def_prog->maps.xsk;
ret = bpf_xdp_query(ifobj->ifindex, XDP_FLAGS_DRV_MODE, &opts);
if (ret)
@@ -1963,6 +1971,8 @@ int main(int argc, char **argv)
pkt_stream_delete(tx_pkt_stream_default);
pkt_stream_delete(rx_pkt_stream_default);
+ xsk_unload_xdp_program(ifobj_tx);
+ xsk_unload_xdp_program(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..eb6355bcc143 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_def_prog.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_def_prog *def_prog;
+ struct bpf_map *xskmap;
int ifindex;
u32 dst_ip;
u32 src_ip;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* [PATCH bpf-next 12/15] selftests/xsk: add test when some packets are XDP_DROPed
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (10 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 11/15] selftests/xsk: get rid of built-in XDP program Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-12 15:13 ` Maciej Fijalkowski
2022-12-06 9:08 ` [PATCH bpf-next 13/15] selftests/xsk: merge dual and single thread dispatchers Magnus Karlsson
` (2 subsequent siblings)
14 siblings, 1 reply; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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>
---
tools/testing/selftests/bpf/Makefile | 2 +-
.../selftests/bpf/progs/xsk_xdp_drop.c | 25 ++++++++++
tools/testing/selftests/bpf/xskxceiver.c | 48 +++++++++++++++++--
tools/testing/selftests/bpf/xskxceiver.h | 3 ++
4 files changed, 72 insertions(+), 6 deletions(-)
create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_drop.c
diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
index 42e15b5a34a7..77ef8a8e6db4 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)/xsk_def_prog.skel.h
+$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_def_prog.skel.h $(OUTPUT)/xsk_xdp_drop.skel.h
BPFTOOL ?= $(DEFAULT_BPFTOOL)
$(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_drop.c b/tools/testing/selftests/bpf/progs/xsk_xdp_drop.c
new file mode 100644
index 000000000000..12a12b0d9fc1
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/xsk_xdp_drop.c
@@ -0,0 +1,25 @@
+// 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");
+
+static unsigned int idx;
+
+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 0cda4e3f1871..522dc1d69c17 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1654,18 +1654,53 @@ static void testapp_invalid_desc(struct test_spec *test)
pkt_stream_restore_default(test);
}
-static int xsk_load_xdp_program(struct ifobject *ifobj)
+static void testapp_xdp_drop(struct test_spec *test)
+{
+ struct ifobject *ifobj = test->ifobj_rx;
+ int err;
+
+ test_spec_set_name(test, "XDP_CONSUMES_SOME_PACKETS");
+ xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
+ err = xsk_attach_xdp_program(ifobj->xdp_drop->progs.xsk_xdp_drop, ifobj->ifindex,
+ ifobj->xdp_flags);
+ if (err) {
+ printf("Error attaching XDP_DROP program\n");
+ test->fail = true;
+ return;
+ }
+ ifobj->xskmap = ifobj->xdp_drop->maps.xsk;
+
+ 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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
+ ifobj->xdp_flags);
+ if (err) {
+ printf("Error restoring default XDP program\n");
+ exit_with_error(-err);
+ }
+ ifobj->xskmap = ifobj->def_prog->maps.xsk;
+}
+
+static int xsk_load_xdp_programs(struct ifobject *ifobj)
{
ifobj->def_prog = xsk_def_prog__open_and_load();
if (libbpf_get_error(ifobj->def_prog))
return libbpf_get_error(ifobj->def_prog);
+ ifobj->xdp_drop = xsk_xdp_drop__open_and_load();
+ if (libbpf_get_error(ifobj->xdp_drop))
+ return libbpf_get_error(ifobj->xdp_drop);
+
return 0;
}
-static void xsk_unload_xdp_program(struct ifobject *ifobj)
+static void xsk_unload_xdp_programs(struct ifobject *ifobj)
{
xsk_def_prog__destroy(ifobj->def_prog);
+ xsk_xdp_drop__destroy(ifobj->xdp_drop);
}
static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
@@ -1692,7 +1727,7 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
if (!load_xdp)
return;
- err = xsk_load_xdp_program(ifobj);
+ err = xsk_load_xdp_programs(ifobj);
if (err) {
printf("Error loading XDP program\n");
exit_with_error(err);
@@ -1804,6 +1839,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_CONSUMES_PACKETS:
+ testapp_xdp_drop(test);
+ break;
default:
break;
}
@@ -1971,8 +2009,8 @@ int main(int argc, char **argv)
pkt_stream_delete(tx_pkt_stream_default);
pkt_stream_delete(rx_pkt_stream_default);
- xsk_unload_xdp_program(ifobj_tx);
- xsk_unload_xdp_program(ifobj_rx);
+ 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 eb6355bcc143..3483ac240b2e 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -6,6 +6,7 @@
#define XSKXCEIVER_H_
#include "xsk_def_prog.skel.h"
+#include "xsk_xdp_drop.skel.h"
#ifndef SOL_XDP
#define SOL_XDP 283
@@ -87,6 +88,7 @@ enum test_type {
TEST_TYPE_STATS_RX_FULL,
TEST_TYPE_STATS_FILL_EMPTY,
TEST_TYPE_BPF_RES,
+ TEST_TYPE_XDP_CONSUMES_PACKETS,
TEST_TYPE_MAX
};
@@ -141,6 +143,7 @@ struct ifobject {
validation_func_t validation_func;
struct pkt_stream *pkt_stream;
struct xsk_def_prog *def_prog;
+ struct xsk_xdp_drop *xdp_drop;
struct bpf_map *xskmap;
int ifindex;
u32 dst_ip;
--
2.34.1
^ permalink raw reply related [flat|nested] 28+ messages in thread* Re: [PATCH bpf-next 12/15] selftests/xsk: add test when some packets are XDP_DROPed
2022-12-06 9:08 ` [PATCH bpf-next 12/15] selftests/xsk: add test when some packets are XDP_DROPed Magnus Karlsson
@ 2022-12-12 15:13 ` Maciej Fijalkowski
2022-12-14 10:53 ` Magnus Karlsson
0 siblings, 1 reply; 28+ messages in thread
From: Maciej Fijalkowski @ 2022-12-12 15:13 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,
jonathan.lemon
On Tue, Dec 06, 2022 at 10:08:23AM +0100, Magnus Karlsson wrote:
> 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>
> ---
> tools/testing/selftests/bpf/Makefile | 2 +-
> .../selftests/bpf/progs/xsk_xdp_drop.c | 25 ++++++++++
> tools/testing/selftests/bpf/xskxceiver.c | 48 +++++++++++++++++--
> tools/testing/selftests/bpf/xskxceiver.h | 3 ++
> 4 files changed, 72 insertions(+), 6 deletions(-)
> create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_drop.c
>
> diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> index 42e15b5a34a7..77ef8a8e6db4 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)/xsk_def_prog.skel.h
> +$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_def_prog.skel.h $(OUTPUT)/xsk_xdp_drop.skel.h
>
> BPFTOOL ?= $(DEFAULT_BPFTOOL)
> $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_drop.c b/tools/testing/selftests/bpf/progs/xsk_xdp_drop.c
> new file mode 100644
> index 000000000000..12a12b0d9fc1
> --- /dev/null
> +++ b/tools/testing/selftests/bpf/progs/xsk_xdp_drop.c
> @@ -0,0 +1,25 @@
> +// 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");
> +
> +static unsigned int idx;
> +
> +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 0cda4e3f1871..522dc1d69c17 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -1654,18 +1654,53 @@ static void testapp_invalid_desc(struct test_spec *test)
> pkt_stream_restore_default(test);
> }
>
> -static int xsk_load_xdp_program(struct ifobject *ifobj)
> +static void testapp_xdp_drop(struct test_spec *test)
> +{
> + struct ifobject *ifobj = test->ifobj_rx;
> + int err;
> +
> + test_spec_set_name(test, "XDP_CONSUMES_SOME_PACKETS");
XDP_DROP_ODD_PACKETS ?
> + xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
> + err = xsk_attach_xdp_program(ifobj->xdp_drop->progs.xsk_xdp_drop, ifobj->ifindex,
> + ifobj->xdp_flags);
> + if (err) {
> + printf("Error attaching XDP_DROP program\n");
> + test->fail = true;
> + return;
> + }
> + ifobj->xskmap = ifobj->xdp_drop->maps.xsk;
> +
> + 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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
> + ifobj->xdp_flags);
> + if (err) {
> + printf("Error restoring default XDP program\n");
> + exit_with_error(-err);
> + }
> + ifobj->xskmap = ifobj->def_prog->maps.xsk;
> +}
> +
> +static int xsk_load_xdp_programs(struct ifobject *ifobj)
> {
> ifobj->def_prog = xsk_def_prog__open_and_load();
> if (libbpf_get_error(ifobj->def_prog))
> return libbpf_get_error(ifobj->def_prog);
>
> + ifobj->xdp_drop = xsk_xdp_drop__open_and_load();
> + if (libbpf_get_error(ifobj->xdp_drop))
> + return libbpf_get_error(ifobj->xdp_drop);
> +
> return 0;
> }
>
> -static void xsk_unload_xdp_program(struct ifobject *ifobj)
> +static void xsk_unload_xdp_programs(struct ifobject *ifobj)
> {
> xsk_def_prog__destroy(ifobj->def_prog);
> + xsk_xdp_drop__destroy(ifobj->xdp_drop);
> }
>
> static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
> @@ -1692,7 +1727,7 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
> if (!load_xdp)
> return;
>
> - err = xsk_load_xdp_program(ifobj);
> + err = xsk_load_xdp_programs(ifobj);
> if (err) {
> printf("Error loading XDP program\n");
> exit_with_error(err);
> @@ -1804,6 +1839,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_CONSUMES_PACKETS:
> + testapp_xdp_drop(test);
> + break;
> default:
> break;
> }
> @@ -1971,8 +2009,8 @@ int main(int argc, char **argv)
>
> pkt_stream_delete(tx_pkt_stream_default);
> pkt_stream_delete(rx_pkt_stream_default);
> - xsk_unload_xdp_program(ifobj_tx);
> - xsk_unload_xdp_program(ifobj_rx);
> + 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 eb6355bcc143..3483ac240b2e 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.h
> +++ b/tools/testing/selftests/bpf/xskxceiver.h
> @@ -6,6 +6,7 @@
> #define XSKXCEIVER_H_
>
> #include "xsk_def_prog.skel.h"
> +#include "xsk_xdp_drop.skel.h"
>
> #ifndef SOL_XDP
> #define SOL_XDP 283
> @@ -87,6 +88,7 @@ enum test_type {
> TEST_TYPE_STATS_RX_FULL,
> TEST_TYPE_STATS_FILL_EMPTY,
> TEST_TYPE_BPF_RES,
> + TEST_TYPE_XDP_CONSUMES_PACKETS,
> TEST_TYPE_MAX
> };
>
> @@ -141,6 +143,7 @@ struct ifobject {
> validation_func_t validation_func;
> struct pkt_stream *pkt_stream;
> struct xsk_def_prog *def_prog;
> + struct xsk_xdp_drop *xdp_drop;
Is this going to scale if we add plenty of XDP progs for testing?
> struct bpf_map *xskmap;
> int ifindex;
> u32 dst_ip;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 28+ messages in thread* Re: [PATCH bpf-next 12/15] selftests/xsk: add test when some packets are XDP_DROPed
2022-12-12 15:13 ` Maciej Fijalkowski
@ 2022-12-14 10:53 ` Magnus Karlsson
0 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-14 10:53 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,
jonathan.lemon
On Mon, Dec 12, 2022 at 4:14 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Tue, Dec 06, 2022 at 10:08:23AM +0100, Magnus Karlsson wrote:
> > 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>
> > ---
> > tools/testing/selftests/bpf/Makefile | 2 +-
> > .../selftests/bpf/progs/xsk_xdp_drop.c | 25 ++++++++++
> > tools/testing/selftests/bpf/xskxceiver.c | 48 +++++++++++++++++--
> > tools/testing/selftests/bpf/xskxceiver.h | 3 ++
> > 4 files changed, 72 insertions(+), 6 deletions(-)
> > create mode 100644 tools/testing/selftests/bpf/progs/xsk_xdp_drop.c
> >
> > diff --git a/tools/testing/selftests/bpf/Makefile b/tools/testing/selftests/bpf/Makefile
> > index 42e15b5a34a7..77ef8a8e6db4 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)/xsk_def_prog.skel.h
> > +$(OUTPUT)/xskxceiver: $(OUTPUT)/xsk.o $(OUTPUT)/xsk_def_prog.skel.h $(OUTPUT)/xsk_xdp_drop.skel.h
> >
> > BPFTOOL ?= $(DEFAULT_BPFTOOL)
> > $(DEFAULT_BPFTOOL): $(wildcard $(BPFTOOLDIR)/*.[ch] $(BPFTOOLDIR)/Makefile) \
> > diff --git a/tools/testing/selftests/bpf/progs/xsk_xdp_drop.c b/tools/testing/selftests/bpf/progs/xsk_xdp_drop.c
> > new file mode 100644
> > index 000000000000..12a12b0d9fc1
> > --- /dev/null
> > +++ b/tools/testing/selftests/bpf/progs/xsk_xdp_drop.c
> > @@ -0,0 +1,25 @@
> > +// 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");
> > +
> > +static unsigned int idx;
> > +
> > +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 0cda4e3f1871..522dc1d69c17 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -1654,18 +1654,53 @@ static void testapp_invalid_desc(struct test_spec *test)
> > pkt_stream_restore_default(test);
> > }
> >
> > -static int xsk_load_xdp_program(struct ifobject *ifobj)
> > +static void testapp_xdp_drop(struct test_spec *test)
> > +{
> > + struct ifobject *ifobj = test->ifobj_rx;
> > + int err;
> > +
> > + test_spec_set_name(test, "XDP_CONSUMES_SOME_PACKETS");
>
> XDP_DROP_ODD_PACKETS ?
Ha! What is an ODD packet?!?! I will think of a better name than these two.
> > + xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
> > + err = xsk_attach_xdp_program(ifobj->xdp_drop->progs.xsk_xdp_drop, ifobj->ifindex,
> > + ifobj->xdp_flags);
> > + if (err) {
> > + printf("Error attaching XDP_DROP program\n");
> > + test->fail = true;
> > + return;
> > + }
> > + ifobj->xskmap = ifobj->xdp_drop->maps.xsk;
> > +
> > + 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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
> > + ifobj->xdp_flags);
> > + if (err) {
> > + printf("Error restoring default XDP program\n");
> > + exit_with_error(-err);
> > + }
> > + ifobj->xskmap = ifobj->def_prog->maps.xsk;
> > +}
> > +
> > +static int xsk_load_xdp_programs(struct ifobject *ifobj)
> > {
> > ifobj->def_prog = xsk_def_prog__open_and_load();
> > if (libbpf_get_error(ifobj->def_prog))
> > return libbpf_get_error(ifobj->def_prog);
> >
> > + ifobj->xdp_drop = xsk_xdp_drop__open_and_load();
> > + if (libbpf_get_error(ifobj->xdp_drop))
> > + return libbpf_get_error(ifobj->xdp_drop);
> > +
> > return 0;
> > }
> >
> > -static void xsk_unload_xdp_program(struct ifobject *ifobj)
> > +static void xsk_unload_xdp_programs(struct ifobject *ifobj)
> > {
> > xsk_def_prog__destroy(ifobj->def_prog);
> > + xsk_xdp_drop__destroy(ifobj->xdp_drop);
> > }
> >
> > static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *src_mac,
> > @@ -1692,7 +1727,7 @@ static void init_iface(struct ifobject *ifobj, const char *dst_mac, const char *
> > if (!load_xdp)
> > return;
> >
> > - err = xsk_load_xdp_program(ifobj);
> > + err = xsk_load_xdp_programs(ifobj);
> > if (err) {
> > printf("Error loading XDP program\n");
> > exit_with_error(err);
> > @@ -1804,6 +1839,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_CONSUMES_PACKETS:
> > + testapp_xdp_drop(test);
> > + break;
> > default:
> > break;
> > }
> > @@ -1971,8 +2009,8 @@ int main(int argc, char **argv)
> >
> > pkt_stream_delete(tx_pkt_stream_default);
> > pkt_stream_delete(rx_pkt_stream_default);
> > - xsk_unload_xdp_program(ifobj_tx);
> > - xsk_unload_xdp_program(ifobj_rx);
> > + 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 eb6355bcc143..3483ac240b2e 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.h
> > +++ b/tools/testing/selftests/bpf/xskxceiver.h
> > @@ -6,6 +6,7 @@
> > #define XSKXCEIVER_H_
> >
> > #include "xsk_def_prog.skel.h"
> > +#include "xsk_xdp_drop.skel.h"
> >
> > #ifndef SOL_XDP
> > #define SOL_XDP 283
> > @@ -87,6 +88,7 @@ enum test_type {
> > TEST_TYPE_STATS_RX_FULL,
> > TEST_TYPE_STATS_FILL_EMPTY,
> > TEST_TYPE_BPF_RES,
> > + TEST_TYPE_XDP_CONSUMES_PACKETS,
> > TEST_TYPE_MAX
> > };
> >
> > @@ -141,6 +143,7 @@ struct ifobject {
> > validation_func_t validation_func;
> > struct pkt_stream *pkt_stream;
> > struct xsk_def_prog *def_prog;
> > + struct xsk_xdp_drop *xdp_drop;
>
> Is this going to scale if we add plenty of XDP progs for testing?
Good point. No. I will put the programs in the same source file and
see if that makes it better.
> > struct bpf_map *xskmap;
> > int ifindex;
> > u32 dst_ip;
> > --
> > 2.34.1
> >
^ permalink raw reply [flat|nested] 28+ messages in thread
* [PATCH bpf-next 13/15] selftests/xsk: merge dual and single thread dispatchers
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (11 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 12/15] selftests/xsk: add test when some packets are XDP_DROPed Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-12 16:10 ` Maciej Fijalkowski
2022-12-06 9:08 ` [PATCH bpf-next 14/15] selftests/xsk: automatically restore packet stream Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 15/15] selftests/xsk: automatically switch XDP programs Magnus Karlsson
14 siblings, 1 reply; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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 522dc1d69c17..0457874c0995 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1364,85 +1364,61 @@ 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 *ifobj_rx,
+ struct ifobject *ifobj_tx)
{
- bool old_shared_umem = ifobj->shared_umem;
- pthread_t t0;
-
- 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);
- 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);
-
- if (type != TEST_TYPE_POLL_TXQ_TMOUT)
- pthread_barrier_wait(&barr);
-
- if (pthread_barrier_destroy(&barr))
- exit_with_error(errno);
-
- pthread_kill(t0, SIGUSR1);
- 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);
- }
-
- 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);
+ if (ifobj_tx)
+ 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;
+ signal(SIGUSR1, handler);
/*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);
+ if (ifobj_tx) {
+ 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);
+ /*Spawn TX thread */
+ pthread_create(&t1, NULL, ifobj_tx->func_ptr, test);
- pthread_join(t1, NULL);
- pthread_join(t0, NULL);
+ pthread_join(t1, NULL);
+ }
+
+ if (!ifobj_tx)
+ pthread_kill(t0, SIGUSR1);
+ else
+ pthread_join(t0, NULL);
if (test->total_steps == test->current_step || test->fail) {
- xsk_socket__delete(ifobj_tx->xsk->xsk);
+ if (ifobj_tx)
+ 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)
+ if (ifobj_tx && !ifobj_tx->shared_umem)
testapp_clean_xsk_umem(ifobj_tx);
}
return !!test->fail;
}
+static int testapp_validate_traffic(struct test_spec *test)
+{
+ return __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
+}
+
+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)
{
int i;
@@ -1684,6 +1660,26 @@ static void testapp_xdp_drop(struct test_spec *test)
ifobj->xskmap = ifobj->def_prog->maps.xsk;
}
+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->def_prog = xsk_def_prog__open_and_load();
@@ -1799,18 +1795,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] 28+ messages in thread* Re: [PATCH bpf-next 13/15] selftests/xsk: merge dual and single thread dispatchers
2022-12-06 9:08 ` [PATCH bpf-next 13/15] selftests/xsk: merge dual and single thread dispatchers Magnus Karlsson
@ 2022-12-12 16:10 ` Maciej Fijalkowski
2022-12-14 10:53 ` Magnus Karlsson
0 siblings, 1 reply; 28+ messages in thread
From: Maciej Fijalkowski @ 2022-12-12 16:10 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,
jonathan.lemon
On Tue, Dec 06, 2022 at 10:08:24AM +0100, Magnus Karlsson wrote:
> 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 522dc1d69c17..0457874c0995 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -1364,85 +1364,61 @@ 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 *ifobj_rx,
> + struct ifobject *ifobj_tx)
> {
> - bool old_shared_umem = ifobj->shared_umem;
> - pthread_t t0;
> -
> - 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);
> - 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);
> -
> - if (type != TEST_TYPE_POLL_TXQ_TMOUT)
> - pthread_barrier_wait(&barr);
> -
> - if (pthread_barrier_destroy(&barr))
> - exit_with_error(errno);
> -
> - pthread_kill(t0, SIGUSR1);
> - 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);
> - }
> -
> - 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);
> + if (ifobj_tx)
> + 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;
>
> + signal(SIGUSR1, handler);
> /*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);
> + if (ifobj_tx) {
> + 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);
> + /*Spawn TX thread */
> + pthread_create(&t1, NULL, ifobj_tx->func_ptr, test);
>
> - pthread_join(t1, NULL);
> - pthread_join(t0, NULL);
> + pthread_join(t1, NULL);
> + }
> +
> + if (!ifobj_tx)
> + pthread_kill(t0, SIGUSR1);
> + else
> + pthread_join(t0, NULL);
>
> if (test->total_steps == test->current_step || test->fail) {
> - xsk_socket__delete(ifobj_tx->xsk->xsk);
> + if (ifobj_tx)
> + 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)
> + if (ifobj_tx && !ifobj_tx->shared_umem)
> testapp_clean_xsk_umem(ifobj_tx);
> }
>
> return !!test->fail;
> }
>
> +static int testapp_validate_traffic(struct test_spec *test)
> +{
> + return __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
> +}
> +
> +static int testapp_validate_traffic_single_thread(struct test_spec *test, struct ifobject *ifobj)
> +{
> + return __testapp_validate_traffic(test, ifobj, NULL);
One minor comment here is that for single thread we either spawn Rx or Tx
thread, whereas reading the code one could tell that single threaded test
works only on Rx.
Maybe rename to ifobj1 ifobj2 from ifobj_rx ifobj_rx? everything within is
generic, like, we have func_ptr, not tx_func_ptr, so this won't look odd
with ifobj1 as a name.
> +}
> +
> static void testapp_teardown(struct test_spec *test)
> {
> int i;
> @@ -1684,6 +1660,26 @@ static void testapp_xdp_drop(struct test_spec *test)
> ifobj->xskmap = ifobj->def_prog->maps.xsk;
> }
>
> +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->def_prog = xsk_def_prog__open_and_load();
> @@ -1799,18 +1795,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 [flat|nested] 28+ messages in thread* Re: [PATCH bpf-next 13/15] selftests/xsk: merge dual and single thread dispatchers
2022-12-12 16:10 ` Maciej Fijalkowski
@ 2022-12-14 10:53 ` Magnus Karlsson
0 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-14 10:53 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,
jonathan.lemon
On Mon, Dec 12, 2022 at 5:12 PM Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> On Tue, Dec 06, 2022 at 10:08:24AM +0100, Magnus Karlsson wrote:
> > 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 522dc1d69c17..0457874c0995 100644
> > --- a/tools/testing/selftests/bpf/xskxceiver.c
> > +++ b/tools/testing/selftests/bpf/xskxceiver.c
> > @@ -1364,85 +1364,61 @@ 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 *ifobj_rx,
> > + struct ifobject *ifobj_tx)
> > {
> > - bool old_shared_umem = ifobj->shared_umem;
> > - pthread_t t0;
> > -
> > - 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);
> > - 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);
> > -
> > - if (type != TEST_TYPE_POLL_TXQ_TMOUT)
> > - pthread_barrier_wait(&barr);
> > -
> > - if (pthread_barrier_destroy(&barr))
> > - exit_with_error(errno);
> > -
> > - pthread_kill(t0, SIGUSR1);
> > - 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);
> > - }
> > -
> > - 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);
> > + if (ifobj_tx)
> > + 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;
> >
> > + signal(SIGUSR1, handler);
> > /*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);
> > + if (ifobj_tx) {
> > + 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);
> > + /*Spawn TX thread */
> > + pthread_create(&t1, NULL, ifobj_tx->func_ptr, test);
> >
> > - pthread_join(t1, NULL);
> > - pthread_join(t0, NULL);
> > + pthread_join(t1, NULL);
> > + }
> > +
> > + if (!ifobj_tx)
> > + pthread_kill(t0, SIGUSR1);
> > + else
> > + pthread_join(t0, NULL);
> >
> > if (test->total_steps == test->current_step || test->fail) {
> > - xsk_socket__delete(ifobj_tx->xsk->xsk);
> > + if (ifobj_tx)
> > + 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)
> > + if (ifobj_tx && !ifobj_tx->shared_umem)
> > testapp_clean_xsk_umem(ifobj_tx);
> > }
> >
> > return !!test->fail;
> > }
> >
> > +static int testapp_validate_traffic(struct test_spec *test)
> > +{
> > + return __testapp_validate_traffic(test, test->ifobj_rx, test->ifobj_tx);
> > +}
> > +
> > +static int testapp_validate_traffic_single_thread(struct test_spec *test, struct ifobject *ifobj)
> > +{
> > + return __testapp_validate_traffic(test, ifobj, NULL);
>
> One minor comment here is that for single thread we either spawn Rx or Tx
> thread, whereas reading the code one could tell that single threaded test
> works only on Rx.
>
> Maybe rename to ifobj1 ifobj2 from ifobj_rx ifobj_rx? everything within is
> generic, like, we have func_ptr, not tx_func_ptr, so this won't look odd
> with ifobj1 as a name.
Makes sense. Will do this.
> > +}
> > +
> > static void testapp_teardown(struct test_spec *test)
> > {
> > int i;
> > @@ -1684,6 +1660,26 @@ static void testapp_xdp_drop(struct test_spec *test)
> > ifobj->xskmap = ifobj->def_prog->maps.xsk;
> > }
> >
> > +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->def_prog = xsk_def_prog__open_and_load();
> > @@ -1799,18 +1795,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 [flat|nested] 28+ messages in thread
* [PATCH bpf-next 14/15] selftests/xsk: automatically restore packet stream
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (12 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 13/15] selftests/xsk: merge dual and single thread dispatchers Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-06 9:08 ` [PATCH bpf-next 15/15] selftests/xsk: automatically switch XDP programs Magnus Karlsson
14 siblings, 0 replies; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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 0457874c0995..26cd64d4209f 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -1509,8 +1509,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)
@@ -1526,8 +1524,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)
@@ -1542,8 +1538,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 */
@@ -1576,7 +1570,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;
}
@@ -1586,7 +1579,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)
@@ -1627,7 +1619,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)
@@ -1649,7 +1640,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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
ifobj->xdp_flags);
@@ -1669,8 +1659,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)
@@ -1781,8 +1769,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;
@@ -1837,6 +1823,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] 28+ messages in thread* [PATCH bpf-next 15/15] selftests/xsk: automatically switch XDP programs
2022-12-06 9:08 [PATCH bpf-next 00/15] selftests/xsk: speed-ups, fixes, and new XDP programs Magnus Karlsson
` (13 preceding siblings ...)
2022-12-06 9:08 ` [PATCH bpf-next 14/15] selftests/xsk: automatically restore packet stream Magnus Karlsson
@ 2022-12-06 9:08 ` Magnus Karlsson
2022-12-13 13:33 ` Maciej Fijalkowski
14 siblings, 1 reply; 28+ messages in thread
From: Magnus Karlsson @ 2022-12-06 9:08 UTC (permalink / raw)
To: magnus.karlsson, bjorn, ast, daniel, netdev, maciej.fijalkowski,
bpf, yhs, andrii, martin.lau, song, john.fastabend, kpsingh, sdf,
haoluo, jolsa
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 | 150 ++++++++++++-----------
tools/testing/selftests/bpf/xskxceiver.h | 7 +-
4 files changed, 102 insertions(+), 70 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 26cd64d4209f..ae9370f7145e 100644
--- a/tools/testing/selftests/bpf/xskxceiver.c
+++ b/tools/testing/selftests/bpf/xskxceiver.c
@@ -96,6 +96,9 @@
#include <time.h>
#include <unistd.h>
#include <stdatomic.h>
+
+#include "xsk_def_prog.skel.h"
+#include "xsk_xdp_drop.skel.h"
#include "xsk.h"
#include "xskxceiver.h"
#include <bpf/bpf.h>
@@ -362,7 +365,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;
@@ -501,6 +503,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->def_prog->progs.xsk_def_prog;
+ test->xskmap_rx = ifobj_rx->def_prog->maps.xsk;
+ test->xdp_prog_tx = ifobj_tx->def_prog->progs.xsk_def_prog;
+ test->xskmap_tx = ifobj_tx->def_prog->maps.xsk;
}
static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
@@ -540,6 +546,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)
@@ -1364,6 +1380,57 @@ 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_attach_xdp_progs(struct test_spec *test, struct ifobject *ifobj_rx,
+ struct ifobject *ifobj_tx)
+{
+ int err;
+
+ if (xdp_prog_changed(test, ifobj_rx)) {
+ xsk_detach_xdp_program(ifobj_rx->ifindex, mode_to_xdp_flags(ifobj_rx->mode));
+ err = xsk_attach_xdp_program(test->xdp_prog_rx, ifobj_rx->ifindex,
+ mode_to_xdp_flags(test->mode));
+ if (err) {
+ printf("Error attaching XDP program\n");
+ exit_with_error(-err);
+ }
+
+ if (ifobj_rx->mode != test->mode && test->mode == TEST_MODE_DRV)
+ if (!xsk_is_in_drv_mode(ifobj_rx->ifindex)) {
+ ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
+ exit_with_error(EINVAL);
+ }
+
+ ifobj_rx->xdp_prog = test->xdp_prog_rx;
+ ifobj_rx->xskmap = test->xskmap_rx;
+ ifobj_rx->mode = test->mode;
+ }
+
+ if (ifobj_tx && !ifobj_tx->shared_umem && xdp_prog_changed(test, ifobj_tx)) {
+ xsk_detach_xdp_program(ifobj_tx->ifindex, mode_to_xdp_flags(ifobj_tx->mode));
+ err = xsk_attach_xdp_program(test->xdp_prog_tx, ifobj_tx->ifindex,
+ mode_to_xdp_flags(test->mode));
+ if (err) {
+ printf("Error attaching XDP program\n");
+ exit_with_error(-err);
+ }
+
+ if (ifobj_rx->mode != test->mode && test->mode == TEST_MODE_DRV)
+ if (!xsk_is_in_drv_mode(ifobj_tx->ifindex)) {
+ ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
+ exit_with_error(EINVAL);
+ }
+
+ ifobj_tx->xdp_prog = test->xdp_prog_tx;
+ ifobj_tx->xskmap = test->xskmap_tx;
+ ifobj_tx->mode = test->mode;
+ }
+}
+
static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *ifobj_rx,
struct ifobject *ifobj_tx)
{
@@ -1411,7 +1478,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)
@@ -1454,7 +1525,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);
}
@@ -1623,31 +1694,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_drop *skel_rx = test->ifobj_rx->xdp_drop;
+ struct xsk_xdp_drop *skel_tx = test->ifobj_tx->xdp_drop;
test_spec_set_name(test, "XDP_CONSUMES_SOME_PACKETS");
- xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
- err = xsk_attach_xdp_program(ifobj->xdp_drop->progs.xsk_xdp_drop, ifobj->ifindex,
- ifobj->xdp_flags);
- if (err) {
- printf("Error attaching XDP_DROP program\n");
- test->fail = true;
- return;
- }
- ifobj->xskmap = ifobj->xdp_drop->maps.xsk;
+ 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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
- ifobj->xdp_flags);
- if (err) {
- printf("Error restoring default XDP program\n");
- exit_with_error(-err);
- }
- ifobj->xskmap = ifobj->def_prog->maps.xsk;
}
static void testapp_poll_txq_tmout(struct test_spec *test)
@@ -1689,7 +1744,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;
@@ -1708,23 +1763,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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
- ifobj->xdp_flags);
- if (err) {
- printf("Error attaching XDP program\n");
- exit_with_error(-err);
- }
- ifobj->xskmap = ifobj->def_prog->maps.xsk;
}
static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
@@ -1886,31 +1929,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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
- ifobj->xdp_flags);
- if (ret) {
- printf("Error attaching XDP program\n");
- exit_with_error(-ret);
- }
- ifobj->xskmap = ifobj->def_prog->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: [%s] XDP prog not in DRV mode\n");
- exit_with_error(EINVAL);
- }
-}
-
int main(int argc, char **argv)
{
struct pkt_stream *rx_pkt_stream_default;
@@ -1951,9 +1969,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);
@@ -1966,12 +1984,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 3483ac240b2e..5c66908577ef 100644
--- a/tools/testing/selftests/bpf/xskxceiver.h
+++ b/tools/testing/selftests/bpf/xskxceiver.h
@@ -145,10 +145,11 @@ struct ifobject {
struct xsk_def_prog *def_prog;
struct xsk_xdp_drop *xdp_drop;
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;
@@ -168,6 +169,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] 28+ messages in thread* Re: [PATCH bpf-next 15/15] selftests/xsk: automatically switch XDP programs
2022-12-06 9:08 ` [PATCH bpf-next 15/15] selftests/xsk: automatically switch XDP programs Magnus Karlsson
@ 2022-12-13 13:33 ` Maciej Fijalkowski
0 siblings, 0 replies; 28+ messages in thread
From: Maciej Fijalkowski @ 2022-12-13 13:33 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,
jonathan.lemon
On Tue, Dec 06, 2022 at 10:08:26AM +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 | 150 ++++++++++++-----------
> tools/testing/selftests/bpf/xskxceiver.h | 7 +-
> 4 files changed, 102 insertions(+), 70 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)
any reason why this can't live in xskxceiver currently?
> +{
> + 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 26cd64d4209f..ae9370f7145e 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.c
> +++ b/tools/testing/selftests/bpf/xskxceiver.c
> @@ -96,6 +96,9 @@
> #include <time.h>
> #include <unistd.h>
> #include <stdatomic.h>
> +
> +#include "xsk_def_prog.skel.h"
> +#include "xsk_xdp_drop.skel.h"
> #include "xsk.h"
> #include "xskxceiver.h"
> #include <bpf/bpf.h>
> @@ -362,7 +365,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;
> @@ -501,6 +503,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->def_prog->progs.xsk_def_prog;
> + test->xskmap_rx = ifobj_rx->def_prog->maps.xsk;
> + test->xdp_prog_tx = ifobj_tx->def_prog->progs.xsk_def_prog;
> + test->xskmap_tx = ifobj_tx->def_prog->maps.xsk;
> }
>
> static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> @@ -540,6 +546,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)
> @@ -1364,6 +1380,57 @@ 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_attach_xdp_progs(struct test_spec *test, struct ifobject *ifobj_rx,
> + struct ifobject *ifobj_tx)
> +{
> + int err;
> +
> + if (xdp_prog_changed(test, ifobj_rx)) {
> + xsk_detach_xdp_program(ifobj_rx->ifindex, mode_to_xdp_flags(ifobj_rx->mode));
> + err = xsk_attach_xdp_program(test->xdp_prog_rx, ifobj_rx->ifindex,
> + mode_to_xdp_flags(test->mode));
> + if (err) {
> + printf("Error attaching XDP program\n");
> + exit_with_error(-err);
> + }
> +
> + if (ifobj_rx->mode != test->mode && test->mode == TEST_MODE_DRV)
This strictly implies that TEST_MODE_SKB is followed by TEST_MODE_DRV,
correct? What if I would swap the order of the tests?
IOW why is it specific to TEST_MODE_DRV? I know that current code also
implies that, but maybe making it generic would make it easier to for
example run a standalone test later on?
> + if (!xsk_is_in_drv_mode(ifobj_rx->ifindex)) {
> + ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
> + exit_with_error(EINVAL);
> + }
> +
> + ifobj_rx->xdp_prog = test->xdp_prog_rx;
> + ifobj_rx->xskmap = test->xskmap_rx;
> + ifobj_rx->mode = test->mode;
> + }
> +
> + if (ifobj_tx && !ifobj_tx->shared_umem && xdp_prog_changed(test, ifobj_tx)) {
> + xsk_detach_xdp_program(ifobj_tx->ifindex, mode_to_xdp_flags(ifobj_tx->mode));
> + err = xsk_attach_xdp_program(test->xdp_prog_tx, ifobj_tx->ifindex,
> + mode_to_xdp_flags(test->mode));
> + if (err) {
> + printf("Error attaching XDP program\n");
> + exit_with_error(-err);
> + }
> +
> + if (ifobj_rx->mode != test->mode && test->mode == TEST_MODE_DRV)
> + if (!xsk_is_in_drv_mode(ifobj_tx->ifindex)) {
> + ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
> + exit_with_error(EINVAL);
> + }
> +
> + ifobj_tx->xdp_prog = test->xdp_prog_tx;
> + ifobj_tx->xskmap = test->xskmap_tx;
> + ifobj_tx->mode = test->mode;
> + }
All of the code above is repeated for ifobj_rx. So what if we do this:
/* FIXME: proper args and naming */
static void xsk_reattach_prog(int xyz, int abc)
{
xsk_detach_xdp_program(ifobj_tx->ifindex, mode_to_xdp_flags(ifobj_tx->mode));
err = xsk_attach_xdp_program(test->xdp_prog_tx, ifobj_tx->ifindex,
mode_to_xdp_flags(test->mode));
if (err) {
printf("Error attaching XDP program\n");
exit_with_error(-err);
}
if (ifobj_rx->mode != test->mode && test->mode == TEST_MODE_DRV)
if (!xsk_is_in_drv_mode(ifobj_tx->ifindex)) {
ksft_print_msg("ERROR: XDP prog not in DRV mode\n");
exit_with_error(EINVAL);
}
ifobj_tx->xdp_prog = test->xdp_prog_tx;
ifobj_tx->xskmap = test->xskmap_tx;
ifobj_tx->mode = test->mode;
}
Then in xsk_attach_xdp_progs:
if (xdp_prog_changed(test, ifobj_rx)) {
xsk_reattach_prog(xyz, abc);
if (!ifobj_tx || ifobj_tx->shared_umem)
return;
if (xdp_prog_changed(test, ifobj_tx))
xsk_reattach_prog(xyz, abc);
?
> +}
> +
> static int __testapp_validate_traffic(struct test_spec *test, struct ifobject *ifobj_rx,
> struct ifobject *ifobj_tx)
> {
> @@ -1411,7 +1478,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)
> @@ -1454,7 +1525,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);
> }
> @@ -1623,31 +1694,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_drop *skel_rx = test->ifobj_rx->xdp_drop;
> + struct xsk_xdp_drop *skel_tx = test->ifobj_tx->xdp_drop;
>
> test_spec_set_name(test, "XDP_CONSUMES_SOME_PACKETS");
> - xsk_detach_xdp_program(ifobj->ifindex, ifobj->xdp_flags);
> - err = xsk_attach_xdp_program(ifobj->xdp_drop->progs.xsk_xdp_drop, ifobj->ifindex,
> - ifobj->xdp_flags);
> - if (err) {
> - printf("Error attaching XDP_DROP program\n");
> - test->fail = true;
> - return;
> - }
> - ifobj->xskmap = ifobj->xdp_drop->maps.xsk;
> + 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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
> - ifobj->xdp_flags);
> - if (err) {
> - printf("Error restoring default XDP program\n");
> - exit_with_error(-err);
> - }
> - ifobj->xskmap = ifobj->def_prog->maps.xsk;
> }
>
> static void testapp_poll_txq_tmout(struct test_spec *test)
> @@ -1689,7 +1744,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;
> @@ -1708,23 +1763,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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
> - ifobj->xdp_flags);
> - if (err) {
> - printf("Error attaching XDP program\n");
> - exit_with_error(-err);
> - }
> - ifobj->xskmap = ifobj->def_prog->maps.xsk;
> }
>
> static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_type type)
> @@ -1886,31 +1929,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->def_prog->progs.xsk_def_prog, ifobj->ifindex,
> - ifobj->xdp_flags);
> - if (ret) {
> - printf("Error attaching XDP program\n");
> - exit_with_error(-ret);
> - }
> - ifobj->xskmap = ifobj->def_prog->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: [%s] XDP prog not in DRV mode\n");
> - exit_with_error(EINVAL);
> - }
> -}
> -
> int main(int argc, char **argv)
> {
> struct pkt_stream *rx_pkt_stream_default;
> @@ -1951,9 +1969,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);
> @@ -1966,12 +1984,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 3483ac240b2e..5c66908577ef 100644
> --- a/tools/testing/selftests/bpf/xskxceiver.h
> +++ b/tools/testing/selftests/bpf/xskxceiver.h
> @@ -145,10 +145,11 @@ struct ifobject {
> struct xsk_def_prog *def_prog;
> struct xsk_xdp_drop *xdp_drop;
> 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;
> @@ -168,6 +169,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] 28+ messages in thread