Re: [PATCH bpf-next v2 15/16] bpf: Realign skb metadata for TC progs using data_meta

[Jakub Sitnicki <jakub@cloudflare.com>]

On Mon, Jan 05, 2026 at 06:04 PM -08, Alexei Starovoitov wrote:
> On Mon, Jan 5, 2026 at 3:19 PM Amery Hung <ameryhung@gmail.com> wrote:
>>
>> >
>> > >
>> > > I guess we can mark such emitted call in insn_aux_data as finalized
>> > > and get_func_proto() isn't needed.
>> >
>> > It is a good idea.
>> >
>>
>> Hmm, insn_aux_data has to be marked in gen_{pro,epi}logue since this
>> is the only place we know whether the call needs fixup or not. However
>> insn_aux_data is not available yet in gen_{pro,epi}logue because we
>> haven't resized insn_aux_data.
>>
>> Can we do some hack based on the fact that calls emitted by
>> BPF_EMIT_CALL() are finalized while calls emitted by BPF_RAW_INSN()
>> most likely are not?
>> Let BPF_EMIT_CALL() mark the call insn as finalized temporarily (e.g.,
>> .off = 1). Then, when do_misc_fixups() encounters it just reset off to
>> 0 and don't call get_func_proto().
>
> marking inside insn via off=1 or whatever is an option,
> but once we remove BPF_CALL_KFUNC from gen_prologue we can
> delete add_kfunc_in_insns() altogether and replace it with
> a similar loop that does
> if (bpf_helper_call()) mark insn_aux_data.
>
> That would be a nice benefit, since add_kfunc_call() from there
> was always a bit odd, since we're adding kfuncs early before the main
> verifier pass and after, because of gen_prologue.

Thanks for all the pointers.

I understood we're looking for something like this:

---8<---
diff --git a/include/linux/bpf_verifier.h b/include/linux/bpf_verifier.h
index b32ddf0f0ab3..9ccd56c04a45 100644
--- a/include/linux/bpf_verifier.h
+++ b/include/linux/bpf_verifier.h
@@ -561,6 +561,7 @@ struct bpf_insn_aux_data {
 	bool non_sleepable; /* helper/kfunc may be called from non-sleepable context */
 	bool is_iter_next; /* bpf_iter_<type>_next() kfunc call */
 	bool call_with_percpu_alloc_ptr; /* {this,per}_cpu_ptr() with prog percpu alloc */
+	bool finalized_call; /* call holds function offset relative to __bpf_base_call */
 	u8 alu_state; /* used in combination with alu_limit */
 	/* true if STX or LDX instruction is a part of a spill/fill
 	 * pattern for a bpf_fastcall call.
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 1ca5c5e895ee..cc737d103cdd 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -21806,6 +21806,14 @@ static int convert_ctx_accesses(struct bpf_verifier_env *env)
 			env->prog = new_prog;
 			delta += cnt - 1;
 
+			/* gen_prologue emits function calls with target address
+			 * relative to __bpf_call_base. Skip patch_call_imm fixup.
+			 */
+			for (i = 0; i < cnt - 1; i++) {
+				if (bpf_helper_call(&env->prog->insnsi[i]))
+					env->insn_aux_data[i].finalized_call = true;
+			}
+
 			ret = add_kfunc_in_insns(env, insn_buf, cnt - 1);
 			if (ret < 0)
 				return ret;
@@ -23412,6 +23420,9 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 			goto next_insn;
 		}
 patch_call_imm:
+		if (env->insn_aux_data[i + delta].finalized_call)
+			goto next_insn;
+
 		fn = env->ops->get_func_proto(insn->imm, env->prog);
 		/* all functions that have prototype and verifier allowed
 		 * programs to call them, must be real in-kernel functions
@@ -23423,6 +23434,7 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
 			return -EFAULT;
 		}
 		insn->imm = fn->func - __bpf_call_base;
+		env->insn_aux_data[i + delta].finalized_call = true;
 next_insn:
 		if (subprogs[cur_subprog + 1].start == i + delta + 1) {
 			subprogs[cur_subprog].stack_depth += stack_depth_extra;
diff --git a/net/core/filter.c b/net/core/filter.c
index 7f5bc6a505e1..53993c2c492d 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -9082,8 +9082,7 @@ static int bpf_unclone_prologue(struct bpf_insn *insn_buf, u32 pkt_access_flags,
 	/* ret = bpf_skb_pull_data(skb, 0); */
 	*insn++ = BPF_MOV64_REG(BPF_REG_6, BPF_REG_1);
 	*insn++ = BPF_ALU64_REG(BPF_XOR, BPF_REG_2, BPF_REG_2);
-	*insn++ = BPF_RAW_INSN(BPF_JMP | BPF_CALL, 0, 0, 0,
-			       BPF_FUNC_skb_pull_data);
+	*insn++ = BPF_EMIT_CALL(bpf_skb_pull_data);
 	/* if (!ret)
 	 *      goto restore;
 	 * return TC_ACT_SHOT;
@@ -9135,11 +9134,8 @@ static int bpf_gen_ld_abs(const struct bpf_insn *orig,
 	return insn - insn_buf;
 }
 
-__bpf_kfunc_start_defs();
-
-__bpf_kfunc void bpf_skb_meta_realign(struct __sk_buff *skb_)
+static void bpf_skb_meta_realign(struct sk_buff *skb)
 {
-	struct sk_buff *skb = (typeof(skb))skb_;
 	u8 *meta_end = skb_metadata_end(skb);
 	u8 meta_len = skb_metadata_len(skb);
 	u8 *meta;
@@ -9161,14 +9157,6 @@ __bpf_kfunc void bpf_skb_meta_realign(struct __sk_buff *skb_)
 	bpf_compute_data_pointers(skb);
 }
 
-__bpf_kfunc_end_defs();
-
-BTF_KFUNCS_START(tc_cls_act_hidden_ids)
-BTF_ID_FLAGS(func, bpf_skb_meta_realign)
-BTF_KFUNCS_END(tc_cls_act_hidden_ids)
-
-BTF_ID_LIST_SINGLE(bpf_skb_meta_realign_ids, func, bpf_skb_meta_realign)
-
 static int tc_cls_act_prologue(struct bpf_insn *insn_buf, u32 pkt_access_flags,
 			       const struct bpf_prog *prog)
 {
@@ -9182,8 +9170,10 @@ static int tc_cls_act_prologue(struct bpf_insn *insn_buf, u32 pkt_access_flags,
 		 * r0 = bpf_skb_meta_realign(r1); // r0 is undefined
 		 * r1 = r6;
 		 */
+		BUILD_BUG_ON(!__same_type(&bpf_skb_meta_realign,
+					  (void (*)(struct sk_buff *skb))NULL));
 		*insn++ = BPF_MOV64_REG(BPF_REG_6, BPF_REG_1);
-		*insn++ = BPF_CALL_KFUNC(0, bpf_skb_meta_realign_ids[0]);
+		*insn++ = BPF_EMIT_CALL(bpf_skb_meta_realign);
 		*insn++ = BPF_MOV64_REG(BPF_REG_1, BPF_REG_6);
 	}
 	cnt = bpf_unclone_prologue(insn, pkt_access_flags, prog, TC_ACT_SHOT);