Registrating struct_ops types from modules

RFC Superseded [email protected] 3 participants 16 messages Sep 13, 2023 – Sep 18, 2023

Subsystems (6)

Kselftest BPF Arm STM32 BPF selftests +2 more subsystems

lore

Share & tools

lore

[email protected]

author

2023-09-13 06:15 UTC #0 lore raw

v2 cover Registrating struct_ops types from modules

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

Given the current constraints of the current implementation, struct_ops cannot be registered dynamically. This presents a significant limitation for modules like coming fuse-bpf, which seeks to implement a new struct_ops type. To address this issue, a new API is introduced that allows the registration of new struct_ops types from modules.

Previously, struct_ops types were defined in bpf_struct_ops_types.h and collected as a static array. The new API lets callers add new struct_ops types to the array dynamically.

The struct_ops subsystem relies on BTF to determine the layout of values in a struct_ops map and identify the subsystem that the struct_ops map registers to. However, the kernel BTF does not include the type information of struct_ops types defined by a module. The struct_ops subsystem requires knowledge of the corresponding module for a given struct_ops map and the utilization of BTF information from that module. We empower libbpf to determine the correct module for accessing the BTF information and pass an identity (FD) to the kernel.

If a module exits while one or more struct_ops maps still refer to a struct_ops type defined by the module, it can lead to unforeseen complications. Therefore, it is crucial to ensure that a module remains intact as long as any struct_ops map is still linked to a struct_ops type defined by the module. To achieve this, you can hold a reference to the module for each struct_ops map.

Kui-Feng Lee (9):
bpf: refactory struct_ops type initialization to a function.
bpf: add register and unregister functions for struct_ops.
bpf: attach a module BTF to a bpf_struct_ops
bpf: use attached BTF to find correct type info of struct_ops progs.
bpf: hold module for bpf_struct_ops_map.
libbpf: Find correct module BTFs for struct_ops maps and progs.
bpf: export btf_ctx_access to modules.
selftests/bpf: test case for register_bpf_struct_ops(). Comments and debug

 include/linux/bpf.h                           |  19 +-
 include/linux/btf.h                           |   1 +
 include/uapi/linux/bpf.h                      |   4 +
 kernel/bpf/bpf_struct_ops.c                   | 346 +++++++++++++-----
 kernel/bpf/btf.c                              |   3 +-
 kernel/bpf/syscall.c                          |   8 +-
 kernel/bpf/verifier.c                         |   4 +-
 tools/include/uapi/linux/bpf.h                |   4 +
 tools/lib/bpf/bpf.c                           |   3 +-
 tools/lib/bpf/bpf.h                           |   4 +-
 tools/lib/bpf/libbpf.c                        | 147 +++++---
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   |  84 +++++
 .../selftests/bpf/bpf_testmod/bpf_testmod.h   |   5 +
 .../bpf/prog_tests/test_struct_ops_module.c   |  40 ++
 .../selftests/bpf/progs/struct_ops_module.c   |  30 ++
 15 files changed, 558 insertions(+), 144 deletions(-)
 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

Signature · 1 line

2.34.1

[email protected]

author

2023-09-13 06:15 UTC #1 lore raw

RFC v2 1/9 bpf: refactory struct_ops type initialization to a function.

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

Move most of code to bpf_struct_ops_init_one() that can be use to initialize new struct_ops types registered dynamically.

---
 kernel/bpf/bpf_struct_ops.c | 157 +++++++++++++++++++-----------------
 1 file changed, 83 insertions(+), 74 deletions(-)

kernel/bpf/bpf_struct_ops.c +83 ~~-74~~

