[v8] Add prog_kfunc feature probe

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

Posted by Tao Chen 9 months, 3 weeks ago

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;
 		break;
 	case BPF_PROG_TYPE_LIRC_MODE2:
 		opts.expected_attach_type = BPF_LIRC_MODE2;
-- 
2.43.0

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

Posted by Andrii Nakryiko 9 months, 3 weeks ago

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?

pw-bot: cr

>                 break;
>         case BPF_PROG_TYPE_LIRC_MODE2:
>                 opts.expected_attach_type = BPF_LIRC_MODE2;
> --
> 2.43.0
>

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

Posted by Tao Chen 9 months, 3 weeks ago

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

+               if (opts.kern_version >= KERNEL_VERSION(6, 12, 0))
+                       opts.expected_attach_type =BPF_TRACE_KPROBE_SESSION;

> pw-bot: cr
> 
>>                  break;
>>          case BPF_PROG_TYPE_LIRC_MODE2:
>>                  opts.expected_attach_type = BPF_LIRC_MODE2;
>> --
>> 2.43.0
>>


-- 
Best Regards
Tao Chen

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

Posted by Andrii Nakryiko 9 months, 3 weeks ago

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.

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

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

Posted by Jiri Olsa 9 months, 3 weeks ago

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

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

Posted by Tao Chen 9 months, 3 weeks ago

在 2025/2/26 19:12, Jiri Olsa 写道:
> 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
> 

It looks great, btw, these probe apis already used in bpftool feature
function, so maybe we can continue to improve it including the 
libbpf_probe_bpf_helper as andrii said.

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


-- 
Best Regards
Tao Chen