From: Daniel Borkmann <daniel@iogearbox.net>
To: Jesper Dangaard Brouer <hawk@kernel.org>, bpf@vger.kernel.org
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <borkmann@iogearbox.net>,
martin.lau@kernel.org, netdev@vger.kernel.org,
kernel-team@cloudflare.com
Subject: Re: [PATCH bpf-next] bpf/lpm_trie: inline longest_prefix_match for fastpath
Date: Fri, 15 Mar 2024 16:03:21 +0100 [thread overview]
Message-ID: <2c2d1b85-9c4a-5122-c471-e4a729b4df03@iogearbox.net> (raw)
In-Reply-To: <171025648415.2098287.4441181253947701605.stgit@firesoul>
On 3/12/24 4:17 PM, Jesper Dangaard Brouer wrote:
> The BPF map type LPM (Longest Prefix Match) is used heavily
> in production by multiple products that have BPF components.
> Perf data shows trie_lookup_elem() and longest_prefix_match()
> being part of kernels perf top.
You mention these are heavy hitters in prod ...
> For every level in the LPM tree trie_lookup_elem() calls out
> to longest_prefix_match(). The compiler is free to inline this
> call, but chooses not to inline, because other slowpath callers
> (that can be invoked via syscall) exists like trie_update_elem(),
> trie_delete_elem() or trie_get_next_key().
>
> bcc/tools/funccount -Ti 1 'trie_lookup_elem|longest_prefix_match.isra.0'
> FUNC COUNT
> trie_lookup_elem 664945
> longest_prefix_match.isra.0 8101507
>
> Observation on a single random metal shows a factor 12 between
> the two functions. Given an average of 12 levels in the trie being
> searched.
>
> This patch force inlining longest_prefix_match(), but only for
> the lookup fastpath to balance object instruction size.
>
> $ bloat-o-meter kernel/bpf/lpm_trie.o.orig-noinline kernel/bpf/lpm_trie.o
> add/remove: 1/1 grow/shrink: 1/0 up/down: 335/-4 (331)
> Function old new delta
> trie_lookup_elem 179 510 +331
> __BTF_ID__struct__lpm_trie__706741 - 4 +4
> __BTF_ID__struct__lpm_trie__706733 4 - -4
> Total: Before=3056, After=3387, chg +10.83%
... and here you quote bloat-o-meter instead. But do you also see an
observable perf gain in prod after this change? (No objection from my
side but might be good to mention here.. given if not then why do the
change?)
> Details: Due to AMD mitigation for SRSO (Speculative Return Stack Overflow)
> these function calls have additional overhead. On newer kernels this shows
> up under srso_safe_ret() + srso_return_thunk(), and on older kernels (6.1)
> under __x86_return_thunk(). Thus, for production workloads the biggest gain
> comes from avoiding this mitigation overhead.
>
> Signed-off-by: Jesper Dangaard Brouer <hawk@kernel.org>
next prev parent reply other threads:[~2024-03-15 15:03 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-03-12 15:17 [PATCH bpf-next] bpf/lpm_trie: inline longest_prefix_match for fastpath Jesper Dangaard Brouer
2024-03-15 15:03 ` Daniel Borkmann [this message]
2024-03-15 17:08 ` Jesper Dangaard Brouer
2024-03-18 12:12 ` Jesper Dangaard Brouer
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=2c2d1b85-9c4a-5122-c471-e4a729b4df03@iogearbox.net \
--to=daniel@iogearbox.net \
--cc=ast@kernel.org \
--cc=borkmann@iogearbox.net \
--cc=bpf@vger.kernel.org \
--cc=hawk@kernel.org \
--cc=kernel-team@cloudflare.com \
--cc=martin.lau@kernel.org \
--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).