[PATCH bpf-next v5 6/9] bpf, net: switch to dynamic registration

[PATCH bpf-next v5 6/9] bpf, net: switch to dynamic registration

[thinker.li]

From: Kui-Feng Lee <thinker.li@gmail.com>

Replace the static list of struct_ops types with pre-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: netdev@vger.kernel.org
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
 include/linux/bpf.h               |   2 +
 include/linux/btf.h               |  29 +++++++
 kernel/bpf/bpf_struct_ops.c       | 124 +++++++++++++++---------------
 kernel/bpf/bpf_struct_ops_types.h |  12 ---
 kernel/bpf/btf.c                  |   2 +-
 net/bpf/bpf_dummy_struct_ops.c    |  14 +++-
 net/ipv4/bpf_tcp_ca.c             |  16 +++-
 7 files changed, 119 insertions(+), 80 deletions(-)
 delete mode 100644 kernel/bpf/bpf_struct_ops_types.h

diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 1e1647c8b0ce..b0f33147aa93 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -3207,4 +3207,6 @@ 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);
+
 #endif /* _LINUX_BPF_H */
diff --git a/include/linux/btf.h b/include/linux/btf.h
index aa2ba77648be..fdc83aa10462 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -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,
@@ -577,4 +580,30 @@ int btf_add_struct_ops(struct bpf_struct_ops *st_ops);
 const struct bpf_struct_ops **
 btf_get_struct_ops(struct btf *btf, u32 *ret_cnt);
 
