Registrating struct_ops types from modules

From: Kui-Feng Lee <[email protected]>

To ensure that a module remains accessible whenever a struct_ops object of a struct_ops type provided by the module is still in use.

Signed-off-by Kui-Feng Lee <[email protected]>

---
 include/linux/bpf.h         |  1 +
 include/linux/btf.h         |  2 +-
 kernel/bpf/bpf_struct_ops.c | 70 ++++++++++++++++++++++++++++++-------
 3 files changed, 60 insertions(+), 13 deletions(-)

include/linux/bpf.h +1 -0

index 4f3b67932ded..26feb8a2da4f 100644
@@ -1626,6 +1626,7 @@ struct bpf_struct_ops {
 	void (*unreg)(void *kdata);
 	int (*update)(void *kdata, void *old_kdata);
 	int (*validate)(void *kdata);
+	struct module *owner;
 	const char *name;
 	struct btf_func_model func_models[BPF_STRUCT_OPS_MAX_NR_MEMBERS];
 };

include/linux/btf.h +1 -1

index 8e37f7eb02c7..6a64b372b7a0 100644
@@ -575,7 +575,7 @@ struct bpf_struct_ops;
 struct bpf_struct_ops_desc;
 
 struct bpf_struct_ops_desc *
-btf_add_struct_ops(struct bpf_struct_ops *st_ops);
+btf_add_struct_ops(struct btf *btf, struct bpf_struct_ops *st_ops);
 const struct bpf_struct_ops_desc *
 btf_get_struct_ops(struct btf *btf, u32 *ret_cnt);
 

kernel/bpf/bpf_struct_ops.c +1 -0

index 0bc21a39257d..413a3f8b26ba 100644
@@ -388,6 +388,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
 	const struct btf_member *member;
 	const struct btf_type *t = st_ops_desc->type;
 	struct bpf_tramp_links *tlinks;
+	struct module *mod = NULL;
 	void *udata, *kdata;
 	int prog_fd, err;
 	void *image, *image_end;

kernel/bpf/bpf_struct_ops.c +53 -12

+			goto unlock;
+		}
+	}
+
 	memcpy(uvalue, value, map->value_size);
 
 	udata = &uvalue->data;

@@ -552,6 +561,10 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
 		 * can be seen once BPF_STRUCT_OPS_STATE_INUSE is set.
 		 */
 		smp_store_release(&kvalue->state, BPF_STRUCT_OPS_STATE_INUSE);
+		/* Hold the owner module until the struct_ops is
+		 * unregistered
+		 */
+		mod = NULL;
 		goto unlock;
 	}
 

@@ -568,6 +581,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
 	memset(uvalue, 0, map->value_size);
 	memset(kvalue, 0, map->value_size);
 unlock:
+	module_put(mod);
 	kfree(tlinks);
 	mutex_unlock(&st_map->lock);
 	return err;

@@ -588,6 +602,7 @@ static long bpf_struct_ops_map_delete_elem(struct bpf_map *map, void *key)
 	switch (prev_state) {
 	case BPF_STRUCT_OPS_STATE_INUSE:
 		st_map->st_ops_desc->st_ops->unreg(&st_map->kvalue.data);
+		module_put(st_map->st_ops_desc->st_ops->owner);
 		bpf_map_put(map);
 		return 0;
 	case BPF_STRUCT_OPS_STATE_TOBEFREE:

@@ -674,6 +689,7 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
 	size_t st_map_size;
 	struct bpf_struct_ops_map *st_map;
 	const struct btf_type *t, *vt;
+	struct module *mod = NULL;
 	struct bpf_map *map;
 	int ret;
 

@@ -681,9 +697,17 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
 	if (!st_ops_desc)
 		return ERR_PTR(-ENOTSUPP);
 
+	if (st_ops_desc->btf != btf_vmlinux) {
+		mod = btf_try_get_module(st_ops_desc->btf);
+		if (!mod)
+			return ERR_PTR(-EINVAL);
+	}
+
 	vt = st_ops_desc->value_type;
-	if (attr->value_size != vt->size)
-		return ERR_PTR(-EINVAL);
+	if (attr->value_size != vt->size) {
+		ret = -EINVAL;
+		goto errout;
+	}
 
 	t = st_ops_desc->type;
 

@@ -694,17 +718,17 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
 		(vt->size - sizeof(struct bpf_struct_ops_value));
 
 	st_map = bpf_map_area_alloc(st_map_size, NUMA_NO_NODE);
-	if (!st_map)
-		return ERR_PTR(-ENOMEM);
+	if (!st_map) {
+		ret = -ENOMEM;
+		goto errout;
+	}
 
 	st_map->st_ops_desc = st_ops_desc;
 	map = &st_map->map;
 
 	ret = bpf_jit_charge_modmem(PAGE_SIZE);
-	if (ret) {
-		__bpf_struct_ops_map_free(map);
-		return ERR_PTR(ret);
-	}
+	if (ret)
+		goto errout_free;
 
 	st_map->image = bpf_jit_alloc_exec(PAGE_SIZE);
 	if (!st_map->image) {

@@ -713,23 +737,32 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
 		 * here.
 		 */
 		bpf_jit_uncharge_modmem(PAGE_SIZE);
-		__bpf_struct_ops_map_free(map);
-		return ERR_PTR(-ENOMEM);
+		ret = -ENOMEM;
+		goto errout_free;
 	}
 	st_map->uvalue = bpf_map_area_alloc(vt->size, NUMA_NO_NODE);
 	st_map->links =
 		bpf_map_area_alloc(btf_type_vlen(t) * sizeof(struct bpf_links *),
 				   NUMA_NO_NODE);
 	if (!st_map->uvalue || !st_map->links) {
-		__bpf_struct_ops_map_free(map);
-		return ERR_PTR(-ENOMEM);
+		ret = -ENOMEM;
+		goto errout_free;
 	}
 
 	mutex_init(&st_map->lock);
 	set_vm_flush_reset_perms(st_map->image);
 	bpf_map_init_from_attr(map, attr);
 
+	module_put(mod);
+
 	return map;
+
+errout_free:
+	__bpf_struct_ops_map_free(map);
+	btf = NULL;		/* has been released */
+errout:
+	module_put(mod);
+	return ERR_PTR(ret);
 }
 
 static u64 bpf_struct_ops_map_mem_usage(const struct bpf_map *map)

@@ -811,6 +844,7 @@ static void bpf_struct_ops_map_link_dealloc(struct bpf_link *link)
 		 * bpf_struct_ops_link_create() fails to register.
 		 */
 		st_map->st_ops_desc->st_ops->unreg(&st_map->kvalue.data);
+		module_put(st_map->st_ops_desc->st_ops->owner);
 		bpf_map_put(&st_map->map);
 	}
 	kfree(st_link);

@@ -857,6 +891,10 @@ static int bpf_struct_ops_map_link_update(struct bpf_link *link, struct bpf_map
 	if (!bpf_struct_ops_valid_to_reg(new_map))
 		return -EINVAL;
 
+	/* The old map is holding the refcount for the owner module.  The
+	 * ownership of the owner module refcount is going to be
+	 * transferred from the old map to the new map.
+	 */
 	if (!st_map->st_ops_desc->st_ops->update)
 		return -EOPNOTSUPP;
 

@@ -902,6 +940,7 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
 	struct bpf_link_primer link_primer;
 	struct bpf_struct_ops_map *st_map;
 	struct bpf_map *map;
+	struct btf *btf;
 	int err;
 
 	map = bpf_map_get(attr->link_create.map_fd);

@@ -926,8 +965,15 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
 	if (err)
 		goto err_out;
 
+	/* Hold the owner module until the struct_ops is unregistered. */
+	btf = st_map->st_ops_desc->btf;
+	if (btf != btf_vmlinux && !btf_try_get_module(btf)) {
+		err = -EINVAL;
+		goto err_out;
+	}
 	err = st_map->st_ops_desc->st_ops->reg(st_map->kvalue.data);
 	if (err) {
+		module_put(st_map->st_ops_desc->st_ops->owner);
 		bpf_link_cleanup(&link_primer);
 		link = NULL;
 		goto err_out;

Quoted 53 lines · On Sat, 2023-10-21 at 22:03 -0700, [email protected] wrote:

From: Kui-Feng Lee <[email protected]>

To ensure that a module remains accessible whenever a struct_ops object of a struct_ops type provided by the module is still in use.

Signed-off-by Kui-Feng Lee <[email protected]>

---
  include/linux/bpf.h         |  1 +
  include/linux/btf.h         |  2 +-
  kernel/bpf/bpf_struct_ops.c | 70 ++++++++++++++++++++++++++++++-------
  3 files changed, 60 insertions(+), 13 deletions(-)

include/linux/bpf.h +1 -0

index 4f3b67932ded..26feb8a2da4f 100644
@@ -1626,6 +1626,7 @@ struct bpf_struct_ops {
  	void (*unreg)(void *kdata);
  	int (*update)(void *kdata, void *old_kdata);
  	int (*validate)(void *kdata);
+	struct module *owner;
  	const char *name;
  	struct btf_func_model func_models[BPF_STRUCT_OPS_MAX_NR_MEMBERS];
  };

include/linux/btf.h +1 -1

index 8e37f7eb02c7..6a64b372b7a0 100644
@@ -575,7 +575,7 @@ struct bpf_struct_ops;
  struct bpf_struct_ops_desc;
 
  struct bpf_struct_ops_desc *
-btf_add_struct_ops(struct bpf_struct_ops *st_ops);
+btf_add_struct_ops(struct btf *btf, struct bpf_struct_ops *st_ops);
  const struct bpf_struct_ops_desc *
  btf_get_struct_ops(struct btf *btf, u32 *ret_cnt);
 

kernel/bpf/bpf_struct_ops.c +1 -0

index 0bc21a39257d..413a3f8b26ba 100644
@@ -388,6 +388,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
  	const struct btf_member *member;
  	const struct btf_type *t = st_ops_desc->type;
  	struct bpf_tramp_links *tlinks;
+	struct module *mod = NULL;
  	void *udata, *kdata;
  	int prog_fd, err;
  	void *image, *image_end;

@@ -425,6 +426,14 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
  		goto unlock;
  	}
 

kernel/bpf/bpf_struct_ops.c static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key, · line 432

Quoted 4 lines

kernel/bpf/bpf_struct_ops.c +4 -0

+	if (st_ops_desc->btf != btf_vmlinux) {
+		mod = btf_try_get_module(st_ops_desc->btf);
+		if (!mod) {
+			err = -EBUSY;

Nit: there is a disagreement about error code returned for
failing btf_try_get_module() across verifier code base:
- EINVAL is used 2 times;
- ENXIO is used 3 times;
- ENOTSUPP is used once.
Are you sure EBUSY is a good choice here?

Quoted 167 lines

kernel/bpf/bpf_struct_ops.c +53 -12

+			goto unlock;
+		}
+	}
+
  	memcpy(uvalue, value, map->value_size);
 
  	udata = &uvalue->data;

@@ -552,6 +561,10 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
  		 * can be seen once BPF_STRUCT_OPS_STATE_INUSE is set.
  		 */
  		smp_store_release(&kvalue->state, BPF_STRUCT_OPS_STATE_INUSE);
+		/* Hold the owner module until the struct_ops is
+		 * unregistered
+		 */
+		mod = NULL;
  		goto unlock;
  	}
 

