netdev - Re: [RFC net-next 11/14] tun: run XDP program in tx path

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]

Date:   Wed, 18 Dec 2019 12:48:59 +0100
From:   Toke Høiland-Jørgensen <toke@...hat.com>
To:     Jesper Dangaard Brouer <jbrouer@...hat.com>,
        Prashant Bhole <prashantbhole.linux@...il.com>
Cc:     "David S . Miller" <davem@...emloft.net>,
        "Michael S . Tsirkin" <mst@...hat.com>,
        Alexei Starovoitov <ast@...nel.org>,
        Daniel Borkmann <daniel@...earbox.net>,
        Jesper Dangaard Brouer <hawk@...nel.org>,
        Jason Wang <jasowang@...hat.com>,
        David Ahern <dsahern@...il.com>,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        John Fastabend <john.fastabend@...il.com>,
        Toshiaki Makita <toshiaki.makita1@...il.com>,
        Martin KaFai Lau <kafai@...com>,
        Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>,
        Andrii Nakryiko <andriin@...com>, netdev@...r.kernel.org,
        Ilias Apalodimas <ilias.apalodimas@...aro.org>
Subject: Re: [RFC net-next 11/14] tun: run XDP program in tx path

Jesper Dangaard Brouer <jbrouer@...hat.com> writes:

> On Wed, 18 Dec 2019 17:10:47 +0900
> Prashant Bhole <prashantbhole.linux@...il.com> wrote:
>
>> +static u32 tun_do_xdp_tx(struct tun_struct *tun, struct tun_file *tfile,
>> +			 struct xdp_frame *frame)
>> +{
>> +	struct bpf_prog *xdp_prog;
>> +	struct tun_page tpage;
>> +	struct xdp_buff xdp;
>> +	u32 act = XDP_PASS;
>> +	int flush = 0;
>> +
>> +	xdp_prog = rcu_dereference(tun->xdp_tx_prog);
>> +	if (xdp_prog) {
>> +		xdp.data_hard_start = frame->data - frame->headroom;
>> +		xdp.data = frame->data;
>> +		xdp.data_end = xdp.data + frame->len;
>> +		xdp.data_meta = xdp.data - frame->metasize;
>
> You have not configured xdp.rxq, thus a BPF-prog accessing this will crash.
>
> For an XDP TX hook, I want us to provide/give BPF-prog access to some
> more information about e.g. the current tx-queue length, or TC-q number.
>
> Question to Daniel or Alexei, can we do this and still keep BPF_PROG_TYPE_XDP?
> Or is it better to introduce a new BPF prog type (enum bpf_prog_type)
> for XDP TX-hook ?

I think a new program type would make the most sense. If/when we
introduce an XDP TX hook[0], it should have different semantics than the
regular XDP hook. I view the XDP TX hook as a hook that executes as the
very last thing before packets leave the interface. It should have
access to different context data as you say, but also I don't think it
makes sense to have XDP_TX and XDP_REDIRECT in an XDP_TX hook. And we
may also want to have a "throttle" return code; or maybe that could be
done via a helper?

In any case, I don't think this "emulated RX hook on the other end of a
virtual device" model that this series introduces is the right semantics
for an XDP TX hook. I can see what you're trying to do, and for virtual
point-to-point links I think it may make sense to emulate the RX hook of
the "other end" on TX. However, form a UAPI perspective, I don't think
we should be calling this a TX hook; logically, it's still an RX hook
on the receive end.

If you guys are up for evolving this design into a "proper" TX hook (as
outlined above an in [0]), that would be awesome, of course. But not
sure what constraints you have on your original problem? Do you
specifically need the "emulated RX hook for unmodified XDP programs"
semantics, or could your problem be solved with a TX hook with different
semantics?

-Toke

[0] We've suggested this in the past, see
https://github.com/xdp-project/xdp-project/blob/master/xdp-project.org#xdp-hook-at-tx