On Sat, 15 Jul 2023 at 04:17, Alexei Starovoitov
<alexei.starovoitov@xxxxxxxxx> wrote:
>
> On Thu, Jul 13, 2023 at 08:02:30AM +0530, Kumar Kartikeya Dwivedi wrote:
> > By default, the subprog generated by the verifier to handle a thrown
> > exception hardcodes a return value of 0. To allow user-defined logic
> > and modification of the return value when an exception is thrown,
> > introduce the bpf_set_exception_callback kfunc, which installs a
> > callback as the default exception handler for the program.
> >
> > Compared to runtime kfuncs, this kfunc acts a built-in, i.e. it only
> > takes semantic effect during verification, and is erased from the
> > program at runtime.
> >
> > This kfunc can only be called once within a program, and always sets the
> > global exception handler, regardless of whether it was invoked in all
> > paths of the program or not. The kfunc is idempotent, and the default
> > exception callback cannot be modified at runtime.
> >
> > Allowing modification of the callback for the current program execution
> > at runtime leads to issues when the programs begin to nest, as any
> > per-CPU state maintaing this information will have to be saved and
> > restored. We don't want it to stay in bpf_prog_aux as this takes a
> > global effect for all programs. An alternative solution is spilling
> > the callback pointer at a known location on the program stack on entry,
> > and then passing this location to bpf_throw as a parameter.
> >
> > However, since exceptions are geared more towards a use case where they
> > are ideally never invoked, optimizing for this use case and adding to
> > the complexity has diminishing returns.
>
> Right. No run-time changes pls.
>
+1
> Instead of bpf_set_exception_callback() how about adding a
> btf_tag("exception_handler") or better name
> and check that such global func is a single func in a program and
> it's argument is a single u64.
>
That does seem better. Also, a conditional bpf_set_exception_callback
taking effect globally may be confusing for users.
I will switch to the BTF tag.
Any specific reason it has to be a global func and cannot have static linkage?
> > In the future, a variant of bpf_throw which allows supplying a callback
> > can also be introduced, to modify the callback for a certain throw
> > statement. For now, bpf_set_exception_callback is meant to serve as a
> > way to set statically set a subprog as the exception handler of a BPF
> > program.
> >
> > TODO: Should we change default behavior to just return whatever cookie
> > value was passed to bpf_throw? That might allow people to avoid
> > installing a callback in case they just want to manipulate the return
> > value.
>
> and the verifier would check that u64 matches allowed return values?
> ex: call check_return_code() on argument of bpf_throw?
> I guess that makes sense.
Ack. I'll make the change.
