From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: David Ahern <dsahern@gmail.com>, David Ahern <dsahern@kernel.org>,
netdev@vger.kernel.org
Cc: davem@davemloft.net, kuba@kernel.org,
prashantbhole.linux@gmail.com, brouer@redhat.com,
daniel@iogearbox.net, john.fastabend@gmail.com, ast@kernel.org,
kafai@fb.com, songliubraving@fb.com, yhs@fb.com, andriin@fb.com,
David Ahern <dahern@digitalocean.com>
Subject: Re: [PATCH v5 bpf-next 00/11] net: Add support for XDP in egress path
Date: Mon, 18 May 2020 20:00:45 +0200 [thread overview]
Message-ID: <87h7wdnmwi.fsf@toke.dk> (raw)
In-Reply-To: <76e2e842-19c0-fd9a-3afa-07e2793dedcd@gmail.com>
David Ahern <dsahern@gmail.com> writes:
> On 5/18/20 3:08 AM, Toke Høiland-Jørgensen wrote:
>> I can see your point that fixing up the whole skb after the program has
>> run is not a good idea. But to me that just indicates that the hook is
>> in the wrong place: that it really should be in the driver, executed at
>> a point where the skb data structure is no longer necessary (similar to
>> how the ingress hook is before the skb is generated).
>
> Have you created a cls_bpf program to modify skbs? Have you looked at
> the helpers, the restrictions and the tight management of skb changes?
> Have you followed the skb from create to device handoff through the
> drivers? Have you looked at the history of encapsulations, gso handling,
> offloads, ...?
Have you tried re-reading the first sentence of the paragraph you're
replying to? You know, the one that started with "I can see your point
that..."
>> Otherwise, what you're proposing is not an egress hook, but rather a
>> 'post-REDIRECT hook', which is strictly less powerful. This may or may
>> not be useful in its own right, but let's not pretend it's a full egress
>> hook. Personally I feel that the egress hook is what we should be going
>> for, not this partial thing.
>
> You are hand waving. Be specific, with details.
Are you deliberately trying to antagonise me or something? It's a really
odd way to try to make your case...
> Less powerful how? There are only so many operations you can do to a
> packet. What do you want to do and what can't be done with this proposed
> change? Why must it be done as XDP vs proper synergy between the 2 paths.
I meant 'less powerful' in the obvious sense: it only sees a subset of
the packets going out of the interface. And so I worry that it will (a)
make an already hard to use set of APIs even more confusing, and (b)
turn out to not be enough so we'll end up needing a "real" egress hook.
As I said in my previous email, a post-REDIRECT hook may or may not be
useful in its own right. I'm kinda on the fence about that, but am
actually leaning towards it being useful; however, I am concerned that
it'll end up being redundant if we do get a full egress hook.
-Toke
next prev parent reply other threads:[~2020-05-18 18:00 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-05-13 1:45 [PATCH v5 bpf-next 00/11] net: Add support for XDP in egress path David Ahern
2020-05-13 1:45 ` [PATCH v5 bpf-next 01/11] net: Refactor convert_to_xdp_frame David Ahern
2020-05-13 1:45 ` [PATCH v5 bpf-next 02/11] net: uapi for XDP programs in the egress path David Ahern
2020-05-13 1:45 ` [PATCH v5 bpf-next 03/11] net: Add XDP setup and query commands for Tx programs David Ahern
2020-05-13 1:46 ` [PATCH v5 bpf-next 04/11] net: Add BPF_XDP_EGRESS as a bpf_attach_type David Ahern
2020-05-13 1:46 ` [PATCH v5 bpf-next 05/11] xdp: Add xdp_txq_info to xdp_buff David Ahern
2020-05-13 1:46 ` [PATCH v5 bpf-next 06/11] net: set XDP egress program on netdevice David Ahern
2020-05-13 1:46 ` [PATCH v5 bpf-next 07/11] net: Support xdp in the Tx path for xdp_frames David Ahern
2020-05-13 1:46 ` [PATCH v5 bpf-next 08/11] libbpf: Add egress XDP support David Ahern
2020-05-13 1:46 ` [PATCH v5 bpf-next 09/11] bpftool: Add support for XDP egress David Ahern
2020-05-13 1:46 ` [PATCH v5 bpf-next 10/11] selftest: Add xdp_egress attach tests David Ahern
2020-05-13 1:46 ` [PATCH v5 bpf-next 11/11] samples/bpf: add XDP egress support to xdp1 David Ahern
2020-05-13 10:43 ` [PATCH v5 bpf-next 00/11] net: Add support for XDP in egress path Toke Høiland-Jørgensen
2020-05-13 19:37 ` David Ahern
2020-05-15 22:54 ` John Fastabend
2020-05-15 23:15 ` David Ahern
2020-05-18 18:10 ` John Fastabend
2020-05-18 23:52 ` David Ahern
2020-05-19 6:04 ` John Fastabend
2020-05-18 3:40 ` David Ahern
2020-05-18 9:08 ` Toke Høiland-Jørgensen
2020-05-18 14:44 ` David Ahern
2020-05-18 18:00 ` Toke Høiland-Jørgensen [this message]
2020-05-18 21:06 ` Daniel Borkmann
2020-05-19 0:02 ` David Ahern
2020-05-19 13:31 ` Daniel Borkmann
2020-05-19 14:21 ` Jesper Dangaard Brouer
2020-05-19 16:58 ` Lorenzo Bianconi
2020-05-19 14:52 ` Toke Høiland-Jørgensen
2020-05-19 16:37 ` David Ahern
2020-05-18 23:37 ` David Ahern
2020-05-18 21:23 ` Daniel Borkmann
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=87h7wdnmwi.fsf@toke.dk \
--to=toke@redhat.com \
--cc=andriin@fb.com \
--cc=ast@kernel.org \
--cc=brouer@redhat.com \
--cc=dahern@digitalocean.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=dsahern@gmail.com \
--cc=dsahern@kernel.org \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kuba@kernel.org \
--cc=netdev@vger.kernel.org \
--cc=prashantbhole.linux@gmail.com \
--cc=songliubraving@fb.com \
--cc=yhs@fb.com \
/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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.