+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
+
+/* 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 {					\
+	BPF_STRUCT_OPS_COMMON_VALUE;				\
+	struct _name data ____cacheline_aligned_in_smp;		\
+}
+
 #endif
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 60445ff32275..175068b083cb 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -13,19 +13,6 @@
 #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;
 	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 * const bpf_struct_ops[] = {
-#define BPF_STRUCT_OPS_TYPE(_name)				\
-	[BPF_STRUCT_OPS_TYPE_##_name] = &bpf_##_name,
-#include "bpf_struct_ops_types.h"
-#undef BPF_STRUCT_OPS_TYPE
-};
-
 const struct bpf_verifier_ops bpf_struct_ops_verifier_ops = {
 };
 
@@ -234,16 +192,51 @@ static void bpf_struct_ops_init_one(struct bpf_struct_ops *st_ops,
 
 }
 
+static int register_bpf_struct_ops_btf(struct bpf_struct_ops *st_ops,
+				       struct btf *btf)
+{
+	struct bpf_verifier_log *log;
+	int err;
+
+	if (st_ops == NULL)
+		return -EINVAL;
+
+	log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);
+	if (!log) {
+		err = -ENOMEM;
+		goto errout;
+	}
+
+	log->level = BPF_LOG_KERNEL;
+
+	bpf_struct_ops_init_one(st_ops, btf, st_ops->owner, log);
+
+	err = btf_add_struct_ops(st_ops);
+
+errout:
+	kfree(log);
+
+	return err;
+}
+
+int register_bpf_struct_ops(struct bpf_struct_ops *st_ops)
+{
+	struct btf *btf;
+	int err;
+
+	btf = btf_get_module_btf(st_ops->owner);
+	if (!btf)
+		return -EINVAL;
+	err = register_bpf_struct_ops_btf(st_ops, btf);
+	btf_put(btf);
+
+	return err;
+}
+EXPORT_SYMBOL_GPL(register_bpf_struct_ops);
+
 void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
 {
-	struct bpf_struct_ops *st_ops;
 	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) {
@@ -259,11 +252,6 @@ void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
 		return;
 	}
 	common_value_type = btf_type_by_id(btf, common_value_id);
-
-	for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
-		st_ops = bpf_struct_ops[i];
-		bpf_struct_ops_init_one(st_ops, btf, NULL, log);
-	}
 }
 
 extern struct btf *btf_vmlinux;
@@ -271,32 +259,44 @@ extern struct btf *btf_vmlinux;
 static const struct bpf_struct_ops *
 bpf_struct_ops_find_value(struct btf *btf, u32 value_id)
 {
+	const struct bpf_struct_ops *st_ops = NULL;
+	const struct bpf_struct_ops **st_ops_list;
 	unsigned int i;
+	u32 cnt = 0;
 
 	if (!value_id || !btf_vmlinux)
 		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) {
+			st_ops = st_ops_list[i];
+			break;
+		}
 	}
 
-	return NULL;
+	return st_ops;
 }
 
 const struct bpf_struct_ops *bpf_struct_ops_find(struct btf *btf, u32 type_id)
 {
+	const struct bpf_struct_ops *st_ops = NULL;
+	const struct bpf_struct_ops **st_ops_list;
 	unsigned int i;
+	u32 cnt;
 
 	if (!type_id || !btf_vmlinux)
 		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) {
+			st_ops = st_ops_list[i];
+			break;
+		}
 	}
 
-	return NULL;
+	return st_ops;
 }
 
 static int bpf_struct_ops_map_get_next_key(struct bpf_map *map, void *key,
diff --git a/kernel/bpf/bpf_struct_ops_types.h b/kernel/bpf/bpf_struct_ops_types.h
deleted file mode 100644
index 5678a9ddf817..000000000000
--- a/kernel/bpf/bpf_struct_ops_types.h
+++ /dev/null
@@ -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
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index be5144dbb53d..990973d6057d 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -7532,7 +7532,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;
diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c
index 5918d1b32e19..724bb7224079 100644
--- a/net/bpf/bpf_dummy_struct_ops.c
+++ b/net/bpf/bpf_dummy_struct_ops.c
@@ -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, ...);
@@ -216,11 +216,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,
@@ -228,4 +230,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);
diff --git a/net/ipv4/bpf_tcp_ca.c b/net/ipv4/bpf_tcp_ca.c
index 39dcccf0f174..20c401c73dfb 100644
--- a/net/ipv4/bpf_tcp_ca.c
+++ b/net/ipv4/bpf_tcp_ca.c
@@ -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),
@@ -271,7 +271,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,
@@ -281,10 +283,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);
-- 
2.34.1

Re: [PATCH bpf-next v5 6/9] bpf, net: switch to dynamic registration

[Martin KaFai Lau]

On 10/17/23 9:23 AM, thinker.li@gmail.com wrote:
> From: Kui-Feng Lee <thinker.li@gmail.com>
> 
> Replace the static list of struct_ops types with pre-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: netdev@vger.kernel.org
> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
> ---
>   include/linux/bpf.h               |   2 +
>   include/linux/btf.h               |  29 +++++++
>   kernel/bpf/bpf_struct_ops.c       | 124 +++++++++++++++---------------
>   kernel/bpf/bpf_struct_ops_types.h |  12 ---
>   kernel/bpf/btf.c                  |   2 +-
>   net/bpf/bpf_dummy_struct_ops.c    |  14 +++-
>   net/ipv4/bpf_tcp_ca.c             |  16 +++-
>   7 files changed, 119 insertions(+), 80 deletions(-)
>   delete mode 100644 kernel/bpf/bpf_struct_ops_types.h
> 
> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
> index 1e1647c8b0ce..b0f33147aa93 100644
> --- a/include/linux/bpf.h
> +++ b/include/linux/bpf.h
> @@ -3207,4 +3207,6 @@ 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);
> +
>   #endif /* _LINUX_BPF_H */
> diff --git a/include/linux/btf.h b/include/linux/btf.h
> index aa2ba77648be..fdc83aa10462 100644
> --- a/include/linux/btf.h
> +++ b/include/linux/btf.h
> @@ -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,
> @@ -577,4 +580,30 @@ int btf_add_struct_ops(struct bpf_struct_ops *st_ops);
>   const struct bpf_struct_ops **
>   btf_get_struct_ops(struct btf *btf, u32 *ret_cnt);
>   
> +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

