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

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

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?

> 
>>
>> Andrii, wdyt about supporting other link types also?
>>
>>> +        fops = &bpf_link_fops;
>>> +    else
>>> +        fops = &bpffs_obj_fops;
>>> +    return bpf_mkobj_ops(dentry, mode, arg, &bpf_link_iops, fops);
>>>   }
>>>   static struct dentry *
>>> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
>>> index 7d392ec83655..f66bc6215faa 100644
>>> --- a/kernel/bpf/syscall.c
>>> +++ b/kernel/bpf/syscall.c
>>> @@ -3108,7 +3108,19 @@ static void bpf_link_show_fdinfo(struct 
>>> seq_file *m, struct file *filp)
>>>   }
>>>   #endif
>>> -static const struct file_operations bpf_link_fops = {
>>> +/* Support opening pinned links */
>>> +static int bpf_link_open(struct inode *inode, struct file *filp)
>>> +{
>>> +    struct bpf_link *link = inode->i_private;
>>> +
>>> +    if (link->type == BPF_LINK_TYPE_STRUCT_OPS)
>>> +        return bpffs_struct_ops_link_open(inode, filp);
>>> +
>>> +    return -EOPNOTSUPP;
>>> +}
>>> +
>>> +const struct file_operations bpf_link_fops = {
>>> +    .open = bpf_link_open,
>>>   #ifdef CONFIG_PROC_FS
>>>       .show_fdinfo    = bpf_link_show_fdinfo,
>>>   #endif
>>