Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Wed, 13 Jul 2022 20:38:41 -0400
From:   Steven Rostedt <rostedt@goodmis.org>
To:     Song Liu <songliubraving@fb.com>
Cc:     Song Liu <song@kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "bpf@vger.kernel.org" <bpf@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "ast@kernel.org" <ast@kernel.org>,
        "daniel@iogearbox.net" <daniel@iogearbox.net>,
        "andrii@kernel.org" <andrii@kernel.org>,
        Kernel Team <Kernel-team@fb.com>,
        "jolsa@kernel.org" <jolsa@kernel.org>,
        "mhiramat@kernel.org" <mhiramat@kernel.org>
Subject: Re: [PATCH v2 bpf-next 1/5] ftrace: allow customized flags for
 ftrace_direct_multi ftrace_ops
Message-ID: <20220713203841.76d66245@rorschach.local.home>
In-Reply-To: <0029EF24-6508-4011-B365-3E2175F9FEAB@fb.com>
References: <20220602193706.2607681-1-song@kernel.org>
        <20220602193706.2607681-2-song@kernel.org>
        <20220713191846.18b05b43@gandalf.local.home>
        <0029EF24-6508-4011-B365-3E2175F9FEAB@fb.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=US-ASCII
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Thu, 14 Jul 2022 00:11:53 +0000
Song Liu <songliubraving@fb.com> 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.

Make sense?

-- Steve