[PATCH kmod] libkmod: add user soft dependecies

[PATCH kmod] libkmod: add user soft dependecies

[Jose Ignacio Tornos Martinez]

It has been seen that for some network mac drivers (i.e. lan78xx) the
related module for the phy is loaded dynamically depending on the current
hardware. In this case, the associated phy is read using mdio bus and then
the associated phy module is loaded during runtime (kernel function
phy_request_driver_module). However, no software dependency is defined, so
the user tools will no be able to get this dependency. For example, if
dracut is used and the hardware is present, lan78xx will be included but no
phy module will be added, and in the next restart the device will not work
from boot because no related phy will be found during initramfs stage.

In order to solve this, we could define a normal 'pre' software dependency
in lan78xx module with all the possible phy modules (there may be some),
but proceeding in that way, all the possible phy modules would be loaded
while only one is necessary.

The idea is to add a new attribute when the software dependency is defined,
apart from the normal ones 'pre' and 'post', I have called it 'user', to be
used only by the user tools that need to detect this situation. In that
way, for example, dracut could check the 'user' attribute of the modules in
order to install these software dependencies in initramfs too. That is, for
the  commented lan78xx module, defining the 'user' attribute to the
software dependency with the possible phy modules list, only the necessary
phy would be loaded on demand keeping the same behavior but all the
possible phy modules would be available from initramfs.

A new function 'kmod_module_get_user_softdeps' in libkmod will be added for
this to avoid breaking the API and maintain backward compatibility. This
general procedure could be useful for other similar cases (not only for
dynamic phy loading).

Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 libkmod/docs/libkmod-sections.txt |  1 +
 libkmod/libkmod-config.c          | 66 +++++++++++++++++++++++++++----
 libkmod/libkmod-internal.h        |  1 +
 libkmod/libkmod-module.c          | 50 +++++++++++++++++++++++
 libkmod/libkmod.h                 |  2 +
 libkmod/libkmod.sym               |  1 +
 6 files changed, 114 insertions(+), 7 deletions(-)

diff --git a/libkmod/docs/libkmod-sections.txt b/libkmod/docs/libkmod-sections.txt
index 33d9eec..04743e4 100644
--- a/libkmod/docs/libkmod-sections.txt
+++ b/libkmod/docs/libkmod-sections.txt
@@ -62,6 +62,7 @@ kmod_module_remove_module
 kmod_module_get_module
 kmod_module_get_dependencies
 kmod_module_get_softdeps
+kmod_module_get_user_softdeps
 kmod_module_apply_filter
 kmod_module_get_filtered_blacklist
 kmod_module_get_install_commands
diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c
index e83621b..c0e15be 100644
--- a/libkmod/libkmod-config.c
+++ b/libkmod/libkmod-config.c
@@ -54,8 +54,10 @@ struct kmod_softdep {
 	char *name;
 	const char **pre;
 	const char **post;
+	const char **user;
 	unsigned int n_pre;
 	unsigned int n_post;
+	unsigned int n_user;
 };
 
 const char *kmod_blacklist_get_modname(const struct kmod_list *l)
@@ -110,6 +112,12 @@ const char * const *kmod_softdep_get_post(const struct kmod_list *l, unsigned in
 	return dep->post;
 }
 
+const char * const *kmod_softdep_get_user(const struct kmod_list *l, unsigned int *count) {
+	const struct kmod_softdep *dep = l->data;
+	*count = dep->n_user;
+	return dep->user;
+}
+
 static int kmod_config_add_command(struct kmod_config *config,
 						const char *modname,
 						const char *command,
@@ -263,11 +271,11 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 	struct kmod_softdep *dep;
 	const char *s, *p;
 	char *itr;
-	unsigned int n_pre = 0, n_post = 0;
+	unsigned int n_pre = 0, n_post = 0, n_user = 0;
 	size_t modnamelen = strlen(modname) + 1;
 	size_t buflen = 0;
 	bool was_space = false;
-	enum { S_NONE, S_PRE, S_POST } mode = S_NONE;
+	enum { S_NONE, S_PRE, S_POST, S_USER } mode = S_NONE;
 
 	DBG(config->ctx, "modname=%s\n", modname);
 
@@ -298,6 +306,9 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 		else if (plen == sizeof("post:") - 1 &&
 				memcmp(p, "post:", sizeof("post:") - 1) == 0)
 			mode = S_POST;
+		else if (plen == sizeof("user:") - 1 &&
+				memcmp(p, "user:", sizeof("user:") - 1) == 0)
+			mode = S_USER;
 		else if (*s != '\0' || (*s == '\0' && !was_space)) {
 			if (mode == S_PRE) {
 				buflen += plen + 1;
@@ -305,6 +316,9 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 			} else if (mode == S_POST) {
 				buflen += plen + 1;
 				n_post++;
+			} else if (mode == S_USER) {
+				buflen += plen + 1;
+				n_user++;
 			}
 		}
 		p = s + 1;
@@ -312,11 +326,12 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 			break;
 	}
 
-	DBG(config->ctx, "%u pre, %u post\n", n_pre, n_post);
+	DBG(config->ctx, "%u pre, %u post, %u user\n", n_pre, n_post, n_user);
 
 	dep = malloc(sizeof(struct kmod_softdep) + modnamelen +
 		     n_pre * sizeof(const char *) +
 		     n_post * sizeof(const char *) +
+		     n_user * sizeof(const char *) +
 		     buflen);
 	if (dep == NULL) {
 		ERR(config->ctx, "out-of-memory modname=%s\n", modname);
@@ -324,9 +339,11 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 	}
 	dep->n_pre = n_pre;
 	dep->n_post = n_post;
+	dep->n_user = n_user;
 	dep->pre = (const char **)((char *)dep + sizeof(struct kmod_softdep));
 	dep->post = dep->pre + n_pre;
-	dep->name = (char *)(dep->post + n_post);
+	dep->user = dep->post + n_post;
+	dep->name = (char *)(dep->user + n_user);
 
 	memcpy(dep->name, modname, modnamelen);
 
@@ -334,6 +351,7 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 	itr = dep->name + modnamelen;
 	n_pre = 0;
 	n_post = 0;
