From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: Magnus Karlsson <magnus.karlsson@gmail.com>
Cc: magnus.karlsson@intel.com, bjorn@kernel.org, ast@kernel.org,
daniel@iogearbox.net, netdev@vger.kernel.org,
jonathan.lemon@gmail.com, bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next] selftests: xsk: fix bpf_res cleanup test
Date: Wed, 26 Jan 2022 21:03:41 +0100 [thread overview]
Message-ID: <YfGpHQmfyT69HNAy@boxer> (raw)
In-Reply-To: <20220125082945.26179-1-magnus.karlsson@gmail.com>
On Tue, Jan 25, 2022 at 09:29:45AM +0100, Magnus Karlsson wrote:
> From: Magnus Karlsson <magnus.karlsson@intel.com>
>
> After commit 710ad98c363a ("veth: Do not record rx queue hint in
> veth_xmit"), veth no longer receives traffic on the same queue as it
> was sent on. This breaks the bpf_res test for the AF_XDP selftests as
> the socket tied to queue 1 will not receive traffic anymore. Modify
> the test so that two sockets are tied to queue id 0 using a shared
> umem instead. When killing the first socket enter the second socket
> into the xskmap so that traffic will flow to it. This will still test
> that the resources are not cleaned up until after the second socket
> dies, without having to rely on veth supporting rx_queue hints.
>
> Reported-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> Signed-off-by: Magnus Karlsson <magnus.karlsson@intel.com>
Thanks, makes sense as we discussed this internally. I didn't have a
chance to apologize to the mess I introduced with the stuff that
710ad98c363a fixed, so sorry everyone, I didn't mean that :<
FWIW I think now we can drop the numtxqueues numrxqueues args from
test_xsk.sh that are used when creating veth pair.
Besides that:
Reviewed-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
> ---
> tools/testing/selftests/bpf/xdpxceiver.c | 80 +++++++++++++++---------
> tools/testing/selftests/bpf/xdpxceiver.h | 2 +-
> 2 files changed, 50 insertions(+), 32 deletions(-)
>
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.c b/tools/testing/selftests/bpf/xdpxceiver.c
> index ffa5502ad95e..5f8296d29e77 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.c
> +++ b/tools/testing/selftests/bpf/xdpxceiver.c
> @@ -266,22 +266,24 @@ static int xsk_configure_umem(struct xsk_umem_info *umem, void *buffer, u64 size
> }
>
> static int xsk_configure_socket(struct xsk_socket_info *xsk, struct xsk_umem_info *umem,
> - struct ifobject *ifobject, u32 qid)
> + struct ifobject *ifobject, bool shared)
> {
> - struct xsk_socket_config cfg;
> + struct xsk_socket_config cfg = {};
> struct xsk_ring_cons *rxr;
> struct xsk_ring_prod *txr;
>
> xsk->umem = umem;
> cfg.rx_size = xsk->rxqsize;
> cfg.tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS;
> - cfg.libbpf_flags = 0;
> + 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;
>
> txr = ifobject->tx_on ? &xsk->tx : NULL;
> rxr = ifobject->rx_on ? &xsk->rx : NULL;
> - return xsk_socket__create(&xsk->xsk, ifobject->ifname, qid, umem->umem, rxr, txr, &cfg);
> + return xsk_socket__create(&xsk->xsk, ifobject->ifname, 0, umem->umem, rxr, txr, &cfg);
> }
>
> static struct option long_options[] = {
> @@ -387,7 +389,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->umem = &ifobj->umem_arr[0];
> ifobj->xsk = &ifobj->xsk_arr[0];
> ifobj->use_poll = false;
> ifobj->pacing_on = true;
> @@ -401,11 +402,12 @@ static void __test_spec_init(struct test_spec *test, struct ifobject *ifobj_tx,
> ifobj->tx_on = false;
> }
>
> + memset(ifobj->umem, 0, sizeof(*ifobj->umem));
> + ifobj->umem->num_frames = DEFAULT_UMEM_BUFFERS;
> + ifobj->umem->frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
> +
> for (j = 0; j < MAX_SOCKETS; j++) {
> - memset(&ifobj->umem_arr[j], 0, sizeof(ifobj->umem_arr[j]));
> memset(&ifobj->xsk_arr[j], 0, sizeof(ifobj->xsk_arr[j]));
> - ifobj->umem_arr[j].num_frames = DEFAULT_UMEM_BUFFERS;
> - ifobj->umem_arr[j].frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE;
> ifobj->xsk_arr[j].rxqsize = XSK_RING_CONS__DEFAULT_NUM_DESCS;
> }
> }
> @@ -950,7 +952,10 @@ static void tx_stats_validate(struct ifobject *ifobject)
>
> static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
> {
> + u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
> int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
> + int ret, ifindex;
> + void *bufs;
> u32 i;
>
> ifobject->ns_fd = switch_namespace(ifobject->nsname);
> @@ -958,23 +963,20 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
> if (ifobject->umem->unaligned_mode)
> mmap_flags |= MAP_HUGETLB;
>
> - for (i = 0; i < test->nb_sockets; i++) {
> - u64 umem_sz = ifobject->umem->num_frames * ifobject->umem->frame_size;
> - u32 ctr = 0;
> - 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);
>
> - bufs = mmap(NULL, umem_sz, PROT_READ | PROT_WRITE, mmap_flags, -1, 0);
> - if (bufs == MAP_FAILED)
> - exit_with_error(errno);
> + ret = xsk_configure_umem(ifobject->umem, bufs, umem_sz);
> + if (ret)
> + exit_with_error(-ret);
>
> - ret = xsk_configure_umem(&ifobject->umem_arr[i], bufs, umem_sz);
> - if (ret)
> - exit_with_error(-ret);
> + for (i = 0; i < test->nb_sockets; i++) {
> + u32 ctr = 0;
>
> while (ctr++ < SOCK_RECONF_CTR) {
> - ret = xsk_configure_socket(&ifobject->xsk_arr[i], &ifobject->umem_arr[i],
> - ifobject, i);
> + ret = xsk_configure_socket(&ifobject->xsk_arr[i], ifobject->umem,
> + ifobject, !!i);
> if (!ret)
> break;
>
> @@ -985,8 +987,22 @@ static void thread_common_ops(struct test_spec *test, struct ifobject *ifobject)
> }
> }
>
> - ifobject->umem = &ifobject->umem_arr[0];
> ifobject->xsk = &ifobject->xsk_arr[0];
> +
> + if (!ifobject->rx_on)
> + return;
> +
> + ifindex = if_nametoindex(ifobject->ifname);
> + if (!ifindex)
> + exit_with_error(errno);
> +
> + ret = xsk_setup_xdp_prog(ifindex, &ifobject->xsk_map_fd);
> + if (ret)
> + exit_with_error(-ret);
> +
> + ret = xsk_socket__update_xskmap(ifobject->xsk->xsk, ifobject->xsk_map_fd);
> + if (ret)
> + exit_with_error(-ret);
> }
>
> static void testapp_cleanup_xsk_res(struct ifobject *ifobj)
> @@ -1142,14 +1158,16 @@ static void testapp_bidi(struct test_spec *test)
>
> static void swap_xsk_resources(struct ifobject *ifobj_tx, struct ifobject *ifobj_rx)
> {
> + int ret;
> +
> xsk_socket__delete(ifobj_tx->xsk->xsk);
> - xsk_umem__delete(ifobj_tx->umem->umem);
> xsk_socket__delete(ifobj_rx->xsk->xsk);
> - xsk_umem__delete(ifobj_rx->umem->umem);
> - ifobj_tx->umem = &ifobj_tx->umem_arr[1];
> ifobj_tx->xsk = &ifobj_tx->xsk_arr[1];
> - ifobj_rx->umem = &ifobj_rx->umem_arr[1];
> ifobj_rx->xsk = &ifobj_rx->xsk_arr[1];
> +
> + ret = xsk_socket__update_xskmap(ifobj_rx->xsk->xsk, ifobj_rx->xsk_map_fd);
> + if (ret)
> + exit_with_error(-ret);
> }
>
> static void testapp_bpf_res(struct test_spec *test)
> @@ -1408,13 +1426,13 @@ static struct ifobject *ifobject_create(void)
> if (!ifobj->xsk_arr)
> goto out_xsk_arr;
>
> - ifobj->umem_arr = calloc(MAX_SOCKETS, sizeof(*ifobj->umem_arr));
> - if (!ifobj->umem_arr)
> - goto out_umem_arr;
> + ifobj->umem = calloc(1, sizeof(*ifobj->umem));
> + if (!ifobj->umem)
> + goto out_umem;
>
> return ifobj;
>
> -out_umem_arr:
> +out_umem:
> free(ifobj->xsk_arr);
> out_xsk_arr:
> free(ifobj);
> @@ -1423,7 +1441,7 @@ static struct ifobject *ifobject_create(void)
>
> static void ifobject_delete(struct ifobject *ifobj)
> {
> - free(ifobj->umem_arr);
> + free(ifobj->umem);
> free(ifobj->xsk_arr);
> free(ifobj);
> }
> diff --git a/tools/testing/selftests/bpf/xdpxceiver.h b/tools/testing/selftests/bpf/xdpxceiver.h
> index 2f705f44b748..62a3e6388632 100644
> --- a/tools/testing/selftests/bpf/xdpxceiver.h
> +++ b/tools/testing/selftests/bpf/xdpxceiver.h
> @@ -125,10 +125,10 @@ struct ifobject {
> struct xsk_socket_info *xsk;
> struct xsk_socket_info *xsk_arr;
> struct xsk_umem_info *umem;
> - struct xsk_umem_info *umem_arr;
> thread_func_t func_ptr;
> struct pkt_stream *pkt_stream;
> int ns_fd;
> + int xsk_map_fd;
> u32 dst_ip;
> u32 src_ip;
> u32 xdp_flags;
>
> base-commit: 74bb0f0c299cdc9c68cb3bc8f452e5812aa9eab0
> --
> 2.34.1
>
next prev parent reply other threads:[~2022-01-26 20:03 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-25 8:29 [PATCH bpf-next] selftests: xsk: fix bpf_res cleanup test Magnus Karlsson
2022-01-25 23:01 ` John Fastabend
2022-01-26 20:03 ` Maciej Fijalkowski [this message]
2022-01-27 16:50 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=YfGpHQmfyT69HNAy@boxer \
--to=maciej.fijalkowski@intel.com \
--cc=ast@kernel.org \
--cc=bjorn@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=jonathan.lemon@gmail.com \
--cc=magnus.karlsson@gmail.com \
--cc=magnus.karlsson@intel.com \
--cc=netdev@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox