Re: [RFC PATCH bpf-next 1/5] bpf: Add link support for sk_msg prog

[Yonghong Song <yonghong.song@linux.dev>]

On 3/11/24 1:30 AM, Jiri Olsa wrote:
> On Tue, Mar 05, 2024 at 12:22:00PM -0800, Yonghong Song wrote:
>
> SNIP
>
>> +
>> +int bpf_skmsg_link_create(const union bpf_attr *attr, struct bpf_prog *prog)
>> +{
>> +	struct bpf_link_primer link_primer;
>> +	struct bpf_skmsg_link *skmsg_link;
>> +	enum bpf_attach_type attach_type;
>> +	struct bpf_map *map;
>> +	int ret;
>> +
>> +	if (attr->link_create.flags)
>> +		return -EINVAL;
>> +
>> +	map = bpf_map_get_with_uref(attr->link_create.target_fd);
>> +	if (IS_ERR(map))
>> +		return PTR_ERR(map);
>> +
>> +	skmsg_link = kzalloc(sizeof(*skmsg_link), GFP_USER);
>> +	if (!skmsg_link) {
>> +		ret = -ENOMEM;
>> +		goto out;
>> +	}
>> +
>> +	attach_type = attr->link_create.attach_type;
>> +	bpf_link_init(&skmsg_link->link, BPF_LINK_TYPE_SK_MSG, &bpf_skmsg_link_ops, prog);
>> +	skmsg_link->map = map;
>> +	skmsg_link->attach_type = attach_type;
>> +
>> +	ret = bpf_link_prime(&skmsg_link->link, &link_primer);
>> +	if (ret) {
>> +		kfree(skmsg_link);
>> +		goto out;
>> +	}
>> +
>> +	ret = sock_map_prog_update(map, prog, NULL, attach_type);
>> +	if (ret) {
>> +		bpf_link_cleanup(&link_primer);
>> +		goto out;
>> +	}
>> +
>> +	bpf_prog_inc(prog);
> there's already prog ref taken in link_create, is this needed?
> also I might be missing some skmsg logic, but I can't see thi
> being released

Here, we are trying to do create/attach.
The prog reference count will be decremented during detach
(sock_map_prog_detach), so we need to increase prog ref count here.

Another thing, it is possible a attached prog is swapped out.
In current implementation, ref count will be decremented for
the prog swapped out. So increasing ref count for prog here
is needed to balance that ref count decrement.

>
> jirka
>
>> +
>> +	return bpf_link_settle(&link_primer);
>> +
>> +out:
>> +	bpf_map_put_with_uref(map);
>> +	return ret;
>> +}
[...]