Messages in this thread | | | Date | Fri, 2 Dec 2022 18:17:24 -0500 | From | Steven Rostedt <> | Subject | Re: [PATCH] error-injection: Add prompt for function error injection |
| |
On Fri, 2 Dec 2022 13:27:11 -0800 Alexei Starovoitov <alexei.starovoitov@gmail.com> wrote:
> --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -1874,8 +1874,12 @@ config NETDEV_NOTIFIER_ERROR_INJECT > If unsure, say N. > > config FUNCTION_ERROR_INJECTION
Why not just call this "ERROR_INJECTION" having this be FUNCTION and the one for functions be FUNC is confusing.
> + bool > + > +config FUNC_ERROR_INJECTION > bool "Fault-injections of functions" > depends on HAVE_FUNCTION_ERROR_INJECTION && KPROBES > + select FUNCTION_ERROR_INJECTION > help > Add fault injections into various functions that are annotated with > ALLOW_ERROR_INJECTION() in the kernel. BPF may also modify the return > @@ -1883,6 +1887,17 @@ config FUNCTION_ERROR_INJECTION > > If unsure, say N > > +config SYSCALL_ERROR_INJECTION > + bool "Error injections in syscalls" > + depends on HAVE_FUNCTION_ERROR_INJECTION && KPROBES > + select FUNCTION_ERROR_INJECTION > + default y
IIUC, Linus prefers everything to be "default n" unless there's a really good reason for it. Like only making other options available, but not doing anything to the kernel. I do have DYNAMIC_FTRACE as "default y" but that's only because it depends on CONFIG_FUNCTION_TRACER and nobody that enables that should have DYNAMIC_FTRACE off (except for academia).
> + help > + Allows error injection framework to return errors from syscalls. > + BPF may modify return values of syscalls as well.
And here's the thing. If BPF returns anything *but* an error, then this is a misnomer and incorrect. Name it something else like "HIJACK_SYSCALLS".
> + > + If unsure, say Y
And I'm curious, why Y if unsure?
-- Steve
> + > config FAULT_INJECTION > bool "Fault-injection framework" > depends on DEBUG_KERNEL > diff --git a/lib/error-inject.c b/lib/error-inject.c > index 1afca1b1cdea..9ba868eb8c43 100644 > --- a/lib/error-inject.c > +++ b/lib/error-inject.c > @@ -71,6 +71,10 @@ static void populate_error_injection_list(struct error_injection_entry *start, > > mutex_lock(&ei_mutex); > for (iter = start; iter < end; iter++) { > + if (iter->etype != EI_ETYPE_SYSCALL && > + !IS_ENABLED(CONFIG_FUNC_ERROR_INJECTION)) > + continue; > + > entry = (unsigned long)dereference_symbol_descriptor((void *)iter->addr); > > if (!kernel_text_address(entry) || > @@ -189,6 +193,8 @@ static const char *error_type_string(int etype) > return "ERRNO_NULL"; > case EI_ETYPE_TRUE: > return "TRUE"; > + case EI_ETYPE_SYSCALL: > + return "SYSCALL"; > default: > return "(unknown)"; > }
| |