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

Commit Message

Kui-Feng Lee April 18, 2024, 4:35 p.m. UTC

Add the "open" operator for the inodes of BPF links to allow applications
to obtain a file descriptor of a struct_ops link from a pinned path.

Applications have the ability to update a struct_ops link with another
struct_ops map. However, they were unable to open pinned paths of the links
with this patch. This implies that updating a link through its pinned paths
was not feasible.

This patch adds the "open" operator to bpf_link_ops and uses bpf_link_ops
as the i_fop for inodes of struct_ops links. "open" will be called to open
the pinned path represented by an inode. Additionally, bpf_link_ops will be
used as the f->f_ops of the opened "file" to provide operators for the
"file".

Signed-off-by: Kui-Feng Lee <[e-mail]>
---
 include/linux/bpf.h         |  6 ++++++
 kernel/bpf/bpf_struct_ops.c | 10 ++++++++++
 kernel/bpf/inode.c          | 11 ++++++++---
 kernel/bpf/syscall.c        | 16 +++++++++++++++-
 4 files changed, 39 insertions(+), 4 deletions(-)

Comments

Andrii Nakryiko April 25, 2024, 12:09 a.m. UTC | #1

On Thu, Apr 18, 2024 at 9:35 AM Kui-Feng Lee <[e-mail]> wrote:
>
> Add the "open" operator for the inodes of BPF links to allow applications
> to obtain a file descriptor of a struct_ops link from a pinned path.
>
> Applications have the ability to update a struct_ops link with another
> struct_ops map. However, they were unable to open pinned paths of the links
> with this patch. This implies that updating a link through its pinned paths
> was not feasible.
>
> This patch adds the "open" operator to bpf_link_ops and uses bpf_link_ops
> as the i_fop for inodes of struct_ops links. "open" will be called to open
> the pinned path represented by an inode. Additionally, bpf_link_ops will be
> used as the f->f_ops of the opened "file" to provide operators for the
> "file".
>
> Signed-off-by: Kui-Feng Lee <[e-mail]>
> ---
>  include/linux/bpf.h         |  6 ++++++
>  kernel/bpf/bpf_struct_ops.c | 10 ++++++++++
>  kernel/bpf/inode.c          | 11 ++++++++---
>  kernel/bpf/syscall.c        | 16 +++++++++++++++-
>  4 files changed, 39 insertions(+), 4 deletions(-)
>

This is already supported, but you don't do it with open() syscall.
bpf() syscall provides BPF_OBJ_GET as a counterpart to BPF_OBJ_PIN. So
what you/your users want to do should be already supported through
libbpf's bpf_obj_get()/bpf_obj_get_ops() APIs. Have you tried that?

> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 5034c1b4ded7..a0c0234d754b 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -2160,6 +2160,12 @@ extern const struct super_operations bpf_super_ops;
>  extern const struct file_operations bpf_map_fops;
>  extern const struct file_operations bpf_prog_fops;
>  extern const struct file_operations bpf_iter_fops;
> +extern const struct file_operations bpf_link_fops;
> +

[...]

Patch

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 5034c1b4ded7..a0c0234d754b 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -2160,6 +2160,12 @@  extern const struct super_operations bpf_super_ops;
 extern const struct file_operations bpf_map_fops;
 extern const struct file_operations bpf_prog_fops;
 extern const struct file_operations bpf_iter_fops;
+extern const struct file_operations bpf_link_fops;
+
+#ifdef CONFIG_BPF_JIT
+/* Required by bpf_link_open() */
+int bpffs_struct_ops_link_open(struct inode *inode, struct file *filp);
+#endif
 
 #define BPF_PROG_TYPE(_id, _name, prog_ctx_type, kern_ctx_type) \
 	extern const struct bpf_prog_ops _name ## _prog_ops; \
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 86c7884abaf8..8be4f755a182 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -1198,3 +1198,13 @@  void bpf_map_struct_ops_info_fill(struct bpf_map_info *info, struct bpf_map *map
 
 	info->btf_vmlinux_id = btf_obj_id(st_map->btf);
 }
+
+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)
+		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..265e2faf317d 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -3108,7 +3108,21 @@  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)
+{
+#ifdef CONFIG_BPF_JIT
+	struct bpf_link *link = inode->i_private;
+
+	if (link->type == BPF_LINK_TYPE_STRUCT_OPS)
+		return bpffs_struct_ops_link_open(inode, filp);
+#endif
+
+	return -EOPNOTSUPP;
+}
+
+const struct file_operations bpf_link_fops = {
+	.open = bpf_link_open,
 #ifdef CONFIG_PROC_FS
 	.show_fdinfo	= bpf_link_show_fdinfo,
 #endif