+	n_user = 0;
 	mode = S_NONE;
 	was_space = false;
 	for (p = s = line; ; s++) {
@@ -362,6 +380,9 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 		else if (plen == sizeof("post:") - 1 &&
 				memcmp(p, "post:", sizeof("post:") - 1) == 0)
 			mode = S_POST;
+		else if (plen == sizeof("user:") - 1 &&
+				memcmp(p, "user:", sizeof("user:") - 1) == 0)
+			mode = S_USER;
 		else if (*s != '\0' || (*s == '\0' && !was_space)) {
 			if (mode == S_PRE) {
 				dep->pre[n_pre] = itr;
@@ -375,6 +396,12 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 				itr[plen] = '\0';
 				itr += plen + 1;
 				n_post++;
+			} else if (mode == S_USER) {
+				dep->user[n_user] = itr;
+				memcpy(itr, p, plen);
+				itr[plen] = '\0';
+				itr += plen + 1;
+				n_user++;
 			}
 		}
 		p = s + 1;
@@ -395,14 +422,15 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 static char *softdep_to_char(struct kmod_softdep *dep) {
 	const size_t sz_preprefix = sizeof("pre: ") - 1;
 	const size_t sz_postprefix = sizeof("post: ") - 1;
+	const size_t sz_userprefix = sizeof("user: ") - 1;
 	size_t sz = 1; /* at least '\0' */
-	size_t sz_pre, sz_post;
+	size_t sz_pre, sz_post, sz_user;
 	const char *start, *end;
 	char *s, *itr;
 
 	/*
-	 * Rely on the fact that dep->pre[] and dep->post[] are strv's that
-	 * point to a contiguous buffer
+	 * Rely on the fact that dep->pre[] dep->post[] and dep->user[]
+	 * are strv's that point to a contiguous buffer
 	 */
 	if (dep->n_pre > 0) {
 		start = dep->pre[0];
@@ -422,6 +450,15 @@ static char *softdep_to_char(struct kmod_softdep *dep) {
 	} else
 		sz_post = 0;
 
+	if (dep->n_user > 0) {
+		start = dep->user[0];
+		end = dep->user[dep->n_user - 1]
+					+ strlen(dep->user[dep->n_user - 1]);
+		sz_user = end - start;
+		sz += sz_user + sz_userprefix;
+	} else
+		sz_user = 0;
+
 	itr = s = malloc(sz);
 	if (s == NULL)
 		return NULL;
@@ -456,6 +493,21 @@ static char *softdep_to_char(struct kmod_softdep *dep) {
 		itr = p;
 	}
 
+	if (sz_user) {
+		char *p;
+
+		memcpy(itr, "user: ", sz_userprefix);
+		itr += sz_userprefix;
+
+		/* include last '\0' */
+		memcpy(itr, dep->user[0], sz_user + 1);
+		for (p = itr; p < itr + sz_user; p++) {
+			if (*p == '\0')
+				*p = ' ';
+		}
+		itr = p;
+	}
+
 	*itr = '\0';
 
 	return s;
diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index 26a7e28..8e4f112 100644
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -145,6 +145,7 @@ const char *kmod_command_get_modname(const struct kmod_list *l) __attribute__((n
 const char *kmod_softdep_get_name(const struct kmod_list *l) __attribute__((nonnull(1)));
 const char * const *kmod_softdep_get_pre(const struct kmod_list *l, unsigned int *count) __attribute__((nonnull(1, 2)));
 const char * const *kmod_softdep_get_post(const struct kmod_list *l, unsigned int *count);
+const char * const *kmod_softdep_get_user(const struct kmod_list *l, unsigned int *count);
 
 
 /* libkmod-module.c */
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index 585da41..dbe676c 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -1664,6 +1664,56 @@ KMOD_EXPORT int kmod_module_get_softdeps(const struct kmod_module *mod,
 	return 0;
 }
 
+/**
+ * kmod_module_get_user_softdeps:
+ * @mod: kmod module
+ * @user: where to save the list of user soft dependencies.
+ *
+ * Get user dependencies for this kmod module. Soft dependencies come
+ * from configuration file and are not cached in @mod because it may include
+ * dependency cycles that would make we leak kmod_module. Any call
+ * to this function will search for this module in configuration, allocate a
+ * list and return the result.
+ *
+ * @user is newly created list of kmod_module and
+ * should be unreferenced with kmod_module_unref_list().
+ *
+ * Returns: 0 on success or < 0 otherwise.
+ */
+KMOD_EXPORT int kmod_module_get_user_softdeps(const struct kmod_module *mod,
+						struct kmod_list **user)
+{
+	const struct kmod_list *l;
+	const struct kmod_config *config;
+
+	if (mod == NULL || user == NULL)
+		return -ENOENT;
+
+	assert(*user == NULL);
+
+	config = kmod_get_config(mod->ctx);
+
+	kmod_list_foreach(l, config->softdeps) {
+		const char *modname = kmod_softdep_get_name(l);
+		const char * const *array;
+		unsigned count;
+
+		if (fnmatch(modname, mod->name, 0) != 0)
+			continue;
+
+		array = kmod_softdep_get_user(l, &count);
+		*user = lookup_softdep(mod->ctx, array, count);
+
+		/*
+		 * find only the first command, as modprobe from
+		 * module-init-tools does
+		 */
+		break;
+	}
+
+	return 0;
+}
+
 /**
  * kmod_module_get_remove_commands:
  * @mod: kmod module
diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
index 7251aa7..ec6d270 100644
--- a/libkmod/libkmod.h
+++ b/libkmod/libkmod.h
@@ -196,6 +196,8 @@ const char *kmod_module_get_remove_commands(const struct kmod_module *mod);
 struct kmod_list *kmod_module_get_dependencies(const struct kmod_module *mod);
 int kmod_module_get_softdeps(const struct kmod_module *mod,
 				struct kmod_list **pre, struct kmod_list **post);
+int kmod_module_get_user_softdeps(const struct kmod_module *mod,
+					struct kmod_list **user);
 int kmod_module_get_filtered_blacklist(const struct kmod_ctx *ctx,
 					const struct kmod_list *input,
 					struct kmod_list **output) __attribute__ ((deprecated));
diff --git a/libkmod/libkmod.sym b/libkmod/libkmod.sym
index 0c04fda..26c3eef 100644
--- a/libkmod/libkmod.sym
+++ b/libkmod/libkmod.sym
@@ -42,6 +42,7 @@ global:
 
 	kmod_module_get_dependencies;
 	kmod_module_get_softdeps;
+	kmod_module_get_user_softdeps;
 	kmod_module_get_filtered_blacklist;
 
 	kmod_module_get_name;
-- 
2.44.0

Re: [PATCH kmod] libkmod: add user soft dependecies

[Lucas De Marchi]

On Mon, Mar 18, 2024 at 05:15:14PM +0100, Jose Ignacio Tornos Martinez wrote:
>It has been seen that for some network mac drivers (i.e. lan78xx) the
>related module for the phy is loaded dynamically depending on the current
>hardware. In this case, the associated phy is read using mdio bus and then
>the associated phy module is loaded during runtime (kernel function
>phy_request_driver_module). However, no software dependency is defined, so
>the user tools will no be able to get this dependency. For example, if
>dracut is used and the hardware is present, lan78xx will be included but no
>phy module will be added, and in the next restart the device will not work
>from boot because no related phy will be found during initramfs stage.
>
>In order to solve this, we could define a normal 'pre' software dependency
>in lan78xx module with all the possible phy modules (there may be some),
>but proceeding in that way, all the possible phy modules would be loaded
>while only one is necessary.
>
>The idea is to add a new attribute when the software dependency is defined,
>apart from the normal ones 'pre' and 'post', I have called it 'user', to be
>used only by the user tools that need to detect this situation. In that
>way, for example, dracut could check the 'user' attribute of the modules in
>order to install these software dependencies in initramfs too. That is, for
>the  commented lan78xx module, defining the 'user' attribute to the
>software dependency with the possible phy modules list, only the necessary
>phy would be loaded on demand keeping the same behavior but all the
>possible phy modules would be available from initramfs.
>
>A new function 'kmod_module_get_user_softdeps' in libkmod will be added for
>this to avoid breaking the API and maintain backward compatibility. This
>general procedure could be useful for other similar cases (not only for
>dynamic phy loading).

so it's basically a pre softdep, but without libkmod (userspace) trying
to load it before the module requested. So, it's "softer than soft" or
even "something before pre".

Thinking this way I find the name chosen odd, as the *user*space side of
module loading will actually *not* look into those deps.

Cc'ing some more people for suggestions, as I only know I don't like
"user", but my suggestions may considered equally bad too:

	dull / still / early / runtime_request / maybe

Anyway, we will need to explain exactly what this is about in
modprobe.d(5).  Other than the use case of creating a initramfs and not
missing any module, I don't think there would be any, right?


Lucas De Marchi

>
>Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
>---
> libkmod/docs/libkmod-sections.txt |  1 +
> libkmod/libkmod-config.c          | 66 +++++++++++++++++++++++++++----
> libkmod/libkmod-internal.h        |  1 +
> libkmod/libkmod-module.c          | 50 +++++++++++++++++++++++
> libkmod/libkmod.h                 |  2 +
> libkmod/libkmod.sym               |  1 +
> 6 files changed, 114 insertions(+), 7 deletions(-)
>
>diff --git a/libkmod/docs/libkmod-sections.txt b/libkmod/docs/libkmod-sections.txt
>index 33d9eec..04743e4 100644
>--- a/libkmod/docs/libkmod-sections.txt
>+++ b/libkmod/docs/libkmod-sections.txt
>@@ -62,6 +62,7 @@ kmod_module_remove_module
> kmod_module_get_module
> kmod_module_get_dependencies
> kmod_module_get_softdeps
>+kmod_module_get_user_softdeps
> kmod_module_apply_filter
> kmod_module_get_filtered_blacklist
> kmod_module_get_install_commands
>diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c
>index e83621b..c0e15be 100644
>--- a/libkmod/libkmod-config.c
>+++ b/libkmod/libkmod-config.c
>@@ -54,8 +54,10 @@ struct kmod_softdep {
> 	char *name;
> 	const char **pre;
> 	const char **post;
>+	const char **user;
> 	unsigned int n_pre;
> 	unsigned int n_post;
>+	unsigned int n_user;
> };
>
> const char *kmod_blacklist_get_modname(const struct kmod_list *l)
>@@ -110,6 +112,12 @@ const char * const *kmod_softdep_get_post(const struct kmod_list *l, unsigned in
> 	return dep->post;
> }
>
>+const char * const *kmod_softdep_get_user(const struct kmod_list *l, unsigned int *count) {
>+	const struct kmod_softdep *dep = l->data;
>+	*count = dep->n_user;
>+	return dep->user;
>+}
>+
> static int kmod_config_add_command(struct kmod_config *config,
> 						const char *modname,
> 						const char *command,
>@@ -263,11 +271,11 @@ static int kmod_config_add_softdep(struct kmod_config *config,
> 	struct kmod_softdep *dep;
> 	const char *s, *p;
> 	char *itr;
>-	unsigned int n_pre = 0, n_post = 0;
>+	unsigned int n_pre = 0, n_post = 0, n_user = 0;
> 	size_t modnamelen = strlen(modname) + 1;
> 	size_t buflen = 0;
> 	bool was_space = false;
>-	enum { S_NONE, S_PRE, S_POST } mode = S_NONE;
>+	enum { S_NONE, S_PRE, S_POST, S_USER } mode = S_NONE;
>
> 	DBG(config->ctx, "modname=%s\n", modname);
>
>@@ -298,6 +306,9 @@ static int kmod_config_add_softdep(struct kmod_config *config,
> 		else if (plen == sizeof("post:") - 1 &&
> 				memcmp(p, "post:", sizeof("post:") - 1) == 0)
> 			mode = S_POST;
>+		else if (plen == sizeof("user:") - 1 &&
>+				memcmp(p, "user:", sizeof("user:") - 1) == 0)
>+			mode = S_USER;
> 		else if (*s != '\0' || (*s == '\0' && !was_space)) {
> 			if (mode == S_PRE) {
> 				buflen += plen + 1;
>@@ -305,6 +316,9 @@ static int kmod_config_add_softdep(struct kmod_config *config,
> 			} else if (mode == S_POST) {
> 				buflen += plen + 1;
> 				n_post++;
>+			} else if (mode == S_USER) {
>+				buflen += plen + 1;
>+				n_user++;
> 			}
> 		}
> 		p = s + 1;
>@@ -312,11 +326,12 @@ static int kmod_config_add_softdep(struct kmod_config *config,
> 			break;
> 	}
>
>-	DBG(config->ctx, "%u pre, %u post\n", n_pre, n_post);
>+	DBG(config->ctx, "%u pre, %u post, %u user\n", n_pre, n_post, n_user);
>
> 	dep = malloc(sizeof(struct kmod_softdep) + modnamelen +
> 		     n_pre * sizeof(const char *) +
> 		     n_post * sizeof(const char *) +
>+		     n_user * sizeof(const char *) +
> 		     buflen);
> 	if (dep == NULL) {
> 		ERR(config->ctx, "out-of-memory modname=%s\n", modname);
>@@ -324,9 +339,11 @@ static int kmod_config_add_softdep(struct kmod_config *config,
> 	}
> 	dep->n_pre = n_pre;
> 	dep->n_post = n_post;
>+	dep->n_user = n_user;
> 	dep->pre = (const char **)((char *)dep + sizeof(struct kmod_softdep));
> 	dep->post = dep->pre + n_pre;
>-	dep->name = (char *)(dep->post + n_post);
>+	dep->user = dep->post + n_post;
>+	dep->name = (char *)(dep->user + n_user);
>
> 	memcpy(dep->name, modname, modnamelen);
>
>@@ -334,6 +351,7 @@ static int kmod_config_add_softdep(struct kmod_config *config,
> 	itr = dep->name + modnamelen;
> 	n_pre = 0;
> 	n_post = 0;
>+	n_user = 0;
> 	mode = S_NONE;
> 	was_space = false;
> 	for (p = s = line; ; s++) {
>@@ -362,6 +380,9 @@ static int kmod_config_add_softdep(struct kmod_config *config,
> 		else if (plen == sizeof("post:") - 1 &&
> 				memcmp(p, "post:", sizeof("post:") - 1) == 0)
> 			mode = S_POST;
>+		else if (plen == sizeof("user:") - 1 &&
>+				memcmp(p, "user:", sizeof("user:") - 1) == 0)
>+			mode = S_USER;
> 		else if (*s != '\0' || (*s == '\0' && !was_space)) {
> 			if (mode == S_PRE) {
> 				dep->pre[n_pre] = itr;
>@@ -375,6 +396,12 @@ static int kmod_config_add_softdep(struct kmod_config *config,
> 				itr[plen] = '\0';
> 				itr += plen + 1;
> 				n_post++;
>+			} else if (mode == S_USER) {
>+				dep->user[n_user] = itr;
>+				memcpy(itr, p, plen);
>+				itr[plen] = '\0';
>+				itr += plen + 1;
>+				n_user++;
> 			}
> 		}
> 		p = s + 1;
>@@ -395,14 +422,15 @@ static int kmod_config_add_softdep(struct kmod_config *config,
> static char *softdep_to_char(struct kmod_softdep *dep) {
> 	const size_t sz_preprefix = sizeof("pre: ") - 1;
> 	const size_t sz_postprefix = sizeof("post: ") - 1;
>+	const size_t sz_userprefix = sizeof("user: ") - 1;
> 	size_t sz = 1; /* at least '\0' */
>-	size_t sz_pre, sz_post;
>+	size_t sz_pre, sz_post, sz_user;
> 	const char *start, *end;
> 	char *s, *itr;
>
> 	/*
>-	 * Rely on the fact that dep->pre[] and dep->post[] are strv's that
>-	 * point to a contiguous buffer
>+	 * Rely on the fact that dep->pre[] dep->post[] and dep->user[]
>+	 * are strv's that point to a contiguous buffer
> 	 */
> 	if (dep->n_pre > 0) {
> 		start = dep->pre[0];
>@@ -422,6 +450,15 @@ static char *softdep_to_char(struct kmod_softdep *dep) {
> 	} else
> 		sz_post = 0;
>
>+	if (dep->n_user > 0) {
>+		start = dep->user[0];
>+		end = dep->user[dep->n_user - 1]
>+					+ strlen(dep->user[dep->n_user - 1]);
>+		sz_user = end - start;
>+		sz += sz_user + sz_userprefix;
>+	} else
>+		sz_user = 0;
>+
> 	itr = s = malloc(sz);
> 	if (s == NULL)
> 		return NULL;
>@@ -456,6 +493,21 @@ static char *softdep_to_char(struct kmod_softdep *dep) {
> 		itr = p;
> 	}
>
>+	if (sz_user) {
>+		char *p;
>+
>+		memcpy(itr, "user: ", sz_userprefix);
>+		itr += sz_userprefix;
>+
>+		/* include last '\0' */
>+		memcpy(itr, dep->user[0], sz_user + 1);
>+		for (p = itr; p < itr + sz_user; p++) {
>+			if (*p == '\0')
>+				*p = ' ';
>+		}
>+		itr = p;
>+	}
>+
> 	*itr = '\0';
>
> 	return s;
>diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
>index 26a7e28..8e4f112 100644
>--- a/libkmod/libkmod-internal.h
>+++ b/libkmod/libkmod-internal.h
>@@ -145,6 +145,7 @@ const char *kmod_command_get_modname(const struct kmod_list *l) __attribute__((n
> const char *kmod_softdep_get_name(const struct kmod_list *l) __attribute__((nonnull(1)));
> const char * const *kmod_softdep_get_pre(const struct kmod_list *l, unsigned int *count) __attribute__((nonnull(1, 2)));
> const char * const *kmod_softdep_get_post(const struct kmod_list *l, unsigned int *count);
>+const char * const *kmod_softdep_get_user(const struct kmod_list *l, unsigned int *count);
>
>
> /* libkmod-module.c */
>diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
>index 585da41..dbe676c 100644
>--- a/libkmod/libkmod-module.c
>+++ b/libkmod/libkmod-module.c
>@@ -1664,6 +1664,56 @@ KMOD_EXPORT int kmod_module_get_softdeps(const struct kmod_module *mod,
> 	return 0;
> }
>
>+/**
>+ * kmod_module_get_user_softdeps:
>+ * @mod: kmod module
>+ * @user: where to save the list of user soft dependencies.
>+ *
>+ * Get user dependencies for this kmod module. Soft dependencies come
>+ * from configuration file and are not cached in @mod because it may include
>+ * dependency cycles that would make we leak kmod_module. Any call
>+ * to this function will search for this module in configuration, allocate a
>+ * list and return the result.
>+ *
>+ * @user is newly created list of kmod_module and
>+ * should be unreferenced with kmod_module_unref_list().
>+ *
>+ * Returns: 0 on success or < 0 otherwise.
>+ */
>+KMOD_EXPORT int kmod_module_get_user_softdeps(const struct kmod_module *mod,
>+						struct kmod_list **user)
>+{
>+	const struct kmod_list *l;
>+	const struct kmod_config *config;
>+
>+	if (mod == NULL || user == NULL)
>+		return -ENOENT;
>+
>+	assert(*user == NULL);
>+
>+	config = kmod_get_config(mod->ctx);
>+
>+	kmod_list_foreach(l, config->softdeps) {
>+		const char *modname = kmod_softdep_get_name(l);
>+		const char * const *array;
>+		unsigned count;
>+
>+		if (fnmatch(modname, mod->name, 0) != 0)
>+			continue;
>+
>+		array = kmod_softdep_get_user(l, &count);
>+		*user = lookup_softdep(mod->ctx, array, count);
>+
>+		/*
>+		 * find only the first command, as modprobe from
>+		 * module-init-tools does
>+		 */
>+		break;
>+	}
>+
>+	return 0;
>+}
>+
> /**
>  * kmod_module_get_remove_commands:
>  * @mod: kmod module
>diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
>index 7251aa7..ec6d270 100644
>--- a/libkmod/libkmod.h
>+++ b/libkmod/libkmod.h
>@@ -196,6 +196,8 @@ const char *kmod_module_get_remove_commands(const struct kmod_module *mod);
> struct kmod_list *kmod_module_get_dependencies(const struct kmod_module *mod);
> int kmod_module_get_softdeps(const struct kmod_module *mod,
> 				struct kmod_list **pre, struct kmod_list **post);
>+int kmod_module_get_user_softdeps(const struct kmod_module *mod,
>+					struct kmod_list **user);
> int kmod_module_get_filtered_blacklist(const struct kmod_ctx *ctx,
> 					const struct kmod_list *input,
> 					struct kmod_list **output) __attribute__ ((deprecated));
>diff --git a/libkmod/libkmod.sym b/libkmod/libkmod.sym
>index 0c04fda..26c3eef 100644
>--- a/libkmod/libkmod.sym
>+++ b/libkmod/libkmod.sym
>@@ -42,6 +42,7 @@ global:
>
> 	kmod_module_get_dependencies;
> 	kmod_module_get_softdeps;
>+	kmod_module_get_user_softdeps;
> 	kmod_module_get_filtered_blacklist;
>
> 	kmod_module_get_name;
>-- 
>2.44.0
>

Re: [PATCH kmod] libkmod: add user soft dependecies

[Jose Ignacio Tornos Martinez]

> so it's basically a pre softdep, but without libkmod (userspace) trying
> to load it before the module requested. So, it's "softer than soft" or
> even "something before pre".
>
> Thinking this way I find the name chosen odd, as the *user*space side of
> module loading will actually *not* look into those deps.
>
> Cc'ing some more people for suggestions, as I only know I don't like
> "user", but my suggestions may considered equally bad too:
>
>        dull / still / early / runtime_request / maybe
Ok, I thought of "user" because it was only going to be used by user
applications but it could have other interpretations.
Maybe another idea: "internal" to inform that there are dependencies and
these are going to be solved internally?

> Anyway, we will need to explain exactly what this is about in
> modprobe.d(5).
Ok, I will complete it when the dependency name is decided.

> Other than the use case of creating a initramfs and not
> missing any module, I don't think there would be any, right?
Yes, my purpose is only that, I don't have detected any other case.

Thanks

Best regards
José Ignacio

Re: [PATCH kmod] libkmod: add user soft dependecies

[Lucas De Marchi]

On Wed, Mar 20, 2024 at 10:05:56AM +0100, Jose Ignacio Tornos Martinez wrote:
>> so it's basically a pre softdep, but without libkmod (userspace) trying
>> to load it before the module requested. So, it's "softer than soft" or
>> even "something before pre".
>>
>> Thinking this way I find the name chosen odd, as the *user*space side of
>> module loading will actually *not* look into those deps.
>>
>> Cc'ing some more people for suggestions, as I only know I don't like
>> "user", but my suggestions may considered equally bad too:
>>
>>        dull / still / early / runtime_request / maybe
>Ok, I thought of "user" because it was only going to be used by user
>applications but it could have other interpretations.
>Maybe another idea: "internal" to inform that there are dependencies and
>these are going to be solved internally?

a night of sleep and I had a dream in which libkmod had the concept of
"weak dependency". Borrowing the concept from weak symbols, I think it
makes perfect sense... the symbol is there and it may or may not be used
by the linker at the end, but the symbol needs to be there until the
linking phase. At least the parallel makes sense in my head :)

Also, I don't think we should mix them with softdep like is done here
after a quick scan through the patch.

 From man page:

        softdep modulename pre: modules... post: modules...
	   The softdep command allows you to specify soft, or optional,
	   module dependencies.  modulename can be used without these
	   optional modules installed, but usually with some features
	   missing. For example, a driver for a storage HBA might
	   require another module be loaded in order to use management
	   features.

	   pre-deps and post-deps modules are lists of names and/or
	   aliases of other modules that modprobe will attempt to
	   install (or remove) in order before and after the main module
	   given in the modulename argument.

	   Example: Assume "softdep c pre: a b post: d e" is provided in
	   the configuration. Running "modprobe c" is now equivalent to
	   "modprobe a b c d e" without the softdep. Flags such as
	   --use-blacklist are applied to all the specified modules,
	   while module parameters only apply to module c.

	   Note: if there are install or remove commands with the same
	   modulename argument, softdep takes precedence.

	weakdep modulename modules...
	   The weakdep command allows you to specify weak module
	   dependecies. Those are similar to pre softdep, with the
	   difference that userspace doesn't attempt to load that
	   dependency before the specified module. Instead the kernel
	   may request one or multiple of them during module probe,
	   depending on the hardware it's binding to. The purpose of
	   weak module is to allow a driver to specify that a certain
	   dependency may be needed, so it should be present in the
	   filesystem (e.g. in initramfs) when that module is probed.

	   Example: Assume "weakdep c a b". A program creating an
	   initramfs knows it should add a, b, and c to the filesystem
	   since a and b may be required/desired at runtime. When c is
	   loaded and is being probed, it may issue calls to
	   request_module() causing a or b to also be loaded.

Also instead of delegating this to the distros, it'd be good if we start
adding those to the ELF section of the modules with

	MODULE_WEAKDEP("...");

... to be defined in the kernel in a similar way that MODULE_SOFTDEP()
is.

>
>> Anyway, we will need to explain exactly what this is about in
>> modprobe.d(5).
>Ok, I will complete it when the dependency name is decided.
>
>> Other than the use case of creating a initramfs and not
>> missing any module, I don't think there would be any, right?
>Yes, my purpose is only that, I don't have detected any other case.


thanks
Lucas De Marchi

>
>Thanks
>
>Best regards
>José Ignacio
>

Re: [PATCH kmod] libkmod: add user soft dependecies

[Jose Ignacio Tornos Martinez]

> a night of sleep and I had a dream in which libkmod had the concept of
> "weak dependency". Borrowing the concept from weak symbols, I think it
> makes perfect sense... the symbol is there and it may or may not be used
> by the linker at the end, but the symbol needs to be there until the
> linking phase. At least the parallel makes sense in my head :)
Ok, I like your dream :)

> Also, I don't think we should mix them with softdep like is done here
> after a quick scan through the patch.
Ok, understood, and if a new case for softdep is not going to be used,
it is clearer: better not mixing with softdep processing.

> From man page:
>    softdep modulename pre: modules... post: modules...
>           The softdep command allows you to specify soft, or optional,
>           module dependencies.  modulename can be used without these
>           optional modules installed, but usually with some features
>           missing. For example, a driver for a storage HBA might
>           require another module be loaded in order to use management
>           features.
>
>           pre-deps and post-deps modules are lists of names and/or
>           aliases of other modules that modprobe will attempt to
>           install (or remove) in order before and after the main module
>           given in the modulename argument.
>
>           Example: Assume "softdep c pre: a b post: d e" is provided in
>           the configuration. Running "modprobe c" is now equivalent to
>           "modprobe a b c d e" without the softdep. Flags such as
>           --use-blacklist are applied to all the specified modules,
>           while module parameters only apply to module c.
>
>           Note: if there are install or remove commands with the same
>           modulename argument, softdep takes precedence.
>
>        weakdep modulename modules...
>           The weakdep command allows you to specify weak module
>           dependecies. Those are similar to pre softdep, with the
>           difference that userspace doesn't attempt to load that
>           dependency before the specified module. Instead the kernel
>           may request one or multiple of them during module probe,
>           depending on the hardware it's binding to. The purpose of
>           weak module is to allow a driver to specify that a certain
>           dependency may be needed, so it should be present in the
>           filesystem (e.g. in initramfs) when that module is probed.
>
>           Example: Assume "weakdep c a b". A program creating an
>           initramfs knows it should add a, b, and c to the filesystem
>           since a and b may be required/desired at runtime. When c is
>           loaded and is being probed, it may issue calls to
>           request_module() causing a or b to also be loaded.
Ok, thanks for completing this.
I will include this in my kmod patch (if it is ok for you).

> Also instead of delegating this to the distros, it'd be good if we start
> adding those to the ELF section of the modules with
>
>        MODULE_WEAKDEP("...");
>
> ... to be defined in the kernel in a similar way that MODULE_SOFTDEP()
> is.
Agree, better to define in kernel code, that's the reason for the patch.
Ok, I will implement in that way and I will create a kernel patch too for
this.
Indeed (with a different name), it was also in my mind but I didn't dare to
create something "new".

Thanks for you comments and help

Best regards
José Ignacio

[PATCH v2 patch] libkmod: add weak dependecies

[Jose Ignacio Tornos Martinez]

It has been seen that for some network mac drivers (i.e. lan78xx) the
related module for the phy is loaded dynamically depending on the current
hardware. In this case, the associated phy is read using mdio bus and then
the associated phy module is loaded during runtime (kernel function
phy_request_driver_module). However, no software dependency is defined, so
the user tools will no be able to get this dependency. For example, if
dracut is used and the hardware is present, lan78xx will be included but no
phy module will be added, and in the next restart the device will not work
from boot because no related phy will be found during initramfs stage.

In order to solve this, we could define a normal 'pre' software dependency
in lan78xx module with all the possible phy modules (there may be some),
but proceeding in that way, all the possible phy modules would be loaded
while only one is necessary.

The idea is to create a new type of dependency, that we are going to call
'weak' to be used only by the user tools that need to detect this situation.
In that way, for example, dracut could check the 'weak' dependency of the
modules involved in order to install these dependencies in initramfs too.
That is, for the commented lan78xx module, defining the 'weak' dependency
with the possible phy modules list, only the necessary phy would be loaded
on demand keeping the same behavior, but all the possible phy modules would
be available from initramfs.

A new function 'kmod_module_get_weakdeps' in libkmod will be added for
this to avoid breaking the API and maintain backward compatibility. This
general procedure could be useful for other similar cases (not only for
dynamic phy loading).

Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
V1 -> V2:
- Better to create a new type of dependency 'weak' as Lucas De Marchi
suggests.
- Complete modprobe.d(5) with the comments from Lucas De Marchi.

 libkmod/docs/libkmod-sections.txt |   2 +
 libkmod/libkmod-config.c          | 212 ++++++++++++++++++++++++++++++
 libkmod/libkmod-internal.h        |   3 +
 libkmod/libkmod-module.c          |  58 +++++++-
 libkmod/libkmod.h                 |   3 +
 libkmod/libkmod.sym               |   2 +
 man/modprobe.d.5.xml              |  24 ++++
 tools/depmod.c                    |  25 ++++
 tools/modprobe.c                  |   1 +
 9 files changed, 326 insertions(+), 4 deletions(-)

diff --git a/libkmod/docs/libkmod-sections.txt b/libkmod/docs/libkmod-sections.txt
index 33d9eec..978b064 100644
--- a/libkmod/docs/libkmod-sections.txt
+++ b/libkmod/docs/libkmod-sections.txt
@@ -37,6 +37,7 @@ kmod_config_get_remove_commands
 kmod_config_get_aliases
 kmod_config_get_options
 kmod_config_get_softdeps
+kmod_config_get_weakdeps
 kmod_config_iter_get_key
 kmod_config_iter_get_value
 kmod_config_iter_next
@@ -62,6 +63,7 @@ kmod_module_remove_module
 kmod_module_get_module
 kmod_module_get_dependencies
 kmod_module_get_softdeps
+kmod_module_get_weakdeps
 kmod_module_apply_filter
 kmod_module_get_filtered_blacklist
 kmod_module_get_install_commands
diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c
index e83621b..a571b6b 100644
--- a/libkmod/libkmod-config.c
+++ b/libkmod/libkmod-config.c
@@ -58,6 +58,12 @@ struct kmod_softdep {
 	unsigned int n_post;
 };
 
+struct kmod_weakdep {
+	char *name;
+	const char **weak;
+	unsigned int n_weak;
+};
+
 const char *kmod_blacklist_get_modname(const struct kmod_list *l)
 {
 	return l->data;
@@ -110,6 +116,16 @@ const char * const *kmod_softdep_get_post(const struct kmod_list *l, unsigned in
 	return dep->post;
 }
 
+const char *kmod_weakdep_get_name(const struct kmod_list *l) {
+	const struct kmod_weakdep *dep = l->data;
+	return dep->name;
+}
+
+const char * const *kmod_weakdep_get_weak(const struct kmod_list *l, unsigned int *count) {
+	const struct kmod_weakdep *dep = l->data;
+	*count = dep->n_weak;
+	return dep->weak;
+}
 static int kmod_config_add_command(struct kmod_config *config,
 						const char *modname,
 						const char *command,
@@ -392,6 +408,112 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 	return 0;
 }
 
+static int kmod_config_add_weakdep(struct kmod_config *config,
+							const char *modname,
+							const char *line)
+{
+	struct kmod_list *list;
+	struct kmod_weakdep *dep;
+	const char *s, *p;
+	char *itr;
+	unsigned int n_weak = 0;
+	size_t modnamelen = strlen(modname) + 1;
+	size_t buflen = 0;
+	bool was_space = false;
+
+	DBG(config->ctx, "modname=%s\n", modname);
+
+	/* analyze and count */
+	for (p = s = line; ; s++) {
+		size_t plen;
+
+		if (*s != '\0') {
+			if (!isspace(*s)) {
+				was_space = false;
+				continue;
+			}
+
+			if (was_space) {
+				p = s + 1;
+				continue;
+			}
+			was_space = true;
+
+			if (p >= s)
+				continue;
+		}
+		plen = s - p;
+
+		if (*s != '\0' || (*s == '\0' && !was_space)) {
+			buflen += plen + 1;
+			n_weak++;
+		}
+		p = s + 1;
+		if (*s == '\0')
+			break;
+	}
+
+	DBG(config->ctx, "%u weak\n", n_weak);
+
+	dep = malloc(sizeof(struct kmod_weakdep) + modnamelen +
+		     n_weak * sizeof(const char *) +
+		     buflen);
+	if (dep == NULL) {
+		ERR(config->ctx, "out-of-memory modname=%s\n", modname);
+		return -ENOMEM;
+	}
+	dep->n_weak = n_weak;
+	dep->weak = (const char **)((char *)dep + sizeof(struct kmod_weakdep));
+	dep->name = (char *)(dep->weak + n_weak);
+
+	memcpy(dep->name, modname, modnamelen);
+
+	/* copy strings */
+	itr = dep->name + modnamelen;
+	n_weak = 0;
+	was_space = false;
+	for (p = s = line; ; s++) {
+		size_t plen;
+
+		if (*s != '\0') {
+			if (!isspace(*s)) {
+				was_space = false;
+				continue;
+			}
+
+			if (was_space) {
+				p = s + 1;
+				continue;
+			}
+			was_space = true;
+
+			if (p >= s)
+				continue;
+		}
+		plen = s - p;
+
+		if (*s != '\0' || (*s == '\0' && !was_space)) {
+			dep->weak[n_weak] = itr;
+			memcpy(itr, p, plen);
+			itr[plen] = '\0';
+			itr += plen + 1;
+			n_weak++;
+		}
+		p = s + 1;
+		if (*s == '\0')
+			break;
+	}
+
+	list = kmod_list_append(config->weakdeps, dep);
+	if (list == NULL) {
+		free(dep);
+		return -ENOMEM;
+	}
+	config->weakdeps = list;
+
+	return 0;
+}
+
 static char *softdep_to_char(struct kmod_softdep *dep) {
 	const size_t sz_preprefix = sizeof("pre: ") - 1;
 	const size_t sz_postprefix = sizeof("post: ") - 1;
@@ -461,6 +583,44 @@ static char *softdep_to_char(struct kmod_softdep *dep) {
 	return s;
 }
 
+static char *weakdep_to_char(struct kmod_weakdep *dep) {
+	size_t sz;
+	const char *start, *end;
+	char *s, *itr;
+
+	/*
+	 * Rely on the fact that dep->weak[] and are strv's that point to a
+	 * contiguous buffer
+	 */
+	if (dep->n_weak > 0) {
+		start = dep->weak[0];
+		end = dep->weak[dep->n_weak - 1]
+					+ strlen(dep->weak[dep->n_weak - 1]);
+		sz = end - start;
+	} else
+		sz = 0;
+
+	itr = s = malloc(sz);
+	if (s == NULL)
+		return NULL;
+
+	if (sz) {
+		char *p;
+
+		/* include last '\0' */
+		memcpy(itr, dep->weak[0], sz + 1);
+		for (p = itr; p < itr + sz; p++) {
+			if (*p == '\0')
+				*p = ' ';
+		}
+		itr = p;
+	}
+
+	*itr = '\0';
+
+	return s;
+}
+
 static void kmod_config_free_softdep(struct kmod_config *config,
 							struct kmod_list *l)
 {
@@ -468,6 +628,13 @@ static void kmod_config_free_softdep(struct kmod_config *config,
 	config->softdeps = kmod_list_remove(l);
 }
 
+static void kmod_config_free_weakdep(struct kmod_config *config,
+							struct kmod_list *l)
+{
+	free(l->data);
+	config->weakdeps = kmod_list_remove(l);
+}
+
 static void kcmdline_parse_result(struct kmod_config *config, char *modname,
 						char *param, char *value)
 {
@@ -703,6 +870,14 @@ static int kmod_config_parse(struct kmod_config *config, int fd,
 				goto syntax_error;
 
 			kmod_config_add_softdep(config, modname, softdeps);
+		} else if (streq(cmd, "weakdep")) {
+			char *modname = strtok_r(NULL, "\t ", &saveptr);
+			char *weakdeps = strtok_r(NULL, "\0", &saveptr);
+
+			if (underscores(modname) < 0 || weakdeps == NULL)
+				goto syntax_error;
+
+			kmod_config_add_weakdep(config, modname, weakdeps);
 		} else if (streq(cmd, "include")
 				|| streq(cmd, "config")) {
 			ERR(ctx, "%s: command %s is deprecated and not parsed anymore\n",
@@ -746,6 +921,9 @@ void kmod_config_free(struct kmod_config *config)
 	while (config->softdeps)
 		kmod_config_free_softdep(config, config->softdeps);
 
+	while (config->weakdeps)
+		kmod_config_free_weakdep(config, config->weakdeps);
+
 	for (; config->paths != NULL;
 				config->paths = kmod_list_remove(config->paths))
 		free(config->paths->data);
@@ -889,6 +1067,7 @@ int kmod_config_new(struct kmod_ctx *ctx, struct kmod_config **p_config,
 	size_t i;
 
 	conf_files_insert_sorted(ctx, &list, kmod_get_dirname(ctx), "modules.softdep");
+	conf_files_insert_sorted(ctx, &list, kmod_get_dirname(ctx), "modules.weakdep");
 
 	for (i = 0; config_paths[i] != NULL; i++) {
 		const char *path = config_paths[i];
@@ -973,6 +1152,7 @@ enum config_type {
 	CONFIG_TYPE_ALIAS,
 	CONFIG_TYPE_OPTION,
 	CONFIG_TYPE_SOFTDEP,
+	CONFIG_TYPE_WEAKDEP,
 };
 
 struct kmod_config_iter {
@@ -991,6 +1171,12 @@ static const char *softdep_get_plain_softdep(const struct kmod_list *l)
 	return s;
 }
 
+static const char *weakdep_get_plain_weakdep(const struct kmod_list *l)
+{
+	char *s = weakdep_to_char(l->data);
+	return s;
+}
+
 static struct kmod_config_iter *kmod_config_iter_new(const struct kmod_ctx* ctx,
 							enum config_type type)
 {
@@ -1033,6 +1219,12 @@ static struct kmod_config_iter *kmod_config_iter_new(const struct kmod_ctx* ctx,
 		iter->get_value = softdep_get_plain_softdep;
 		iter->intermediate = true;
 		break;
+	case CONFIG_TYPE_WEAKDEP:
+		iter->list = config->weakdeps;
+		iter->get_key = kmod_weakdep_get_name;
+		iter->get_value = weakdep_get_plain_weakdep;
+		iter->intermediate = true;
+		break;
 	}
 
 	return iter;
@@ -1163,6 +1355,26 @@ KMOD_EXPORT struct kmod_config_iter *kmod_config_get_softdeps(const struct kmod_
 	return kmod_config_iter_new(ctx, CONFIG_TYPE_SOFTDEP);
 }
 
+/**
+ * kmod_config_get_weakdeps:
+ * @ctx: kmod library context
+ *
+ * Retrieve an iterator to deal with the weakdeps maintained inside the
+ * library. See kmod_config_iter_get_key(), kmod_config_iter_get_value() and
+ * kmod_config_iter_next(). At least one call to kmod_config_iter_next() must
+ * be made to initialize the iterator and check if it's valid.
+ *
+ * Returns: a new iterator over the weakdeps or NULL on failure. Free it with
+ * kmod_config_iter_free_iter().
+ */
+KMOD_EXPORT struct kmod_config_iter *kmod_config_get_weakdeps(const struct kmod_ctx *ctx)
+{
+	if (ctx == NULL)
+		return NULL;;
+
+	return kmod_config_iter_new(ctx, CONFIG_TYPE_WEAKDEP);
+}
+
 /**
  * kmod_config_iter_get_key:
  * @iter: iterator over a certain configuration
diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index 26a7e28..52ddbe6 100644
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -128,6 +128,7 @@ struct kmod_config {
 	struct kmod_list *remove_commands;
 	struct kmod_list *install_commands;
 	struct kmod_list *softdeps;
+	struct kmod_list *weakdeps;
 
 	struct kmod_list *paths;
 };
@@ -146,6 +147,8 @@ const char *kmod_softdep_get_name(const struct kmod_list *l) __attribute__((nonn
 const char * const *kmod_softdep_get_pre(const struct kmod_list *l, unsigned int *count) __attribute__((nonnull(1, 2)));
 const char * const *kmod_softdep_get_post(const struct kmod_list *l, unsigned int *count);
 
+const char *kmod_weakdep_get_name(const struct kmod_list *l) __attribute__((nonnull(1)));
+const char * const *kmod_weakdep_get_weak(const struct kmod_list *l, unsigned int *count) __attribute__((nonnull(1, 2)));
 
 /* libkmod-module.c */
 int kmod_module_new_from_alias(struct kmod_ctx *ctx, const char *alias, const char *name, struct kmod_module **mod);
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index 585da41..4f23ffb 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -1589,7 +1589,7 @@ void kmod_module_set_install_commands(struct kmod_module *mod, const char *cmd)
 	mod->install_commands = cmd;
 }
 
-static struct kmod_list *lookup_softdep(struct kmod_ctx *ctx, const char * const * array, unsigned int count)
+static struct kmod_list *lookup_dep(struct kmod_ctx *ctx, const char * const * array, unsigned int count)
 {
 	struct kmod_list *ret = NULL;
 	unsigned i;
@@ -1601,7 +1601,7 @@ static struct kmod_list *lookup_softdep(struct kmod_ctx *ctx, const char * const
 
 		err = kmod_module_new_from_lookup(ctx, depname, &lst);
 		if (err < 0) {
-			ERR(ctx, "failed to lookup soft dependency '%s', continuing anyway.\n", depname);
+			ERR(ctx, "failed to lookup dependency '%s', continuing anyway.\n", depname);
 			continue;
 		} else if (lst != NULL)
 			ret = kmod_list_append_list(ret, lst);
@@ -1650,9 +1650,59 @@ KMOD_EXPORT int kmod_module_get_softdeps(const struct kmod_module *mod,
 			continue;
 
 		array = kmod_softdep_get_pre(l, &count);
-		*pre = lookup_softdep(mod->ctx, array, count);
+		*pre = lookup_dep(mod->ctx, array, count);
 		array = kmod_softdep_get_post(l, &count);
-		*post = lookup_softdep(mod->ctx, array, count);
+		*post = lookup_dep(mod->ctx, array, count);
+
+		/*
+		 * find only the first command, as modprobe from
+		 * module-init-tools does
+		 */
+		break;
+	}
+
+	return 0;
+}
+
+/*
+ * kmod_module_get_weakdeps:
+ * @mod: kmod module
+ * @weak: where to save the list of weak dependencies.
+ *
+ * Get weak dependencies for this kmod module. Weak dependencies come
+ * from configuration file and are not cached in @mod because it may include
+ * dependency cycles that would make we leak kmod_module. Any call
+ * to this function will search for this module in configuration, allocate a
+ * list and return the result.
+ *
+ * @weak is newly created list of kmod_module and
+ * should be unreferenced with kmod_module_unref_list().
+ *
+ * Returns: 0 on success or < 0 otherwise.
+ */
+KMOD_EXPORT int kmod_module_get_weakdeps(const struct kmod_module *mod,
+						struct kmod_list **weak)
+{
+	const struct kmod_list *l;
+	const struct kmod_config *config;
+
+	if (mod == NULL || weak == NULL)
+		return -ENOENT;
+
+	assert(*weak == NULL);
+
+	config = kmod_get_config(mod->ctx);
+
+	kmod_list_foreach(l, config->weakdeps) {
+		const char *modname = kmod_weakdep_get_name(l);
+		const char * const *array;
+		unsigned count;
+
+		if (fnmatch(modname, mod->name, 0) != 0)
+			continue;
+
+		array = kmod_weakdep_get_weak(l, &count);
+		*weak = lookup_dep(mod->ctx, array, count);
 
 		/*
 		 * find only the first command, as modprobe from
diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
index 7251aa7..fce66d1 100644
--- a/libkmod/libkmod.h
+++ b/libkmod/libkmod.h
@@ -112,6 +112,7 @@ struct kmod_config_iter *kmod_config_get_remove_commands(const struct kmod_ctx *
 struct kmod_config_iter *kmod_config_get_aliases(const struct kmod_ctx *ctx);
 struct kmod_config_iter *kmod_config_get_options(const struct kmod_ctx *ctx);
 struct kmod_config_iter *kmod_config_get_softdeps(const struct kmod_ctx *ctx);
+struct kmod_config_iter *kmod_config_get_weakdeps(const struct kmod_ctx *ctx);
 const char *kmod_config_iter_get_key(const struct kmod_config_iter *iter);
 const char *kmod_config_iter_get_value(const struct kmod_config_iter *iter);
 bool kmod_config_iter_next(struct kmod_config_iter *iter);
@@ -196,6 +197,8 @@ const char *kmod_module_get_remove_commands(const struct kmod_module *mod);
 struct kmod_list *kmod_module_get_dependencies(const struct kmod_module *mod);
 int kmod_module_get_softdeps(const struct kmod_module *mod,
 				struct kmod_list **pre, struct kmod_list **post);
+int kmod_module_get_weakdeps(const struct kmod_module *mod,
+				struct kmod_list **weak);
 int kmod_module_get_filtered_blacklist(const struct kmod_ctx *ctx,
 					const struct kmod_list *input,
 					struct kmod_list **output) __attribute__ ((deprecated));
diff --git a/libkmod/libkmod.sym b/libkmod/libkmod.sym
index 0c04fda..0d6d338 100644
--- a/libkmod/libkmod.sym
+++ b/libkmod/libkmod.sym
@@ -21,6 +21,7 @@ global:
 	kmod_config_get_aliases;
 	kmod_config_get_options;
 	kmod_config_get_softdeps;
+	kmod_config_get_weakdeps;
 	kmod_config_iter_get_key;
 	kmod_config_iter_get_value;
 	kmod_config_iter_next;
@@ -42,6 +43,7 @@ global:
 
 	kmod_module_get_dependencies;
 	kmod_module_get_softdeps;
+	kmod_module_get_weakdeps;
 	kmod_module_get_filtered_blacklist;
 
 	kmod_module_get_name;
diff --git a/man/modprobe.d.5.xml b/man/modprobe.d.5.xml
index 2bf6537..cc90da6 100644
--- a/man/modprobe.d.5.xml
+++ b/man/modprobe.d.5.xml
@@ -212,6 +212,30 @@
           </para>
         </listitem>
       </varlistentry>
+      <varlistentry>
+        <term>weakdep <replaceable>modulename</replaceable> <replaceable>modules...</replaceable>
+        </term>
+        <listitem>
+          <para>
+            The <command>weakdep</command> command allows you to specify weak module
+            dependencies. Those are similar to pre softdep, with the
+            difference that userspace doesn't attempt to load that
+            dependency before the specified module. Instead the kernel
+            may request one or multiple of them during module probe,
+            depending on the hardware it's binding to. The purpose of
+            weak module is to allow a driver to specify that a certain
+            dependency may be needed, so it should be present in the
+            filesystem (e.g. in initramfs) when that module is probed.
+          </para>
+	  <para>
+            Example: Assume "weakdep c a b". A program creating an
+            initramfs knows it should add a, b, and c to the filesystem
+            since a and b may be required/desired at runtime. When c is
+            loaded and is being probed, it may issue calls to
+            request_module() causing a or b to also be loaded.
+          </para>
+        </listitem>
+      </varlistentry>
     </variablelist>
   </refsect1>
   <refsect1><title>COMPATIBILITY</title>
diff --git a/tools/depmod.c b/tools/depmod.c
index 43fc354..06618fa 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -2296,6 +2296,30 @@ static int output_softdeps(struct depmod *depmod, FILE *out)
 	return 0;
 }
 
+static int output_weakdeps(struct depmod *depmod, FILE *out)
+{
+	size_t i;
+
+	fputs("# Weak dependencies extracted from modules themselves.\n", out);
+
+	for (i = 0; i < depmod->modules.count; i++) {
+		const struct mod *mod = depmod->modules.array[i];
+		struct kmod_list *l;
+
+		kmod_list_foreach(l, mod->info_list) {
+			const char *key = kmod_module_info_get_key(l);
+			const char *value = kmod_module_info_get_value(l);
+
+			if (!streq(key, "weakdep"))
+				continue;
+
+			fprintf(out, "weakdep %s %s\n", mod->modname, value);
+		}
+	}
+
+	return 0;
+}
+
 static int output_symbols(struct depmod *depmod, FILE *out)
 {
 	struct hash_iter iter;
@@ -2574,6 +2598,7 @@ static int depmod_output(struct depmod *depmod, FILE *out)
 		{ "modules.alias", output_aliases },
 		{ "modules.alias.bin", output_aliases_bin },
 		{ "modules.softdep", output_softdeps },
+		{ "modules.weakdep", output_weakdeps },
 		{ "modules.symbols", output_symbols },
 		{ "modules.symbols.bin", output_symbols_bin },
 		{ "modules.builtin.bin", output_builtin_bin },
diff --git a/tools/modprobe.c b/tools/modprobe.c
index 5306bef..4328da6 100644
--- a/tools/modprobe.c
+++ b/tools/modprobe.c
@@ -182,6 +182,7 @@ static int show_config(struct kmod_ctx *ctx)
 		{ "alias", kmod_config_get_aliases },
 		{ "options", kmod_config_get_options },
 		{ "softdep", kmod_config_get_softdeps },
+		{ "weakdep", kmod_config_get_weakdeps },
 	};
 	size_t i;
 
-- 
2.44.0

[PATCH v2 kmod] libkmod: add weak dependecies

[Jose Ignacio Tornos Martinez]

It has been seen that for some network mac drivers (i.e. lan78xx) the
related module for the phy is loaded dynamically depending on the current
hardware. In this case, the associated phy is read using mdio bus and then
the associated phy module is loaded during runtime (kernel function
phy_request_driver_module). However, no software dependency is defined, so
the user tools will no be able to get this dependency. For example, if
dracut is used and the hardware is present, lan78xx will be included but no
phy module will be added, and in the next restart the device will not work
from boot because no related phy will be found during initramfs stage.

In order to solve this, we could define a normal 'pre' software dependency
in lan78xx module with all the possible phy modules (there may be some),
but proceeding in that way, all the possible phy modules would be loaded
while only one is necessary.

The idea is to create a new type of dependency, that we are going to call
'weak' to be used only by the user tools that need to detect this situation.
In that way, for example, dracut could check the 'weak' dependency of the
modules involved in order to install these dependencies in initramfs too.
That is, for the commented lan78xx module, defining the 'weak' dependency
with the possible phy modules list, only the necessary phy would be loaded
on demand keeping the same behavior, but all the possible phy modules would
be available from initramfs.

A new function 'kmod_module_get_weakdeps' in libkmod will be added for
this to avoid breaking the API and maintain backward compatibility. This
general procedure could be useful for other similar cases (not only for
dynamic phy loading).

Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
V1 -> V2:
- Better to create a new type of dependency 'weak' as Lucas De Marchi
suggests.
- Complete modprobe.d(5) with the comments from Lucas De Marchi.

 libkmod/docs/libkmod-sections.txt |   2 +
 libkmod/libkmod-config.c          | 212 ++++++++++++++++++++++++++++++
 libkmod/libkmod-internal.h        |   3 +
 libkmod/libkmod-module.c          |  58 +++++++-
 libkmod/libkmod.h                 |   3 +
 libkmod/libkmod.sym               |   2 +
 man/modprobe.d.5.xml              |  24 ++++
 tools/depmod.c                    |  25 ++++
 tools/modprobe.c                  |   1 +
 9 files changed, 326 insertions(+), 4 deletions(-)

diff --git a/libkmod/docs/libkmod-sections.txt b/libkmod/docs/libkmod-sections.txt
index 33d9eec..978b064 100644
--- a/libkmod/docs/libkmod-sections.txt
+++ b/libkmod/docs/libkmod-sections.txt
@@ -37,6 +37,7 @@ kmod_config_get_remove_commands
 kmod_config_get_aliases
 kmod_config_get_options
 kmod_config_get_softdeps
+kmod_config_get_weakdeps
 kmod_config_iter_get_key
 kmod_config_iter_get_value
 kmod_config_iter_next
@@ -62,6 +63,7 @@ kmod_module_remove_module
 kmod_module_get_module
 kmod_module_get_dependencies
 kmod_module_get_softdeps
+kmod_module_get_weakdeps
 kmod_module_apply_filter
 kmod_module_get_filtered_blacklist
 kmod_module_get_install_commands
diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c
index e83621b..a571b6b 100644
--- a/libkmod/libkmod-config.c
+++ b/libkmod/libkmod-config.c
@@ -58,6 +58,12 @@ struct kmod_softdep {
 	unsigned int n_post;
 };
 
+struct kmod_weakdep {
+	char *name;
+	const char **weak;
+	unsigned int n_weak;
+};
+
 const char *kmod_blacklist_get_modname(const struct kmod_list *l)
 {
 	return l->data;
@@ -110,6 +116,16 @@ const char * const *kmod_softdep_get_post(const struct kmod_list *l, unsigned in
 	return dep->post;
 }
 
+const char *kmod_weakdep_get_name(const struct kmod_list *l) {
+	const struct kmod_weakdep *dep = l->data;
+	return dep->name;
+}
+
+const char * const *kmod_weakdep_get_weak(const struct kmod_list *l, unsigned int *count) {
+	const struct kmod_weakdep *dep = l->data;
+	*count = dep->n_weak;
+	return dep->weak;
+}
 static int kmod_config_add_command(struct kmod_config *config,
 						const char *modname,
 						const char *command,
@@ -392,6 +408,112 @@ static int kmod_config_add_softdep(struct kmod_config *config,
 	return 0;
 }
 
+static int kmod_config_add_weakdep(struct kmod_config *config,
+							const char *modname,
+							const char *line)
+{
+	struct kmod_list *list;
+	struct kmod_weakdep *dep;
+	const char *s, *p;
+	char *itr;
+	unsigned int n_weak = 0;
+	size_t modnamelen = strlen(modname) + 1;
+	size_t buflen = 0;
+	bool was_space = false;
+
+	DBG(config->ctx, "modname=%s\n", modname);
+
+	/* analyze and count */
+	for (p = s = line; ; s++) {
+		size_t plen;
+
+		if (*s != '\0') {
+			if (!isspace(*s)) {
+				was_space = false;
+				continue;
+			}
+
+			if (was_space) {
+				p = s + 1;
+				continue;
+			}
+			was_space = true;
+
+			if (p >= s)
+				continue;
+		}
+		plen = s - p;
+
+		if (*s != '\0' || (*s == '\0' && !was_space)) {
+			buflen += plen + 1;
+			n_weak++;
+		}
+		p = s + 1;
+		if (*s == '\0')
+			break;
+	}
+
+	DBG(config->ctx, "%u weak\n", n_weak);
+
+	dep = malloc(sizeof(struct kmod_weakdep) + modnamelen +
+		     n_weak * sizeof(const char *) +
+		     buflen);
+	if (dep == NULL) {
+		ERR(config->ctx, "out-of-memory modname=%s\n", modname);
+		return -ENOMEM;
+	}
+	dep->n_weak = n_weak;
+	dep->weak = (const char **)((char *)dep + sizeof(struct kmod_weakdep));
+	dep->name = (char *)(dep->weak + n_weak);
+
+	memcpy(dep->name, modname, modnamelen);
+
+	/* copy strings */
+	itr = dep->name + modnamelen;
+	n_weak = 0;
+	was_space = false;
+	for (p = s = line; ; s++) {
+		size_t plen;
+
+		if (*s != '\0') {
+			if (!isspace(*s)) {
+				was_space = false;
+				continue;
+			}
+
+			if (was_space) {
+				p = s + 1;
+				continue;
+			}
+			was_space = true;
+
+			if (p >= s)
+				continue;
+		}
+		plen = s - p;
+
+		if (*s != '\0' || (*s == '\0' && !was_space)) {
+			dep->weak[n_weak] = itr;
+			memcpy(itr, p, plen);
+			itr[plen] = '\0';
+			itr += plen + 1;
+			n_weak++;
+		}
+		p = s + 1;
+		if (*s == '\0')
+			break;
+	}
+
+	list = kmod_list_append(config->weakdeps, dep);
+	if (list == NULL) {
+		free(dep);
+		return -ENOMEM;
+	}
+	config->weakdeps = list;
+
+	return 0;
+}
+
 static char *softdep_to_char(struct kmod_softdep *dep) {
 	const size_t sz_preprefix = sizeof("pre: ") - 1;
 	const size_t sz_postprefix = sizeof("post: ") - 1;
@@ -461,6 +583,44 @@ static char *softdep_to_char(struct kmod_softdep *dep) {
 	return s;
 }
 
+static char *weakdep_to_char(struct kmod_weakdep *dep) {
+	size_t sz;
+	const char *start, *end;
+	char *s, *itr;
+
+	/*
+	 * Rely on the fact that dep->weak[] and are strv's that point to a
+	 * contiguous buffer
+	 */
+	if (dep->n_weak > 0) {
+		start = dep->weak[0];
+		end = dep->weak[dep->n_weak - 1]
+					+ strlen(dep->weak[dep->n_weak - 1]);
+		sz = end - start;
+	} else
+		sz = 0;
+
+	itr = s = malloc(sz);
+	if (s == NULL)
+		return NULL;
+
+	if (sz) {
+		char *p;
+
+		/* include last '\0' */
+		memcpy(itr, dep->weak[0], sz + 1);
+		for (p = itr; p < itr + sz; p++) {
+			if (*p == '\0')
+				*p = ' ';
+		}
+		itr = p;
+	}
+
+	*itr = '\0';
+
+	return s;
+}
+
 static void kmod_config_free_softdep(struct kmod_config *config,
 							struct kmod_list *l)
 {
@@ -468,6 +628,13 @@ static void kmod_config_free_softdep(struct kmod_config *config,
 	config->softdeps = kmod_list_remove(l);
 }
 
+static void kmod_config_free_weakdep(struct kmod_config *config,
+							struct kmod_list *l)
+{
+	free(l->data);
+	config->weakdeps = kmod_list_remove(l);
+}
+
 static void kcmdline_parse_result(struct kmod_config *config, char *modname,
 						char *param, char *value)
 {
@@ -703,6 +870,14 @@ static int kmod_config_parse(struct kmod_config *config, int fd,
 				goto syntax_error;
 
 			kmod_config_add_softdep(config, modname, softdeps);
+		} else if (streq(cmd, "weakdep")) {
+			char *modname = strtok_r(NULL, "\t ", &saveptr);
+			char *weakdeps = strtok_r(NULL, "\0", &saveptr);
+
+			if (underscores(modname) < 0 || weakdeps == NULL)
+				goto syntax_error;
+
+			kmod_config_add_weakdep(config, modname, weakdeps);
 		} else if (streq(cmd, "include")
 				|| streq(cmd, "config")) {
 			ERR(ctx, "%s: command %s is deprecated and not parsed anymore\n",
@@ -746,6 +921,9 @@ void kmod_config_free(struct kmod_config *config)
 	while (config->softdeps)
 		kmod_config_free_softdep(config, config->softdeps);
 
+	while (config->weakdeps)
+		kmod_config_free_weakdep(config, config->weakdeps);
+
 	for (; config->paths != NULL;
 				config->paths = kmod_list_remove(config->paths))
 		free(config->paths->data);
@@ -889,6 +1067,7 @@ int kmod_config_new(struct kmod_ctx *ctx, struct kmod_config **p_config,
 	size_t i;
 
 	conf_files_insert_sorted(ctx, &list, kmod_get_dirname(ctx), "modules.softdep");
+	conf_files_insert_sorted(ctx, &list, kmod_get_dirname(ctx), "modules.weakdep");
 
 	for (i = 0; config_paths[i] != NULL; i++) {
 		const char *path = config_paths[i];
@@ -973,6 +1152,7 @@ enum config_type {
 	CONFIG_TYPE_ALIAS,
 	CONFIG_TYPE_OPTION,
 	CONFIG_TYPE_SOFTDEP,
+	CONFIG_TYPE_WEAKDEP,
 };
 
 struct kmod_config_iter {
@@ -991,6 +1171,12 @@ static const char *softdep_get_plain_softdep(const struct kmod_list *l)
 	return s;
 }
 
+static const char *weakdep_get_plain_weakdep(const struct kmod_list *l)
+{
+	char *s = weakdep_to_char(l->data);
+	return s;
+}
+
 static struct kmod_config_iter *kmod_config_iter_new(const struct kmod_ctx* ctx,
 							enum config_type type)
 {
@@ -1033,6 +1219,12 @@ static struct kmod_config_iter *kmod_config_iter_new(const struct kmod_ctx* ctx,
 		iter->get_value = softdep_get_plain_softdep;
 		iter->intermediate = true;
 		break;
+	case CONFIG_TYPE_WEAKDEP:
+		iter->list = config->weakdeps;
+		iter->get_key = kmod_weakdep_get_name;
+		iter->get_value = weakdep_get_plain_weakdep;
+		iter->intermediate = true;
+		break;
 	}
 
 	return iter;
@@ -1163,6 +1355,26 @@ KMOD_EXPORT struct kmod_config_iter *kmod_config_get_softdeps(const struct kmod_
 	return kmod_config_iter_new(ctx, CONFIG_TYPE_SOFTDEP);
 }
 
+/**
+ * kmod_config_get_weakdeps:
+ * @ctx: kmod library context
+ *
+ * Retrieve an iterator to deal with the weakdeps maintained inside the
+ * library. See kmod_config_iter_get_key(), kmod_config_iter_get_value() and
+ * kmod_config_iter_next(). At least one call to kmod_config_iter_next() must
+ * be made to initialize the iterator and check if it's valid.
+ *
+ * Returns: a new iterator over the weakdeps or NULL on failure. Free it with
+ * kmod_config_iter_free_iter().
+ */
+KMOD_EXPORT struct kmod_config_iter *kmod_config_get_weakdeps(const struct kmod_ctx *ctx)
+{
+	if (ctx == NULL)
+		return NULL;;
+
+	return kmod_config_iter_new(ctx, CONFIG_TYPE_WEAKDEP);
+}
+
 /**
  * kmod_config_iter_get_key:
  * @iter: iterator over a certain configuration
diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
index 26a7e28..52ddbe6 100644
--- a/libkmod/libkmod-internal.h
+++ b/libkmod/libkmod-internal.h
@@ -128,6 +128,7 @@ struct kmod_config {
 	struct kmod_list *remove_commands;
 	struct kmod_list *install_commands;
 	struct kmod_list *softdeps;
+	struct kmod_list *weakdeps;
 
 	struct kmod_list *paths;
 };
@@ -146,6 +147,8 @@ const char *kmod_softdep_get_name(const struct kmod_list *l) __attribute__((nonn
 const char * const *kmod_softdep_get_pre(const struct kmod_list *l, unsigned int *count) __attribute__((nonnull(1, 2)));
 const char * const *kmod_softdep_get_post(const struct kmod_list *l, unsigned int *count);
 
+const char *kmod_weakdep_get_name(const struct kmod_list *l) __attribute__((nonnull(1)));
+const char * const *kmod_weakdep_get_weak(const struct kmod_list *l, unsigned int *count) __attribute__((nonnull(1, 2)));
 
 /* libkmod-module.c */
 int kmod_module_new_from_alias(struct kmod_ctx *ctx, const char *alias, const char *name, struct kmod_module **mod);
diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
index 585da41..4f23ffb 100644
--- a/libkmod/libkmod-module.c
+++ b/libkmod/libkmod-module.c
@@ -1589,7 +1589,7 @@ void kmod_module_set_install_commands(struct kmod_module *mod, const char *cmd)
 	mod->install_commands = cmd;
 }
 
-static struct kmod_list *lookup_softdep(struct kmod_ctx *ctx, const char * const * array, unsigned int count)
+static struct kmod_list *lookup_dep(struct kmod_ctx *ctx, const char * const * array, unsigned int count)
 {
 	struct kmod_list *ret = NULL;
 	unsigned i;
@@ -1601,7 +1601,7 @@ static struct kmod_list *lookup_softdep(struct kmod_ctx *ctx, const char * const
 
 		err = kmod_module_new_from_lookup(ctx, depname, &lst);
 		if (err < 0) {
-			ERR(ctx, "failed to lookup soft dependency '%s', continuing anyway.\n", depname);
+			ERR(ctx, "failed to lookup dependency '%s', continuing anyway.\n", depname);
 			continue;
 		} else if (lst != NULL)
 			ret = kmod_list_append_list(ret, lst);
@@ -1650,9 +1650,59 @@ KMOD_EXPORT int kmod_module_get_softdeps(const struct kmod_module *mod,
 			continue;
 
 		array = kmod_softdep_get_pre(l, &count);
-		*pre = lookup_softdep(mod->ctx, array, count);
+		*pre = lookup_dep(mod->ctx, array, count);
 		array = kmod_softdep_get_post(l, &count);
-		*post = lookup_softdep(mod->ctx, array, count);
+		*post = lookup_dep(mod->ctx, array, count);
+
+		/*
+		 * find only the first command, as modprobe from
+		 * module-init-tools does
+		 */
+		break;
+	}
+
+	return 0;
+}
+
+/*
+ * kmod_module_get_weakdeps:
+ * @mod: kmod module
+ * @weak: where to save the list of weak dependencies.
+ *
+ * Get weak dependencies for this kmod module. Weak dependencies come
+ * from configuration file and are not cached in @mod because it may include
+ * dependency cycles that would make we leak kmod_module. Any call
+ * to this function will search for this module in configuration, allocate a
+ * list and return the result.
+ *
+ * @weak is newly created list of kmod_module and
+ * should be unreferenced with kmod_module_unref_list().
+ *
+ * Returns: 0 on success or < 0 otherwise.
+ */
+KMOD_EXPORT int kmod_module_get_weakdeps(const struct kmod_module *mod,
+						struct kmod_list **weak)
+{
+	const struct kmod_list *l;
+	const struct kmod_config *config;
+
+	if (mod == NULL || weak == NULL)
+		return -ENOENT;
+
+	assert(*weak == NULL);
+
+	config = kmod_get_config(mod->ctx);
+
+	kmod_list_foreach(l, config->weakdeps) {
+		const char *modname = kmod_weakdep_get_name(l);
+		const char * const *array;
+		unsigned count;
+
+		if (fnmatch(modname, mod->name, 0) != 0)
+			continue;
+
+		array = kmod_weakdep_get_weak(l, &count);
+		*weak = lookup_dep(mod->ctx, array, count);
 
 		/*
 		 * find only the first command, as modprobe from
diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
index 7251aa7..fce66d1 100644
--- a/libkmod/libkmod.h
+++ b/libkmod/libkmod.h
@@ -112,6 +112,7 @@ struct kmod_config_iter *kmod_config_get_remove_commands(const struct kmod_ctx *
 struct kmod_config_iter *kmod_config_get_aliases(const struct kmod_ctx *ctx);
 struct kmod_config_iter *kmod_config_get_options(const struct kmod_ctx *ctx);
 struct kmod_config_iter *kmod_config_get_softdeps(const struct kmod_ctx *ctx);
+struct kmod_config_iter *kmod_config_get_weakdeps(const struct kmod_ctx *ctx);
 const char *kmod_config_iter_get_key(const struct kmod_config_iter *iter);
 const char *kmod_config_iter_get_value(const struct kmod_config_iter *iter);
 bool kmod_config_iter_next(struct kmod_config_iter *iter);
@@ -196,6 +197,8 @@ const char *kmod_module_get_remove_commands(const struct kmod_module *mod);
 struct kmod_list *kmod_module_get_dependencies(const struct kmod_module *mod);
 int kmod_module_get_softdeps(const struct kmod_module *mod,
 				struct kmod_list **pre, struct kmod_list **post);
+int kmod_module_get_weakdeps(const struct kmod_module *mod,
+				struct kmod_list **weak);
 int kmod_module_get_filtered_blacklist(const struct kmod_ctx *ctx,
 					const struct kmod_list *input,
 					struct kmod_list **output) __attribute__ ((deprecated));
diff --git a/libkmod/libkmod.sym b/libkmod/libkmod.sym
index 0c04fda..0d6d338 100644
--- a/libkmod/libkmod.sym
+++ b/libkmod/libkmod.sym
@@ -21,6 +21,7 @@ global:
 	kmod_config_get_aliases;
 	kmod_config_get_options;
 	kmod_config_get_softdeps;
+	kmod_config_get_weakdeps;
 	kmod_config_iter_get_key;
 	kmod_config_iter_get_value;
 	kmod_config_iter_next;
@@ -42,6 +43,7 @@ global:
 
 	kmod_module_get_dependencies;
 	kmod_module_get_softdeps;
+	kmod_module_get_weakdeps;
 	kmod_module_get_filtered_blacklist;
 
 	kmod_module_get_name;
diff --git a/man/modprobe.d.5.xml b/man/modprobe.d.5.xml
index 2bf6537..cc90da6 100644
--- a/man/modprobe.d.5.xml
+++ b/man/modprobe.d.5.xml
@@ -212,6 +212,30 @@
           </para>
         </listitem>
       </varlistentry>
+      <varlistentry>
+        <term>weakdep <replaceable>modulename</replaceable> <replaceable>modules...</replaceable>
+        </term>
+        <listitem>
+          <para>
+            The <command>weakdep</command> command allows you to specify weak module
+            dependencies. Those are similar to pre softdep, with the
+            difference that userspace doesn't attempt to load that
+            dependency before the specified module. Instead the kernel
+            may request one or multiple of them during module probe,
+            depending on the hardware it's binding to. The purpose of
+            weak module is to allow a driver to specify that a certain
+            dependency may be needed, so it should be present in the
+            filesystem (e.g. in initramfs) when that module is probed.
+          </para>
+	  <para>
+            Example: Assume "weakdep c a b". A program creating an
+            initramfs knows it should add a, b, and c to the filesystem
+            since a and b may be required/desired at runtime. When c is
+            loaded and is being probed, it may issue calls to
+            request_module() causing a or b to also be loaded.
+          </para>
+        </listitem>
+      </varlistentry>
     </variablelist>
   </refsect1>
   <refsect1><title>COMPATIBILITY</title>
diff --git a/tools/depmod.c b/tools/depmod.c
index 43fc354..06618fa 100644
--- a/tools/depmod.c
+++ b/tools/depmod.c
@@ -2296,6 +2296,30 @@ static int output_softdeps(struct depmod *depmod, FILE *out)
 	return 0;
 }
 
+static int output_weakdeps(struct depmod *depmod, FILE *out)
+{
+	size_t i;
+
+	fputs("# Weak dependencies extracted from modules themselves.\n", out);
+
+	for (i = 0; i < depmod->modules.count; i++) {
+		const struct mod *mod = depmod->modules.array[i];
+		struct kmod_list *l;
+
+		kmod_list_foreach(l, mod->info_list) {
+			const char *key = kmod_module_info_get_key(l);
+			const char *value = kmod_module_info_get_value(l);
+
+			if (!streq(key, "weakdep"))
+				continue;
+
+			fprintf(out, "weakdep %s %s\n", mod->modname, value);
+		}
+	}
+
+	return 0;
+}
+
 static int output_symbols(struct depmod *depmod, FILE *out)
 {
 	struct hash_iter iter;
@@ -2574,6 +2598,7 @@ static int depmod_output(struct depmod *depmod, FILE *out)
 		{ "modules.alias", output_aliases },
 		{ "modules.alias.bin", output_aliases_bin },
 		{ "modules.softdep", output_softdeps },
+		{ "modules.weakdep", output_weakdeps },
 		{ "modules.symbols", output_symbols },
 		{ "modules.symbols.bin", output_symbols_bin },
 		{ "modules.builtin.bin", output_builtin_bin },
diff --git a/tools/modprobe.c b/tools/modprobe.c
index 5306bef..4328da6 100644
--- a/tools/modprobe.c
+++ b/tools/modprobe.c
@@ -182,6 +182,7 @@ static int show_config(struct kmod_ctx *ctx)
 		{ "alias", kmod_config_get_aliases },
 		{ "options", kmod_config_get_options },
 		{ "softdep", kmod_config_get_softdeps },
+		{ "weakdep", kmod_config_get_weakdeps },
 	};
 	size_t i;
 
-- 
2.44.0

[PATCH] module: create weak dependecies

[Jose Ignacio Tornos Martinez]

It has been seen that for some network mac drivers (i.e. lan78xx) the
related module for the phy is loaded dynamically depending on the current
hardware. In this case, the associated phy is read using mdio bus and then
the associated phy module is loaded during runtime (kernel function
phy_request_driver_module). However, no software dependency is defined, so
the user tools will no be able to get this dependency. For example, if
dracut is used and the hardware is present, lan78xx will be included but no
phy module will be added, and in the next restart the device will not work
from boot because no related phy will be found during initramfs stage.

In order to solve this, we could define a normal 'pre' software dependency
in lan78xx module with all the possible phy modules (there may be some),
but proceeding in that way, all the possible phy modules would be loaded
while only one is necessary.

The idea is to create a new type of dependency, that we are going to call
'weak' to be used only by the user tools that need to detect this situation.
In that way, for example, dracut could check the 'weak' dependency of the
modules involved in order to install these dependencies in initramfs too.
That is, for the commented lan78xx module, defining the 'weak' dependency
with the possible phy modules list, only the necessary phy would be loaded
on demand keeping the same behavior, but all the possible phy modules would
be available from initramfs.

Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
---
 include/linux/module.h | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/include/linux/module.h b/include/linux/module.h
index 1153b0d99a80..231e710d8736 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -173,6 +173,11 @@ extern void cleanup_module(void);
  */
 #define MODULE_SOFTDEP(_softdep) MODULE_INFO(softdep, _softdep)
 
+/* Weak module dependencies. See man modprobe.d for details.
+ * Example: MODULE_WEAKDEP("module-foo")
+ */
+#define MODULE_WEAKDEP(_weakdep) MODULE_INFO(weakdep, _weakdep)
+
 /*
  * MODULE_FILE is used for generating modules.builtin
  * So, make it no-op when this is being built as a module
-- 
2.44.0

Re: [PATCH v2 patch] libkmod: add weak dependecies

[Lucas De Marchi]

Sorry for the delay. I will review this later this week.
For now, I'm a bit confused. Why do we have 2 patches?

	[PATCH v2 kmod] libkmod: add weak dependecies
	[PATCH v2 patch] libkmod: add weak dependecies

Was one of them sent by mistake?

No need to resend now, but it'd be preferred to skip --in-reply-to=<v1>
and just rely on -v2 and let b4 do its thing to detect new versions.

thanks
Lucas De Marchi

Re: [PATCH v2 patch] libkmod: add weak dependecies

[Jose Ignacio Tornos Martinez]

Hello Lucas,

> Sorry for the delay. I will review this later this week.
No problem!

> For now, I'm a bit confused. Why do we have 2 patches?
>
>	[PATCH v2 kmod] libkmod: add weak dependecies
>	[PATCH v2 patch] libkmod: add weak dependecies
>
> Was one of them sent by mistake?
Yes, one is a mistake but only in the suffix in [PATH v2 ...] that should
be kmod. Indeed the content of the patches is the same for both.
You can choose [PATCH v2 kmod] because as I said the suffix is ok.
Sorry for the confusion, I should have commented ...

> No need to resend now, but it'd be preferred to skip --in-reply-to=<v1>
> and just rely on -v2 and let b4 do its thing to detect new versions.
Sorry again, I didn't know it, next time I will do as you say.

Thanks

Best regards
José Ignacio

Re: [PATCH v2 patch] libkmod: add weak dependecies

[Jose Ignacio Tornos Martinez]

Hello Lucas,

If you have time, let me ask you about the status of the weak dependency feature.
Do you have any feedback/review for me?
Should I do something to improve it and/or progress?

Thanks in advance

Best regards
José Ignacio

Re: [PATCH v2 patch] libkmod: add weak dependecies

[Lucas De Marchi]

On Mon, May 06, 2024 at 02:36:02PM GMT, Jose Ignacio Tornos Martinez wrote:
>Hello Lucas,
>
>If you have time, let me ask you about the status of the weak dependency feature.
>Do you have any feedback/review for me?
>Should I do something to improve it and/or progress?

that's totally on me, sorry. I was busy with other tasks and started
cleaning up the backlog in kmod with the older things that were also
pending. I will review your shortly.

Lucas De Marchi

>
>Thanks in advance
>
>Best regards
>José Ignacio
>
>

Re: [PATCH v2 patch] libkmod: add weak dependecies

[Lucas De Marchi]

On Wed, Mar 27, 2024 at 03:11:16PM GMT, Jose Ignacio Tornos Martinez wrote:
>It has been seen that for some network mac drivers (i.e. lan78xx) the
>related module for the phy is loaded dynamically depending on the current
>hardware. In this case, the associated phy is read using mdio bus and then
>the associated phy module is loaded during runtime (kernel function
>phy_request_driver_module). However, no software dependency is defined, so
>the user tools will no be able to get this dependency. For example, if
>dracut is used and the hardware is present, lan78xx will be included but no
>phy module will be added, and in the next restart the device will not work
>from boot because no related phy will be found during initramfs stage.
>
>In order to solve this, we could define a normal 'pre' software dependency
>in lan78xx module with all the possible phy modules (there may be some),
>but proceeding in that way, all the possible phy modules would be loaded
>while only one is necessary.
>
>The idea is to create a new type of dependency, that we are going to call
>'weak' to be used only by the user tools that need to detect this situation.
>In that way, for example, dracut could check the 'weak' dependency of the
>modules involved in order to install these dependencies in initramfs too.
>That is, for the commented lan78xx module, defining the 'weak' dependency
>with the possible phy modules list, only the necessary phy would be loaded
>on demand keeping the same behavior, but all the possible phy modules would
>be available from initramfs.
>
>A new function 'kmod_module_get_weakdeps' in libkmod will be added for
>this to avoid breaking the API and maintain backward compatibility. This
>general procedure could be useful for other similar cases (not only for
>dynamic phy loading).
>
>Signed-off-by: Jose Ignacio Tornos Martinez <jtornosm@redhat.com>
>---
>V1 -> V2:
>- Better to create a new type of dependency 'weak' as Lucas De Marchi
>suggests.
>- Complete modprobe.d(5) with the comments from Lucas De Marchi.
>
> libkmod/docs/libkmod-sections.txt |   2 +
> libkmod/libkmod-config.c          | 212 ++++++++++++++++++++++++++++++
> libkmod/libkmod-internal.h        |   3 +
> libkmod/libkmod-module.c          |  58 +++++++-
> libkmod/libkmod.h                 |   3 +
> libkmod/libkmod.sym               |   2 +
> man/modprobe.d.5.xml              |  24 ++++
> tools/depmod.c                    |  25 ++++
> tools/modprobe.c                  |   1 +
> 9 files changed, 326 insertions(+), 4 deletions(-)
>
>diff --git a/libkmod/docs/libkmod-sections.txt b/libkmod/docs/libkmod-sections.txt
>index 33d9eec..978b064 100644
>--- a/libkmod/docs/libkmod-sections.txt
>+++ b/libkmod/docs/libkmod-sections.txt
>@@ -37,6 +37,7 @@ kmod_config_get_remove_commands
> kmod_config_get_aliases
> kmod_config_get_options
> kmod_config_get_softdeps
>+kmod_config_get_weakdeps
> kmod_config_iter_get_key
> kmod_config_iter_get_value
> kmod_config_iter_next
>@@ -62,6 +63,7 @@ kmod_module_remove_module
> kmod_module_get_module
> kmod_module_get_dependencies
> kmod_module_get_softdeps
>+kmod_module_get_weakdeps
> kmod_module_apply_filter
> kmod_module_get_filtered_blacklist
> kmod_module_get_install_commands
>diff --git a/libkmod/libkmod-config.c b/libkmod/libkmod-config.c
>index e83621b..a571b6b 100644
>--- a/libkmod/libkmod-config.c
>+++ b/libkmod/libkmod-config.c
>@@ -58,6 +58,12 @@ struct kmod_softdep {
> 	unsigned int n_post;
> };
>
>+struct kmod_weakdep {
>+	char *name;
>+	const char **weak;
>+	unsigned int n_weak;
>+};
>+
> const char *kmod_blacklist_get_modname(const struct kmod_list *l)
> {
> 	return l->data;
>@@ -110,6 +116,16 @@ const char * const *kmod_softdep_get_post(const struct kmod_list *l, unsigned in
> 	return dep->post;
> }
>
>+const char *kmod_weakdep_get_name(const struct kmod_list *l) {
>+	const struct kmod_weakdep *dep = l->data;
>+	return dep->name;
>+}
>+
>+const char * const *kmod_weakdep_get_weak(const struct kmod_list *l, unsigned int *count) {
>+	const struct kmod_weakdep *dep = l->data;
>+	*count = dep->n_weak;
>+	return dep->weak;
>+}
> static int kmod_config_add_command(struct kmod_config *config,
> 						const char *modname,
> 						const char *command,
>@@ -392,6 +408,112 @@ static int kmod_config_add_softdep(struct kmod_config *config,
> 	return 0;
> }
>
>+static int kmod_config_add_weakdep(struct kmod_config *config,
>+							const char *modname,
>+							const char *line)
>+{
>+	struct kmod_list *list;
>+	struct kmod_weakdep *dep;
>+	const char *s, *p;
>+	char *itr;
>+	unsigned int n_weak = 0;
>+	size_t modnamelen = strlen(modname) + 1;
>+	size_t buflen = 0;
>+	bool was_space = false;
>+
>+	DBG(config->ctx, "modname=%s\n", modname);
>+
>+	/* analyze and count */
>+	for (p = s = line; ; s++) {
>+		size_t plen;
>+
>+		if (*s != '\0') {
>+			if (!isspace(*s)) {
>+				was_space = false;
>+				continue;
>+			}
>+
>+			if (was_space) {
>+				p = s + 1;
>+				continue;
>+			}
>+			was_space = true;
>+
>+			if (p >= s)
>+				continue;
>+		}
>+		plen = s - p;
>+
>+		if (*s != '\0' || (*s == '\0' && !was_space)) {
>+			buflen += plen + 1;
>+			n_weak++;
>+		}
>+		p = s + 1;
>+		if (*s == '\0')
>+			break;
>+	}
>+
>+	DBG(config->ctx, "%u weak\n", n_weak);
>+
>+	dep = malloc(sizeof(struct kmod_weakdep) + modnamelen +
>+		     n_weak * sizeof(const char *) +
>+		     buflen);
>+	if (dep == NULL) {
>+		ERR(config->ctx, "out-of-memory modname=%s\n", modname);
>+		return -ENOMEM;
>+	}
>+	dep->n_weak = n_weak;
>+	dep->weak = (const char **)((char *)dep + sizeof(struct kmod_weakdep));
>+	dep->name = (char *)(dep->weak + n_weak);
>+
>+	memcpy(dep->name, modname, modnamelen);
>+
>+	/* copy strings */
>+	itr = dep->name + modnamelen;
>+	n_weak = 0;
>+	was_space = false;
>+	for (p = s = line; ; s++) {
>+		size_t plen;
>+
>+		if (*s != '\0') {
>+			if (!isspace(*s)) {
>+				was_space = false;
>+				continue;
>+			}
>+
>+			if (was_space) {
>+				p = s + 1;
>+				continue;
>+			}
>+			was_space = true;
>+
>+			if (p >= s)
>+				continue;
>+		}
>+		plen = s - p;
>+
>+		if (*s != '\0' || (*s == '\0' && !was_space)) {
>+			dep->weak[n_weak] = itr;
>+			memcpy(itr, p, plen);
>+			itr[plen] = '\0';
>+			itr += plen + 1;
>+			n_weak++;
>+		}
>+		p = s + 1;
>+		if (*s == '\0')
>+			break;
>+	}
>+
>+	list = kmod_list_append(config->weakdeps, dep);
>+	if (list == NULL) {
>+		free(dep);
>+		return -ENOMEM;
>+	}
>+	config->weakdeps = list;

this is a long function that we may eventually break in smaller pieces
and share with the softdep parser.

>+
>+	return 0;
>+}
>+
> static char *softdep_to_char(struct kmod_softdep *dep) {
> 	const size_t sz_preprefix = sizeof("pre: ") - 1;
> 	const size_t sz_postprefix = sizeof("post: ") - 1;
>@@ -461,6 +583,44 @@ static char *softdep_to_char(struct kmod_softdep *dep) {
> 	return s;
> }
>
>+static char *weakdep_to_char(struct kmod_weakdep *dep) {
>+	size_t sz;
>+	const char *start, *end;
>+	char *s, *itr;
>+
>+	/*
>+	 * Rely on the fact that dep->weak[] and are strv's that point to a
>+	 * contiguous buffer
>+	 */
>+	if (dep->n_weak > 0) {
>+		start = dep->weak[0];
>+		end = dep->weak[dep->n_weak - 1]
>+					+ strlen(dep->weak[dep->n_weak - 1]);
>+		sz = end - start;
>+	} else
>+		sz = 0;
>+
>+	itr = s = malloc(sz);
>+	if (s == NULL)
>+		return NULL;
>+
>+	if (sz) {
>+		char *p;
>+
>+		/* include last '\0' */
>+		memcpy(itr, dep->weak[0], sz + 1);
>+		for (p = itr; p < itr + sz; p++) {
>+			if (*p == '\0')
>+				*p = ' ';
>+		}
>+		itr = p;
>+	}
>+
>+	*itr = '\0';

ditto for this one

>+
>+	return s;
>+}
>+
> static void kmod_config_free_softdep(struct kmod_config *config,
> 							struct kmod_list *l)
> {
>@@ -468,6 +628,13 @@ static void kmod_config_free_softdep(struct kmod_config *config,
> 	config->softdeps = kmod_list_remove(l);
> }
>
>+static void kmod_config_free_weakdep(struct kmod_config *config,
>+							struct kmod_list *l)
>+{
>+	free(l->data);
>+	config->weakdeps = kmod_list_remove(l);
>+}
>+
> static void kcmdline_parse_result(struct kmod_config *config, char *modname,
> 						char *param, char *value)
> {
>@@ -703,6 +870,14 @@ static int kmod_config_parse(struct kmod_config *config, int fd,
> 				goto syntax_error;
>
> 			kmod_config_add_softdep(config, modname, softdeps);
>+		} else if (streq(cmd, "weakdep")) {
>+			char *modname = strtok_r(NULL, "\t ", &saveptr);
>+			char *weakdeps = strtok_r(NULL, "\0", &saveptr);
>+
>+			if (underscores(modname) < 0 || weakdeps == NULL)
>+				goto syntax_error;
>+
>+			kmod_config_add_weakdep(config, modname, weakdeps);
> 		} else if (streq(cmd, "include")
> 				|| streq(cmd, "config")) {
> 			ERR(ctx, "%s: command %s is deprecated and not parsed anymore\n",
>@@ -746,6 +921,9 @@ void kmod_config_free(struct kmod_config *config)
> 	while (config->softdeps)
> 		kmod_config_free_softdep(config, config->softdeps);
>
>+	while (config->weakdeps)
>+		kmod_config_free_weakdep(config, config->weakdeps);
>+
> 	for (; config->paths != NULL;
> 				config->paths = kmod_list_remove(config->paths))
> 		free(config->paths->data);
>@@ -889,6 +1067,7 @@ int kmod_config_new(struct kmod_ctx *ctx, struct kmod_config **p_config,
> 	size_t i;
>
> 	conf_files_insert_sorted(ctx, &list, kmod_get_dirname(ctx), "modules.softdep");
>+	conf_files_insert_sorted(ctx, &list, kmod_get_dirname(ctx), "modules.weakdep");
>
> 	for (i = 0; config_paths[i] != NULL; i++) {
> 		const char *path = config_paths[i];
>@@ -973,6 +1152,7 @@ enum config_type {
> 	CONFIG_TYPE_ALIAS,
> 	CONFIG_TYPE_OPTION,
> 	CONFIG_TYPE_SOFTDEP,
>+	CONFIG_TYPE_WEAKDEP,
> };
>
> struct kmod_config_iter {
>@@ -991,6 +1171,12 @@ static const char *softdep_get_plain_softdep(const struct kmod_list *l)
> 	return s;
> }
>
>+static const char *weakdep_get_plain_weakdep(const struct kmod_list *l)
>+{
>+	char *s = weakdep_to_char(l->data);
>+	return s;
>+}
>+
> static struct kmod_config_iter *kmod_config_iter_new(const struct kmod_ctx* ctx,
> 							enum config_type type)
> {
>@@ -1033,6 +1219,12 @@ static struct kmod_config_iter *kmod_config_iter_new(const struct kmod_ctx* ctx,
> 		iter->get_value = softdep_get_plain_softdep;
> 		iter->intermediate = true;
> 		break;
>+	case CONFIG_TYPE_WEAKDEP:
>+		iter->list = config->weakdeps;
>+		iter->get_key = kmod_weakdep_get_name;
>+		iter->get_value = weakdep_get_plain_weakdep;
>+		iter->intermediate = true;
>+		break;
> 	}
>
> 	return iter;
>@@ -1163,6 +1355,26 @@ KMOD_EXPORT struct kmod_config_iter *kmod_config_get_softdeps(const struct kmod_
> 	return kmod_config_iter_new(ctx, CONFIG_TYPE_SOFTDEP);
> }
>
>+/**
>+ * kmod_config_get_weakdeps:
>+ * @ctx: kmod library context
>+ *
>+ * Retrieve an iterator to deal with the weakdeps maintained inside the
>+ * library. See kmod_config_iter_get_key(), kmod_config_iter_get_value() and
>+ * kmod_config_iter_next(). At least one call to kmod_config_iter_next() must
>+ * be made to initialize the iterator and check if it's valid.
>+ *
>+ * Returns: a new iterator over the weakdeps or NULL on failure. Free it with
>+ * kmod_config_iter_free_iter().
>+ */
>+KMOD_EXPORT struct kmod_config_iter *kmod_config_get_weakdeps(const struct kmod_ctx *ctx)
>+{
>+	if (ctx == NULL)
>+		return NULL;;
>+
>+	return kmod_config_iter_new(ctx, CONFIG_TYPE_WEAKDEP);
>+}
>+
> /**
>  * kmod_config_iter_get_key:
>  * @iter: iterator over a certain configuration
>diff --git a/libkmod/libkmod-internal.h b/libkmod/libkmod-internal.h
>index 26a7e28..52ddbe6 100644
>--- a/libkmod/libkmod-internal.h
>+++ b/libkmod/libkmod-internal.h
>@@ -128,6 +128,7 @@ struct kmod_config {
> 	struct kmod_list *remove_commands;
> 	struct kmod_list *install_commands;
> 	struct kmod_list *softdeps;
>+	struct kmod_list *weakdeps;
>
> 	struct kmod_list *paths;
> };
>@@ -146,6 +147,8 @@ const char *kmod_softdep_get_name(const struct kmod_list *l) __attribute__((nonn
> const char * const *kmod_softdep_get_pre(const struct kmod_list *l, unsigned int *count) __attribute__((nonnull(1, 2)));
> const char * const *kmod_softdep_get_post(const struct kmod_list *l, unsigned int *count);
>
>+const char *kmod_weakdep_get_name(const struct kmod_list *l) __attribute__((nonnull(1)));
>+const char * const *kmod_weakdep_get_weak(const struct kmod_list *l, unsigned int *count) __attribute__((nonnull(1, 2)));
>
> /* libkmod-module.c */
> int kmod_module_new_from_alias(struct kmod_ctx *ctx, const char *alias, const char *name, struct kmod_module **mod);
>diff --git a/libkmod/libkmod-module.c b/libkmod/libkmod-module.c
>index 585da41..4f23ffb 100644
>--- a/libkmod/libkmod-module.c
>+++ b/libkmod/libkmod-module.c
>@@ -1589,7 +1589,7 @@ void kmod_module_set_install_commands(struct kmod_module *mod, const char *cmd)
> 	mod->install_commands = cmd;
> }
>
>-static struct kmod_list *lookup_softdep(struct kmod_ctx *ctx, const char * const * array, unsigned int count)
>+static struct kmod_list *lookup_dep(struct kmod_ctx *ctx, const char * const * array, unsigned int count)
> {
> 	struct kmod_list *ret = NULL;
> 	unsigned i;
>@@ -1601,7 +1601,7 @@ static struct kmod_list *lookup_softdep(struct kmod_ctx *ctx, const char * const
>
> 		err = kmod_module_new_from_lookup(ctx, depname, &lst);
> 		if (err < 0) {
>-			ERR(ctx, "failed to lookup soft dependency '%s', continuing anyway.\n", depname);
>+			ERR(ctx, "failed to lookup dependency '%s', continuing anyway.\n", depname);
> 			continue;
> 		} else if (lst != NULL)
> 			ret = kmod_list_append_list(ret, lst);
>@@ -1650,9 +1650,59 @@ KMOD_EXPORT int kmod_module_get_softdeps(const struct kmod_module *mod,
> 			continue;
>
> 		array = kmod_softdep_get_pre(l, &count);
>-		*pre = lookup_softdep(mod->ctx, array, count);
>+		*pre = lookup_dep(mod->ctx, array, count);
> 		array = kmod_softdep_get_post(l, &count);
>-		*post = lookup_softdep(mod->ctx, array, count);
>+		*post = lookup_dep(mod->ctx, array, count);
>+
>+		/*
>+		 * find only the first command, as modprobe from
>+		 * module-init-tools does
>+		 */
>+		break;
>+	}
>+
>+	return 0;
>+}
>+
>+/*
>+ * kmod_module_get_weakdeps:
>+ * @mod: kmod module
>+ * @weak: where to save the list of weak dependencies.
>+ *
>+ * Get weak dependencies for this kmod module. Weak dependencies come
>+ * from configuration file and are not cached in @mod because it may include
>+ * dependency cycles that would make we leak kmod_module. Any call
>+ * to this function will search for this module in configuration, allocate a
>+ * list and return the result.
>+ *
>+ * @weak is newly created list of kmod_module and
>+ * should be unreferenced with kmod_module_unref_list().
>+ *
>+ * Returns: 0 on success or < 0 otherwise.
>+ */
>+KMOD_EXPORT int kmod_module_get_weakdeps(const struct kmod_module *mod,
>+						struct kmod_list **weak)
>+{
>+	const struct kmod_list *l;
>+	const struct kmod_config *config;
>+
>+	if (mod == NULL || weak == NULL)
>+		return -ENOENT;
>+
>+	assert(*weak == NULL);
>+
>+	config = kmod_get_config(mod->ctx);
>+
>+	kmod_list_foreach(l, config->weakdeps) {
>+		const char *modname = kmod_weakdep_get_name(l);
>+		const char * const *array;
>+		unsigned count;
>+
>+		if (fnmatch(modname, mod->name, 0) != 0)
>+			continue;
>+
>+		array = kmod_weakdep_get_weak(l, &count);
>+		*weak = lookup_dep(mod->ctx, array, count);
>
> 		/*
> 		 * find only the first command, as modprobe from
>diff --git a/libkmod/libkmod.h b/libkmod/libkmod.h
>index 7251aa7..fce66d1 100644
>--- a/libkmod/libkmod.h
>+++ b/libkmod/libkmod.h
>@@ -112,6 +112,7 @@ struct kmod_config_iter *kmod_config_get_remove_commands(const struct kmod_ctx *
> struct kmod_config_iter *kmod_config_get_aliases(const struct kmod_ctx *ctx);
> struct kmod_config_iter *kmod_config_get_options(const struct kmod_ctx *ctx);
> struct kmod_config_iter *kmod_config_get_softdeps(const struct kmod_ctx *ctx);
>+struct kmod_config_iter *kmod_config_get_weakdeps(const struct kmod_ctx *ctx);
> const char *kmod_config_iter_get_key(const struct kmod_config_iter *iter);
> const char *kmod_config_iter_get_value(const struct kmod_config_iter *iter);
> bool kmod_config_iter_next(struct kmod_config_iter *iter);
>@@ -196,6 +197,8 @@ const char *kmod_module_get_remove_commands(const struct kmod_module *mod);
> struct kmod_list *kmod_module_get_dependencies(const struct kmod_module *mod);
> int kmod_module_get_softdeps(const struct kmod_module *mod,
> 				struct kmod_list **pre, struct kmod_list **post);
>+int kmod_module_get_weakdeps(const struct kmod_module *mod,
>+				struct kmod_list **weak);
> int kmod_module_get_filtered_blacklist(const struct kmod_ctx *ctx,
> 					const struct kmod_list *input,
> 					struct kmod_list **output) __attribute__ ((deprecated));
>diff --git a/libkmod/libkmod.sym b/libkmod/libkmod.sym
>index 0c04fda..0d6d338 100644
>--- a/libkmod/libkmod.sym
>+++ b/libkmod/libkmod.sym
>@@ -21,6 +21,7 @@ global:
> 	kmod_config_get_aliases;
> 	kmod_config_get_options;
> 	kmod_config_get_softdeps;
>+	kmod_config_get_weakdeps;
> 	kmod_config_iter_get_key;
> 	kmod_config_iter_get_value;
> 	kmod_config_iter_next;
>@@ -42,6 +43,7 @@ global:
>
> 	kmod_module_get_dependencies;
> 	kmod_module_get_softdeps;
>+	kmod_module_get_weakdeps;
> 	kmod_module_get_filtered_blacklist;
>
> 	kmod_module_get_name;
>diff --git a/man/modprobe.d.5.xml b/man/modprobe.d.5.xml
>index 2bf6537..cc90da6 100644
>--- a/man/modprobe.d.5.xml
>+++ b/man/modprobe.d.5.xml
>@@ -212,6 +212,30 @@
>           </para>
>         </listitem>
>       </varlistentry>
>+      <varlistentry>
>+        <term>weakdep <replaceable>modulename</replaceable> <replaceable>modules...</replaceable>
>+        </term>
>+        <listitem>
>+          <para>
>+            The <command>weakdep</command> command allows you to specify weak module
>+            dependencies. Those are similar to pre softdep, with the
>+            difference that userspace doesn't attempt to load that
>+            dependency before the specified module. Instead the kernel
>+            may request one or multiple of them during module probe,
>+            depending on the hardware it's binding to. The purpose of
>+            weak module is to allow a driver to specify that a certain
>+            dependency may be needed, so it should be present in the
>+            filesystem (e.g. in initramfs) when that module is probed.
>+          </para>
>+	  <para>
>+            Example: Assume "weakdep c a b". A program creating an
>+            initramfs knows it should add a, b, and c to the filesystem
>+            since a and b may be required/desired at runtime. When c is
>+            loaded and is being probed, it may issue calls to
>+            request_module() causing a or b to also be loaded.
>+          </para>
>+        </listitem>
>+      </varlistentry>
>     </variablelist>
>   </refsect1>
>   <refsect1><title>COMPATIBILITY</title>
>diff --git a/tools/depmod.c b/tools/depmod.c
>index 43fc354..06618fa 100644
>--- a/tools/depmod.c
>+++ b/tools/depmod.c

I'd keep this in a separate patch, but ok.

>@@ -2296,6 +2296,30 @@ static int output_softdeps(struct depmod *depmod, FILE *out)
> 	return 0;
> }
>
>+static int output_weakdeps(struct depmod *depmod, FILE *out)
>+{
>+	size_t i;
>+
>+	fputs("# Weak dependencies extracted from modules themselves.\n", out);
>+
>+	for (i = 0; i < depmod->modules.count; i++) {
>+		const struct mod *mod = depmod->modules.array[i];
>+		struct kmod_list *l;
>+
>+		kmod_list_foreach(l, mod->info_list) {
>+			const char *key = kmod_module_info_get_key(l);
>+			const char *value = kmod_module_info_get_value(l);
>+
>+			if (!streq(key, "weakdep"))
>+				continue;
>+
>+			fprintf(out, "weakdep %s %s\n", mod->modname, value);
>+		}
>+	}
>+
>+	return 0;
>+}
>+
> static int output_symbols(struct depmod *depmod, FILE *out)
> {
> 	struct hash_iter iter;
>@@ -2574,6 +2598,7 @@ static int depmod_output(struct depmod *depmod, FILE *out)
> 		{ "modules.alias", output_aliases },
> 		{ "modules.alias.bin", output_aliases_bin },
> 		{ "modules.softdep", output_softdeps },
>+		{ "modules.weakdep", output_weakdeps },
> 		{ "modules.symbols", output_symbols },
> 		{ "modules.symbols.bin", output_symbols_bin },
> 		{ "modules.builtin.bin", output_builtin_bin },
>diff --git a/tools/modprobe.c b/tools/modprobe.c
>index 5306bef..4328da6 100644
>--- a/tools/modprobe.c
>+++ b/tools/modprobe.c
>@@ -182,6 +182,7 @@ static int show_config(struct kmod_ctx *ctx)
> 		{ "alias", kmod_config_get_aliases },
> 		{ "options", kmod_config_get_options },
> 		{ "softdep", kmod_config_get_softdeps },
>+		{ "weakdep", kmod_config_get_weakdeps },

things look good here, thanks for doing this

I think next step would be to add some tests to our testsuite to make
sure we are parsing things correctly. Could you take a look in in
`git grep softdep` testsuite/ to take inspiration on how to write one
for weakdep? it seems we only have 1 test for a failure scenrario, but
we could add more too.

Tests I'm looking for:

1) make sure we don't load a module due to being a weakdep
2) make sure depmod outputs the weakdep correctly
3) make sure the weakdep is parsed correctly from the conf file

Anyway,

Reviewed-by: Lucas De Marchi <lucas.demarchi@intel.com>

and pushed.

thanks
Lucas De Marchi

> 	};
> 	size_t i;
>
>-- 
>2.44.0
>

Re: [PATCH v2 patch] libkmod: add weak dependecies

[Jose Ignacio Tornos Martinez]

>> kmod_config_add_weakdep ...
> this is a long function that we may eventually break in smaller pieces
> and share with the softdep parser.
>> weakdep_to_char ...
> ditto for this one
> I'd keep this in a separate patch, but ok.
> things look good here, thanks for doing this
Ok, let me have a look to get more modular functions in a later patch.

> I think next step would be to add some tests to our testsuite to make
> sure we are parsing things correctly. Could you take a look in in
> `git grep softdep` testsuite/ to take inspiration on how to write one
> for weakdep? it seems we only have 1 test for a failure scenrario, but
> we could add more too.
>
>Tests I'm looking for:
>
> 1) make sure we don't load a module due to being a weakdep
> 2) make sure depmod outputs the weakdep correctly
> 3) make sure the weakdep is parsed correctly from the conf file
Ok, I will complete with some tests as you comment in another later patch.
I will try to cover at least what you say.

Regarding the necessary kernel patch I will re-submit it, adding that
the weak procedure is already included in kmod.

Thank you

Best regards
José Ignacio