netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: "Jason A. Donenfeld" <Jason@zx2c4.com>, netdev@vger.kernel.org
Subject: Re: [PATCH RFC v1] net: xdp: allow for layer 3 packets in generic skb handler
Date: Mon, 27 Apr 2020 09:20:34 +0200	[thread overview]
Message-ID: <87h7x51jjx.fsf@toke.dk> (raw)
In-Reply-To: <20200427011002.320081-1-Jason@zx2c4.com>

"Jason A. Donenfeld" <Jason@zx2c4.com> writes:

> A user reported a few days ago that packets from wireguard were possibly
> ignored by XDP [1]. We haven't heard back from the original reporter to
> receive more info, so this here is mostly speculative. Successfully nerd
> sniped, Toke and I started poking around. Toke noticed that 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 is untested, but I
> wanted to gauge interest in this approach: if the mac_len is 0, then we
> assume that it's a layer 3 packet, and figure out skb->protocol from
> looking at the IP header. This patch also adds some stricter testing
> around mac_len before we assume that it's an ethhdr.

While your patch will fix the header pointer mangling for the skb, it
unfortunately won't fix generic XDP for Wireguard: The assumption that
there's an Ethernet header present is made for compatibility with native
XDP, so you might say it's deliberate. I.e., the eBPF programs running
in the XDP hook expect to see an Ethernet header as part of the packet
data (and parses the packet like in [0]).

So, to make XDP generic work for Wireguard (or other IP-header-only
devices) we'd need to either (1) introduce a new XDP sub-type that
assumes L4 packets, or (2) make Wireguard add a fake Ethernet header to
the head of the packet and set the skb mac_header accordingly.

We've discussed (1) before in other contexts (specifically, adding a
802.11 sub-type), but IIRC we decided that there wasn't enough interest.
I wonder if the same wouldn't be the case for an IP sub-type, since
users would have to re-write their XDP programs to fit that hook type,
and it would only be usable for generic XDP on certain tunnel interface
types. Not sure about the feasibility of (2).

-Toke

[0] https://github.com/xdp-project/xdp-tutorial/blob/master/packet01-parsing/xdp_prog_kern.c


  reply	other threads:[~2020-04-27  7:20 UTC|newest]

Thread overview: 27+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-04-27  1:10 [PATCH RFC v1] net: xdp: allow for layer 3 packets in generic skb handler Jason A. Donenfeld
2020-04-27  7:20 ` Toke Høiland-Jørgensen [this message]
2020-04-27 10:05   ` Jason A. Donenfeld
2020-04-27 10:22     ` [PATCH RFC v2] " Jason A. Donenfeld
2020-04-27 11:16       ` Toke Høiland-Jørgensen
2020-04-27 14:45       ` David Ahern
2020-04-27 19:58         ` Jason A. Donenfeld
2020-04-27 20:08           ` Toke Høiland-Jørgensen
2020-04-27 20:10             ` Jason A. Donenfeld
2020-04-27 20:42               ` [PATCH net v3] net: xdp: account " Jason A. Donenfeld
2020-04-27 20:52                 ` Jakub Kicinski
2020-04-27 20:57                   ` Jason A. Donenfeld
2020-04-27 21:00                   ` Jakub Kicinski
2020-04-27 21:08                     ` Jason A. Donenfeld
2020-04-27 21:19                       ` Jakub Kicinski
2020-04-27 21:14                     ` Toke Høiland-Jørgensen
2020-04-27 21:31                       ` Jakub Kicinski
2020-04-27 23:00                         ` Jason A. Donenfeld
2020-04-27 23:45                           ` Jason A. Donenfeld
2020-04-28  0:15                             ` Jakub Kicinski
2020-04-28  0:17                               ` Jason A. Donenfeld
2020-04-28  0:40                                 ` Jakub Kicinski
2020-04-28  9:27                         ` Toke Høiland-Jørgensen
2020-04-28 16:51                           ` Jakub Kicinski
2020-04-28 17:03                             ` Alexei Starovoitov
2020-04-27 21:00                 ` Toke Høiland-Jørgensen
2020-04-27 10:30     ` [PATCH RFC v1] net: xdp: allow " Toke Høiland-Jørgensen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87h7x51jjx.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=Jason@zx2c4.com \
    --cc=netdev@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).