netdev - Re: [PATCH v3 net-next RFC] Generic XDP

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: <20170414192814.GD41922@ast-mbp.thefacebook.com>
Date:   Fri, 14 Apr 2017 12:28:15 -0700
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Jesper Dangaard Brouer <brouer@...hat.com>
Cc:     David Miller <davem@...emloft.net>, netdev@...r.kernel.org,
        xdp-newbies@...r.kernel.org
Subject: Re: [PATCH v3 net-next RFC] Generic XDP

On Fri, Apr 14, 2017 at 11:05:25AM +0200, Jesper Dangaard Brouer wrote:
> > 
> > We are consistently finding that there is this real need to
> > communicate XDP capabilities, or somehow verify that the needs
> > of an XDP program can be satisfied by a given implementation.
> 
> I fully agree that we need some way to express capabilities[1]
> 
> > Maximum headroom is just one.

I don't like the idea of asking program author to explain capabilities
to the kernel. Right now the verifier already understands more about
the program than human does. If the verifier cannot deduct from the
insns what program will be doing, it's pretty much guarantee
that program author has no idea either.
If we add 'required_headroom' as an extra flag to BPF_PROG_LOAD,
the users will just pass something like 64 or 128 whereas the program
might only be doing IPIP encap and that will cause kernel to
provide extra headroom for no good reason or reject the program
whereas it could have run just fine.

So I very much agree with this part:
> ... or somehow verify that the needs
> of an XDP program can be satisfied by a given implementation.

we already have cb_access, dst_needed, xdp_adjust_head flags
that verifier discovers in the program.
For headroom we need one more. The verifier can see
the constant passed into bpf_xdp_adjust_head().
It's trickier if it's a variable, but the verifier can estimate min/max
of the variable already and worst case it can say that it
will be XDP_PACKET_HEADROOM.
If the program is doing variable length bpf_xdp_adjust_head(),
the human has no idea how much they need and cannot tell kernel anyway.