Re: [LTP] [PATCH 2/4] lib: Add .modprobe

Li Wang
2023-10-16T08:28:58Z

Hi Petr,

On Fri, Oct 13, 2023 at 9:50 PM Petr Vorel <pvorel@suse.cz> wrote:

>
> Hi Li,
>
> thanks for your input!
>
> > > Hi all,
>
> > > maybe .modprobe is too short name, but I'm not sure what would be
> better.
> > > Maybe .modprobe_module ?
>
>
> > .modprobe_module sounds better.
> +1
>
> > Also, I think that maybe we can support modprobe some
> > third-party modules (written by users) in test case, there are
> > a few managed by shell scripts, but if .modprobe_module
> > manages them unify in C, it would be nice for test variety.
>
> +1. Also I plan to move some of the LTP kernel modules - tests which use
> kernel
> modules from LTP (e.g. delete_module, init_module, ...)to KUnit or
> kselftest (to
> solve problem with signed modules required by distro kernels, kernel
> modules
> from LTP are then untestable on lockdown). But maybe these modules can
> stay in
> LTP and also be added to KUnit.
>

Yes, they can stay in LTP, because we do have '.skip_in_lockdown/secureboot'
to avoid the unsigned loading error.

> But these modules use tst_module_exists_() and SAFE_OPEN(). So you might
> mean
> 3rd party modules like nvidia or other proprietary modules, right?
>

Hmm, for sure we can't satisfy all situations, but at least there could be
a
separate way to use init/finit_module() without .'modprobe_module'.

But for tst_module_load/unload, I think they could be integrated within
.modprobe_module whatever loading by `insmod` or `modprobe`.

>
> Then, some of the tests in testcases/kernel/device-drivers/ might be
> obsolete or
> also be more suitable in kselftest or KUnit or elsewhere.
>
> ...
> > > > + if (tst_test->modprobe) {
> > > > + const char *name;
> > > > + int i;
> > > > +
> > > > + for (i = 0; (name = tst_test->modprobe[i]); ++i) {
> > > > + /* only load module if not already loaded */
> > > > + if (!module_loaded(name) &&
> > > tst_check_builtin_driver(name)) {
>
> > > Also we could make it independent on modules.builtin (NixOS based
> problem -
> > > missing these files). I.e. we would keep only module_loaded(), remove
> > > tst_check_builtin_driver(). But then we could not run rmmod / modprobe
> -r,
> > > or we would have to ignore it's exit code (rmmod on builtin module)
> fails.
>
>
> > Or we add one step to detect modules.builtin file, if no,
> > then just print a Warning at unload in fails?
>
> Unloading shouldn't be problem since it's in cleanup (thus TBROK => TWARN).
> But I'll test it.
>
> Or do you mean that on missing modules.builtin would test itself be
> working as
> module is presented (have warning on that modules.* files are missing and
> warning on rmmod?).
>

No, I didn't think so deeply:).

>
> Would you do it for both .modprobe_module and .needs_drivers? Or just one
> of them?
>

Maybe both.

--
Regards,
Li Wang