bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions
@ 2025-10-29 19:02 Ihor Solodrai
  2025-10-29 19:02 ` [PATCH dwarves v1 1/3] btf_encoder: refactor btf_encoder__add_func_proto Ihor Solodrai
                   ` (3 more replies)
  0 siblings, 4 replies; 10+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:02 UTC (permalink / raw)
  To: dwarves, alan.maguire, acme; +Cc: bpf, andrii, ast, eddyz87, tj, kernel-team

This series implements BTF encoding of BPF kernel functions marked
with KF_MAGIC_ARGS flag in pahole.

The kfunc flag indicates that the arguments of a kfunc with __magic
suffix are implicitly set by the verifier, and so pahole must emit two
functions to BTF:
  * kfunc_impl() with the arguments matching kernel declaration
  * kfunc() with __magic arguments omitted

For more details see relevant patch series for BPF:
"bpf: magic kernel functions"

This series is built upon KF_IMPLICIT_PROG_AUX_ARG support [1],
although the approach changed signifcantly to call it a v2.

[1] https://lore.kernel.org/dwarves/20250924211512.1287298-1-ihor.solodrai@linux.dev/

Ihor Solodrai (3):
  btf_encoder: refactor btf_encoder__add_func_proto
  btf_encoder: factor out btf_encoder__add_bpf_kfunc()
  btf_encoder: support kfuncs with KF_MAGIC_ARGS flag

 btf_encoder.c | 292 ++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 222 insertions(+), 70 deletions(-)

