* {twofish,aes}-{x86_64,i586} versus C implementations
@ 2007-08-20 0:34 Andi Kleen
2007-08-20 1:01 ` Herbert Xu
0 siblings, 1 reply; 45+ messages in thread
From: Andi Kleen @ 2007-08-20 0:34 UTC (permalink / raw)
To: linux-crypto
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
^ permalink raw reply [flat|nested] 45+ messages in thread* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 0:34 {twofish,aes}-{x86_64,i586} versus C implementations Andi Kleen @ 2007-08-20 1:01 ` Herbert Xu 2007-08-20 10:16 ` Andi Kleen 0 siblings, 1 reply; 45+ messages in thread From: Herbert Xu @ 2007-08-20 1:01 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-crypto Andi Kleen <ak@suse.de> 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 1:01 ` Herbert Xu @ 2007-08-20 10:16 ` Andi Kleen 2007-08-20 9:45 ` Sebastian Siewior 2007-08-20 12:06 ` Herbert Xu 0 siblings, 2 replies; 45+ messages in thread From: Andi Kleen @ 2007-08-20 10:16 UTC (permalink / raw) To: Herbert Xu; +Cc: Andi Kleen, linux-crypto > 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 10:16 ` Andi Kleen @ 2007-08-20 9:45 ` Sebastian Siewior 2007-08-20 10:47 ` Andi Kleen 2007-08-20 12:06 ` Herbert Xu 1 sibling, 1 reply; 45+ messages in thread From: Sebastian Siewior @ 2007-08-20 9:45 UTC (permalink / raw) To: Andi Kleen; +Cc: Herbert Xu, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 9:45 ` Sebastian Siewior @ 2007-08-20 10:47 ` Andi Kleen 2007-08-20 10:08 ` Sebastian Siewior 0 siblings, 1 reply; 45+ messages in thread From: Andi Kleen @ 2007-08-20 10:47 UTC (permalink / raw) To: Andi Kleen, Herbert Xu, linux-crypto > 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 10:47 ` Andi Kleen @ 2007-08-20 10:08 ` Sebastian Siewior 2007-08-20 11:12 ` Andi Kleen 0 siblings, 1 reply; 45+ messages in thread From: Sebastian Siewior @ 2007-08-20 10:08 UTC (permalink / raw) To: Andi Kleen; +Cc: Herbert Xu, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 10:08 ` Sebastian Siewior @ 2007-08-20 11:12 ` Andi Kleen 2007-08-20 11:27 ` Sebastian Siewior 0 siblings, 1 reply; 45+ messages in thread From: Andi Kleen @ 2007-08-20 11:12 UTC (permalink / raw) To: Andi Kleen, Herbert Xu, linux-crypto 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 11:12 ` Andi Kleen @ 2007-08-20 11:27 ` Sebastian Siewior 0 siblings, 0 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-08-20 11:27 UTC (permalink / raw) To: Andi Kleen; +Cc: Herbert Xu, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 10:16 ` Andi Kleen 2007-08-20 9:45 ` Sebastian Siewior @ 2007-08-20 12:06 ` Herbert Xu 2007-08-20 13:06 ` Andi Kleen 2007-09-30 12:42 ` Sebastian Siewior 1 sibling, 2 replies; 45+ messages in thread From: Herbert Xu @ 2007-08-20 12:06 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 12:06 ` Herbert Xu @ 2007-08-20 13:06 ` Andi Kleen 2007-08-20 13:07 ` Herbert Xu 2007-09-02 22:42 ` Sebastian Siewior 2007-09-30 12:42 ` Sebastian Siewior 1 sibling, 2 replies; 45+ messages in thread From: Andi Kleen @ 2007-08-20 13:06 UTC (permalink / raw) To: Herbert Xu; +Cc: Andi Kleen, linux-crypto > 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 13:06 ` Andi Kleen @ 2007-08-20 13:07 ` Herbert Xu 2007-09-02 22:42 ` Sebastian Siewior 1 sibling, 0 replies; 45+ messages in thread From: Herbert Xu @ 2007-08-20 13:07 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 13:06 ` Andi Kleen 2007-08-20 13:07 ` Herbert Xu @ 2007-09-02 22:42 ` Sebastian Siewior 2007-09-04 13:58 ` Andi Kleen 2007-09-19 12:29 ` Herbert Xu 1 sibling, 2 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-09-02 22:42 UTC (permalink / raw) To: Andi Kleen; +Cc: Herbert Xu, linux-crypto * 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 <sebastian@breakpoint.cc> --- 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 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-09-02 22:42 ` Sebastian Siewior @ 2007-09-04 13:58 ` Andi Kleen 2007-09-19 12:29 ` Herbert Xu 1 sibling, 0 replies; 45+ messages in thread From: Andi Kleen @ 2007-09-04 13:58 UTC (permalink / raw) To: Sebastian Siewior; +Cc: Herbert Xu, linux-crypto > What about: Looks good. -Andi > > [crypto] do not use generic AES on i386 and x86_64 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-09-02 22:42 ` Sebastian Siewior 2007-09-04 13:58 ` Andi Kleen @ 2007-09-19 12:29 ` Herbert Xu 2007-09-19 21:46 ` Sebastian Siewior 1 sibling, 1 reply; 45+ messages in thread From: Herbert Xu @ 2007-09-19 12:29 UTC (permalink / raw) To: Andi Kleen, linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-09-19 12:29 ` Herbert Xu @ 2007-09-19 21:46 ` Sebastian Siewior 2007-09-20 0:20 ` Herbert Xu 0 siblings, 1 reply; 45+ messages in thread From: Sebastian Siewior @ 2007-09-19 21:46 UTC (permalink / raw) To: Herbert Xu; +Cc: Andi Kleen, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-09-19 21:46 ` Sebastian Siewior @ 2007-09-20 0:20 ` Herbert Xu 2007-09-20 21:09 ` Sebastian Siewior 0 siblings, 1 reply; 45+ messages in thread From: Herbert Xu @ 2007-09-20 0:20 UTC (permalink / raw) To: Andi Kleen, linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-09-20 0:20 ` Herbert Xu @ 2007-09-20 21:09 ` Sebastian Siewior 2007-09-30 12:23 ` Sebastian Siewior 0 siblings, 1 reply; 45+ messages in thread From: Sebastian Siewior @ 2007-09-20 21:09 UTC (permalink / raw) To: Herbert Xu; +Cc: Andi Kleen, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-09-20 21:09 ` Sebastian Siewior @ 2007-09-30 12:23 ` Sebastian Siewior 0 siblings, 0 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-09-30 12:23 UTC (permalink / raw) To: Herbert Xu; +Cc: Andi Kleen, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-08-20 12:06 ` Herbert Xu 2007-08-20 13:06 ` Andi Kleen @ 2007-09-30 12:42 ` Sebastian Siewior 2007-10-03 7:35 ` Herbert Xu 1 sibling, 1 reply; 45+ messages in thread From: Sebastian Siewior @ 2007-09-30 12:42 UTC (permalink / raw) To: Herbert Xu; +Cc: Andi Kleen, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-09-30 12:42 ` Sebastian Siewior @ 2007-10-03 7:35 ` Herbert Xu 2007-10-04 8:35 ` Sebastian Siewior 0 siblings, 1 reply; 45+ messages in thread From: Herbert Xu @ 2007-10-03 7:35 UTC (permalink / raw) To: Andi Kleen, linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-10-03 7:35 ` Herbert Xu @ 2007-10-04 8:35 ` Sebastian Siewior 2007-10-03 19:23 ` [PATCH] [crypto] load the DES module by an alias Sebastian Siewior ` (3 more replies) 0 siblings, 4 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-10-04 8:35 UTC (permalink / raw) To: Herbert Xu; +Cc: Andi Kleen, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [crypto] load the DES module by an alias 2007-10-04 8:35 ` Sebastian Siewior @ 2007-10-03 19:23 ` Sebastian Siewior 2007-10-05 8:48 ` Herbert Xu 2007-10-04 7:37 ` [PATCH] [crypto] load the AES " Sebastian Siewior ` (2 subsequent siblings) 3 siblings, 1 reply; 45+ messages in thread From: Sebastian Siewior @ 2007-10-03 19:23 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-crypto 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 <sebastian@breakpoint.cc> --- 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 <da@osvik.no>"); +MODULE_ALIAS("des"); -- 1.5.3.2 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the DES module by an alias 2007-10-03 19:23 ` [PATCH] [crypto] load the DES module by an alias Sebastian Siewior @ 2007-10-05 8:48 ` Herbert Xu 0 siblings, 0 replies; 45+ messages in thread From: Herbert Xu @ 2007-10-05 8:48 UTC (permalink / raw) To: Sebastian Siewior; +Cc: linux-crypto 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 <sebastian@breakpoint.cc> Applied. Thanks! -- 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [crypto] load the AES module by an alias 2007-10-04 8:35 ` Sebastian Siewior 2007-10-03 19:23 ` [PATCH] [crypto] load the DES module by an alias Sebastian Siewior @ 2007-10-04 7:37 ` Sebastian Siewior 2007-10-05 8:52 ` Herbert Xu 2007-10-04 7:37 ` [PATCH] [crypto] load the SHA1[1|256] " Sebastian Siewior 2007-10-04 8:48 ` {twofish,aes}-{x86_64,i586} versus C implementations Herbert Xu 3 siblings, 1 reply; 45+ messages in thread From: Sebastian Siewior @ 2007-10-04 7:37 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-crypto 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 <sebastian@breakpoint.cc> --- 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 (jang@de.ibm.com) * - * 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 <michal@logix.cz> * - * 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 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the AES module by an alias 2007-10-04 7:37 ` [PATCH] [crypto] load the AES " Sebastian Siewior @ 2007-10-05 8:52 ` Herbert Xu 0 siblings, 0 replies; 45+ messages in thread From: Herbert Xu @ 2007-10-05 8:52 UTC (permalink / raw) To: Sebastian Siewior; +Cc: linux-crypto 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 <sebastian@breakpoint.cc> Patch applied. Thanks! -- 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [crypto] load the SHA1[1|256] module by an alias 2007-10-04 8:35 ` Sebastian Siewior 2007-10-03 19:23 ` [PATCH] [crypto] load the DES module by an alias Sebastian Siewior 2007-10-04 7:37 ` [PATCH] [crypto] load the AES " Sebastian Siewior @ 2007-10-04 7:37 ` Sebastian Siewior 2007-10-05 8:57 ` Herbert Xu 2007-10-08 11:25 ` Jan Glauber 2007-10-04 8:48 ` {twofish,aes}-{x86_64,i586} versus C implementations Herbert Xu 3 siblings, 2 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-10-04 7:37 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-crypto 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 <sebastian@breakpoint.cc> --- 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 (jan.glauber@de.ibm.com) * - * Derived from "crypto/sha1.c" + * Derived from "crypto/sha1_generic.c" * Copyright (c) Alan Smithee. * Copyright (c) Andrew McDonald <andrew@mcdonald.org.uk> * Copyright (c) Jean-Francois Dive <jef@linuxbe.org> 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 (jang@de.ibm.com) * - * 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 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias 2007-10-04 7:37 ` [PATCH] [crypto] load the SHA1[1|256] " Sebastian Siewior @ 2007-10-05 8:57 ` Herbert Xu 2007-10-05 13:50 ` Sebastian Siewior 2007-10-08 11:25 ` Jan Glauber 1 sibling, 1 reply; 45+ messages in thread From: Herbert Xu @ 2007-10-05 8:57 UTC (permalink / raw) To: Sebastian Siewior; +Cc: linux-crypto 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 <sebastian@breakpoint.cc> 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias 2007-10-05 8:57 ` Herbert Xu @ 2007-10-05 13:50 ` Sebastian Siewior 2007-10-05 13:12 ` [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) Sebastian Siewior 2007-10-05 14:20 ` [PATCH] [crypto] load the SHA1[1|256] module by an alias Herbert Xu 0 siblings, 2 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-10-05 13:50 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) 2007-10-05 13:50 ` Sebastian Siewior @ 2007-10-05 13:12 ` Sebastian Siewior 2007-10-05 15:10 ` Herbert Xu 2007-10-08 4:12 ` Herbert Xu 2007-10-05 14:20 ` [PATCH] [crypto] load the SHA1[1|256] module by an alias Herbert Xu 1 sibling, 2 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-10-05 13:12 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-crypto 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 <sebastian@breakpoint.cc> --- 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 (jan.glauber@de.ibm.com) * - * Derived from "crypto/sha1.c" + * Derived from "crypto/sha1_generic.c" * Copyright (c) Alan Smithee. * Copyright (c) Andrew McDonald <andrew@mcdonald.org.uk> * Copyright (c) Jean-Francois Dive <jef@linuxbe.org> 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 (jang@de.ibm.com) * - * 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 ^ permalink raw reply related [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) 2007-10-05 13:12 ` [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) Sebastian Siewior @ 2007-10-05 15:10 ` Herbert Xu 2007-10-06 22:02 ` Sebastian Siewior 2007-10-07 21:42 ` Sebastian Siewior 2007-10-08 4:12 ` Herbert Xu 1 sibling, 2 replies; 45+ messages in thread From: Herbert Xu @ 2007-10-05 15:10 UTC (permalink / raw) To: Sebastian Siewior; +Cc: linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) 2007-10-05 15:10 ` Herbert Xu @ 2007-10-06 22:02 ` Sebastian Siewior 2007-10-08 3:21 ` Herbert Xu 2007-10-07 21:42 ` Sebastian Siewior 1 sibling, 1 reply; 45+ messages in thread From: Sebastian Siewior @ 2007-10-06 22:02 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) 2007-10-06 22:02 ` Sebastian Siewior @ 2007-10-08 3:21 ` Herbert Xu 0 siblings, 0 replies; 45+ messages in thread From: Herbert Xu @ 2007-10-08 3:21 UTC (permalink / raw) To: linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) 2007-10-05 15:10 ` Herbert Xu 2007-10-06 22:02 ` Sebastian Siewior @ 2007-10-07 21:42 ` Sebastian Siewior 2007-10-08 3:20 ` Herbert Xu 1 sibling, 1 reply; 45+ messages in thread From: Sebastian Siewior @ 2007-10-07 21:42 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) 2007-10-07 21:42 ` Sebastian Siewior @ 2007-10-08 3:20 ` Herbert Xu 2007-10-08 12:35 ` Sebastian Siewior 0 siblings, 1 reply; 45+ messages in thread From: Herbert Xu @ 2007-10-08 3:20 UTC (permalink / raw) To: linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) 2007-10-08 3:20 ` Herbert Xu @ 2007-10-08 12:35 ` Sebastian Siewior 0 siblings, 0 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-10-08 12:35 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) 2007-10-05 13:12 ` [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) Sebastian Siewior 2007-10-05 15:10 ` Herbert Xu @ 2007-10-08 4:12 ` Herbert Xu 1 sibling, 0 replies; 45+ messages in thread From: Herbert Xu @ 2007-10-08 4:12 UTC (permalink / raw) To: Sebastian Siewior; +Cc: linux-crypto 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 <sebastian@breakpoint.cc> 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias 2007-10-05 13:50 ` Sebastian Siewior 2007-10-05 13:12 ` [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) Sebastian Siewior @ 2007-10-05 14:20 ` Herbert Xu 2007-10-06 21:54 ` Sebastian Siewior 1 sibling, 1 reply; 45+ messages in thread From: Herbert Xu @ 2007-10-05 14:20 UTC (permalink / raw) To: linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias 2007-10-05 14:20 ` [PATCH] [crypto] load the SHA1[1|256] module by an alias Herbert Xu @ 2007-10-06 21:54 ` Sebastian Siewior 0 siblings, 0 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-10-06 21:54 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias 2007-10-04 7:37 ` [PATCH] [crypto] load the SHA1[1|256] " Sebastian Siewior 2007-10-05 8:57 ` Herbert Xu @ 2007-10-08 11:25 ` Jan Glauber 2007-10-08 11:30 ` Sebastian Siewior 1 sibling, 1 reply; 45+ messages in thread From: Jan Glauber @ 2007-10-08 11:25 UTC (permalink / raw) To: Sebastian Siewior; +Cc: Herbert Xu, linux-crypto 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 <sebastian@breakpoint.cc> > --- > 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 (jan.glauber@de.ibm.com) > * > - * Derived from "crypto/sha1.c" > + * Derived from "crypto/sha1_generic.c" > * Copyright (c) Alan Smithee. > * Copyright (c) Andrew McDonald <andrew@mcdonald.org.uk> > * Copyright (c) Jean-Francois Dive <jef@linuxbe.org> > 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 (jang@de.ibm.com) > * > - * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: [PATCH] [crypto] load the SHA1[1|256] module by an alias 2007-10-08 11:25 ` Jan Glauber @ 2007-10-08 11:30 ` Sebastian Siewior 0 siblings, 0 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-10-08 11:30 UTC (permalink / raw) To: Jan Glauber; +Cc: Herbert Xu, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-10-04 8:35 ` Sebastian Siewior ` (2 preceding siblings ...) 2007-10-04 7:37 ` [PATCH] [crypto] load the SHA1[1|256] " Sebastian Siewior @ 2007-10-04 8:48 ` Herbert Xu 2007-10-04 9:31 ` Andi Kleen 2007-10-04 9:39 ` Sebastian Siewior 3 siblings, 2 replies; 45+ messages in thread From: Herbert Xu @ 2007-10-04 8:48 UTC (permalink / raw) To: Andi Kleen, linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-10-04 8:48 ` {twofish,aes}-{x86_64,i586} versus C implementations Herbert Xu @ 2007-10-04 9:31 ` Andi Kleen 2007-10-04 10:00 ` Sebastian Siewior 2007-10-04 10:00 ` Herbert Xu 2007-10-04 9:39 ` Sebastian Siewior 1 sibling, 2 replies; 45+ messages in thread From: Andi Kleen @ 2007-10-04 9:31 UTC (permalink / raw) To: Herbert Xu; +Cc: linux-crypto 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-10-04 9:31 ` Andi Kleen @ 2007-10-04 10:00 ` Sebastian Siewior 2007-10-04 10:00 ` Herbert Xu 1 sibling, 0 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-10-04 10:00 UTC (permalink / raw) To: Andi Kleen; +Cc: Herbert Xu, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-10-04 9:31 ` Andi Kleen 2007-10-04 10:00 ` Sebastian Siewior @ 2007-10-04 10:00 ` Herbert Xu 1 sibling, 0 replies; 45+ messages in thread From: Herbert Xu @ 2007-10-04 10:00 UTC (permalink / raw) To: Andi Kleen; +Cc: linux-crypto 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~} <herbert@gondor.apana.org.au> Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt ^ permalink raw reply [flat|nested] 45+ messages in thread
* Re: {twofish,aes}-{x86_64,i586} versus C implementations 2007-10-04 8:48 ` {twofish,aes}-{x86_64,i586} versus C implementations Herbert Xu 2007-10-04 9:31 ` Andi Kleen @ 2007-10-04 9:39 ` Sebastian Siewior 1 sibling, 0 replies; 45+ messages in thread From: Sebastian Siewior @ 2007-10-04 9:39 UTC (permalink / raw) To: Herbert Xu; +Cc: Andi Kleen, linux-crypto * 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 ^ permalink raw reply [flat|nested] 45+ messages in thread
end of thread, other threads:[~2007-10-08 12:35 UTC | newest]
Thread overview: 45+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-08-20 0:34 {twofish,aes}-{x86_64,i586} versus C implementations Andi Kleen
2007-08-20 1:01 ` Herbert Xu
2007-08-20 10:16 ` Andi Kleen
2007-08-20 9:45 ` Sebastian Siewior
2007-08-20 10:47 ` Andi Kleen
2007-08-20 10:08 ` Sebastian Siewior
2007-08-20 11:12 ` Andi Kleen
2007-08-20 11:27 ` Sebastian Siewior
2007-08-20 12:06 ` Herbert Xu
2007-08-20 13:06 ` Andi Kleen
2007-08-20 13:07 ` Herbert Xu
2007-09-02 22:42 ` Sebastian Siewior
2007-09-04 13:58 ` Andi Kleen
2007-09-19 12:29 ` Herbert Xu
2007-09-19 21:46 ` Sebastian Siewior
2007-09-20 0:20 ` Herbert Xu
2007-09-20 21:09 ` Sebastian Siewior
2007-09-30 12:23 ` Sebastian Siewior
2007-09-30 12:42 ` Sebastian Siewior
2007-10-03 7:35 ` Herbert Xu
2007-10-04 8:35 ` Sebastian Siewior
2007-10-03 19:23 ` [PATCH] [crypto] load the DES module by an alias Sebastian Siewior
2007-10-05 8:48 ` Herbert Xu
2007-10-04 7:37 ` [PATCH] [crypto] load the AES " Sebastian Siewior
2007-10-05 8:52 ` Herbert Xu
2007-10-04 7:37 ` [PATCH] [crypto] load the SHA1[1|256] " Sebastian Siewior
2007-10-05 8:57 ` Herbert Xu
2007-10-05 13:50 ` Sebastian Siewior
2007-10-05 13:12 ` [PATCH] [crypto] load the SHA1[1|256] module by an alias (v2) Sebastian Siewior
2007-10-05 15:10 ` Herbert Xu
2007-10-06 22:02 ` Sebastian Siewior
2007-10-08 3:21 ` Herbert Xu
2007-10-07 21:42 ` Sebastian Siewior
2007-10-08 3:20 ` Herbert Xu
2007-10-08 12:35 ` Sebastian Siewior
2007-10-08 4:12 ` Herbert Xu
2007-10-05 14:20 ` [PATCH] [crypto] load the SHA1[1|256] module by an alias Herbert Xu
2007-10-06 21:54 ` Sebastian Siewior
2007-10-08 11:25 ` Jan Glauber
2007-10-08 11:30 ` Sebastian Siewior
2007-10-04 8:48 ` {twofish,aes}-{x86_64,i586} versus C implementations Herbert Xu
2007-10-04 9:31 ` Andi Kleen
2007-10-04 10:00 ` Sebastian Siewior
2007-10-04 10:00 ` Herbert Xu
2007-10-04 9:39 ` Sebastian Siewior
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox