From: "Björn Töpel" <bjorn.topel@intel.com>
To: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>,
"Björn Töpel" <bjorn.topel@gmail.com>
Cc: ast@kernel.org, daniel@iogearbox.net, netdev@vger.kernel.org,
bpf@vger.kernel.org, kafai@fb.com, songliubraving@fb.com,
yhs@fb.com, andrii@kernel.org, john.fastabend@gmail.com,
hawk@kernel.org, kuba@kernel.org, magnus.karlsson@intel.com,
maciej.fijalkowski@intel.com
Subject: Re: [PATCH bpf-next] bpf, xdp: add bpf_redirect{,_map}() leaf node detection and optimization
Date: Wed, 2 Dec 2020 09:01:41 +0100 [thread overview]
Message-ID: <1f3efb08-498c-7e77-040d-5551e8237d17@intel.com> (raw)
In-Reply-To: <20201202044638.zqqlgabmx2xjsunf@ast-mbp>
On 2020-12-02 05:46, Alexei Starovoitov wrote:
[...]
>
> Sorry I don't like this check at all. It's too fragile.
> It will work for one hard coded program.
> It may work for something more real, but will break with minimal
> changes to the prog or llvm changes.
> How are we going to explain that fragility to users?
>
[...]
>
> I haven't looked through all possible paths, but it feels very dangerous.
> The stack growth is big. Calling xsk_rcv from preempt_disabled
> and recursively calling into another bpf prog?
> That violates all stack checks we have in the verifier.
>
Fair points, and thanks for pointing them out.
If the robustness (your first point) is improved, say via proper
indirect jump support, the stack usage will still be a concern.
> I see plenty of cons and not a single pro in this patch.
> 5% improvement for micro benchmark? That's hardly a justification.
>
It's indeed a ubench, and something that is mostly beneficial to AF_XDP.
I'll go back to the drawing board and make sure the cons/pro balance is
improved.
Thanks for the feedback!
Björn
prev parent reply other threads:[~2020-12-02 8:02 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-12-01 17:23 [PATCH bpf-next] bpf, xdp: add bpf_redirect{,_map}() leaf node detection and optimization Björn Töpel
2020-12-01 21:42 ` Toke Høiland-Jørgensen
2020-12-02 7:55 ` Björn Töpel
2020-12-02 4:46 ` Alexei Starovoitov
2020-12-02 8:01 ` Björn Töpel [this message]
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=1f3efb08-498c-7e77-040d-5551e8237d17@intel.com \
--to=bjorn.topel@intel.com \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bjorn.topel@gmail.com \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=hawk@kernel.org \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kuba@kernel.org \
--cc=maciej.fijalkowski@intel.com \
--cc=magnus.karlsson@intel.com \
--cc=netdev@vger.kernel.org \
--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 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).