-- 
2.51.1


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH dwarves v1 1/3] btf_encoder: refactor btf_encoder__add_func_proto
  2025-10-29 19:02 [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions Ihor Solodrai
@ 2025-10-29 19:02 ` Ihor Solodrai
  2025-10-30  0:34   ` Eduard Zingerman
  2025-10-29 19:02 ` [PATCH dwarves v1 2/3] btf_encoder: factor out btf_encoder__add_bpf_kfunc() Ihor Solodrai
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 10+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:02 UTC (permalink / raw)
  To: dwarves, alan.maguire, acme; +Cc: bpf, andrii, ast, eddyz87, tj, kernel-team

btf_encoder__add_func_proto() essentially implements two independent
code paths depending on input arguments: one for struct ftype and the
other for struct btf_encoder_func_state.

Split btf_encoder__add_func_proto() into two variants:
  * btf_encoder__add_func_proto_for_ftype()
  * btf_encoder__add_func_proto_for_state()

And factor out common btf_encoder__emit_func_proto() subroutine.

No functional changes.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 btf_encoder.c | 135 +++++++++++++++++++++++++++++---------------------
 1 file changed, 79 insertions(+), 56 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 03bc3c7..0416824 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -815,82 +815,105 @@ static inline bool is_kfunc_state(struct btf_encoder_func_state *state)
 	return state && state->elf && state->elf->kfunc;
 }
 
-static int32_t btf_encoder__add_func_proto(struct btf_encoder *encoder, struct ftype *ftype,
-					   struct btf_encoder_func_state *state)
+static int32_t btf_encoder__emit_func_proto(struct btf_encoder *encoder,
+					    uint32_t type_id,
+					    uint16_t nr_params)
 {
 	const struct btf_type *t;
-	struct btf *btf;
-	struct parameter *param;
+	uint32_t ret;
+
+	ret = btf__add_func_proto(encoder->btf, type_id);
+	if (ret > 0) {
+		t = btf__type_by_id(encoder->btf, ret);
+		btf_encoder__log_type(encoder, t, false, false,
+			"return=%u args=(%s", t->type, !nr_params ? "void)\n" : "");
+	} else {
+		btf__log_err(encoder->btf, BTF_KIND_FUNC_PROTO, NULL, true, ret,
+			     "return=%u vlen=%u Error emitting BTF type",
+			     type_id, nr_params);
+	}
+
+	return ret;
+}
+
+static int32_t btf_encoder__add_func_proto_for_ftype(struct btf_encoder *encoder,
+						     struct ftype *ftype)
+{
 	uint16_t nr_params, param_idx;
+	struct parameter *param;
 	int32_t id, type_id;
+	const char *name;
+
+	assert(ftype != NULL);
+
+	/* add btf_type for func_proto */
+	nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
+	type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
+
+	id = btf_encoder__emit_func_proto(encoder, type_id, nr_params);
+	if (id < 0)
+		return id;
+
+	/* add parameters */
+	param_idx = 0;
+
+	ftype__for_each_parameter(ftype, param) {
+		name = parameter__name(param);
+		type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type;
+		++param_idx;
+		if (btf_encoder__add_func_param(encoder, name, type_id, param_idx == nr_params))
+			return -1;
+	}
+
+	++param_idx;
+	if (ftype->unspec_parms)
+		if (btf_encoder__add_func_param(encoder, NULL, 0, param_idx == nr_params))
+			return -1;
+
+	return id;
+}
+
+static int32_t btf_encoder__add_func_proto_for_state(struct btf_encoder *encoder, struct btf_encoder_func_state *state)
+{
+	uint16_t nr_params, param_idx;
 	char tmp_name[KSYM_NAME_LEN];
+	int32_t id, type_id;
 	const char *name;
+	struct btf *btf;
 
-	assert(ftype != NULL || state != NULL);
+	assert(state != NULL);
 
 	if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes)
 		if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
 			return -1;
 
-	/* add btf_type for func_proto */
-	if (ftype) {
-		btf = encoder->btf;
-		nr_params = ftype->nr_parms + (ftype->unspec_parms ? 1 : 0);
-		type_id = btf_encoder__tag_type(encoder, ftype->tag.type);
-	} else if (state) {
-		encoder = state->encoder;
-		btf = state->encoder->btf;
-		nr_params = state->nr_parms;
-		type_id = state->ret_type_id;
-	} else {
-		return 0;
-	}
+	encoder = state->encoder;
+	btf = state->encoder->btf;
+	nr_params = state->nr_parms;
+	type_id = state->ret_type_id;
 
-	id = btf__add_func_proto(btf, type_id);
-	if (id > 0) {
-		t = btf__type_by_id(btf, id);
-		btf_encoder__log_type(encoder, t, false, false, "return=%u args=(%s", t->type, !nr_params ? "void)\n" : "");
-	} else {
-		btf__log_err(btf, BTF_KIND_FUNC_PROTO, NULL, true, id,
-			     "return=%u vlen=%u Error emitting BTF type",
-			     type_id, nr_params);
+	id = btf_encoder__emit_func_proto(encoder, type_id, nr_params);
+	if (id < 0)
 		return id;
-	}
 
 	/* add parameters */
 	param_idx = 0;
-	if (ftype) {
-		ftype__for_each_parameter(ftype, param) {
-			const char *name = parameter__name(param);
-
-			type_id = param->tag.type == 0 ? 0 : encoder->type_id_off + param->tag.type;
-			++param_idx;
-			if (btf_encoder__add_func_param(encoder, name, type_id,
-							param_idx == nr_params))
-				return -1;
-		}
 
-		++param_idx;
-		if (ftype->unspec_parms)
-			if (btf_encoder__add_func_param(encoder, NULL, 0,
-							param_idx == nr_params))
-				return -1;
-	} else {
-		for (param_idx = 0; param_idx < nr_params; param_idx++) {
-			struct btf_encoder_func_parm *p = &state->parms[param_idx];
+	for (param_idx = 0; param_idx < nr_params; param_idx++) {
+		struct btf_encoder_func_parm *p = &state->parms[param_idx];
 
-			name = btf__name_by_offset(btf, p->name_off);
+		name = btf__name_by_offset(btf, p->name_off);
 
-			/* adding BTF data may result in a move of the
-			 * name string memory, so make a temporary copy.
-			 */
-			strncpy(tmp_name, name, sizeof(tmp_name) - 1);
+		/* adding BTF data may result in a move of the
+		 * name string memory, so make a temporary copy.
+		 */
+		strncpy(tmp_name, name, sizeof(tmp_name) - 1);
 
-			if (btf_encoder__add_func_param(encoder, tmp_name, p->type_id,
-							param_idx == nr_params))
-				return -1;
-		}
+		if (btf_encoder__add_func_param(encoder, tmp_name, p->type_id,
+						param_idx == nr_params))
+			return -1;
 	}
+
 	return id;
 }
 
@@ -1349,7 +1372,7 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
 	uint16_t idx;
 	int err;
 
-	btf_fnproto_id = btf_encoder__add_func_proto(encoder, NULL, state);
+	btf_fnproto_id = btf_encoder__add_func_proto_for_state(encoder, state);
 	name = func->name;
 	if (btf_fnproto_id >= 0)
 		btf_fn_id = btf_encoder__add_ref_type(encoder, BTF_KIND_FUNC, btf_fnproto_id,
@@ -1686,7 +1709,7 @@ static int btf_encoder__encode_tag(struct btf_encoder *encoder, struct tag *tag,
 	case DW_TAG_enumeration_type:
 		return btf_encoder__add_enum_type(encoder, tag, conf_load);
 	case DW_TAG_subroutine_type:
-		return btf_encoder__add_func_proto(encoder, tag__ftype(tag), NULL);
+		return btf_encoder__add_func_proto_for_ftype(encoder, tag__ftype(tag));
         case DW_TAG_unspecified_type:
 		/* Just don't encode this for now, converting anything with this type to void (0) instead.
 		 *
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH dwarves v1 2/3] btf_encoder: factor out btf_encoder__add_bpf_kfunc()
  2025-10-29 19:02 [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions Ihor Solodrai
  2025-10-29 19:02 ` [PATCH dwarves v1 1/3] btf_encoder: refactor btf_encoder__add_func_proto Ihor Solodrai
@ 2025-10-29 19:02 ` Ihor Solodrai
  2025-10-29 19:02 ` [PATCH dwarves v1 3/3] btf_encoder: support kfuncs with KF_MAGIC_ARGS flag Ihor Solodrai
  2025-11-04 20:59 ` [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions Alan Maguire
  3 siblings, 0 replies; 10+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:02 UTC (permalink / raw)
  To: dwarves, alan.maguire, acme; +Cc: bpf, andrii, ast, eddyz87, tj, kernel-team

Emitting BTF for BPF kernel functions requires special
handling. Consolidate this behavior into btf_encoder__add_bpf_kfunc()
function, which uses simplified btf_encoder_add_func()

No functional changes.

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 btf_encoder.c | 42 +++++++++++++++++++++++++++---------------
 1 file changed, 27 insertions(+), 15 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index 0416824..b730280 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -774,6 +774,7 @@ static int btf__tag_bpf_arena_arg(struct btf *btf, struct btf_encoder_func_state
 	return id;
 }
 
+/* Modifies state->ret_type_id and state->parms[i].type_id for flagged kfuncs */
 static int btf__add_bpf_arena_type_tags(struct btf *btf, struct btf_encoder_func_state *state)
 {
 	uint32_t flags = state->elf->kfunc_flags;
@@ -883,10 +884,6 @@ static int32_t btf_encoder__add_func_proto_for_state(struct btf_encoder *encoder
 
 	assert(state != NULL);
 
-	if (is_kfunc_state(state) && encoder->tag_kfuncs && encoder->encode_attributes)
-		if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
-			return -1;
-
 	encoder = state->encoder;
 	btf = state->encoder->btf;
 	nr_params = state->nr_parms;
@@ -1370,7 +1367,6 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
 	const char *value;
 	char tmp_value[KSYM_NAME_LEN];
 	uint16_t idx;
-	int err;
 
 	btf_fnproto_id = btf_encoder__add_func_proto_for_state(encoder, state);
 	name = func->name;
@@ -1383,15 +1379,6 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
 		return -1;
 	}
 
-	if (func->kfunc && encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag) {
-		err = btf__tag_kfunc(encoder->btf, func, btf_fn_id);
-		if (err < 0)
-			return err;
-	}
-
-	if (state->nr_annots == 0)
-		return 0;
-
 	for (idx = 0; idx < state->nr_annots; idx++) {
 		struct btf_encoder_func_annot *a = &state->annots[idx];
 
@@ -1414,6 +1401,28 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
 		return -1;
 	}
 
+	return btf_fn_id;
+}
+
+static int btf_encoder__add_bpf_kfunc(struct btf_encoder *encoder,
+				      struct btf_encoder_func_state *state)
+{
+	int btf_fn_id, err;
+
+	if (encoder->tag_kfuncs && encoder->encode_attributes)
+		if (btf__add_bpf_arena_type_tags(encoder->btf, state) < 0)
+			return -1;
+
+	btf_fn_id = btf_encoder__add_func(encoder, state);
+	if (btf_fn_id < 0)
+		return -1;
+
+	if (encoder->tag_kfuncs && !encoder->skip_encoding_decl_tag) {
+		err = btf__tag_kfunc(encoder->btf, state->elf, btf_fn_id);
+		if (err < 0)
+			return -1;
+	}
+
 	return 0;
 }
 
@@ -1511,7 +1520,10 @@ static int btf_encoder__add_saved_funcs(struct btf_encoder *encoder, bool skip_e
 					0, 0);
 
 		if (add_to_btf) {
-			err = btf_encoder__add_func(state->encoder, state);
+			if (is_kfunc_state(state))
+				err = btf_encoder__add_bpf_kfunc(state->encoder, state);
+			else
+				err = btf_encoder__add_func(state->encoder, state);
 			if (err < 0)
 				goto out;
 		}
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH dwarves v1 3/3] btf_encoder: support kfuncs with KF_MAGIC_ARGS flag
  2025-10-29 19:02 [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions Ihor Solodrai
  2025-10-29 19:02 ` [PATCH dwarves v1 1/3] btf_encoder: refactor btf_encoder__add_func_proto Ihor Solodrai
  2025-10-29 19:02 ` [PATCH dwarves v1 2/3] btf_encoder: factor out btf_encoder__add_bpf_kfunc() Ihor Solodrai
@ 2025-10-29 19:02 ` Ihor Solodrai
  2025-11-04 20:59 ` [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions Alan Maguire
  3 siblings, 0 replies; 10+ messages in thread
From: Ihor Solodrai @ 2025-10-29 19:02 UTC (permalink / raw)
  To: dwarves, alan.maguire, acme; +Cc: bpf, andrii, ast, eddyz87, tj, kernel-team

For BPF kernel functions marked with KF_MAGIC_ARGS flag we emit two
functions to BTF:
  * kfunc_impl() with prototype that matches kernel declaration
  * kfunc() with prototype that omits __magic arguments

Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>
---
 btf_encoder.c | 121 +++++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 119 insertions(+), 2 deletions(-)

diff --git a/btf_encoder.c b/btf_encoder.c
index b730280..7a10be3 100644
--- a/btf_encoder.c
+++ b/btf_encoder.c
@@ -48,6 +48,7 @@
 #define KF_ARENA_RET  (1 << 13)
 #define KF_ARENA_ARG1 (1 << 14)
 #define KF_ARENA_ARG2 (1 << 15)
+#define KF_MAGIC_ARGS (1 << 16)
 
 struct btf_id_and_flag {
 	uint32_t id;
@@ -1404,8 +1405,8 @@ static int32_t btf_encoder__add_func(struct btf_encoder *encoder,
 	return btf_fn_id;
 }
 
-static int btf_encoder__add_bpf_kfunc(struct btf_encoder *encoder,
-				      struct btf_encoder_func_state *state)
+static int btf_encoder__add_bpf_kfunc_instance(struct btf_encoder *encoder,
+					       struct btf_encoder_func_state *state)
 {
 	int btf_fn_id, err;
 
@@ -1426,6 +1427,122 @@ static int btf_encoder__add_bpf_kfunc(struct btf_encoder *encoder,
 	return 0;
 }
 
+static inline bool str_ends_with(const char *str, const char *suffix)
+{
+	int suffix_len = strlen(suffix);
+	int str_len = strlen(str);
+
+	if (str_len < suffix_len)
+		return false;
+
+	return strcmp(str + str_len - suffix_len, suffix) == 0;
+}
+
+#define BPF_KF_IMPL_SUFFIX "_impl"
+#define BPF_KF_ARG_MAGIC_SUFFIX "__magic"
+
+static inline bool btf__is_kf_magic_arg(const struct btf *btf, const struct btf_encoder_func_parm *p)
+{
+	const char *name;
+
+	name = btf__name_by_offset(btf, p->name_off);
+	if (!name)
+		return false;
+
+	return str_ends_with(name, BPF_KF_ARG_MAGIC_SUFFIX);
+}
+
+/*
+ * A kfunc with KF_MAGIC_ARGS flag has some number of arguments set implicitly by the BPF
+ * verifier. Such arguments are identified by __magic suffix in the argument name.
+ * process_kfunc_magic_args() checks the arguments from last to first, and returns the number of
+ * magic arguments. Note that *all* magic arguments must come after *all* normal arguments in the
+ * function signature. If this assumption is violated, or if no __magic arguments are found,
+ * process_kfunc_magic_args() returns an error.
+ */
+static int process_kfunc_magic_args(struct btf_encoder_func_state *state)
+{
+	const struct btf *btf = state->encoder->btf;
+	const struct btf_encoder_func_parm *p;
+	int cnt = 0, i;
+
+	for (i = state->nr_parms - 1; i >= 0; i--) {
+		p = &state->parms[i];
+		if (btf__is_kf_magic_arg(btf, p)) {
+			cnt++;
+			if (cnt != state->nr_parms - i)
+				goto out_err;
+		} else if (cnt == 0) {
+			goto out_err;
+		}
+	}
+
+	return cnt;
+
+out_err:
+	btf__log_err(btf, BTF_KIND_FUNC_PROTO, state->elf->name, true, 0,
+		     "return=%u Error emitting BTF func proto for KF_MAGIC_ARGS kfunc: unexpected kfunc signature",
+		     p->type_id);
+	return -1;
+}
+
+/*
+ * For KF_MAGIC_ARGS kfuncs we emit two BTF functions (and protos):
+ *   - bpf_foo_impl(<original kernel args>)
+ *   - bpf_foo(<bpf args w/o magic args>)
+ * We achieve this by creating a temporary btf_encoder_func_state-s
+ */
+static int btf_encoder__add_bpf_kfunc_with_magic_args(struct btf_encoder *encoder,
+						      struct btf_encoder_func_state *state)
+{
+	struct btf_encoder_func_annot tmp_annots[state->nr_annots];
+	struct btf_encoder_func_state tmp_state = *state;
+	struct elf_function tmp_elf = *state->elf;
+	char tmp_name[KSYM_NAME_LEN];
+	int err, i, j, nr_magic_args;
+
+	/* First, add kfunc_impl(), modifying only the name */
+	strcpy(tmp_name, state->elf->name);
+	strcat(tmp_name, BPF_KF_IMPL_SUFFIX);
+	tmp_elf.name = tmp_name;
+	tmp_state.elf = &tmp_elf;
+	err = btf_encoder__add_bpf_kfunc_instance(encoder, &tmp_state);
+	if (err < 0)
+		return -1;
+
+	/* Then add kfunc() with omitted magic arguments */
+	nr_magic_args = process_kfunc_magic_args(state);
+	if (nr_magic_args <= 0)
+		return -1;
+
+	tmp_state.elf = state->elf;
+	tmp_state.nr_parms -= nr_magic_args;
+	j = 0;
+	for (i = 0; i < state->nr_annots; i++) {
+		if (state->annots[i].component_idx < tmp_state.nr_parms)
+			tmp_annots[j++] = state->annots[i];
+	}
+	tmp_state.nr_annots = j;
+	tmp_state.annots = tmp_annots;
+	err = btf_encoder__add_bpf_kfunc_instance(encoder, &tmp_state);
+	if (err < 0)
+		return -1;
+
+	return 0;
+}
+
+static inline int btf_encoder__add_bpf_kfunc(struct btf_encoder *encoder,
+					     struct btf_encoder_func_state *state)
+{
+	uint32_t flags = state->elf->kfunc_flags;
+
+	if (KF_MAGIC_ARGS & flags)
+		return btf_encoder__add_bpf_kfunc_with_magic_args(encoder, state);
+
+	return btf_encoder__add_bpf_kfunc_instance(encoder, state);
+}
+
+
 static int elf_function__name_cmp(const void *_a, const void *_b)
 {
 	const struct elf_function *a = _a;
-- 
2.51.1


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH dwarves v1 1/3] btf_encoder: refactor btf_encoder__add_func_proto
  2025-10-29 19:02 ` [PATCH dwarves v1 1/3] btf_encoder: refactor btf_encoder__add_func_proto Ihor Solodrai
@ 2025-10-30  0:34   ` Eduard Zingerman
  0 siblings, 0 replies; 10+ messages in thread
From: Eduard Zingerman @ 2025-10-30  0:34 UTC (permalink / raw)
  To: Ihor Solodrai, dwarves, alan.maguire, acme
  Cc: bpf, andrii, ast, tj, kernel-team

On Wed, 2025-10-29 at 12:02 -0700, Ihor Solodrai wrote:
> btf_encoder__add_func_proto() essentially implements two independent
> code paths depending on input arguments: one for struct ftype and the
> other for struct btf_encoder_func_state.
> 
> Split btf_encoder__add_func_proto() into two variants:
>   * btf_encoder__add_func_proto_for_ftype()
>   * btf_encoder__add_func_proto_for_state()
> 
> And factor out common btf_encoder__emit_func_proto() subroutine.
> 
> No functional changes.
> 
> Signed-off-by: Ihor Solodrai <ihor.solodrai@linux.dev>

Ha, check this out:
https://lore.kernel.org/bpf/18bee370c4804e666eb7d5360c47439c246e1cb7.camel@gmail.com/
Wrote same thing to Alan on Friday :)

> ---
>  btf_encoder.c | 135 +++++++++++++++++++++++++++++---------------------
>  1 file changed, 79 insertions(+), 56 deletions(-)
> 
> diff --git a/btf_encoder.c b/btf_encoder.c
> index 03bc3c7..0416824 100644
> --- a/btf_encoder.c
> +++ b/btf_encoder.c

[...]

> +static int32_t btf_encoder__add_func_proto_for_state(struct btf_encoder *encoder, struct btf_encoder_func_state *state)

You can get rid of the `encoder` parameter here.
See https://github.com/acmel/dwarves/commit/080d1f27ae71e30c269a1e26e85bb86c3683f195 .

[...]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions
  2025-10-29 19:02 [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions Ihor Solodrai
                   ` (2 preceding siblings ...)
  2025-10-29 19:02 ` [PATCH dwarves v1 3/3] btf_encoder: support kfuncs with KF_MAGIC_ARGS flag Ihor Solodrai
@ 2025-11-04 20:59 ` Alan Maguire
  2025-11-04 22:25   ` Ihor Solodrai
  3 siblings, 1 reply; 10+ messages in thread
From: Alan Maguire @ 2025-11-04 20:59 UTC (permalink / raw)
  To: Ihor Solodrai, dwarves, acme; +Cc: bpf, andrii, ast, eddyz87, tj, kernel-team

On 29/10/2025 19:02, Ihor Solodrai wrote:
> This series implements BTF encoding of BPF kernel functions marked
> with KF_MAGIC_ARGS flag in pahole.
> 
> The kfunc flag indicates that the arguments of a kfunc with __magic
> suffix are implicitly set by the verifier, and so pahole must emit two
> functions to BTF:
>   * kfunc_impl() with the arguments matching kernel declaration
>   * kfunc() with __magic arguments omitted
> 
> For more details see relevant patch series for BPF:
> "bpf: magic kernel functions"
> 
> This series is built upon KF_IMPLICIT_PROG_AUX_ARG support [1],
> although the approach changed signifcantly to call it a v2.
> 
> [1] https://lore.kernel.org/dwarves/20250924211512.1287298-1-ihor.solodrai@linux.dev/
> 
> Ihor Solodrai (3):
>   btf_encoder: refactor btf_encoder__add_func_proto
>   btf_encoder: factor out btf_encoder__add_bpf_kfunc()
>   btf_encoder: support kfuncs with KF_MAGIC_ARGS flag
> 
>  btf_encoder.c | 292 ++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 222 insertions(+), 70 deletions(-)
> 

seems like we could potentially pull in patches 1 and 2 as cleanups
prior to handling the KF_MAGIC/IMPLICIT change; would that be worthwhile?

Thanks!

Alan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions
  2025-11-04 20:59 ` [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions Alan Maguire
@ 2025-11-04 22:25   ` Ihor Solodrai
  2025-11-04 22:33     ` Eduard Zingerman
  2025-11-14 18:33     ` Alan Maguire
  0 siblings, 2 replies; 10+ messages in thread
From: Ihor Solodrai @ 2025-11-04 22:25 UTC (permalink / raw)
  To: Alan Maguire, andrii, dwarves, acme; +Cc: bpf, ast, eddyz87, tj, kernel-team

On 11/4/25 12:59 PM, Alan Maguire wrote:
> On 29/10/2025 19:02, Ihor Solodrai wrote:
>> This series implements BTF encoding of BPF kernel functions marked
>> with KF_MAGIC_ARGS flag in pahole.
>>
>> The kfunc flag indicates that the arguments of a kfunc with __magic
>> suffix are implicitly set by the verifier, and so pahole must emit two
>> functions to BTF:
>>   * kfunc_impl() with the arguments matching kernel declaration
>>   * kfunc() with __magic arguments omitted
>>
>> For more details see relevant patch series for BPF:
>> "bpf: magic kernel functions"
>>
>> This series is built upon KF_IMPLICIT_PROG_AUX_ARG support [1],
>> although the approach changed signifcantly to call it a v2.
>>
>> [1] https://lore.kernel.org/dwarves/20250924211512.1287298-1-ihor.solodrai@linux.dev/
>>
>> Ihor Solodrai (3):
>>   btf_encoder: refactor btf_encoder__add_func_proto
>>   btf_encoder: factor out btf_encoder__add_bpf_kfunc()
>>   btf_encoder: support kfuncs with KF_MAGIC_ARGS flag
>>
>>  btf_encoder.c | 292 ++++++++++++++++++++++++++++++++++++++------------
>>  1 file changed, 222 insertions(+), 70 deletions(-)
>>
> 
> seems like we could potentially pull in patches 1 and 2 as cleanups
> prior to handling the KF_MAGIC/IMPLICIT change; would that be worthwhile?
> 

Hi Alan.

Feel free to merge in the refactoring patches if you think they are
useful. No objections.

I've had another off-list discussion with Andrii, and I am going to
try implementing the next iteration of KF_IMPLICIT_ARGS (no magic,
sorry) feature via resolve_btfids in the kernel tree.

As you of course well know, maintaining backwards compatibility by
tracking pahole version and ensuring correct feature flags in the
Linux kernel build has been tiresome.

We are thinking to address this by separating BTF generation for the
kernel into two independent stages:
  * generate BTF from DWARF with pahole
  * then modify BTF with kernel-specific transformations (with
    resolve_btfids, or however we rename it)

This will reduce the burden from pahole to know the kernel-specific
tweaks of the BTF: adding kfunc decl tags, handling kernel function
flags, etc. pahole will only be concerned with "generic" BTF
generation from DWARF.

This will also free us from the need to control exactly what pahole
features are available (maybe except specifying minimum version) in
the kernel build, because btf2btf transformations will live in the
Linux tree.

KF_IMPLICIT_ARGS will be the proof-of-concept for the approach. If it
works well, then eventually we can migrate existing kernel-specific
BTF generation out from pahole.

Let me know what you think about all this.

Thank you.


> Thanks!
> 
> Alan


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions
  2025-11-04 22:25   ` Ihor Solodrai
@ 2025-11-04 22:33     ` Eduard Zingerman
  2025-11-04 22:37       ` Ihor Solodrai
  2025-11-14 18:33     ` Alan Maguire
  1 sibling, 1 reply; 10+ messages in thread
From: Eduard Zingerman @ 2025-11-04 22:33 UTC (permalink / raw)
  To: Ihor Solodrai, Alan Maguire, andrii, dwarves, acme
  Cc: bpf, ast, tj, kernel-team

On Tue, 2025-11-04 at 14:25 -0800, Ihor Solodrai wrote:
> On 11/4/25 12:59 PM, Alan Maguire wrote:
> > On 29/10/2025 19:02, Ihor Solodrai wrote:
> > > This series implements BTF encoding of BPF kernel functions marked
> > > with KF_MAGIC_ARGS flag in pahole.
> > > 
> > > The kfunc flag indicates that the arguments of a kfunc with __magic
> > > suffix are implicitly set by the verifier, and so pahole must emit two
> > > functions to BTF:
> > >   * kfunc_impl() with the arguments matching kernel declaration
> > >   * kfunc() with __magic arguments omitted
> > > 
> > > For more details see relevant patch series for BPF:
> > > "bpf: magic kernel functions"
> > > 
> > > This series is built upon KF_IMPLICIT_PROG_AUX_ARG support [1],
> > > although the approach changed signifcantly to call it a v2.
> > > 
> > > [1] https://lore.kernel.org/dwarves/20250924211512.1287298-1-ihor.solodrai@linux.dev/
> > > 
> > > Ihor Solodrai (3):
> > >   btf_encoder: refactor btf_encoder__add_func_proto
> > >   btf_encoder: factor out btf_encoder__add_bpf_kfunc()
> > >   btf_encoder: support kfuncs with KF_MAGIC_ARGS flag
> > > 
> > >  btf_encoder.c | 292 ++++++++++++++++++++++++++++++++++++++------------
> > >  1 file changed, 222 insertions(+), 70 deletions(-)
> > > 
> > 
> > seems like we could potentially pull in patches 1 and 2 as cleanups
> > prior to handling the KF_MAGIC/IMPLICIT change; would that be worthwhile?
> > 
> 
> Hi Alan.
> 
> Feel free to merge in the refactoring patches if you think they are
> useful. No objections.

Hi Alan, Ihor,

If you thinking about merging patch #1, please consider my comment:

  > > +static int32_t btf_encoder__add_func_proto_for_state(struct btf_encoder *encoder, struct btf_encoder_func_state *state)

  > You can get rid of the `encoder` parameter here.
  > See https://github.com/acmel/dwarves/commit/080d1f27ae71e30c269a1e26e85bb86c3683f195 .

I sound a bit like a broken record, sorry.

[...]

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions
  2025-11-04 22:33     ` Eduard Zingerman
@ 2025-11-04 22:37       ` Ihor Solodrai
  0 siblings, 0 replies; 10+ messages in thread
From: Ihor Solodrai @ 2025-11-04 22:37 UTC (permalink / raw)
  To: Eduard Zingerman, Alan Maguire, andrii, dwarves, acme
  Cc: bpf, ast, tj, kernel-team

On 11/4/25 2:33 PM, Eduard Zingerman wrote:
> On Tue, 2025-11-04 at 14:25 -0800, Ihor Solodrai wrote:
>> On 11/4/25 12:59 PM, Alan Maguire wrote:
>>> On 29/10/2025 19:02, Ihor Solodrai wrote:
>>>> This series implements BTF encoding of BPF kernel functions marked
>>>> with KF_MAGIC_ARGS flag in pahole.
>>>>
>>>> The kfunc flag indicates that the arguments of a kfunc with __magic
>>>> suffix are implicitly set by the verifier, and so pahole must emit two
>>>> functions to BTF:
>>>>   * kfunc_impl() with the arguments matching kernel declaration
>>>>   * kfunc() with __magic arguments omitted
>>>>
>>>> For more details see relevant patch series for BPF:
>>>> "bpf: magic kernel functions"
>>>>
>>>> This series is built upon KF_IMPLICIT_PROG_AUX_ARG support [1],
>>>> although the approach changed signifcantly to call it a v2.
>>>>
>>>> [1] https://lore.kernel.org/dwarves/20250924211512.1287298-1-ihor.solodrai@linux.dev/
>>>>
>>>> Ihor Solodrai (3):
>>>>   btf_encoder: refactor btf_encoder__add_func_proto
>>>>   btf_encoder: factor out btf_encoder__add_bpf_kfunc()
>>>>   btf_encoder: support kfuncs with KF_MAGIC_ARGS flag
>>>>
>>>>  btf_encoder.c | 292 ++++++++++++++++++++++++++++++++++++++------------
>>>>  1 file changed, 222 insertions(+), 70 deletions(-)
>>>>
>>>
>>> seems like we could potentially pull in patches 1 and 2 as cleanups
>>> prior to handling the KF_MAGIC/IMPLICIT change; would that be worthwhile?
>>>
>>
>> Hi Alan.
>>
>> Feel free to merge in the refactoring patches if you think they are
>> useful. No objections.
> 
> Hi Alan, Ihor,
> 
> If you thinking about merging patch #1, please consider my comment:
> 
>   > > +static int32_t btf_encoder__add_func_proto_for_state(struct btf_encoder *encoder, struct btf_encoder_func_state *state)
> 
>   > You can get rid of the `encoder` parameter here.
>   > See https://github.com/acmel/dwarves/commit/080d1f27ae71e30c269a1e26e85bb86c3683f195 .
> 
> I sound a bit like a broken record, sorry.

Oh, thanks for the ping, I missed this comment.

Alan, let me re-spin the refactoring patches separately.

> 
> [...]


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions
  2025-11-04 22:25   ` Ihor Solodrai
  2025-11-04 22:33     ` Eduard Zingerman
@ 2025-11-14 18:33     ` Alan Maguire
  1 sibling, 0 replies; 10+ messages in thread
From: Alan Maguire @ 2025-11-14 18:33 UTC (permalink / raw)
  To: Ihor Solodrai, andrii, dwarves, acme; +Cc: bpf, ast, eddyz87, tj, kernel-team

On 04/11/2025 22:25, Ihor Solodrai wrote:
> On 11/4/25 12:59 PM, Alan Maguire wrote:
>> On 29/10/2025 19:02, Ihor Solodrai wrote:
>>> This series implements BTF encoding of BPF kernel functions marked
>>> with KF_MAGIC_ARGS flag in pahole.
>>>
>>> The kfunc flag indicates that the arguments of a kfunc with __magic
>>> suffix are implicitly set by the verifier, and so pahole must emit two
>>> functions to BTF:
>>>   * kfunc_impl() with the arguments matching kernel declaration
>>>   * kfunc() with __magic arguments omitted
>>>
>>> For more details see relevant patch series for BPF:
>>> "bpf: magic kernel functions"
>>>
>>> This series is built upon KF_IMPLICIT_PROG_AUX_ARG support [1],
>>> although the approach changed signifcantly to call it a v2.
>>>
>>> [1] https://lore.kernel.org/dwarves/20250924211512.1287298-1-ihor.solodrai@linux.dev/
>>>
>>> Ihor Solodrai (3):
>>>   btf_encoder: refactor btf_encoder__add_func_proto
>>>   btf_encoder: factor out btf_encoder__add_bpf_kfunc()
>>>   btf_encoder: support kfuncs with KF_MAGIC_ARGS flag
>>>
>>>  btf_encoder.c | 292 ++++++++++++++++++++++++++++++++++++++------------
>>>  1 file changed, 222 insertions(+), 70 deletions(-)
>>>
>>
>> seems like we could potentially pull in patches 1 and 2 as cleanups
>> prior to handling the KF_MAGIC/IMPLICIT change; would that be worthwhile?
>>
> 
> Hi Alan.
> 
> Feel free to merge in the refactoring patches if you think they are
> useful. No objections.
> 
> I've had another off-list discussion with Andrii, and I am going to
> try implementing the next iteration of KF_IMPLICIT_ARGS (no magic,
> sorry) feature via resolve_btfids in the kernel tree.
> 
> As you of course well know, maintaining backwards compatibility by
> tracking pahole version and ensuring correct feature flags in the
> Linux kernel build has been tiresome.
> 
> We are thinking to address this by separating BTF generation for the
> kernel into two independent stages:
>   * generate BTF from DWARF with pahole
>   * then modify BTF with kernel-specific transformations (with
>     resolve_btfids, or however we rename it)
> 
> This will reduce the burden from pahole to know the kernel-specific
> tweaks of the BTF: adding kfunc decl tags, handling kernel function
> flags, etc. pahole will only be concerned with "generic" BTF
> generation from DWARF.
> 
> This will also free us from the need to control exactly what pahole
> features are available (maybe except specifying minimum version) in
> the kernel build, because btf2btf transformations will live in the
> Linux tree.
> 
> KF_IMPLICIT_ARGS will be the proof-of-concept for the approach. If it
> works well, then eventually we can migrate existing kernel-specific
> BTF generation out from pahole.
> 
> Let me know what you think about all this.
> 

This sounds great to me; separating the basic BTF generation from the
handling of kernel-specific transformations makes a lot of sense. In a
way we've been conflating core BTF types (enum64, float) and BTF
kernel-specific features up until now. These evolve at different speeds
and the latter is naturally more kernel-tied, so I think the separation
makes a lot of sense. It'd be nice to give resolve_btfids a more
descriptive name if its role changes in this way, but let's not open
another naming can of worms now ;-)

Alan



> Thank you.
> 
> 
>> Thanks!
>>
>> Alan
> 


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2025-11-14 18:33 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29 19:02 [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions Ihor Solodrai
2025-10-29 19:02 ` [PATCH dwarves v1 1/3] btf_encoder: refactor btf_encoder__add_func_proto Ihor Solodrai
2025-10-30  0:34   ` Eduard Zingerman
2025-10-29 19:02 ` [PATCH dwarves v1 2/3] btf_encoder: factor out btf_encoder__add_bpf_kfunc() Ihor Solodrai
2025-10-29 19:02 ` [PATCH dwarves v1 3/3] btf_encoder: support kfuncs with KF_MAGIC_ARGS flag Ihor Solodrai
2025-11-04 20:59 ` [PATCH dwarves v1 0/3] btf_encoder: support for BPF magic kernel functions Alan Maguire
2025-11-04 22:25   ` Ihor Solodrai
2025-11-04 22:33     ` Eduard Zingerman
2025-11-04 22:37       ` Ihor Solodrai
2025-11-14 18:33     ` Alan Maguire

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).