crypto: libcrc32c should select crc32c

crypto: libcrc32c should select crc32c

[Jan Engelhardt]

parent 683fd3522476a2b9e9278d99a7bb367c887faa5a (v2.6.29-rc2-19-g683fd35)
commit db86cb2732110f756b95f7d5a2aa37f35ba67cff
Author: Jan Engelhardt <jengelh@medozas.de>
Date:   Sat Jan 17 20:40:45 2009 +0100

crypto: libcrc32c should select crc32c

Whilst trying to make use of btrfs of a root fs with 2.6.29-rc1/rc2,
I get the following output from my initramfs:

	[...]
	Mounting root /dev/sda3
	modprobe: WARNING: Error inserting libcrc32c
	(/lib/modules/2.6.29-rc2-jen73-experimental/kernel/lib/libcrc32c.ko.gz):
	Unknown symbol in module, or unknown parameter (see dmesg)

	<4>btrfs: Unknown symbol crc32c
	modprobe: FATAL: Error inserting btrfs
	(/lib/modules/2.6.29-rc2-jen73-experimental/kernel/fs/btrfs/btrfs.ko.gz):
	Unknown symbol in module, or unknown parameter (see dmesg)

	mount: unknown filesystem type 'btrfs'

Since "unknown symbol crc32c" is the only kernel-reported message,
just what keeps libcrc32c from loading -- especially because it does
not depend on any other symbol from another kernel module according
to modinfo.

Looking at libcrc32c.c shows that it essentially depends on the
crc32c crypto module, which was not packed into my initramfs image
by mkinitrd because.. there is no dependency.

Because a simple Kconfig "select" will not make mkinitrd aware of the
dependency, this patch adds a source-level dep similar to what
Netfilter does with need_conntrack().

Signed-off-by: Jan Engelhardt <jengelh@medozas.de>
---
 crypto/crc32c.c        |    6 ++++++
 include/linux/crc32c.h |    1 +
 lib/libcrc32c.c        |    2 ++
 3 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/crypto/crc32c.c b/crypto/crc32c.c
index 973bc2c..b3dd3ec 100644
--- a/crypto/crc32c.c
+++ b/crypto/crc32c.c
@@ -242,6 +242,12 @@ static struct shash_alg alg = {
 	}
 };
 
