Bug 208607

Created attachment 137405 [details]
mkinitrd-crc32c-for-iscsi.patch

... 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.

(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.

Is it acceptable to just build the crc32c module into the kernel instead of as a
module?

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.

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?

Nevermind. I see you are on rh-kernel.

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

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

Whadda you know? crc32c is built-in from kernel-1.2744.fc6

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.

Can we reopen this bug? Since crc32c in now a module iSCSI root is broken again.

We'll probably need to make this built-in again?

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 ;)

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.

(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.

"To me, it just makes it feel like nobody cares. "

I think you hit the nail on the head.

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.

(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.

You're right Jeremy. For F8, this is the sanest thing to do. Dave, Chuck?

Jon.

Done for F8.  Hopefully we'll have MAYUSE in place for F9.
I'll leave this open for tracking.

Does this include kernel-xen as well or should we open a new bug for it?

Removing from blocker with the fix in the kernel.  And yes, a separate change is
needed for kernel-xen

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.

It should stay open (comment #21).

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

This hits btrfs too, now.

I may just bail on the fancy solutions and go ahead & build crc32c into the kernel.

I've committed the config change to rawhide.

(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.)

MENTAL NOTE: still a need for additional module dependency tracking

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.

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 :(

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

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.