index fdc3e8705a3c..1662875e0ebe 100644
@@ -110,102 +110,111 @@ const struct bpf_prog_ops bpf_struct_ops_prog_ops = {
 
 static const struct btf_type *module_type;
 
-void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
+static void bpf_struct_ops_init_one(struct bpf_struct_ops *st_ops,
+				    struct btf *btf,
+				    struct bpf_verifier_log *log)
 {
-	s32 type_id, value_id, module_id;
 	const struct btf_member *member;
-	struct bpf_struct_ops *st_ops;
 	const struct btf_type *t;
+	s32 type_id, value_id;
 	char value_name[128];
 	const char *mname;
-	u32 i, j;
+	int 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
+	if (strlen(st_ops->name) + VALUE_PREFIX_LEN >=
+	    sizeof(value_name)) {
+		pr_warn("struct_ops name %s is too long\n",
+			st_ops->name);
+		return;
+	}
+	sprintf(value_name, "%s%s", VALUE_PREFIX, st_ops->name);
 
-	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");
+	value_id = btf_find_by_name_kind(btf, value_name,
+					 BTF_KIND_STRUCT);
+	if (value_id < 0) {
+		pr_warn("Cannot find struct %s in btf_vmlinux\n",
+			value_name);
 		return;
 	}
-	module_type = btf_type_by_id(btf, module_id);
 
-	for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
-		st_ops = bpf_struct_ops[i];
+	type_id = btf_find_by_name_kind(btf, st_ops->name,
+					BTF_KIND_STRUCT);
+	if (type_id < 0) {
+		pr_warn("Cannot find struct %s in btf_vmlinux\n",
+			st_ops->name);
+		return;
+	}
+	t = btf_type_by_id(btf, type_id);
+	if (btf_type_vlen(t) > BPF_STRUCT_OPS_MAX_NR_MEMBERS) {
+		pr_warn("Cannot support #%u members in struct %s\n",
+			btf_type_vlen(t), st_ops->name);
+		return;
+	}
 
-		if (strlen(st_ops->name) + VALUE_PREFIX_LEN >=
-		    sizeof(value_name)) {
-			pr_warn("struct_ops name %s is too long\n",
+	for_each_member(i, t, member) {
+		const struct btf_type *func_proto;
+
+		mname = btf_name_by_offset(btf, member->name_off);
+		if (!*mname) {
+			pr_warn("anon member in struct %s is not supported\n",
 				st_ops->name);
-			continue;
+			break;
 		}
-		sprintf(value_name, "%s%s", VALUE_PREFIX, st_ops->name);
 
-		value_id = btf_find_by_name_kind(btf, value_name,
-						 BTF_KIND_STRUCT);
-		if (value_id < 0) {
-			pr_warn("Cannot find struct %s in btf_vmlinux\n",
-				value_name);
-			continue;
+		if (__btf_member_bitfield_size(t, member)) {
+			pr_warn("bit field member %s in struct %s is not supported\n",
+				mname, st_ops->name);
+			break;
 		}
 
-		type_id = btf_find_by_name_kind(btf, st_ops->name,
-						BTF_KIND_STRUCT);
-		if (type_id < 0) {
-			pr_warn("Cannot find struct %s in btf_vmlinux\n",
-				st_ops->name);
-			continue;
-		}
-		t = btf_type_by_id(btf, type_id);
-		if (btf_type_vlen(t) > BPF_STRUCT_OPS_MAX_NR_MEMBERS) {
-			pr_warn("Cannot support #%u members in struct %s\n",
-				btf_type_vlen(t), st_ops->name);
-			continue;
+		func_proto = btf_type_resolve_func_ptr(btf,
+						       member->type,
+						       NULL);
+		if (func_proto &&
+		    btf_distill_func_proto(log, btf,
+					   func_proto, mname,
+					   &st_ops->func_models[i])) {
+			pr_warn("Error in parsing func ptr %s in struct %s\n",
+				mname, st_ops->name);
+			break;
 		}
+	}
 
-		for_each_member(j, t, member) {
-			const struct btf_type *func_proto;
+	if (i == btf_type_vlen(t)) {
+		if (st_ops->init(btf)) {
+			pr_warn("Error in init bpf_struct_ops %s\n",
+				st_ops->name);
+		} else {
+			st_ops->type_id = type_id;
+			st_ops->type = t;
+			st_ops->value_id = value_id;
+			st_ops->value_type = btf_type_by_id(btf,
+							    value_id);
+		}
+	}
+}
 
-			mname = btf_name_by_offset(btf, member->name_off);
-			if (!*mname) {
-				pr_warn("anon member in struct %s is not supported\n",
-					st_ops->name);
-				break;
-			}
+void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
+{
+	struct bpf_struct_ops *st_ops;
+	s32 module_id;
+	u32 i;
 
-			if (__btf_member_bitfield_size(t, member)) {
-				pr_warn("bit field member %s in struct %s is not supported\n",
-					mname, st_ops->name);
-				break;
-			}
+	/* 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
 
-			func_proto = btf_type_resolve_func_ptr(btf,
-							       member->type,
-							       NULL);
-			if (func_proto &&
-			    btf_distill_func_proto(log, btf,
-						   func_proto, mname,
-						   &st_ops->func_models[j])) {
-				pr_warn("Error in parsing func ptr %s in struct %s\n",
-					mname, st_ops->name);
-				break;
-			}
-		}
+	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);
 
-		if (j == btf_type_vlen(t)) {
-			if (st_ops->init(btf)) {
-				pr_warn("Error in init bpf_struct_ops %s\n",
-					st_ops->name);
-			} else {
-				st_ops->type_id = type_id;
-				st_ops->type = t;
-				st_ops->value_id = value_id;
-				st_ops->value_type = btf_type_by_id(btf,
-								    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, log);
 	}
 }

Signature · 1 line

2.34.1

Martin KaFai Lau

reviewer

2023-09-15 22:44 UTC #2 lore raw

Re: [RFC bpf-next v2 1/9] bpf: refactory struct_ops type initialization to a function.

Quoted 4 lines · On 9/12/23 11:14 PM, [email protected] wrote:

From: Kui-Feng Lee <[email protected]>
Move most of code to bpf_struct_ops_init_one() that can be use to initialize new struct_ops types registered dynamically.

While in RFC, still better to have SOB so that it won't be overlooked in the future.

Quoted 34 lines

---
  kernel/bpf/bpf_struct_ops.c | 157 +++++++++++++++++++-----------------
  1 file changed, 83 insertions(+), 74 deletions(-)

kernel/bpf/bpf_struct_ops.c +8 -8

index fdc3e8705a3c..1662875e0ebe 100644
@@ -110,102 +110,111 @@ const struct bpf_prog_ops bpf_struct_ops_prog_ops = {
 
  static const struct btf_type *module_type;
 
-void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
+static void bpf_struct_ops_init_one(struct bpf_struct_ops *st_ops,
+				    struct btf *btf,
+				    struct bpf_verifier_log *log)
  {
-	s32 type_id, value_id, module_id;
  	const struct btf_member *member;
-	struct bpf_struct_ops *st_ops;
  	const struct btf_type *t;
+	s32 type_id, value_id;
  	char value_name[128];
  	const char *mname;
-	u32 i, j;
+	int 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
+	if (strlen(st_ops->name) + VALUE_PREFIX_LEN >=
+	    sizeof(value_name)) {
+		pr_warn("struct_ops name %s is too long\n",

kernel/bpf/bpf_struct_ops.c const struct bpf_prog_ops bpf_struct_ops_prog_ops = { · line 133

Quoted 10 lines

kernel/bpf/bpf_struct_ops.c +6 -3

+			st_ops->name);
+		return;
+	}
+	sprintf(value_name, "%s%s", VALUE_PREFIX, st_ops->name);
 
-	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");
+	value_id = btf_find_by_name_kind(btf, value_name,
+					 BTF_KIND_STRUCT);

It needs to do some sanity checks on the value_type since this won't be statically enforced by bpf_struct_ops.c.

[ ... ]

Quoted 5 lines

kernel/bpf/bpf_struct_ops.c +5 -0

+void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
+{
+	struct bpf_struct_ops *st_ops;
+	s32 module_id;
+	u32 i;

kernel/bpf/bpf_struct_ops.c const struct bpf_prog_ops bpf_struct_ops_prog_ops = { · line 206

Quoted 10 lines

kernel/bpf/bpf_struct_ops.c +4 -5

 
-			if (__btf_member_bitfield_size(t, member)) {
-				pr_warn("bit field member %s in struct %s is not supported\n",
-					mname, st_ops->name);
-				break;
-			}
+	/* 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

Can this static way of defining struct_ops be removed? bpf_tcp_ca should be able to use the register_bpf_struct_ops() introduced in patch 2.

For the future subsystem supporting struct_ops, the subsystem could be compiled as a kernel module or as a built-in. register_bpf_struct_ops() should work for both.

Kui-Feng Lee

2023-09-16 01:35 UTC #3 lore raw

Re: [RFC bpf-next v2 1/9] bpf: refactory struct_ops type initialization to a function.

Quoted 44 lines · Martin KaFai Lau

Quoted 4 lines · On 9/12/23 11:14 PM, [email protected] wrote:

From: Kui-Feng Lee <[email protected]>
Move most of code to bpf_struct_ops_init_one() that can be use to initialize new struct_ops types registered dynamically.

While in RFC, still better to have SOB so that it won't be overlooked in the future.

Quoted 36 lines

---
kernel/bpf/bpf_struct_ops.c | 157 +++++++++++++++++++-----------------

  1 file changed, 83 insertions(+), 74 deletions(-)

kernel/bpf/bpf_struct_ops.c +11 -8

index fdc3e8705a3c..1662875e0ebe 100644
@@ -110,102 +110,111 @@ const struct bpf_prog_ops 
 bpf_struct_ops_prog_ops = {
   static const struct btf_type *module_type;
-void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
+static void bpf_struct_ops_init_one(struct bpf_struct_ops *st_ops,
+                    struct btf *btf,
+                    struct bpf_verifier_log *log)
   {
-    s32 type_id, value_id, module_id;
       const struct btf_member *member;
-    struct bpf_struct_ops *st_ops;
       const struct btf_type *t;
+    s32 type_id, value_id;
       char value_name[128];
       const char *mname;
-    u32 i, j;
+    int 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
+    if (strlen(st_ops->name) + VALUE_PREFIX_LEN >=
+        sizeof(value_name)) {
+        pr_warn("struct_ops name %s is too long\n",
+            st_ops->name);
+        return;
+    }

Quoted 8 lines

kernel/bpf/bpf_struct_ops.c const struct bpf_prog_ops bpf_struct_ops_prog_ops = { · line 133
Quoted 6 lines
kernel/bpf/bpf_struct_ops.c +3 -3
+    sprintf(value_name, "%s%s", VALUE_PREFIX, st_ops->name);
-    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");
+    value_id = btf_find_by_name_kind(btf, value_name,
+                     BTF_KIND_STRUCT);
It needs to do some sanity checks on the value_type since this won't be statically enforced by bpf_struct_ops.c.

Do you mean to check if a value_type has refcnt, state and data field?

Quoted 13 lines

[ ... ]
Quoted 10 lines
kernel/bpf/bpf_struct_ops.c +5 -2
+void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
+{
+    struct bpf_struct_ops *st_ops;
+    s32 module_id;
+    u32 i;
-            if (__btf_member_bitfield_size(t, member)) {
-                pr_warn("bit field member %s in struct %s is not 
supported\n",
- mname, st_ops->name);
- break;

Quoted 8 lines

kernel/bpf/bpf_struct_ops.c const struct bpf_prog_ops bpf_struct_ops_prog_ops = { · line 206
Quoted 6 lines
- }
+ /* 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);
kernel/bpf/bpf_struct_ops.c +2 -0
+#include "bpf_struct_ops_types.h"
+#undef BPF_STRUCT_OPS_TYPE
Can this static way of defining struct_ops be removed? bpf_tcp_ca should be able to use the register_bpf_struct_ops() introduced in patch 2.

It sounds good for me.

Quoted 3 lines

For the future subsystem supporting struct_ops, the subsystem could be compiled as a kernel module or as a built-in. register_bpf_struct_ops() should work for both.

[email protected]

author

2023-09-13 06:15 UTC #4 lore raw

RFC v2 2/9 bpf: add register and unregister functions for struct_ops.

In reply to [email protected]

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

Provide registration functions to add/remove struct_ops types.

Currently, it does linear search to find a struct_ops type. It should be fine for now since we don't have many struct_ops types.

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

---
 include/linux/bpf.h         |  13 ++++
 include/linux/btf.h         |   1 +
 kernel/bpf/bpf_struct_ops.c | 115 ++++++++++++++++++++++++++++++++++--
 kernel/bpf/btf.c            |   2 +-
 4 files changed, 126 insertions(+), 5 deletions(-)

include/linux/bpf.h +13 -0

index 024e8b28c34b..10f98f0ddc77 100644
@@ -1561,6 +1561,8 @@ struct btf_member;
  * @init: A callback that is invoked a single time, and before any other
  *	  callback, to initialize the structure. A nonzero return value means
  *	  the subsystem could not be initialized.
+ * @uninit: A callback that is invoked after any other
+ *	    callback to release resources used by the subsystem.
  * @check_member: When defined, a callback invoked by the verifier to allow
  *		  the subsystem to determine if an entry in the struct_ops map
  *		  is valid. A nonzero return value means that the map is

@@ -1601,6 +1603,7 @@ struct btf_member;
 struct bpf_struct_ops {
 	const struct bpf_verifier_ops *verifier_ops;
 	int (*init)(struct btf *btf);
+	int (*uninit)(void);
 	int (*check_member)(const struct btf_type *t,
 			    const struct btf_member *member,
 			    const struct bpf_prog *prog);

@@ -1619,6 +1622,11 @@ struct bpf_struct_ops {
 	u32 value_id;
 };
 
+struct bpf_struct_ops_mod {
+	struct module *owner;
+	struct bpf_struct_ops *st_ops;
+};
+
 #if defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_SYSCALL)
 #define BPF_MODULE_OWNER ((void *)((0xeB9FUL << 2) + POISON_POINTER_DELTA))
 const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id);

@@ -3183,4 +3191,9 @@ static inline gfp_t bpf_memcg_flags(gfp_t flags)
 	return flags;
 }
 
+#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+int register_bpf_struct_ops(struct bpf_struct_ops_mod *mod);
+int unregister_bpf_struct_ops(struct bpf_struct_ops_mod *mod);
+#endif
+
 #endif /* _LINUX_BPF_H */

include/linux/btf.h +1 -0

index 928113a80a95..d6ed3d99ba41 100644
@@ -200,6 +200,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,

kernel/bpf/bpf_struct_ops.c +111 -4

149 lines not on this page · raw message

kernel/bpf/btf.c +1 -1

index 1095bbe29859..55d76d85c6ec 100644
@@ -7498,7 +7498,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;

Signature · 1 line

2.34.1

Martin KaFai Lau

reviewer

2023-09-16 00:05 UTC #5 lore raw

Re: [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops.

Quoted 10 lines · On 9/12/23 11:14 PM, [email protected] wrote:

kernel/bpf/bpf_struct_ops.c +10 -0

+int register_bpf_struct_ops(struct bpf_struct_ops_mod *mod)
+{
+	struct bpf_struct_ops *st_ops = mod->st_ops;
+	struct bpf_verifier_log *log;
+	struct btf *btf;
+	int err;
+
+	if (mod->st_ops == NULL ||
+	    mod->owner == NULL)
+		return -EINVAL;

kernel/bpf/bpf_struct_ops.c void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log) · line 299

Quoted 10 lines

kernel/bpf/bpf_struct_ops.c +10 -0

+
+	log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);
+	if (!log) {
+		err = -ENOMEM;
+		goto errout;
+	}
+
+	log->level = BPF_LOG_KERNEL;
+
+	btf = btf_get_module_btf(mod->owner);

Where is btf_put called?

It is not stored anywhere in patch 2, so a bit confusing. I quickly looked at the following patches but also don't see the bpf_put.

Quoted 6 lines

kernel/bpf/bpf_struct_ops.c +6 -0

+	if (!btf) {
+		err = -EINVAL;
+		goto errout;
+	}
+
+	bpf_struct_ops_init_one(st_ops, btf, log);

kernel/bpf/bpf_struct_ops.c void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log) · line 315

Quoted 10 lines

kernel/bpf/bpf_struct_ops.c +10 -0

+	err = add_struct_ops(st_ops);
+
+errout:
+	kfree(log);
+
+	return err;
+}
+EXPORT_SYMBOL(register_bpf_struct_ops);
+
+int unregister_bpf_struct_ops(struct bpf_struct_ops_mod *mod)

It is not clear to me why the subsystem needs to explicitly call unregister_bpf_struct_ops(). Can it be done similar to the module kfunc support (the kfunc_set_tab goes away with the btf)?

Related to this, does it need to maintain a global struct_ops array for all kernel module? Can the struct_ops be maintained under its corresponding module btf itself?

Quoted 11 lines

kernel/bpf/bpf_struct_ops.c +11 -0

+{
+	struct bpf_struct_ops *st_ops = mod->st_ops;
+	int err;
+
+	err = remove_struct_ops(st_ops);
+	if (!err && st_ops->uninit)
+		err = st_ops->uninit();
+
+	return err;
+}
+EXPORT_SYMBOL(unregister_bpf_struct_ops);

Kui-Feng Lee

2023-09-16 01:14 UTC #6 lore raw

Re: [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops.

Quoted 24 lines · Martin KaFai Lau

Quoted 10 lines · On 9/12/23 11:14 PM, [email protected] wrote:
kernel/bpf/bpf_struct_ops.c +10 -0
+int register_bpf_struct_ops(struct bpf_struct_ops_mod *mod)
+{
+    struct bpf_struct_ops *st_ops = mod->st_ops;
+    struct bpf_verifier_log *log;
+    struct btf *btf;
+    int err;
+
+    if (mod->st_ops == NULL ||
+        mod->owner == NULL)
+        return -EINVAL;
kernel/bpf/bpf_struct_ops.c void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log) · line 299
Quoted 10 lines
kernel/bpf/bpf_struct_ops.c +10 -0
+
+    log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);
+    if (!log) {
+        err = -ENOMEM;
+        goto errout;
+    }
+
+    log->level = BPF_LOG_KERNEL;
+
+    btf = btf_get_module_btf(mod->owner);
Where is btf_put called?

Quoted 1 line

It is not stored anywhere in patch 2, so a bit confusing. I quickly looked at the following patches but also don't see the bpf_put.

It is my fault to use it without calling btf_put().
I miss-understood the API, thought it doesn't increase refcount by mistake.

Quoted 11 lines

Quoted 11 lines
+    if (!btf) {
+        err = -EINVAL;
+        goto errout;
+    }
kernel/bpf/bpf_struct_ops.c +6 -0
+
+    bpf_struct_ops_init_one(st_ops, btf, log);
+    err = add_struct_ops(st_ops);
+
+errout:
+    kfree(log);

Quoted 8 lines

kernel/bpf/bpf_struct_ops.c void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log) · line 315
Quoted 6 lines
kernel/bpf/bpf_struct_ops.c +6 -0
+
+    return err;
+}
+EXPORT_SYMBOL(register_bpf_struct_ops);
+
+int unregister_bpf_struct_ops(struct bpf_struct_ops_mod *mod)
It is not clear to me why the subsystem needs to explicitly call unregister_bpf_struct_ops(). Can it be done similar to the module kfunc support (the kfunc_set_tab goes away with the btf)?

It could be. However, registering to module notifier (register_module_notifier()) is more straight forward if we go this way. What do you think?

Quoted 2 lines

Related to this, does it need to maintain a global struct_ops array for all kernel module? Can the struct_ops be maintained under its corresponding module btf itself?

What is the purpose?
We have a global struct_ops array already, although it is not per-module. For now, the number of struct_ops is pretty small. We have only one so far, and it is unlikely to grow fast in near future. It is probably a bit overkill to have per-module ones if this is what you mean.

Quoted 14 lines

Quoted 12 lines

kernel/bpf/bpf_struct_ops.c +11 -0

+{
+    struct bpf_struct_ops *st_ops = mod->st_ops;
+    int err;
+
+    err = remove_struct_ops(st_ops);
+    if (!err && st_ops->uninit)
+        err = st_ops->uninit();
+
+    return err;
+}
+EXPORT_SYMBOL(unregister_bpf_struct_ops);

Martin KaFai Lau

reviewer

2023-09-18 18:48 UTC #7 lore raw

Re: [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops.

Quoted 53 lines · Kui-Feng Lee

Quoted 24 lines · Martin KaFai Lau
Quoted 10 lines · On 9/12/23 11:14 PM, [email protected] wrote:
kernel/bpf/bpf_struct_ops.c +10 -0
+int register_bpf_struct_ops(struct bpf_struct_ops_mod *mod)
+{
+    struct bpf_struct_ops *st_ops = mod->st_ops;
+    struct bpf_verifier_log *log;
+    struct btf *btf;
+    int err;
+
+    if (mod->st_ops == NULL ||
+        mod->owner == NULL)
+        return -EINVAL;
kernel/bpf/bpf_struct_ops.c void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log) · line 299
Quoted 10 lines
kernel/bpf/bpf_struct_ops.c +10 -0
+
+    log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);
+    if (!log) {
+        err = -ENOMEM;
+        goto errout;
+    }
+
+    log->level = BPF_LOG_KERNEL;
+
+    btf = btf_get_module_btf(mod->owner);
Where is btf_put called?
Quoted 1 line
It is not stored anywhere in patch 2, so a bit confusing. I quickly looked at the following patches but also don't see the bpf_put.
It is my fault to use it without calling btf_put().
I miss-understood the API, thought it doesn't increase refcount by mistake.
Quoted 11 lines
Quoted 11 lines
+    if (!btf) {
+        err = -EINVAL;
+        goto errout;
+    }
kernel/bpf/bpf_struct_ops.c +6 -0
+
+    bpf_struct_ops_init_one(st_ops, btf, log);
+    err = add_struct_ops(st_ops);
+
+errout:
+    kfree(log);
Quoted 8 lines
kernel/bpf/bpf_struct_ops.c void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log) · line 315
Quoted 6 lines
kernel/bpf/bpf_struct_ops.c +6 -0
+
+    return err;
+}
+EXPORT_SYMBOL(register_bpf_struct_ops);
+
+int unregister_bpf_struct_ops(struct bpf_struct_ops_mod *mod)
It is not clear to me why the subsystem needs to explicitly call unregister_bpf_struct_ops(). Can it be done similar to the module kfunc support (the kfunc_set_tab goes away with the btf)?

Quoted 3 lines

It could be. However, registering to module notifier (register_module_notifier()) is more straight forward if we go this way. What do you think?

Right, but not sure if struct_ops needs to create yet another notifier considering there is already a btf_module_notify(). It is why the earlier question on btf_put because I was trying to understand if the struct_ops can go away together during btf_free. More on this next.

Quoted 10 lines

Quoted 3 lines
Related to this, does it need to maintain a global struct_ops array for all kernel module? Can the struct_ops be maintained under its corresponding module btf itself?
What is the purpose?
We have a global struct_ops array already, although it is not per-module. For now, the number of struct_ops is pretty small. We have only one so far, and it is unlikely to grow fast in near future. It is probably a bit overkill to have per-module ones if this is what you mean.

The array size is not the concern.

The global struct_ops array was created before btf supporting kernel module. Since then, btf module and kfunc module support were added.

To maintain this global struct_ops array, it needs to register its own module notifier, maintains its own mutex_lock (in patch 5), and also the modified bpf_struct_ops_find*() is searching something under a specific btf module.

afaict, the current btf kfunc support has the infrastructure to do all these (for example, the global LIST_HEAD(btf_modules), btf_module_mutex, btf_module_notify()...etc). Why struct_ops needs to be special and reinvent something which looks very similar to btf kfunc? Did I missing something that struct_ops needs special handling?

Quoted 15 lines

Quoted 13 lines

kernel/bpf/bpf_struct_ops.c +11 -0

+{
+    struct bpf_struct_ops *st_ops = mod->st_ops;
+    int err;
+
+    err = remove_struct_ops(st_ops);
+    if (!err && st_ops->uninit)
+        err = st_ops->uninit();
+
+    return err;
+}
+EXPORT_SYMBOL(unregister_bpf_struct_ops);

Kui-Feng Lee

2023-09-18 20:40 UTC #8 lore raw

Re: [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops.

Quoted 54 lines · Martin KaFai Lau

Quoted 53 lines · Kui-Feng Lee
Quoted 24 lines · Martin KaFai Lau
Quoted 10 lines · On 9/12/23 11:14 PM, [email protected] wrote:
kernel/bpf/bpf_struct_ops.c +10 -0
+int register_bpf_struct_ops(struct bpf_struct_ops_mod *mod)
+{
+    struct bpf_struct_ops *st_ops = mod->st_ops;
+    struct bpf_verifier_log *log;
+    struct btf *btf;
+    int err;
+
+    if (mod->st_ops == NULL ||
+        mod->owner == NULL)
+        return -EINVAL;
kernel/bpf/bpf_struct_ops.c void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log) · line 299
Quoted 10 lines
kernel/bpf/bpf_struct_ops.c +10 -0
+
+    log = kzalloc(sizeof(*log), GFP_KERNEL | __GFP_NOWARN);
+    if (!log) {
+        err = -ENOMEM;
+        goto errout;
+    }
+
+    log->level = BPF_LOG_KERNEL;
+
+    btf = btf_get_module_btf(mod->owner);
Where is btf_put called?
Quoted 1 line
It is not stored anywhere in patch 2, so a bit confusing. I quickly looked at the following patches but also don't see the bpf_put.
It is my fault to use it without calling btf_put().
I miss-understood the API, thought it doesn't increase refcount by mistake.
Quoted 19 lines
Quoted 7 lines
+    if (!btf) {
+        err = -EINVAL;
+        goto errout;
+    }
kernel/bpf/bpf_struct_ops.c +2 -0
+
+    bpf_struct_ops_init_one(st_ops, btf, log);
kernel/bpf/bpf_struct_ops.c void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log) · line 315
Quoted 10 lines
kernel/bpf/bpf_struct_ops.c +10 -0
+    err = add_struct_ops(st_ops);
+
+errout:
+    kfree(log);
+
+    return err;
+}
+EXPORT_SYMBOL(register_bpf_struct_ops);
+
+int unregister_bpf_struct_ops(struct bpf_struct_ops_mod *mod)
It is not clear to me why the subsystem needs to explicitly call unregister_bpf_struct_ops(). Can it be done similar to the module kfunc support (the kfunc_set_tab goes away with the btf)?

