Hallo,
Currently there are two twofish and two aes implementions on x86.
Worse when both are enabled as modules a modprobe aes
will get the C version which seems to be slower on K8
and about the same speed on Core2 on my tests.
Is there a specific reason why anybody would prefer the C functions
over the assembler functions?
Possible reasons I could think of:
- If the assembler functions are optimized for a specific CPU the compiler
might be able to do a better job on other CPUs.
Is there evidence for this? I suspect it's not true.
- They are not trusted and might be buggy. I assume they have
been validated against the C versions with a wide range of input
data, correct?
If none of these reasons are valid it might make sense to disable
the C versions for x86 and only offer the assembler versions.
Then modprobe aes|twofish would DTRT automatically.
Comments?
-Andi
Andi Kleen <[email protected]> wrote:
> Hallo,
>
> Currently there are two twofish and two aes implementions on x86.
> Worse when both are enabled as modules a modprobe aes
> will get the C version which seems to be slower on K8
> and about the same speed on Core2 on my tests.
Hi Andi:
Are you sure you get the C version when both are built-in
or loaded as modules? If so then we have a bug in the priority
code.
If you only have the C version loaded then that is understandable
since the module loading code doesn't know about the other one.
> Is there a specific reason why anybody would prefer the C functions
> over the assembler functions?
We don't, but the system is meant to allow multiple
implementations to coexist and picking the best one
at run-time.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> Are you sure you get the C version when both are built-in
> or loaded as modules? If so then we have a bug in the priority
> code.
The usual use case is: Somebody -- either admin or some command
implicitely -- executes modprobe aes because some text file
specifies the aes cipher. At least on my system that loads
the C version when both are enabled. modprobe will not load
multiple modules in this case.
I don't think modprobe knows anything about these priorities.
> We don't, but the system is meant to allow multiple
> implementations to coexist and picking the best one
> at run-time.
But that would require teaching the module loading user space
about all this first, right?
Also if one implementation is always better than the other
then I see little reason to ever have both.
-Andi
* Andi Kleen | 2007-08-20 12:16:18 [+0200]:
>> Are you sure you get the C version when both are built-in
>> or loaded as modules? If so then we have a bug in the priority
>> code.
>
>The usual use case is: Somebody -- either admin or some command
>implicitely -- executes modprobe aes because some text file
>specifies the aes cipher. At least on my system that loads
>the C version when both are enabled. modprobe will not load
>multiple modules in this case.
>
>I don't think modprobe knows anything about these priorities.
Not modprobe, but the crypto subsystem. If you have the generic C code
and the assembly variant it picks the assembly over C. The selection is
done before the particular subsystem, dm-crypt for instance, requests
the algorithm / module. It makes no sense to load the AES-i586 module
_after_ it is in use (dm-crypt loaded the encrypted root partition).
However, further requests will get the assembly variant.
>> We don't, but the system is meant to allow multiple
>> implementations to coexist and picking the best one
>> at run-time.
>
>But that would require teaching the module loading user space
>about all this first, right?
In that case yes. Would it help to add MODULE_ALIAS("aes") to the
assembly version in order to load it (atleast both)?
>Also if one implementation is always better than the other
>then I see little reason to ever have both.
If you are sure that nobody needs aes on machnies prio i586 than you
could disable the generic version on i386. Also on x86_64 the generic
version isn't required since an assembly optimized version is provided.
BUT: you might get into some trouble if you remove it from selections
because some modules select it automaticly, IEEE80211_CRYPT_CCMP for
instance.
>-Andi
Sebastian
> Not modprobe, but the crypto subsystem. If you have the generic C code
> and the assembly variant it picks the assembly over C. The selection is
But only if they're both loaded. Who loads both?
> In that case yes. Would it help to add MODULE_ALIAS("aes") to the
> assembly version in order to load it (atleast both)?
No, modprobe will only load the first it finds.
> >Also if one implementation is always better than the other
> >then I see little reason to ever have both.
>
> If you are sure that nobody needs aes on machnies prio i586 than you
> could disable the generic version on i386.
Why should the i586 version not run on 486/386?
> BUT: you might get into some trouble if you remove it from selections
> because some modules select it automaticly, IEEE80211_CRYPT_CCMP for
> instance.
Ok that is a problem.
-Andi
* Andi Kleen | 2007-08-20 12:47:14 [+0200]:
>> Not modprobe, but the crypto subsystem. If you have the generic C code
>> and the assembly variant it picks the assembly over C. The selection is
>
>But only if they're both loaded. Who loads both?
In my case I do. I have the assembly version compiled into the kernel.
The generic aes.o isn't loaded because the crypto API allready has an
algorithm.
>> In that case yes. Would it help to add MODULE_ALIAS("aes") to the
>> assembly version in order to load it (atleast both)?
>
>No, modprobe will only load the first it finds.
The s390 guys have MODULE_ALIAS("aes"); in their hw driver [1]. If it
doesn't load both (aes.ko + aes_s390.ko) modules, than I wonder what's
the reason for this.
>> >Also if one implementation is always better than the other
>> >then I see little reason to ever have both.
>>
>> If you are sure that nobody needs aes on machnies prio i586 than you
>> could disable the generic version on i386.
>
>Why should the i586 version not run on 486/386?
I assumed it uses some opcodes which are not available on 486.
>
>> BUT: you might get into some trouble if you remove it from selections
>> because some modules select it automaticly, IEEE80211_CRYPT_CCMP for
>> instance.
>
>Ok that is a problem.
Not really I guess. The aes algorithm shouldn't be directly used by the
wlan stack. It should only make sure that the user does not forget to
enable aes since it is required for CCMP.
>
>-Andi
Sebastian
[1] arch/s390/crypto/aes_s390.c
On Mon, Aug 20, 2007 at 12:08:19PM +0200, Sebastian Siewior wrote:
> * Andi Kleen | 2007-08-20 12:47:14 [+0200]:
>
> >> Not modprobe, but the crypto subsystem. If you have the generic C code
> >> and the assembly variant it picks the assembly over C. The selection is
> >
> >But only if they're both loaded. Who loads both?
> In my case I do.
You're unusual then.
I'm thinking of standard distribution kernel users though. They
just want to tell some high level configuration they want aes
(or twofish) and expect the most efficient implementation
to be loaded automatically.
The distribution kernel could just disable the generic AES,
but if that's a good idea there this could as well be done in all
kernels.
> >> In that case yes. Would it help to add MODULE_ALIAS("aes") to the
> >> assembly version in order to load it (atleast both)?
> >
> >No, modprobe will only load the first it finds.
>
> The s390 guys have MODULE_ALIAS("aes"); in their hw driver [1]. If it
> doesn't load both (aes.ko + aes_s390.ko) modules, than I wonder what's
> the reason for this.
When only one is enabled then aes_s390 will be loaded.
But when both are enabled only one wins. At least on my system
that seems to be the C version.
>
> >> >Also if one implementation is always better than the other
> >> >then I see little reason to ever have both.
> >>
> >> If you are sure that nobody needs aes on machnies prio i586 than you
> >> could disable the generic version on i386.
> >
> >Why should the i586 version not run on 486/386?
>
> I assumed it uses some opcodes which are not available on 486.
There are not many. From a quick scan I didn't find any.
I assume the 586 refers to it being tuned for P5? Although that would
be also weird, few people still care about P5 tuning and it's quite
different from newer CPUs and likely not beneficial on them.
> >> BUT: you might get into some trouble if you remove it from selections
> >> because some modules select it automaticly, IEEE80211_CRYPT_CCMP for
> >> instance.
> >
> >Ok that is a problem.
>
> Not really I guess. The aes algorithm shouldn't be directly used by the
> wlan stack. It should only make sure that the user does not forget to
> enable aes since it is required for CCMP.
Well it still would need to be solved to get rid of the generic
aes/twofish. I don't know how unfortunately. Or could the select
just be dropped?
-Andi
* Andi Kleen | 2007-08-20 13:12:39 [+0200]:
>I'm thinking of standard distribution kernel users though. They
>just want to tell some high level configuration they want aes
>(or twofish) and expect the most efficient implementation
>to be loaded automatically.
Sure.
>The distribution kernel could just disable the generic AES,
>but if that's a good idea there this could as well be done in all
>kernels.
If the i586 variant is working on all x86 machines than be my guest.
We still have the problem with hardware drivers like the via-aes driver.
In that case you don't to have the generic nor the i586 version. Adding
a priority to modprobe in case of two drivers with the same name/alias
might not to be worst idea. On s390 you might need both, since not every
machine provides hardware acceleration.
>> The s390 guys have MODULE_ALIAS("aes"); in their hw driver [1]. If it
>> doesn't load both (aes.ko + aes_s390.ko) modules, than I wonder what's
>> the reason for this.
>
>When only one is enabled then aes_s390 will be loaded.
>
>But when both are enabled only one wins. At least on my system
>that seems to be the C version.
Okay, same here.
>> >> BUT: you might get into some trouble if you remove it from selections
>> >> because some modules select it automaticly, IEEE80211_CRYPT_CCMP for
>> >> instance.
>> >
>> >Ok that is a problem.
>>
>> Not really I guess. The aes algorithm shouldn't be directly used by the
>> wlan stack. It should only make sure that the user does not forget to
>> enable aes since it is required for CCMP.
>
>Well it still would need to be solved to get rid of the generic
>aes/twofish. I don't know how unfortunately. Or could the select
>just be dropped?
Yes, but wait for Herbert's ACK. Technically only the crypto subsystem
is required for compilation. AES is required because it is part of CCMP.
It is not needed for linking or anything. You have just to convince
your users to enable AES if they select CCMP or else it will not work
(that's the whole point about select in first place from my POV). This
should not be a problem in a distro kernel.
>-Andi
On Mon, Aug 20, 2007 at 12:16:18PM +0200, Andi Kleen wrote:
>
> The usual use case is: Somebody -- either admin or some command
> implicitely -- executes modprobe aes because some text file
> specifies the aes cipher. At least on my system that loads
> the C version when both are enabled. modprobe will not load
> multiple modules in this case.
>
> I don't think modprobe knows anything about these priorities.
Right, in that case we'd only load one of them, usually
the generic one since its name is what we're trying to
load.
> But that would require teaching the module loading user space
> about all this first, right?
That would be the best. However, it's not hard to do a
simple probing in the kernel until modprobe(8) gets this
feature.
I'll code something up.
> Also if one implementation is always better than the other
> then I see little reason to ever have both.
Well it's not that useful for an assembly implementation
that works on all instances of a given architecture.
However, one of the things we need to handle are drivers
that only work on a subset of an architecture, such as
padlock-aes.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
> That would be the best. However, it's not hard to do a
> simple probing in the kernel until modprobe(8) gets this
> feature.
Sounds like a big hack, and at least for aes / aes-x86_64 and
twofish it's not needed. Just disable aes on x86.
The only problem is the select issue with wireless.
Unfortunately
select CRYPTO_AES_X86_64 if X86_64
select CRYPTO_AES_I586 if X86_32
select CRYPTO_AES if !X86
produces warnings for unreferenced symbols :/
Perhaps it can be just removed for now.
> > Also if one implementation is always better than the other
> > then I see little reason to ever have both.
>
> Well it's not that useful for an assembly implementation
> that works on all instances of a given architecture.
I meant on x86. Sure for other architectures the C version is needed.
-Andi
On Mon, Aug 20, 2007 at 03:06:39PM +0200, Andi Kleen wrote:
>
> > Well it's not that useful for an assembly implementation
> > that works on all instances of a given architecture.
>
> I meant on x86. Sure for other architectures the C version is needed.
My point was that while it's not useful for aes-i586 on x86,
it definitely is useful for padlock-aes which is also x86 asm
but only works on a subset of x86 processors.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
* Andi Kleen | 2007-08-20 15:06:39 [+0200]:
>> That would be the best. However, it's not hard to do a
>> simple probing in the kernel until modprobe(8) gets this
>> feature.
>
>Sounds like a big hack, and at least for aes / aes-x86_64 and
>twofish it's not needed. Just disable aes on x86.
>
>The only problem is the select issue with wireless.
>
>Unfortunately
>
>select CRYPTO_AES_X86_64 if X86_64
>select CRYPTO_AES_I586 if X86_32
>select CRYPTO_AES if !X86
>
>produces warnings for unreferenced symbols :/
>Perhaps it can be just removed for now.
What about:
[crypto] do not use generic AES on i386 and x86_64
This patch automatically selects the assembly optimized version
of AES (if selected) and the generic version can no longer be
selected. The module will be called aes.ko
Signed-off-by: Sebastian Siewior <[email protected]>
---
arch/i386/crypto/Makefile | 4 +-
arch/i386/crypto/{aes.c => aes_key.c} | 0
arch/x86_64/crypto/Makefile | 5 +--
arch/x86_64/crypto/{aes.c => aes_key.c} | 0
crypto/Kconfig | 46 +++++-------------------------
crypto/Makefile | 2 +-
6 files changed, 13 insertions(+), 44 deletions(-)
rename arch/i386/crypto/{aes.c => aes_key.c} (100%)
rename arch/x86_64/crypto/{aes.c => aes_key.c} (100%)
diff --git a/arch/i386/crypto/Makefile b/arch/i386/crypto/Makefile
index 3fd19af..e725951 100644
--- a/arch/i386/crypto/Makefile
+++ b/arch/i386/crypto/Makefile
@@ -4,9 +4,9 @@
# Arch-specific CryptoAPI modules.
#
-obj-$(CONFIG_CRYPTO_AES_586) += aes-i586.o
+obj-$(CONFIG_CRYPTO_AES_586) += aes.o
obj-$(CONFIG_CRYPTO_TWOFISH_586) += twofish-i586.o
-aes-i586-y := aes-i586-asm.o aes.o
+aes-y := aes-i586-asm.o aes_key.o
twofish-i586-y := twofish-i586-asm.o twofish.o
diff --git a/arch/i386/crypto/aes.c b/arch/i386/crypto/aes_key.c
similarity index 100%
rename from arch/i386/crypto/aes.c
rename to arch/i386/crypto/aes_key.c
diff --git a/arch/x86_64/crypto/Makefile b/arch/x86_64/crypto/Makefile
index 15b538a..e34e716 100644
--- a/arch/x86_64/crypto/Makefile
+++ b/arch/x86_64/crypto/Makefile
@@ -4,9 +4,8 @@
# Arch-specific CryptoAPI modules.
#
-obj-$(CONFIG_CRYPTO_AES_X86_64) += aes-x86_64.o
+obj-$(CONFIG_CRYPTO_AES_X86_64) += aes.o
obj-$(CONFIG_CRYPTO_TWOFISH_X86_64) += twofish-x86_64.o
-aes-x86_64-y := aes-x86_64-asm.o aes.o
+aes-y := aes-x86_64-asm.o aes_key.o
twofish-x86_64-y := twofish-x86_64-asm.o twofish.o
-
diff --git a/arch/x86_64/crypto/aes.c b/arch/x86_64/crypto/aes_key.c
similarity index 100%
rename from arch/x86_64/crypto/aes.c
rename to arch/x86_64/crypto/aes_key.c
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 3d1a1e2..87d7bce 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -286,6 +286,9 @@ config CRYPTO_SERPENT
config CRYPTO_AES
tristate "AES cipher algorithms"
+ select CRYPTO_AES_586 if (X86 || UML_X86) && !64BIT
+ select CRYPTO_AES_X86_64 if (X86 || UML_X86) && 64BIT
+ select CRYPTO_AES_GENERIC if !X86
select CRYPTO_ALGAPI
help
AES cipher algorithms (FIPS-197). AES uses the Rijndael
@@ -304,47 +307,14 @@ config CRYPTO_AES
See <http://csrc.nist.gov/CryptoToolkit/aes/> for more information.
-config CRYPTO_AES_586
- tristate "AES cipher algorithms (i586)"
- depends on (X86 || UML_X86) && !64BIT
- select CRYPTO_ALGAPI
- help
- AES cipher algorithms (FIPS-197). AES uses the Rijndael
- algorithm.
-
- Rijndael appears to be consistently a very good performer in
- both hardware and software across a wide range of computing
- environments regardless of its use in feedback or non-feedback
- modes. Its key setup time is excellent, and its key agility is
- good. Rijndael's very low memory requirements make it very well
- suited for restricted-space environments, in which it also
- demonstrates excellent performance. Rijndael's operations are
- among the easiest to defend against power and timing attacks.
-
- The AES specifies three key sizes: 128, 192 and 256 bits
+config CRYPTO_AES_GENERIC
+ tristate
- See <http://csrc.nist.gov/encryption/aes/> for more information.
+config CRYPTO_AES_586
+ tristate
config CRYPTO_AES_X86_64
- tristate "AES cipher algorithms (x86_64)"
- depends on (X86 || UML_X86) && 64BIT
- select CRYPTO_ALGAPI
- help
- AES cipher algorithms (FIPS-197). AES uses the Rijndael
- algorithm.
-
- Rijndael appears to be consistently a very good performer in
- both hardware and software across a wide range of computing
- environments regardless of its use in feedback or non-feedback
- modes. Its key setup time is excellent, and its key agility is
- good. Rijndael's very low memory requirements make it very well
- suited for restricted-space environments, in which it also
- demonstrates excellent performance. Rijndael's operations are
- among the easiest to defend against power and timing attacks.
-
- The AES specifies three key sizes: 128, 192 and 256 bits
-
- See <http://csrc.nist.gov/encryption/aes/> for more information.
+ tristate
config CRYPTO_CAST5
tristate "CAST5 (CAST-128) cipher algorithm"
diff --git a/crypto/Makefile b/crypto/Makefile
index 0cf17f1..af44fd5 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -37,7 +37,7 @@ obj-$(CONFIG_CRYPTO_BLOWFISH) += blowfish.o
obj-$(CONFIG_CRYPTO_TWOFISH) += twofish.o
obj-$(CONFIG_CRYPTO_TWOFISH_COMMON) += twofish_common.o
obj-$(CONFIG_CRYPTO_SERPENT) += serpent.o
-obj-$(CONFIG_CRYPTO_AES) += aes.o
+obj-$(CONFIG_CRYPTO_AES_GENERIC) += aes.o
obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia.o
obj-$(CONFIG_CRYPTO_CAST5) += cast5.o
obj-$(CONFIG_CRYPTO_CAST6) += cast6.o
--
1.5.3.rc7
> What about:
Looks good.
-Andi
>
> [crypto] do not use generic AES on i386 and x86_64
On Mon, Sep 03, 2007 at 12:42:27AM +0200, Sebastian Siewior wrote:
>
> [crypto] do not use generic AES on i386 and x86_64
>
> This patch automatically selects the assembly optimized version
> of AES (if selected) and the generic version can no longer be
> selected. The module will be called aes.ko
You don't need to rename it because it already provides an
alias for aes.
Also please provide a way to build the generic AES code so
that it can at least be tested on i386/x86_64.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
* Herbert Xu | 2007-09-19 20:29:43 [+0800]:
>On Mon, Sep 03, 2007 at 12:42:27AM +0200, Sebastian Siewior wrote:
>>
>> [crypto] do not use generic AES on i386 and x86_64
>>
>> This patch automatically selects the assembly optimized version
>> of AES (if selected) and the generic version can no longer be
>> selected. The module will be called aes.ko
>
>You don't need to rename it because it already provides an
>alias for aes.
Right, forgot about that.
>Also please provide a way to build the generic AES code so
>that it can at least be tested on i386/x86_64.
You want to auto compile aes-x86_64 if you are on x86_64 and additonally
the generic version. Is that correct?
If so, why do you want to have them both?
Sebastian
On Wed, Sep 19, 2007 at 11:46:52PM +0200, Sebastian Siewior wrote:
>
> >Also please provide a way to build the generic AES code so
> >that it can at least be tested on i386/x86_64.
>
> You want to auto compile aes-x86_64 if you are on x86_64 and additonally
> the generic version. Is that correct?
> If so, why do you want to have them both?
So that the generic C version can actually be tested on x86.
We don't want it to bit-rot which would make non-x86 2nd-class
citizens.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
* Herbert Xu | 2007-09-20 08:20:58 [+0800]:
>So that the generic C version can actually be tested on x86.
>We don't want it to bit-rot which would make non-x86 2nd-class
>citizens.
Okey. I try to post something what you might like in the new few days.
>Cheers,
Sebastian
* Sebastian Siewior | 2007-09-20 23:09:47 [+0200]:
>* Herbert Xu | 2007-09-20 08:20:58 [+0800]:
>
>>So that the generic C version can actually be tested on x86.
>>We don't want it to bit-rot which would make non-x86 2nd-class
>>citizens.
>
>Okey. I try to post something what you might like in the new few days.
This one should do it? Now you should not select both as module in order
to get the assembly version auto loaded by the kernel :)
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -286,6 +286,9 @@ config CRYPTO_SERPENT
config CRYPTO_AES
tristate "AES cipher algorithms"
+ select CRYPTO_AES_586 if (X86 || UML_X86) && !64BIT
+ select CRYPTO_AES_X86_64 if (X86 || UML_X86) && 64BIT
+ select CRYPTO_AES_GENERIC if (!X86 && !UML_X86)
select CRYPTO_ALGAPI
help
AES cipher algorithms (FIPS-197). AES uses the Rijndael
@@ -304,47 +307,23 @@ config CRYPTO_AES
See <http://csrc.nist.gov/CryptoToolkit/aes/> for more information.
-config CRYPTO_AES_586
- tristate "AES cipher algorithms (i586)"
- depends on (X86 || UML_X86) && !64BIT
- select CRYPTO_ALGAPI
- help
- AES cipher algorithms (FIPS-197). AES uses the Rijndael
- algorithm.
-
- Rijndael appears to be consistently a very good performer in
- both hardware and software across a wide range of computing
- environments regardless of its use in feedback or non-feedback
- modes. Its key setup time is excellent, and its key agility is
- good. Rijndael's very low memory requirements make it very well
- suited for restricted-space environments, in which it also
- demonstrates excellent performance. Rijndael's operations are
- among the easiest to defend against power and timing attacks.
-
- The AES specifies three key sizes: 128, 192 and 256 bits
+config CRYPTO_AES_GENERIC
+ tristate
- See <http://csrc.nist.gov/encryption/aes/> for more information.
-
-config CRYPTO_AES_X86_64
- tristate "AES cipher algorithms (x86_64)"
- depends on (X86 || UML_X86) && 64BIT
+config CRYPTO_AES_GENERIC_ENFORCE
+ tristate "AES cipher algorithm (generic Version)"
+ depends on (X86 || UML_X86)
+ select CRYPTO_AES_GENERIC
select CRYPTO_ALGAPI
help
- AES cipher algorithms (FIPS-197). AES uses the Rijndael
- algorithm.
-
- Rijndael appears to be consistently a very good performer in
- both hardware and software across a wide range of computing
- environments regardless of its use in feedback or non-feedback
- modes. Its key setup time is excellent, and its key agility is
- good. Rijndael's very low memory requirements make it very well
- suited for restricted-space environments, in which it also
- demonstrates excellent performance. Rijndael's operations are
- among the easiest to defend against power and timing attacks.
+ This is the generic implementation of AES instead of the assembly
+ optimized version.
- The AES specifies three key sizes: 128, 192 and 256 bits
+config CRYPTO_AES_586
+ tristate
- See <http://csrc.nist.gov/encryption/aes/> for more information.
+config CRYPTO_AES_X86_64
+ tristate
config CRYPTO_CAST5
tristate "CAST5 (CAST-128) cipher algorithm"
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -37,7 +37,7 @@ obj-$(CONFIG_CRYPTO_BLOWFISH) += blowfis
obj-$(CONFIG_CRYPTO_TWOFISH) += twofish.o
obj-$(CONFIG_CRYPTO_TWOFISH_COMMON) += twofish_common.o
obj-$(CONFIG_CRYPTO_SERPENT) += serpent.o
-obj-$(CONFIG_CRYPTO_AES) += aes.o
+obj-$(CONFIG_CRYPTO_AES_GENERIC) += aes.o
obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia.o
obj-$(CONFIG_CRYPTO_CAST5) += cast5.o
obj-$(CONFIG_CRYPTO_CAST6) += cast6.o
* Herbert Xu | 2007-08-20 20:06:05 [+0800]:
>On Mon, Aug 20, 2007 at 12:16:18PM +0200, Andi Kleen wrote:
>> I don't think modprobe knows anything about these priorities.
>
>Right, in that case we'd only load one of them, usually
>the generic one since its name is what we're trying to
>load.
>
>> But that would require teaching the module loading user space
>> about all this first, right?
>
>That would be the best. However, it's not hard to do a
>simple probing in the kernel until modprobe(8) gets this
>feature.
>
>I'll code something up.
Herbert, I tried to implement a MODULE_PRIO() macro which would be used
by the module loader and I noticed that it may not be required.
If there are two modules providing the same alias and we modprobe that
alias than modprobe will load both of them. It may be a waste of memory
if we load the generic algorithm and the assembly optimized one.
However, it would be good for some hardware drivers which need the
generic implementation for some "corner cases".
What do you thing?
Sebastian
On Sun, Sep 30, 2007 at 02:42:39PM +0200, Sebastian Siewior wrote:
>
> Herbert, I tried to implement a MODULE_PRIO() macro which would be used
> by the module loader and I noticed that it may not be required.
> If there are two modules providing the same alias and we modprobe that
> alias than modprobe will load both of them. It may be a waste of memory
> if we load the generic algorithm and the assembly optimized one.
> However, it would be good for some hardware drivers which need the
> generic implementation for some "corner cases".
> What do you thing?
I think you're brilliant!
Could you please do a patch to rename aes.c to aes-generic.c and
add the appropriate alias to it? Please add the aes alias to padlock
and geode while you're at it.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
* Herbert Xu | 2007-10-03 15:35:22 [+0800]:
>On Sun, Sep 30, 2007 at 02:42:39PM +0200, Sebastian Siewior wrote:
>>
>> Herbert, I tried to implement a MODULE_PRIO() macro which would be used
>> by the module loader and I noticed that it may not be required.
>> If there are two modules providing the same alias and we modprobe that
>> alias than modprobe will load both of them. It may be a waste of memory
>> if we load the generic algorithm and the assembly optimized one.
>> However, it would be good for some hardware drivers which need the
>> generic implementation for some "corner cases".
>> What do you thing?
>
>I think you're brilliant!
>
>Could you please do a patch to rename aes.c to aes-generic.c and
>add the appropriate alias to it? Please add the aes alias to padlock
>and geode while you're at it.
I am sorry that I did not reply earlier but I was busy framing that
email :)
I send out rename patches for aes, des (s390 hw driver available) and
sha1+252 (s390 + via padlock hw).
Two last questions:
- What about the i386 assembly vs generic implementation? Do you prefer
the patch that I have send earlier (choose the assembly by default
making the generic optional) or do you want both of them loaded at
the same time.
- What might be the best fallback strategy for the s390 + geode aes
driver?
I tried to move the key size to the algorithm request function what
works good most of the time. All users except the wlan stack do alloc
cipher followed by setkey in the same function. The benefit would be
that you don't return a cipher that is not able to handle certain key
sizes (and you don't have to implement the fallback in every driver (2
right now)) separately.
I started to implement a fallback within the driver (two is not that
much right now) and wanted to make sure that this is the prefered way
of fixing this.
>Thanks,
Sebastian
On Thu, Oct 04, 2007 at 10:35:12AM +0200, Sebastian Siewior wrote:
>
> Two last questions:
> - What about the i386 assembly vs generic implementation? Do you prefer
> the patch that I have send earlier (choose the assembly by default
> making the generic optional) or do you want both of them loaded at
> the same time.
I'd prefer both to be built by default so that if something
does go wrong we can ask people to check by using aes-generic.
> - What might be the best fallback strategy for the s390 + geode aes
> driver?
I've been trying to avoid this :)
Yes your proposal is definitely on the right track.
My original plan is to have a template like keylen(aes,16) that
only supports key length 16. So perhaps you can take a look at
doing that or come up with a different solution.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Loading the crypto algorithm by the alias instead of by module directly
has the advantage that all possible implementations of this algorithm
are loaded automatically and the crypto API can choose the best one
depending on its priority.
Additionally it ensures that the generic implementation as well as the
HW driver (if available) is loaded in case the HW driver needs the
generic version as fallback in corner cases.
Signed-off-by: Sebastian Siewior <[email protected]>
---
arch/s390/crypto/aes_s390.c | 2 +-
crypto/Makefile | 2 +-
crypto/{aes.c => aes_generic.c} | 2 +-
drivers/crypto/geode-aes.c | 1 +
drivers/crypto/padlock-aes.c | 4 ++--
5 files changed, 6 insertions(+), 5 deletions(-)
rename crypto/{aes.c => aes_generic.c} (99%)
diff --git a/arch/s390/crypto/aes_s390.c b/arch/s390/crypto/aes_s390.c
index 3660ca6..5126696 100644
--- a/arch/s390/crypto/aes_s390.c
+++ b/arch/s390/crypto/aes_s390.c
@@ -7,7 +7,7 @@
* Copyright IBM Corp. 2005,2007
* Author(s): Jan Glauber ([email protected])
*
- * Derived from "crypto/aes.c"
+ * Derived from "crypto/aes_generic.c"
*
* This program is free software; you can redistribute it and/or modify it
* under the terms of the GNU General Public License as published by the Free
diff --git a/crypto/Makefile b/crypto/Makefile
index e96a07e..7025bd5 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -39,7 +39,7 @@ obj-$(CONFIG_CRYPTO_BLOWFISH) += blowfish.o
obj-$(CONFIG_CRYPTO_TWOFISH) += twofish.o
obj-$(CONFIG_CRYPTO_TWOFISH_COMMON) += twofish_common.o
obj-$(CONFIG_CRYPTO_SERPENT) += serpent.o
-obj-$(CONFIG_CRYPTO_AES) += aes.o
+obj-$(CONFIG_CRYPTO_AES) += aes_generic.o
obj-$(CONFIG_CRYPTO_CAMELLIA) += camellia.o
obj-$(CONFIG_CRYPTO_CAST5) += cast5.o
obj-$(CONFIG_CRYPTO_CAST6) += cast6.o
diff --git a/crypto/aes.c b/crypto/aes_generic.c
similarity index 99%
rename from crypto/aes.c
rename to crypto/aes_generic.c
index e244077..9401dca 100644
--- a/crypto/aes.c
+++ b/crypto/aes_generic.c
@@ -453,4 +453,4 @@ module_exit(aes_fini);
MODULE_DESCRIPTION("Rijndael (AES) Cipher Algorithm");
MODULE_LICENSE("Dual BSD/GPL");
-
+MODULE_ALIAS("aes");
diff --git a/drivers/crypto/geode-aes.c b/drivers/crypto/geode-aes.c
index 6a86958..f9a34ab 100644
--- a/drivers/crypto/geode-aes.c
+++ b/drivers/crypto/geode-aes.c
@@ -473,6 +473,7 @@ geode_aes_exit(void)
MODULE_AUTHOR("Advanced Micro Devices, Inc.");
MODULE_DESCRIPTION("Geode LX Hardware AES driver");
MODULE_LICENSE("GPL");
+MODULE_ALIAS("aes");
module_init(geode_aes_init);
module_exit(geode_aes_exit);
diff --git a/drivers/crypto/padlock-aes.c b/drivers/crypto/padlock-aes.c
index d4501dc..abbcff0 100644
--- a/drivers/crypto/padlock-aes.c
+++ b/drivers/crypto/padlock-aes.c
@@ -5,7 +5,7 @@
*
* Copyright (c) 2004 Michal Ludvig <[email protected]>
*
- * Key expansion routine taken from crypto/aes.c
+ * Key expansion routine taken from crypto/aes_generic.c
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
@@ -660,4 +660,4 @@ MODULE_DESCRIPTION("VIA PadLock AES algorithm support");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Michal Ludvig");
-MODULE_ALIAS("aes-padlock");
+MODULE_ALIAS("aes");
--
1.5.3.2
Loading the crypto algorithm by the alias instead of by module directly
has the advantage that all possible implementations of this algorithm
are loaded automatically and the crypto API can choose the best one
depending on its priority.
Signed-off-by: Sebastian Siewior <[email protected]>
---
crypto/Makefile | 2 +-
crypto/{des.c => des_generic.c} | 1 +
2 files changed, 2 insertions(+), 1 deletions(-)
rename crypto/{des.c => des_generic.c} (99%)
diff --git a/crypto/Makefile b/crypto/Makefile
index 7025bd5..b6ef5e4 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -33,7 +33,7 @@ obj-$(CONFIG_CRYPTO_PCBC) += pcbc.o
obj-$(CONFIG_CRYPTO_LRW) += lrw.o
obj-$(CONFIG_CRYPTO_XTS) += xts.o
obj-$(CONFIG_CRYPTO_CRYPTD) += cryptd.o
-obj-$(CONFIG_CRYPTO_DES) += des.o
+obj-$(CONFIG_CRYPTO_DES) += des_generic.o
obj-$(CONFIG_CRYPTO_FCRYPT) += fcrypt.o
obj-$(CONFIG_CRYPTO_BLOWFISH) += blowfish.o
obj-$(CONFIG_CRYPTO_TWOFISH) += twofish.o
diff --git a/crypto/des.c b/crypto/des_generic.c
similarity index 99%
rename from crypto/des.c
rename to crypto/des_generic.c
index 1df3a71..59966d1 100644
--- a/crypto/des.c
+++ b/crypto/des_generic.c
@@ -1009,3 +1009,4 @@ module_exit(fini);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("DES & Triple DES EDE Cipher Algorithms");
MODULE_AUTHOR("Dag Arne Osvik <[email protected]>");
+MODULE_ALIAS("des");
--
1.5.3.2
Loading the crypto algorithm by the alias instead of by module directly
has the advantage that all possible implementations of this algorithm
are loaded automatically and the crypto API can choose the best one
depending on its priority.
Additionally it ensures that the generic implementation as well as the
HW driver (if available) is loaded in case the HW driver needs the
generic version as fallback in corner cases.
Signed-off-by: Sebastian Siewior <[email protected]>
---
arch/s390/crypto/sha1_s390.c | 2 +-
arch/s390/crypto/sha256_s390.c | 2 +-
crypto/Makefile | 4 ++--
crypto/{sha1.c => sha1_generic.c} | 2 +-
crypto/{sha256.c => sha256_generic.c} | 2 +-
drivers/crypto/padlock-sha.c | 4 ++--
6 files changed, 8 insertions(+), 8 deletions(-)
rename crypto/{sha1.c => sha1_generic.c} (99%)
rename crypto/{sha256.c => sha256_generic.c} (99%)
diff --git a/arch/s390/crypto/sha1_s390.c b/arch/s390/crypto/sha1_s390.c
index af4460e..8ebd3cd 100644
--- a/arch/s390/crypto/sha1_s390.c
+++ b/arch/s390/crypto/sha1_s390.c
@@ -12,7 +12,7 @@
* Author(s): Thomas Spatzier
* Jan Glauber ([email protected])
*
- * Derived from "crypto/sha1.c"
+ * Derived from "crypto/sha1_generic.c"
* Copyright (c) Alan Smithee.
* Copyright (c) Andrew McDonald <[email protected]>
* Copyright (c) Jean-Francois Dive <[email protected]>
diff --git a/arch/s390/crypto/sha256_s390.c b/arch/s390/crypto/sha256_s390.c
index 2ced333..c728bd0 100644
--- a/arch/s390/crypto/sha256_s390.c
+++ b/arch/s390/crypto/sha256_s390.c
@@ -7,7 +7,7 @@
* Copyright IBM Corp. 2005,2007
* Author(s): Jan Glauber ([email protected])
*
- * Derived from "crypto/sha256.c"
+ * Derived from "crypto/sha256_generic.c"
* and "arch/s390/crypto/sha1_s390.c"
*
* This program is free software; you can redistribute it and/or modify it
diff --git a/crypto/Makefile b/crypto/Makefile
index b6ef5e4..43c2a0d 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -21,8 +21,8 @@ obj-$(CONFIG_CRYPTO_XCBC) += xcbc.o
obj-$(CONFIG_CRYPTO_NULL) += crypto_null.o
obj-$(CONFIG_CRYPTO_MD4) += md4.o
obj-$(CONFIG_CRYPTO_MD5) += md5.o
-obj-$(CONFIG_CRYPTO_SHA1) += sha1.o
-obj-$(CONFIG_CRYPTO_SHA256) += sha256.o
+obj-$(CONFIG_CRYPTO_SHA1) += sha1_generic.o
+obj-$(CONFIG_CRYPTO_SHA256) += sha256_generic.o
obj-$(CONFIG_CRYPTO_SHA512) += sha512.o
obj-$(CONFIG_CRYPTO_WP512) += wp512.o
obj-$(CONFIG_CRYPTO_TGR192) += tgr192.o
diff --git a/crypto/sha1.c b/crypto/sha1_generic.c
similarity index 99%
rename from crypto/sha1.c
rename to crypto/sha1_generic.c
index 1bba551..70364dd 100644
--- a/crypto/sha1.c
+++ b/crypto/sha1_generic.c
@@ -139,4 +139,4 @@ module_exit(fini);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("SHA1 Secure Hash Algorithm");
-MODULE_ALIAS("sha1-generic");
+MODULE_ALIAS("sha1");
diff --git a/crypto/sha256.c b/crypto/sha256_generic.c
similarity index 99%
rename from crypto/sha256.c
rename to crypto/sha256_generic.c
index 716195b..74bf2f9 100644
--- a/crypto/sha256.c
+++ b/crypto/sha256_generic.c
@@ -339,4 +339,4 @@ module_exit(fini);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("SHA256 Secure Hash Algorithm");
-MODULE_ALIAS("sha256-generic");
+MODULE_ALIAS("sha256");
diff --git a/drivers/crypto/padlock-sha.c b/drivers/crypto/padlock-sha.c
index a781fd2..b8ee645 100644
--- a/drivers/crypto/padlock-sha.c
+++ b/drivers/crypto/padlock-sha.c
@@ -314,5 +314,5 @@ MODULE_DESCRIPTION("VIA PadLock SHA1/SHA256 algorithms support.");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Michal Ludvig");
-MODULE_ALIAS("sha1-padlock");
-MODULE_ALIAS("sha256-padlock");
+MODULE_ALIAS("sha1");
+MODULE_ALIAS("sha256");
--
1.5.3.2
On Thursday 04 October 2007 10:48, Herbert Xu wrote:
> On Thu, Oct 04, 2007 at 10:35:12AM +0200, Sebastian Siewior wrote:
> > Two last questions:
> > - What about the i386 assembly vs generic implementation? Do you prefer
> > the patch that I have send earlier (choose the assembly by default
> > making the generic optional) or do you want both of them loaded at
> > the same time.
>
> I'd prefer both to be built by default so that if something
> does go wrong we can ask people to check by using aes-generic.
Is that really needed? How often did you see a broken AES implementation?
They tend to be well tested and high quality after all and I haven't ever seen
any evidence that the assembler functions are any less stable than C.
In fact they're probably more stable because they don't have to worry
about being miscompiled.
I also think it is a bad idea to install the generic function by default -- it
increases the risk the user ends up with a unnecessary slow implementation
-Andi
* Herbert Xu | 2007-10-04 16:48:18 [+0800]:
>On Thu, Oct 04, 2007 at 10:35:12AM +0200, Sebastian Siewior wrote:
>>
>> Two last questions:
>> - What about the i386 assembly vs generic implementation? Do you prefer
>> the patch that I have send earlier (choose the assembly by default
>> making the generic optional) or do you want both of them loaded at
>> the same time.
>
>I'd prefer both to be built by default so that if something
>does go wrong we can ask people to check by using aes-generic.
That sounds fair to me. I don't touch twofish since it is not auto
selected like aes is and the assembly version is available by default
unless the user enabled both. Maybe a hint in Kconfig's help would
help but I don't thing it is necessary. The distros should not compile
it in first place :)
>> - What might be the best fallback strategy for the s390 + geode aes
>> driver?
>
>I've been trying to avoid this :)
>
>Yes your proposal is definitely on the right track.
>
>My original plan is to have a template like keylen(aes,16) that
>only supports key length 16. So perhaps you can take a look at
>doing that or come up with a different solution.
Okey. So I drop my changes to the driver and try to fix the crypto API
like I was doing in first place.
>Cheers,
Sebastian
On Thu, Oct 04, 2007 at 11:31:56AM +0200, Andi Kleen wrote:
>
> I also think it is a bad idea to install the generic function by default -- it
> increases the risk the user ends up with a unnecessary slow implementation
With Sebastian's change this won't be possible at all since
modprobe will load all aliases.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
* Andi Kleen | 2007-10-04 11:31:56 [+0200]:
>On Thursday 04 October 2007 10:48, Herbert Xu wrote:
>> On Thu, Oct 04, 2007 at 10:35:12AM +0200, Sebastian Siewior wrote:
>> > Two last questions:
>> > - What about the i386 assembly vs generic implementation? Do you prefer
>> > the patch that I have send earlier (choose the assembly by default
>> > making the generic optional) or do you want both of them loaded at
>> > the same time.
>>
>> I'd prefer both to be built by default so that if something
>> does go wrong we can ask people to check by using aes-generic.
>
>Is that really needed? How often did you see a broken AES implementation?
>They tend to be well tested and high quality after all and I haven't ever seen
>any evidence that the assembler functions are any less stable than C.
>In fact they're probably more stable because they don't have to worry
>about being miscompiled.
>
>I also think it is a bad idea to install the generic function by default -- it
>increases the risk the user ends up with a unnecessary slow implementation
With the first patch I send earlier (where you wrote "it looks good to
me") the assembly version is selected by default and the generic is
optional (you don't have to compile it at all).
With the patch I've send today both variants (generic + assembly) are
loaded (if available of course) and thr crypto API can decide which one
is the best.
The end user will not end up with a slow implementation because the
distro does not have to ship generic version plus if it does than both
modules are loaded and crypto API picks the best.
>-Andi
Sebastian
On Wed, Oct 03, 2007 at 09:23:37PM +0200, Sebastian Siewior wrote:
> Loading the crypto algorithm by the alias instead of by module directly
> has the advantage that all possible implementations of this algorithm
> are loaded automatically and the crypto API can choose the best one
> depending on its priority.
>
> Signed-off-by: Sebastian Siewior <[email protected]>
Applied. Thanks!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Oct 04, 2007 at 09:37:27AM +0200, Sebastian Siewior wrote:
> Loading the crypto algorithm by the alias instead of by module directly
> has the advantage that all possible implementations of this algorithm
> are loaded automatically and the crypto API can choose the best one
> depending on its priority.
> Additionally it ensures that the generic implementation as well as the
> HW driver (if available) is loaded in case the HW driver needs the
> generic version as fallback in corner cases.
>
> Signed-off-by: Sebastian Siewior <[email protected]>
Patch applied. Thanks!
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Oct 04, 2007 at 09:37:39AM +0200, Sebastian Siewior wrote:
> Loading the crypto algorithm by the alias instead of by module directly
> has the advantage that all possible implementations of this algorithm
> are loaded automatically and the crypto API can choose the best one
> depending on its priority.
> Additionally it ensures that the generic implementation as well as the
> HW driver (if available) is loaded in case the HW driver needs the
> generic version as fallback in corner cases.
>
> Signed-off-by: Sebastian Siewior <[email protected]>
This patch is correct per se but it combined with the code in
padlock-sha can cause a dead-lock. Padlock-sha tries to load
the generic sha module in its init function. At this point it
is still holding the module_mutex so the probe will dead-lock.
The probe is actually pointless since we can always probe when
the algorithm is actually used which does not lead to dead-locks
like this.
So please delete the padlock_sha_check_fallbacks code in your
patch to make it load correctly.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
* Herbert Xu | 2007-10-05 16:57:44 [+0800]:
>This patch is correct per se but it combined with the code in
>padlock-sha can cause a dead-lock. Padlock-sha tries to load
>the generic sha module in its init function. At this point it
>is still holding the module_mutex so the probe will dead-lock.
I did not find a module_mutex. We hold a readlock of crypto_alg_sem
within the crypto subsystem and request_module() increments
kmod_concurrent (which may not exceed a certain limit).
>The probe is actually pointless since we can always probe when
>the algorithm is actually used which does not lead to dead-locks
>like this.
Right.
>So please delete the padlock_sha_check_fallbacks code in your
>patch to make it load correctly.
okey, updated patch will follow.
>Cheers,
Sebastian
Loading the crypto algorithm by the alias instead of by module directly
has the advantage that all possible implementations of this algorithm
are loaded automatically and the crypto API can choose the best one
depending on its priority.
Additionally it ensures that the generic implementation as well as the
HW driver (if available) is loaded in case the HW driver needs the
generic version as fallback in corner cases.
Also remove the probe for sha1 in padlock's init code.
Quote from Herbert:
The probe is actually pointless since we can always probe when
the algorithm is actually used which does not lead to dead-locks
like this.
Signed-off-by: Sebastian Siewior <[email protected]>
---
arch/s390/crypto/sha1_s390.c | 2 +-
arch/s390/crypto/sha256_s390.c | 2 +-
crypto/Makefile | 4 ++--
crypto/{sha1.c => sha1_generic.c} | 2 +-
crypto/{sha256.c => sha256_generic.c} | 8 ++++----
drivers/crypto/padlock-sha.c | 19 ++-----------------
6 files changed, 11 insertions(+), 26 deletions(-)
rename crypto/{sha1.c => sha1_generic.c} (99%)
rename crypto/{sha256.c => sha256_generic.c} (99%)
diff --git a/arch/s390/crypto/sha1_s390.c b/arch/s390/crypto/sha1_s390.c
index af4460e..8ebd3cd 100644
--- a/arch/s390/crypto/sha1_s390.c
+++ b/arch/s390/crypto/sha1_s390.c
@@ -12,7 +12,7 @@
* Author(s): Thomas Spatzier
* Jan Glauber ([email protected])
*
- * Derived from "crypto/sha1.c"
+ * Derived from "crypto/sha1_generic.c"
* Copyright (c) Alan Smithee.
* Copyright (c) Andrew McDonald <[email protected]>
* Copyright (c) Jean-Francois Dive <[email protected]>
diff --git a/arch/s390/crypto/sha256_s390.c b/arch/s390/crypto/sha256_s390.c
index 2ced333..c728bd0 100644
--- a/arch/s390/crypto/sha256_s390.c
+++ b/arch/s390/crypto/sha256_s390.c
@@ -7,7 +7,7 @@
* Copyright IBM Corp. 2005,2007
* Author(s): Jan Glauber ([email protected])
*
- * Derived from "crypto/sha256.c"
+ * Derived from "crypto/sha256_generic.c"
* and "arch/s390/crypto/sha1_s390.c"
*
* This program is free software; you can redistribute it and/or modify it
diff --git a/crypto/Makefile b/crypto/Makefile
index b6ef5e4..43c2a0d 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -21,8 +21,8 @@ obj-$(CONFIG_CRYPTO_XCBC) += xcbc.o
obj-$(CONFIG_CRYPTO_NULL) += crypto_null.o
obj-$(CONFIG_CRYPTO_MD4) += md4.o
obj-$(CONFIG_CRYPTO_MD5) += md5.o
-obj-$(CONFIG_CRYPTO_SHA1) += sha1.o
-obj-$(CONFIG_CRYPTO_SHA256) += sha256.o
+obj-$(CONFIG_CRYPTO_SHA1) += sha1_generic.o
+obj-$(CONFIG_CRYPTO_SHA256) += sha256_generic.o
obj-$(CONFIG_CRYPTO_SHA512) += sha512.o
obj-$(CONFIG_CRYPTO_WP512) += wp512.o
obj-$(CONFIG_CRYPTO_TGR192) += tgr192.o
diff --git a/crypto/sha1.c b/crypto/sha1_generic.c
similarity index 99%
rename from crypto/sha1.c
rename to crypto/sha1_generic.c
index 1bba551..70364dd 100644
--- a/crypto/sha1.c
+++ b/crypto/sha1_generic.c
@@ -139,4 +139,4 @@ module_exit(fini);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("SHA1 Secure Hash Algorithm");
-MODULE_ALIAS("sha1-generic");
+MODULE_ALIAS("sha1");
diff --git a/crypto/sha256.c b/crypto/sha256_generic.c
similarity index 99%
rename from crypto/sha256.c
rename to crypto/sha256_generic.c
index 716195b..c7097dd 100644
--- a/crypto/sha256.c
+++ b/crypto/sha256_generic.c
@@ -339,4 +339,4 @@ module_exit(fini);
MODULE_LICENSE("GPL");
MODULE_DESCRIPTION("SHA256 Secure Hash Algorithm");
-MODULE_ALIAS("sha256-generic");
+MODULE_ALIAS("sha256");
diff --git a/drivers/crypto/padlock-sha.c b/drivers/crypto/padlock-sha.c
index a781fd2..bc1de34 100644
--- a/drivers/crypto/padlock-sha.c
+++ b/drivers/crypto/padlock-sha.c
@@ -253,19 +253,6 @@ static struct crypto_alg sha256_alg = {
}
};
-static void __init padlock_sha_check_fallbacks(void)
-{
- if (!crypto_has_hash("sha1", 0, CRYPTO_ALG_ASYNC |
- CRYPTO_ALG_NEED_FALLBACK))
- printk(KERN_WARNING PFX
- "Couldn't load fallback module for sha1.\n");
-
- if (!crypto_has_hash("sha256", 0, CRYPTO_ALG_ASYNC |
- CRYPTO_ALG_NEED_FALLBACK))
- printk(KERN_WARNING PFX
- "Couldn't load fallback module for sha256.\n");
-}
-
static int __init padlock_init(void)
{
int rc = -ENODEV;
@@ -280,8 +267,6 @@ static int __init padlock_init(void)
return -ENODEV;
}
- padlock_sha_check_fallbacks();
-
rc = crypto_register_alg(&sha1_alg);
if (rc)
goto out;
@@ -314,5 +299,5 @@ MODULE_DESCRIPTION("VIA PadLock SHA1/SHA256 algorithms support.");
MODULE_LICENSE("GPL");
MODULE_AUTHOR("Michal Ludvig");
-MODULE_ALIAS("sha1-padlock");
-MODULE_ALIAS("sha256-padlock");
+MODULE_ALIAS("sha1");
+MODULE_ALIAS("sha256");
--
1.5.3.2
On Fri, Oct 05, 2007 at 03:50:56PM +0200, Sebastian Siewior wrote:
>
> I did not find a module_mutex. We hold a readlock of crypto_alg_sem
> within the crypto subsystem and request_module() increments
> kmod_concurrent (which may not exceed a certain limit).
Yes you're right. The module_mutex (in kernel/module.c) is
dropped for the init call so the dead-lock won't actually occur.
However, it's still good to remove this code since doing a nested
modprobe of "sha" within a modprobe of "sha" just feels wrong.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Fri, Oct 05, 2007 at 03:12:10PM +0200, Sebastian Siewior wrote:
> Loading the crypto algorithm by the alias instead of by module directly
> has the advantage that all possible implementations of this algorithm
> are loaded automatically and the crypto API can choose the best one
> depending on its priority.
> Additionally it ensures that the generic implementation as well as the
> HW driver (if available) is loaded in case the HW driver needs the
> generic version as fallback in corner cases.
> Also remove the probe for sha1 in padlock's init code.
> Quote from Herbert:
> The probe is actually pointless since we can always probe when
> the algorithm is actually used which does not lead to dead-locks
> like this.
Thanks. I'll apply this tomorrow.
BTW, the dead-lock does exist after all. The lock that's held
is the user-space file lock held by modprobe. Assuming that
padlock-sha comes before sha in the alias list, then the second
modprobe will hit padlock-sha again and dead-lock.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
* Herbert Xu | 2007-10-05 22:20:22 [+0800]:
>On Fri, Oct 05, 2007 at 03:50:56PM +0200, Sebastian Siewior wrote:
>>
>> I did not find a module_mutex. We hold a readlock of crypto_alg_sem
>> within the crypto subsystem and request_module() increments
>> kmod_concurrent (which may not exceed a certain limit).
>
>However, it's still good to remove this code since doing a nested
>modprobe of "sha" within a modprobe of "sha" just feels wrong.
Yes it feels wrong. It even may show that the fallback is not available
if padlock is loaded before the generic ones.
Sebastian
* Herbert Xu | 2007-10-05 23:10:51 [+0800]:
>On Fri, Oct 05, 2007 at 03:12:10PM +0200, Sebastian Siewior wrote:
>> Loading the crypto algorithm by the alias instead of by module directly
>> has the advantage that all possible implementations of this algorithm
>> are loaded automatically and the crypto API can choose the best one
>> depending on its priority.
>> Additionally it ensures that the generic implementation as well as the
>> HW driver (if available) is loaded in case the HW driver needs the
>> generic version as fallback in corner cases.
>> Also remove the probe for sha1 in padlock's init code.
>> Quote from Herbert:
>> The probe is actually pointless since we can always probe when
>> the algorithm is actually used which does not lead to dead-locks
>> like this.
>
>Thanks. I'll apply this tomorrow.
>
>BTW, the dead-lock does exist after all. The lock that's held
>is the user-space file lock held by modprobe. Assuming that
>padlock-sha comes before sha in the alias list, then the second
>modprobe will hit padlock-sha again and dead-lock.
Really, hmm. That CRYPTO_ALG_NEED_FALLBACK flag is unused than?
I try to remove the hardware specific code and test the via-sha code as
it once I have some time in my hands :)
>Cheers,
Sebastian
* Herbert Xu | 2007-10-05 23:10:51 [+0800]:
>BTW, the dead-lock does exist after all. The lock that's held
>is the user-space file lock held by modprobe. Assuming that
>padlock-sha comes before sha in the alias list, then the second
>modprobe will hit padlock-sha again and dead-lock.
I tried to deadlock and I did not succeed, maybe I did something wrong.
# fgrep sha modules.alias
alias sha256 padlock_sha
alias sha1 padlock_sha
alias sha384 sha512
alias sha256 sha256_generic
alias sha1 sha1_generic
padlock comes first as you said. A "modprobe tcrypt mode=2" worked fine
(with the test passed).
# lsmod |grep sha
sha1_generic 3008 0
padlock_sha 4160 0
crypto_algapi 13824 4
sha1_generic,padlock_sha,blkcipher,cryptomgr
I have a version of the padlock-sha driver with no HW dependency
(fallback only, that one I used) at
git://git.breakpoint.cc/bigeasy/linux via_sha
if you want to test :)
>Cheers,
Sebastian
On Sun, Oct 07, 2007 at 11:42:27PM +0200, Sebastian Siewior wrote:
>
> I tried to deadlock and I did not succeed, maybe I did something wrong.
>
> # fgrep sha modules.alias
> alias sha256 padlock_sha
> alias sha1 padlock_sha
> alias sha384 sha512
> alias sha256 sha256_generic
> alias sha1 sha1_generic
Try reversing this. modprobe aliases are added to the head
of the list so they'll come out in reverse.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Sun, Oct 07, 2007 at 12:02:20AM +0200, Sebastian Siewior wrote:
>
> >BTW, the dead-lock does exist after all. The lock that's held
> >is the user-space file lock held by modprobe. Assuming that
> >padlock-sha comes before sha in the alias list, then the second
> >modprobe will hit padlock-sha again and dead-lock.
>
> Really, hmm. That CRYPTO_ALG_NEED_FALLBACK flag is unused than?
That flag is only known to the crypto code. Modprobe knows
nothing of it so the two probes will look the same to modprobe.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Fri, Oct 05, 2007 at 03:12:10PM +0200, Sebastian Siewior wrote:
> Loading the crypto algorithm by the alias instead of by module directly
> has the advantage that all possible implementations of this algorithm
> are loaded automatically and the crypto API can choose the best one
> depending on its priority.
> Additionally it ensures that the generic implementation as well as the
> HW driver (if available) is loaded in case the HW driver needs the
> generic version as fallback in corner cases.
> Also remove the probe for sha1 in padlock's init code.
> Quote from Herbert:
> The probe is actually pointless since we can always probe when
> the algorithm is actually used which does not lead to dead-locks
> like this.
>
> Signed-off-by: Sebastian Siewior <[email protected]>
Patch applied with one small change where I left the *-padlock
aliases around since people might already be using them and
they're correct as the specific name of those algorithms.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Hi Sebastian,
what about SHA-512? Should that not also be renamed to sha512_generic.c ?
--Jan
On Thu, 2007-10-04 at 09:37 +0200, Sebastian Siewior wrote:
> Loading the crypto algorithm by the alias instead of by module directly
> has the advantage that all possible implementations of this algorithm
> are loaded automatically and the crypto API can choose the best one
> depending on its priority.
> Additionally it ensures that the generic implementation as well as the
> HW driver (if available) is loaded in case the HW driver needs the
> generic version as fallback in corner cases.
>
> Signed-off-by: Sebastian Siewior <[email protected]>
> ---
> arch/s390/crypto/sha1_s390.c | 2 +-
> arch/s390/crypto/sha256_s390.c | 2 +-
> crypto/Makefile | 4 ++--
> crypto/{sha1.c => sha1_generic.c} | 2 +-
> crypto/{sha256.c => sha256_generic.c} | 2 +-
> drivers/crypto/padlock-sha.c | 4 ++--
> 6 files changed, 8 insertions(+), 8 deletions(-)
> rename crypto/{sha1.c => sha1_generic.c} (99%)
> rename crypto/{sha256.c => sha256_generic.c} (99%)
>
> diff --git a/arch/s390/crypto/sha1_s390.c b/arch/s390/crypto/sha1_s390.c
> index af4460e..8ebd3cd 100644
> --- a/arch/s390/crypto/sha1_s390.c
> +++ b/arch/s390/crypto/sha1_s390.c
> @@ -12,7 +12,7 @@
> * Author(s): Thomas Spatzier
> * Jan Glauber ([email protected])
> *
> - * Derived from "crypto/sha1.c"
> + * Derived from "crypto/sha1_generic.c"
> * Copyright (c) Alan Smithee.
> * Copyright (c) Andrew McDonald <[email protected]>
> * Copyright (c) Jean-Francois Dive <[email protected]>
> diff --git a/arch/s390/crypto/sha256_s390.c b/arch/s390/crypto/sha256_s390.c
> index 2ced333..c728bd0 100644
> --- a/arch/s390/crypto/sha256_s390.c
> +++ b/arch/s390/crypto/sha256_s390.c
> @@ -7,7 +7,7 @@
> * Copyright IBM Corp. 2005,2007
> * Author(s): Jan Glauber ([email protected])
> *
> - * Derived from "crypto/sha256.c"
> + * Derived from "crypto/sha256_generic.c"
> * and "arch/s390/crypto/sha1_s390.c"
> *
> * This program is free software; you can redistribute it and/or modify it
> diff --git a/crypto/Makefile b/crypto/Makefile
> index b6ef5e4..43c2a0d 100644
> --- a/crypto/Makefile
> +++ b/crypto/Makefile
> @@ -21,8 +21,8 @@ obj-$(CONFIG_CRYPTO_XCBC) += xcbc.o
> obj-$(CONFIG_CRYPTO_NULL) += crypto_null.o
> obj-$(CONFIG_CRYPTO_MD4) += md4.o
> obj-$(CONFIG_CRYPTO_MD5) += md5.o
> -obj-$(CONFIG_CRYPTO_SHA1) += sha1.o
> -obj-$(CONFIG_CRYPTO_SHA256) += sha256.o
> +obj-$(CONFIG_CRYPTO_SHA1) += sha1_generic.o
> +obj-$(CONFIG_CRYPTO_SHA256) += sha256_generic.o
> obj-$(CONFIG_CRYPTO_SHA512) += sha512.o
> obj-$(CONFIG_CRYPTO_WP512) += wp512.o
> obj-$(CONFIG_CRYPTO_TGR192) += tgr192.o
> diff --git a/crypto/sha1.c b/crypto/sha1_generic.c
> similarity index 99%
> rename from crypto/sha1.c
> rename to crypto/sha1_generic.c
> index 1bba551..70364dd 100644
> --- a/crypto/sha1.c
> +++ b/crypto/sha1_generic.c
> @@ -139,4 +139,4 @@ module_exit(fini);
> MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("SHA1 Secure Hash Algorithm");
>
> -MODULE_ALIAS("sha1-generic");
> +MODULE_ALIAS("sha1");
> diff --git a/crypto/sha256.c b/crypto/sha256_generic.c
> similarity index 99%
> rename from crypto/sha256.c
> rename to crypto/sha256_generic.c
> index 716195b..74bf2f9 100644
> --- a/crypto/sha256.c
> +++ b/crypto/sha256_generic.c
> @@ -339,4 +339,4 @@ module_exit(fini);
> MODULE_LICENSE("GPL");
> MODULE_DESCRIPTION("SHA256 Secure Hash Algorithm");
>
> -MODULE_ALIAS("sha256-generic");
> +MODULE_ALIAS("sha256");
> diff --git a/drivers/crypto/padlock-sha.c b/drivers/crypto/padlock-sha.c
> index a781fd2..b8ee645 100644
> --- a/drivers/crypto/padlock-sha.c
> +++ b/drivers/crypto/padlock-sha.c
> @@ -314,5 +314,5 @@ MODULE_DESCRIPTION("VIA PadLock SHA1/SHA256 algorithms support.");
> MODULE_LICENSE("GPL");
> MODULE_AUTHOR("Michal Ludvig");
>
> -MODULE_ALIAS("sha1-padlock");
> -MODULE_ALIAS("sha256-padlock");
> +MODULE_ALIAS("sha1");
> +MODULE_ALIAS("sha256");
--
++49 7031 161911
IBM Deutschland Entwicklung GmbH
Vorsitzender des Aufsichtsrats: Martin Jetter Gesch?ftsf?hrung: Herbert
Kircher, Sitz der Gesellschaft: B?blingen, Registergericht Amtsgericht
Stuttgart, HRB 243294
* Jan Glauber | 2007-10-08 13:25:23 [+0200]:
>Hi Sebastian,
Hi Jan, good to see you back :)
>what about SHA-512? Should that not also be renamed to sha512_generic.c ?
We could do it, but I haven't seen a HW implementation yet. If your new
CPU has an opcode for this, we could rename it than :)
>
>--Jan
Sebastian
* Herbert Xu | 2007-10-08 11:20:17 [+0800]:
>On Sun, Oct 07, 2007 at 11:42:27PM +0200, Sebastian Siewior wrote:
>>
>> I tried to deadlock and I did not succeed, maybe I did something wrong.
>>
>> # fgrep sha modules.alias
>> alias sha256 padlock_sha
>> alias sha1 padlock_sha
>> alias sha384 sha512
>> alias sha256 sha256_generic
>> alias sha1 sha1_generic
>
>Try reversing this. modprobe aliases are added to the head
>of the list so they'll come out in reverse.
# fgrep sha modules.alias
alias sha1 sha1_generic
alias sha256 sha256_generic
alias sha384 sha512
alias sha1 padlock_sha
alias sha256 padlock_sha
Still no dead-lock.
>Cheers,
Sebastian