Re: bpf: ability to attach freplace to multiple parents

From: Alexei Starovoitov <alexei.starovoitov@gmail.com>
To: Alan Maguire <alan.maguire@oracle.com>
Cc: "Toke Høiland-Jørgensen" <toke@redhat.com>,
	"Daniel Borkmann" <daniel@iogearbox.net>,
	"Andrii Nakryiko" <andriin@fb.com>,
	"David S. Miller" <davem@davemloft.net>,
	"Andrey Ignatov" <rdna@fb.com>,
	Networking <netdev@vger.kernel.org>, bpf <bpf@vger.kernel.org>
Subject: Re: bpf: ability to attach freplace to multiple parents
Date: Tue, 12 May 2020 16:18:34 -0700	[thread overview]
Message-ID: <20200512231834.sbyhb3lmelw43z7v@ast-mbp> (raw)
In-Reply-To: <alpine.LRH.2.21.2005121009220.22093@localhost>

On Tue, May 12, 2020 at 10:53:38AM +0100, Alan Maguire wrote:
> On Tue, 12 May 2020, Toke Høiland-Jørgensen wrote:
> 
> > Alexei Starovoitov <alexei.starovoitov@gmail.com> writes:
> > 
> > >> > Currently fentry/fexit/freplace progs have single prog->aux->linked_prog pointer.
> > >> > It just needs to become a linked list.
> > >> > The api extension could be like this:
> > >> > bpf_raw_tp_open(prog_fd, attach_prog_fd, attach_btf_id);
> > >> > (currently it's just bpf_raw_tp_open(prog_fd))
> > >> > The same pair of (attach_prog_fd, attach_btf_id) is already passed into prog_load
> > >> > to hold the linked_prog and its corresponding btf_id.
> > >> > I'm proposing to extend raw_tp_open with this pair as well to
> > >> > attach existing fentry/fexit/freplace prog to another target.
> > >> > Internally the kernel verify that btf of current linked_prog
> > >> > exactly matches to btf of another requested linked_prog and
> > >> > if they match it will attach the same prog to two target programs (in case of freplace)
> > >> > or two kernel functions (in case of fentry/fexit).
> > >> 
> > >> API-wise this was exactly what I had in mind as well.
> > >
> > > perfect!
> >
> 
> Apologies in advance if I've missed a way to do this, but
> for fentry/fexit, if we could attach the same program to
> multiple kernel functions, it would be great it we could
> programmatically access the BTF func proto id for the
> attached function (prog->aux->attach_btf_id I think?).
> Then perhaps we could support access to that and associated
> ids via a helper, roughly like:
> 
> s32 btf_attach_info(enum btf_info_wanted wanted,
> 		    struct __btf_ptr *ptr,__u64 flags);
> 
> The info_wanted would be BTF_INFO_FUNC_PROTO, BTF_INFO_RET_TYPE,
> BTF_INFO_NARGS, BTF_INFO_ARG1, etc.
> 
> With that and the BTF-based printk support in hand, we could
> potentially use bpf_trace_printk() to print function arguments
> in an attach-point agnostic way.  The BTF printk v2 patchset has
> support for BTF id-based display (it's not currently used in that
> patchset though). We'd have to teach it to print BTF func protos
> but that's not too tricky I think. An ftrace-like program that
> would print out function prototypes for the attached function
> would look something like this:
> 
> 	struct __btf_ptr func = { 0 };
> 	btf_attach_info(BTF_INFO_FUNC_PROTO, &func, 0); 
> 	btf_printk("%pT", &func);

Currently fentry/fexit cannot be attached to multiple kernel funcs
and no one is working on it.
Attaching freplace to multiple bpf progs with the same signature
is a different use case. I've started on it, but priority went down.

In general it's possible to make the same fentry prog run on multiple
kernel funcs, but I don't see a use case yet, since bpf side won't
be able to see any arguments. It will be less useful than kprobe+bpf.
That prog at least has struct pr_regs to examine.
So I'm not sure what you're trying to achieve.
If you just want to print kernel function arguments than invent
a new helper for bpf kprobe progs. Like bpf_printk_current_func(pt_regs);
Since it's all dynamic that helper would need to resolve regs->IP
into string than search vmlinux btf for that function and then
print regs->DI,SI according to btf func proto.