Quoted 5 lines

Quoted 3 lines
It could be. However, registering to module notifier (register_module_notifier()) is more straight forward if we go this way. What do you think?
Right, but not sure if struct_ops needs to create yet another notifier considering there is already a btf_module_notify(). It is why the earlier question on btf_put because I was trying to understand if the struct_ops can go away together during btf_free. More on this next.

In short, it is not necessary to have another notifier.
The benefit with a separated notifier is loose coupling without touching btf code. I don't have a strong opinion on this.

Quoted 12 lines

Quoted 11 lines
Quoted 4 lines
Related to this, does it need to maintain a global struct_ops array for all kernel module? Can the struct_ops be maintained under its corresponding module btf itself?
What is the purpose?
We have a global struct_ops array already, although it is not per-module. For now, the number of struct_ops is pretty small. We have only one so far, and it is unlikely to grow fast in near future. It is probably a bit overkill to have per-module ones if this is what you mean.

Quoted 7 lines

The array size is not the concern.
The global struct_ops array was created before btf supporting kernel module. Since then, btf module and kfunc module support were added.
To maintain this global struct_ops array, it needs to register its own module notifier, maintains its own mutex_lock (in patch 5), and also the modified bpf_struct_ops_find*() is searching something under a specific btf module.
afaict, the current btf kfunc support has the infrastructure to do all these (for example, the global LIST_HEAD(btf_modules), btf_module_mutex, btf_module_notify()...etc). Why struct_ops needs to be special and reinvent something which looks very similar to btf kfunc? Did I missing something that struct_ops needs special handling?

I don't think you missing anything.

Quoted 17 lines

Quoted 16 lines

Quoted 14 lines

kernel/bpf/bpf_struct_ops.c +11 -0

+{
+    struct bpf_struct_ops *st_ops = mod->st_ops;
+    int err;
+
+    err = remove_struct_ops(st_ops);
+    if (!err && st_ops->uninit)
+        err = st_ops->uninit();
+
+    return err;
+}
+EXPORT_SYMBOL(unregister_bpf_struct_ops);

[email protected]

author

2023-09-13 06:15 UTC #9 lore raw

RFC v2 3/9 bpf: attach a module BTF to a bpf_struct_ops

In reply to [email protected]

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

Every struct_ops type should has an associated module BTF to provide type information since we are going to allow modules to define and register new struct_ops types. New types may exist only in module itself, and the kernel BTF (vmlinux) doesn't know it at all. The attached module BTF here will be used to resolve type IDs of a struct_ops map.

---
 include/linux/bpf.h         |  5 +++--
 kernel/bpf/bpf_struct_ops.c | 34 +++++++++++++++++++++++-----------
 kernel/bpf/verifier.c       |  2 +-
 3 files changed, 27 insertions(+), 14 deletions(-)

include/linux/bpf.h +3 -2

index 10f98f0ddc77..a9369f982cd5 100644
@@ -1614,6 +1614,7 @@ struct bpf_struct_ops {
 	void (*unreg)(void *kdata);
 	int (*update)(void *kdata, void *old_kdata);
 	int (*validate)(void *kdata);
+	const struct btf *btf;
 	const struct btf_type *type;
 	const struct btf_type *value_type;
 	const char *name;

@@ -1629,7 +1630,7 @@ struct bpf_struct_ops_mod {
 
 #if defined(CONFIG_BPF_JIT) && defined(CONFIG_BPF_SYSCALL)
 #define BPF_MODULE_OWNER ((void *)((0xeB9FUL << 2) + POISON_POINTER_DELTA))
-const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id);
+const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id, struct btf *btf);
 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);

@@ -1672,7 +1673,7 @@ int bpf_struct_ops_test_run(struct bpf_prog *prog, const union bpf_attr *kattr,
 			    union bpf_attr __user *uattr);
 #endif
 #else
-static inline const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id)
+static inline const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id, struct btf *btf)
 {
 	return NULL;
 }

kernel/bpf/bpf_struct_ops.c +23 ~~-11~~

index 9be6e07ccba5..82cc3f0638fa 100644
@@ -188,6 +188,10 @@ static void bpf_struct_ops_init_one(struct bpf_struct_ops *st_ops,
 			pr_warn("Error in init bpf_struct_ops %s\n",
 				st_ops->name);
 		} else {
+			/* XXX: We need a owner (module) here to company
+			 * with type_id and value_id.
+			 */
+			st_ops->btf = btf;
 			st_ops->type_id = type_id;
 			st_ops->type = t;
 			st_ops->value_id = value_id;

@@ -328,7 +332,7 @@ EXPORT_SYMBOL(unregister_bpf_struct_ops);
 extern struct btf *btf_vmlinux;
 
 static const struct bpf_struct_ops *
-bpf_struct_ops_find_value(u32 value_id)
+bpf_struct_ops_find_value(u32 value_id, struct btf *btf)
 {
 	unsigned int i;

@@ -336,14 +340,15 @@ bpf_struct_ops_find_value(u32 value_id)
 		return NULL;
 
 	for (i = 0; i < bpf_struct_ops_num; i++) {
-		if (bpf_struct_ops[i]->value_id == value_id)
+		if (bpf_struct_ops[i]->value_id == value_id &&
+		    bpf_struct_ops[i]->btf == btf)
 			return bpf_struct_ops[i];
 	}
 
 	return NULL;
 }
 
-const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id)
+const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id, struct btf *btf)
 {
 	unsigned int i;

@@ -351,7 +356,8 @@ const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id)
 		return NULL;
 
 	for (i = 0; i < bpf_struct_ops_num; i++) {
-		if (bpf_struct_ops[i]->type_id == type_id)
+		if (bpf_struct_ops[i]->type_id == type_id &&
+		    bpf_struct_ops[i]->btf == btf)
 			return bpf_struct_ops[i];
 	}

@@ -423,7 +429,7 @@ static void bpf_struct_ops_map_put_progs(struct bpf_struct_ops_map *st_map)
 	}
 }
 
-static int check_zero_holes(const struct btf_type *t, void *data)
+static int check_zero_holes(const struct btf *btf, const struct btf_type *t, void *data)
 {
 	const struct btf_member *member;
 	u32 i, moff, msize, prev_mend = 0;

@@ -435,8 +441,8 @@ static int check_zero_holes(const struct btf_type *t, void *data)
 		    memchr_inv(data + prev_mend, 0, moff - prev_mend))
 			return -EINVAL;
 
-		mtype = btf_type_by_id(btf_vmlinux, member->type);
-		mtype = btf_resolve_size(btf_vmlinux, mtype, &msize);
+		mtype = btf_type_by_id(btf, member->type);
+		mtype = btf_resolve_size(btf, mtype, &msize);
 		if (IS_ERR(mtype))
 			return PTR_ERR(mtype);
 		prev_mend = moff + msize;

@@ -489,7 +495,7 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
 	const struct bpf_struct_ops *st_ops = st_map->st_ops;
 	struct bpf_struct_ops_value *uvalue, *kvalue;
 	const struct btf_member *member;
-	const struct btf_type *t = st_ops->type;
+	const struct btf_type *t;
 	struct bpf_tramp_links *tlinks;
 	void *udata, *kdata;
 	int prog_fd, err;

@@ -499,15 +505,20 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
 	if (flags)
 		return -EINVAL;
 
+	if (!st_ops)
+		return -EINVAL;
+
+	t = st_ops->type;
+
 	if (*(u32 *)key != 0)
 		return -E2BIG;
 
-	err = check_zero_holes(st_ops->value_type, value);
+	err = check_zero_holes(st_ops->btf, st_ops->value_type, value);
 	if (err)
 		return err;
 
 	uvalue = value;
-	err = check_zero_holes(t, uvalue->data);
+	err = check_zero_holes(st_ops->btf, t, uvalue->data);
 	if (err)
 		return err;

@@ -773,8 +784,9 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
 	struct bpf_struct_ops_map *st_map;
 	const struct btf_type *t, *vt;
 	struct bpf_map *map;
+	struct btf *btf;
 
-	st_ops = bpf_struct_ops_find_value(attr->btf_vmlinux_value_type_id);
+	st_ops = bpf_struct_ops_find_value(attr->btf_vmlinux_value_type_id, btf_vmlinux);
 	if (!st_ops)
 		return ERR_PTR(-ENOTSUPP);

kernel/bpf/verifier.c +1 -1

index bb78212fa5b2..c0cab601b2a6 100644
@@ -19191,7 +19191,7 @@ static int check_struct_ops_btf_id(struct bpf_verifier_env *env)
 	}
 
 	btf_id = prog->aux->attach_btf_id;
