Re: [LTP] [PATCH 1/1] keyctl30: Check module is presented before loading

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> > It's more subtle than this, in most cases kernel actually loads the
> > module automatically when the functionality is used (e.g. you create a
> > socket and kernel loads the module with the particular socket support).
> > The cases where the module needs to be loaded are quite rare. It's never
> > unloaded though, so maybe we should attempt that too (if it wasn't
> > loaded and was loaded because of the test).
> 
> kconfig is more than modules (e.g. module or even kernel core functionality
> enabled or disabled). I wonder if it's useful for users to know some module
> needs to be presented to be loaded (if not built-in) => bool vs. tristate
> kernel config. Maybe not.

We actually check and only do the module presence if the CONFIG_FOO=m,
if the code is build in the branch that checks for the module existence
never runs.

> Other thing is these few modules which don't support autoloading. (Mostly not
> bugs, at least the latest vmw_vsock_virtio_transport not supporting it was meant
> to be for testing/debugging [1].). Also the old .needs_drivers documented via
> metadata both cases - the modules needed to be loaded manually (e.g. "sctp" in
> testcases/cve/sctphantom.c) and autoloaded modules (e.g. "dummy" in
> testcases/cve/tcindex01.c or "tun" in testcases/kernel/uevents/uevent02.c or
> "quota_v2" in testcases/kernel/syscalls/quotactl/quotactl0*.c).

I do not see the problem here, autoloading is as simple as:

diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
index 46eeb8ec0..5d4e4d248 100644
--- a/lib/tst_kconfig.c
+++ b/lib/tst_kconfig.c
@@ -147,6 +147,7 @@ static void kconfig_runtime_check(struct tst_kconfig_var *var)
 static struct config_module_map {
        const char *config;
        const char *module_name;
+       int autoload;
 } config_module_maps[] = {
        {"CONFIG_KVM", "kvm"},
        {"CONFIG_ZRAM", "zram"},
@@ -165,8 +166,8 @@ static struct config_module_map {
        {"CONFIG_NET_SCH_INGRESS", "sch_ingress"},
        {"CONFIG_NET_CLS_MATCHALL", "cls_matchall"},
        {"CONFIG_NET_ACT_PEDIT", "act_pedit"},
-       {"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser"},
-       {"CONFIG_X509_CERTIFICATE_PARSER", "x509_key_parser"},
+       {"CONFIG_PKCS8_PRIVATE_KEY_PARSER", "pcks8_key_parser", 1},
+       {"CONFIG_X509_CERTIFICATE_PARSER", "x509_key_parser", 1},
        {}
 };

@@ -186,6 +187,11 @@ static void kconfig_module_check(struct tst_kconfig_var *var)
                        var->choice = 'n';
                        return;
                }
+
+               if (config_module_maps[i].autoload) {
+                       tst_res(TINFO, "Autoloading '%s'", config_module_maps[i].module_name);
+                       tst_modprobe(config_module_maps[i].module_name);
+               }
        }
 }

If we want that information to be present in metadata and documentation,
we can render that table from a (generated) json that could be parsed by
the documentation code as well.

I think it's better to have that information the the test library once,
rather than to keep a copy in each test.

> > As long as we have complete list of CONFIG_* variables from the tests,
> > which can be easily extracted from the test, we know which modules we
> > need mapping for. No need to keep anything that isn't in that set.
> 
> Again, not all kconfigs means tristate (module), some bool configs are required
> to make sure certain functionality is enabled. Unfortunately we need to check
> them one by one with kernel sources to see if they are modules (tristate) or bools.

There should be include/config/tristate.conf in the kernel sources once
make defconfig is done in linux kernel sources. The harder part is the
CONFIG_ to module name mapping, for that we would have to grep kernel
Makefiles.

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp