netdev - Re: [PATCH net v3] net: xdp: account for layer 3 packets in generic skb handler

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: <20200428095113.330e83aa@kicinski-fedora-pc1c0hjn.dhcp.thefacebook.com>
Date:   Tue, 28 Apr 2020 09:51:13 -0700
From:   Jakub Kicinski <kuba@...nel.org>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     "Jason A. Donenfeld" <Jason@...c4.com>, netdev@...r.kernel.org,
        Adhipati Blambangan <adhipati@...a.io>,
        David Ahern <dsahern@...il.com>
Subject: Re: [PATCH net v3] net: xdp: account for layer 3 packets in generic
 skb handler

On Tue, 28 Apr 2020 11:27:31 +0200 Toke Høiland-Jørgensen wrote:
> Jakub Kicinski <kuba@...nel.org> writes:
> > I wonder, maybe our documentation is not clear. IOW we were saying that
> > XDP is a faster cls_bpf, which leaves out the part that XDP only makes
> > sense for HW/virt devices.  
> 
> I'm not sure it's just because people think it's faster. There's also a
> semantic difference; if you just want to do ingress filtering, simply
> sticking an XDP program on the interface is a natural fit.

I'm afraid if we take that stand we're going to struggle to deliver. 
I'd personally prefer to keep XDP simple and focused.

> Whereas figuring out the tc semantics for ingress is non-trivial.

You mean adding an ingress qdisc and installing a filter?
If it is perhaps we could alleviate that with better user space tooling?

> And also reusability of XDP programs from the native hook is an important
> consideration, I believe. Which is also why I think the pseudo-MAC
> header approach is the right fix for L3 devices :)

Yes, valid point, I've never tried it myself, but I believe the C-level
portability shouldn't be too hard? At least the context members are
named the same. And there isn't that much XDP can do..

Perhaps the portability is something we should keep in mind going
forward. Just in case.

> > Kinda same story as XDP egress, folks may be asking for it but that
> > doesn't mean it makes sense.  
> 
> Well I do also happen to think that XDP egress is a good idea ;)

I was planning not to get involved in that conversation any more,
let's move on :P

> > Perhaps the original reporter realized this and that's why they
> > disappeared?
> >
> > My understanding is that XDP generic is aimed at testing and stop gap
> > for drivers which don't implement native. Defining behavior based on
> > XDP generic's needs seems a little backwards, and risky.  
> 
> That I can agree with - generic XDP should follow the semantics of
> native XDP, not the other way around. But that's what we're doing here
> (with the pseudo-MAC header approach), isn't it? Whereas if we were
> saying "just write your XDP programs to assume only L3 packets" we would
> be creating a new semantic for generic XDP...

But you do see the problem this creates on redirect already, right?
Do we want to support all that? Add an if in the redirect fast path?
There will _never_ be native XDP for WireGuard, it just doesn't make
sense. 

Generic XDP is not a hook in its own right, frame is already firmly
inside the stack, XDP is on the perimeter.