2007-09-01 08:54:29

by Herbert Xu

[permalink] [raw]
Subject: Re: 2.6.23-rc4-mm1

On Fri, Aug 31, 2007 at 11:58:15PM -0700, Andrew Morton wrote:
>
> > crypto/built-in.o: In function `update2':
> > digest.c:(.text+0x94a): undefined reference to `crypto_km_types'
> > digest.c:(.text+0x9bf): undefined reference to `crypto_km_types'
> >
> > digest.c (CONFIG_CRYPTO) uses crypto/scatterwalk.c's object (CONFIG_CRYPTO_ALGAPI)
> > I meet this when CONFIG_CRYPTO_ALGAPI=m. I need to make CONFIG_CRYPTO_ALGAPI=y.
>
> cc herbert..

Sorry, only tested on x86-64 which doesn't have HIGHMEM.

I've just pushed the following fix into cryptodev-2.6.

commit 25531e010a2a1d0099b62d473244d09e72402ce5
Author: Herbert Xu <[email protected]>
Date: Sat Sep 1 16:52:13 2007 +0800

[CRYPTO] api: Kill crypto_km_types

When scatterwalk is built as a module digest.c was broken because it
requires the crypto_km_types structure which is in scatterwalk. This
patch removes the crypto_km_types structure by encoding the logic into
crypto_kmap_type directly.

In fact, this even saves a few bytes of code (not to mention the data
structure itself) on i386 which is about the only place where it's
needed.

Signed-off-by: Herbert Xu <[email protected]>

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
--
diff --git a/crypto/internal.h b/crypto/internal.h
index 60acad9..abb01f7 100644
--- a/crypto/internal.h
+++ b/crypto/internal.h
@@ -50,11 +50,16 @@ extern struct list_head crypto_alg_list;
extern struct rw_semaphore crypto_alg_sem;
extern struct blocking_notifier_head crypto_chain;

-extern enum km_type crypto_km_types[];
-
static inline enum km_type crypto_kmap_type(int out)
{
- return crypto_km_types[(in_softirq() ? 2 : 0) + out];
+ enum km_type type;
+
+ if (in_softirq())
+ type = out * (KM_SOFTIRQ1 - KM_SOFTIRQ0) + KM_SOFTIRQ0;
+ else
+ type = out * (KM_USER1 - KM_USER0) + KM_USER0;
+
+ return type;
}

static inline void *crypto_kmap(struct page *page, int out)


2007-09-01 20:56:18

by Satyam Sharma

[permalink] [raw]
Subject: Re: 2.6.23-rc4-mm1



On Sat, 1 Sep 2007, Herbert Xu wrote:

> On Fri, Aug 31, 2007 at 11:58:15PM -0700, Andrew Morton wrote:
> >
> > > crypto/built-in.o: In function `update2':
> > > digest.c:(.text+0x94a): undefined reference to `crypto_km_types'
> > > digest.c:(.text+0x9bf): undefined reference to `crypto_km_types'
> > >
> > > digest.c (CONFIG_CRYPTO) uses crypto/scatterwalk.c's object (CONFIG_CRYPTO_ALGAPI)
> > > I meet this when CONFIG_CRYPTO_ALGAPI=m. I need to make CONFIG_CRYPTO_ALGAPI=y.

Tangential, but I've often wondered what are the upsides of keeping
CONFIG_CRYPTO_ALGAPI as a separate config option in the first place? Every
single item in crypto/ ends up "select"ing it (directly or transitively)
so it makes all sense to just do away with it and keep it == CONFIG_CRYPTO
in the Makefile, thusly:


[PATCH] crypto: Remove CONFIG_CRYPTO_ALGAPI config option

Because all other options in crypto/ end up selecting it anyway. So let's
make it a default part of the rest of "core" crypto stuff, that gets built
whenever CONFIG_CRYPTO == y.

Signed-off-by: Satyam Sharma <[email protected]>

---

arch/s390/crypto/Kconfig | 4 ----
crypto/Kconfig | 37 -------------------------------------
crypto/Makefile | 7 ++-----
drivers/crypto/Kconfig | 2 --
4 files changed, 2 insertions(+), 48 deletions(-)

