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

Li Wang
2023-10-13T12:09:44Z

Hi Petr,

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

> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> doc/C-Test-API.asciidoc | 5 +++
> doc/Test-Writing-Guidelines.asciidoc | 1 +
> include/tst_test.h | 5 ++-
> lib/tst_test.c | 53 ++++++++++++++++++++++++++++
> 4 files changed, 63 insertions(+), 1 deletion(-)
>
> diff --git a/doc/C-Test-API.asciidoc b/doc/C-Test-API.asciidoc
> index dab811564..f2ba302e2 100644
> --- a/doc/C-Test-API.asciidoc
> +++ b/doc/C-Test-API.asciidoc
> @@ -1609,6 +1609,11 @@ first missing driver.
> The detection is based on reading 'modules.dep' and 'modules.builtin'
> files
> generated by kmod. The check is skipped on Android.
>
> +NULL terminated array '.modprobe' of kernel module names are tried to be
> loaded
> +with 'modprobe' unless they are builtin or already loaded. Test exits with
> +'TCONF' on first 'modprobe' non-zero exit. During cleanup are the modules
> +loaded by the test unloaded with 'rmmod'.
> +
> 1.27 Saving & restoring /proc|sys values
> ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> diff --git a/doc/Test-Writing-Guidelines.asciidoc
> b/doc/Test-Writing-Guidelines.asciidoc
> index 0db852ae6..19487816e 100644
> --- a/doc/Test-Writing-Guidelines.asciidoc
> +++ b/doc/Test-Writing-Guidelines.asciidoc
> @@ -371,6 +371,7 @@
> https://github.com/linux-test-project/ltp/wiki/Shell-Test-API[Shell Test
> API].
> | '.min_mem_avail' | not applicable
> | '.mnt_flags' | 'TST_MNT_PARAMS'
> | '.min_swap_avail' | not applicable
> +| '.modprobe' | –
> | '.mntpoint', '.mnt_data' | 'TST_MNTPOINT'
> | '.mount_device' | 'TST_MOUNT_DEVICE'
> | '.needs_cgroup_ctrls' | –
> diff --git a/include/tst_test.h b/include/tst_test.h
> index 75c2109b9..6b4fac985 100644
> --- a/include/tst_test.h
> +++ b/include/tst_test.h
> @@ -297,9 +297,12 @@ struct tst_test {
> /* NULL terminated array of resource file names */
> const char *const *resource_files;
>
> - /* NULL terminated array of needed kernel drivers */
> + /* NULL terminated array of needed kernel drivers to be checked */
> const char * const *needs_drivers;
>
> + /* NULL terminated array of needed kernel drivers to be loaded
> with modprobe */
> + const char * const *modprobe;
> +
> /*
> * {NULL, NULL} terminated array of (/proc, /sys) files to save
> * before setup and restore after cleanup
> diff --git a/lib/tst_test.c b/lib/tst_test.c
> index 087c62a16..ccbaa4c02 100644
> --- a/lib/tst_test.c
> +++ b/lib/tst_test.c
> @@ -49,6 +49,7 @@ const char *TCID __attribute__((weak));
> #define CVE_DB_URL "https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-"
>
> #define DEFAULT_TIMEOUT 30
> +#define MODULES_MAX_LEN 10
>
> struct tst_test *tst_test;
>
> @@ -83,6 +84,8 @@ const char *tst_ipc_path = ipc_path;
>
> static char shm_path[1024];
>
> +static int modules_loaded[MODULES_MAX_LEN];
> +
> int TST_ERR;
> int TST_PASS;
> long TST_RET;
> @@ -1135,6 +1138,29 @@ static void do_cgroup_requires(void)
> tst_cg_init();
> }
>
> +/*
> + * Search kernel driver in /proc/modules.
> + *
> + * @param driver The name of the driver.
> + * @return 1 if driver is found, otherwise 0.
> + */
> +static int module_loaded(const char *driver)
>

What about renaming it as tst_module_is_loaded() and move into tst_kernel.h?
I guess we could make use of it widely for checking module loading or not.

> +{
> + char line[4096];
> + int found = 0;
> + FILE *file = SAFE_FOPEN("/proc/modules", "r");
> +
> + while (fgets(line, sizeof(line), file)) {
> + if (strstr(line, driver)) {
> + found = 1;
> + break;
> + }
> + }
> + SAFE_FCLOSE(file);
> +
> + return found;
> +}
> +
> static void do_setup(int argc, char *argv[])
> {
> if (!tst_test)
> @@ -1194,6 +1220,20 @@ static void do_setup(int argc, char *argv[])
> safe_check_driver(name);
> }
>
> + 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)) {
> + const char *const cmd_modprobe[] =
> {"modprobe", name, NULL};
> + SAFE_CMD(cmd_modprobe, NULL, NULL);
>

And here print the name to tell people the module is loaded.

> + modules_loaded[i] = 1;
> + }
> + }
> + }
>

This part could be as a separate function like tst_load_module() and
built single into another lib. We prefer to keep the main tst_test.c
as a simple outline.

On the other hand, the extern functions can be used separately to let
modules to be loaded and unloaded during the test iteration.
It gives us more flexibility in test case design.

> +
> if (tst_test->mount_device)
> tst_test->format_device = 1;
>
> @@ -1362,6 +1402,19 @@ static void do_cleanup(void)
>
> tst_sys_conf_restore(0);
>
> + if (tst_test->modprobe) {
>

> + const char *name;
> + int i;
> +
> + for (i = 0; (name = tst_test->modprobe[i]); ++i) {
> + if (!modules_loaded[i])
> + continue;
> +
> + const char *const cmd_rmmod[] = {"rmmod", name,
> NULL};
> + SAFE_CMD(cmd_rmmod, NULL, NULL);
>

Print unload module name.

> + }
> + }
>

Here as well. something maybe like tst_unload_module().

+
> if (tst_test->restore_wallclock)
> tst_wallclock_restore();
>
> --
> 2.42.0
>
>
> --
> Mailing list info: https://lists.linux.it/listinfo/ltp
>

--
Regards,
Li Wang