Re: [PATCH RFC bpf-next v3 06/16] bpf/helpers: introduce bpf_timer_set_sleepable_cb() kfunc

On Wed, Feb 21, 2024 at 8:25 AM Benjamin Tissoires bentiss@kernel.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

Signed-off-by: Benjamin Tissoires bentiss@kernel.org

---

new in v3 (split from v2 02/10)

kernel/bpf/helpers.c | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- kernel/bpf/verifier.c | 31 +++++++++++++++++++++++++++++-- 2 files changed, 75 insertions(+), 4 deletions(-)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c index f9add0abe40a..2c6dc3d0ffff 100644 --- a/kernel/bpf/helpers.c +++ b/kernel/bpf/helpers.c @@ -1108,6 +1108,7 @@ struct bpf_hrtimer { void __rcu *callback_fn; void *value; struct semaphore sleepable_lock;

•   bool is_sleepable;
  

};

/* the actual struct hidden inside uapi struct bpf_timer */ @@ -1270,8 +1271,8 @@ static const struct bpf_func_proto bpf_timer_init_proto = { .arg3_type = ARG_ANYTHING, };

-BPF_CALL_3(bpf_timer_set_callback, struct bpf_timer_kern *, timer, void *, callback_fn,

•      struct bpf_prog_aux *, aux)
  

+static int __bpf_timer_set_callback(struct bpf_timer_kern *timer, void *callback_fn,

•                               struct bpf_prog_aux *aux, bool is_sleepable)
  

{ struct bpf_prog *prev, *prog = aux->prog; struct bpf_hrtimer *t; @@ -1311,12 +1312,19 @@ BPF_CALL_3(bpf_timer_set_callback, struct bpf_timer_kern *, timer, void *, callb t->prog = prog; } rcu_assign_pointer(t->callback_fn, callback_fn);

•   t->is_sleepable = is_sleepable;
    up(&t->sleepable_lock);
  

out: __bpf_spin_unlock_irqrestore(&timer->lock); return ret; }

+BPF_CALL_3(bpf_timer_set_callback, struct bpf_timer_kern *, timer, void *, callback_fn,

•      struct bpf_prog_aux *, aux)
  

+{

•   return __bpf_timer_set_callback(timer, callback_fn, aux, false);
  

+}

static const struct bpf_func_proto bpf_timer_set_callback_proto = { .func = bpf_timer_set_callback, .gpl_only = true, @@ -1342,6 +1350,11 @@ BPF_CALL_3(bpf_timer_start, struct bpf_timer_kern *, timer, u64, nsecs, u64, fla goto out; }

•   if (t->is_sleepable && !(flags & BPF_F_TIMER_SLEEPABLE)) {
  

•           ret = -EINVAL;
  

•           goto out;
  

•   }
  

•   if (flags & BPF_F_TIMER_ABS)
            mode = HRTIMER_MODE_ABS_SOFT;
    else
  

@@ -2606,6 +2619,36 @@ __bpf_kfunc void bpf_throw(u64 cookie) WARN(1, "A call to BPF exception callback should never return\n"); }

+/**

• • bpf_timer_set_sleepable_cb() - Configure the timer to call %callback_fn
• • static function in a sleepable context.
• • @timer: The bpf_timer that needs to be configured
• • @callback_fn: a static bpf function
• • @returns %0 on success. %-EINVAL if %timer was not initialized with
• • bpf_timer_init() earlier. %-EPERM if %timer is in a map that doesn't
• • have any user references.
• • The user space should either hold a file descriptor to a map with timers
• • or pin such map in bpffs. When map is unpinned or file descriptor is
• • closed all timers in the map will be cancelled and freed.
• • This kfunc is equivalent to %bpf_timer_set_callback except that it tells
• • the verifier that the target callback is run in a sleepable context.
• */

+__bpf_kfunc int bpf_timer_set_sleepable_cb(struct bpf_timer_kern *timer,

•                                      int (callback_fn)(void *map, int *key, struct bpf_timer *timer))
  

+{

•   struct bpf_throw_ctx ctx = {};
  

•   arch_bpf_stack_walk(bpf_stack_walker, &ctx);
  

•   WARN_ON_ONCE(!ctx.aux);
  

Sorry. Why such complexity? Please see how do_misc_fixups() handles BPF_FUNC_timer_set_callback.