On 2024/5/9 23:05, Leon Hwang wrote: > This patch fixes a tailcall issue caused by abusing the tailcall in > bpf2bpf feature. > > As we know, tail_call_cnt propagates by rax from caller to callee when > to call subprog in tailcall context. But, like the following example, > MAX_TAIL_CALL_CNT won't work because of missing tail_call_cnt > back-propagation from callee to caller. > > \#include <linux/bpf.h> > \#include <bpf/bpf_helpers.h> > \#include "bpf_legacy.h" > > struct { > __uint(type, BPF_MAP_TYPE_PROG_ARRAY); > __uint(max_entries, 1); > __uint(key_size, sizeof(__u32)); > __uint(value_size, sizeof(__u32)); > } jmp_table SEC(".maps"); > > int count = 0; > > static __noinline > int subprog_tail1(struct __sk_buff *skb) > { > bpf_tail_call_static(skb, &jmp_table, 0); > return 0; > } > > static __noinline > int subprog_tail2(struct __sk_buff *skb) > { > bpf_tail_call_static(skb, &jmp_table, 0); > return 0; > } > > SEC("tc") > int entry(struct __sk_buff *skb) > { > volatile int ret = 1; > > count++; > subprog_tail1(skb); > subprog_tail2(skb); > > return ret; > } > > char __license[] SEC("license") = "GPL"; > > At run time, the tail_call_cnt in entry() will be propagated to > subprog_tail1() and subprog_tail2(). But, when the tail_call_cnt in > subprog_tail1() updates when bpf_tail_call_static(), the tail_call_cnt > in entry() won't be updated at the same time. As a result, in entry(), > when tail_call_cnt in entry() is less than MAX_TAIL_CALL_CNT and > subprog_tail1() returns because of MAX_TAIL_CALL_CNT limit, > bpf_tail_call_static() in suprog_tail2() is able to run because the > tail_call_cnt in subprog_tail2() propagated from entry() is less than > MAX_TAIL_CALL_CNT. > > So, how many tailcalls are there for this case if no error happens? > > From top-down view, does it look like hierarchy layer and layer? > > With view, there will be 2+4+8+...+2^33 = 2^34 - 2 = 17,179,869,182 > tailcalls for this case. > > How about there are N subprog_tail() in entry()? There will be almost > N^34 tailcalls. > > Then, in this patch, it resolves this case on x86_64. > > In stead of propagating tail_call_cnt from caller to callee, it > propagate its pointer, tail_call_cnt_ptr, tcc_ptr for short. > > However, where does it store tail_call_cnt? > > It stores tail_call_cnt on the stack of bpf prog's caller by the way in > previous patch "bpf: Introduce bpf_jit_supports_tail_call_cnt_ptr()". > Then, in bpf prog's prologue, it loads tcc_ptr from bpf_tail_call_run_ctx, > and restores the original ctx from bpf_tail_call_run_ctx meanwhile. > > Then, when a tailcall runs, it compares tail_call_cnt accessed by > tcc_ptr with MAX_TAIL_CALL_CNT and then increments tail_call_cnt at > tcc_ptr. > > Furthermore, when trampoline is the caller of bpf prog, it is required > to prepare tail_call_cnt and tail call run ctx on the stack of the > trampoline. > Oh, I missed a case here. This patch set is unable to provide tcc_ptr for freplace programs that use tail calls in bpf2bpf. How can this approach provide tcc_ptr for freplace programs? Achieving this is not straightforward. However, it is simpler to disable the use of tail calls in bpf2bpf for freplace programs, even though this is a desired feature for my project. Therefore, I will disable it in the v5 patch set. Thanks, Leon