-	st_ops = bpf_struct_ops_find(btf_id);
+	st_ops = bpf_struct_ops_find(btf_id, btf_vmlinux);
 	if (!st_ops) {
 		verbose(env, "attach_btf_id %u is not a supported struct\n",
 			btf_id);

Signature · 1 line

2.34.1

[email protected]

author

2023-09-13 06:15 UTC #10 lore raw

RFC v2 4/9 bpf: use attached BTF to find correct type info of struct_ops progs.

In reply to [email protected]

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

The signatures may be declared in the module defining the structs type. So, we need to know which module BTF to look for type information. The later patches will make libbpf to attach module BTFs to programs. This patch tries to use the attached BTF if there is.

---
 include/uapi/linux/bpf.h       |  4 ++++
 kernel/bpf/bpf_struct_ops.c    | 11 ++++++++++-
 kernel/bpf/syscall.c           |  2 +-
 kernel/bpf/verifier.c          |  4 +++-
 tools/include/uapi/linux/bpf.h |  4 ++++
 5 files changed, 22 insertions(+), 3 deletions(-)

include/uapi/linux/bpf.h +4 -0

index 8790b3962e4b..f0882d341433 100644
@@ -1383,6 +1383,10 @@ union bpf_attr {
 		 * to using 5 hash functions).
 		 */
 		__u64	map_extra;
+
+		__u32   mod_btf_fd;	/* fd pointing to a BTF type data
+					 * for btf_vmlinux_value_type_id.
+					 */
 	};
 
 	struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */

kernel/bpf/bpf_struct_ops.c +10 -1

index 82cc3f0638fa..c93baf54a538 100644
@@ -786,7 +786,16 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
 	struct bpf_map *map;
 	struct btf *btf;
 
-	st_ops = bpf_struct_ops_find_value(attr->btf_vmlinux_value_type_id, btf_vmlinux);
+	/* XXX: We need a module name or ID to find a BTF type. */
+	/* XXX: should use btf from attr->btf_fd */
+	if (attr->mod_btf_fd) {
+		btf = btf_get_by_fd(attr->mod_btf_fd);
+		if (IS_ERR(btf))
+			return ERR_PTR(PTR_ERR(btf));
+	} else {
+		btf = btf_vmlinux;
+	}
+	st_ops = bpf_struct_ops_find_value(attr->btf_vmlinux_value_type_id, btf);
 	if (!st_ops)
 		return ERR_PTR(-ENOTSUPP);

kernel/bpf/syscall.c +1 -1

index eb01c31ed591..04d3017b7db1 100644
@@ -1093,7 +1093,7 @@ static int map_check_btf(struct bpf_map *map, const struct btf *btf,
 	return ret;
 }
 
