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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/24/24 17:17, Andrii Nakryiko wrote:
On Tue, Apr 23, 2024 at 5:29 PM Martin KaFai Lau <martin.lau@xxxxxxxxx> wrote:

On 4/23/24 10:16 AM, Kui-Feng Lee wrote:


On 4/22/24 16:43, Martin KaFai Lau wrote:
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?

It should work.
So, this patch is not necessary. However, it is still a nice and
intuitive feature. WDYT?

There is already BPF_OBJ_GET which works for all major bpf obj types (prog, map,
and link). Having open only works for link and only works for one link type
(struct_ops) is not very convincing.

Beside, I am not sure how the file flags (e.g. rdonly...etc) should be handled.
I don't know enough in this area, so I will defer to others to comment in
general the usefulness and the approach.



Didn't see this discussion before replying on the other patch. But I
agree with Martin, we already use the BPF_OBJ_GET method, and we don't
support directly open()'ing progs/maps, so I don't think we should do
this for links either.

pw-bot: cr

Understand! Let's drop this patch.




[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux