Re: [PATCH RFC 0/4] Add support to directly attach BPF program to ftrace

From: Joel Fernandes <joel@joelfernandes.org>
To: Steven Rostedt <rostedt@goodmis.org>
Cc: Alexei Starovoitov <alexei.starovoitov@gmail.com>,
	linux-kernel@vger.kernel.org,
	Adrian Ratiu <adrian.ratiu@collabora.com>,
	Alexei Starovoitov <ast@kernel.org>,
	bpf@vger.kernel.org, Brendan Gregg <brendan.d.gregg@gmail.com>,
	connoro@google.com, Daniel Borkmann <daniel@iogearbox.net>,
	duyuchao <yuchao.du@unisoc.com>, Ingo Molnar <mingo@redhat.com>,
	jeffv@google.com, Karim Yaghmour <karim.yaghmour@opersys.com>,
	kernel-team@android.com, linux-kselftest@vger.kernel.org,
	Manali Shukla <manalishukla14@gmail.com>,
	Manjo Raja Rao <linux@manojrajarao.com>,
	Martin KaFai Lau <kafai@fb.com>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Matt Mullins <mmullins@fb.com>,
	Michal Gregorczyk <michalgr@fb.com>,
	Michal Gregorczyk <michalgr@live.com>,
	Mohammad Husain <russoue@gmail.com>,
	namhyung@google.com, namhyung@kernel.org, netdev@vger.kernel.org,
	paul.chaignon@gmail.com, primiano@google.com,
	Qais Yousef <qais.yousef@arm.com>, Shuah Khan <shuah@kernel.org>,
	Song Liu <songliubraving@fb.com>,
	Srinivas Ramana <sramana@codeaurora.org>,
	Tamir Carmeli <carmeli.tamir@gmail.com>,
	Yonghong Song <yhs@fb.com>
Subject: Re: [PATCH RFC 0/4] Add support to directly attach BPF program to ftrace
Date: Tue, 16 Jul 2019 18:46:06 -0400	[thread overview]
Message-ID: <20190716224606.GD172157@google.com> (raw)
In-Reply-To: <20190716183117.77b3ed49@gandalf.local.home>

On Tue, Jul 16, 2019 at 06:31:17PM -0400, Steven Rostedt wrote:
> On Tue, 16 Jul 2019 17:30:50 -0400
> Joel Fernandes <joel@joelfernandes.org> wrote:
> 
> > I don't see why a new bpf node for a trace event is a bad idea, really.
> > tracefs is how we deal with trace events on Android. We do it in production
> > systems. This is a natural extension to that and fits with the security model
> > well.
> 
> What I would like to see is a way to have BPF inject data into the
> ftrace ring buffer directly. There's a bpf_trace_printk() that I find a
> bit of a hack (especially since it hooks into trace_printk() which is
> only for debugging purposes). Have a dedicated bpf ftrace ring
> buffer event that can be triggered is what I am looking for. Then comes
> the issue of what ring buffer to place it in, as ftrace can have
> multiple ring buffer instances. But these instances are defined by the
> tracefs instances directory. Having a way to associate a bpf program to
> a specific event in a specific tracefs directory could allow for ways to
> trigger writing into the correct ftrace buffer.

But his problem is with doing the association of a BPF program with tracefs
itself. How would you attach a BPF program with tracefs without doing a text
based approach? His problem is with the text based approach per his last
email.

> But looking over the patches, I see what Alexei means that there's no
> overlap with ftrace and these patches except for the tracefs directory
> itself (which is part of the ftrace infrastructure). And the trace
> events are technically part of the ftrace infrastructure too. I see the
> tracefs interface being used, but I don't see how the bpf programs
> being added affect the ftrace ring buffer or other parts of ftrace. And
> I'm guessing that's what is confusing Alexei.

In a follow-up patch which I am still writing, I am using the trace ring
buffer as temporary storage since I am formatting the trace event into it.
This patch you are replying to is just for raw tracepoint and yes, I agree
this one does not use the ring buffer, but a future addition to it does. So
I don't think the association of this patch series with ftrace is going to be
an issue IMO.

thanks,

 - Joel