Bug 208607 - mkinitrd: iSCSI root requires crc32c module
mkinitrd: iSCSI root requires crc32c module
Status: CLOSED WONTFIX
Product: Fedora
Classification: Fedora
Component: module-init-tools (Show other bugs)
11
All Linux
medium Severity medium
: ---
: ---
Assigned To: Jon Masters
Fedora Extras Quality Assurance
bzcl34nup
: Reopened
Depends On:
Blocks: 210232 283781
  Show dependency treegraph
 
Reported: 2006-09-29 13:03 EDT by Mark McLoughlin
Modified: 2013-01-09 23:05 EST (History)
9 users (show)

See Also:
Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of:
Environment:
Last Closed: 2009-12-01 00:50:19 EST
Type: ---
Regression: ---
Mount Type: ---
Documentation: ---
CRM:
Verified Versions:
Category: ---
oVirt Team: ---
RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: ---


Attachments (Terms of Use)
mkinitrd-crc32c-for-iscsi.patch (434 bytes, patch)
2006-09-29 13:03 EDT, Mark McLoughlin
no flags Details | Diff

  None (edit)
Description Mark McLoughlin 2006-09-29 13:03:38 EDT
Because of this in linux-2.6-iscsi-update-to-2-6-19-rc1.upstream.patch

+       tcp_conn->tx_tfm = crypto_alloc_tfm("crc32c", 0);
+       if (!tcp_conn->tx_tfm)
+               goto free_tcp_conn;
+
+       tcp_conn->rx_tfm = crypto_alloc_tfm("crc32c", 0);
+       if (!tcp_conn->rx_tfm)
+               goto free_tx_tfm;

we now need the crc32c module on the initrd for iSCSI root.

Attaching a simple hack of a fix for mkinitrd, but maybe we can make the module
dep explicit in iscsi_tcp.c or something ...
Comment 1 Mark McLoughlin 2006-09-29 13:03:38 EDT
Created attachment 137405 [details]
mkinitrd-crc32c-for-iscsi.patch
Comment 2 Jeremy Katz 2006-09-29 13:15:51 EDT
... I'd far rather fix the module deps.  Otherwise, it's going to break for
anaconda also unless we've just been lucky enough to load crc32 already for
something else.
Comment 3 Mike Christie 2006-09-29 13:23:28 EDT
(In reply to comment #0)
> Because of this in linux-2.6-iscsi-update-to-2-6-19-rc1.upstream.patch
> 
> +       tcp_conn->tx_tfm = crypto_alloc_tfm("crc32c", 0);
> +       if (!tcp_conn->tx_tfm)
> +               goto free_tcp_conn;
> +
> +       tcp_conn->rx_tfm = crypto_alloc_tfm("crc32c", 0);
> +       if (!tcp_conn->rx_tfm)
> +               goto free_tx_tfm;
> 
> we now need the crc32c module on the initrd for iSCSI root.
> 
> Attaching a simple hack of a fix for mkinitrd, but maybe we can make the module
> dep explicit in iscsi_tcp.c or something ...


I am not sure how to do that in the iscsi code. I do not think there is a way.
As you know from above the iscsi module can only interfact with the crypto
interface through crypto_* calls and what module is being used is abstracted
behind the crypto_tfm struct so the iscsi module never directly touches the crc
module. It always goes through some crypto call which then references some
function pointer.


Comment 4 Mike Christie 2006-09-29 13:25:35 EDT
Is it acceptable to just build the crc32c module into the kernel instead of as a
module?
Comment 5 Mark McLoughlin 2006-10-02 09:35:09 EDT
Right, there doesn't seem to be a way to make the module dep explicit.

Building crc32c into the kernel should fix the problem. And we already build
e.g. sha1 into the kernel.
Comment 6 Mike Christie 2006-10-02 19:08:42 EDT
Jeremy, Jeff and Dave NAKd the idea of compiling it in the kernel. Is Mark's
patch ok then or do you think a common solution to this type of problem can be
found. As you know from mkinitrd there are other examples that could benefit if
there was some magic way to hook things in. I just don't know how?
Comment 7 Mike Christie 2006-10-02 19:09:46 EDT
Nevermind. I see you are on rh-kernel.
Comment 8 Mark McLoughlin 2006-10-05 06:19:38 EDT
Doesn't look like building the module into the kernel is going to happen leaving
short term options of:

  1) Add the mkinitrd hack or 
  2) Tell people to use mkinitrd --with crc32c

