Netdev Archive on lore.kernel.org
help / color / mirror / Atom feed
From: "Jason A. Donenfeld" <Jason@zx2c4.com>
To: David Miller <davem@davemloft.net>
Cc: "Jakub Kicinski" <kuba@kernel.org>,
	Netdev <netdev@vger.kernel.org>,
	"Thomas Ptacek" <thomas@sockpuppet.org>,
	"Adhipati Blambangan" <adhipati@tuta.io>,
	"David Ahern" <dsahern@gmail.com>,
	"Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Alexei Starovoitov" <alexei.starovoitov@gmail.com>
Subject: Re: [PATCH net v4] net: xdp: account for layer 3 packets in generic skb handler
Date: Sat, 15 Aug 2020 09:54:56 +0200	[thread overview]
Message-ID: <CAHmME9pq8iFfuf2EZG_4xQhDekP+hZR8yUDgYEf3gWNONM_3Ew@mail.gmail.com> (raw)
In-Reply-To: <20200814.142656.1061722366614948972.davem@davemloft.net>

On Fri, Aug 14, 2020 at 11:27 PM David Miller <davem@davemloft.net> wrote:
>
> From: "Jason A. Donenfeld" <Jason@zx2c4.com>
> Date: Fri, 14 Aug 2020 23:04:56 +0200
>
> > What? No. It comes up repeatedly because people want to reuse their
> > XDP processing logic with layer 3 devices.
>
> XDP is a layer 2 packet processing technology.  It assumes an L2
> ethernet and/or VLAN header is going to be there.
>
> Putting a pretend ethernet header there doesn't really change that.

Actually, I wasn't aware that XDP was explicitly limited to L2-only,
as some kind of fundamental thing. A while back when this patchset
first came up, I initially posted something that gave unmodified L3
frames to XDP programs in the generic handler, but commenters pointed
out that this loses the skb->protocol changing capability, which could
be useful (e.g. some kind of custom v4->v6 modifying code), and adding
the pretend ethernet header would allow people to keep the same
programs for the L2 case as for the L3 case, which seemed *extremely*
compelling to me. Hence, this patch series has gone in the pretend
ethernet header direction.

But, anyway, as I said, I wasn't aware that XDP was explicitly limited
to L2-only, as some kind of fundamental thing. This actually surprises
me. I always thought the original motivation of XDP was that it
allowed putting a lot of steering and modification logic into the
network card hardware, for fancy new cards that support eBPF. Later,
the generic handler got added on so people could reuse those programs
in heterogeneous environments, where some cards have hardware support
and others do not. That seemed like a good idea to me. Extending that
a step further for layer 3 devices seems like a logical next step, in
the sense that if that step is invalid, surely the previous one of
adding the generic handler must be invalid too? At least that's my
impression of the historical evolution of this; I'm confident you know
much better than I do.

It makes me wonder, though, what will you say when hardware comes out
that has layer 3 semantics and a thirst for eBPF? Also deny that
hardware of XDP, because "XDP is a layer 2 packet processing
technology"? I know what you'll say now: "we don't design our
networking stack around hypothetical hardware, so why even bring it
up? I won't entertain that." But nevertheless, contemplating those
hypotheticals might be a good exercise for seeing how committed you
are to the XDP=L2-only assertion. For example, maybe there are
fundamental L2 semantics that XDP needs, and can't be emulated with my
pretend ethernet header -- like if you really are relying on the MACs
for something I'm not aware of; were that the case, it'd be compelling
to me. But if it's a bit weaker, in the form of, "we just don't want
to try anything with L3 at all because," then I admit I'm still a bit
mystified.

Nevertheless, at the risk of irritating you further, I will willingly
drop this patchset at your request, even though I don't completely
understand the entire scope of reasoning for doing so. (For posterity,
I just posted a v6, which splits out that other bug fix into something
separate for you to take, so that this one here can exist on its own.)

Jason

  reply	other threads:[~2020-08-15 23:55 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-08-13 19:58 Jason A. Donenfeld
2020-08-13 21:01 ` Jakub Kicinski
2020-08-14  6:56   ` Jason A. Donenfeld
2020-08-14  7:30     ` [PATCH net v5] " Jason A. Donenfeld
2020-08-14 20:55       ` David Miller
2020-08-14 20:57         ` Jason A. Donenfeld
2020-08-15  7:41         ` [PATCH net v6] " Jason A. Donenfeld
2020-08-19  7:07           ` Jesper Dangaard Brouer
2020-08-19 23:22           ` David Miller
2020-08-20  9:13             ` Jason A. Donenfeld
2020-08-20 18:55               ` David Miller
2020-08-20 20:29                 ` Jason A. Donenfeld
2020-08-14 15:31     ` [PATCH net v4] " Jakub Kicinski
2020-08-14 21:04       ` Jason A. Donenfeld
2020-08-14 21:26         ` David Miller
2020-08-15  7:54           ` Jason A. Donenfeld [this message]
2020-08-14 21:14       ` David Miller

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=CAHmME9pq8iFfuf2EZG_4xQhDekP+hZR8yUDgYEf3gWNONM_3Ew@mail.gmail.com \
    --to=jason@zx2c4.com \
    --cc=adhipati@tuta.io \
    --cc=alexei.starovoitov@gmail.com \
    --cc=davem@davemloft.net \
    --cc=dsahern@gmail.com \
    --cc=kuba@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=thomas@sockpuppet.org \
    --cc=toke@redhat.com \
    --subject='Re: [PATCH net v4] net: xdp: account for layer 3 packets in generic skb handler' \
    /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

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).