* [RFC bpf-next v2 1/9] bpf: refactory struct_ops type initialization to a function.
2023-09-13 6:14 [RFC bpf-next v2 0/9] Registrating struct_ops types from modules thinker.li
@ 2023-09-13 6:14 ` thinker.li
2023-09-15 22:43 ` Martin KaFai Lau
2023-09-13 6:14 ` [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops thinker.li
` (7 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: thinker.li @ 2023-09-13 6:14 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
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(-)
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index fdc3e8705a3c..1662875e0ebe 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -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);
}
}
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFC bpf-next v2 1/9] bpf: refactory struct_ops type initialization to a function.
2023-09-13 6:14 ` [RFC bpf-next v2 1/9] bpf: refactory struct_ops type initialization to a function thinker.li
@ 2023-09-15 22:43 ` Martin KaFai Lau
2023-09-16 1:35 ` Kui-Feng Lee
0 siblings, 1 reply; 16+ messages in thread
From: Martin KaFai Lau @ 2023-09-15 22:43 UTC (permalink / raw)
To: thinker.li; +Cc: sinquersw, kuifeng, bpf, ast, song, kernel-team, andrii
On 9/12/23 11:14 PM, thinker.li@gmail.com wrote:
> From: Kui-Feng Lee <thinker.li@gmail.com>
>
> 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.
> ---
> kernel/bpf/bpf_struct_ops.c | 157 +++++++++++++++++++-----------------
> 1 file changed, 83 insertions(+), 74 deletions(-)
>
> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
> index fdc3e8705a3c..1662875e0ebe 100644
> --- a/kernel/bpf/bpf_struct_ops.c
> +++ b/kernel/bpf/bpf_struct_ops.c
> @@ -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);
It needs to do some sanity checks on the value_type since this won't be
statically enforced by bpf_struct_ops.c.
[ ... ]
> +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
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.
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC bpf-next v2 1/9] bpf: refactory struct_ops type initialization to a function.
2023-09-15 22:43 ` Martin KaFai Lau
@ 2023-09-16 1:35 ` Kui-Feng Lee
0 siblings, 0 replies; 16+ messages in thread
From: Kui-Feng Lee @ 2023-09-16 1:35 UTC (permalink / raw)
To: Martin KaFai Lau, thinker.li; +Cc: kuifeng, bpf, ast, song, kernel-team, andrii
On 9/15/23 15:43, Martin KaFai Lau wrote:
> On 9/12/23 11:14 PM, thinker.li@gmail.com wrote:
>> From: Kui-Feng Lee <thinker.li@gmail.com>
>>
>> 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.
>
>> ---
>> kernel/bpf/bpf_struct_ops.c | 157 +++++++++++++++++++-----------------
>> 1 file changed, 83 insertions(+), 74 deletions(-)
>>
>> diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
>> index fdc3e8705a3c..1662875e0ebe 100644
>> --- a/kernel/bpf/bpf_struct_ops.c
>> +++ b/kernel/bpf/bpf_struct_ops.c
>> @@ -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);
>
> 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?
>
> [ ... ]
>
>> +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
>
> 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.
>
> 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.
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops.
2023-09-13 6:14 [RFC bpf-next v2 0/9] Registrating struct_ops types from modules thinker.li
2023-09-13 6:14 ` [RFC bpf-next v2 1/9] bpf: refactory struct_ops type initialization to a function thinker.li
@ 2023-09-13 6:14 ` thinker.li
2023-09-16 0:05 ` Martin KaFai Lau
2023-09-13 6:14 ` [RFC bpf-next v2 3/9] bpf: attach a module BTF to a bpf_struct_ops thinker.li
` (6 subsequent siblings)
8 siblings, 1 reply; 16+ messages in thread
From: thinker.li @ 2023-09-13 6:14 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
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 <thinker.li@gmail.com>
---
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(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 024e8b28c34b..10f98f0ddc77 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -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 */
diff --git a/include/linux/btf.h b/include/linux/btf.h
index 928113a80a95..d6ed3d99ba41 100644
--- a/include/linux/btf.h
+++ b/include/linux/btf.h
@@ -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,
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 1662875e0ebe..9be6e07ccba5 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -92,12 +92,15 @@ enum {
__NR_BPF_STRUCT_OPS_TYPE,
};
-static struct bpf_struct_ops * const bpf_struct_ops[] = {
+static struct bpf_struct_ops *bpf_struct_ops_static[] = {
#define BPF_STRUCT_OPS_TYPE(_name) \
[BPF_STRUCT_OPS_TYPE_##_name] = &bpf_##_name,
#include "bpf_struct_ops_types.h"
#undef BPF_STRUCT_OPS_TYPE
};
+static struct bpf_struct_ops **bpf_struct_ops;
+static int bpf_struct_ops_num;
+static int bpf_struct_ops_capacity;
const struct bpf_verifier_ops bpf_struct_ops_verifier_ops = {
};
@@ -212,12 +215,116 @@ void bpf_struct_ops_init(struct btf *btf, struct bpf_verifier_log *log)
}
module_type = btf_type_by_id(btf, module_id);
- for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
+ bpf_struct_ops_num = ARRAY_SIZE(bpf_struct_ops_static);
+ bpf_struct_ops_capacity = bpf_struct_ops_num;
+ bpf_struct_ops = bpf_struct_ops_static;
+
+ for (i = 0; i < bpf_struct_ops_num; i++) {
st_ops = bpf_struct_ops[i];
bpf_struct_ops_init_one(st_ops, btf, log);
}
}
+static int add_struct_ops(struct bpf_struct_ops *st_ops)
+{
+ struct bpf_struct_ops **new_ops;
+ int i;
+
+ for (i = 0; i < bpf_struct_ops_num; i++) {
+ if (bpf_struct_ops[i] == st_ops)
+ return -EEXIST;
+ if (strcmp(bpf_struct_ops[i]->name, st_ops->name) == 0)
+ return -EEXIST;
+ }
+
+ if (bpf_struct_ops_num == bpf_struct_ops_capacity) {
+ if (bpf_struct_ops == bpf_struct_ops_static) {
+ new_ops = kmalloc_array(((bpf_struct_ops_capacity + 0x7) & ~0x7) * 2,
+ sizeof(*new_ops),
+ GFP_KERNEL);
+ if (!new_ops)
+ return -ENOMEM;
+ memcpy(new_ops, bpf_struct_ops,
+ sizeof(*new_ops) * bpf_struct_ops_num);
+ } else {
+ new_ops = krealloc_array(bpf_struct_ops,
+ bpf_struct_ops_capacity * 2,
+ sizeof(*new_ops),
+ GFP_KERNEL);
+ if (!new_ops)
+ return -ENOMEM;
+ }
+ bpf_struct_ops = new_ops;
+ bpf_struct_ops_capacity *= 2;
+ }
+
+ bpf_struct_ops[bpf_struct_ops_num++] = st_ops;
+ return 0;
+}
+
+static int remove_struct_ops(struct bpf_struct_ops *st_ops)
+{
+ int i;
+
+ for (i = 0; i < bpf_struct_ops_num; i++) {
+ if (bpf_struct_ops[i] == st_ops) {
+ bpf_struct_ops_num--;
+ bpf_struct_ops[i] = bpf_struct_ops[bpf_struct_ops_num];
+ return 0;
+ }
+ }
+
+ return -ENOENT;
+}
+
+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;
+
+ 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);
+ if (!btf) {
+ err = -EINVAL;
+ goto errout;
+ }
+
+ bpf_struct_ops_init_one(st_ops, btf, log);
+ 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)
+{
+ 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);
+
extern struct btf *btf_vmlinux;
static const struct bpf_struct_ops *
@@ -228,7 +335,7 @@ bpf_struct_ops_find_value(u32 value_id)
if (!value_id || !btf_vmlinux)
return NULL;
- for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
+ for (i = 0; i < bpf_struct_ops_num; i++) {
if (bpf_struct_ops[i]->value_id == value_id)
return bpf_struct_ops[i];
}
@@ -243,7 +350,7 @@ const struct bpf_struct_ops *bpf_struct_ops_find(u32 type_id)
if (!type_id || !btf_vmlinux)
return NULL;
- for (i = 0; i < ARRAY_SIZE(bpf_struct_ops); i++) {
+ for (i = 0; i < bpf_struct_ops_num; i++) {
if (bpf_struct_ops[i]->type_id == type_id)
return bpf_struct_ops[i];
}
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 1095bbe29859..55d76d85c6ec 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -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;
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* Re: [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops.
2023-09-13 6:14 ` [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops thinker.li
@ 2023-09-16 0:05 ` Martin KaFai Lau
2023-09-16 1:14 ` Kui-Feng Lee
0 siblings, 1 reply; 16+ messages in thread
From: Martin KaFai Lau @ 2023-09-16 0:05 UTC (permalink / raw)
To: thinker.li; +Cc: sinquersw, kuifeng, bpf, ast, song, kernel-team, andrii
On 9/12/23 11:14 PM, thinker.li@gmail.com wrote:
> +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;
> +
> + 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.
> + if (!btf) {
> + err = -EINVAL;
> + goto errout;
> + }
> +
> + bpf_struct_ops_init_one(st_ops, btf, log);
> + 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?
> +{
> + 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);
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops.
2023-09-16 0:05 ` Martin KaFai Lau
@ 2023-09-16 1:14 ` Kui-Feng Lee
2023-09-18 18:47 ` Martin KaFai Lau
0 siblings, 1 reply; 16+ messages in thread
From: Kui-Feng Lee @ 2023-09-16 1:14 UTC (permalink / raw)
To: Martin KaFai Lau, thinker.li; +Cc: kuifeng, bpf, ast, song, kernel-team, andrii
On 9/15/23 17:05, Martin KaFai Lau wrote:
> On 9/12/23 11:14 PM, thinker.li@gmail.com wrote:
>> +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;
>> +
>> + 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.
It is my fault to use it without calling btf_put().
I miss-understood the API, thought it doesn't increase refcount by
mistake.
>
>> + if (!btf) {
>> + err = -EINVAL;
>> + goto errout;
>> + }
>> +
>> + bpf_struct_ops_init_one(st_ops, btf, log);
>> + 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)?
It could be. However, registering to module notifier
(register_module_notifier()) is more straight forward if we go
this way. What do you think?
>
> 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.
>
>> +{
>> + 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);
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops.
2023-09-16 1:14 ` Kui-Feng Lee
@ 2023-09-18 18:47 ` Martin KaFai Lau
2023-09-18 20:40 ` Kui-Feng Lee
0 siblings, 1 reply; 16+ messages in thread
From: Martin KaFai Lau @ 2023-09-18 18:47 UTC (permalink / raw)
To: Kui-Feng Lee; +Cc: kuifeng, bpf, ast, song, kernel-team, andrii, thinker.li
On 9/15/23 6:14 PM, Kui-Feng Lee wrote:
>
>
> On 9/15/23 17:05, Martin KaFai Lau wrote:
>> On 9/12/23 11:14 PM, thinker.li@gmail.com wrote:
>>> +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;
>>> +
>>> + 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.
>
> It is my fault to use it without calling btf_put().
> I miss-understood the API, thought it doesn't increase refcount by
> mistake.
>
>>
>>> + if (!btf) {
>>> + err = -EINVAL;
>>> + goto errout;
>>> + }
>>> +
>>> + bpf_struct_ops_init_one(st_ops, btf, log);
>>> + 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)?
>
> 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.
>
>>
>> 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?
>
>>
>>> +{
>>> + 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);
>>
>>
^ permalink raw reply [flat|nested] 16+ messages in thread* Re: [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops.
2023-09-18 18:47 ` Martin KaFai Lau
@ 2023-09-18 20:40 ` Kui-Feng Lee
0 siblings, 0 replies; 16+ messages in thread
From: Kui-Feng Lee @ 2023-09-18 20:40 UTC (permalink / raw)
To: Martin KaFai Lau; +Cc: kuifeng, bpf, ast, song, kernel-team, andrii, thinker.li
On 9/18/23 11:47, Martin KaFai Lau wrote:
> On 9/15/23 6:14 PM, Kui-Feng Lee wrote:
>>
>>
>> On 9/15/23 17:05, Martin KaFai Lau wrote:
>>> On 9/12/23 11:14 PM, thinker.li@gmail.com wrote:
>>>> +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;
>>>> +
>>>> + 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.
>>
>> It is my fault to use it without calling btf_put().
>> I miss-understood the API, thought it doesn't increase refcount by
>> mistake.
>>
>>>
>>>> + if (!btf) {
>>>> + err = -EINVAL;
>>>> + goto errout;
>>>> + }
>>>> +
>>>> + bpf_struct_ops_init_one(st_ops, btf, log);
>>>> + 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)?
>>
>> 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.
>
>>
>>>
>>> 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?
I don't think you missing anything.
>
>>
>>>
>>>> +{
>>>> + 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);
>>>
>>>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [RFC bpf-next v2 3/9] bpf: attach a module BTF to a bpf_struct_ops
2023-09-13 6:14 [RFC bpf-next v2 0/9] Registrating struct_ops types from modules thinker.li
2023-09-13 6:14 ` [RFC bpf-next v2 1/9] bpf: refactory struct_ops type initialization to a function thinker.li
2023-09-13 6:14 ` [RFC bpf-next v2 2/9] bpf: add register and unregister functions for struct_ops thinker.li
@ 2023-09-13 6:14 ` thinker.li
2023-09-13 6:14 ` [RFC bpf-next v2 4/9] bpf: use attached BTF to find correct type info of struct_ops progs thinker.li
` (5 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: thinker.li @ 2023-09-13 6:14 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
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(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index 10f98f0ddc77..a9369f982cd5 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -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;
}
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 9be6e07ccba5..82cc3f0638fa 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -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);
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index bb78212fa5b2..c0cab601b2a6 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -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);
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RFC bpf-next v2 4/9] bpf: use attached BTF to find correct type info of struct_ops progs.
2023-09-13 6:14 [RFC bpf-next v2 0/9] Registrating struct_ops types from modules thinker.li
` (2 preceding siblings ...)
2023-09-13 6:14 ` [RFC bpf-next v2 3/9] bpf: attach a module BTF to a bpf_struct_ops thinker.li
@ 2023-09-13 6:14 ` thinker.li
2023-09-13 6:14 ` [RFC bpf-next v2 5/9] bpf: hold module for bpf_struct_ops_map thinker.li
` (4 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: thinker.li @ 2023-09-13 6:14 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
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(-)
diff --git a/include/uapi/linux/bpf.h b/include/uapi/linux/bpf.h
index 8790b3962e4b..f0882d341433 100644
--- a/include/uapi/linux/bpf.h
+++ b/include/uapi/linux/bpf.h
@@ -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 */
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 82cc3f0638fa..c93baf54a538 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -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);
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index eb01c31ed591..04d3017b7db1 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -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)
{
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index c0cab601b2a6..8eab8d3fc398 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -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);
diff --git a/tools/include/uapi/linux/bpf.h b/tools/include/uapi/linux/bpf.h
index 8790b3962e4b..f0882d341433 100644
--- a/tools/include/uapi/linux/bpf.h
+++ b/tools/include/uapi/linux/bpf.h
@@ -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 */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RFC bpf-next v2 5/9] bpf: hold module for bpf_struct_ops_map.
2023-09-13 6:14 [RFC bpf-next v2 0/9] Registrating struct_ops types from modules thinker.li
` (3 preceding siblings ...)
2023-09-13 6:14 ` [RFC bpf-next v2 4/9] bpf: use attached BTF to find correct type info of struct_ops progs thinker.li
@ 2023-09-13 6:14 ` thinker.li
2023-09-13 6:14 ` [RFC bpf-next v2 6/9] libbpf: Find correct module BTFs for struct_ops maps and progs thinker.li
` (3 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: thinker.li @ 2023-09-13 6:14 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
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 <thinker.li@gmail.com>
---
include/linux/bpf.h | 1 +
kernel/bpf/bpf_struct_ops.c | 34 ++++++++++++++++++++++++++++------
2 files changed, 29 insertions(+), 6 deletions(-)
diff --git a/include/linux/bpf.h b/include/linux/bpf.h
index a9369f982cd5..236a53330c85 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -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;
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index c93baf54a538..845873bc806d 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -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);
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RFC bpf-next v2 6/9] libbpf: Find correct module BTFs for struct_ops maps and progs.
2023-09-13 6:14 [RFC bpf-next v2 0/9] Registrating struct_ops types from modules thinker.li
` (4 preceding siblings ...)
2023-09-13 6:14 ` [RFC bpf-next v2 5/9] bpf: hold module for bpf_struct_ops_map thinker.li
@ 2023-09-13 6:14 ` thinker.li
2023-09-13 6:14 ` [RFC bpf-next v2 7/9] bpf: export btf_ctx_access to modules thinker.li
` (2 subsequent siblings)
8 siblings, 0 replies; 16+ messages in thread
From: thinker.li @ 2023-09-13 6:14 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
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(-)
diff --git a/tools/lib/bpf/bpf.c b/tools/lib/bpf/bpf.c
index b0f1913763a3..df4b7570ad92 100644
--- a/tools/lib/bpf/bpf.c
+++ b/tools/lib/bpf/bpf.c
@@ -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);
diff --git a/tools/lib/bpf/bpf.h b/tools/lib/bpf/bpf.h
index 74c2887cfd24..d18f75b0ccc9 100644
--- a/tools/lib/bpf/bpf.h
+++ b/tools/lib/bpf/bpf.h
@@ -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,
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 96ff1aa4bf6a..211889d37320 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -517,6 +517,7 @@ struct bpf_map {
struct bpf_map_def def;
__u32 numa_node;
__u32 btf_var_idx;
+ struct module_btf *mod_btf;
__u32 btf_key_type_id;
__u32 btf_value_type_id;
__u32 btf_vmlinux_value_type_id;
@@ -888,6 +889,42 @@ bpf_object__add_programs(struct bpf_object *obj, Elf_Data *sec_data,
return 0;
}
+static int load_module_btfs(struct bpf_object *obj);
+
+static int find_kern_btf_id(struct bpf_object *obj, const char *kern_name,
+ __u16 kind, struct btf **res_btf,
+ struct module_btf **res_mod_btf)
+{
+ struct module_btf *mod_btf;
+ struct btf *btf;
+ int i, id, err;
+
+ btf = obj->btf_vmlinux;
+ mod_btf = NULL;
+ id = btf__find_by_name_kind(btf, kern_name, kind);
+
+ if (id == -ENOENT) {
+ err = load_module_btfs(obj);
+ if (err)
+ return err;
+
+ for (i = 0; i < obj->btf_module_cnt; i++) {
+ /* we assume module_btf's BTF FD is always >0 */
+ mod_btf = &obj->btf_modules[i];
+ btf = mod_btf->btf;
+ id = btf__find_by_name_kind_own(btf, kern_name, kind);
+ if (id != -ENOENT)
+ break;
+ }
+ }
+ if (id <= 0)
+ return -ESRCH;
+
+ *res_btf = btf;
+ *res_mod_btf = mod_btf;
+ return id;
+}
+
static const struct btf_member *
find_member_by_offset(const struct btf_type *t, __u32 bit_offset)
{
@@ -922,17 +959,23 @@ static int find_btf_by_prefix_kind(const struct btf *btf, const char *prefix,
const char *name, __u32 kind);
static int
-find_struct_ops_kern_types(const struct btf *btf, const char *tname,
+find_struct_ops_kern_types(struct bpf_object *obj, const char *tname,
+ struct module_btf **mod_btf,
const struct btf_type **type, __u32 *type_id,
const struct btf_type **vtype, __u32 *vtype_id,
const struct btf_member **data_member)
{
const struct btf_type *kern_type, *kern_vtype;
const struct btf_member *kern_data_member;
+ struct btf *btf;
__s32 kern_vtype_id, kern_type_id;
__u32 i;
- kern_type_id = btf__find_by_name_kind(btf, tname, BTF_KIND_STRUCT);
+ /* XXX: should search module BTFs as well. We need module name here
+ * to locate a correct BTF type.
+ */
+ kern_type_id = find_kern_btf_id(obj, tname, BTF_KIND_STRUCT,
+ &btf, mod_btf);
if (kern_type_id < 0) {
pr_warn("struct_ops init_kern: struct %s is not found in kernel BTF\n",
tname);
@@ -987,13 +1030,15 @@ static bool bpf_map__is_struct_ops(const struct bpf_map *map)
/* Init the map's fields that depend on kern_btf */
static int bpf_map__init_kern_struct_ops(struct bpf_map *map,
- const struct btf *btf,
- const struct btf *kern_btf)
+ struct bpf_object *obj)
{
const struct btf_member *member, *kern_member, *kern_data_member;
const struct btf_type *type, *kern_type, *kern_vtype;
__u32 i, kern_type_id, kern_vtype_id, kern_data_off;
struct bpf_struct_ops *st_ops;
+ const struct btf *kern_btf;
+ struct module_btf *mod_btf;
+ const struct btf *btf = obj->btf;
void *data, *kern_data;
const char *tname;
int err;
@@ -1001,16 +1046,19 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map,
st_ops = map->st_ops;
type = st_ops->type;
tname = st_ops->tname;
- err = find_struct_ops_kern_types(kern_btf, tname,
+ err = find_struct_ops_kern_types(obj, tname, &mod_btf,
&kern_type, &kern_type_id,
&kern_vtype, &kern_vtype_id,
&kern_data_member);
if (err)
return err;
+ kern_btf = mod_btf ? mod_btf->btf : obj->btf_vmlinux;
+
pr_debug("struct_ops init_kern %s: type_id:%u kern_type_id:%u kern_vtype_id:%u\n",
map->name, st_ops->type_id, kern_type_id, kern_vtype_id);
+ map->mod_btf = mod_btf;
map->def.value_size = kern_vtype->size;
map->btf_vmlinux_value_type_id = kern_vtype_id;
@@ -1086,6 +1134,9 @@ static int bpf_map__init_kern_struct_ops(struct bpf_map *map,
return -ENOTSUP;
}
+ /* XXX: attach_btf_obj_fd is needed as well */
+ if (mod_btf)
+ prog->attach_btf_obj_fd = mod_btf->fd;
prog->attach_btf_id = kern_type_id;
prog->expected_attach_type = kern_member_idx;
@@ -1128,8 +1179,8 @@ static int bpf_object__init_kern_struct_ops_maps(struct bpf_object *obj)
if (!bpf_map__is_struct_ops(map))
continue;
- err = bpf_map__init_kern_struct_ops(map, obj->btf,
- obj->btf_vmlinux);
+ /* XXX: should be a module btf if not vmlinux */
+ err = bpf_map__init_kern_struct_ops(map, obj);
if (err)
return err;
}
@@ -5108,8 +5159,10 @@ static int bpf_object__create_map(struct bpf_object *obj, struct bpf_map *map, b
create_attr.numa_node = map->numa_node;
create_attr.map_extra = map->map_extra;
- if (bpf_map__is_struct_ops(map))
+ if (bpf_map__is_struct_ops(map)) {
create_attr.btf_vmlinux_value_type_id = map->btf_vmlinux_value_type_id;
+ create_attr.mod_btf_fd = map->mod_btf ? map->mod_btf->fd : 0;
+ }
if (obj->btf && btf__fd(obj->btf) >= 0) {
create_attr.btf_fd = btf__fd(obj->btf);
@@ -7582,40 +7635,6 @@ static int bpf_object__read_kallsyms_file(struct bpf_object *obj)
return libbpf_kallsyms_parse(kallsyms_cb, obj);
}
-static int find_ksym_btf_id(struct bpf_object *obj, const char *ksym_name,
- __u16 kind, struct btf **res_btf,
- struct module_btf **res_mod_btf)
-{
- struct module_btf *mod_btf;
- struct btf *btf;
- int i, id, err;
-
- btf = obj->btf_vmlinux;
- mod_btf = NULL;
- id = btf__find_by_name_kind(btf, ksym_name, kind);
-
- if (id == -ENOENT) {
- err = load_module_btfs(obj);
- if (err)
- return err;
-
- for (i = 0; i < obj->btf_module_cnt; i++) {
- /* we assume module_btf's BTF FD is always >0 */
- mod_btf = &obj->btf_modules[i];
- btf = mod_btf->btf;
- id = btf__find_by_name_kind_own(btf, ksym_name, kind);
- if (id != -ENOENT)
- break;
- }
- }
- if (id <= 0)
- return -ESRCH;
-
- *res_btf = btf;
- *res_mod_btf = mod_btf;
- return id;
-}
-
static int bpf_object__resolve_ksym_var_btf_id(struct bpf_object *obj,
struct extern_desc *ext)
{
@@ -7626,7 +7645,7 @@ static int bpf_object__resolve_ksym_var_btf_id(struct bpf_object *obj,
struct btf *btf = NULL;
int id, err;
- id = find_ksym_btf_id(obj, ext->name, BTF_KIND_VAR, &btf, &mod_btf);
+ id = find_kern_btf_id(obj, ext->name, BTF_KIND_VAR, &btf, &mod_btf);
if (id < 0) {
if (id == -ESRCH && ext->is_weak)
return 0;
@@ -7680,7 +7699,7 @@ static int bpf_object__resolve_ksym_func_btf_id(struct bpf_object *obj,
local_func_proto_id = ext->ksym.type_id;
- kfunc_id = find_ksym_btf_id(obj, ext->essent_name ?: ext->name, BTF_KIND_FUNC, &kern_btf,
+ kfunc_id = find_kern_btf_id(obj, ext->essent_name ?: ext->name, BTF_KIND_FUNC, &kern_btf,
&mod_btf);
if (kfunc_id < 0) {
if (kfunc_id == -ESRCH && ext->is_weak)
@@ -9346,9 +9365,9 @@ static int libbpf_find_prog_btf_id(const char *name, __u32 attach_prog_fd)
return err;
}
-static int find_kernel_btf_id(struct bpf_object *obj, const char *attach_name,
- enum bpf_attach_type attach_type,
- int *btf_obj_fd, int *btf_type_id)
+static int find_kernel_attach_btf_id(struct bpf_object *obj, const char *attach_name,
+ enum bpf_attach_type attach_type,
+ int *btf_obj_fd, int *btf_type_id)
{
int ret, i;
@@ -9413,7 +9432,7 @@ static int libbpf_find_attach_btf_id(struct bpf_program *prog, const char *attac
*btf_obj_fd = 0;
*btf_type_id = 1;
} else {
- err = find_kernel_btf_id(prog->obj, attach_name, attach_type, btf_obj_fd, btf_type_id);
+ err = find_kernel_attach_btf_id(prog->obj, attach_name, attach_type, btf_obj_fd, btf_type_id);
}
if (err) {
pr_warn("prog '%s': failed to find kernel BTF type ID of '%s': %d\n",
@@ -12827,9 +12846,9 @@ int bpf_program__set_attach_target(struct bpf_program *prog,
err = bpf_object__load_vmlinux_btf(prog->obj, true);
if (err)
return libbpf_err(err);
- err = find_kernel_btf_id(prog->obj, attach_func_name,
- prog->expected_attach_type,
- &btf_obj_fd, &btf_id);
+ err = find_kernel_attach_btf_id(prog->obj, attach_func_name,
+ prog->expected_attach_type,
+ &btf_obj_fd, &btf_id);
if (err)
return libbpf_err(err);
}
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RFC bpf-next v2 7/9] bpf: export btf_ctx_access to modules.
2023-09-13 6:14 [RFC bpf-next v2 0/9] Registrating struct_ops types from modules thinker.li
` (5 preceding siblings ...)
2023-09-13 6:14 ` [RFC bpf-next v2 6/9] libbpf: Find correct module BTFs for struct_ops maps and progs thinker.li
@ 2023-09-13 6:14 ` thinker.li
2023-09-13 6:14 ` [RFC bpf-next v2 8/9] selftests/bpf: test case for register_bpf_struct_ops() thinker.li
2023-09-13 6:14 ` [RFC bpf-next v2 9/9] Comments and debug thinker.li
8 siblings, 0 replies; 16+ messages in thread
From: thinker.li @ 2023-09-13 6:14 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
btf_ctx_access() is needed by module to call bpf_tracing_btf_ctx_access().
---
kernel/bpf/btf.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
index 55d76d85c6ec..ae2cd120e426 100644
--- a/kernel/bpf/btf.c
+++ b/kernel/bpf/btf.c
@@ -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 */
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RFC bpf-next v2 8/9] selftests/bpf: test case for register_bpf_struct_ops().
2023-09-13 6:14 [RFC bpf-next v2 0/9] Registrating struct_ops types from modules thinker.li
` (6 preceding siblings ...)
2023-09-13 6:14 ` [RFC bpf-next v2 7/9] bpf: export btf_ctx_access to modules thinker.li
@ 2023-09-13 6:14 ` thinker.li
2023-09-13 6:14 ` [RFC bpf-next v2 9/9] Comments and debug thinker.li
8 siblings, 0 replies; 16+ messages in thread
From: thinker.li @ 2023-09-13 6:14 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
.../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
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
index cefc5dd72573..136638c15047 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.c
@@ -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);
}
diff --git a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
index f32793efe095..ca5435751c79 100644
--- a/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
+++ b/tools/testing/selftests/bpf/bpf_testmod/bpf_testmod.h
@@ -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 */
diff --git a/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
new file mode 100644
index 000000000000..29dd203121f5
--- /dev/null
+++ b/tools/testing/selftests/bpf/prog_tests/test_struct_ops_module.c
@@ -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();
+}
+
diff --git a/tools/testing/selftests/bpf/progs/struct_ops_module.c b/tools/testing/selftests/bpf/progs/struct_ops_module.c
new file mode 100644
index 000000000000..cb305d04342f
--- /dev/null
+++ b/tools/testing/selftests/bpf/progs/struct_ops_module.c
@@ -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,
+};
+
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread* [RFC bpf-next v2 9/9] Comments and debug
2023-09-13 6:14 [RFC bpf-next v2 0/9] Registrating struct_ops types from modules thinker.li
` (7 preceding siblings ...)
2023-09-13 6:14 ` [RFC bpf-next v2 8/9] selftests/bpf: test case for register_bpf_struct_ops() thinker.li
@ 2023-09-13 6:14 ` thinker.li
8 siblings, 0 replies; 16+ messages in thread
From: thinker.li @ 2023-09-13 6:14 UTC (permalink / raw)
To: bpf, ast, martin.lau, song, kernel-team, andrii
Cc: sinquersw, kuifeng, Kui-Feng Lee
From: Kui-Feng Lee <thinker.li@gmail.com>
Signed-off-by: Kui-Feng Lee <thinker.li@gmail.com>
---
kernel/bpf/bpf_struct_ops.c | 15 +++++++++++++++
kernel/bpf/syscall.c | 6 ++++++
tools/lib/bpf/libbpf.c | 26 ++++++++++++++++++++++++++
3 files changed, 47 insertions(+)
diff --git a/kernel/bpf/bpf_struct_ops.c b/kernel/bpf/bpf_struct_ops.c
index 845873bc806d..47045b026bec 100644
--- a/kernel/bpf/bpf_struct_ops.c
+++ b/kernel/bpf/bpf_struct_ops.c
@@ -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;
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
index 04d3017b7db1..75c4f0b251a3 100644
--- a/kernel/bpf/syscall.c
+++ b/kernel/bpf/syscall.c
@@ -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);
diff --git a/tools/lib/bpf/libbpf.c b/tools/lib/bpf/libbpf.c
index 211889d37320..cd866a30471b 100644
--- a/tools/lib/bpf/libbpf.c
+++ b/tools/lib/bpf/libbpf.c
@@ -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;
}
--
2.34.1
^ permalink raw reply related [flat|nested] 16+ messages in thread