@@ -568,6 +581,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
  	memset(uvalue, 0, map->value_size);
  	memset(kvalue, 0, map->value_size);
  unlock:
+	module_put(mod);
  	kfree(tlinks);
  	mutex_unlock(&st_map->lock);
  	return err;

@@ -588,6 +602,7 @@ static long bpf_struct_ops_map_delete_elem(struct bpf_map *map, void *key)
  	switch (prev_state) {
  	case BPF_STRUCT_OPS_STATE_INUSE:
  		st_map->st_ops_desc->st_ops->unreg(&st_map->kvalue.data);
+		module_put(st_map->st_ops_desc->st_ops->owner);
  		bpf_map_put(map);
  		return 0;
  	case BPF_STRUCT_OPS_STATE_TOBEFREE:

@@ -674,6 +689,7 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
  	size_t st_map_size;
  	struct bpf_struct_ops_map *st_map;
  	const struct btf_type *t, *vt;
+	struct module *mod = NULL;
  	struct bpf_map *map;
  	int ret;
 

@@ -681,9 +697,17 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
  	if (!st_ops_desc)
  		return ERR_PTR(-ENOTSUPP);
 
+	if (st_ops_desc->btf != btf_vmlinux) {
+		mod = btf_try_get_module(st_ops_desc->btf);
+		if (!mod)
+			return ERR_PTR(-EINVAL);
+	}
+
  	vt = st_ops_desc->value_type;
-	if (attr->value_size != vt->size)
-		return ERR_PTR(-EINVAL);
+	if (attr->value_size != vt->size) {
+		ret = -EINVAL;
+		goto errout;
+	}
 
  	t = st_ops_desc->type;
 

@@ -694,17 +718,17 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
  		(vt->size - sizeof(struct bpf_struct_ops_value));
 
  	st_map = bpf_map_area_alloc(st_map_size, NUMA_NO_NODE);
-	if (!st_map)
-		return ERR_PTR(-ENOMEM);
+	if (!st_map) {
+		ret = -ENOMEM;
+		goto errout;
+	}
 
  	st_map->st_ops_desc = st_ops_desc;
  	map = &st_map->map;
 
  	ret = bpf_jit_charge_modmem(PAGE_SIZE);
-	if (ret) {
-		__bpf_struct_ops_map_free(map);
-		return ERR_PTR(ret);
-	}
+	if (ret)
+		goto errout_free;
 
  	st_map->image = bpf_jit_alloc_exec(PAGE_SIZE);
  	if (!st_map->image) {

@@ -713,23 +737,32 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
  		 * here.
  		 */
  		bpf_jit_uncharge_modmem(PAGE_SIZE);
-		__bpf_struct_ops_map_free(map);
-		return ERR_PTR(-ENOMEM);
+		ret = -ENOMEM;
+		goto errout_free;
  	}
  	st_map->uvalue = bpf_map_area_alloc(vt->size, NUMA_NO_NODE);
  	st_map->links =
  		bpf_map_area_alloc(btf_type_vlen(t) * sizeof(struct bpf_links *),
  				   NUMA_NO_NODE);
  	if (!st_map->uvalue || !st_map->links) {
-		__bpf_struct_ops_map_free(map);
-		return ERR_PTR(-ENOMEM);
+		ret = -ENOMEM;
+		goto errout_free;
  	}
 
  	mutex_init(&st_map->lock);
  	set_vm_flush_reset_perms(st_map->image);
  	bpf_map_init_from_attr(map, attr);
 
+	module_put(mod);
+
  	return map;
+
+errout_free:
+	__bpf_struct_ops_map_free(map);
+	btf = NULL;		/* has been released */
+errout:
+	module_put(mod);
+	return ERR_PTR(ret);
  }
 
  static u64 bpf_struct_ops_map_mem_usage(const struct bpf_map *map)

@@ -811,6 +844,7 @@ static void bpf_struct_ops_map_link_dealloc(struct bpf_link *link)
  		 * bpf_struct_ops_link_create() fails to register.
  		 */
  		st_map->st_ops_desc->st_ops->unreg(&st_map->kvalue.data);
+		module_put(st_map->st_ops_desc->st_ops->owner);
  		bpf_map_put(&st_map->map);
  	}
  	kfree(st_link);

