Message-ID: <20191108211400.m6kuuyvkp2p56gmo@ast-mbp.dhcp.thefacebook.com>
Date:   Fri, 8 Nov 2019 13:14:02 -0800
From:   Alexei Starovoitov <alexei.starovoitov@...il.com>
To:     Toke Høiland-Jørgensen <toke@...hat.com>
Cc:     Alexei Starovoitov <ast@...nel.org>, davem@...emloft.net,
        daniel@...earbox.net, x86@...nel.org, netdev@...r.kernel.org,
        bpf@...r.kernel.org, kernel-team@...com
Subject: Re: [PATCH v3 bpf-next 15/18] bpf: Support attaching tracing BPF
 program to other BPF programs

On Fri, Nov 08, 2019 at 09:17:12PM +0100, Toke Høiland-Jørgensen wrote:
> Alexei Starovoitov <ast@...nel.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@...nel.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.

> - 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.

> 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");

> - 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.

Powered by blists - more mailing lists