From: Stanislav Fomichev <sdf@google.com>
To: Martin KaFai Lau <martin.lau@linux.dev>
Cc: ast@kernel.org, daniel@iogearbox.net, andrii@kernel.org,
song@kernel.org, yhs@fb.com, john.fastabend@gmail.com,
kpsingh@kernel.org, haoluo@google.com, jolsa@kernel.org,
David Ahern <dsahern@gmail.com>, Jakub Kicinski <kuba@kernel.org>,
Willem de Bruijn <willemb@google.com>,
Jesper Dangaard Brouer <brouer@redhat.com>,
Anatoly Burakov <anatoly.burakov@intel.com>,
Alexander Lobakin <alexandr.lobakin@intel.com>,
Magnus Karlsson <magnus.karlsson@gmail.com>,
Maryam Tahhan <mtahhan@redhat.com>,
xdp-hints@xdp-project.net, netdev@vger.kernel.org,
bpf@vger.kernel.org
Subject: [xdp-hints] Re: [PATCH bpf-next v5 11/17] selftests/bpf: Verify xdp_metadata xdp->af_xdp path
Date: Thu, 22 Dec 2022 20:06:48 -0800 [thread overview]
Message-ID: <CAKH8qBst6==Rw1mQohjNimf5QZrPJ05d+XLjqyyT1W8fENNz4w@mail.gmail.com> (raw)
In-Reply-To: <18bed458-0128-d434-8b7a-bf676a0ea863@linux.dev>
On Thu, Dec 22, 2022 at 4:40 PM Martin KaFai Lau <martin.lau@linux.dev> wrote:
>
> On 12/20/22 2:20 PM, Stanislav Fomichev wrote:
> > +static int open_xsk(const char *ifname, struct xsk *xsk)
> > +{
> > + int mmap_flags = MAP_PRIVATE | MAP_ANONYMOUS | MAP_NORESERVE;
> > + const struct xsk_socket_config socket_config = {
> > + .rx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
> > + .tx_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
> > + .libbpf_flags = XSK_LIBBPF_FLAGS__INHIBIT_PROG_LOAD,
> > + .xdp_flags = XDP_FLAGS,
> > + .bind_flags = XDP_COPY,
> > + };
> > + const struct xsk_umem_config umem_config = {
> > + .fill_size = XSK_RING_PROD__DEFAULT_NUM_DESCS,
> > + .comp_size = XSK_RING_CONS__DEFAULT_NUM_DESCS,
> > + .frame_size = XSK_UMEM__DEFAULT_FRAME_SIZE,
> > + .flags = XDP_UMEM_UNALIGNED_CHUNK_FLAG,
> > + };
> > + __u32 idx;
> > + u64 addr;
> > + int ret;
> > + int i;
> > +
> > + xsk->umem_area = mmap(NULL, UMEM_SIZE, PROT_READ | PROT_WRITE, mmap_flags, -1, 0);
> > + if (!ASSERT_NEQ(xsk->umem_area, MAP_FAILED, "mmap"))
> > + return -1;
> > +
> > + ret = xsk_umem__create(&xsk->umem,
> > + xsk->umem_area, UMEM_SIZE,
> > + &xsk->fill,
> > + &xsk->comp,
> > + &umem_config);
> > + if (!ASSERT_OK(ret, "xsk_umem__create"))
> > + return ret;
> > +
> > + ret = xsk_socket__create(&xsk->socket, ifname, QUEUE_ID,
> > + xsk->umem,
> > + &xsk->rx,
> > + &xsk->tx,
> > + &socket_config);
> > + if (!ASSERT_OK(ret, "xsk_socket__create"))
> > + return ret;
> > +
> > + /* First half of umem is for TX. This way address matches 1-to-1
> > + * to the completion queue index.
> > + */
> > +
> > + for (i = 0; i < UMEM_NUM / 2; i++) {
> > + addr = i * UMEM_FRAME_SIZE;
> > + printf("%p: tx_desc[%d] -> %lx\n", xsk, i, addr);
>
> Do you still need this verbose printf which is in a loop? Also, how about other
> printf in this test?
In case we'd ever need to debug this test, those printfs shouldn't
hurt, right? Or are you concerned about this test polluting the output
with something like 'test_progs -v -v' ?
> > + }
> > +
> > + /* Second half of umem is for RX. */
> > +
> > + ret = xsk_ring_prod__reserve(&xsk->fill, UMEM_NUM / 2, &idx);
> > + if (!ASSERT_EQ(UMEM_NUM / 2, ret, "xsk_ring_prod__reserve"))
> > + return ret;
> > + if (!ASSERT_EQ(idx, 0, "fill idx != 0"))
> > + return -1;
> > +
> > + for (i = 0; i < UMEM_NUM / 2; i++) {
> > + addr = (UMEM_NUM / 2 + i) * UMEM_FRAME_SIZE;
> > + printf("%p: rx_desc[%d] -> %lx\n", xsk, i, addr);
> > + *xsk_ring_prod__fill_addr(&xsk->fill, i) = addr;
> > + }
> > + xsk_ring_prod__submit(&xsk->fill, ret);
> > +
> > + return 0;
> > +}
> > +
>
> [ ... ]
>
> > +void test_xdp_metadata(void)
> > +{
> > + struct xdp_metadata2 *bpf_obj2 = NULL;
> > + struct xdp_metadata *bpf_obj = NULL;
> > + struct bpf_program *new_prog, *prog;
> > + struct nstoken *tok = NULL;
> > + __u32 queue_id = QUEUE_ID;
> > + struct bpf_map *prog_arr;
> > + struct xsk tx_xsk = {};
> > + struct xsk rx_xsk = {};
> > + __u32 val, key = 0;
> > + int retries = 10;
> > + int rx_ifindex;
> > + int sock_fd;
> > + int ret;
> > +
> > + /* Setup new networking namespace, with a veth pair. */
> > +
> > + SYS("ip netns add xdp_metadata");
> > + tok = open_netns("xdp_metadata");
> > + SYS("ip link add numtxqueues 1 numrxqueues 1 " TX_NAME
> > + " type veth peer " RX_NAME " numtxqueues 1 numrxqueues 1");
> > + SYS("ip link set dev " TX_NAME " address 00:00:00:00:00:01");
> > + SYS("ip link set dev " RX_NAME " address 00:00:00:00:00:02");
> > + SYS("ip link set dev " TX_NAME " up");
> > + SYS("ip link set dev " RX_NAME " up");
> > + SYS("ip addr add " TX_ADDR "/" PREFIX_LEN " dev " TX_NAME);
> > + SYS("ip addr add " RX_ADDR "/" PREFIX_LEN " dev " RX_NAME);
> > +
> > + rx_ifindex = if_nametoindex(RX_NAME);
> > +
> > + /* Setup separate AF_XDP for TX and RX interfaces. */
> > +
> > + ret = open_xsk(TX_NAME, &tx_xsk);
> > + if (!ASSERT_OK(ret, "open_xsk(TX_NAME)"))
> > + goto out;
> > +
> > + ret = open_xsk(RX_NAME, &rx_xsk);
> > + if (!ASSERT_OK(ret, "open_xsk(RX_NAME)"))
> > + goto out;
> > +
> > + bpf_obj = xdp_metadata__open();
> > + if (!ASSERT_OK_PTR(bpf_obj, "open skeleton"))
> > + goto out;
> > +
> > + prog = bpf_object__find_program_by_name(bpf_obj->obj, "rx");
> > + bpf_program__set_ifindex(prog, rx_ifindex);
> > + bpf_program__set_flags(prog, BPF_F_XDP_DEV_BOUND_ONLY);
> > +
> > + if (!ASSERT_OK(xdp_metadata__load(bpf_obj), "load skeleton"))
> > + goto out;
> > +
> > + /* Make sure we can't add dev-bound programs to prog maps. */
> > + prog_arr = bpf_object__find_map_by_name(bpf_obj->obj, "prog_arr");
> > + if (!ASSERT_OK_PTR(prog_arr, "no prog_arr map"))
> > + goto out;
> > +
> > + val = bpf_program__fd(prog);
> > + if (!ASSERT_ERR(bpf_map__update_elem(prog_arr, &key, sizeof(key),
> > + &val, sizeof(val), BPF_ANY),
> > + "update prog_arr"))
> > + goto out;
> > +
> > + /* Attach BPF program to RX interface. */
> > +
> > + ret = bpf_xdp_attach(rx_ifindex,
> > + bpf_program__fd(bpf_obj->progs.rx),
> > + XDP_FLAGS, NULL);
> > + if (!ASSERT_GE(ret, 0, "bpf_xdp_attach"))
> > + goto out;
> > +
> > + sock_fd = xsk_socket__fd(rx_xsk.socket);
> > + ret = bpf_map_update_elem(bpf_map__fd(bpf_obj->maps.xsk), &queue_id, &sock_fd, 0);
> > + if (!ASSERT_GE(ret, 0, "bpf_map_update_elem"))
> > + goto out;
> > +
> > + /* Send packet destined to RX AF_XDP socket. */
> > + if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT), 0,
> > + "generate AF_XDP_CONSUMER_PORT"))
> > + goto out;
> > +
> > + /* Verify AF_XDP RX packet has proper metadata. */
> > + if (!ASSERT_GE(verify_xsk_metadata(&rx_xsk), 0,
> > + "verify_xsk_metadata"))
> > + goto out;
> > +
> > + complete_tx(&tx_xsk);
> > +
> > + /* Make sure freplace correctly picks up original bound device
> > + * and doesn't crash.
> > + */
> > +
> > + bpf_obj2 = xdp_metadata2__open();
> > + if (!ASSERT_OK_PTR(bpf_obj2, "open skeleton"))
> > + goto out;
> > +
> > + new_prog = bpf_object__find_program_by_name(bpf_obj2->obj, "freplace_rx");
> > + bpf_program__set_attach_target(new_prog, bpf_program__fd(prog), "rx");
> > +
> > + if (!ASSERT_OK(xdp_metadata2__load(bpf_obj2), "load freplace skeleton"))
> > + goto out;
> > +
> > + if (!ASSERT_OK(xdp_metadata2__attach(bpf_obj2), "attach freplace"))
> > + goto out;
> > +
> > + /* Send packet to trigger . */
> > + if (!ASSERT_GE(generate_packet(&tx_xsk, AF_XDP_CONSUMER_PORT), 0,
> > + "generate freplace packet"))
> > + goto out;
> > +
> > + while (!retries--) {
> > + if (bpf_obj2->bss->called)
> > + break;
> > + usleep(10);
> > + }
> > + ASSERT_GT(bpf_obj2->bss->called, 0, "not called");
> > +
> > +out:
> > + close_xsk(&rx_xsk);
> > + close_xsk(&tx_xsk);
> > + if (bpf_obj2)
>
> nit. no need to test NULL. xdp_metadata2__destroy() can handle it.
SG, thanks!
> > + xdp_metadata2__destroy(bpf_obj2);
> > + if (bpf_obj)
>
> Same here.
>
> > + xdp_metadata__destroy(bpf_obj);
> > + system("ip netns del xdp_metadata");
>
> didn't know netns can be deleted before close_netns(tok). Can you double check?
Should probably work because 'ip netns del' just unlinks the netns fd?
But let's properly reoder.
> > + if (tok)
> > + close_netns(tok);
> > +}
>
>
next prev parent reply other threads:[~2022-12-23 4:07 UTC|newest]
Thread overview: 34+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-20 22:20 [xdp-hints] [PATCH bpf-next v5 00/17] xdp: hints via kfuncs Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 01/17] bpf: Document XDP RX metadata Stanislav Fomichev
2022-12-28 17:25 ` [xdp-hints] " David Vernet
2023-01-03 22:23 ` Stanislav Fomichev
2023-01-04 16:02 ` David Vernet
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 02/17] bpf: Rename bpf_{prog,map}_is_dev_bound to is_offloaded Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 03/17] bpf: Move offload initialization into late_initcall Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 04/17] bpf: Reshuffle some parts of bpf/offload.c Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 05/17] bpf: Introduce device-bound XDP programs Stanislav Fomichev
2022-12-23 0:19 ` [xdp-hints] " Martin KaFai Lau
2022-12-23 4:06 ` Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 06/17] selftests/bpf: Update expected test_offload.py messages Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 07/17] bpf: XDP metadata RX kfuncs Stanislav Fomichev
2022-12-21 5:43 ` [xdp-hints] " kernel test robot
2022-12-21 17:49 ` Stanislav Fomichev
2022-12-23 0:31 ` Martin KaFai Lau
2022-12-23 4:06 ` Stanislav Fomichev
2022-12-27 20:33 ` David Vernet
2023-01-03 22:23 ` Stanislav Fomichev
2023-01-03 22:35 ` David Vernet
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 09/17] veth: Introduce veth_xdp_buff wrapper for xdp_buff Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 10/17] veth: Support RX XDP metadata Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 11/17] selftests/bpf: Verify xdp_metadata xdp->af_xdp path Stanislav Fomichev
2022-12-23 0:40 ` [xdp-hints] " Martin KaFai Lau
2022-12-23 4:06 ` Stanislav Fomichev [this message]
2023-01-04 2:05 ` Martin KaFai Lau
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 12/17] net/mlx4_en: Introduce wrapper for xdp_buff Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 13/17] net/mlx4_en: Support RX XDP metadata Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 14/17] xsk: Add cb area to struct xdp_buff_xsk Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 15/17] net/mlx5e: Introduce wrapper for xdp_buff Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 16/17] net/mlx5e: Support RX XDP metadata Stanislav Fomichev
2022-12-20 22:20 ` [xdp-hints] [PATCH bpf-next v5 17/17] selftests/bpf: Simple program to dump XDP RX metadata Stanislav Fomichev
2022-12-23 0:53 ` [xdp-hints] " Martin KaFai Lau
2022-12-23 4:07 ` Stanislav Fomichev
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
List information: https://lists.xdp-project.net/postorius/lists/xdp-hints.xdp-project.net/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAKH8qBst6==Rw1mQohjNimf5QZrPJ05d+XLjqyyT1W8fENNz4w@mail.gmail.com' \
--to=sdf@google.com \
--cc=alexandr.lobakin@intel.com \
--cc=anatoly.burakov@intel.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=brouer@redhat.com \
--cc=daniel@iogearbox.net \
--cc=dsahern@gmail.com \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=jolsa@kernel.org \
--cc=kpsingh@kernel.org \
--cc=kuba@kernel.org \
--cc=magnus.karlsson@gmail.com \
--cc=martin.lau@linux.dev \
--cc=mtahhan@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=song@kernel.org \
--cc=willemb@google.com \
--cc=xdp-hints@xdp-project.net \
--cc=yhs@fb.com \
/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