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

[Martin KaFai Lau <martin.lau@xxxxxxxxx>]

On 3/7/23 4:50 PM, Kui-Feng Lee wrote:
> @@ -11566,22 +11591,34 @@ struct bpf_link *bpf_program__attach(const struct bpf_program *prog)
>   	return link;
>   }
>   
> +struct bpf_link_struct_ops {
> +	struct bpf_link link;
> +	int map_fd;
> +};
> +
>   static int bpf_link__detach_struct_ops(struct bpf_link *link)
>   {
> +	struct bpf_link_struct_ops *st_link;
>   	__u32 zero = 0;
>   
> -	if (bpf_map_delete_elem(link->fd, &zero))
> -		return -errno;
> +	st_link = container_of(link, struct bpf_link_struct_ops, link);
>   
> -	return 0;
> +	if (st_link->map_fd < 0) {

map_fd < 0 should always be true?

> +		/* Fake bpf_link */
> +		if (bpf_map_delete_elem(link->fd, &zero))
> +			return -errno;
> +		return 0;
> +	}
> +
> +	/* Doesn't support detaching. */
> +	return -EOPNOTSUPP;
>   }
>   
>   struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
>   {
> -	struct bpf_struct_ops *st_ops;
> -	struct bpf_link *link;
> -	__u32 i, zero = 0;
> -	int err;
> +	struct bpf_link_struct_ops *link;
> +	__u32 zero = 0;
> +	int err, fd;
>   
>   	if (!bpf_map__is_struct_ops(map) || map->fd == -1)
>   		return libbpf_err_ptr(-EINVAL);
> @@ -11590,31 +11627,34 @@ struct bpf_link *bpf_map__attach_struct_ops(const struct bpf_map *map)
>   	if (!link)
>   		return libbpf_err_ptr(-EINVAL);
>   
> -	st_ops = map->st_ops;
> -	for (i = 0; i < btf_vlen(st_ops->type); i++) {
> -		struct bpf_program *prog = st_ops->progs[i];
> -		void *kern_data;
> -		int prog_fd;
> +	/* kern_vdata should be prepared during the loading phase. */
> +	err = bpf_map_update_elem(map->fd, &zero, map->st_ops->kern_vdata, 0);
> +	if (err) {
> +		err = -errno;
> +		free(link);
> +		return libbpf_err_ptr(err);
> +	}
>   
> -		if (!prog)
> -			continue;
>   
> -		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)) {
> +		/* Fake bpf_link */
> +		link->link.fd = map->fd;
> +		link->map_fd = -1;
> +		link->link.detach = bpf_link__detach_struct_ops;
> +		return &link->link;
>   	}
>   
> -	err = bpf_map_update_elem(map->fd, &zero, st_ops->kern_vdata, 0);
> -	if (err) {
> +	fd = bpf_link_create(map->fd, -1, BPF_STRUCT_OPS, NULL);
> +	if (fd < 0) {
>   		err = -errno;
>   		free(link);
>   		return libbpf_err_ptr(err);
>   	}
>   
> -	link->detach = bpf_link__detach_struct_ops;
> -	link->fd = map->fd;
> +	link->link.fd = fd;
> +	link->map_fd = map->fd;

Does it need to set link->link.detach?

>   
> -	return link;
> +	return &link->link;
>   }
>   
>   typedef enum bpf_perf_event_ret (*bpf_perf_event_print_t)(struct perf_event_header *hdr,