Re: [PATCH 2/2] moderr: add module error injection tool

[Alexei Starovoitov]

On Tue, Feb 4, 2025 at 1:30 PM Daniel Gomez <da.gomez@xxxxxxxxxx> wrote:
>
> On Wed, Jan 22, 2025 at 09:02:19AM +0100, Alexei Starovoitov wrote:
> > On Wed, Jan 22, 2025 at 5:12 AM Daniel Gomez <da.gomez@xxxxxxxxxxx> wrote:
> > >
> > > Add support for a module error injection tool. The tool
> > > can inject errors in the annotated module kernel functions
> > > such as complete_formation(), do_init_module() and
> > > module_enable_rodata_after_init(). Module name and module function are
> > > required parameters to have control over the error injection.
> > >
> > > Example: Inject error -22 to module_enable_rodata_ro_after_init for
> > > brd module:
> > >
> > > sudo moderr --modname=brd --modfunc=module_enable_rodata_ro_after_init \
> > > --error=-22 --trace
> > > Monitoring module error injection... Hit Ctrl-C to end.
> > > MODULE     ERROR FUNCTION
> > > brd        -22   module_enable_rodata_after_init()
> > >
> > > Kernel messages:
> > > [   89.463690] brd: module loaded
> > > [   89.463855] brd: module_enable_rodata_ro_after_init() returned -22,
> > > ro_after_init data might still be writable
> > >
> > > Signed-off-by: Daniel Gomez <da.gomez@xxxxxxxxxxx>
> > > ---
> > >  tools/bpf/Makefile            |  13 ++-
> > >  tools/bpf/moderr/.gitignore   |   2 +
> > >  tools/bpf/moderr/Makefile     |  95 +++++++++++++++++
> > >  tools/bpf/moderr/moderr.bpf.c | 127 +++++++++++++++++++++++
> > >  tools/bpf/moderr/moderr.c     | 236 ++++++++++++++++++++++++++++++++++++++++++
> > >  tools/bpf/moderr/moderr.h     |  40 +++++++
> > >  6 files changed, 510 insertions(+), 3 deletions(-)
> >
> > The tool looks useful, but we don't add tools to the kernel repo.
> > It has to stay out of tree.
>
> Can you clarify what do you mean? There are other tools under tools/ and tools/
> bpf [1].
>
> [1] https://lore.kernel.org/bpf/20200114184230.GA204154@krava/

As you noticed we added only one tool out of many and recently
discussed removing it, since the value of keeping it in the tree
is minimal.

> I will anyway move the tool to the suggested location in the other thread.

I don't think it's a good idea.
Keep it outside. There is no reason for it to live in the tree.