On Tue, Apr 26, 2022 at 12:52 AM Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> wrote: > > On Tue, Apr 26, 2022 at 6:11 AM Alexei Starovoitov > <alexei.starovoitov@xxxxxxxxx> wrote: > > > > On Thu, Apr 21, 2022 at 04:07:36PM +0200, Benjamin Tissoires wrote: > > > When using error-injection function through bpf to change the return > > > code, we need to know if the function is sleepable or not. > > > > > > Currently the code assumes that all error-inject functions are sleepable, > > > except for a few selected of them, hardcoded in kernel/bpf/verifier.c > > > > > > Add a new flag to error-inject so we can code that information where the > > > function is declared. > > > > > > Signed-off-by: Benjamin Tissoires <benjamin.tissoires@xxxxxxxxxx> > > > > > > --- > > > > > > new in v4: > > > - another approach would be to define a new kfunc_set, and register > > > it with btf. But in that case, what program type would we use? > > > BPF_PROG_TYPE_UNSPEC? > > > - also note that maybe we should consider all of the functions > > > non-sleepable and only mark some as sleepable. IMO it makes more > > > sense to be more restrictive by default. > > > > I think the approach in this patch is fine. > > We didn't have issues with check_non_sleepable_error_inject() so far, > > so I wouldn't start refactoring it. > > OK... though I can't help but thinking that adding a new > error-inject.h enum value is going to be bad, because it's an API > change, and users might not expect NS_ERRNO. Not sure about api concern. This is the kernel internal tag. bpf progs are not aware of them. The functions can change from sleepable to non-sleepable too. allow_error_inject can be removed. And so on. > OTOH, if we had a new kfunc_set, we keep the existing error-inject API > in place with all the variants and we just teach the verifier that the > function is non sleepable. ... > IIUC, the kfunc_set approach would solve that, no? Makes sense. Let's figure out an extensible kfunc_set approach that is not centralized in verifier.c