@@ -857,6 +891,10 @@ static int bpf_struct_ops_map_link_update(struct bpf_link *link, struct bpf_map
  	if (!bpf_struct_ops_valid_to_reg(new_map))
  		return -EINVAL;
 
+	/* The old map is holding the refcount for the owner module.  The
+	 * ownership of the owner module refcount is going to be
+	 * transferred from the old map to the new map.
+	 */
  	if (!st_map->st_ops_desc->st_ops->update)
  		return -EOPNOTSUPP;
 

@@ -902,6 +940,7 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
  	struct bpf_link_primer link_primer;
  	struct bpf_struct_ops_map *st_map;
  	struct bpf_map *map;
+	struct btf *btf;
  	int err;
 
  	map = bpf_map_get(attr->link_create.map_fd);

@@ -926,8 +965,15 @@ int bpf_struct_ops_link_create(union bpf_attr *attr)
  	if (err)
  		goto err_out;
 
+	/* Hold the owner module until the struct_ops is unregistered. */
+	btf = st_map->st_ops_desc->btf;
+	if (btf != btf_vmlinux && !btf_try_get_module(btf)) {
+		err = -EINVAL;
+		goto err_out;
+	}
  	err = st_map->st_ops_desc->st_ops->reg(st_map->kvalue.data);
  	if (err) {
+		module_put(st_map->st_ops_desc->st_ops->owner);
  		bpf_link_cleanup(&link_primer);
  		link = NULL;
  		goto err_out;

From: Kui-Feng Lee <[email protected]>

Replace the static list of struct_ops types with per-btf struct_ops_tab to enable dynamic registration.

Both bpf_dummy_ops and bpf_tcp_ca now utilize the registration function instead of being listed in bpf_struct_ops_types.h.

Cc [email protected]

Signed-off-by Kui-Feng Lee <[email protected]>

---
 include/linux/bpf.h               |  36 +++++++--
 include/linux/btf.h               |   5 +-
 kernel/bpf/bpf_struct_ops.c       | 123 ++++++++----------------------
 kernel/bpf/bpf_struct_ops_types.h |  12 ---
 kernel/bpf/btf.c                  |  41 +++++++++-
 net/bpf/bpf_dummy_struct_ops.c    |  14 +++-
 net/ipv4/bpf_tcp_ca.c             |  16 +++-
 7 files changed, 130 insertions(+), 117 deletions(-)
 delete mode 100644 kernel/bpf/bpf_struct_ops_types.h

include/linux/bpf.h +31 -5

index 26feb8a2da4f..a53b2181c845 100644
@@ -1644,7 +1644,6 @@ struct bpf_struct_ops_desc {
 #if defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_SYSCALL)
 #define BPF_MODULE_OWNER ((void *)((0xeB9FUL << 2) + POISON_POINTER_DELTA))
 const struct bpf_struct_ops_desc *bpf_struct_ops_find(struct btf *btf, u32 type_id);
-void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log);
 bool bpf_struct_ops_get(const void *kdata);
 void bpf_struct_ops_put(const void *kdata);
 int bpf_struct_ops_map_sys_lookup_elem(struct bpf_map *map, void *key,

@@ -1690,10 +1689,6 @@ static inline const struct bpf_struct_ops_desc *bpf_struct_ops_find(struct btf *
 {
 	return NULL;
 }
-static inline void bpf_struct_ops_init(struct btf *btf,
-				       struct bpf_verifier_log *log)
-{
-}
 static inline bool bpf_try_module_get(const void *data, struct module *owner)
 {
 	return try_module_get(owner);

@@ -3212,4 +3207,35 @@ static inline bool bpf_is_subprog(const struct bpf_prog *prog)
 	return prog->aux->func_idx != 0;
 }
 
+int register_bpf_struct_ops(struct bpf_struct_ops *st_ops);
+
+enum bpf_struct_ops_state {
+	BPF_STRUCT_OPS_STATE_INIT,
+	BPF_STRUCT_OPS_STATE_INUSE,
+	BPF_STRUCT_OPS_STATE_TOBEFREE,
+	BPF_STRUCT_OPS_STATE_READY,
+};
+
+struct bpf_struct_ops_common_value {
+	refcount_t refcnt;
+	enum bpf_struct_ops_state state;
+};
+
+/* bpf_struct_ops_##_name (e.g. bpf_struct_ops_tcp_congestion_ops) is
+ * the map's value exposed to the userspace and its btf-type-id is
+ * stored at the map->btf_vmlinux_value_type_id.
+ *
+ */
+#define DEFINE_STRUCT_OPS_VALUE_TYPE(_name)			\
+extern struct bpf_struct_ops bpf_##_name;			\
+								\
+struct bpf_struct_ops_##_name {					\
+	struct bpf_struct_ops_common_value common;		\
+	struct _name data ____cacheline_aligned_in_smp;		\
+}
+
+extern void bpf_struct_ops_init(struct bpf_struct_ops_desc *st_ops_desc,
+				struct btf *btf,
+				struct bpf_verifier_log *log);
+
 #endif /* _LINUX_BPF_H */

include/linux/btf.h +3 -2

index 6a64b372b7a0..533db3f406b3 100644
@@ -12,6 +12,8 @@
 #include <uapi/linux/bpf.h>
 
 #define BTF_TYPE_EMIT(type) ((void)(type *)0)
+#define BTF_STRUCT_OPS_TYPE_EMIT(type) {((void)(struct type *)0);	\
+		((void)(struct bpf_struct_ops_##type *)0); }
 #define BTF_TYPE_EMIT_ENUM(enum_val) ((void)enum_val)
 
 /* These need to be macros, as the expressions are used in assembler input */

@@ -200,6 +202,7 @@ u32 btf_obj_id(const struct btf *btf);
 bool btf_is_kernel(const struct btf *btf);
 bool btf_is_module(const struct btf *btf);
 struct module *btf_try_get_module(const struct btf *btf);
+struct btf *btf_get_module_btf(const struct module *module);
 u32 btf_nr_types(const struct btf *btf);
 bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
 			   const struct btf_member *m,

@@ -574,8 +577,6 @@ static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type
 struct bpf_struct_ops;
 struct bpf_struct_ops_desc;
 
-struct bpf_struct_ops_desc *
-btf_add_struct_ops(struct btf *btf, struct bpf_struct_ops *st_ops);
 const struct bpf_struct_ops_desc *
 btf_get_struct_ops(struct btf *btf, u32 *ret_cnt);
 

kernel/bpf/bpf_struct_ops.c +33 -90

index 99ab61cc6cad..44412f95bc32 100644
@@ -13,21 +13,8 @@
 #include <linux/btf_ids.h>
 #include <linux/rcupdate_wait.h>
 
-enum bpf_struct_ops_state {
-	BPF_STRUCT_OPS_STATE_INIT,
-	BPF_STRUCT_OPS_STATE_INUSE,
-	BPF_STRUCT_OPS_STATE_TOBEFREE,
-	BPF_STRUCT_OPS_STATE_READY,
-};
-
-struct bpf_struct_ops_common_value {
-	refcount_t refcnt;
-	enum bpf_struct_ops_state state;
-};
-#define BPF_STRUCT_OPS_COMMON_VALUE struct bpf_struct_ops_common_value common
-
 struct bpf_struct_ops_value {
-	BPF_STRUCT_OPS_COMMON_VALUE;
+	struct bpf_struct_ops_common_value common;
 	char data[] ____cacheline_aligned_in_smp;
 };
 

@@ -72,35 +59,6 @@ static DEFINE_MUTEX(update_mutex);
 #define VALUE_PREFIX "bpf_struct_ops_"
 #define VALUE_PREFIX_LEN (sizeof(VALUE_PREFIX) - 1)
 
-/* bpf_struct_ops_##_name (e.g. bpf_struct_ops_tcp_congestion_ops) is
- * the map's value exposed to the userspace and its btf-type-id is
- * stored at the map->btf_vmlinux_value_type_id.
- *
- */
-#define BPF_STRUCT_OPS_TYPE(_name)				\
-extern struct bpf_struct_ops bpf_##_name;			\
-								\
-struct bpf_struct_ops_##_name {						\
-	BPF_STRUCT_OPS_COMMON_VALUE;				\
-	struct _name data ____cacheline_aligned_in_smp;		\
-};
-#include "bpf_struct_ops_types.h"
-#undef BPF_STRUCT_OPS_TYPE
-
-enum {
-#define BPF_STRUCT_OPS_TYPE(_name) BPF_STRUCT_OPS_TYPE_##_name,
-#include "bpf_struct_ops_types.h"
-#undef BPF_STRUCT_OPS_TYPE
-	__NR_BPF_STRUCT_OPS_TYPE,
-};
-
-static struct bpf_struct_ops_desc bpf_struct_ops[] = {
-#define BPF_STRUCT_OPS_TYPE(_name)				\
-	[BPF_STRUCT_OPS_TYPE_##_name] = { .st_ops = &bpf_##_name },
-#include "bpf_struct_ops_types.h"
-#undef BPF_STRUCT_OPS_TYPE
-};
-
 const struct bpf_verifier_ops bpf_struct_ops_verifier_ops = {
 };
 

@@ -110,13 +68,22 @@ const struct bpf_prog_ops bpf_struct_ops_prog_ops = {
 #endif
 };
 
-static const struct btf_type *module_type;
-static const struct btf_type *common_value_type;
+BTF_ID_LIST(st_ops_ids)
+BTF_ID(struct, module)
+BTF_ID(struct, bpf_struct_ops_common_value)
+
+enum {
+	idx_module_id,
+	idx_st_ops_common_value_id,
+};
+
+extern struct btf *btf_vmlinux;
 
 static bool is_valid_value_type(struct btf *btf, s32 value_id,
 				const struct btf_type *type,
 				const char *value_name)
 {
+	const struct btf_type *common_value_type;
 	const struct btf_member *member;
 	const struct btf_type *vt, *mt;
 

@@ -128,6 +95,8 @@ static bool is_valid_value_type(struct btf *btf, s32 value_id,
 	}
 	member = btf_type_member(vt);
 	mt = btf_type_by_id(btf, member->type);
+	common_value_type = btf_type_by_id(btf_vmlinux,
+					   st_ops_ids[idx_st_ops_common_value_id]);
 	if (mt != common_value_type) {
 		pr_warn("The first member of %s should be bpf_struct_ops_common_value\n",
 			value_name);

@@ -144,9 +113,9 @@ static bool is_valid_value_type(struct btf *btf, s32 value_id,
 	return true;
 }
 
-static void bpf_struct_ops_init_one(struct bpf_struct_ops_desc *st_ops_desc,
-				    struct btf *btf,
-				    struct bpf_verifier_log *log)
+void bpf_struct_ops_init(struct bpf_struct_ops_desc *st_ops_desc,
+			 struct btf *btf,
+			 struct bpf_verifier_log *log)
 {
 	struct bpf_struct_ops *st_ops = st_ops_desc->st_ops;
 	const struct btf_member *member;

@@ -232,51 +201,20 @@ static void bpf_struct_ops_init_one(struct bpf_struct_ops_desc *st_ops_desc,
 	}
 }
 
-void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
-{
-	struct bpf_struct_ops_desc *st_ops_desc;
-	s32 module_id, common_value_id;
-	u32 i;
-
-	/* Ensure BTF type is emitted for "struct bpf_struct_ops_##_name" */
-#define BPF_STRUCT_OPS_TYPE(_name) BTF_TYPE_EMIT(struct bpf_struct_ops_##_name);
-#include "bpf_struct_ops_types.h"
-#undef BPF_STRUCT_OPS_TYPE
-
-	module_id = btf_find_by_name_kind(btf, "module", BTF_KIND_STRUCT);
-	if (module_id < 0) {
-		pr_warn("Cannot find struct module in btf_vmlinux\n");
-		return;
-	}
-	module_type = btf_type_by_id(btf, module_id);
-	common_value_id = btf_find_by_name_kind(btf,
-						"bpf_struct_ops_common_value",
-						BTF_KIND_STRUCT);
-	if (common_value_id < 0) {
-		pr_warn("Cannot find struct common_value in btf_vmlinux\n");
-		return;
-	}
-	common_value_type = btf_type_by_id(btf, common_value_id);
-
-	for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
-		st_ops_desc = &bpf_struct_ops[i];
-		bpf_struct_ops_init_one(st_ops_desc, btf, log);
-	}
-}
-
-extern struct btf *btf_vmlinux;
-
 static const struct bpf_struct_ops_desc *
 bpf_struct_ops_find_value(struct btf *btf, u32 value_id)
 {
+	const struct bpf_struct_ops_desc *st_ops_list;
 	unsigned int i;
+	u32 cnt = 0;
 
-	if (!value_id || !btf_vmlinux)
+	if (!value_id)
 		return NULL;
 
-	for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
-		if (bpf_struct_ops[i].value_id == value_id)
-			return &bpf_struct_ops[i];
+	st_ops_list = btf_get_struct_ops(btf, &cnt);
+	for (i = 0; i < cnt; i++) {
+		if (st_ops_list[i].value_id == value_id)
+			return &st_ops_list[i];
 	}
 
 	return NULL;

@@ -285,14 +223,17 @@ bpf_struct_ops_find_value(struct btf *btf, u32 value_id)
 const struct bpf_struct_ops_desc *
 bpf_struct_ops_find(struct btf *btf, u32 type_id)
 {
+	const struct bpf_struct_ops_desc *st_ops_list;
 	unsigned int i;
+	u32 cnt;
 
-	if (!type_id || !btf_vmlinux)
+	if (!type_id)
 		return NULL;
 
-	for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
-		if (bpf_struct_ops[i].type_id == type_id)
-			return &bpf_struct_ops[i];
+	st_ops_list = btf_get_struct_ops(btf, &cnt);
+	for (i = 0; i < cnt; i++) {
+		if (st_ops_list[i].type_id == type_id)
+			return &st_ops_list[i];
 	}
 
 	return NULL;

@@ -429,6 +370,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
 	const struct bpf_struct_ops_desc *st_ops_desc = st_map->st_ops_desc;
 	const struct bpf_struct_ops *st_ops = st_ops_desc->st_ops;
 	struct bpf_struct_ops_value *uvalue, *kvalue;
+	const struct btf_type *module_type;
 	const struct btf_member *member;
 	const struct btf_type *t = st_ops_desc->type;
 	struct bpf_tramp_links *tlinks;

@@ -485,6 +427,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
 	image = st_map->image;
 	image_end = st_map->image + PAGE_SIZE;
 
+	module_type = btf_type_by_id(btf_vmlinux, st_ops_ids[idx_module_id]);
 	for_each_member(i, t, member) {
 		const struct btf_type *mtype, *ptype;
 		struct bpf_prog *prog;

kernel/bpf/bpf_struct_ops_types.h deleted +0 -12

deleted file mode 100644
index 5678a9ddf817..000000000000
@@ -1,12 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* internal file - do not include directly */
-
-#ifdef CONFIG_BPF_JIT
-#ifdef CONFIG_NET
-BPF_STRUCT_OPS_TYPE(bpf_dummy_ops)
-#endif
-#ifdef CONFIG_INET
-#include <net/tcp.h>
-BPF_STRUCT_OPS_TYPE(tcp_congestion_ops)
-#endif
-#endif

kernel/bpf/btf.c +12 -3

index c1e2ed6c972e..c53888e8dddb 100644
@@ -5790,8 +5790,6 @@ struct btf *btf_parse_vmlinux(void)
 	/* btf_parse_vmlinux() runs under bpf_verifier_lock */
 	bpf_ctx_convert.t = btf_type_by_id(btf, bpf_ctx_convert_btf_id[0]);
 
-	bpf_struct_ops_init(btf, log);
-
 	refcount_set(&btf->refcnt, 1);
 
 	err = btf_alloc_id(btf);

@@ -7532,7 +7530,7 @@ struct module *btf_try_get_module(const struct btf *btf)
 /* Returns struct btf corresponding to the struct module.
  * This function can return NULL or ERR_PTR.
  */
-static struct btf *btf_get_module_btf(const struct module *module)
+struct btf *btf_get_module_btf(const struct module *module)
 {
 #ifdef CONFIG_DEBUG_INFO_BTF_MODULES
 	struct btf_module *btf_mod, *tmp;

@@ -8672,3 +8670,40 @@ const struct bpf_struct_ops_desc *btf_get_struct_ops(struct btf *btf, u32 *ret_c
 	return (const struct bpf_struct_ops_desc *)btf->struct_ops_tab->ops;
 }
 
+int register_bpf_struct_ops(struct bpf_struct_ops *st_ops)
+{
+	struct bpf_struct_ops_desc *desc;
+	struct bpf_verifier_log *log;
+	struct btf *btf;
+	int err = 0;
+
+	if (st_ops == NULL)
+		return -EINVAL;
+
+	btf = btf_get_module_btf(st_ops->owner);

kernel/bpf/btf.c +8 -0

+
+errout:
+	kfree(log);
+	btf_put(btf);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(register_bpf_struct_ops);

net/bpf/bpf_dummy_struct_ops.c +12 -2

index ffa224053a6c..148a5851c4fa 100644
@@ -7,7 +7,7 @@
 #include <linux/bpf.h>
 #include <linux/btf.h>
 
-extern struct bpf_struct_ops bpf_bpf_dummy_ops;
+static struct bpf_struct_ops bpf_bpf_dummy_ops;
 
 /* A common type for test_N with return value in bpf_dummy_ops */
 typedef int (*dummy_ops_test_ret_fn)(struct bpf_dummy_ops_state *state, ...);

@@ -223,11 +223,13 @@ static int bpf_dummy_reg(void *kdata)
 	return -EOPNOTSUPP;
 }
 
+DEFINE_STRUCT_OPS_VALUE_TYPE(bpf_dummy_ops);
+
 static void bpf_dummy_unreg(void *kdata)
 {
 }
 
-struct bpf_struct_ops bpf_bpf_dummy_ops = {
+static struct bpf_struct_ops bpf_bpf_dummy_ops = {
 	.verifier_ops = &bpf_dummy_verifier_ops,
 	.init = bpf_dummy_init,
 	.check_member = bpf_dummy_ops_check_member,

@@ -235,4 +237,12 @@ struct bpf_struct_ops bpf_bpf_dummy_ops = {
 	.reg = bpf_dummy_reg,
 	.unreg = bpf_dummy_unreg,
 	.name = "bpf_dummy_ops",
+	.owner = THIS_MODULE,
 };
+
+static int __init bpf_dummy_struct_ops_init(void)
+{
+	BTF_STRUCT_OPS_TYPE_EMIT(bpf_dummy_ops);
+	return register_bpf_struct_ops(&bpf_bpf_dummy_ops);
+}
+late_initcall(bpf_dummy_struct_ops_init);

net/ipv4/bpf_tcp_ca.c +13 -3

index 3c8b76578a2a..b36a19274e5b 100644
@@ -12,7 +12,7 @@
 #include <net/bpf_sk_storage.h>
 
 /* "extern" is to avoid sparse warning.  It is only used in bpf_struct_ops.c. */
-extern struct bpf_struct_ops bpf_tcp_congestion_ops;
+static struct bpf_struct_ops bpf_tcp_congestion_ops;
 
 static u32 unsupported_ops[] = {
 	offsetof(struct tcp_congestion_ops, get_info),

@@ -277,7 +277,9 @@ static int bpf_tcp_ca_validate(void *kdata)
 	return tcp_validate_congestion_control(kdata);
 }
 
-struct bpf_struct_ops bpf_tcp_congestion_ops = {
+DEFINE_STRUCT_OPS_VALUE_TYPE(tcp_congestion_ops);
+
+static struct bpf_struct_ops bpf_tcp_congestion_ops = {
 	.verifier_ops = &bpf_tcp_ca_verifier_ops,
 	.reg = bpf_tcp_ca_reg,
 	.unreg = bpf_tcp_ca_unreg,

@@ -287,10 +289,18 @@ struct bpf_struct_ops bpf_tcp_congestion_ops = {
 	.init = bpf_tcp_ca_init,
 	.validate = bpf_tcp_ca_validate,
 	.name = "tcp_congestion_ops",
+	.owner = THIS_MODULE,
 };
 
 static int __init bpf_tcp_ca_kfunc_init(void)
 {
-	return register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_tcp_ca_kfunc_set);
+	int ret;
+
+	BTF_STRUCT_OPS_TYPE_EMIT(tcp_congestion_ops);
+
+	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_tcp_ca_kfunc_set);
+	ret = ret ?: register_bpf_struct_ops(&bpf_tcp_congestion_ops);
+
+	return ret;
 }
 late_initcall(bpf_tcp_ca_kfunc_init);

Quoted 10 lines · On Sat, 2023-10-21 at 22:03 -0700, [email protected] wrote:

From: Kui-Feng Lee <[email protected]>

Replace the static list of struct_ops types with per-btf struct_ops_tab to enable dynamic registration.

Both bpf_dummy_ops and bpf_tcp_ca now utilize the registration function instead of being listed in bpf_struct_ops_types.h.

Cc [email protected]

Signed-off-by Kui-Feng Lee <[email protected]>

Hello,

I left two nitpicks below, feel free to ignore if you think is good as-is.

Quoted 366 lines

---
  include/linux/bpf.h               |  36 +++++++--
  include/linux/btf.h               |   5 +-
  kernel/bpf/bpf_struct_ops.c       | 123 ++++++++----------------------
  kernel/bpf/bpf_struct_ops_types.h |  12 ---
  kernel/bpf/btf.c                  |  41 +++++++++-
  net/bpf/bpf_dummy_struct_ops.c    |  14 +++-
  net/ipv4/bpf_tcp_ca.c             |  16 +++-
  7 files changed, 130 insertions(+), 117 deletions(-)

delete mode 100644 kernel/bpf/bpf_struct_ops_types.h

include/linux/bpf.h +31 -5

index 26feb8a2da4f..a53b2181c845 100644
@@ -1644,7 +1644,6 @@ struct bpf_struct_ops_desc {
  #if defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_SYSCALL)
  #define BPF_MODULE_OWNER ((void *)((0xeB9FUL << 2) + POISON_POINTER_DELTA))
  const struct bpf_struct_ops_desc *bpf_struct_ops_find(struct btf *btf, u32 type_id);
-void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log);
  bool bpf_struct_ops_get(const void *kdata);
  void bpf_struct_ops_put(const void *kdata);
  int bpf_struct_ops_map_sys_lookup_elem(struct bpf_map *map, void *key,

@@ -1690,10 +1689,6 @@ static inline const struct bpf_struct_ops_desc *bpf_struct_ops_find(struct btf *
  {
  	return NULL;
  }
-static inline void bpf_struct_ops_init(struct btf *btf,
-				       struct bpf_verifier_log *log)
-{
-}
  static inline bool bpf_try_module_get(const void *data, struct module *owner)
  {
  	return try_module_get(owner);

@@ -3212,4 +3207,35 @@ static inline bool bpf_is_subprog(const struct bpf_prog *prog)
  	return prog->aux->func_idx != 0;
  }
 
+int register_bpf_struct_ops(struct bpf_struct_ops *st_ops);
+
+enum bpf_struct_ops_state {
+	BPF_STRUCT_OPS_STATE_INIT,
+	BPF_STRUCT_OPS_STATE_INUSE,
+	BPF_STRUCT_OPS_STATE_TOBEFREE,
+	BPF_STRUCT_OPS_STATE_READY,
+};
+
+struct bpf_struct_ops_common_value {
+	refcount_t refcnt;
+	enum bpf_struct_ops_state state;
+};
+
+/* bpf_struct_ops_##_name (e.g. bpf_struct_ops_tcp_congestion_ops) is
+ * the map's value exposed to the userspace and its btf-type-id is
+ * stored at the map->btf_vmlinux_value_type_id.
+ *
+ */
+#define DEFINE_STRUCT_OPS_VALUE_TYPE(_name)			\
+extern struct bpf_struct_ops bpf_##_name;			\
+								\
+struct bpf_struct_ops_##_name {					\
+	struct bpf_struct_ops_common_value common;		\
+	struct _name data ____cacheline_aligned_in_smp;		\
+}
+
+extern void bpf_struct_ops_init(struct bpf_struct_ops_desc *st_ops_desc,
+				struct btf *btf,
+				struct bpf_verifier_log *log);
+
  #endif /* _LINUX_BPF_H */

include/linux/btf.h +3 -2

index 6a64b372b7a0..533db3f406b3 100644
@@ -12,6 +12,8 @@
  #include <uapi/linux/bpf.h>
 
  #define BTF_TYPE_EMIT(type) ((void)(type *)0)
+#define BTF_STRUCT_OPS_TYPE_EMIT(type) {((void)(struct type *)0);	\
+		((void)(struct bpf_struct_ops_##type *)0); }
  #define BTF_TYPE_EMIT_ENUM(enum_val) ((void)enum_val)
 
  /* These need to be macros, as the expressions are used in assembler input */

@@ -200,6 +202,7 @@ u32 btf_obj_id(const struct btf *btf);
  bool btf_is_kernel(const struct btf *btf);
  bool btf_is_module(const struct btf *btf);
  struct module *btf_try_get_module(const struct btf *btf);
+struct btf *btf_get_module_btf(const struct module *module);
  u32 btf_nr_types(const struct btf *btf);
  bool btf_member_is_reg_int(const struct btf *btf, const struct btf_type *s,
  			   const struct btf_member *m,

@@ -574,8 +577,6 @@ static inline bool btf_type_is_struct_ptr(struct btf *btf, const struct btf_type
  struct bpf_struct_ops;
  struct bpf_struct_ops_desc;
 
-struct bpf_struct_ops_desc *
-btf_add_struct_ops(struct btf *btf, struct bpf_struct_ops *st_ops);
  const struct bpf_struct_ops_desc *
  btf_get_struct_ops(struct btf *btf, u32 *ret_cnt);
 

kernel/bpf/bpf_struct_ops.c +33 -90

index 99ab61cc6cad..44412f95bc32 100644
@@ -13,21 +13,8 @@
  #include <linux/btf_ids.h>
  #include <linux/rcupdate_wait.h>
 
-enum bpf_struct_ops_state {
-	BPF_STRUCT_OPS_STATE_INIT,
-	BPF_STRUCT_OPS_STATE_INUSE,
-	BPF_STRUCT_OPS_STATE_TOBEFREE,
-	BPF_STRUCT_OPS_STATE_READY,
-};
-
-struct bpf_struct_ops_common_value {
-	refcount_t refcnt;
-	enum bpf_struct_ops_state state;
-};
-#define BPF_STRUCT_OPS_COMMON_VALUE struct bpf_struct_ops_common_value common
-
  struct bpf_struct_ops_value {
-	BPF_STRUCT_OPS_COMMON_VALUE;
+	struct bpf_struct_ops_common_value common;
  	char data[] ____cacheline_aligned_in_smp;
  };
 

@@ -72,35 +59,6 @@ static DEFINE_MUTEX(update_mutex);
  #define VALUE_PREFIX "bpf_struct_ops_"
  #define VALUE_PREFIX_LEN (sizeof(VALUE_PREFIX) - 1)
 
-/* bpf_struct_ops_##_name (e.g. bpf_struct_ops_tcp_congestion_ops) is
- * the map's value exposed to the userspace and its btf-type-id is
- * stored at the map->btf_vmlinux_value_type_id.
- *
- */
-#define BPF_STRUCT_OPS_TYPE(_name)				\
-extern struct bpf_struct_ops bpf_##_name;			\
-								\
-struct bpf_struct_ops_##_name {						\
-	BPF_STRUCT_OPS_COMMON_VALUE;				\
-	struct _name data ____cacheline_aligned_in_smp;		\
-};
-#include "bpf_struct_ops_types.h"
-#undef BPF_STRUCT_OPS_TYPE
-
-enum {
-#define BPF_STRUCT_OPS_TYPE(_name) BPF_STRUCT_OPS_TYPE_##_name,
-#include "bpf_struct_ops_types.h"
-#undef BPF_STRUCT_OPS_TYPE
-	__NR_BPF_STRUCT_OPS_TYPE,
-};
-
-static struct bpf_struct_ops_desc bpf_struct_ops[] = {
-#define BPF_STRUCT_OPS_TYPE(_name)				\
-	[BPF_STRUCT_OPS_TYPE_##_name] = { .st_ops = &bpf_##_name },
-#include "bpf_struct_ops_types.h"
-#undef BPF_STRUCT_OPS_TYPE
-};
-
  const struct bpf_verifier_ops bpf_struct_ops_verifier_ops = {
  };
 

@@ -110,13 +68,22 @@ const struct bpf_prog_ops bpf_struct_ops_prog_ops = {
  #endif
  };
 
-static const struct btf_type *module_type;
-static const struct btf_type *common_value_type;
+BTF_ID_LIST(st_ops_ids)
+BTF_ID(struct, module)
+BTF_ID(struct, bpf_struct_ops_common_value)
+
+enum {
+	idx_module_id,
+	idx_st_ops_common_value_id,
+};
+
+extern struct btf *btf_vmlinux;
 
  static bool is_valid_value_type(struct btf *btf, s32 value_id,
  				const struct btf_type *type,
  				const char *value_name)
  {
+	const struct btf_type *common_value_type;
  	const struct btf_member *member;
  	const struct btf_type *vt, *mt;
 

@@ -128,6 +95,8 @@ static bool is_valid_value_type(struct btf *btf, s32 value_id,
  	}
  	member = btf_type_member(vt);
  	mt = btf_type_by_id(btf, member->type);
+	common_value_type = btf_type_by_id(btf_vmlinux,
+					   st_ops_ids[idx_st_ops_common_value_id]);
  	if (mt != common_value_type) {
  		pr_warn("The first member of %s should be bpf_struct_ops_common_value\n",
  			value_name);

@@ -144,9 +113,9 @@ static bool is_valid_value_type(struct btf *btf, s32 value_id,
  	return true;
  }
 
-static void bpf_struct_ops_init_one(struct bpf_struct_ops_desc *st_ops_desc,
-				    struct btf *btf,
-				    struct bpf_verifier_log *log)
+void bpf_struct_ops_init(struct bpf_struct_ops_desc *st_ops_desc,
+			 struct btf *btf,
+			 struct bpf_verifier_log *log)
  {
  	struct bpf_struct_ops *st_ops = st_ops_desc->st_ops;
  	const struct btf_member *member;

@@ -232,51 +201,20 @@ static void bpf_struct_ops_init_one(struct bpf_struct_ops_desc *st_ops_desc,
  	}
  }
 
-void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
-{
-	struct bpf_struct_ops_desc *st_ops_desc;
-	s32 module_id, common_value_id;
-	u32 i;
-
-	/* Ensure BTF type is emitted for "struct bpf_struct_ops_##_name" */
-#define BPF_STRUCT_OPS_TYPE(_name) BTF_TYPE_EMIT(struct bpf_struct_ops_##_name);
-#include "bpf_struct_ops_types.h"
-#undef BPF_STRUCT_OPS_TYPE
-
-	module_id = btf_find_by_name_kind(btf, "module", BTF_KIND_STRUCT);
-	if (module_id < 0) {
-		pr_warn("Cannot find struct module in btf_vmlinux\n");
-		return;
-	}
-	module_type = btf_type_by_id(btf, module_id);
-	common_value_id = btf_find_by_name_kind(btf,
-						"bpf_struct_ops_common_value",
-						BTF_KIND_STRUCT);
-	if (common_value_id < 0) {
-		pr_warn("Cannot find struct common_value in btf_vmlinux\n");
-		return;
-	}
-	common_value_type = btf_type_by_id(btf, common_value_id);
-
-	for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
-		st_ops_desc = &bpf_struct_ops[i];
-		bpf_struct_ops_init_one(st_ops_desc, btf, log);
-	}
-}
-
-extern struct btf *btf_vmlinux;
-
  static const struct bpf_struct_ops_desc *
  bpf_struct_ops_find_value(struct btf *btf, u32 value_id)
  {
+	const struct bpf_struct_ops_desc *st_ops_list;
  	unsigned int i;
+	u32 cnt = 0;
 
-	if (!value_id || !btf_vmlinux)
+	if (!value_id)
  		return NULL;
 
-	for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
-		if (bpf_struct_ops[i].value_id == value_id)
-			return &bpf_struct_ops[i];
+	st_ops_list = btf_get_struct_ops(btf, &cnt);
+	for (i = 0; i < cnt; i++) {
+		if (st_ops_list[i].value_id == value_id)
+			return &st_ops_list[i];
  	}
 
  	return NULL;

@@ -285,14 +223,17 @@ bpf_struct_ops_find_value(struct btf *btf, u32 value_id)
  const struct bpf_struct_ops_desc *
  bpf_struct_ops_find(struct btf *btf, u32 type_id)
  {
+	const struct bpf_struct_ops_desc *st_ops_list;
  	unsigned int i;
+	u32 cnt;
 
-	if (!type_id || !btf_vmlinux)
+	if (!type_id)
  		return NULL;
 
-	for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
-		if (bpf_struct_ops[i].type_id == type_id)
-			return &bpf_struct_ops[i];
+	st_ops_list = btf_get_struct_ops(btf, &cnt);
+	for (i = 0; i < cnt; i++) {
+		if (st_ops_list[i].type_id == type_id)
+			return &st_ops_list[i];
  	}
 
  	return NULL;

@@ -429,6 +370,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
  	const struct bpf_struct_ops_desc *st_ops_desc = st_map->st_ops_desc;
  	const struct bpf_struct_ops *st_ops = st_ops_desc->st_ops;
  	struct bpf_struct_ops_value *uvalue, *kvalue;
+	const struct btf_type *module_type;
  	const struct btf_member *member;
  	const struct btf_type *t = st_ops_desc->type;
  	struct bpf_tramp_links *tlinks;

@@ -485,6 +427,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
  	image = st_map->image;
  	image_end = st_map->image + PAGE_SIZE;
 
+	module_type = btf_type_by_id(btf_vmlinux, st_ops_ids[idx_module_id]);
  	for_each_member(i, t, member) {
  		const struct btf_type *mtype, *ptype;
  		struct bpf_prog *prog;

kernel/bpf/bpf_struct_ops_types.h deleted +0 -12

deleted file mode 100644
index 5678a9ddf817..000000000000
@@ -1,12 +0,0 @@
-/* SPDX-License-Identifier: GPL-2.0 */
-/* internal file - do not include directly */
-
-#ifdef CONFIG_BPF_JIT
-#ifdef CONFIG_NET
-BPF_STRUCT_OPS_TYPE(bpf_dummy_ops)
-#endif
-#ifdef CONFIG_INET
-#include <net/tcp.h>
-BPF_STRUCT_OPS_TYPE(tcp_congestion_ops)
-#endif
-#endif

kernel/bpf/btf.c +16 -3

index c1e2ed6c972e..c53888e8dddb 100644
@@ -5790,8 +5790,6 @@ struct btf *btf_parse_vmlinux(void)
  	/* btf_parse_vmlinux() runs under bpf_verifier_lock */
  	bpf_ctx_convert.t = btf_type_by_id(btf, bpf_ctx_convert_btf_id[0]);
 
-	bpf_struct_ops_init(btf, log);
-
  	refcount_set(&btf->refcnt, 1);
 
  	err = btf_alloc_id(btf);

@@ -7532,7 +7530,7 @@ struct module *btf_try_get_module(const struct btf *btf)
  /* Returns struct btf corresponding to the struct module.
   * This function can return NULL or ERR_PTR.
   */
-static struct btf *btf_get_module_btf(const struct module *module)
+struct btf *btf_get_module_btf(const struct module *module)
  {
  #ifdef CONFIG_DEBUG_INFO_BTF_MODULES
  	struct btf_module *btf_mod, *tmp;

@@ -8672,3 +8670,40 @@ const struct bpf_struct_ops_desc *btf_get_struct_ops(struct btf *btf, u32 *ret_c
  	return (const struct bpf_struct_ops_desc *)btf->struct_ops_tab->ops;
  }
 
+int register_bpf_struct_ops(struct bpf_struct_ops *st_ops)
+{
+	struct bpf_struct_ops_desc *desc;
+	struct bpf_verifier_log *log;
+	struct btf *btf;
+	int err = 0;
+
+	if (st_ops == NULL)
+		return -EINVAL;
+
+	btf = btf_get_module_btf(st_ops->owner);
+	if (!btf)
+		return -EINVAL;
+
+	log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);

kernel/bpf/btf.c const struct bpf_struct_ops_desc *btf_get_struct_ops(struct btf *btf, u32 *ret_c · line 8693

Quoted 6 lines

kernel/bpf/btf.c +6 -0

+	if (!log) {
+		err = -ENOMEM;
+		goto errout;
+	}
+
+	log->level = BPF_LOG_KERNEL;

Nit: maybe use bpf_vlog_init() here to avoid breaking encapsulation?

kernel/bpf/btf.c const struct bpf_struct_ops_desc *btf_get_struct_ops(struct btf *btf, u32 *ret_c · line 8701

Quoted 9 lines

kernel/bpf/btf.c +8 -0

+
+	desc = btf_add_struct_ops(btf, st_ops);
+	if (IS_ERR(desc)) {
+		err = PTR_ERR(desc);
+		goto errout;
+	}
+
+	bpf_struct_ops_init(desc, btf, log);

Nit: I think bpf_struct_ops_init() could be changed to return 'int',
then register_bpf_struct_ops() could report to calling module if something went wrong on the last phase, wdyt?

Quoted 94 lines

kernel/bpf/btf.c +8 -0

+
+errout:
+	kfree(log);
+	btf_put(btf);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(register_bpf_struct_ops);

net/bpf/bpf_dummy_struct_ops.c +12 -2

index ffa224053a6c..148a5851c4fa 100644
@@ -7,7 +7,7 @@
  #include <linux/bpf.h>
  #include <linux/btf.h>
 
-extern struct bpf_struct_ops bpf_bpf_dummy_ops;
+static struct bpf_struct_ops bpf_bpf_dummy_ops;
 
  /* A common type for test_N with return value in bpf_dummy_ops */
  typedef int (*dummy_ops_test_ret_fn)(struct bpf_dummy_ops_state *state, ...);

@@ -223,11 +223,13 @@ static int bpf_dummy_reg(void *kdata)
  	return -EOPNOTSUPP;
  }
 
+DEFINE_STRUCT_OPS_VALUE_TYPE(bpf_dummy_ops);
+
  static void bpf_dummy_unreg(void *kdata)
  {
  }
 
-struct bpf_struct_ops bpf_bpf_dummy_ops = {
+static struct bpf_struct_ops bpf_bpf_dummy_ops = {
  	.verifier_ops = &bpf_dummy_verifier_ops,
  	.init = bpf_dummy_init,
  	.check_member = bpf_dummy_ops_check_member,

@@ -235,4 +237,12 @@ struct bpf_struct_ops bpf_bpf_dummy_ops = {
  	.reg = bpf_dummy_reg,
  	.unreg = bpf_dummy_unreg,
  	.name = "bpf_dummy_ops",
+	.owner = THIS_MODULE,
  };
+
+static int __init bpf_dummy_struct_ops_init(void)
+{
+	BTF_STRUCT_OPS_TYPE_EMIT(bpf_dummy_ops);
+	return register_bpf_struct_ops(&bpf_bpf_dummy_ops);
+}
+late_initcall(bpf_dummy_struct_ops_init);

net/ipv4/bpf_tcp_ca.c +13 -3

index 3c8b76578a2a..b36a19274e5b 100644
@@ -12,7 +12,7 @@
  #include <net/bpf_sk_storage.h>
 
  /* "extern" is to avoid sparse warning.  It is only used in bpf_struct_ops.c. */
-extern struct bpf_struct_ops bpf_tcp_congestion_ops;
+static struct bpf_struct_ops bpf_tcp_congestion_ops;
 
  static u32 unsupported_ops[] = {
  	offsetof(struct tcp_congestion_ops, get_info),

@@ -277,7 +277,9 @@ static int bpf_tcp_ca_validate(void *kdata)
  	return tcp_validate_congestion_control(kdata);
  }
 
-struct bpf_struct_ops bpf_tcp_congestion_ops = {
+DEFINE_STRUCT_OPS_VALUE_TYPE(tcp_congestion_ops);
+
+static struct bpf_struct_ops bpf_tcp_congestion_ops = {
  	.verifier_ops = &bpf_tcp_ca_verifier_ops,
  	.reg = bpf_tcp_ca_reg,
  	.unreg = bpf_tcp_ca_unreg,

@@ -287,10 +289,18 @@ struct bpf_struct_ops bpf_tcp_congestion_ops = {
  	.init = bpf_tcp_ca_init,
  	.validate = bpf_tcp_ca_validate,
  	.name = "tcp_congestion_ops",
+	.owner = THIS_MODULE,
  };
 
  static int __init bpf_tcp_ca_kfunc_init(void)
  {
-	return register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_tcp_ca_kfunc_set);
+	int ret;
+
+	BTF_STRUCT_OPS_TYPE_EMIT(tcp_congestion_ops);
+
+	ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, &bpf_tcp_ca_kfunc_set);
+	ret = ret ?: register_bpf_struct_ops(&bpf_tcp_congestion_ops);
+
+	return ret;
  }
  late_initcall(bpf_tcp_ca_kfunc_init);

Quoted 2 lines · Eduard Zingerman

Quoted 1 line · On Sat, 2023-10-21 at 22:03 -0700, [email protected] wrote:

From: Kui-Feng Lee <[email protected]>

Quoted 6 lines

Quoted 6 lines

kernel/bpf/btf.c +6 -0

+
+    btf = btf_get_module_btf(st_ops->owner);
+    if (!btf)
+        return -EINVAL;
+
+    log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);

Quoted 8 lines

Quoted 6 lines

+    if (!log) {
+        err = -ENOMEM;
+        goto errout;
+    }
+
+    log->level = BPF_LOG_KERNEL;

Nit: maybe use bpf_vlog_init() here to avoid breaking encapsulation?

Agree!

Quoted 13 lines

kernel/bpf/btf.c const struct bpf_struct_ops_desc *btf_get_struct_ops(struct btf *btf, u32 *ret_c · line 8701

Quoted 9 lines

kernel/bpf/btf.c +8 -0

+
+    desc = btf_add_struct_ops(btf, st_ops);
+    if (IS_ERR(desc)) {
+        err = PTR_ERR(desc);
+        goto errout;
+    }
+
+    bpf_struct_ops_init(desc, btf, log);

Nit: I think bpf_struct_ops_init() could be changed to return 'int', then register_bpf_struct_ops() could report to calling module if something went wrong on the last phase, wdyt?

Agree!

Quoted 89 lines

Quoted 88 lines

kernel/bpf/btf.c +8 -0

+
+errout:
+    kfree(log);
+    btf_put(btf);
+
+    return err;
+}
+EXPORT_SYMBOL_GPL(register_bpf_struct_ops);

net/bpf/bpf_dummy_struct_ops.c +12 -2

index ffa224053a6c..148a5851c4fa 100644
@@ -7,7 +7,7 @@
   #include <linux/bpf.h>
   #include <linux/btf.h>
-extern struct bpf_struct_ops bpf_bpf_dummy_ops;
+static struct bpf_struct_ops bpf_bpf_dummy_ops;
   /* A common type for test_N with return value in bpf_dummy_ops */
   typedef int (*dummy_ops_test_ret_fn)(struct bpf_dummy_ops_state *state, ...);

@@ -223,11 +223,13 @@ static int bpf_dummy_reg(void *kdata)
       return -EOPNOTSUPP;
   }
+DEFINE_STRUCT_OPS_VALUE_TYPE(bpf_dummy_ops);
+
   static void bpf_dummy_unreg(void *kdata)
   {
   }
-struct bpf_struct_ops bpf_bpf_dummy_ops = {
+static struct bpf_struct_ops bpf_bpf_dummy_ops = {
       .verifier_ops = &bpf_dummy_verifier_ops,
       .init = bpf_dummy_init,
       .check_member = bpf_dummy_ops_check_member,

@@ -235,4 +237,12 @@ struct bpf_struct_ops bpf_bpf_dummy_ops = {
       .reg = bpf_dummy_reg,
       .unreg = bpf_dummy_unreg,
       .name = "bpf_dummy_ops",
+    .owner = THIS_MODULE,
   };
+
+static int __init bpf_dummy_struct_ops_init(void)
+{
+    BTF_STRUCT_OPS_TYPE_EMIT(bpf_dummy_ops);
+    return register_bpf_struct_ops(&bpf_bpf_dummy_ops);
+}
+late_initcall(bpf_dummy_struct_ops_init);

net/ipv4/bpf_tcp_ca.c +13 -3

index 3c8b76578a2a..b36a19274e5b 100644
@@ -12,7 +12,7 @@
   #include <net/bpf_sk_storage.h>
   /* "extern" is to avoid sparse warning.  It is only used in bpf_struct_ops.c. */
-extern struct bpf_struct_ops bpf_tcp_congestion_ops;
+static struct bpf_struct_ops bpf_tcp_congestion_ops;
   static u32 unsupported_ops[] = {
       offsetof(struct tcp_congestion_ops, get_info),

@@ -277,7 +277,9 @@ static int bpf_tcp_ca_validate(void *kdata)
       return tcp_validate_congestion_control(kdata);
   }
-struct bpf_struct_ops bpf_tcp_congestion_ops = {
+DEFINE_STRUCT_OPS_VALUE_TYPE(tcp_congestion_ops);
+
+static struct bpf_struct_ops bpf_tcp_congestion_ops = {
       .verifier_ops = &bpf_tcp_ca_verifier_ops,
       .reg = bpf_tcp_ca_reg,
       .unreg = bpf_tcp_ca_unreg,

@@ -287,10 +289,18 @@ struct bpf_struct_ops bpf_tcp_congestion_ops = {
       .init = bpf_tcp_ca_init,
       .validate = bpf_tcp_ca_validate,
       .name = "tcp_congestion_ops",
+    .owner = THIS_MODULE,
   };
   static int __init bpf_tcp_ca_kfunc_init(void)
   {
-    return register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, 
 &bpf_tcp_ca_kfunc_set);
+    int ret;
+
+    BTF_STRUCT_OPS_TYPE_EMIT(tcp_congestion_ops);
+
+    ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, 
 &bpf_tcp_ca_kfunc_set);
+    ret = ret ?: register_bpf_struct_ops(&bpf_tcp_congestion_ops);
+
+    return ret;
   }

  late_initcall(bpf_tcp_ca_kfunc_init);

Quoted 5 lines · Kui-Feng Lee

Quoted 2 lines · Eduard Zingerman

Quoted 1 line · On Sat, 2023-10-21 at 22:03 -0700, [email protected] wrote:

From: Kui-Feng Lee <[email protected]>

[...]

Quoted 15 lines

Quoted 6 lines

Quoted 6 lines

kernel/bpf/btf.c +6 -0

+
+    btf = btf_get_module_btf(st_ops->owner);
+    if (!btf)
+        return -EINVAL;
+
+    log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);

Quoted 8 lines

Quoted 6 lines

+    if (!log) {
+        err = -ENOMEM;
+        goto errout;
+    }
+
+    log->level = BPF_LOG_KERNEL;

Nit: maybe use bpf_vlog_init() here to avoid breaking encapsulation?

Quoted 2 lines

Agree!

I don't use bpf_vlog_init() eventually.

I found bpf_vlog_init() is not for BPF_LOG_KERNEL. According to the comment next to BPF_LOG_KERNEL, it
is an internal log level.
According to the code of bpf_vlog_init(), the level passing to bpf_vlog_init() should be covered by BPF_LOG_MASK. BPF_LOG_KERNEL is defined as BPF_LOG_MASK + 1. So, it is intended not being used with bpf_vlog_init().

Quoted 110 lines

Quoted 14 lines

kernel/bpf/btf.c const struct bpf_struct_ops_desc *btf_get_struct_ops(struct btf *btf, u32 *ret_c · line 8701

Quoted 10 lines

kernel/bpf/btf.c +8 -0

+
+    desc = btf_add_struct_ops(btf, st_ops);
+    if (IS_ERR(desc)) {
+        err = PTR_ERR(desc);
+        goto errout;
+    }
+
+    bpf_struct_ops_init(desc, btf, log);

Nit: I think bpf_struct_ops_init() could be changed to return 'int', then register_bpf_struct_ops() could report to calling module if something went wrong on the last phase, wdyt?

Agree!

Quoted 92 lines

Quoted 91 lines

kernel/bpf/btf.c +8 -0

+
+errout:
+    kfree(log);
+    btf_put(btf);
+
+    return err;
+}
+EXPORT_SYMBOL_GPL(register_bpf_struct_ops);

net/bpf/bpf_dummy_struct_ops.c +1 -1

index ffa224053a6c..148a5851c4fa 100644
@@ -7,7 +7,7 @@
   #include <linux/bpf.h>
   #include <linux/btf.h>
-extern struct bpf_struct_ops bpf_bpf_dummy_ops;
+static struct bpf_struct_ops bpf_bpf_dummy_ops;
   /* A common type for test_N with return value in bpf_dummy_ops */

  typedef int (*dummy_ops_test_ret_fn)(struct bpf_dummy_ops_state *state, ...);

net/bpf/bpf_dummy_struct_ops.c +11 -1

@@ -223,11 +223,13 @@ static int bpf_dummy_reg(void *kdata)
       return -EOPNOTSUPP;
   }
+DEFINE_STRUCT_OPS_VALUE_TYPE(bpf_dummy_ops);
+
   static void bpf_dummy_unreg(void *kdata)
   {
   }
-struct bpf_struct_ops bpf_bpf_dummy_ops = {
+static struct bpf_struct_ops bpf_bpf_dummy_ops = {
       .verifier_ops = &bpf_dummy_verifier_ops,
       .init = bpf_dummy_init,
       .check_member = bpf_dummy_ops_check_member,

@@ -235,4 +237,12 @@ struct bpf_struct_ops bpf_bpf_dummy_ops = {
       .reg = bpf_dummy_reg,
       .unreg = bpf_dummy_unreg,
       .name = "bpf_dummy_ops",
+    .owner = THIS_MODULE,
   };
+
+static int __init bpf_dummy_struct_ops_init(void)
+{
+    BTF_STRUCT_OPS_TYPE_EMIT(bpf_dummy_ops);
+    return register_bpf_struct_ops(&bpf_bpf_dummy_ops);
+}
+late_initcall(bpf_dummy_struct_ops_init);

net/ipv4/bpf_tcp_ca.c +0 -0

index 3c8b76578a2a..b36a19274e5b 100644
@@ -12,7 +12,7 @@
   #include <net/bpf_sk_storage.h>
   /* "extern" is to avoid sparse warning.  It is only used in 

bpf_struct_ops.c. */

net/ipv4/bpf_tcp_ca.c +1 -1

-extern struct bpf_struct_ops bpf_tcp_congestion_ops;
+static struct bpf_struct_ops bpf_tcp_congestion_ops;

  static u32 unsupported_ops[] = { offsetof(struct tcp_congestion_ops, get_info),

net/ipv4/bpf_tcp_ca.c +12 -2

@@ -277,7 +277,9 @@ static int bpf_tcp_ca_validate(void *kdata)
       return tcp_validate_congestion_control(kdata);
   }
-struct bpf_struct_ops bpf_tcp_congestion_ops = {
+DEFINE_STRUCT_OPS_VALUE_TYPE(tcp_congestion_ops);
+
+static struct bpf_struct_ops bpf_tcp_congestion_ops = {
       .verifier_ops = &bpf_tcp_ca_verifier_ops,
       .reg = bpf_tcp_ca_reg,
       .unreg = bpf_tcp_ca_unreg,

@@ -287,10 +289,18 @@ struct bpf_struct_ops bpf_tcp_congestion_ops = {
       .init = bpf_tcp_ca_init,
       .validate = bpf_tcp_ca_validate,
       .name = "tcp_congestion_ops",
+    .owner = THIS_MODULE,
   };
   static int __init bpf_tcp_ca_kfunc_init(void)
   {
-    return register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, 
 &bpf_tcp_ca_kfunc_set);
+    int ret;
+
+    BTF_STRUCT_OPS_TYPE_EMIT(tcp_congestion_ops);
+
+    ret = register_btf_kfunc_id_set(BPF_PROG_TYPE_STRUCT_OPS, 
 &bpf_tcp_ca_kfunc_set);
+    ret = ret ?: register_bpf_struct_ops(&bpf_tcp_congestion_ops);
+
+    return ret;
   }

  late_initcall(bpf_tcp_ca_kfunc_init);

From: Kui-Feng Lee <[email protected]>

Create a new struct_ops type called bpf_testmod_ops within the bpf_testmod module. When a struct_ops object is registered, the bpf_testmod module will invoke test_2 from the module.

Signed-off-by Kui-Feng Lee <[email protected]>

---
 tools/testing/selftests/bpf/Makefile          |  2 +
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 59 +++++++++++++++++++
 .../selftests/bpf/bpf_testmod/bpf_testmod.h   |  5 ++
 .../bpf/prog_tests/test_struct_ops_module.c   | 38 ++++++++++++
 .../selftests/bpf/progs/struct_ops_module.c   | 30 ++++++++++
 tools/testing/selftests/bpf/testing_helpers.c | 35 +++++++++++
 6 files changed, 169 insertions(+)
 create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
 create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_module.c

tools/testing/selftests/bpf/Makefile +2 -0

index caede9b574cb..dd7ff14e1fdf 100644
@@ -706,6 +706,8 @@ $(OUTPUT)/uprobe_multi: uprobe_multi.c
 	$(call msg,BINARY,,$@)
 	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $^ $(LDLIBS) -o $@
 
+$(OUTPUT)/testing_helpers.o: $(OUTPUT)/rcu_tasks_trace_gp.skel.h
+
 EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)	\
 	prog_tests/tests.h map_tests/tests.h verifier/tests.h		\
 	feature bpftool							\

tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c +59 -0

index cefc5dd72573..f1a20669d884 100644
@@ -1,5 +1,6 @@
 // SPDX-License-Identifier: GPL-2.0
 /* Copyright (c) 2020 Facebook */
+#include <linux/bpf.h>
 #include <linux/btf.h>
 #include <linux/btf_ids.h>
 #include <linux/error-injection.h>

@@ -517,11 +518,66 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_static_unused_arg)
 BTF_ID_FLAGS(func, bpf_kfunc_call_test_offset)
 BTF_SET8_END(bpf_testmod_check_kfunc_ids)
 
+#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+
+DEFINE_STRUCT_OPS_VALUE_TYPE(bpf_testmod_ops);
+
+static int bpf_testmod_ops_init(struct btf *btf)
+{
+	return 0;
+}
+
+static bool bpf_testmod_ops_is_valid_access(int off, int size,
+					    enum bpf_access_type type,
+					    const struct bpf_prog *prog,
+					    struct bpf_insn_access_aux *info)
+{
+	return bpf_tracing_btf_ctx_access(off, size, type, prog, info);
+}
+
+static int bpf_testmod_ops_init_member(const struct btf_type *t,
+				       const struct btf_member *member,
+				       void *kdata, const void *udata)
+{
+	return 0;
+}
+
 static const struct btf_kfunc_id_set bpf_testmod_kfunc_set = {
 	.owner = THIS_MODULE,
 	.set   = &bpf_testmod_check_kfunc_ids,
 };
 
+static const struct bpf_verifier_ops bpf_testmod_verifier_ops = {
+	.is_valid_access = bpf_testmod_ops_is_valid_access,
+};
+
+static int bpf_dummy_reg(void *kdata)
+{
+	struct bpf_testmod_ops *ops = kdata;
+	int r;
+
+	BTF_STRUCT_OPS_TYPE_EMIT(bpf_testmod_ops);
+	r = ops->test_2(4, 3);
+
+	return 0;
+}
+
+static void bpf_dummy_unreg(void *kdata)
+{
+}
+
+struct bpf_struct_ops bpf_bpf_testmod_ops = {
+	.verifier_ops = &bpf_testmod_verifier_ops,
+	.init = bpf_testmod_ops_init,
+	.init_member = bpf_testmod_ops_init_member,
+	.reg = bpf_dummy_reg,
+	.unreg = bpf_dummy_unreg,
+	.name = "bpf_testmod_ops",
+	.owner = THIS_MODULE,
+};
+
+#endif /* CONFIG_DEBUG_INFO_BTF_MODULES */
+
 extern int bpf_fentry_test1(int a);
 
 static int bpf_testmod_init(void)

@@ -532,6 +588,9 @@ static int bpf_testmod_init(void)
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_testmod_kfunc_set);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_kfunc_set);
 	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_kfunc_set);
+#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+	ret = ret ?: register_bpf_struct_ops(&bpf_bpf_testmod_ops);
+#endif
 	if (ret < 0)
 		return ret;
 	if (bpf_fentry_test1(0) < 0)

tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h +5 -0

index f32793efe095..ca5435751c79 100644
@@ -28,4 +28,9 @@ struct bpf_iter_testmod_seq {
 	int cnt;
 };
 
+struct bpf_testmod_ops {
+	int (*test_1)(void);
+	int (*test_2)(int a, int b);
+};
+
 #endif /* _BPF_TESTMOD_H */

tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c new file +38 -0

new file mode 100644
index 000000000000..7261fc6c377a
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+#include <test_progs.h>
+#include <time.h>
+
+#include "rcu_tasks_trace_gp.skel.h"
+#include "struct_ops_module.skel.h"
+
+static void test_regular_load(void)
+{
+	struct struct_ops_module *skel;
+	struct bpf_link *link;
+	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts);
+	int err;
+
+	skel = struct_ops_module__open_opts(&opts);
+	if (!ASSERT_OK_PTR(skel, "struct_ops_module_open"))
+		return;
+	err = struct_ops_module__load(skel);
+	if (!ASSERT_OK(err, "struct_ops_module_load"))
+		return;
+
+	link = bpf_map__attach_struct_ops(skel->maps.testmod_1);
+	ASSERT_OK_PTR(link, "attach_test_mod_1");
+
+	ASSERT_EQ(skel->bss->test_2_result, 7, "test_2_result");
+
+	bpf_link__destroy(link);
+
+	struct_ops_module__destroy(skel);
+}
+
+void serial_test_struct_ops_module(void)
+{
+	if (test__start_subtest("regular_load"))
+		test_regular_load();
+}
+

tools/testing/selftests/bpf/progs/struct_ops_module.c new file +30 -0

new file mode 100644
index 000000000000..cb305d04342f
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "../bpf_testmod/bpf_testmod.h"
+
+char _license[] SEC("license") = "GPL";
+
+int test_2_result = 0;
+
+SEC("struct_ops/test_1")
+int BPF_PROG(test_1)
+{
+	return 0xdeadbeef;
+}
+
+SEC("struct_ops/test_2")
+int BPF_PROG(test_2, int a, int b)
+{
+	test_2_result = a + b;
+	return a + b;
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops testmod_1 = {
+	.test_1 = (void *)test_1,
+	.test_2 = (void *)test_2,
+};
+

tools/testing/selftests/bpf/testing_helpers.c +35 -0

index 8d994884c7b4..05870cd62458 100644
@@ -10,6 +10,7 @@
 #include "test_progs.h"
 #include "testing_helpers.h"
 #include <linux/membarrier.h>
+#include "rcu_tasks_trace_gp.skel.h"
 
 int parse_num_list(const char *s, bool **num_set, int *num_set_len)
 {

@@ -380,10 +381,44 @@ int load_bpf_testmod(bool verbose)
 	return 0;
 }
 
+/* This function will trigger call_rcu_tasks_trace() in the kernel */
+static int kern_sync_rcu_tasks_trace(void)
+{
+	struct rcu_tasks_trace_gp *rcu;
+	time_t start;
+	long gp_seq;
+	LIBBPF_OPTS(bpf_test_run_opts, opts);
+
+	rcu = rcu_tasks_trace_gp__open_and_load();
+	if (IS_ERR(rcu))
+		return -EFAULT;
+	if (rcu_tasks_trace_gp__attach(rcu))
+		return -EFAULT;
+
+	gp_seq = READ_ONCE(rcu->bss->gp_seq);
+
+	if (bpf_prog_test_run_opts(bpf_program__fd(rcu->progs.do_call_rcu_tasks_trace),
+				   &opts))
+		return -EFAULT;
+	if (opts.retval != 0)
+		return -EFAULT;
+
+	start = time(NULL);
+	while ((start + 2) > time(NULL) &&
+	       gp_seq == READ_ONCE(rcu->bss->gp_seq))
+		sched_yield();
+
+	rcu_tasks_trace_gp__destroy(rcu);
+
+	return 0;
+}
+
 /*
  * Trigger synchronize_rcu() in kernel.
  */
 int kern_sync_rcu(void)
 {
+	if (kern_sync_rcu_tasks_trace())
+		return -EFAULT;
 	return syscall(__NR_membarrier, MEMBARRIER_CMD_SHARED, 0, 0);
 }

Quoted 7 lines · On Sat, 2023-10-21 at 22:03 -0700, [email protected] wrote:

From: Kui-Feng Lee <[email protected]>

Create a new struct_ops type called bpf_testmod_ops within the bpf_testmod module. When a struct_ops object is registered, the bpf_testmod module will invoke test_2 from the module.

Signed-off-by Kui-Feng Lee <[email protected]>

Hello,

Sorry for the late response, was moving through the patch-set very slowly. Please note that CI currently fails for this series [0], reported error is:

testing_helpers.c:13:10: fatal error: 'rcu_tasks_trace_gp.skel.h' file not found

    13 | #include "rcu_tasks_trace_gp.skel.h"
       |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~

I get the same error when try to run tests locally (after full clean). On the other hand it looks like kern_sync_rcu_tasks_trace changes are not really necessary, when I undo these changes but keep changes in:

- .../selftests/bpf/bpf_testmod/bpf_testmod.c
- .../selftests/bpf/bpf_testmod/bpf_testmod.h
- .../bpf/prog_tests/test_struct_ops_module.c
- .../selftests/bpf/progs/struct_ops_module.c

struct_ops_module/regular_load test still passes.

Regarding assertion:

tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c line 26

Quoted 1 line

tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c new file +1 -0

+	ASSERT_EQ(skel->bss->test_2_result, 7, "test_2_result");

Could you please leave a comment explaining why the value is 7? I don't understand what invokes 'test_2' but changing it to 8 forces test to fail, so something does call 'test_2' :)

Also, when running test_maps I get the following error:

libbpf: bpf_map_create_opts has non-zero extra bytes map_create_opts(317):FAIL:bpf_map_create() error:Invalid argument (name=hash_of_maps)

[0] https://patchwork.kernel.org/project/netdevbpf/patch/[email protected]/
(look for 'Logs for x86_64-gcc / build / build for x86_64 with gcc ')

Quoted 263 lines

---
  tools/testing/selftests/bpf/Makefile          |  2 +
  .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 59 +++++++++++++++++++
  .../selftests/bpf/bpf_testmod/bpf_testmod.h   |  5 ++
  .../bpf/prog_tests/test_struct_ops_module.c   | 38 ++++++++++++
  .../selftests/bpf/progs/struct_ops_module.c   | 30 ++++++++++
  tools/testing/selftests/bpf/testing_helpers.c | 35 +++++++++++
  6 files changed, 169 insertions(+)

create mode 100644 tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c create mode 100644 tools/testing/selftests/bpf/progs/struct_ops_module.c

tools/testing/selftests/bpf/Makefile +2 -0

index caede9b574cb..dd7ff14e1fdf 100644
@@ -706,6 +706,8 @@ $(OUTPUT)/uprobe_multi: uprobe_multi.c
  	$(call msg,BINARY,,$@)
  	$(Q)$(CC) $(CFLAGS) $(LDFLAGS) $^ $(LDLIBS) -o $@
 
+$(OUTPUT)/testing_helpers.o: $(OUTPUT)/rcu_tasks_trace_gp.skel.h
+
  EXTRA_CLEAN := $(TEST_CUSTOM_PROGS) $(SCRATCH_DIR) $(HOST_SCRATCH_DIR)	\
  	prog_tests/tests.h map_tests/tests.h verifier/tests.h		\
  	feature bpftool							\

tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c +59 -0

index cefc5dd72573..f1a20669d884 100644
@@ -1,5 +1,6 @@
  // SPDX-License-Identifier: GPL-2.0
  /* Copyright (c) 2020 Facebook */
+#include <linux/bpf.h>
  #include <linux/btf.h>
  #include <linux/btf_ids.h>
  #include <linux/error-injection.h>

@@ -517,11 +518,66 @@ BTF_ID_FLAGS(func, bpf_kfunc_call_test_static_unused_arg)
  BTF_ID_FLAGS(func, bpf_kfunc_call_test_offset)
  BTF_SET8_END(bpf_testmod_check_kfunc_ids)
 
+#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+
+DEFINE_STRUCT_OPS_VALUE_TYPE(bpf_testmod_ops);
+
+static int bpf_testmod_ops_init(struct btf *btf)
+{
+	return 0;
+}
+
+static bool bpf_testmod_ops_is_valid_access(int off, int size,
+					    enum bpf_access_type type,
+					    const struct bpf_prog *prog,
+					    struct bpf_insn_access_aux *info)
+{
+	return bpf_tracing_btf_ctx_access(off, size, type, prog, info);
+}
+
+static int bpf_testmod_ops_init_member(const struct btf_type *t,
+				       const struct btf_member *member,
+				       void *kdata, const void *udata)
+{
+	return 0;
+}
+
  static const struct btf_kfunc_id_set bpf_testmod_kfunc_set = {
  	.owner = THIS_MODULE,
  	.set   = &bpf_testmod_check_kfunc_ids,
  };
 
+static const struct bpf_verifier_ops bpf_testmod_verifier_ops = {
+	.is_valid_access = bpf_testmod_ops_is_valid_access,
+};
+
+static int bpf_dummy_reg(void *kdata)
+{
+	struct bpf_testmod_ops *ops = kdata;
+	int r;
+
+	BTF_STRUCT_OPS_TYPE_EMIT(bpf_testmod_ops);
+	r = ops->test_2(4, 3);
+
+	return 0;
+}
+
+static void bpf_dummy_unreg(void *kdata)
+{
+}
+
+struct bpf_struct_ops bpf_bpf_testmod_ops = {
+	.verifier_ops = &bpf_testmod_verifier_ops,
+	.init = bpf_testmod_ops_init,
+	.init_member = bpf_testmod_ops_init_member,
+	.reg = bpf_dummy_reg,
+	.unreg = bpf_dummy_unreg,
+	.name = "bpf_testmod_ops",
+	.owner = THIS_MODULE,
+};
+
+#endif /* CONFIG_DEBUG_INFO_BTF_MODULES */
+
  extern int bpf_fentry_test1(int a);
 
  static int bpf_testmod_init(void)

@@ -532,6 +588,9 @@ static int bpf_testmod_init(void)
  	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SCHED_CLS, &bpf_testmod_kfunc_set);
  	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_TRACING, &bpf_testmod_kfunc_set);
  	ret = ret ?: register_btf_kfunc_id_set(BPF_PROG_TYPE_SYSCALL, &bpf_testmod_kfunc_set);
+#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+	ret = ret ?: register_bpf_struct_ops(&bpf_bpf_testmod_ops);
+#endif
  	if (ret < 0)
  		return ret;
  	if (bpf_fentry_test1(0) < 0)

tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h +5 -0

index f32793efe095..ca5435751c79 100644
@@ -28,4 +28,9 @@ struct bpf_iter_testmod_seq {
  	int cnt;
  };
 
+struct bpf_testmod_ops {
+	int (*test_1)(void);
+	int (*test_2)(int a, int b);
+};
+
  #endif /* _BPF_TESTMOD_H */

tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c new file +38 -0

new file mode 100644
index 000000000000..7261fc6c377a
@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+#include <test_progs.h>
+#include <time.h>
+
+#include "rcu_tasks_trace_gp.skel.h"
+#include "struct_ops_module.skel.h"
+
+static void test_regular_load(void)
+{
+	struct struct_ops_module *skel;
+	struct bpf_link *link;
+	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts);
+	int err;
+
+	skel = struct_ops_module__open_opts(&opts);
+	if (!ASSERT_OK_PTR(skel, "struct_ops_module_open"))
+		return;
+	err = struct_ops_module__load(skel);
+	if (!ASSERT_OK(err, "struct_ops_module_load"))
+		return;
+
+	link = bpf_map__attach_struct_ops(skel->maps.testmod_1);
+	ASSERT_OK_PTR(link, "attach_test_mod_1");
+
+	ASSERT_EQ(skel->bss->test_2_result, 7, "test_2_result");
+
+	bpf_link__destroy(link);
+
+	struct_ops_module__destroy(skel);
+}
+
+void serial_test_struct_ops_module(void)
+{
+	if (test__start_subtest("regular_load"))
+		test_regular_load();
+}
+

tools/testing/selftests/bpf/progs/struct_ops_module.c new file +30 -0

new file mode 100644
index 000000000000..cb305d04342f
@@ -0,0 +1,30 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+#include <vmlinux.h>
+#include <bpf/bpf_helpers.h>
+#include <bpf/bpf_tracing.h>
+#include "../bpf_testmod/bpf_testmod.h"
+
+char _license[] SEC("license") = "GPL";
+
+int test_2_result = 0;
+
+SEC("struct_ops/test_1")
+int BPF_PROG(test_1)
+{
+	return 0xdeadbeef;
+}
+
+SEC("struct_ops/test_2")
+int BPF_PROG(test_2, int a, int b)
+{
+	test_2_result = a + b;
+	return a + b;
+}
+
+SEC(".struct_ops.link")
+struct bpf_testmod_ops testmod_1 = {
+	.test_1 = (void *)test_1,
+	.test_2 = (void *)test_2,
+};
+

tools/testing/selftests/bpf/testing_helpers.c +35 -0

index 8d994884c7b4..05870cd62458 100644
@@ -10,6 +10,7 @@
  #include "test_progs.h"
  #include "testing_helpers.h"
  #include <linux/membarrier.h>
+#include "rcu_tasks_trace_gp.skel.h"
 
  int parse_num_list(const char *s, bool **num_set, int *num_set_len)
  {

@@ -380,10 +381,44 @@ int load_bpf_testmod(bool verbose)
  	return 0;
  }
 
+/* This function will trigger call_rcu_tasks_trace() in the kernel */
+static int kern_sync_rcu_tasks_trace(void)
+{
+	struct rcu_tasks_trace_gp *rcu;
+	time_t start;
+	long gp_seq;
+	LIBBPF_OPTS(bpf_test_run_opts, opts);
+
+	rcu = rcu_tasks_trace_gp__open_and_load();
+	if (IS_ERR(rcu))
+		return -EFAULT;
+	if (rcu_tasks_trace_gp__attach(rcu))
+		return -EFAULT;
+
+	gp_seq = READ_ONCE(rcu->bss->gp_seq);
+
+	if (bpf_prog_test_run_opts(bpf_program__fd(rcu->progs.do_call_rcu_tasks_trace),
+				   &opts))
+		return -EFAULT;
+	if (opts.retval != 0)
+		return -EFAULT;
+
+	start = time(NULL);
+	while ((start + 2) > time(NULL) &&
+	       gp_seq == READ_ONCE(rcu->bss->gp_seq))
+		sched_yield();
+
+	rcu_tasks_trace_gp__destroy(rcu);
+
+	return 0;
+}
+
  /*
   * Trigger synchronize_rcu() in kernel.
   */
  int kern_sync_rcu(void)
  {
+	if (kern_sync_rcu_tasks_trace())
+		return -EFAULT;
  	return syscall(__NR_membarrier, MEMBARRIER_CMD_SHARED, 0, 0);
  }