Re: [PATCH v2 bpf-next 1/5] ftrace: allow customized flags for ftrace_direct_multi ftrace_ops

From: Song Liu
Date: Wed Jul 13 2022 - 21:43:10 EST

> On Jul 13, 2022, at 5:38 PM, Steven Rostedt <rostedt@xxxxxxxxxxx> wrote:
> On Thu, 14 Jul 2022 00:11:53 +0000
> Song Liu <songliubraving@xxxxxx> wrote:
>>> That is, can we register a direct function with this function and pick a
>>> function with IPMODIFY already attached?
>> Yes, if the direct function follows regs->ip, it works.
>> For example, BPF trampoline with only fentry calls will just work with only this patch.
> I replied with my thoughts on this to patch 3, but I disagree with this.
> ftrace has no idea if the direct trampoline modifies the IP or not.
> ftrace must assume that it does, and the management should be done in
> the infrastructure.
> As I replied to patch 3, here's my thoughts.
> DIRECT is treated as though it changes the IP. If you register it to a
> function that has an IPMODIFY already set to it, it will call the
> ops->ops_func() asking if the ops can use SHARED_IPMODIFY (which means
> a DIRECT can be shared with IPMODIFY). If it can, then it returns true,
> and the SHARED_IPMODIFY is set *by ftrace*. The user of the ftrace APIs
> should not have to manage this. It should be managed by the ftrace
> infrastructure.

Hmm... I don't think this gonna work.

First, two IPMODIFY ftrace ops cannot work together on the same kernel
function. So there won't be a ops with both IPMODIFY and SHARE_IPMODIFY.

non-direct ops without IPMODIFY can already share with IPMODIFY ops.
Only direct ops need SHARE_IPMODIFY flag, and it means "I can share with
another ops with IPMODIFY". So there will be different flavors of
direct ops:


#1 can never work on the same function with another IPMODIFY ops, and
we don't plan to make it work. #2 does not work with another IPMODIFY
ops. And #3 works with another IPMODIFY ops.

The owner of the direct trampoline uses these flags to tell ftrace
infrastructure the property of this trampoline.

BPF trampolines with only fentry calls are #3 direct ops. BPF
trampolines with fexit or fmod_ret calls will be #2 trampoline by
default, but it is also possible to generate #3 trampoline for it.

BPF side will try to register #2 trampoline, If ftrace detects another
IPMODIFY ops on the same function, it will let BPF trampoline know
with -EAGAIN from register_ftrace_direct_multi(). Then, BPF side will
regenerate a #3 trampoline and register it again.

I know this somehow changes the policy with direct ops, but it is the
only way this can work, AFAICT.

Does this make sense? Did I miss something?