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

[Tao Chen <chen.dylane@linux.dev>]

在 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