Since there is 'struct bpf_struct_ops_common_value' now, the 
BPF_STRUCT_OPS_COMMON_VALUE macro is not as useful as before. Lets remove it.

> +
> +/* 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 {					\
> +	BPF_STRUCT_OPS_COMMON_VALUE;				\
> +	struct _name data ____cacheline_aligned_in_smp;		\
> +}

I think the bpp_struct_ops_* should not be in btf.h. Probably move them to bpf.h 
instead. or there is some other considerations I am missing?

> +
>   #endif
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index 60445ff32275..175068b083cb 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -13,19 +13,6 @@
>   #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;
>   	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 * const bpf_struct_ops[] = {
> -#define BPF_STRUCT_OPS_TYPE(_name)				\
> -	[BPF_STRUCT_OPS_TYPE_##_name] = &bpf_##_name,
> -#include "bpf_struct_ops_types.h"
> -#undef BPF_STRUCT_OPS_TYPE
> -};
> -
>   const struct bpf_verifier_ops bpf_struct_ops_verifier_ops = {
>   };
>   
> @@ -234,16 +192,51 @@ static void bpf_struct_ops_init_one(struct bpf_struct_ops *st_ops,
>   
>   }
>   
> +static int register_bpf_struct_ops_btf(struct bpf_struct_ops *st_ops,
> +				       struct btf *btf)

Please combine this function into register_bpf_struct_ops(). They are both very 
short.

> +{
> +	struct bpf_verifier_log *log;
> +	int err;
> +
> +	if (st_ops == NULL)
> +		return -EINVAL;
> +
> +	log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);
> +	if (!log) {
> +		err = -ENOMEM;
> +		goto errout;
> +	}
> +
> +	log->level = BPF_LOG_KERNEL;
> +
> +	bpf_struct_ops_init_one(st_ops, btf, st_ops->owner, log);
> +
> +	err = btf_add_struct_ops(st_ops);
> +
> +errout:
> +	kfree(log);
> +
> +	return err;
> +}
> +
> +int register_bpf_struct_ops(struct bpf_struct_ops *st_ops)

Similar to the register kfunc counterpart, can this be moved to btf.c instead by 
extern-ing bpf_struct_ops_init_one()? or there are some other structs/functions 
need to extern?

> +{
> +	struct btf *btf;
> +	int err;
> +
> +	btf = btf_get_module_btf(st_ops->owner);
> +	if (!btf)
> +		return -EINVAL;
> +	err = register_bpf_struct_ops_btf(st_ops, btf);
> +	btf_put(btf);
> +
> +	return err;
> +}
> +EXPORT_SYMBOL_GPL(register_bpf_struct_ops);
> +
>   void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)

The bpf_struct_ops_init() is pretty much only finding the btf "module_id" and 
"common_value_id". Lets use the BTF_ID_LIST to do it instead. Then the newly 
added bpf_struct_ops_init_one() could use a proper name bpf_struct_ops_init() 
instead of having the special "_one" suffix.

>   {
> -	struct bpf_struct_ops *st_ops;
>   	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) {
> @@ -259,11 +252,6 @@ void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
>   		return;
>   	}
>   	common_value_type = btf_type_by_id(btf, common_value_id);
> -
> -	for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
> -		st_ops = bpf_struct_ops[i];
> -		bpf_struct_ops_init_one(st_ops, btf, NULL, log);
> -	}
>   }
>   
>   extern struct btf *btf_vmlinux;
> @@ -271,32 +259,44 @@ extern struct btf *btf_vmlinux;
>   static const struct bpf_struct_ops *
>   bpf_struct_ops_find_value(struct btf *btf, u32 value_id)
>   {
> +	const struct bpf_struct_ops *st_ops = NULL;
> +	const struct bpf_struct_ops **st_ops_list;
>   	unsigned int i;
> +	u32 cnt = 0;
>   
>   	if (!value_id || !btf_vmlinux)

The "!btf_vmlinux" should have been changed to "!btf" in the earlier patch 
(patch 2?),

and is this null check still needed now?

>   		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) {
> +			st_ops = st_ops_list[i];

nit. Like the change in the earlier patch that is being replaced here,
directly "return st_ops_list[i];".

> +			break;
> +		}
>   	}
>   
> -	return NULL;
> +	return st_ops;
>   }
>   
>   const struct bpf_struct_ops *bpf_struct_ops_find(struct btf *btf, u32 type_id)
>   {
> +	const struct bpf_struct_ops *st_ops = NULL;
> +	const struct bpf_struct_ops **st_ops_list;
>   	unsigned int i;
> +	u32 cnt;
>   
>   	if (!type_id || !btf_vmlinux)
>   		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) {
> +			st_ops = st_ops_list[i];

Same.

> +			break;
> +		}
>   	}
>   
> -	return NULL;
> +	return st_ops;
>   }
>   
>   static int bpf_struct_ops_map_get_next_key(struct bpf_map *map, void *key,
> diff --git a/kernel/bpf/bpf_struct_ops_types.h b/kernel/bpf/bpf_struct_ops_types.h
> deleted file mode 100644
> index 5678a9ddf817..000000000000
> --- a/kernel/bpf/bpf_struct_ops_types.h
> +++ /dev/null
> @@ -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

Seeing this gone is satisfying.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index be5144dbb53d..990973d6057d 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -7532,7 +7532,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;
> diff --git a/net/bpf/bpf_dummy_struct_ops.c b/net/bpf/bpf_dummy_struct_ops.c
> index 5918d1b32e19..724bb7224079 100644
> --- a/net/bpf/bpf_dummy_struct_ops.c
> +++ b/net/bpf/bpf_dummy_struct_ops.c
> @@ -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;

Is it still needed ?

Re: [PATCH bpf-next v5 6/9] bpf, net: switch to dynamic registration

[Kui-Feng Lee]

On 10/18/23 18:49, Martin KaFai Lau wrote:
> On 10/17/23 9:23 AM, thinker.li@gmail.com wrote:
>> From: Kui-Feng Lee <thinker.li@gmail.com>
>>
>> Replace the static list of struct_ops types with pre-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: netdev@vger.kernel.org
>> Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
>> ---
>>   include/linux/bpf.h               |   2 +
>>   include/linux/btf.h               |  29 +++++++
>>   kernel/bpf/bpf_struct_ops.c       | 124 +++++++++++++++---------------
>>   kernel/bpf/bpf_struct_ops_types.h |  12 ---
>>   kernel/bpf/btf.c                  |   2 +-
>>   net/bpf/bpf_dummy_struct_ops.c    |  14 +++-
>>   net/ipv4/bpf_tcp_ca.c             |  16 +++-
>>   7 files changed, 119 insertions(+), 80 deletions(-)
>>   delete mode 100644 kernel/bpf/bpf_struct_ops_types.h
>>
>> diff --git a/include/linux/bpf.h b/include/linux/bpf.h
>> index 1e1647c8b0ce..b0f33147aa93 100644
>> --- a/include/linux/bpf.h
>> +++ b/include/linux/bpf.h
>> @@ -3207,4 +3207,6 @@ 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);
>> +
>>   #endif /* _LINUX_BPF_H */
>> diff --git a/include/linux/btf.h b/include/linux/btf.h
>> index aa2ba77648be..fdc83aa10462 100644
>> --- a/include/linux/btf.h
>> +++ b/include/linux/btf.h
>> @@ -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,
>> @@ -577,4 +580,30 @@ int btf_add_struct_ops(struct bpf_struct_ops 
>> *st_ops);
>>   const struct bpf_struct_ops **
>>   btf_get_struct_ops(struct btf *btf, u32 *ret_cnt);
>> +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
> 
> Since there is 'struct bpf_struct_ops_common_value' now, the 
> BPF_STRUCT_OPS_COMMON_VALUE macro is not as useful as before. Lets 
> remove it.

