[PATCH v1] initrd: Allow extra search paths with ‘initrd-extra-module-paths’

This patch allows copying of out-of-tree kernel modules to the Linux

initrd.

For out-of-tree modules to found, an extra slot has been added to

‘operating-system’: ‘initrd-extra-module-paths’, which contains a list

containing items of either FILE-LIKE or (FILE-LIKE OUTPUT) which will be

searched for modules in addition to the standard Linux module path. The

required modules can then be added to ‘initrd-modules’ as normal and all paths

will be searched for it, including for any modules depended on.

DIRECTORIES. Now takes a list of directories to search, rather than a single

one.

field and accessor. Takes a list of file-like objects.

argument to PACKAGES. Now contains a list of file-likes to search for modules.

(raw-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it

to (flat-linux-extra-module-paths) along with the selected LINUX package.

(base-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it

to (raw-initrd).

---

gnu/build/linux-modules.scm | 19 ++++++++------

gnu/system.scm | 5 ++++

gnu/system/linux-initrd.scm | 52 +++++++++++++++++++++++++------------

3 files changed, 51 insertions(+), 25 deletions(-)

Brian Cully via Guix-patches via schreef op ma 02-05-2022 om 15:11 [-

0400]:

This needs some information in the manual -- when does the field need

to be set? For what kernel modules? Is this required by v4l2loopback-

linux-module and librem-ec-acpi-linux-module? Things like that.

As-is, this functionality is hard to discover.

Greetings,

Maxime.

Maxime Devos <maximedevos@telenet.be> writes:

I knew I missed something. How’s this look?

---

doc/guix.texi | 42 ++++++++++++++++++++++++++++++++++--------

1 file changed, 34 insertions(+), 8 deletions(-)

I've read through the original patch and the doc patch, and based on my own tinkering with guix initrd generation, the patches look good to me. I'm also happy to see the functionality being added, and already have plans for using it with zfs (to further the goal of an encrypted multi-disk zfs root).

Cheers,

Kaelyn

(for whatever reason, looks like this didn't end up in my inbox)

As-is, I don't think this is a good example, because

'expression->initrd' does not set #:substitutable? #false,

the 'zfs' package has the comment (*)

`(;; The ZFS kernel module should not be downloaded since the

license

;; terms don't allow for distributing it, only building it

locally.

#:substitutable? #f [...])

and IIUC, the code inside expression->initrd copies the kernel module

into

the new store item, so it looks like this accidentally suggest people

to

commit copyvios, and copyvios are currently against the law.

Greetings,

Maxime.

(*) Though I don't understand that comment: Guix is a distribution, so

by definition

it's distributing zfs -- unless it's only talking about the binaries

and not the

source code ...

This patch allows copying of out-of-tree kernel modules to the Linux

initrd.

For out-of-tree modules to found, an extra slot has been added to

‘operating-system’: ‘initrd-extra-module-paths’, which contains a list

containing items of either FILE-LIKE or (FILE-LIKE OUTPUT) which will be

searched for modules in addition to the standard Linux module path. The

required modules can then be added to ‘initrd-modules’ as normal and all paths

will be searched for it, including for any modules depended on.

DIRECTORIES. Now takes a list of directories to search, rather than a single

one.

field and accessor. Takes a list of file-like objects.

argument to PACKAGES. Now contains a list of file-likes to search for modules.

(raw-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it

to (flat-linux-extra-module-paths) along with the selected LINUX package.

(base-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it

to (raw-initrd).

---

This version includes both the previous patch and the documentation.

doc/guix.texi | 42 ++++++++++++++++++++++++------

gnu/build/linux-modules.scm | 19 ++++++++------

gnu/system.scm | 5 ++++

gnu/system/linux-initrd.scm | 52 +++++++++++++++++++++++++------------

4 files changed, 85 insertions(+), 33 deletions(-)

As the earlier patch discussion seemed to have focused on ZFS licensing issues that are largely orothogonal to the functionality being added, I'd like to again voice my support for the proposed patch. While ZFS is indeed a bad example due to open questions about module licensing and redistribution, it isn't the only out-of-tree module currently packaged in Guix which could be used with the extra initrd support (not counting future packages or external channels). Within the guix repo, the packages using the `linux-module-build-system` aside from zfs are:

- acpi-call-linux-module

- bbswitch-module

- ddcci-driver-linux

- rtl8812au-aircrack-ng-linux-module

- rtl8821ce-linux-module

- ttyebus-linux-module

- v4l2loopback-linux-module

- vhba-module

- wiregard-linux-compat

Of those nine, at least four look to be for specific hardware, and I can come up with theoretical use cases for why someone might want those or wiregard-linux-compat (though the cases are definitely contrived straw-men that may or may not be implementable in practice or actually useful in an initrd context, such as network booting with an older kernel and the root filesystem can only be accessed over a wireguard connection).

Cheers,

Kaelyn

Hello Brian,

Brian Cully <bjc@spork.org> skribis:

Nice, it looks like a welcome addition.

I wonder if we could reuse the ‘kernel-loadable-modules’ field for this

purpose instead of introducing a new field. We’d need to pass it to the

initrd procedures and have them search in there in addition to the

kernel package, pretty much like this patch already does actually.

WDYT?

Nitpick: the GNU convention is to use “path” to denote “search paths”,

and other “file”, “file name”, or similar. In this case, that’d be

“kernel module” or “Linux module”.

Thank you!

Ludo’.

Ludovic Courtès <ludo@gnu.org> writes:

This sounds like it could be made to work as you suggest. My

feeling is that the two contexts are slightly different, though,

as the Linux modules are a superset of the initrd modules, so I’d

prefer not to mix them as it might be confusing to people who are

used to other Linux distros where the initrd modules are called

out separately. I admit I’m probably being silly here, and don’t

have any serious objection in principle.

I struggled with this a fair amount, actually. What these

file-likes actually represent is an element of a search path, even

if they come in the odd form of a file-like object, which is why I

used ‘path’. ‘file’ seems wrong, as it implies (to me) that it’s

the ‘initrd-extra-module-files’ option itself that would include

the module, rather than the ‘initrd-modules’ option.

Of course, all this goes away if we just reuse

‘kernel-loadable-modules’ as an additional input, rather than

adding another option, so that’s a distinct mark in favor of doing

that.

When I get some time (hopefully soon!) I’ll try to thread

‘kernel-loadable-modules’ through instead and see how far I can

get with that approach. Do you think the documentation for it will

need to be updated to specify that it’s also used as a search path

for initrd building? Or maybe the better option is to update the

documentation for ‘initrd-modules’ to say that it uses

‘kernel-loadable-modules’ as input?

-bjc

Hi,

My $0.02 on 'initrd-extra-module-paths' vs 'kernel-loadable-modules': if the initrd code is smart enough to not include the entire packages and only includes the requested modules from the packages (which I think is already true based on behavior observed some time ago), then not having to duplicate the list would be preferable.

I found this issue in the tracker after being surprised that the initrd builder ignored 'kernel-loadable-modules' with 'initrd-modules' only working for modules that are included in the primary linux kernel package. That surprise over the incompatibility between 'initrd-modules' and 'kernel-loadable-modules' is also why I've been relatively vocal about the patch. ;)

Cheers,

Kaelyn

Hi,

Brian Cully <bjc@spork.org> skribis:

Right. Here, we want the initrd machinery to search for modules in any

place where they can be found, which is either ‘kernel’ or

‘kernel-loadable-modules’.

One could define ‘initrd-extra-module-paths’ to be different from

‘kernel-loadable-modules’, for example if you can be sure the module

will be loaded from the initrd and not after, but in general both are

likely to have the same value, no?

Hmm right, you have a point! :-)

I think you should update the documentation in the commit that changes

things, so that the patch is self-contained.

It may be enough to state in the documentation of the ‘initrd-modules’

field that its value is a list of module names that are searched for in

‘kernel’ and ‘kernel-loadable-modules’.

WDYT?

Thanks,

Ludo’.

With this patch, modules for ‘initrd-modules’ will not only be searched for in

the in-tree Linux modules, but also any additional modules specified in

‘kernel-loadable-modules’.

DIRECTORIES. Now takes a list of directories to search, rather than a single

one.

argument to PACKAGES. Now contains a list of file-likes to search for modules.

(raw-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it

to (flat-linux-extra-module-paths) along with the selected LINUX package.

(base-initrd): Add LINUX-EXTRA-MODULE-PATHS keyword argument. Pass it

to (raw-initrd).

definition's kernel-loadable-modules into (make-initrd) as

LINUX-EXTRA-MODULE-PATHS.

---

I've removed the new operating-system slot in preference to re-using

kernel-loadable-modules, as discussed.

I did leave the old argument names for the lower level routines in place,

as I feel that they more accurately represent how the data are being

used.

I've also pulled out the documentation. Once this patch set is beaten

into acceptability, the documentation can be adjusted if that's deemed

necessary.

gnu/build/linux-modules.scm | 19 ++++++++------

gnu/system.scm | 2 ++

gnu/system/linux-initrd.scm | 52 +++++++++++++++++++++++++------------

3 files changed, 48 insertions(+), 25 deletions(-)

Hi Brian,

Brian Cully <bjc@spork.org> skribis:

Alright!

It looks great to me. I have two cosmetic comments:

I think you can remove this definition and use ‘gexp-input’ directly.

Nitpick: I’d call it #:linux-extra-module-path (singular, as in “search

path”) or simply #:linux-extra-module-directories.

If you could introduce a couple of lines in doc/guix.texi to explain

where modules are searched for, that’d be perfect.

Thanks for taking the time to prepare this revision!

Ludo’.

With this patch, modules for ‘initrd-modules’ will not only be searched for in

the in-tree Linux modules, but also any additional modules specified in

‘kernel-loadable-modules’.

DIRECTORIES. Now takes a list of directories to search, rather than a single

one.

argument to PACKAGES. Now contains a list of file-likes to search for modules.

(raw-initrd): Add LINUX-EXTRA-MODULE-DIRECTORIES keyword argument. Pass it

to (flat-linux-module-directory) along with the selected LINUX package.

(base-initrd): Add LINUX-EXTRA-MODULE-DIRECTORIES keyword argument. Pass it

to (raw-initrd).

definition's kernel-loadable-modules into (make-initrd) as

LINUX-EXTRA-MODULE-DIRECTORIES.

---

This version removes the redundant (package+out->input) procedure, and

renames ‘linux-extra-module-paths’ to ‘linux-extra-module-directories’.

gnu/build/linux-modules.scm | 19 ++++++++------

gnu/system.scm | 2 ++

gnu/system/linux-initrd.scm | 49 ++++++++++++++++++++++++-------------

3 files changed, 45 insertions(+), 25 deletions(-)

---

doc/guix.texi | 13 +++++++++++++

1 file changed, 13 insertions(+)

Brian Cully via Guix-patches via schreef op za 18-06-2022 om 15:11 [-

0400]:

As written previously, this is not a good example, because:

and because the defense for not considering the ZFS license to be a

problem consists of the ZFS module not being distributed in binary

form, whereas this suggestion would in some situations cause it to be

distributed in binary form.

Greetings,

Maxime.

If you have another example, I'll put it in instead. I used this

one, because I know this one works, and that's all.

That said, I'm not sure how this would cause the module to be

distributed as a binary. In order for it to be added to the

initrd, it will still need to built using the DKMS mechanism, and

thus compiled on (or at least for) the target Guix installation.

If the complaint is that one could generate a USB stick, or some

such, with ZFS in the initrd, then yes, that's possible. But

that's also possible by using the existing

‘kernel-loadable-modules’ mechanism to generate an image with

‘guix system image’ and distributing that. I don't think it's our

job to try and prevent such things, since, even if desirable, it's

not really feasible.

If the complaint is merely that it's in the documentation, then

ok, I'll change it to whatever module you want.

-bjc

Brian Cully schreef op za 18-06-2022 om 16:43 [-0400]:

Because #:substitutable? isn't set appropriately in dependents,

substitute servers exist, ZFS can be used on substitute servers and

someone using the substitute server might have a sufficiently similar

system configuration to do substitution of a store item containing a

copy of the ZFS binary.

Greetings,

Maxime.

Maxime Devos <maximedevos@telenet.be> writes:

So, simply using ZFS in ‘kernel-loadable-modules’ would be enough

to trigger this misbehavior? That sounds like a pretty serious

issue. Would it be possible to have the substitute servers filter

on the #:substitutable flag?

This is getting out of scope, though. I don't really want this

patch to go in without /some/ documentation regarding how modules

get looked up, so if anyone has an alternate module and use-case,

I'll just swap it in for the ZFS one.

-bjc

Brian Cully schreef op za 18-06-2022 om 19:11 [-0400]:

That's exactly what the #:substitutable flag is for (IIUC)!

However, the problem is that currently, that flag isn't set for

derivations that make a copy of the ZFS module. As I wrote previously:

I don't have any.

Greetings,

Maxime.

Hi again,

Regarding the documentation example, one (contrived) alternative is:

"""

For example, if you need the driver for a Realtek RTL8821CE wireless network adapter for mounting the root filesystem over NFS, your configuration might include the following:

@lisp

(operating-system

;; @dots{}

(initrd-modules (cons "8821ce" %base-initrd-modules))

(kernel-loadable-modules (list (list rtl8821ce-linux-module "module"))))

@end lisp

"""

While I don't have the hardware, I did verify the kernel module name by building the rtl8821ce-linux-module package.

Cheers,

Kaelyn

With this patch, modules for ‘initrd-modules’ will not only be searched for in

the in-tree Linux modules, but also any additional modules specified in

‘kernel-loadable-modules’.

DIRECTORIES. Now takes a list of directories to search, rather than a single

one.

argument to PACKAGES. Now contains a list of file-likes to search for modules.

(raw-initrd): Add LINUX-EXTRA-MODULE-DIRECTORIES keyword argument. Pass it

to (flat-linux-module-directory) along with the selected LINUX package.

(base-initrd): Add LINUX-EXTRA-MODULE-DIRECTORIES keyword argument. Pass it

to (raw-initrd).

definition's kernel-loadable-modules into (make-initrd) as

LINUX-EXTRA-MODULE-DIRECTORIES.

---

gnu/build/linux-modules.scm | 19 ++++++++------

gnu/system.scm | 2 ++

gnu/system/linux-initrd.scm | 49 ++++++++++++++++++++++++-------------

3 files changed, 45 insertions(+), 25 deletions(-)

---

doc/guix.texi | 15 +++++++++++++++

1 file changed, 15 insertions(+)

Hi,

I just want to say that the latest v4 of the patches look good to me, and has removed the example with the questionable licensing situation. I have also been using this patch quite successfully, in a slightly modified version (mostly module name changes) imported into a local channel, for almost 2 years now.

Also, if needed, I would be happy to rebase the patches against the current master.

Cheers,

Kaelyn