-#define BPF_MAP_CREATE_LAST_FIELD map_extra
+#define BPF_MAP_CREATE_LAST_FIELD mod_btf_fd
 /* called via syscall */
 static int map_create(union bpf_attr *attr)
 {

kernel/bpf/verifier.c +3 -1

index c0cab601b2a6..8eab8d3fc398 100644
@@ -19183,6 +19183,7 @@ static int check_struct_ops_btf_id(struct bpf_verifier_env *env)
 	const struct btf_member *member;
 	struct bpf_prog *prog = env->prog;
 	u32 btf_id, member_idx;
+	struct btf *btf;
 	const char *mname;
 
 	if (!prog->gpl_compatible) {

@@ -19190,8 +19191,9 @@ static int check_struct_ops_btf_id(struct bpf_verifier_env *env)
 		return -EINVAL;
 	}
 
+	btf = prog->aux->attach_btf;
 	btf_id = prog->aux->attach_btf_id;
-	st_ops = bpf_struct_ops_find(btf_id, btf_vmlinux);
+	st_ops = bpf_struct_ops_find(btf_id, btf);
 	if (!st_ops) {
 		verbose(env, "attach_btf_id %u is not a supported struct\n",
 			btf_id);

tools/include/uapi/linux/bpf.h +4 -0

index 8790b3962e4b..f0882d341433 100644
@@ -1383,6 +1383,10 @@ union bpf_attr {
 		 * to using 5 hash functions).
 		 */
 		__u64	map_extra;
+
+		__u32   mod_btf_fd;	/* fd pointing to a BTF type data
+					 * for btf_vmlinux_value_type_id.
+					 */
 	};
 
 	struct { /* anonymous struct used by BPF_MAP_*_ELEM commands */

Signature · 1 line

2.34.1

[email protected]

author

2023-09-13 06:15 UTC #11 lore raw

RFC v2 5/9 bpf: hold module for bpf_struct_ops_map.

In reply to [email protected]

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

Ensure a module doesn't go away when a struct_ops map is still alive with a struct_ops type defined by the module.

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

---
 include/linux/bpf.h         |  1 +
 kernel/bpf/bpf_struct_ops.c | 34 ++++++++++++++++++++++++++++------
 2 files changed, 29 insertions(+), 6 deletions(-)

include/linux/bpf.h +1 -0

index a9369f982cd5..236a53330c85 100644
@@ -1615,6 +1615,7 @@ struct bpf_struct_ops {
 	int (*update)(void *kdata, void *old_kdata);
 	int (*validate)(void *kdata);
 	const struct btf *btf;
+	struct module *owner;
 	const struct btf_type *type;
 	const struct btf_type *value_type;
 	const char *name;

kernel/bpf/bpf_struct_ops.c +28 -6

index c93baf54a538..845873bc806d 100644
@@ -101,6 +101,7 @@ static struct bpf_struct_ops *bpf_struct_ops_static[] = {
 static struct bpf_struct_ops **bpf_struct_ops;
 static int bpf_struct_ops_num;
 static int bpf_struct_ops_capacity;
+static DEFINE_MUTEX(bpf_struct_ops_mutex);
 
 const struct bpf_verifier_ops bpf_struct_ops_verifier_ops = {
 };

@@ -307,7 +308,10 @@ int register_bpf_struct_ops(struct bpf_struct_ops_mod *mod)
 	}
 
 	bpf_struct_ops_init_one(st_ops, btf, log);
+	st_ops->owner = mod->owner;
+	mutex_lock(&bpf_struct_ops_mutex);
 	err = add_struct_ops(st_ops);
+	mutex_unlock(&bpf_struct_ops_mutex);
 
 errout:
 	kfree(log);

@@ -321,7 +325,9 @@ int unregister_bpf_struct_ops(struct bpf_struct_ops_mod *mod)
 	struct bpf_struct_ops *st_ops = mod->st_ops;
 	int err;
 
+	mutex_lock(&bpf_struct_ops_mutex);
 	err = remove_struct_ops(st_ops);
+	mutex_unlock(&bpf_struct_ops_mutex);
 	if (!err && st_ops->uninit)
 		err = st_ops->uninit();

@@ -334,34 +340,44 @@ extern struct btf *btf_vmlinux;
 static const struct bpf_struct_ops *
 bpf_struct_ops_find_value(u32 value_id, struct btf *btf)
 {
+	struct bpf_struct_ops *st_ops = NULL;
 	unsigned int i;
 
 	if (!value_id || !btf_vmlinux)
 		return NULL;
 
+	mutex_lock(&bpf_struct_ops_mutex);
 	for (i = 0; i < bpf_struct_ops_num; i++) {
 		if (bpf_struct_ops[i]->value_id == value_id &&
-		    bpf_struct_ops[i]->btf == btf)
-			return bpf_struct_ops[i];
+		    bpf_struct_ops[i]->btf == btf) {
+			st_ops = bpf_struct_ops[i];
+			break;
+		}
 	}
+	mutex_unlock(&bpf_struct_ops_mutex);
 
-	return NULL;
+	return st_ops;
 }
 
 const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id, struct btf *btf)
 {
+	struct bpf_struct_ops *st_ops = NULL;
 	unsigned int i;
 
 	if (!type_id || !btf_vmlinux)
 		return NULL;
 
+	mutex_lock(&bpf_struct_ops_mutex);
 	for (i = 0; i < bpf_struct_ops_num; i++) {
 		if (bpf_struct_ops[i]->type_id == type_id &&
-		    bpf_struct_ops[i]->btf == btf)
-			return bpf_struct_ops[i];
+		    bpf_struct_ops[i]->btf == btf) {
+			st_ops = bpf_struct_ops[i];
+			break;
+		}
 	}
+	mutex_unlock(&bpf_struct_ops_mutex);
 
-	return NULL;
+	return st_ops;
 }
 
 static int bpf_struct_ops_map_get_next_key(struct bpf_map *map, void *key,

@@ -749,6 +765,8 @@ static void __bpf_struct_ops_map_free(struct bpf_map *map)
 
 static void bpf_struct_ops_map_free(struct bpf_map *map)
 {
+	struct bpf_struct_ops_map *st_map = (struct bpf_struct_ops_map *)map;
+
 	/* The struct_ops's function may switch to another struct_ops.
 	 *
 	 * For example, bpf_tcp_cc_x->init() may switch to

@@ -766,6 +784,7 @@ static void bpf_struct_ops_map_free(struct bpf_map *map)
 	 */
 	synchronize_rcu_mult(call_rcu, call_rcu_tasks);
 
+	module_put(st_map->st_ops->owner);
 	__bpf_struct_ops_map_free(map);
 }

@@ -799,6 +818,9 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
 	if (!st_ops)
 		return ERR_PTR(-ENOTSUPP);
 
+	if (!try_module_get(st_ops->owner))
+		return ERR_PTR(-EINVAL);
+
 	vt = st_ops->value_type;
 	if (attr->value_size != vt->size)
 		return ERR_PTR(-EINVAL);

Signature · 1 line

2.34.1

[email protected]

author

2023-09-13 06:15 UTC #12 lore raw

RFC v2 6/9 libbpf: Find correct module BTFs for struct_ops maps and progs.

In reply to [email protected]

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

Find module BTFs for struct_ops maps and progs and pass them to the kernel. It ensures the kernel resolve type IDs from correct module BTFs.

---
 tools/lib/bpf/bpf.c    |   3 +-
 tools/lib/bpf/bpf.h    |   4 +-
 tools/lib/bpf/libbpf.c | 121 ++++++++++++++++++++++++-----------------
 3 files changed, 75 insertions(+), 53 deletions(-)

tools/lib/bpf/bpf.c +2 -1

index b0f1913763a3..df4b7570ad92 100644
@@ -169,7 +169,7 @@ int bpf_map_create(enum bpf_map_type map_type,
 		   __u32 max_entries,
 		   const struct bpf_map_create_opts *opts)
 {
-	const size_t attr_sz = offsetofend(union bpf_attr, map_extra);
+	const size_t attr_sz = offsetofend(union bpf_attr, mod_btf_fd);
 	union bpf_attr attr;
 	int fd;

@@ -191,6 +191,7 @@ int bpf_map_create(enum bpf_map_type map_type,
 	attr.btf_key_type_id = OPTS_GET(opts, btf_key_type_id, 0);
 	attr.btf_value_type_id = OPTS_GET(opts, btf_value_type_id, 0);
 	attr.btf_vmlinux_value_type_id = OPTS_GET(opts, btf_vmlinux_value_type_id, 0);
+	attr.mod_btf_fd = OPTS_GET(opts, mod_btf_fd, 0);
 
 	attr.inner_map_fd = OPTS_GET(opts, inner_map_fd, 0);
 	attr.map_flags = OPTS_GET(opts, map_flags, 0);

tools/lib/bpf/bpf.h +3 -1

index 74c2887cfd24..d18f75b0ccc9 100644
@@ -51,8 +51,10 @@ struct bpf_map_create_opts {
 
 	__u32 numa_node;
 	__u32 map_ifindex;
+
+	__u32 mod_btf_fd;
 };
-#define bpf_map_create_opts__last_field map_ifindex
+#define bpf_map_create_opts__last_field mod_btf_fd
 
 LIBBPF_API int bpf_map_create(enum bpf_map_type map_type,
 			      const char *map_name,

tools/lib/bpf/libbpf.c +70 ~~-51~~

229 lines not on this page · raw message

Signature · 1 line

2.34.1

[email protected]

author

2023-09-13 06:15 UTC #13 lore raw

RFC v2 7/9 bpf: export btf_ctx_access to modules.

In reply to [email protected]

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

btf_ctx_access() is needed by module to call bpf_tracing_btf_ctx_access().

---
 kernel/bpf/btf.c | 1 +
 1 file changed, 1 insertion(+)

kernel/bpf/btf.c +1 -0

index 55d76d85c6ec..ae2cd120e426 100644
@@ -6115,6 +6115,7 @@ bool btf_ctx_access(int off, int size, enum bpf_access_type type,
 		__btf_name_by_offset(btf, t->name_off));
 	return true;
 }
+EXPORT_SYMBOL_GPL(btf_ctx_access);
 
 enum bpf_struct_walk_result {
 	/* < 0 error */

Signature · 1 line

2.34.1

[email protected]

author

2023-09-13 06:15 UTC #14 lore raw

RFC v2 8/9 selftests/bpf: test case for register_bpf_struct_ops().

In reply to [email protected]

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

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

---
 .../selftests/bpf/bpf_testmod/bpf_testmod.c   | 84 +++++++++++++++++++
 .../selftests/bpf/bpf_testmod/bpf_testmod.h   |  5 ++
 .../bpf/prog_tests/test_struct_ops_module.c   | 40 +++++++++
 .../selftests/bpf/progs/struct_ops_module.c   | 30 +++++++
 4 files changed, 159 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/bpf_testmod/bpf_testmod.c +84 -0

index cefc5dd72573..136638c15047 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,88 @@ 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
+
+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,
+};
+
+#define BPF_STRUCT_OPS_COMMON_VALUE			\
+	refcount_t refcnt;				\
+	enum bpf_struct_ops_state state
+
+#define BPF_STRUCT_OPS_TYPE(_name)				\
+struct bpf_struct_ops_##_name {						\
+	BPF_STRUCT_OPS_COMMON_VALUE;				\
+	struct _name data ____cacheline_aligned_in_smp;		\
+}
+
+BPF_STRUCT_OPS_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;
+
+	((struct bpf_struct_ops_bpf_testmod_ops *)0);
+	r = ops->test_2(4, 3);
+	printk(KERN_CRIT "bpf_dummy_reg: test_2 %d\n", r);
+	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",
+};
+
+static struct bpf_struct_ops_mod bpf_testmod_struct_ops = {
+	.owner = THIS_MODULE,
+	.st_ops = &bpf_bpf_testmod_ops,
+};
+
+#endif /* CONFIG_DEBUG_INFO_BTF_MODULES */
+
 extern int bpf_fentry_test1(int a);
 
 static int bpf_testmod_init(void)

@@ -532,6 +610,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_testmod_struct_ops);
+#endif
 	if (ret < 0)
 		return ret;
 	if (bpf_fentry_test1(0) < 0)

@@ -541,6 +622,9 @@ static int bpf_testmod_init(void)
 
 static void bpf_testmod_exit(void)
 {
+#ifdef CONFIG_DEBUG_INFO_BTF_MODULES
+	unregister_bpf_struct_ops(&bpf_testmod_struct_ops);
+#endif
 	return sysfs_remove_bin_file(kernel_kobj, &bin_attr_bpf_testmod_file);
 }

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 +40 -0

new file mode 100644
index 000000000000..29dd203121f5
@@ -0,0 +1,40 @@
+// SPDX-License-Identifier: GPL-2.0
+/* Copyright (c) 2023 Meta Platforms, Inc. and affiliates. */
+#include <test_progs.h>
+
+#include "struct_ops_module.skel.h"
+#include "testing_helpers.h"
+
+static void test_regular_load()
+{
+	struct struct_ops_module *skel;
+	struct bpf_link *link;
+	DECLARE_LIBBPF_OPTS(bpf_object_open_opts, opts);
+	int err;
+
+	opts.btf_custom_path = "/sys/kernel/btf/bpf_testmod",
+
+	printf("test_regular_load\n");
+	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,
+};
+

