| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAJ8uoz1ENfOwXUdzGnnVqTj2NUzFGAgRCv0CeYev0_PcGT6iSA@mail.gmail.com>
Date: Mon, 13 Jun 2022 13:37:30 +0200
From: Magnus Karlsson <magnus.karlsson@...il.com>
To: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
Cc: bpf <bpf@...r.kernel.org>, Alexei Starovoitov <ast@...nel.org>,
Daniel Borkmann <daniel@...earbox.net>,
Network Development <netdev@...r.kernel.org>,
"Karlsson, Magnus" <magnus.karlsson@...el.com>,
Björn Töpel <bjorn@...nel.org>
Subject: Re: [PATCH bpf-next 10/10] selftests: xsk: add support for zero copy testing
On Fri, Jun 10, 2022 at 5:20 PM Maciej Fijalkowski
<maciej.fijalkowski@...el.com> wrote:
>
> Introduce new mode to xdpxceiver responsible for testing AF_XDP zero
> copy support of driver that serves underlying physical device. When
> setting up test suite, determine whether driver has ZC support or not by
> trying to bind XSK ZC socket to the interface. If it succeeded,
> interpret it as ZC support being in place and do softirq and busy poll
> tests for zero copy mode.
>
> Note that Rx dropped tests are skipped since ZC path is not touching
> rx_dropped stat at all.
Great to be able to test zero-copy drivers. Thank you! Some small
comments below.
> Signed-off-by: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
> ---
> tools/testing/selftests/bpf/xdpxceiver.c | 80 ++++++++++++++++++++++--
> tools/testing/selftests/bpf/xdpxceiver.h | 2 +
> 2 files changed, 77 insertions(+), 5 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index a2aa652d0bb8..beef8d694fa6 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -124,9 +124,20 @@ static void __exit_with_error(int error, const char *file, const char *func, int
> }
>
> #define exit_with_error(error) __exit_with_error(error, __FILE__, __func__, __LINE__)
> -
> -#define mode_string(test) (test)->ifobj_tx->xdp_flags & XDP_FLAGS_SKB_MODE ? "SKB" : "DRV"
> #define busy_poll_string(test) (test)->ifobj_tx->busy_poll ? "BUSY-POLL " : ""
> +static char *mode_string(struct test_spec *test)
> +{
> + switch (test->mode) {
> + case TEST_MODE_SKB:
> + return "SKB";
> + case TEST_MODE_DRV:
> + return "DRV";
> + case TEST_MODE_ZC:
> + return "ZC";
> + default:
> + return "BOGUS";
> + }
> +}
>
> static void report_failure(struct test_spec *test)
> {
> @@ -317,6 +328,53 @@ static int __xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_i
> return xsk_socket__create(&xsk->xsk, ifobject->ifname, 0, umem->umem, rxr, txr, &cfg);
> }
>
> +static bool ifobj_zc_avail(struct ifobject *ifobject)
> +{
> + size_t umem_sz = DEFAULT_UMEM_BUFFERS * XSK_UMEM__DEFAULT_FRAME_SIZE;
> + int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
> + struct xsk_socket_info *xsk;
> + struct xsk_umem_info *umem;
> + bool zc_avail = false;
> + void *bufs;
> + int ret;
> +
> + bufs = mmap(NULL, umem_sz, PROT_READ | PROT_WRITE, mmap_flags, -1, 0);
> + if (bufs == MAP_FAILED)
> + exit_with_error(errno);
> +
> + umem = calloc(1, sizeof(struct xsk_umem_info));
> + if (!umem) {
> + munmap(bufs, umem_sz);
> + exit_with_error(-ENOMEM);
> + }
> + umem->frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
> + ret = xsk_configure_umem(umem, bufs, umem_sz);
> + if (ret)
> + exit_with_error(-ret);
> +
> + xsk = calloc(1, sizeof(struct xsk_socket_info));
> + if (!xsk)
> + goto out;
> + ifobject->xdp_flags = 0;
This is redundant.
> + ifobject->xdp_flags = XDP_FLAGS_UPDATE_IF_NOEXIST;
> + 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;
> + ret = __xsk_configure_socket(xsk, umem, ifobject, false);
> + if (!ret)
> + zc_avail = true;
> +
> + ifobject->xdp_flags = 0;
Why this assignment?
> + xsk_socket__delete(xsk->xsk);
> + free(xsk);
> +out:
> + munmap(umem->buffer, umem_sz);
> + xsk_umem__delete(umem->umem);
> + free(umem);
> + return zc_avail;
> +}
> +
> static struct option long_options[] = {
> {"interface", required_argument, 0, 'i'},
> {"busy-poll", no_argument, 0, 'b'},
> @@ -483,9 +541,14 @@ static void test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> else
> ifobj->xdp_flags |= XDP_FLAGS_DRV_MODE;
>
> - ifobj->bind_flags = XDP_USE_NEED_WAKEUP | XDP_COPY;
> + ifobj->bind_flags = XDP_USE_NEED_WAKEUP;
> + if (mode == TEST_MODE_ZC)
> + ifobj->bind_flags |= XDP_ZEROCOPY;
> + else
> + ifobj->bind_flags |= XDP_COPY;
> }
>
> + test->mode = mode;
> __test_spec_init(test, ifobj_tx, ifobj_rx);
> }
>
> @@ -1543,6 +1606,10 @@ static void run_pkt_test(struct test_spec *test, enum test_mode mode, enum test_
> {
> switch (type) {
> case TEST_TYPE_STATS_RX_DROPPED:
> + if (mode == TEST_MODE_ZC) {
> + ksft_test_result_skip("Can not run RX_DROPPED test for ZC mode\n");
> + return;
> + }
> testapp_stats_rx_dropped(test);
> break;
> case TEST_TYPE_STATS_TX_INVALID_DESCS:
> @@ -1723,8 +1790,11 @@ int main(int argc, char **argv)
> init_iface(ifobj_rx, MAC2, MAC1, IP2, IP1, UDP_PORT2, UDP_PORT1,
> worker_testapp_validate_rx);
>
> - if (is_xdp_supported(ifobj_tx))
> - modes++;
> + if (is_xdp_supported(ifobj_tx)) {
> + modes++;
> + if (ifobj_zc_avail(ifobj_tx))
> + modes++;
> + }
>
> test_spec_init(&test, ifobj_tx, ifobj_rx, 0);
> tx_pkt_stream_default = pkt_stream_generate(ifobj_tx->umem, DEFAULT_PKT_CNT, PKT_SIZE);
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> index 12b792004163..a86331c6b0c5 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.h
> +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> @@ -61,6 +61,7 @@
> enum test_mode {
> TEST_MODE_SKB,
> TEST_MODE_DRV,
> + TEST_MODE_ZC,
> TEST_MODE_MAX
> };
>
> @@ -162,6 +163,7 @@ struct test_spec {
> u16 current_step;
> u16 nb_sockets;
> bool fail;
> + enum test_mode mode;
> char name[MAX_TEST_NAME_SIZE];
> };
>
> --
> 2.27.0
>
Powered by blists - more mailing lists