BPF Archive on lore.kernel.org
 help / color / Atom feed
From: "Toke Høiland-Jørgensen" <toke@redhat.com>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Alexei Starovoitov <ast@kernel.org>,
	davem@davemloft.net, daniel@iogearbox.net, x86@kernel.org,
	netdev@vger.kernel.org, bpf@vger.kernel.org, kernel-team@fb.com
Subject: Re: [PATCH v3 bpf-next 15/18] bpf: Support attaching tracing BPF program to other BPF programs
Date: Fri, 08 Nov 2019 22:32:10 +0100
Message-ID: <87mud6caw5.fsf@toke.dk> (raw)
In-Reply-To: <20191108211400.m6kuuyvkp2p56gmo@ast-mbp.dhcp.thefacebook.com>

Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:

> On Fri, Nov 08, 2019 at 09:17:12PM +0100, Toke Høiland-Jørgensen wrote:
>> Alexei Starovoitov <ast@kernel.org> writes:
>> 
>> > Allow FENTRY/FEXIT BPF programs to attach to other BPF programs of any type
>> > including their subprograms. This feature allows snooping on input and output
>> > packets in XDP, TC programs including their return values. In order to do that
>> > the verifier needs to track types not only of vmlinux, but types of other BPF
>> > programs as well. The verifier also needs to translate uapi/linux/bpf.h types
>> > used by networking programs into kernel internal BTF types used by FENTRY/FEXIT
>> > BPF programs. In some cases LLVM optimizations can remove arguments from BPF
>> > subprograms without adjusting BTF info that LLVM backend knows. When BTF info
>> > disagrees with actual types that the verifiers sees the BPF trampoline has to
>> > fallback to conservative and treat all arguments as u64. The FENTRY/FEXIT
>> > program can still attach to such subprograms, but won't be able to recognize
>> > pointer types like 'struct sk_buff *' into won't be able to pass them to
>> > bpf_skb_output() for dumping to user space.
>> >
>> > The BPF_PROG_LOAD command is extended with attach_prog_fd field. When it's set
>> > to zero the attach_btf_id is one vmlinux BTF type ids. When attach_prog_fd
>> > points to previously loaded BPF program the attach_btf_id is BTF type id of
>> > main function or one of its subprograms.
>> >
>> > Signed-off-by: Alexei Starovoitov <ast@kernel.org>
>> 
>> This is cool! Certainly solves the xdpdump use case; thanks!
>> 
>> I do have a few questions (thinking about whether it can also be used
>> for running multiple XDP programs):
>
> excellent questions.
>
>> - Can a FEXIT function loaded this way only *observe* the return code of
>>   the BPF program it attaches to, or can it also change it?
>
> yes. the verifier can be taught to support that for certain class of programs.
> That needs careful thinking to make sure it's safe.

OK. I think this could potentially be useful to have for XDP (for
instance, to have xdpdump "steal" any packets it is observing by
changing the return code to XDP_DROP).

>> - Is it possible to attach multiple FENTRY/FEXIT programs to the same
>>   XDP program 
>
> Yes. Already possible. See fexit_stress.c that attaches 40 progs to the same
> kernel function. Same thing when attaching fexit BPF to any XDP program.
> Since all of them are read only tracing prog all progs have access to skb on
> input and ouput along with unmodified return value.

Right, cool.

>> and/or to recursively attach FENTRY/FEXIT programs to each
>>   other?
>
> Not right now to avoid complex logic of detecting cycles. See simple bit:
>    if (tgt_prog->type == BPF_PROG_TYPE_TRACING) {
>            /* prevent cycles */
>            verbose(env, "Cannot recursively attach\n");

OK, that is probably a reasonable tradeoff.

>> - Could it be possible for an FENTRY/FEXIT program to call into another
>>   XDP program (i.e., one that has the regular XDP program type)?
>
> It's possible to teach verifier to do that, but we probably shouldn't take that
> route. Instead I've started exploring the idea of dynamic linking. The
> trampoline logic will be used to replace existing BPF program or subprogram
> instead of attaching read-only to it. If types match the new program can
> replace existing one. The concept allows to build any kind of callchain
> programmatically. Pretty much what Ed proposed with static linking, but doing
> it dynamically. I'll start a separate email thread explaining details.

SGTM; will wait for the sequel, then :)

