Re: [PATCH bpf-next 1/2] bpf: enable the "open" operator on a pinned path of a struct_osp link.

[Martin KaFai Lau <martin.lau@linux.dev>]

On 4/22/24 10:30 AM, Kui-Feng Lee wrote:
> 
> 
> On 4/22/24 10:12, Kui-Feng Lee wrote:
>>
>>
>> On 4/19/24 17:05, Martin KaFai Lau wrote:
>>> On 4/16/24 5:25 PM, Kui-Feng Lee wrote:
>>>> +int bpffs_struct_ops_link_open(struct inode *inode, struct file *filp)
>>>> +{
>>>> +    struct bpf_struct_ops_link *link = inode->i_private;
>>>> +
>>>> +    /* Paired with bpf_link_put_direct() in bpf_link_release(). */
>>>> +    bpf_link_inc(&link->link);
>>>> +    filp->private_data = link;
>>>> +    return 0;
>>>> +}
>>>> diff --git a/kernel/bpf/inode.c b/kernel/bpf/inode.c
>>>> index af5d2ffadd70..b020d761ab0a 100644
>>>> --- a/kernel/bpf/inode.c
>>>> +++ b/kernel/bpf/inode.c
>>>> @@ -360,11 +360,16 @@ static int bpf_mkmap(struct dentry *dentry, umode_t 
>>>> mode, void *arg)
>>>>   static int bpf_mklink(struct dentry *dentry, umode_t mode, void *arg)
>>>>   {
>>>> +    const struct file_operations *fops;
>>>>       struct bpf_link *link = arg;
>>>> -    return bpf_mkobj_ops(dentry, mode, arg, &bpf_link_iops,
>>>> -                 bpf_link_is_iter(link) ?
>>>> -                 &bpf_iter_fops : &bpffs_obj_fops);
>>>> +    if (bpf_link_is_iter(link))
>>>> +        fops = &bpf_iter_fops;
>>>> +    else if (link->type == BPF_LINK_TYPE_STRUCT_OPS)
>>>
>>> Open a pinned link and then update should not be specific to struct_ops link. 
>>> e.g. should be useful to the cgroup link also?
>>
>> It could be. Here, I played safe in case it creates any unwanted side
>> effect for links of unknown types.
> 
> By the way, may I put it in a follow up patch if we want cgroup links?

This does not feel right. It is not struct_ops specific.

Before we dive in further, there is BPF_OBJ_GET which can get a fd of a pinned 
bpf obj (prog, map, and link). Take a look at bpf_link__open() in libbpf. Does 
it work for the use case that needs to update the link?