netdev - RE: [PATCH bpf-next 6/8] bpf: Add XDP_REDIRECT support to XDP for bpf_prog

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <61b1537634e07_979572086f@john.notmuch>
Date:   Wed, 08 Dec 2021 16:53:10 -0800
From:   John Fastabend <john.fastabend@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Andrii Nakryiko <andrii@...nel.org>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        John Fastabend <john.fastabend@...il.com>,
        KP Singh <kpsingh@...nel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jesper Dangaard Brouer <hawk@...nel.org>
Cc:     Toke Høiland-Jørgensen <toke@...hat.com>,
        netdev@...r.kernel.org, bpf@...r.kernel.org
Subject: RE: [PATCH bpf-next 6/8] bpf: Add XDP_REDIRECT support to XDP for
 bpf_prog_run()

Toke Høiland-Jørgensen wrote:
> This adds support for doing real redirects when an XDP program returns
> XDP_REDIRECT in bpf_prog_run(). To achieve this, we create a page pool
> instance while setting up the test run, and feed pages from that into the
> XDP program. The setup cost of this is amortised over the number of
> repetitions specified by userspace.
> 
> To support performance testing use case, we further optimise the setup step
> so that all pages in the pool are pre-initialised with the packet data, and
> pre-computed context and xdp_frame objects stored at the start of each
> page. This makes it possible to entirely avoid touching the page content on
> each XDP program invocation, and enables sending up to 11.5 Mpps/core on my
> test box.
> 
> Because the data pages are recycled by the page pool, and the test runner
> doesn't re-initialise them for each run, subsequent invocations of the XDP
> program will see the packet data in the state it was after the last time it
> ran on that particular page. This means that an XDP program that modifies
> the packet before redirecting it has to be careful about which assumptions
> it makes about the packet content, but that is only an issue for the most
> naively written programs.
> 
> Previous uses of bpf_prog_run() for XDP returned the modified packet data
> and return code to userspace, which is a different semantic then this new
> redirect mode. For this reason, the caller has to set the new
> BPF_F_TEST_XDP_DO_REDIRECT flag when calling bpf_prog_run() to opt in to
> the different semantics. Enabling this flag is only allowed if not setting
> ctx_out and data_out in the test specification, since it means frames will
> be redirected somewhere else, so they can't be returned.
> 
> Signed-off-by: Toke Høiland-Jørgensen <toke@...hat.com>
> ---

[...]

> +static int bpf_test_run_xdp_redirect(struct bpf_test_timer *t,
> +				     struct bpf_prog *prog, struct xdp_buff *orig_ctx)
> +{
> +	void *data, *data_end, *data_meta;
> +	struct xdp_frame *frm;
> +	struct xdp_buff *ctx;
> +	struct page *page;
> +	int ret, err = 0;
> +
> +	page = page_pool_dev_alloc_pages(t->xdp.pp);
> +	if (!page)
> +		return -ENOMEM;
> +
> +	ctx = ctx_from_page(page);
> +	data = ctx->data;
> +	data_meta = ctx->data_meta;
> +	data_end = ctx->data_end;
> +
> +	ret = bpf_prog_run_xdp(prog, ctx);
> +	if (ret == XDP_REDIRECT) {
> +		frm = (struct xdp_frame *)(ctx + 1);
> +		/* if program changed pkt bounds we need to update the xdp_frame */

Because this reuses the frame repeatedly is there any issue with also
updating the ctx each time? Perhaps if the prog keeps shrinking
the pkt it might wind up with 0 len pkt? Just wanted to ask.

> +		if (unlikely(data != ctx->data ||
> +			     data_meta != ctx->data_meta ||
> +			     data_end != ctx->data_end))
> +			xdp_update_frame_from_buff(ctx, frm);
> +
> +		err = xdp_do_redirect_frame(ctx->rxq->dev, ctx, frm, prog);
> +		if (err)
> +			ret = err;
> +	}
> +	if (ret != XDP_REDIRECT)
> +		xdp_return_buff(ctx);
> +
> +	if (++t->xdp.frame_cnt >= NAPI_POLL_WEIGHT) {
> +		xdp_do_flush();
> +		t->xdp.frame_cnt = 0;
> +	}
> +
> +	return ret;
> +}
> +