-Toke


  reply index

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-11-08  6:40 [PATCH v3 bpf-next 00/18] Introduce BPF trampoline Alexei Starovoitov
2019-11-08  6:40 ` [PATCH v3 bpf-next 01/18] bpf: refactor x86 JIT into helpers Alexei Starovoitov
2019-11-08 19:27   ` Andrii Nakryiko
2019-11-08  6:40 ` [PATCH v3 bpf-next 02/18] bpf: Add bpf_arch_text_poke() helper Alexei Starovoitov
2019-11-08  6:56   ` Song Liu
2019-11-08  8:23   ` Björn Töpel
2019-11-08 14:09     ` Alexei Starovoitov
2019-11-08  9:11   ` Peter Zijlstra
2019-11-08  9:36     ` Peter Zijlstra
2019-11-08 13:41       ` Alexei Starovoitov
2019-11-08 19:32         ` Alexei Starovoitov
2019-11-08 21:36           ` Peter Zijlstra
2019-11-08 21:39             ` David Miller
2019-11-11  8:14               ` Peter Zijlstra
2019-11-11 10:21                 ` Daniel Borkmann
2019-11-11 16:10                 ` Jonathan Corbet
2019-11-08 23:05             ` Alexei Starovoitov
2019-11-10 10:54               ` Thomas Gleixner
2019-11-08  6:40 ` [PATCH v3 bpf-next 03/18] bpf: Introduce BPF trampoline Alexei Starovoitov
2019-11-08  7:04   ` Song Liu
2019-11-08  6:40 ` [PATCH v3 bpf-next 04/18] libbpf: Introduce btf__find_by_name_kind() Alexei Starovoitov
2019-11-08  7:05   ` Song Liu
2019-11-08 19:21   ` Andrii Nakryiko
2019-11-08  6:40 ` [PATCH v3 bpf-next 05/18] libbpf: Add support to attach to fentry/fexit tracing progs Alexei Starovoitov
2019-11-08  7:12   ` Song Liu
2019-11-08 19:44   ` Andrii Nakryiko
2019-11-08  6:40 ` [PATCH v3 bpf-next 06/18] selftest/bpf: Simple test for fentry/fexit Alexei Starovoitov
2019-11-08  6:40 ` [PATCH v3 bpf-next 07/18] bpf: Add kernel test functions for fentry testing Alexei Starovoitov
2019-11-08  6:40 ` [PATCH v3 bpf-next 08/18] selftests/bpf: Add test for BPF trampoline Alexei Starovoitov
2019-11-08  6:40 ` [PATCH v3 bpf-next 09/18] selftests/bpf: Add fexit tests " Alexei Starovoitov
2019-11-08  6:40 ` [PATCH v3 bpf-next 10/18] selftests/bpf: Add combined fentry/fexit test Alexei Starovoitov
2019-11-08  7:14   ` Song Liu
2019-11-08  6:40 ` [PATCH v3 bpf-next 11/18] selftests/bpf: Add stress test for maximum number of progs Alexei Starovoitov
2019-11-08  7:24   ` Song Liu
2019-11-08  6:40 ` [PATCH v3 bpf-next 12/18] bpf: Reserve space for BPF trampoline in BPF programs Alexei Starovoitov
2019-11-08  7:25   ` Song Liu
2019-11-08  6:40 ` [PATCH v3 bpf-next 13/18] bpf: Fix race in btf_resolve_helper_id() Alexei Starovoitov
2019-11-08  7:32   ` Song Liu
2019-11-08 19:58   ` Andrii Nakryiko
2019-11-08  6:40 ` [PATCH v3 bpf-next 14/18] bpf: Compare BTF types of functions arguments with actual types Alexei Starovoitov
2019-11-08 17:28   ` Song Liu
2019-11-08 17:32     ` Song Liu
2019-11-08 17:57       ` Alexei Starovoitov
2019-11-08 17:59         ` Song Liu
2019-11-08 23:46   ` Andrii Nakryiko
2019-11-08  6:40 ` [PATCH v3 bpf-next 15/18] bpf: Support attaching tracing BPF program to other BPF programs Alexei Starovoitov
2019-11-08 18:49   ` Song Liu
2019-11-08 18:59     ` Alexei Starovoitov
2019-11-08 20:17   ` Toke Høiland-Jørgensen
2019-11-08 21:14     ` Alexei Starovoitov
2019-11-08 21:32       ` Toke Høiland-Jørgensen [this message]
2019-11-10  7:17   ` Andrii Nakryiko
2019-11-11 23:04     ` Alexei Starovoitov
2019-11-12  4:38       ` Andrii Nakryiko
2019-11-12  4:47         ` Alexei Starovoitov
2019-11-08  6:40 ` [PATCH v3 bpf-next 16/18] libbpf: Add support for attaching BPF programs " Alexei Starovoitov
2019-11-08 18:57   ` Song Liu
2019-11-08 19:13     ` Alexei Starovoitov
2019-11-08 19:14       ` Song Liu
2019-11-10 16:56   ` Andrii Nakryiko
2019-11-08  6:40 ` [PATCH v3 bpf-next 17/18] selftests/bpf: Extend test_pkt_access test Alexei Starovoitov
2019-11-08 19:03   ` Song Liu
2019-11-10 16:58   ` Andrii Nakryiko
2019-11-08  6:40 ` [PATCH v3 bpf-next 18/18] selftests/bpf: Add a test for attaching BPF prog to another BPF prog and subprog Alexei Starovoitov
2019-11-08 19:13   ` Song Liu
2019-11-10 17:04   ` Andrii Nakryiko
2019-11-11 23:07     ` Alexei Starovoitov

Reply instructions:

You may reply publically 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:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

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

  git send-email \
    --in-reply-to=87mud6caw5.fsf@toke.dk \
    --to=toke@redhat.com \
    --cc=alexei.starovoitov@gmail.com \
    --cc=ast@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=daniel@iogearbox.net \
    --cc=davem@davemloft.net \
    --cc=kernel-team@fb.com \
    --cc=netdev@vger.kernel.org \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

BPF Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/bpf/0 bpf/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 bpf bpf/ https://lore.kernel.org/bpf \
		bpf@vger.kernel.org
	public-inbox-index bpf

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.bpf


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git