Jakub Kicinski <k...@kernel.org> writes: > On Mon, 27 Apr 2020 23:14:05 +0200 Toke Høiland-Jørgensen wrote: >> Jakub Kicinski <k...@kernel.org> writes: >> > On Mon, 27 Apr 2020 13:52:54 -0700 Jakub Kicinski wrote: >> >> On Mon, 27 Apr 2020 14:42:08 -0600 Jason A. Donenfeld wrote: >> >> > A user reported that packets from wireguard were possibly ignored by XDP >> >> > [1]. Apparently, the generic skb xdp handler path seems to assume that >> >> > packets will always have an ethernet header, which really isn't always >> >> > the case for layer 3 packets, which are produced by multiple drivers. >> >> > This patch fixes the oversight. If the mac_len is 0, then we assume >> >> > that it's a layer 3 packet, and in that case prepend a pseudo ethhdr to >> >> > the packet whose h_proto is copied from skb->protocol, which will have >> >> > the appropriate v4 or v6 ethertype. This allows us to keep XDP programs' >> >> > assumption correct about packets always having that ethernet header, so >> >> > that existing code doesn't break, while still allowing layer 3 devices >> >> > to use the generic XDP handler. >> >> >> >> Is this going to work correctly with XDP_TX? presumably wireguard >> >> doesn't want the ethernet L2 on egress, either? And what about >> >> redirects? >> >> >> >> I'm not sure we can paper over the L2 differences between interfaces. >> >> Isn't user supposed to know what interface the program is attached to? >> >> I believe that's the case for cls_bpf ingress, right? >> > >> > In general we should also ask ourselves if supporting XDPgeneric on >> > software interfaces isn't just pointless code bloat, and it wouldn't >> > be better to let XDP remain clearly tied to the in-driver native use >> > case. >> >> I was mostly ignoring generic XDP for a long time for this reason. But >> it seems to me that people find generic XDP quite useful, so I'm no >> longer so sure this is the right thing to do... > > 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. Whereas figuring out the tc semantics for ingress is non-trivial. 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 :) > 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 ;) > 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... -Toke