Comment 9 Mark McLoughlin 2006-10-05 06:26:14 EDT
Longer term options discussed include:

  1) Suck the information out of the Kconfig tree and make it available to 
     mkinitrd somehow (i.e. CONFIG_ISCSI_TCP auto-selects CONFIG_CRYPTO_CRC32C)

  2) Add MODULE_MAYUSE() which iscsi_tcp would use to indicate that it may
     load crc32c - depmod would need to stick this info somewhere
Comment 10 Mark McLoughlin 2006-10-07 05:39:13 EDT
Whadda you know? crc32c is built-in from kernel-1.2744.fc6
Comment 11 Dave Jones 2006-10-11 20:03:36 EDT
Note that this is a quick-fix just so that we can get FC6 out the door.
As soon as we ship this, I'm flipping it back to be a module again, so this
problem needs to be fixed properly eventually.
Comment 12 Kostas Georgiou 2007-09-08 21:32:37 EDT
Can we reopen this bug? Since crc32c in now a module iSCSI root is broken again.
Comment 13 Chuck Ebbert 2007-09-18 12:54:34 EDT
We'll probably need to make this built-in again?
Comment 14 Jeremy Katz 2007-09-18 13:12:38 EDT
I don't think we're going to get any quick fix :-/  But maybe jcm wants to
champion something upstream so that we can get out of doing this every release ;)
Comment 15 Dave Jones 2007-09-25 12:24:38 EDT
I'm really against flip-flopping this to built-in just before every release.
It takes away all the motivation for someone to actually step up and fix it.
A year has passed, and the situation hasn't changed at all.