Signature · 1 line

2.34.1

[email protected]

author

2023-09-13 06:15 UTC #15 lore raw

RFC v2 9/9 Comments and debug

In reply to [email protected]

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

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

---
 kernel/bpf/bpf_struct_ops.c | 15 +++++++++++++++
 kernel/bpf/syscall.c        |  6 ++++++
 tools/lib/bpf/libbpf.c      | 26 ++++++++++++++++++++++++++
 3 files changed, 47 insertions(+)

kernel/bpf/bpf_struct_ops.c +15 -0

index 845873bc806d..47045b026bec 100644
@@ -133,6 +133,9 @@ static void bpf_struct_ops_init_one(struct bpf_struct_ops *st_ops,
 	}
 	sprintf(value_name, "%s%s", VALUE_PREFIX, st_ops->name);
 
+	/* XXX: This ID is not unique across modules. We need to include
+	 * module or module ID as an unique ID.
+	 */
 	value_id = btf_find_by_name_kind(btf, value_name,
 					 BTF_KIND_STRUCT);
 	if (value_id < 0) {

@@ -141,6 +144,9 @@ static void bpf_struct_ops_init_one(struct bpf_struct_ops *st_ops,
 		return;
 	}
 
+	/* XXX: This ID is not unique across modules. We need to include
+	 * module or module ID as an unique ID.
+	 */
 	type_id = btf_find_by_name_kind(btf, st_ops->name,
 					BTF_KIND_STRUCT);
 	if (type_id < 0) {

@@ -569,6 +575,9 @@ static long bpf_struct_ops_map_update_elem(struct bpf_map *map, void *key,
 		u32 moff;
 
 		moff = __btf_member_bit_offset(t, member) / 8;
+		/* XXX: Should resolve member types from module BTF, but
+		 * it's not available yet.
+		 */
 		ptype = btf_type_resolve_ptr(btf_vmlinux, member->type, NULL);
 		if (ptype == module_type) {
 			if (*(void **)(udata + moff))

@@ -837,6 +846,12 @@ static struct bpf_map *bpf_struct_ops_map_alloc(union bpf_attr *attr)
 	if (!st_map)
 		return ERR_PTR(-ENOMEM);
 
+	/* XXX: should sync with the unregister path */
+	/* XXX: Since we assign a st_ops, we need to do a rcu_synchronize()
+	 *      twice to make sure the st_ops is not freed while other
+	 *      tasks use this value. Or, we can find st_ops again holding
+	 *      the mutex to make sure it is not freed.
+	 */
 	st_map->st_ops = st_ops;
 	map = &st_map->map;

kernel/bpf/syscall.c +6 -0

index 04d3017b7db1..75c4f0b251a3 100644
@@ -1204,6 +1204,10 @@ static int map_create(union bpf_attr *attr)
 		return -EPERM;
 	}
 
+	/* XXX: attr->attach_btf_obj_fd should be initialized by the user
+	 *      space. We should use it to find type infor from
+	 *      attach_btf_id.
+	 */
 	map = ops->map_alloc(attr);
 	if (IS_ERR(map))
 		return PTR_ERR(map);

@@ -2624,6 +2628,7 @@ static int bpf_prog_load(union bpf_attr *attr, bpfptr_t uattr, u32 uattr_size)
 		btf_get(attach_btf);
 	}
 
+
 	bpf_prog_load_fixup_attach_type(attr);
 	if (bpf_prog_load_check_attach(type, attr->expected_attach_type,
 				       attach_btf, attr->attach_btf_id,

@@ -4576,6 +4581,7 @@ static int bpf_map_get_info_by_fd(struct file *file,
 		info.btf_value_type_id = map->btf_value_type_id;
 	}
 	info.btf_vmlinux_value_type_id = map->btf_vmlinux_value_type_id;
+	/* XXX: copy map->mod_btf->name as well? */
 
 	if (bpf_map_is_offloaded(map)) {
 		err = bpf_map_offload_info_fill(&info, map);

tools/lib/bpf/libbpf.c +26 -0

index 211889d37320..cd866a30471b 100644
@@ -988,6 +988,7 @@ find_struct_ops_kern_types(struct bpf_object *obj, const char *tname,
 	 * find "struct bpf_struct_ops_tcp_congestion_ops" from the
 	 * btf_vmlinux.
 	 */
+	/* XXX: Should search module BTFs as well. */
 	kern_vtype_id = find_btf_by_prefix_kind(btf, STRUCT_OPS_VALUE_PREFIX,
 						tname, BTF_KIND_STRUCT);
 	if (kern_vtype_id < 0) {

@@ -5143,6 +5144,8 @@ bpf_object__populate_internal_map(struct bpf_object *obj, struct bpf_map *map)
 	return 0;
 }
 
+int turnon_kk = false;
+
 static void bpf_map__destroy(struct bpf_map *map);
 
 static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, bool is_inner)

@@ -7945,13 +7948,32 @@ static int bpf_object_load(struct bpf_object *obj, int extra_log_level, const ch
 		bpf_gen__init(obj->gen_loader, extra_log_level, obj->nr_programs, obj->nr_maps);
 
 	err = bpf_object__probe_loading(obj);
+	if (turnon_kk)
+		printf("bpf_object__probe_loading err=%d\n", err);
+	/* XXX: should correct module btf if needed.
+	 *      obj->btf_vmlinux provides the information of members of
+	 *      the struct_ops type required to load the object.
+	 *      (see bpf_object__init_kern_struct_ops_maps() and
+	 *      bpf_map__init_kern_struct_ops())
+	 */
 	err = err ? : bpf_object__load_vmlinux_btf(obj, false);
+	if (turnon_kk)
+		printf("bpf_object__probe_loading err=%d\n", err);
 	err = err ? : bpf_object__resolve_externs(obj, obj->kconfig);
 	err = err ? : bpf_object__sanitize_and_load_btf(obj);
 	err = err ? : bpf_object__sanitize_maps(obj);
+	if (turnon_kk)
+		printf("bpf_object__probe_loading err=%d\n", err);
+	/* XXX: obj->btf_vmliux is not used for loading the object. */
 	err = err ? : bpf_object__init_kern_struct_ops_maps(obj);
+	if (turnon_kk)
+		printf("bpf_object__probe_loading err=%d\n", err);
 	err = err ? : bpf_object__create_maps(obj);
+	if (turnon_kk)
+		printf("bpf_object__probe_loading err=%d\n", err);
 	err = err ? : bpf_object__relocate(obj, obj->btf_custom_path ? : target_btf_path);
+	if (turnon_kk)
+		printf("bpf_object__probe_loading err=%d\n", err);
 	err = err ? : bpf_object__load_progs(obj, extra_log_level);
 	err = err ? : bpf_object_init_prog_arrays(obj);
 	err = err ? : bpf_object_prepare_struct_ops(obj);

@@ -9230,6 +9252,7 @@ static int bpf_object__collect_st_ops_relos(struct bpf_object *obj,
 		 * attach_btf_id and member_idx
 		 */
 		if (!prog->attach_btf_id) {
+			/* XXX: attach_btf_obj_fd is needed as well */
 			prog->attach_btf_id = st_ops->type_id;
 			prog->expected_attach_type = member_idx;
 		}

@@ -13124,7 +13147,9 @@ int bpf_object__load_skeleton(struct bpf_object_skeleton *s)
 {
 	int i, err;
 
+	printf("Loading BPF skeleton '%s'...\n", s->name);
 	err = bpf_object__load(*s->obj);
+	printf("bpf_object__load\n");
 	if (err) {
 		pr_warn("failed to load BPF skeleton '%s': %d\n", s->name, err);
 		return libbpf_err(err);

@@ -13169,6 +13194,7 @@ int bpf_object__load_skeleton(struct bpf_object_skeleton *s)
 		}
 	}
 
+	printf("BPF skeleton '%s' loaded successfully\n", s->name);
 	return 0;
 }

Signature · 1 line

2.34.1