diff --git a/arch/s390/crypto/Kconfig b/arch/s390/crypto/Kconfig
index d1defbb..d35f901 100644
--- a/arch/s390/crypto/Kconfig
+++ b/arch/s390/crypto/Kconfig
@@ -1,7 +1,6 @@
config CRYPTO_SHA1_S390
tristate "SHA1 digest algorithm"
depends on S390
- select CRYPTO_ALGAPI
help
This is the s390 hardware accelerated implementation of the
SHA-1 secure hash standard (FIPS 180-1/DFIPS 180-2).
@@ -9,7 +8,6 @@ config CRYPTO_SHA1_S390
config CRYPTO_SHA256_S390
tristate "SHA256 digest algorithm"
depends on S390
- select CRYPTO_ALGAPI
help
This is the s390 hardware accelerated implementation of the
SHA256 secure hash standard (DFIPS 180-2).
@@ -20,7 +18,6 @@ config CRYPTO_SHA256_S390
config CRYPTO_DES_S390
tristate "DES and Triple DES cipher algorithms"
depends on S390
- select CRYPTO_ALGAPI
select CRYPTO_BLKCIPHER
help
This us the s390 hardware accelerated implementation of the
@@ -29,7 +26,6 @@ config CRYPTO_DES_S390
config CRYPTO_AES_S390
tristate "AES cipher algorithms"
depends on S390
- select CRYPTO_ALGAPI
select CRYPTO_BLKCIPHER
help
This is the s390 hardware accelerated implementation of the
diff --git a/crypto/Kconfig b/crypto/Kconfig
index 3d1a1e2..0a52118 100644
--- a/crypto/Kconfig
+++ b/crypto/Kconfig
@@ -19,26 +19,18 @@ menuconfig CRYPTO

if CRYPTO

-config CRYPTO_ALGAPI
- tristate
- help
- This option provides the API for cryptographic algorithms.
-
config CRYPTO_ABLKCIPHER
tristate
select CRYPTO_BLKCIPHER

config CRYPTO_BLKCIPHER
tristate
- select CRYPTO_ALGAPI

config CRYPTO_HASH
tristate
- select CRYPTO_ALGAPI

config CRYPTO_MANAGER
tristate "Cryptographic algorithm manager"
- select CRYPTO_ALGAPI
help
Create default cryptographic template instantiations such as
cbc(aes).
@@ -64,31 +56,26 @@ config CRYPTO_XCBC

config CRYPTO_NULL
tristate "Null algorithms"
- select CRYPTO_ALGAPI
help
These are 'Null' algorithms, used by IPsec, which do nothing.

config CRYPTO_MD4
tristate "MD4 digest algorithm"
- select CRYPTO_ALGAPI
help
MD4 message digest algorithm (RFC1320).

config CRYPTO_MD5
tristate "MD5 digest algorithm"
- select CRYPTO_ALGAPI
help
MD5 message digest algorithm (RFC1321).

config CRYPTO_SHA1
tristate "SHA1 digest algorithm"
- select CRYPTO_ALGAPI
help
SHA-1 secure hash standard (FIPS 180-1/DFIPS 180-2).

config CRYPTO_SHA256
tristate "SHA256 digest algorithm"
- select CRYPTO_ALGAPI
help
SHA256 secure hash standard (DFIPS 180-2).

@@ -97,7 +84,6 @@ config CRYPTO_SHA256

config CRYPTO_SHA512
tristate "SHA384 and SHA512 digest algorithms"
- select CRYPTO_ALGAPI
help
SHA512 secure hash standard (DFIPS 180-2).

@@ -109,7 +95,6 @@ config CRYPTO_SHA512

config CRYPTO_WP512
tristate "Whirlpool digest algorithms"
- select CRYPTO_ALGAPI
help
Whirlpool hash algorithm 512, 384 and 256-bit hashes

@@ -121,7 +106,6 @@ config CRYPTO_WP512

config CRYPTO_TGR192
tristate "Tiger digest algorithms"
- select CRYPTO_ALGAPI
help
Tiger hash algorithm 192, 160 and 128-bit hashes

@@ -194,20 +178,17 @@ config CRYPTO_CRYPTD

config CRYPTO_DES
tristate "DES and Triple DES EDE cipher algorithms"
- select CRYPTO_ALGAPI
help
DES cipher algorithm (FIPS 46-2), and Triple DES EDE (FIPS 46-3).

config CRYPTO_FCRYPT
tristate "FCrypt cipher algorithm"
- select CRYPTO_ALGAPI
select CRYPTO_BLKCIPHER
help
FCrypt algorithm used by RxRPC.

config CRYPTO_BLOWFISH
tristate "Blowfish cipher algorithm"
- select CRYPTO_ALGAPI
help
Blowfish cipher algorithm, by Bruce Schneier.

@@ -220,7 +201,6 @@ config CRYPTO_BLOWFISH

config CRYPTO_TWOFISH
tristate "Twofish cipher algorithm"
- select CRYPTO_ALGAPI
select CRYPTO_TWOFISH_COMMON
help
Twofish cipher algorithm.
@@ -242,7 +222,6 @@ config CRYPTO_TWOFISH_COMMON
config CRYPTO_TWOFISH_586
tristate "Twofish cipher algorithms (i586)"
depends on (X86 || UML_X86) && !64BIT
- select CRYPTO_ALGAPI
select CRYPTO_TWOFISH_COMMON
help
Twofish cipher algorithm.
@@ -258,7 +237,6 @@ config CRYPTO_TWOFISH_586
config CRYPTO_TWOFISH_X86_64
tristate "Twofish cipher algorithm (x86_64)"
depends on (X86 || UML_X86) && 64BIT
- select CRYPTO_ALGAPI
select CRYPTO_TWOFISH_COMMON
help
Twofish cipher algorithm (x86_64).
@@ -273,7 +251,6 @@ config CRYPTO_TWOFISH_X86_64

