netdev - Re: [RFC PATCH 1/5] bpf: add PHYS_DEV prog type for early driver filter

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:	Tue, 5 Apr 2016 00:04:39 +0200
From:	Thomas Graf <tgraf@...g.ch>
To:	Alexei Starovoitov <alexei.starovoitov@...il.com>
Cc:	Brenden Blanco <bblanco@...mgrid.com>,
	John Fastabend <john.fastabend@...il.com>,
	Jesper Dangaard Brouer <brouer@...hat.com>,
	Tom Herbert <tom@...bertland.com>,
	Daniel Borkmann <daniel@...earbox.net>,
	"David S. Miller" <davem@...emloft.net>,
	Linux Kernel Network Developers <netdev@...r.kernel.org>,
	ogerlitz@...lanox.com
Subject: Re: [RFC PATCH 1/5] bpf: add PHYS_DEV prog type for early driver
 filter

On 04/04/16 at 01:00pm, Alexei Starovoitov wrote:
> Exactly. That the most important part of this rfc.
> Right now redirect to different queue, batching, prefetch and tons of
> other code are mising. We have to plan the whole project, so we can
> incrementally add features without breaking abi.
> So new IFLA, xdp_metadata struct and enum for bpf return codes are
> the main things to agree on.

+1
This is the most important statement in this thread so far. A plan
that gets us from this RFC series to a functional forwarding engine
with redirect and load/write is essential. [...]

> Another reason for going with 'pseudo skb' structure was to reuse
> load_byte/half/word instructions in bpf interpreter as-is.
> Right now these instructions have to see in-kernel
> 'struct sk_buff' as context (note we have mirror __sk_buff
> for user space), so to use load_byte for bpf_prog_type_phys_dev
> we have to give real 'struct sk_buff' to interpter with
> data, head, len, data_len fields initialized, so that
> interpreter 'just works'.
> The potential fix would be to add phys_dev specific load_byte/word
> instructions. Then we can drop all the legacy negative offset
> stuff that <1% uses, but it slows down everyone.
> We can also drop byteswap that load_word does (which turned out
> to be confusing and often programs do 2nd byteswap to go
> back to cpu endiannes).

[...] I would really like to see a common set of helpers which
applies to both cls_bpf and phys_dev. Given the existing skb based
helpers cannot be overloaded, at least the phys_dev helpers should
be made to work in cls_bpf context as well to allow for some
portability. Otherwise we'll end up with half a dozen flavours of
BPF which are all incompatible.

> And if we do it smart, we can drop length check as well,
> then new_load_byte will actually be single load byte cpu instruction.
> We can drop length check when offset is constant in the verfier
> and that constant is less than 64, since all packets are larger.
> As seen in 'perf report' from patch 5:
>   3.32%  ksoftirqd/1    [kernel.vmlinux]  [k] sk_load_byte_positive_offset
> this is 14Mpps and 4 assembler instructions in the above function
> are consuming 3% of the cpu. Making new_load_byte to be single
> x86 insn would be really cool.

Neat.