Subject: Re: [PATCH bpf-next 1/3] bpf: add bpf_progenyof helper

From: Javier Honduvilla Coto <[email-masked]>
Date: Fri, 1 Mar 2019 17:43:42 +0000

On Wed, Feb 27, 2019 at 06:26:41AM +0000, Martin Lau wrote:

Sorry, accidentally did not reply to all your comments:
> On Tue, Feb 26, 2019 at 02:36:49PM -0800, Javier Honduvilla Coto wrote:
> > This patch adds the bpf_progenyof helper which receives a PID and returns
> What is progenof?

progenyof is a helper we'd love to have in thr kernel as right now
checking if the current process pid is among the progeny of some
arbitrary pid has several disadvantages. There are more details in the
first commit message

>
> > 1 if the process currently being executed is in the process hierarchy
> > including itself or 0 if not.
> >
> > This is very useful in tracing programs when we want to filter by a
> > given PID and all the children it might spawn. The current workarounds
> > most people implement for this purpose have issues:
> >
> > - Attaching to process spawning syscalls and dynamically add those PIDs
> >   to some bpf map that would be used to filter is cumbersome and
> > potentially racy.
> > - Unrolling some loop to perform what this helper is doing consumes lots
> >   of instructions. That and the impossibility to jump backwards makes it
> > really hard to be correct in really large process chains.
> >
> > Signed-off-by: Javier Honduvilla Coto <[email-masked]>
> > ---
> >  include/linux/bpf.h      |  1 +
> >  include/uapi/linux/bpf.h |  3 ++-
> >  kernel/bpf/core.c        |  1 +
> >  kernel/bpf/helpers.c     | 29 +++++++++++++++++++++++++++++
> >  kernel/trace/bpf_trace.c |  2 ++
> >  5 files changed, 35 insertions(+), 1 deletion(-)
> >
> > diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> > index de18227b3d95..447395ba202b 100644
> > --- a/include/linux/bpf.h
> > +++ b/include/linux/bpf.h
> > @@ -921,6 +921,7 @@ extern const struct bpf_func_proto bpf_sk_redirect_map_proto;
> >  extern const struct bpf_func_proto bpf_spin_lock_proto;
> >  extern const struct bpf_func_proto bpf_spin_unlock_proto;
> >  extern const struct bpf_func_proto bpf_get_local_storage_proto;
> > +extern const struct bpf_func_proto bpf_progenyof_proto;
> It seems only used in bpf_trace.c.  Does it have to be here?

Yes, we need it in bpf_trace.c where we are requiring it