config CRYPTO_SERPENT
tristate "Serpent cipher algorithm"
- select CRYPTO_ALGAPI
help
Serpent cipher algorithm, by Anderson, Biham & Knudsen.

@@ -286,7 +263,6 @@ config CRYPTO_SERPENT

config CRYPTO_AES
tristate "AES cipher algorithms"
- select CRYPTO_ALGAPI
help
AES cipher algorithms (FIPS-197). AES uses the Rijndael
algorithm.
@@ -307,7 +283,6 @@ config CRYPTO_AES
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.
@@ -328,7 +303,6 @@ config CRYPTO_AES_586
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.
@@ -348,21 +322,18 @@ config CRYPTO_AES_X86_64

config CRYPTO_CAST5
tristate "CAST5 (CAST-128) cipher algorithm"
- select CRYPTO_ALGAPI
help
The CAST5 encryption algorithm (synonymous with CAST-128) is
described in RFC2144.

config CRYPTO_CAST6
tristate "CAST6 (CAST-256) cipher algorithm"
- select CRYPTO_ALGAPI
help
The CAST6 encryption algorithm (synonymous with CAST-256) is
described in RFC2612.

config CRYPTO_TEA
tristate "TEA, XTEA and XETA cipher algorithms"
- select CRYPTO_ALGAPI
help
TEA cipher algorithm.

@@ -379,7 +350,6 @@ config CRYPTO_TEA

config CRYPTO_ARC4
tristate "ARC4 cipher algorithm"
- select CRYPTO_ALGAPI
help
ARC4 cipher algorithm.

@@ -390,7 +360,6 @@ config CRYPTO_ARC4

config CRYPTO_KHAZAD
tristate "Khazad cipher algorithm"
- select CRYPTO_ALGAPI
help
Khazad cipher algorithm.

@@ -403,7 +372,6 @@ config CRYPTO_KHAZAD

config CRYPTO_ANUBIS
tristate "Anubis cipher algorithm"
- select CRYPTO_ALGAPI
help
Anubis cipher algorithm.

@@ -418,7 +386,6 @@ config CRYPTO_ANUBIS

config CRYPTO_DEFLATE
tristate "Deflate compression algorithm"
- select CRYPTO_ALGAPI
select ZLIB_INFLATE
select ZLIB_DEFLATE
help
@@ -429,7 +396,6 @@ config CRYPTO_DEFLATE

config CRYPTO_MICHAEL_MIC
tristate "Michael MIC keyed digest algorithm"
- select CRYPTO_ALGAPI
help
Michael MIC is used for message integrity protection in TKIP
(IEEE 802.11i). This algorithm is required for TKIP, but it
@@ -438,7 +404,6 @@ config CRYPTO_MICHAEL_MIC

config CRYPTO_CRC32C
tristate "CRC32c CRC algorithm"
- select CRYPTO_ALGAPI
select LIBCRC32C
help
Castagnoli, et al Cyclic Redundancy-Check Algorithm. Used
@@ -449,7 +414,6 @@ config CRYPTO_CRC32C
config CRYPTO_CAMELLIA
tristate "Camellia cipher algorithms"
depends on CRYPTO
- select CRYPTO_ALGAPI
help
Camellia cipher algorithms module.

@@ -464,7 +428,6 @@ config CRYPTO_CAMELLIA
config CRYPTO_TEST
tristate "Testing module"
depends on m
- select CRYPTO_ALGAPI
help
Quick & dirty crypto test module.

diff --git a/crypto/Makefile b/crypto/Makefile
index 0cf17f1..a8ea4f6 100644
--- a/crypto/Makefile
+++ b/crypto/Makefile
@@ -2,11 +2,8 @@
# Cryptographic API
#

-obj-$(CONFIG_CRYPTO) += api.o scatterwalk.o cipher.o digest.o compress.o
-
-crypto_algapi-$(CONFIG_PROC_FS) += proc.o
-crypto_algapi-objs := algapi.o $(crypto_algapi-y)
-obj-$(CONFIG_CRYPTO_ALGAPI) += crypto_algapi.o
+obj-$(CONFIG_CRYPTO) += api.o scatterwalk.o cipher.o digest.o compress.o algapi.o
+obj-$(CONFIG_PROC_FS) += proc.o

