Re: [PATCH 13/19] bpf: Add support to link multi func tracing program

On Tue, Jun 8, 2021 at 4:07 PM Alexei Starovoitov
[off-list ref] wrote:

On Tue, Jun 8, 2021 at 2:07 PM Jiri Olsa [off-list ref] wrote:

quoted

On Tue, Jun 08, 2021 at 11:49:03AM -0700, Alexei Starovoitov wrote:

quoted

On Tue, Jun 08, 2021 at 08:17:00PM +0200, Jiri Olsa wrote:

quoted

On Tue, Jun 08, 2021 at 08:42:32AM -0700, Alexei Starovoitov wrote:

quoted

On Sat, Jun 5, 2021 at 4:11 AM Jiri Olsa [off-list ref] wrote:

quoted

Adding support to attach multiple functions to tracing program
by using the link_create/link_update interface.

Adding multi_btf_ids/multi_btf_ids_cnt pair to link_create struct
API, that define array of functions btf ids that will be attached
to prog_fd.

The prog_fd needs to be multi prog tracing program (BPF_F_MULTI_FUNC).

The new link_create interface creates new BPF_LINK_TYPE_TRACING_MULTI
link type, which creates separate bpf_trampoline and registers it
as direct function for all specified btf ids.

The new bpf_trampoline is out of scope (bpf_trampoline_lookup) of
standard trampolines, so all registered functions need to be free
of direct functions, otherwise the link fails.

Overall the api makes sense to me.
The restriction of multi vs non-multi is too severe though.
The multi trampoline can serve normal fentry/fexit too.

so multi trampoline gets called from all the registered functions,
so there would need to be filter for specific ip before calling the
standard program.. single cmp/jnz might not be that bad, I'll check

You mean reusing the same multi trampoline for all IPs and regenerating
it with a bunch of cmp/jnz checks? There should be a better way to scale.
Maybe clone multi trampoline instead?
IPs[1-10] will point to multi.
IP[11] will point to a clone of multi that serves multi prog and
fentry/fexit progs specific for that IP.

ok, so we'd clone multi trampoline if there's request to attach
standard trampoline to some IP from multi trampoline

.. and transform currently attached standard trampoline for IP
into clone of multi trampoline, if there's request to create
multi trampoline that covers that IP

yep. For every IP==btf_id there will be only two possible trampolines.
Should be easy enough to track and transition between them.
The standard fentry/fexit will only get negligible slowdown from
going through multi.
multi+fexit and fmod_ret needs to be thought through as well.
That's why I thought that 'ip' at the end should simplify things.

Putting ip at the end has downsides. We might support >6 arguments
eventually, at which point it will be super weird to have 6 args, ip,
then the rest of arguments?..

Would it be too bad to put IP at -8 offset relative to ctx? That will
also work for normal fentry/fexit, for which it's useful to have ip
passed in as well, IMO. So no special casing for multi/non-multi, and
it's backwards compatible.

Ideally, I'd love it to be actually retrievable through a new BPF
helper, something like bpf_caller_ip(ctx), but I'm not sure if we can
implement this sanely, so I don't hold high hopes.

Only multi will have access to it.
But we can store it first too. fentry/fexit will see ctx=r1 with +8 offset
and will have normal args in ctx. Like ip isn't even there.
While multi trampoline is always doing ip, arg1,arg2, .., arg6
and passes ctx = &ip into multi prog and ctx = &arg1 into fentry/fexit.
'ret' for fexit is problematic though. hmm.
Maybe such clone multi trampoline for specific ip with 2 args will do:
ip, arg1, arg2, ret, 0, 0, 0, ret.
Then multi will have 6 args, though 3rd is actually ret.
Then fexit will have ret in the right place and multi prog will have
it as 7th arg.