Re: [PATCH bpf-next v7 4/8] libbpf: Create a bpf_link in bpf_map__attach_struct_ops().

[Kui-Feng Lee <sinquersw@gmail.com>]

On 3/17/23 15:23, Andrii Nakryiko wrote:
> On Wed, Mar 15, 2023 at 7:37 PM Kui-Feng Lee <kuifeng@meta.com> wrote:
>>
>> bpf_map__attach_struct_ops() was creating a dummy bpf_link as a
>> placeholder, but now it is constructing an authentic one by calling
>> bpf_link_create() if the map has the BPF_F_LINK flag.
>>
>> You can flag a struct_ops map with BPF_F_LINK by calling
>> bpf_map__set_map_flags().
>>
>> Signed-off-by: Kui-Feng Lee <kuifeng@meta.com>
>> ---
>>   tools/lib/bpf/libbpf.c | 90 +++++++++++++++++++++++++++++++-----------
>>   1 file changed, 66 insertions(+), 24 deletions(-)
>>
> 
> [...]
> 
>> -               if (!prog)
>> -                       continue;
>> +       link->link.detach = bpf_link__detach_struct_ops;
>>
>> -               prog_fd = bpf_program__fd(prog);
>> -               kern_data = st_ops->kern_vdata + st_ops->kern_func_off[i];
>> -               *(unsigned long *)kern_data = prog_fd;
>> +       if (!(map->def.map_flags & BPF_F_LINK)) {
>> +               /* w/o a real link */
>> +               link->link.fd = map->fd;
>> +               link->map_fd = -1;
>> +               return &link->link;
>>          }
>>
>> -       err = bpf_map_update_elem(map->fd, &zero, st_ops->kern_vdata, 0);
>> -       if (err) {
>> -               err = -errno;
>> +       fd = bpf_link_create(map->fd, -1, BPF_STRUCT_OPS, NULL);
> 
> pass 0, not -1. BPF APIs have a convention that fd=0 means "no fd was
> provided". And actually kernel should have rejected this -1, so please
> check why that didn't happen in your testing, we might be missing some
> kernel validation.

Oh! probe_perf_link() also pass -1 as well.
I will fix it.

> 
> 
>> +       if (fd < 0) {
>>                  free(link);
>> -               return libbpf_err_ptr(err);
>> +               return libbpf_err_ptr(fd);
>>          }
>>
>> -       link->detach = bpf_link__detach_struct_ops;
>> -       link->fd = map->fd;
>> +       link->link.fd = fd;
>> +       link->map_fd = map->fd;
>>
>> -       return link;
>> +       return &link->link;
>>   }
>>
>>   typedef enum bpf_perf_event_ret (*bpf_perf_event_print_t)(struct perf_event_header *hdr,
>> --
>> 2.34.1
>>