obj-$(CONFIG_CRYPTO_ABLKCIPHER) += ablkcipher.o
obj-$(CONFIG_CRYPTO_BLKCIPHER) += blkcipher.o
diff --git a/drivers/crypto/Kconfig b/drivers/crypto/Kconfig
index c0fc4ae..90f6c6d 100644
--- a/drivers/crypto/Kconfig
+++ b/drivers/crypto/Kconfig
@@ -13,7 +13,6 @@ if CRYPTO_HW
config CRYPTO_DEV_PADLOCK
tristate "Support for VIA PadLock ACE"
depends on X86_32
- select CRYPTO_ALGAPI
default m
help
Some VIA processors come with an integrated crypto engine
@@ -56,7 +55,6 @@ source "arch/s390/crypto/Kconfig"
config CRYPTO_DEV_GEODE
tristate "Support for the Geode LX AES engine"
depends on X86_32 && PCI
- select CRYPTO_ALGAPI
select CRYPTO_BLKCIPHER
default m
help

2007-09-02 01:46:42

by Herbert Xu

[permalink] [raw]
Subject: Re: 2.6.23-rc4-mm1

On Sun, Sep 02, 2007 at 02:39:15AM +0530, Satyam Sharma wrote:
>
> Tangential, but I've often wondered what are the upsides of keeping
> CONFIG_CRYPTO_ALGAPI as a separate config option in the first place? Every
> single item in crypto/ ends up "select"ing it (directly or transitively)
> so it makes all sense to just do away with it and keep it == CONFIG_CRYPTO
> in the Makefile, thusly:

NACK. ALGAPI exists so that it can be built as a module, as
opposed to CRYPTO which is always built-in. It's already
invisible to the user so I don't see why you have a problem
with it.

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

2007-09-02 02:39:43

by Satyam Sharma

[permalink] [raw]
Subject: Re: 2.6.23-rc4-mm1



On Sun, 2 Sep 2007, Herbert Xu wrote:
>
> On Sun, Sep 02, 2007 at 02:39:15AM +0530, Satyam Sharma wrote:
> >
> > Tangential, but I've often wondered what are the upsides of keeping
> > CONFIG_CRYPTO_ALGAPI as a separate config option in the first place? Every
> > single item in crypto/ ends up "select"ing it (directly or transitively)
> > so it makes all sense to just do away with it and keep it == CONFIG_CRYPTO
> > in the Makefile, thusly:
>
> NACK. ALGAPI exists so that it can be built as a module, as
> opposed to CRYPTO which is always built-in.

I had already noticed that, and was even *expecting* you to reply with
*exactly* this ;-)

[ BTW CRYPTO is _not_ always built-in -- but only when CONFIG_CRYPTO=y ]

Anyway, the natural follow-up to your argument is -- why is the other
stuff in CRYPTO always built-in too ?

Take the crypto_alloc_xxx() callchain for example (I chose it because
it is the _first_ call any cryptoapi user ever has to make, and hence
it's the one that deals with module-loading stuff).

So what finally got exported out of crypto/ to the rest of the kernel
was just the crypto_alloc_xxx() wrapper. That resolves to a call to
crypto_alloc_base() in crypto/api.c, which first loads the specific
low-level algo modules, and then proceeds to crypto_init_ops(), which
itself may, say, resolve to a crypto_init_digest_ops() -- the only
interface exported from digest.c.

The point is, because the module-loading (if necessary) already takes
place before the call to digest.c is made, there is _no_ reason why
even digest.c can't be made modular -- or _any_ of the other CRYPTO
stuff (with the exception of api.c itself, of course) that "always
gets built-in" as you mentioned above.

And so caring about the optimization of making ALGAPI modular rather
than simply built-in with rest of "core" crypto stuff such as digest.c
(which could _also_ have been made modular by the same logic but wasn't)
sounds like a bogus argument to me. [ BTW did you notice that the
__crypto_alloc_tfm() has been EXPORT_SYMBOL'ed _only_ because of one
solitary modular-callsite in algapi.c ? ]


Satyam

2007-09-02 04:00:23

by Herbert Xu

[permalink] [raw]
Subject: Re: 2.6.23-rc4-mm1

On Sun, Sep 02, 2007 at 08:22:42AM +0530, Satyam Sharma wrote:
>
> So what finally got exported out of crypto/ to the rest of the kernel
> was just the crypto_alloc_xxx() wrapper. That resolves to a call to
> crypto_alloc_base() in crypto/api.c, which first loads the specific
> low-level algo modules, and then proceeds to crypto_init_ops(), which
> itself may, say, resolve to a crypto_init_digest_ops() -- the only
> interface exported from digest.c.

The mid-level code such as digest.c are only built-in because
they are legacy code. All the new mid-level code such as
blkcipher/hash are registered dynamically.

Once all the digest stuff have been converted to hash digest.c
will be removed.

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