bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: "Joel Fernandes (Google)" <joel@joelfernandes.org>
Cc: 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>,
	Steven Rostedt <rostedt@goodmis.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 13:54:57 -0700	[thread overview]
Message-ID: <20190716205455.iimn3pqpvsc3k4ry@ast-mbp.dhcp.thefacebook.com> (raw)
In-Reply-To: <20190710141548.132193-1-joel@joelfernandes.org>

On Wed, Jul 10, 2019 at 10:15:44AM -0400, Joel Fernandes (Google) wrote:
> Hi,

why are you cc-ing the whole world for this patch set?
I'll reply to all as well, but I suspect a bunch of folks consider it spam.
Please read Documentation/bpf/bpf_devel_QA.rst

Also, I think, netdev@vger rejects emails with 80+ characters in cc as spam,
so I'm not sure this set reached public mailing lists.

> These patches make it possible to attach BPF programs directly to tracepoints
> using ftrace (/sys/kernel/debug/tracing) without needing the process doing the
> attach to be alive. This has the following benefits:
> 1. Simplified Security: In Android, we have finer-grained security controls to
> specific ftrace trace events using SELinux labels. We control precisely who is
> allowed to enable an ftrace event already. By adding a node to ftrace for
> attaching BPF programs, we can use the same mechanism to further control who is
> allowed to attach to a trace event.
> 2. Process lifetime: In Android we are adding usecases where a tracing program
> needs to be attached all the time to a tracepoint, for the full life time of
> the system. Such as to gather statistics where there no need for a detach for
> the full system lifetime. With perf or bpf(2)'s BPF_RAW_TRACEPOINT_OPEN, this
> means keeping a process alive all the time.  However, in Android our BPF loader
> currently (for hardeneded security) involves just starting a process at boot
> time, doing the BPF program loading, and then pinning them to /sys/fs/bpf.  We
> don't keep this process alive all the time. It is more suitable to do a
> one-shot attach of the program using ftrace and not need to have a process
> alive all the time anymore for this. Such process also needs elevated
> privileges since tracepoint program loading currently requires CAP_SYS_ADMIN
> anyway so by design Android's bpfloader runs once at init and exits.
> This series add a new bpf file to /sys/kernel/debug/tracing/events/X/Y/bpf
> The following commands can be written into it:
> attach:<fd>     Attaches BPF prog fd to tracepoint
> detach:<fd>     Detaches BPF prog fd to tracepoint

Looks like, to detach a program the user needs to read a text file,
parse bpf prog id from text into binary. Then call fd_from_id bpf syscall,
get a binary FD, convert it back to text and write as a text back into this file.
I think this is just a single example why text based apis are not accepted
in bpf anymore.

Through the patch set you call it ftrace. As far as I can see, this set
has zero overlap with ftrace. There is no ftrace-bpf connection here at all
that we discussed in the past Steven. It's all quite confusing.

I suggest android to solve sticky raw_tracepoint problem with user space deamon.
The reasons, you point out why user daemon cannot be used, sound weak to me.
Another acceptable solution would be to introduce pinning of raw_tp objects.
bpf progs and maps can be pinned in bpffs already. Pinning raw_tp would
be natural extension.

  parent reply	other threads:[~2019-07-16 20:55 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-10 14:15 [PATCH RFC 0/4] Add support to directly attach BPF program to ftrace Joel Fernandes (Google)
2019-07-10 14:15 ` [PATCH RFC 1/4] Move bpf_raw_tracepoint functionality into bpf_trace.c Joel Fernandes (Google)
2019-07-10 14:15 ` [PATCH RFC 2/4] trace/bpf: Add support for attach/detach of ftrace events to BPF Joel Fernandes (Google)
2019-07-10 14:15 ` [PATCH RFC 3/4] lib/bpf: Add support for ftrace event attach and detach Joel Fernandes (Google)
2019-07-10 14:15 ` [PATCH RFC 4/4] selftests/bpf: Add test for ftrace-based BPF attach/detach Joel Fernandes (Google)
2019-07-16 20:54 ` Alexei Starovoitov [this message]
2019-07-16 21:30   ` [PATCH RFC 0/4] Add support to directly attach BPF program to ftrace Joel Fernandes
2019-07-16 22:26     ` Alexei Starovoitov
2019-07-16 22:41       ` Joel Fernandes
2019-07-16 23:55         ` Joel Fernandes
2019-07-17  1:24           ` Alexei Starovoitov
2019-07-17 13:01             ` Joel Fernandes
2019-07-17 21:40               ` Alexei Starovoitov
2019-07-18  2:51                 ` Joel Fernandes
2019-07-23 22:11                   ` Alexei Starovoitov
2019-07-24 13:57                     ` Joel Fernandes
2019-07-26 18:39                       ` Alexei Starovoitov
2019-07-26 19:18                         ` Joel Fernandes
2019-07-26 19:49                           ` Joel Fernandes
2019-07-16 22:43       ` Steven Rostedt
2019-07-16 22:31     ` Steven Rostedt
2019-07-16 22:46       ` Joel Fernandes
2019-07-17  1:30       ` Alexei Starovoitov

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20190716205455.iimn3pqpvsc3k4ry@ast-mbp.dhcp.thefacebook.com \
    --to=alexei.starovoitov@gmail.com \
    --cc=adrian.ratiu@collabora.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=brendan.d.gregg@gmail.com \
    --cc=carmeli.tamir@gmail.com \
    --cc=connoro@google.com \
    --cc=daniel@iogearbox.net \
    --cc=jeffv@google.com \
    --cc=joel@joelfernandes.org \
    --cc=kafai@fb.com \
    --cc=karim.yaghmour@opersys.com \
    --cc=kernel-team@android.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=linux@manojrajarao.com \
    --cc=manalishukla14@gmail.com \
    --cc=mhiramat@kernel.org \
    --cc=michalgr@fb.com \
    --cc=michalgr@live.com \
    --cc=mingo@redhat.com \
    --cc=mmullins@fb.com \
    --cc=namhyung@google.com \
    --cc=namhyung@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=paul.chaignon@gmail.com \
    --cc=primiano@google.com \
    --cc=qais.yousef@arm.com \
    --cc=rostedt@goodmis.org \
    --cc=russoue@gmail.com \
    --cc=shuah@kernel.org \
    --cc=songliubraving@fb.com \
    --cc=sramana@codeaurora.org \
    --cc=yhs@fb.com \
    --cc=yuchao.du@unisoc.com \


* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).