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

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

On 2/14/23 18:58, Stanislav Fomichev wrote:
> On 02/14, Kui-Feng Lee 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 | 73 +++++++++++++++++++++++++++++++++---------
>>   1 file changed, 58 insertions(+), 15 deletions(-)
>
>> diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
>> index 75ed95b7e455..1eff6a03ddd9 100644
>> --- a/tools/lib/bpf/libbpf.c
>> +++ b/tools/lib/bpf/libbpf.c
>> @@ -11430,29 +11430,41 @@ struct bpf_link *bpf_program__attach(const 
>> struct bpf_program *prog)
>>       return link;
>>   }
>
>> +struct bpf_link_struct_ops_map {
>> +    struct bpf_link link;
>> +    int map_fd;
>> +};
>
> Ah, ok, now you're adding bpf_link_struct_ops_map. I guess I'm now
> confused why you haven't done it in the first patch :-/

Just won't to mix the libbpf part and kernel part in one patch.


>
> And what are these fake bpf_links? Can you share more about it means?

For the next version, I will detail it in the commit log. In a nutshell, 
before this point, there was no bpf_link for struct_ops. Libbpf 
attempted to create an equivalent interface to other BPF programs by 
providing a simulated bpf_link instead of a true one from the kernel; 
that fake bpf_link stores FDs associated with struct_ops maps rather 
than real bpf_links.


>
>> +
>>   static int bpf_link__detach_struct_ops(struct bpf_link *link)
>>   {
>> +    struct bpf_link_struct_ops_map *st_link;
>>       __u32 zero = 0;
>
>> -    if (bpf_map_delete_elem(link->fd, &zero))
>> +    st_link = container_of(link, struct bpf_link_struct_ops_map, link);
>> +
>> +    if (st_link->map_fd < 0) {
>> +        /* Fake bpf_link */
>> +        if (bpf_map_delete_elem(link->fd, &zero))
>> +            return -errno;
>> +        return 0;
>> +    }
>> +
>> +    if (bpf_map_delete_elem(st_link->map_fd, &zero))
>> +        return -errno;
>> +
>> +    if (close(link->fd))
>>           return -errno;
>
>>       return 0;
>>   }
>
>> -struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
>> +/*
>> + * Update the map with the prepared vdata.
>> + */
>> +static int bpf_map__update_vdata(const struct bpf_map *map)
>>   {
>>       struct bpf_struct_ops *st_ops;
>> -    struct bpf_link *link;
>>       __u32 i, zero = 0;
>> -    int err;
>> -
>> -    if (!bpf_map__is_struct_ops(map) || map->fd == -1)
>> -        return libbpf_err_ptr(-EINVAL);
>> -
>> -    link = calloc(1, sizeof(*link));
>> -    if (!link)
>> -        return libbpf_err_ptr(-EINVAL);
>
>>       st_ops = map->st_ops;
>>       for (i = 0; i < btf_vlen(st_ops->type); i++) {
>> @@ -11468,17 +11480,48 @@ struct bpf_link 
>> *bpf_map__attach_struct_ops(const struct bpf_map *map)
>>           *(unsigned long *)kern_data = prog_fd;
>>       }
>
>> -    err = bpf_map_update_elem(map->fd, &zero, st_ops->kern_vdata, 0);
>> +    return bpf_map_update_elem(map->fd, &zero, st_ops->kern_vdata, 0);
>> +}
>> +
>> +struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
>> +{
>> +    struct bpf_link_struct_ops_map *link;
>> +    int err, fd;
>> +
>> +    if (!bpf_map__is_struct_ops(map) || map->fd == -1)
>> +        return libbpf_err_ptr(-EINVAL);
>> +
>> +    link = calloc(1, sizeof(*link));
>> +    if (!link)
>> +        return libbpf_err_ptr(-EINVAL);
>> +
>> +    err = bpf_map__update_vdata(map);
>>       if (err) {
>>           err = -errno;
>>           free(link);
>>           return libbpf_err_ptr(err);
>>       }
>
>> -    link->detach = bpf_link__detach_struct_ops;
>> -    link->fd = map->fd;
>> +    link->link.detach = bpf_link__detach_struct_ops;
>
>> -    return link;
>> +    if (!(map->def.map_flags & BPF_F_LINK)) {
>> +        /* Fake bpf_link */
>> +        link->link.fd = map->fd;
>> +        link->map_fd = -1;
>> +        return &link->link;
>> +    }
>> +
>> +    fd = bpf_link_create(map->fd, -1, BPF_STRUCT_OPS_MAP, NULL);
>> +    if (fd < 0) {
>> +        err = -errno;
>> +        free(link);
>> +        return libbpf_err_ptr(err);
>> +    }
>> +
>> +    link->link.fd = fd;
>> +    link->map_fd = map->fd;
>> +
>> +    return &link->link;
>>   }
>
>>   typedef enum bpf_perf_event_ret (*bpf_perf_event_print_t)(struct 
>> perf_event_header *hdr,
>> -- 
>> 2.30.2
>