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 for Android: free password hash cracker in your pocket

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

Date:   Thu, 09 Dec 2021 17:10:44 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     John Fastabend <john.fastabend@...il.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:     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()

John Fastabend <john.fastabend@...il.com> writes:

> 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.

Sure, it could. But the data buffer comes from userspace anyway, and
there's nothing preventing userspace from passing a 0-length packet
anyway, so I just mentally put this in the "don't do that, then" bucket :)

At least I don't *think* there's actually any problem with this that we
don't have already? A regular XDP program can also shrink an incoming
packet to zero, then redirect it, no?

-Toke