Re: [PATCH bpf-next v5 11/17] selftests/bpf: Verify xdp_metadata xdp->af_xdp path

Martin KaFai Lau
2022-12-23T00:40:13Z

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?

> + }
> +
> + /* 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.

> + 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?

> + if (tok)
> + close_netns(tok);
> +}