mirror of
https://github.com/torvalds/linux.git
synced 2026-11-06 21:58:01 +01:00
bpf: End bpf_loop() when the time of may_goto is up
When the time of a prog is up all loops with may_goto end, but bpf_loop()
goes on. It calls its callback up to 1 << 23 times. The loops in the
callback make no iteration, but the rest of the callback runs, and that can
be another bpf_loop(). Two test progs with bpf_loop(1 << 23, cb) and some
work in cb return after 4 sec and 10 sec. With this patch they return after
250 ms.
bpf_loop() and bpf_for_each_map_elem() end their loop when the callback
returns 1. So return 1 in the first insns of the callback when the time is
up:
r0 = address of the count
r0 = &(void __percpu *)(r0)
r0 = *(u64 *)(r0 +0)
if r0 != 0x0 goto pc+2
r0 = 1
exit
The verifier has seen what follows: the loop of the helper can end after
each call of the callback.
A function that BPF code calls too, or that is the callback of another
helper or kfunc, stays as it is: there the return value has another
meaning. The callback of bpf_user_ringbuf_drain() stays as it is too. The
helper takes a sample out of the buffer for each call, so a call that
does nothing loses the sample. Its loop ends after 128K samples.
This patch does not limit bpf_loop() itself. Only may_goto counts and reads
the clock. So nested bpf_loop() are not stopped when the prog has no
may_goto, or has none in the callbacks: bpf_loop(1 << 20, cb) with
bpf_loop(4096, cb2) in cb takes 5 sec. Nested open coded iterators have the
same problem. A follow up will fix these holes.
A call of such a callback executes 4 more insns. The cost on x86-64 (Zen 5)
in nsec:
before after
callback makes 1 iteration, inlined bpf_loop() 1.16 1.54
callback makes 1 iteration, bpf_loop() 6.52 6.74
callback without may_goto, inlined bpf_loop() 1.16 1.16
callback without may_goto, bpf_loop() 4.95 4.95
callback without may_goto, nested bpf_loop() 1.13 1.38
Signed-off-by: Alexei Starovoitov <ast@kernel.org>
Link: https://patch.msgid.link/20261031215543.682284-9-alexei.starovoitov@gmail.com
This commit is contained in:
parent
ec274bb466
commit
c16c6c4683
|
|
@ -969,6 +969,10 @@ struct bpf_subprog_info {
|
|||
bool is_cb: 1;
|
||||
bool is_async_cb: 1;
|
||||
bool is_exception_cb: 1;
|
||||
/* bpf_loop() or bpf_for_each_map_elem() call it in a loop */
|
||||
bool is_loop_cb: 1;
|
||||
/* BPF code, another helper or a kfunc call it */
|
||||
bool is_called: 1;
|
||||
bool args_cached: 1;
|
||||
/* true if the return value is passed in the R0:R2 register pair */
|
||||
bool ret_reg_pair: 1;
|
||||
|
|
|
|||
|
|
@ -2771,9 +2771,12 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
|
|||
int stack_slots = subprogs[i].stack_extra / 8;
|
||||
/* an invocation begins, see bpf_timed_may_goto_save() for async callbacks */
|
||||
bool main_prog = tmg && !i;
|
||||
/* only a helper calls it, in a loop that ends when it returns 1 */
|
||||
bool loop_cb = tmg && i && subprogs[i].is_loop_cb && !subprogs[i].is_called &&
|
||||
!subprogs[i].is_async_cb;
|
||||
int cnt = 0;
|
||||
|
||||
if (timed_may_goto ? !main_prog : !stack_slots)
|
||||
if (timed_may_goto ? !main_prog && !loop_cb : !stack_slots)
|
||||
continue;
|
||||
if (stack_slots > 1) {
|
||||
verifier_bug(env, "stack_slots supports may_goto only");
|
||||
|
|
@ -2793,10 +2796,23 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env)
|
|||
insn_buf[cnt++] = ld_tmg[0];
|
||||
insn_buf[cnt++] = ld_tmg[1];
|
||||
insn_buf[cnt++] = BPF_MOV64_PERCPU_REG(BPF_REG_0, BPF_REG_0);
|
||||
insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, count_off,
|
||||
BPF_MAX_TIMED_LOOPS);
|
||||
insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, timestamp_off, 0);
|
||||
insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, owner_off, 0);
|
||||
if (main_prog) {
|
||||
insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, count_off,
|
||||
BPF_MAX_TIMED_LOOPS);
|
||||
insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, timestamp_off, 0);
|
||||
insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_0, owner_off, 0);
|
||||
} else {
|
||||
/*
|
||||
* The time is up, but the helper would call the function again and
|
||||
* again. End its loop. The verifier has seen that: the loop can end
|
||||
* after each call.
|
||||
*/
|
||||
insn_buf[cnt++] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0,
|
||||
count_off);
|
||||
insn_buf[cnt++] = BPF_JMP_IMM(BPF_JNE, BPF_REG_0, 0, 2);
|
||||
insn_buf[cnt++] = BPF_MOV64_IMM(BPF_REG_0, 1);
|
||||
insn_buf[cnt++] = BPF_EXIT_INSN();
|
||||
}
|
||||
} else {
|
||||
/* Add ST insn to subprog prologue to init extra stack */
|
||||
insn_buf[cnt++] = BPF_ST_MEM(BPF_DW, BPF_REG_FP, -stack_depth,
|
||||
|
|
|
|||
|
|
@ -11508,6 +11508,12 @@ static int push_callback_call(struct bpf_verifier_env *env, struct bpf_insn *ins
|
|||
if (err)
|
||||
return err;
|
||||
|
||||
if (!bpf_pseudo_kfunc_call(insn) &&
|
||||
(insn->imm == BPF_FUNC_loop || insn->imm == BPF_FUNC_for_each_map_elem))
|
||||
env->subprog_info[subprog].is_loop_cb = true;
|
||||
else
|
||||
env->subprog_info[subprog].is_called = true;
|
||||
|
||||
/* for callback functions enqueue entry to callback and
|
||||
* proceed with next instruction within current frame.
|
||||
*/
|
||||
|
|
@ -11544,6 +11550,7 @@ static int check_static_func_call(struct bpf_verifier_env *env, int subprog,
|
|||
int err;
|
||||
|
||||
caller = state->frame[state->curframe];
|
||||
env->subprog_info[subprog].is_called = true;
|
||||
|
||||
/*
|
||||
* Track caller's total stack arg count (incoming + max outgoing).
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user