Re: [PATCH bpf-next v8 4/5] libbpf: Init kprobe prog expected_attach_type for kfunc probe

[Jiri Olsa <olsajiri@gmail.com>]

On Tue, Feb 25, 2025 at 09:04:58AM -0800, Andrii Nakryiko wrote:
> On Mon, Feb 24, 2025 at 9:44 PM Tao Chen <chen.dylane@linux.dev> wrote:
> >
> > 在 2025/2/25 09:15, Andrii Nakryiko 写道:
> > > On Mon, Feb 24, 2025 at 9:03 AM Tao Chen <chen.dylane@linux.dev> wrote:
> > >>
> > >> Kprobe prog type kfuncs like bpf_session_is_return and
> > >> bpf_session_cookie will check the expected_attach_type,
> > >> so init the expected_attach_type here.
> > >>
> > >> Signed-off-by: Tao Chen <chen.dylane@linux.dev>
> > >> ---
> > >>   tools/lib/bpf/libbpf_probes.c | 1 +
> > >>   1 file changed, 1 insertion(+)
> > >>
> > >> diff --git a/tools/lib/bpf/libbpf_probes.c b/tools/lib/bpf/libbpf_probes.c
> > >> index 8efebc18a215..bb5b457ddc80 100644
> > >> --- a/tools/lib/bpf/libbpf_probes.c
> > >> +++ b/tools/lib/bpf/libbpf_probes.c
> > >> @@ -126,6 +126,7 @@ static int probe_prog_load(enum bpf_prog_type prog_type,
> > >>                  break;
> > >>          case BPF_PROG_TYPE_KPROBE:
> > >>                  opts.kern_version = get_kernel_version();
> > >> +               opts.expected_attach_type = BPF_TRACE_KPROBE_SESSION;
> > >
> > > so KPROBE_SESSION is relative recent feature, if we unconditionally
> > > specify this, we'll regress some feature probes for old kernels where
> > > KPROBE_SESSION isn't supported, no?
> > >
> >
> > Yeah, maybe we can detect the kernel version first, will fix it.
> 
> Hold on. I think the entire probing API is kind of unfortunately
> inadequate. Just the fact that we randomly pick some specific
> expected_attach_type to do helpers/kfunc compatibility detection is
> telling. expected_attach_type can change a set of available helpers,
> and yet it's not even an input parameter for either
> libbpf_probe_bpf_helper() or kfunc variant you are trying to add.

could we use the libbpf_probe_bpf_kfunc opts argument and
allow to specify and override expected_attach_type?

jirka

> 
> Basically, I'm questioning the validity of even adding this API to
> libbpf. It feels like this kind of detection is simple enough for
> application to do on its own.
> 
> >
> > +               if (opts.kern_version >= KERNEL_VERSION(6, 12, 0))
> > +                       opts.expected_attach_type =BPF_TRACE_KPROBE_SESSION;
> 
> no, we shouldn't hard-code kernel version for feature detection (but
> also see above, I'm not sure this API should be added in the first
> place)
> 
> >
> > > pw-bot: cr
> > >
> > >>                  break;
> > >>          case BPF_PROG_TYPE_LIRC_MODE2:
> > >>                  opts.expected_attach_type = BPF_LIRC_MODE2;
> > >> --
> > >> 2.43.0
> > >>
> >
> >
> > --
> > Best Regards
> > Tao Chen