[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAP01T74+naPau2_=1G2_TUSjY_ZCAWQ2XVBKxs9xSKHobZcEeQ@mail.gmail.com>
Date: Fri, 22 Mar 2024 17:50:11 +0100
From: Kumar Kartikeya Dwivedi <memxor@...il.com>
To: Benjamin Tissoires <bentiss@...nel.org>
Cc: Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>,
Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <martin.lau@...ux.dev>,
Eduard Zingerman <eddyz87@...il.com>, Song Liu <song@...nel.org>,
Yonghong Song <yonghong.song@...ux.dev>, John Fastabend <john.fastabend@...il.com>,
KP Singh <kpsingh@...nel.org>, Stanislav Fomichev <sdf@...gle.com>, Hao Luo <haoluo@...gle.com>,
Jiri Olsa <jolsa@...nel.org>, Mykola Lysenko <mykolal@...com>, Shuah Khan <shuah@...nel.org>,
bpf@...r.kernel.org, linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org
Subject: Re: [PATCH bpf-next v5 3/6] bpf/helpers: introduce
bpf_timer_set_sleepable_cb() kfunc
On Fri, 22 Mar 2024 at 15:57, Benjamin Tissoires <bentiss@...nel.org> 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 <eddyz87@...il.com>
> Signed-off-by: Benjamin Tissoires <bentiss@...nel.org>
>
> ---
> [...]
>
> @@ -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
>
>
Powered by blists - more mailing lists