Agree

> 
>> +
>> +/* 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 {                    \
>> +    BPF_STRUCT_OPS_COMMON_VALUE;                \
>> +    struct _name data ____cacheline_aligned_in_smp;        \
>> +}
> 
> I think the bpp_struct_ops_* should not be in btf.h. Probably move them 
> to bpf.h instead. or there is some other considerations I am missing?

Yes, I think bpf.h is the right place.

> 
>> +
>>   #endif
>> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
>> index 60445ff32275..175068b083cb 100644
>> --- a/kernel/bpf/bpf_struct_ops.c
>> +++ b/kernel/bpf/bpf_struct_ops.c
>> @@ -13,19 +13,6 @@
>>   #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;
>>       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 * const bpf_struct_ops[] = {
>> -#define BPF_STRUCT_OPS_TYPE(_name)                \
>> -    [BPF_STRUCT_OPS_TYPE_##_name] = &bpf_##_name,
>> -#include "bpf_struct_ops_types.h"
>> -#undef BPF_STRUCT_OPS_TYPE
>> -};
>> -
>>   const struct bpf_verifier_ops bpf_struct_ops_verifier_ops = {
>>   };
>> @@ -234,16 +192,51 @@ static void bpf_struct_ops_init_one(struct 
>> bpf_struct_ops *st_ops,
>>   }
>> +static int register_bpf_struct_ops_btf(struct bpf_struct_ops *st_ops,
>> +                       struct btf *btf)
> 
> Please combine this function into register_bpf_struct_ops(). They are 
> both very short.
> 

Got it!

>> +{
>> +    struct bpf_verifier_log *log;
>> +    int err;
>> +
>> +    if (st_ops == NULL)
>> +        return -EINVAL;
>> +
>> +    log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);
>> +    if (!log) {
>> +        err = -ENOMEM;
>> +        goto errout;
>> +    }
>> +
>> +    log->level = BPF_LOG_KERNEL;
>> +
>> +    bpf_struct_ops_init_one(st_ops, btf, st_ops->owner, log);
>> +
>> +    err = btf_add_struct_ops(st_ops);
>> +
>> +errout:
>> +    kfree(log);
>> +
>> +    return err;
>> +}
>> +
>> +int register_bpf_struct_ops(struct bpf_struct_ops *st_ops)
> 
> Similar to the register kfunc counterpart, can this be moved to btf.c 
> instead by extern-ing bpf_struct_ops_init_one()? or there are some other 
> structs/functions need to extern?

It is wierd to move a function of bpf_struct_ops to btf.
But, kfunc already did that, I don't mind to follow it.

> 
>> +{
>> +    struct btf *btf;
>> +    int err;
>> +
>> +    btf = btf_get_module_btf(st_ops->owner);
>> +    if (!btf)
>> +        return -EINVAL;
>> +    err = register_bpf_struct_ops_btf(st_ops, btf);
>> +    btf_put(btf);
>> +
>> +    return err;
>> +}
>> +EXPORT_SYMBOL_GPL(register_bpf_struct_ops);
>> +
>>   void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
> 
> The bpf_struct_ops_init() is pretty much only finding the btf 
> "module_id" and "common_value_id". Lets use the BTF_ID_LIST to do it 
> instead. Then the newly added bpf_struct_ops_init_one() could use a 
> proper name bpf_struct_ops_init() instead of having the special "_one" 
> suffix.

Got it!

> 
>>   {
>> -    struct bpf_struct_ops *st_ops;
>>       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) {
>> @@ -259,11 +252,6 @@ void bpf_struct_ops_init(struct btf *btf, struct 
>> bpf_verifier_log *log)
>>           return;
>>       }
>>       common_value_type = btf_type_by_id(btf, common_value_id);
>> -
>> -    for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
>> -        st_ops = bpf_struct_ops[i];
>> -        bpf_struct_ops_init_one(st_ops, btf, NULL, log);
>> -    }
>>   }
>>   extern struct btf *btf_vmlinux;
>> @@ -271,32 +259,44 @@ extern struct btf *btf_vmlinux;
>>   static const struct bpf_struct_ops *
>>   bpf_struct_ops_find_value(struct btf *btf, u32 value_id)
>>   {
>> +    const struct bpf_struct_ops *st_ops = NULL;
>> +    const struct bpf_struct_ops **st_ops_list;
>>       unsigned int i;
>> +    u32 cnt = 0;
>>       if (!value_id || !btf_vmlinux)
> 
> The "!btf_vmlinux" should have been changed to "!btf" in the earlier 
> patch (patch 2?),

This is not btf. It mean to check if btf_vmlinux is initialized.
It is not necessary anymore.
For checking btf, the following btf_get_struct_ops() will keep cnt zero
if btf is NULL, so it is unnecessary as well.

> 
> and is this null check still needed now?
> 
>>           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) {
>> +            st_ops = st_ops_list[i];
> 
> nit. Like the change in the earlier patch that is being replaced here,
> directly "return st_ops_list[i];".

Got it!

> 
>> +            break;
>> +        }
>>       }
>> -    return NULL;
>> +    return st_ops;
>>   }
>>   const struct bpf_struct_ops *bpf_struct_ops_find(struct btf *btf, 
>> u32 type_id)
>>   {
>> +    const struct bpf_struct_ops *st_ops = NULL;
>> +    const struct bpf_struct_ops **st_ops_list;
>>       unsigned int i;
>> +    u32 cnt;
>>       if (!type_id || !btf_vmlinux)
>>           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) {
>> +            st_ops = st_ops_list[i];
> 
> Same.

Ack!

> 
>> +            break;
>> +        }
>>       }
>> -    return NULL;
>> +    return st_ops;
>>   }
>>   static int bpf_struct_ops_map_get_next_key(struct bpf_map *map, void 
>> *key,
>> diff --git a/kernel/bpf/bpf_struct_ops_types.h 
>> b/kernel/bpf/bpf_struct_ops_types.h
>> deleted file mode 100644
>> index 5678a9ddf817..000000000000
>> --- a/kernel/bpf/bpf_struct_ops_types.h
>> +++ /dev/null
>> @@ -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
> 
> Seeing this gone is satisfying
> 
>> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
>> index be5144dbb53d..990973d6057d 100644
>> --- a/kernel/bpf/btf.c
>> +++ b/kernel/bpf/btf.c
>> @@ -7532,7 +7532,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;
>> diff --git a/net/bpf/bpf_dummy_struct_ops.c 
>> b/net/bpf/bpf_dummy_struct_ops.c
>> index 5918d1b32e19..724bb7224079 100644
>> --- a/net/bpf/bpf_dummy_struct_ops.c
>> +++ b/net/bpf/bpf_dummy_struct_ops.c
>> @@ -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;
> 
> Is it still needed ?

Yes, it will be used by bpf_struct_ops_test_run().


> 
> 
> 

Re: [PATCH bpf-next v5 6/9] bpf, net: switch to dynamic registration

[Kui-Feng Lee]

On 10/20/23 08:12, Kui-Feng Lee wrote:
> 
> 
> On 10/18/23 18:49, Martin KaFai Lau wrote:
>> On 10/17/23 9:23 AM, thinker.li@gmail.com wrote:
>>> From: Kui-Feng Lee <thinker.li@gmail.com>
>>>   static const struct bpf_struct_ops *
>>>   bpf_struct_ops_find_value(struct btf *btf, u32 value_id)
>>>   {
>>> +    const struct bpf_struct_ops *st_ops = NULL;
>>> +    const struct bpf_struct_ops **st_ops_list;
>>>       unsigned int i;
>>> +    u32 cnt = 0;
>>>       if (!value_id || !btf_vmlinux)
>>
>> The "!btf_vmlinux" should have been changed to "!btf" in the earlier 
>> patch (patch 2?),
> 
> This is not btf. It mean to check if btf_vmlinux is initialized.
> It is not necessary anymore.
> For checking btf, the following btf_get_struct_ops() will keep cnt zero
> if btf is NULL, so it is unnecessary as well.

Forget my previous comment.  I think you are right!