From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Jiri Olsa <olsajiri@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Andrii Nakryiko <andrii@kernel.org>, bpf <bpf@vger.kernel.org>,
Martin KaFai Lau <kafai@fb.com>, Song Liu <songliubraving@fb.com>,
Yonghong Song <yhs@fb.com>,
John Fastabend <john.fastabend@gmail.com>,
KP Singh <kpsingh@chromium.org>,
Stanislav Fomichev <sdf@google.com>, Hao Luo <haoluo@google.com>
Subject: Re: [RFC PATCH bpf-next 10/17] bpf: Add support to attach program to multiple trampolines
Date: Thu, 25 Aug 2022 10:43:56 -0700 [thread overview]
Message-ID: <CAADnVQKVnSu-wDiVk6E3mU9J_LGC+0ou63T8TUv-J=BSCZf6iQ@mail.gmail.com> (raw)
In-Reply-To: <YweedGDaL7yI382D@krava>
On Thu, Aug 25, 2022 at 9:08 AM Jiri Olsa <olsajiri@gmail.com> wrote:
>
> On Tue, Aug 23, 2022 at 06:22:37PM -0700, Alexei Starovoitov wrote:
> > On Mon, Aug 08, 2022 at 04:06:19PM +0200, Jiri Olsa wrote:
> > > Adding support to attach program to multiple trampolines
> > > with new attach/detach interface:
> > >
> > > int bpf_trampoline_multi_attach(struct bpf_tramp_prog *tp,
> > > struct bpf_tramp_id *id)
> > > int bpf_trampoline_multi_detach(struct bpf_tramp_prog *tp,
> > > struct bpf_tramp_id *id)
> > >
> > > The program is passed as bpf_tramp_prog object and trampolines to
> > > attach it to are passed as bpf_tramp_id object.
> > >
> > > The interface creates new bpf_trampoline object which is initialized
> > > as 'multi' trampoline and stored separtely from standard trampolines.
> > >
> > > There are following rules how the standard and multi trampolines
> > > go along:
> > > - multi trampoline can attach on top of existing single trampolines,
> > > which creates 2 types of function IDs:
> > >
> > > 1) single-IDs - functions that are attached within existing single
> > > trampolines
> > > 2) multi-IDs - functions that were 'free' and are now taken by new
> > > 'multi' trampoline
> > >
> > > - we allow overlapping of 2 'multi' trampolines if they are attached
> > > to same IDs
> > > - we do now allow any other overlapping of 2 'multi' trampolines
> > > - any new 'single' trampoline cannot attach to existing multi-IDs IDs.
> > >
> > > Maybe better explained on following example:
> > >
> > > - you want to attach program P to functions A,B,C,D,E,F
> > > via bpf_trampoline_multi_attach
> > >
> > > - D,E,F already have standard trampoline attached
> > >
> > > - the bpf_trampoline_multi_attach will create new 'multi' trampoline
> > > which spans over A,B,C functions and attach program P to single
> > > trampolines D,E,F
> > >
> > > - A,B,C functions are now 'not attachable' by any trampoline
> > > until the above 'multi' trampoline is released
> >
> > This restriction is probably too severe.
> > Song added support for trampoline and KLPs to co-exist on the same function.
> > This multi trampoline restriction will resurface the same issue.
> > afiak this restriction is only because multi trampoline image
> > is the same for A,B,C. This memory optimization is probably going too far.
> > How about we keep existing logic of one tramp image per function.
> > Pretend that multi-prog P matches BTF of the target function,
> > create normal tramp for it and attach prog P there.
> > The prototype of P allows six u64. The args are potentially rearding
> > garbage, but there are no safety issues, since multi progs don't know BTF types.
> >
> > We still need sinle bpf_link_multi to contain btf_ids of all functions,
> > but it can point to many bpf tramps. One for each attach function.
> >
> > iirc we discussed something like this long ago, but I don't remember
> > why we didn't go that route.
> > arch_prepare_bpf_trampoline is fast.
> > bpf_tramp_image_alloc is fast too.
> > So attaching one multi-prog to thousands of btf_id-s should be fast too.
> > The destroy part is interesting.
> > There we will be doing thousands of bpf_tramp_image_put,
> > but it's all async now. We used to have synchronize_rcu() which could
> > be the reason why this approach was slow.
> > Or is this unregister_fentry that slows it down?
> > But register_ftrace_direct_multi() interface should have solved it
> > for both register and unregister?
>
> I think it's the synchronize_rcu_tasks at the end of each ftrace update,
> that's why we added un/register_ftrace_direct_multi that makes the changes
> for multiple ips and syncs once at the end
hmm. Can synchronize_rcu_tasks be made optional?
For ftrace_direct that points to bpf tramps is it really needed?
> un/register_ftrace_direct_multi will attach/detach multiple multiple ips
> to single address (trampoline), so for this approach we would need to add new
> ftrace direct api that would allow to set multiple ips to multiple trampolines
> within one call..
right
> I was already checking on that and looks doable
awesome.
> another problem might be that this update function will need to be called with
> all related trampoline locks, which in this case would be thousands
sure. but these will be newly allocated trampolines and
brand new mutexes, so no contention.
But thousands of cmpxchg-s will take time. Would be good to measure
though. It might not be that bad.
next prev parent reply other threads:[~2022-08-25 17:44 UTC|newest]
Thread overview: 31+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-08 14:06 [RFC PATCH bpf-next 00/17] bpf: Add tracing multi link Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 01/17] bpf: Link shimlink directly in trampoline Jiri Olsa
2022-08-08 17:40 ` Song Liu
2022-08-08 17:58 ` Stanislav Fomichev
2022-08-09 15:36 ` Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 02/17] bpf: Replace bpf_tramp_links with bpf_tramp_progs Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 03/17] bpf: Store trampoline progs in arrays Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 04/17] bpf: Add multi tracing attach types Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 05/17] bpf: Add bpf_tramp_id object Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 06/17] bpf: Pass image struct to reg/unreg/modify fentry functions Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 07/17] bpf: Add support to postpone trampoline update Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 08/17] bpf: Factor bpf_trampoline_lookup function Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 09/17] bpf: Factor bpf_trampoline_put function Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 10/17] bpf: Add support to attach program to multiple trampolines Jiri Olsa
2022-08-24 1:22 ` Alexei Starovoitov
2022-08-25 16:08 ` Jiri Olsa
2022-08-25 17:43 ` Alexei Starovoitov [this message]
2022-08-26 2:35 ` Andrii Nakryiko
2022-08-26 14:20 ` Jiri Olsa
2022-08-27 5:15 ` Andrii Nakryiko
2022-08-27 12:16 ` Jiri Olsa
2022-08-26 4:37 ` Song Liu
2022-08-08 14:06 ` [RFC PATCH bpf-next 11/17] bpf: Add support to create tracing multi link Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 12/17] libbpf: Add btf__find_by_glob_kind function Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 13/17] libbpf: Add support to create tracing multi link Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 14/17] selftests/bpf: Add fentry tracing multi func test Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 15/17] selftests/bpf: Add fexit " Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 16/17] selftests/bpf: Add fentry/fexit " Jiri Olsa
2022-08-08 14:06 ` [RFC PATCH bpf-next 17/17] selftests/bpf: Add mixed " Jiri Olsa
2022-08-08 17:50 ` [RFC PATCH bpf-next 00/17] bpf: Add tracing multi link Song Liu
2022-08-08 20:35 ` Jiri Olsa
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='CAADnVQKVnSu-wDiVk6E3mU9J_LGC+0ou63T8TUv-J=BSCZf6iQ@mail.gmail.com' \
--to=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=haoluo@google.com \
--cc=john.fastabend@gmail.com \
--cc=kafai@fb.com \
--cc=kpsingh@chromium.org \
--cc=olsajiri@gmail.com \
--cc=sdf@google.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 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).