> 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. w/ IPMODIFY, w/o SHARE_IPMODIFY; 2. w/o IPMODIFY, w/o SHARE_IPMODIFY; 3. w/o IPMODIFY, w/ SHARE_IPMODIFY. #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? Thanks, Song