+/* Some modules need us, but do not depend directly on any symbol. */
+void need_crc32c(void)
+{
+}
+EXPORT_SYMBOL_GPL(need_crc32c);
+
 static int __init crc32c_mod_init(void)
 {
 	return crypto_register_shash(&alg);
diff --git a/include/linux/crc32c.h b/include/linux/crc32c.h
index bd8b44d..139416a 100644
--- a/include/linux/crc32c.h
+++ b/include/linux/crc32c.h
@@ -3,6 +3,7 @@
 
 #include <linux/types.h>
 
+extern void need_crc32c(void);
 extern u32 crc32c(u32 crc, const void *address, unsigned int length);
 
 /* This macro exists for backwards-compatibility. */
diff --git a/lib/libcrc32c.c b/lib/libcrc32c.c
index 244f548..948f50a 100644
--- a/lib/libcrc32c.c
+++ b/lib/libcrc32c.c
@@ -32,6 +32,7 @@
  */
 
 #include <crypto/hash.h>
+#include <linux/crc32c.h>
 #include <linux/err.h>
 #include <linux/init.h>
 #include <linux/kernel.h>
@@ -61,6 +62,7 @@ EXPORT_SYMBOL(crc32c);
 
 static int __init libcrc32c_mod_init(void)
 {
+	need_crc32c();
 	tfm = crypto_alloc_shash("crc32c", 0, 0);
 	if (IS_ERR(tfm))
 		return PTR_ERR(tfm);
-- 
# Created with git-export-patch

Re: crypto: libcrc32c should select crc32c

[Herbert Xu]

On Sat, Jan 17, 2009 at 11:48:42PM +0100, Jan Engelhardt wrote:
>
> Looking at libcrc32c.c shows that it essentially depends on the
> crc32c crypto module, which was not packed into my initramfs image
> by mkinitrd because.. there is no dependency.

Actually the whole point of doing the crc32c/libcrc32c reversal
was to allow multiple providers of crc32c.  As it stands we have
a generic C version plus an Intel version.

So by applying yuor patch we'll go back to always using the C
version which is unacceptable.

I think a better way of tackling this is to note this information
explicitly in the module.  For example, just like module aliases
we can add explicit module dependencies.  mkinitrd can then use
this for its computation.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: crypto: libcrc32c should select crc32c

[Jan Engelhardt]

On Sunday 2009-01-18 06:37, Herbert Xu wrote:
>On Sat, Jan 17, 2009 at 11:48:42PM +0100, Jan Engelhardt wrote:
>>
>> Looking at libcrc32c.c shows that it essentially depends on the
>> crc32c crypto module, which was not packed into my initramfs image
>> by mkinitrd because.. there is no dependency.
>
>Actually the whole point of doing the crc32c/libcrc32c reversal
>was to allow multiple providers of crc32c.  As it stands we have
>a generic C version plus an Intel version.
>
>So by applying yuor patch we'll go back to always using the C
>version which is unacceptable.
>
>I think a better way of tackling this is to note this information
>explicitly in the module.  For example, just like module aliases
>we can add explicit module dependencies.

Can we?

I was already thinking about it.. the Solaris kernel has
its "_depends_on" variable for such things

	char _depends_on[] = "crc32c";

kbuild has something similar in its .mod.c files:

	static const char __module_depends[]
	__used
	__attribute__((section(".modinfo"))) =
	"depends=crc32c";

But in kbuild, this functionality does not seem exported
to me as a macro (maybe MODULE_DEPENDS?).

Re: crypto: libcrc32c should select crc32c

[Sam Ravnborg]

On Sun, Jan 18, 2009 at 09:14:16AM +0100, Jan Engelhardt wrote:
> 
> On Sunday 2009-01-18 06:37, Herbert Xu wrote:
> >On Sat, Jan 17, 2009 at 11:48:42PM +0100, Jan Engelhardt wrote:
> >>
> >> Looking at libcrc32c.c shows that it essentially depends on the
> >> crc32c crypto module, which was not packed into my initramfs image
> >> by mkinitrd because.. there is no dependency.
> >
> >Actually the whole point of doing the crc32c/libcrc32c reversal
> >was to allow multiple providers of crc32c.  As it stands we have
> >a generic C version plus an Intel version.
> >
> >So by applying yuor patch we'll go back to always using the C
> >version which is unacceptable.
> >
> >I think a better way of tackling this is to note this information
> >explicitly in the module.  For example, just like module aliases
> >we can add explicit module dependencies.
> 
> Can we?
> 
> I was already thinking about it.. the Solaris kernel has
> its "_depends_on" variable for such things
> 
> 	char _depends_on[] = "crc32c";
> 
> kbuild has something similar in its .mod.c files:
> 
> 	static const char __module_depends[]
> 	__used
> 	__attribute__((section(".modinfo"))) =
> 	"depends=crc32c";
> 
> But in kbuild, this functionality does not seem exported
> to me as a macro (maybe MODULE_DEPENDS?).

kbuild finds out during modpost what modules depends on what other modules
and this is used for the dependencies.
There is no way to control this manually today.

	Sam

Re: crypto: libcrc32c should select crc32c

[Jan Engelhardt]

On Sunday 2009-01-18 21:45, Sam Ravnborg wrote:
>On Sun, Jan 18, 2009 at 09:14:16AM +0100, Jan Engelhardt wrote:
>> On Sunday 2009-01-18 06:37, Herbert Xu wrote:
>> >On Sat, Jan 17, 2009 at 11:48:42PM +0100, Jan Engelhardt wrote:
>> >>
>> >> Looking at libcrc32c.c shows that it essentially depends on the
>> >> crc32c crypto module, which was not packed into my initramfs image
>> >> by mkinitrd because.. there is no dependency.
>> >
>> >Actually the whole point of doing the crc32c/libcrc32c reversal
>> >was to allow multiple providers of crc32c.  As it stands we have
>> >a generic C version plus an Intel version.[...]
>> >I think a better way of tackling this is to note this information
>> >explicitly in the module.  For example, just like module aliases
>> >we can add explicit module dependencies.
>> 
>> Can we?[...]
>> But in kbuild, this functionality does not seem exported
>> to me as a macro (maybe MODULE_DEPENDS?).
>
>kbuild finds out during modpost what modules depends on what other modules
>and this is used for the dependencies.
>There is no way to control this manually today.

So what will the crypto subsystem try to do then, implementation-wise,
given that kbuild cannot be manually tuned this way?

Re: crypto: libcrc32c should select crc32c

[Herbert Xu]

On Mon, Jan 19, 2009 at 08:51:49PM +0100, Jan Engelhardt wrote:
>
> So what will the crypto subsystem try to do then, implementation-wise,
> given that kbuild cannot be manually tuned this way?

Well if nobody wants to do the work to get the explcit dependencies
than I suggest that you hardcode this into your mkinitrd.

The patch you gave initially is unacceptable because it means
that most people will be stuck with the unoptimised version.

Cheers,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

RFC 4868 support for Linux IPsec stack

[Tarun Ahuja]

Does anybody know the status of support for RFC 4868 (SHA256/384/512 
with iv length of 16/24/32) to
Linux IPsec stack?sending mail as text I believe, as of kernel 2.6.28, 
only SHA256 is supported and that
too only 12 bytes for the icv (instead of 16).

-Tarun