My stance on this is fix it properly, or ship with busted iscsi support.
Comment 16 Jeremy Katz 2007-10-04 11:55:00 EDT
(In reply to comment #15)
> I'm really against flip-flopping this to built-in just before every release.
> It takes away all the motivation for someone to actually step up and fix it.
> A year has passed, and the situation hasn't changed at all.
>
> My stance on this is fix it properly, or ship with busted iscsi support.

Leaving it broken helps who exactly?  To me, it just makes it feel like nobody
cares.  And fixing it in every place we ever do anything with module loads is
entirely busted and not at all sustainable.  If you want to get the situation
improved, fine.  Fix it then.  Don't screw our users.
Comment 17 Dave Jones 2007-10-12 19:37:14 EDT
"To me, it just makes it feel like nobody cares. "

I think you hit the nail on the head.
Comment 18 Jon Masters 2007-10-12 21:54:56 EDT
I think we could do with a MODULE_MAYUSE type approach upstream, in order to
give a give depmod something to chew on. As it is, 2.6 module versioning doesn't
have a nice solution to this...and we need one because there are a bunch of
other modules with quasi-circular deps too. But that isn't exactly easily
solved. This first one is at least a bit easier - I can give modules an option
to add a forced dep.

Let me think about it...meantime you need a custom mkinitrd hack.

Jon.
Comment 19 Jeremy Katz 2007-10-28 15:45:50 EDT
(In reply to comment #18)
> Let me think about it...meantime you need a custom mkinitrd hack.

Just a "custom mkinitrd hack" doesn't cut it.  Come on.  We're talking about
[katzj@aglarond F-8]$ cvs -q diff -u 
diff -u -u -r1.33 config-generic
--- config-generic      25 Oct 2007 20:52:52 -0000      1.33
+++ config-generic      28 Oct 2007 19:42:50 -0000
@@ -2938,7 +2938,7 @@
-CONFIG_CRYPTO_CRC32C=m
+CONFIG_CRYPTO_CRC32C=y


vs having to go in and touch a fair bit of code in anaconda and mkinitrd that
isn't entirely obvious or straight-forward.  And given that we already build in
some of the crypto algorithms, any argument about bloat are pretty much invented
as far as I'm concerned.

Longer term, MODULE_MAYUSE sounds like it's the right answer and we should get
that pushed forward and then we can add support throughout our stack of stuff to
work properly when it is, but that's not going to happen by Tuesday.
Comment 20 Jon Masters 2007-10-28 17:06:47 EDT
You're right Jeremy. For F8, this is the sanest thing to do. Dave, Chuck?

Jon.
Comment 21 Dave Jones 2007-10-29 13:29:30 EDT
Done for F8.  Hopefully we'll have MAYUSE in place for F9.
I'll leave this open for tracking.
Comment 22 Kostas Georgiou 2007-10-29 13:44:14 EDT
Does this include kernel-xen as well or should we open a new bug for it?
Comment 23 Jeremy Katz 2007-10-29 13:57:37 EDT
Removing from blocker with the fix in the kernel.  And yes, a separate change is
needed for kernel-xen
Comment 24 Bug Zapper 2008-04-03 14:21:43 EDT
Based on the date this bug was created, it appears to have been reported
against rawhide during the development of a Fedora release that is no
longer maintained. In order to refocus our efforts as a project we are
flagging all of the open bugs for releases which are no longer
maintained. If this bug remains in NEEDINFO thirty (30) days from now,
we will automatically close it.

If you can reproduce this bug in a maintained Fedora version (7, 8, or
rawhide), please change this bug to the respective version and change
the status to ASSIGNED. (If you're unable to change the bug's version
or status, add a comment to the bug and someone will change it for you.)

Thanks for your help, and we apologize again that we haven't handled
these issues to this point.

The process we're following is outlined here:
http://fedoraproject.org/wiki/BugZappers/F9CleanUp

We will be following the process here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping to ensure this
doesn't happen again.
Comment 25 Kostas Georgiou 2008-04-03 16:09:30 EDT
It should stay open (comment #21).
Comment 26 Bug Zapper 2008-05-13 22:23:11 EDT
Changing version to '9' as part of upcoming Fedora 9 GA.
More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 27 Eric Sandeen 2009-01-23 13:02:42 EST
This hits btrfs too, now.

I may just bail on the fancy solutions and go ahead & build crc32c into the kernel.
Comment 28 Eric Sandeen 2009-01-23 14:50:34 EST
I've committed the config change to rawhide.
Comment 29 Eric Sandeen 2009-01-23 14:52:51 EST
(BTW: Despite the protestations above, I went ahead & built it in both as a quick fix, and because we seem to be on a track to build in more anyway, for fast-boot reasons.  Still, this should be fixed in general!  So apologies for hiding the problem, again.)
Comment 30 Jon Masters 2009-03-13 12:19:18 EDT
MENTAL NOTE: still a need for additional module dependency tracking
Comment 31 Jesse Keating 2009-04-13 20:21:44 EDT
This should be fixed for F11, I'm dropping it from the blocker, but leaving the bug open for somebody a bit more knowledgeable of the situation to close or whatever.
Comment 32 Jeremy Katz 2009-04-13 22:35:14 EDT
We still need the functionality, it's just less of a concern with this specific case since we're building it in.  The fact that this still hasn't been implemented 2.5 years later is kind of sad, though :(
Comment 33 Bug Zapper 2009-06-09 05:10:16 EDT
This bug appears to have been reported against 'rawhide' during the Fedora 11 development cycle.
Changing version to '11'.

More information and reason for this action is here:
http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Comment 34 Jon Masters 2009-12-01 00:50:19 EST
We have softdep support being worked on upstream, and I am going to close this bug because it has been addressed with a workaround previously in Fedora. Expect to see softdep support upstream providing for this kind of case in the future.

Note You need to log in before you can comment on or make changes to this bug.