On Fri, 22 Mar 2024 at 15:57, Benjamin Tissoires <[email protected]> wrote: > > In this patch, bpf_timer_set_sleepable_cb() is functionally equivalent > to bpf_timer_set_callback(), to the exception that it enforces > the timer to be started with BPF_F_TIMER_SLEEPABLE. > > But given that bpf_timer_set_callback() is a helper when > bpf_timer_set_sleepable_cb() is a kfunc, we need to teach the verifier > about its attached callback. > Marking that callback as sleepable will be done in a separate patch > > Acked-by: Eduard Zingerman <[email protected]> > Signed-off-by: Benjamin Tissoires <[email protected]> > > --- > [...] > > @@ -19548,6 +19582,28 @@ static int fixup_kfunc_call(struct bpf_verifier_env > *env, struct bpf_insn *insn, > desc->func_id == special_kfunc_list[KF_bpf_rdonly_cast]) { > insn_buf[0] = BPF_MOV64_REG(BPF_REG_0, BPF_REG_1); > *cnt = 1; > + } else if (is_bpf_timer_set_sleepable_cb_impl_kfunc(desc->func_id)) { > + /* The verifier will process callback_fn as many times as > necessary > + * with different maps and the register states prepared by > + * set_timer_callback_state will be accurate. > + * > + * The following use case is valid: > + * map1 is shared by prog1, prog2, prog3. > + * prog1 calls bpf_timer_init for some map1 elements > + * prog2 calls bpf_timer_set_callback for some map1 > elements. > + * Those that were not bpf_timer_init-ed will return > -EINVAL. > + * prog3 calls bpf_timer_start for some map1 elements. > + * Those that were not both bpf_timer_init-ed and > + * bpf_timer_set_callback-ed will return -EINVAL. > + */ > + struct bpf_insn ld_addrs[2] = { > + BPF_LD_IMM64(BPF_REG_3, (long)env->prog->aux), > + }; > + > + insn_buf[0] = ld_addrs[0]; > + insn_buf[1] = ld_addrs[1]; > + insn_buf[2] = *insn; > + *cnt = 3; > }
Would be better to handle the fixup of all kfuncs in one place, i.e. fixup_kfunc_call. > return 0; > } > > -- > 2.44.0 > >
