Bloat-o-meter shows rather high readings for cast6...
crypto/cast6.c:
cast6_setkey | -1310
cast6_encrypt | -4567
cast6_decrypt | -4561
3 functions changed, 10438 bytes removed, diff: -10438
crypto/cast6.c:
W | +659
Q | +308
QBAR | +316
3 functions changed, 1283 bytes added, diff: +1283
crypto/cast6.o:
6 functions changed, 1283 bytes added, 10438 bytes removed, diff: -9155
Signed-off-by: Ilpo J?rvinen <[email protected]>
---
crypto/cast6.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/crypto/cast6.c b/crypto/cast6.c
index 136ab6d..5fd9420 100644
--- a/crypto/cast6.c
+++ b/crypto/cast6.c
@@ -369,7 +369,7 @@ static const u8 Tr[4][8] = {
};
/* forward octave */
-static inline void W(u32 *key, unsigned int i) {
+static void W(u32 *key, unsigned int i) {
u32 I;
key[6] ^= F1(key[7], Tr[i % 4][0], Tm[i][0]);
key[5] ^= F2(key[6], Tr[i % 4][1], Tm[i][1]);
@@ -428,7 +428,7 @@ static int cast6_setkey(struct crypto_tfm *tfm, const u8 *in_key,
}
/*forward quad round*/
-static inline void Q (u32 * block, u8 * Kr, u32 * Km) {
+static void Q (u32 * block, u8 * Kr, u32 * Km) {
u32 I;
block[2] ^= F1(block[3], Kr[0], Km[0]);
block[1] ^= F2(block[2], Kr[1], Km[1]);
@@ -437,7 +437,7 @@ static inline void Q (u32 * block, u8 * Kr, u32 * Km) {
}
/*reverse quad round*/
-static inline void QBAR (u32 * block, u8 * Kr, u32 * Km) {
+static void QBAR (u32 * block, u8 * Kr, u32 * Km) {
u32 I;
block[3] ^= F1(block[0], Kr[3], Km[3]);
block[0] ^= F3(block[1], Kr[2], Km[2]);
--
1.5.0.6
On Mon, Jan 07, 2008 at 03:40:58PM +0200, Ilpo J?rvinen wrote:
> Bloat-o-meter shows rather high readings for cast6...
>
> crypto/cast6.c:
> cast6_setkey | -1310
> cast6_encrypt | -4567
> cast6_decrypt | -4561
> 3 functions changed, 10438 bytes removed, diff: -10438
>
> crypto/cast6.c:
> W | +659
> Q | +308
> QBAR | +316
> 3 functions changed, 1283 bytes added, diff: +1283
>
> crypto/cast6.o:
> 6 functions changed, 1283 bytes added, 10438 bytes removed, diff: -9155
>
> Signed-off-by: Ilpo J?rvinen <[email protected]>
Patch applied. Thanks a lot!
--
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
"Ilpo J?rvinen" <[email protected]> writes:
> Bloat-o-meter shows rather high readings for cast6...
Have you measured if the performance doesn't suffer from that
change? Inner loops of ciphers tend to be quite performance
sensitive and the inlines might actually help a lot.
On the other hand setkey is probably not as sensitive.
-Andi
On Thu, 10 Jan 2008, Andi Kleen wrote:
> "Ilpo J?rvinen" <[email protected]> writes:
>
> > Bloat-o-meter shows rather high readings for cast6...
>
> Have you measured if the performance doesn't suffer from that
> change? Inner loops of ciphers tend to be quite performance
> sensitive and the inlines might actually help a lot.
No, I haven't measured.
--
i.
On Thu, Jan 10, 2008 at 11:17:10AM +0200, Ilpo J?rvinen wrote:
> On Thu, 10 Jan 2008, Andi Kleen wrote:
>
> > "Ilpo J?rvinen" <[email protected]> writes:
> >
> > > Bloat-o-meter shows rather high readings for cast6...
> >
> > Have you measured if the performance doesn't suffer from that
> > change? Inner loops of ciphers tend to be quite performance
> > sensitive and the inlines might actually help a lot.
>
> No, I haven't measured.
Then I don't think the patch should have been applied.
-Andi
On Thu, Jan 10, 2008 at 10:25:55AM +0100, Andi Kleen wrote:
>
> Then I don't think the patch should have been applied.
I disagree. There isn't any evidence showing that the inlined version
is significantly faster either. In the absence of that, the version
with the smaller size is preferable.
Of course if anyone is keen enough about cast6 to provide benchmarks
showing that the inlined version is significantly faster then I'm
happy to revert the patch.
Thanks,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
On Thu, Jan 10, 2008 at 08:27:46PM +1100, Herbert Xu wrote:
> On Thu, Jan 10, 2008 at 10:25:55AM +0100, Andi Kleen wrote:
> >
> > Then I don't think the patch should have been applied.
>
> I disagree. There isn't any evidence showing that the inlined version
> is significantly faster either. In the absence of that, the version
> with the smaller size is preferable.
Just seems very risky to mess with cipher inner loops like that. Normally
people originally spent quite some effort tuning those; especially for
an AES candidate. Also putting quite a lot of function calls in there
doesn't seem right -- especially if frame pointer is enabled function
calls are not that cheap anymore. Also it messes up the register
allocation.
> Of course if anyone is keen enough about cast6 to provide benchmarks
If nobody is keen enough then the code could be just dropped
completely, couldn't it?
> showing that the inlined version is significantly faster then I'm
> happy to revert the patch.
There used to be papers with performance analysis of all the AES
candidates, but I cannot find them right now for CAST6. But I doubt
the code was written without proper thought.
-Andi
* Herbert Xu | 2008-01-10 20:27:46 [+1100]:
>On Thu, Jan 10, 2008 at 10:25:55AM +0100, Andi Kleen wrote:
>>
>> Then I don't think the patch should have been applied.
>
>I disagree. There isn't any evidence showing that the inlined version
>is significantly faster either. In the absence of that, the version
>with the smaller size is preferable.
I tried to get rid of all those macros in AES and replace them with
static only. I noticed that this makes the implementation slower. The
performance increased a little after switch to static inline but was
still less than with those macros. Unfortunately I had not the time to
investigate it any further.
Sebastian
On Thu, Jan 10, 2008 at 02:35:29PM +0100, Sebastian Siewior wrote:
> * Herbert Xu | 2008-01-10 20:27:46 [+1100]:
>
> >On Thu, Jan 10, 2008 at 10:25:55AM +0100, Andi Kleen wrote:
> >>
> >> Then I don't think the patch should have been applied.
> >
> >I disagree. There isn't any evidence showing that the inlined version
> >is significantly faster either. In the absence of that, the version
> >with the smaller size is preferable.
> I tried to get rid of all those macros in AES and replace them with
> static only. I noticed that this makes the implementation slower. The
Yes not unexpected. These crypto functions tend to be carefully tuned
(or at least their critical loops are) and changing inlines in carefully
tuned code is usually a bad idea.
-Andi
* Andi Kleen | 2008-01-10 16:46:50 [+0100]:
>On Thu, Jan 10, 2008 at 02:35:29PM +0100, Sebastian Siewior wrote:
>> * Herbert Xu | 2008-01-10 20:27:46 [+1100]:
>>
>> >On Thu, Jan 10, 2008 at 10:25:55AM +0100, Andi Kleen wrote:
>> >>
>> >> Then I don't think the patch should have been applied.
>> >
>> >I disagree. There isn't any evidence showing that the inlined version
>> >is significantly faster either. In the absence of that, the version
>> >with the smaller size is preferable.
>> I tried to get rid of all those macros in AES and replace them with
>> static only. I noticed that this makes the implementation slower. The
>
>Yes not unexpected. These crypto functions tend to be carefully tuned
>(or at least their critical loops are) and changing inlines in carefully
>tuned code is usually a bad idea.
While I was sitting in a train, I bench marked cast6 on my notebook, that's a
|bigeasy@kibibi /mnt/crypto/git/linux-2.6 $ cat /proc/cpuinfo
|processor : 0
|vendor_id : GenuineIntel
|cpu family : 6
|model : 13
|model name : Intel(R) Pentium(R) M processor 1.73GHz
|stepping : 8
|cpu MHz : 1733.000
|cache size : 2048 KB
|fdiv_bug : no
|hlt_bug : no
|f00f_bug : no
|coma_bug : no
|fpu : yes
|fpu_exception : yes
|cpuid level : 2
|wp : yes
|flags : fpu vme de pse tsc msr pae mce cx8 apic sep mtrr pge
|mca cmov pat clflush dts acpi mmx fxsr sse sse2 ss tm pbe bts est tm2
|bogomips : 3463.18
|clflush size : 64
on a
|Linux kibibi 2.6.24-rc6 #2 Sat Jan 5 17:52:39 CET 2008 i686 Intel(R)
|Pentium(R) M processor 1.73GHz GenuineIntel GNU/Linux
with CONFIG_NO_HZ=y & CONFIG_HZ=100.
I created a crypto partition via
|cryptsetup create --cipher cast6 bench /dev/hda3
and used ext2 on it.
First run was write + inline. The module was
|cast6 17216 0
|bigeasy@kibibi /mnt/usb/l $ dd if=/dev/zero of=basilimi
|dd: writing to `basilimi': No space left on device
|1826865+0 records in
|1826864+0 records out
|935354368 bytes (935 MB) copied, 49.1721 s, 19.0 MB/s
Then I run 'modprobe tcrypt mode=207' with a quick hack [1].
|testing speed of ecb(cast6) encryption
|test 0 (128 bit key, 16 byte blocks): 1 operation in 1049 cycles (16 bytes)
|test 1 (128 bit key, 64 byte blocks): 1 operation in 3398 cycles (64 bytes)
|test 2 (128 bit key, 256 byte blocks): 1 operation in 12828 cycles (256 bytes)
|test 3 (128 bit key, 1024 byte blocks): 1 operation in 50512 cycles (1024 bytes)
|test 4 (128 bit key, 8192 byte blocks): 1 operation in 403118 cycles (8192 bytes)
|test 5 (192 bit key, 16 byte blocks): 1 operation in 1045 cycles (16 bytes)
|test 6 (192 bit key, 64 byte blocks): 1 operation in 3397 cycles (64 bytes)
|test 7 (192 bit key, 256 byte blocks): 1 operation in 12828 cycles (256 bytes)
|test 8 (192 bit key, 1024 byte blocks): 1 operation in 50565 cycles (1024 bytes)
|test 9 (192 bit key, 8192 byte blocks): 1 operation in 406205 cycles (8192 bytes)
|test 10 (256 bit key, 16 byte blocks): 1 operation in 1044 cycles (16 bytes)
|test 11 (256 bit key, 64 byte blocks): 1 operation in 3399 cycles (64 bytes)
|test 12 (256 bit key, 256 byte blocks): 1 operation in 12834 cycles (256 bytes)
|test 13 (256 bit key, 1024 byte blocks): 1 operation in 50567 cycles (1024 bytes)
|test 14 (256 bit key, 8192 byte blocks): 1 operation in 403528 cycles (8192 bytes)
|
|testing speed of ecb(cast6) decryption
|test 0 (128 bit key, 16 byte blocks): 1 operation in 1044 cycles (16 bytes)
|test 1 (128 bit key, 64 byte blocks): 1 operation in 3419 cycles (64 bytes)
|test 2 (128 bit key, 256 byte blocks): 1 operation in 12865 cycles (256 bytes)
|test 3 (128 bit key, 1024 byte blocks): 1 operation in 50683 cycles (1024 bytes)
|test 4 (128 bit key, 8192 byte blocks): 1 operation in 404385 cycles (8192 bytes)
|test 5 (192 bit key, 16 byte blocks): 1 operation in 1043 cycles (16 bytes)
|test 6 (192 bit key, 64 byte blocks): 1 operation in 3419 cycles (64 bytes)
|test 7 (192 bit key, 256 byte blocks): 1 operation in 12866 cycles (256 bytes)
|test 8 (192 bit key, 1024 byte blocks): 1 operation in 50753 cycles (1024 bytes)
|test 9 (192 bit key, 8192 byte blocks): 1 operation in 407210 cycles (8192 bytes)
|test 10 (256 bit key, 16 byte blocks): 1 operation in 1043 cycles (16 bytes)
|test 11 (256 bit key, 64 byte blocks): 1 operation in 3419 cycles (64 bytes)
|test 12 (256 bit key, 256 byte blocks): 1 operation in 12863 cycles (256 bytes)
|test 13 (256 bit key, 1024 byte blocks): 1 operation in 50742 cycles (1024 bytes)
|test 14 (256 bit key, 8192 byte blocks): 1 operation in 404250 cycles (8192 bytes)
After that, I removed inline and tried again, the module shrank to
|cast6 8704 0
Same procedure
|bigeasy@kibibi /mnt/usb/l $ dd if=/dev/zero of=basilimi
|dd: writing to `basilimi': No space left on device
|1826865+0 records in
|1826864+0 records out
|935354368 bytes (935 MB) copied, 47.9814 s, 19.5 MB/s
and tcrypt
|testing speed of ecb(cast6) encryption
|test 0 (128 bit key, 16 byte blocks): 1 operation in 929 cycles (16 bytes)
|test 1 (128 bit key, 64 byte blocks): 1 operation in 2951 cycles (64 bytes)
|test 2 (128 bit key, 256 byte blocks): 1 operation in 11062 cycles (256 bytes)
|test 3 (128 bit key, 1024 byte blocks): 1 operation in 43509 cycles (1024 bytes)
|test 4 (128 bit key, 8192 byte blocks): 1 operation in 347532 cycles (8192 bytes)
|test 5 (192 bit key, 16 byte blocks): 1 operation in 926 cycles (16 bytes)
|test 6 (192 bit key, 64 byte blocks): 1 operation in 2947 cycles (64 bytes)
|test 7 (192 bit key, 256 byte blocks): 1 operation in 11064 cycles (256 bytes)
|test 8 (192 bit key, 1024 byte blocks): 1 operation in 43503 cycles (1024 bytes)
|test 9 (192 bit key, 8192 byte blocks): 1 operation in 350597 cycles (8192 bytes)
|test 10 (256 bit key, 16 byte blocks): 1 operation in 926 cycles (16 bytes)
|test 11 (256 bit key, 64 byte blocks): 1 operation in 2953 cycles (64 bytes)
|test 12 (256 bit key, 256 byte blocks): 1 operation in 11063 cycles (256 bytes)
|test 13 (256 bit key, 1024 byte blocks): 1 operation in 43489 cycles (1024 bytes)
|test 14 (256 bit key, 8192 byte blocks): 1 operation in 347208 cycles (8192 bytes)
|
|testing speed of ecb(cast6) decryption
|test 0 (128 bit key, 16 byte blocks): 1 operation in 927 cycles (16 bytes)
|test 1 (128 bit key, 64 byte blocks): 1 operation in 2953 cycles (64 bytes)
|test 2 (128 bit key, 256 byte blocks): 1 operation in 11073 cycles (256 bytes)
|test 3 (128 bit key, 1024 byte blocks): 1 operation in 43571 cycles (1024 bytes)
|test 4 (128 bit key, 8192 byte blocks): 1 operation in 347875 cycles (8192 bytes)
|test 5 (192 bit key, 16 byte blocks): 1 operation in 922 cycles (16 bytes)
|test 6 (192 bit key, 64 byte blocks): 1 operation in 2955 cycles (64 bytes)
|test 7 (192 bit key, 256 byte blocks): 1 operation in 11080 cycles (256 bytes)
|test 8 (192 bit key, 1024 byte blocks): 1 operation in 43622 cycles (1024 bytes)
|test 9 (192 bit key, 8192 byte blocks): 1 operation in 351189 cycles (8192 bytes)
|test 10 (256 bit key, 16 byte blocks): 1 operation in 923 cycles (16 bytes)
|test 11 (256 bit key, 64 byte blocks): 1 operation in 2954 cycles (64 bytes)
|test 12 (256 bit key, 256 byte blocks): 1 operation in 11080 cycles (256 bytes)
|test 13 (256 bit key, 1024 byte blocks): 1 operation in 43601 cycles (1024 bytes)
|test 14 (256 bit key, 8192 byte blocks): 1 operation in 348054 cycles (8192 bytes)
The dd performance is pretty close. Maybe my hd isn't that fast...
|kibibi linux-2.6 # hdparm -a 0 /dev/hda
|
|/dev/hda:
| setting fs readahead to 0
| readahead = 0 (off)
|kibibi linux-2.6 # hdparm --direct -t /dev/hda
|
|/dev/hda:
| Timing O_DIRECT disk reads: 112 MB in 3.04 seconds = 36.87 MB/sec
|
Maybe it is. The same file but with read this time, first the inline edition
|bigeasy@kibibi /mnt/usb/l $ time dd of=/dev/null if=basilimi
|1826864+0 records in
|1826864+0 records out
|935354368 bytes (935 MB) copied, 47.5538 s, 19.7 MB/s
|
|real 0m47.617s
|user 0m0.670s
|sys 0m4.560s
|bigeasy@kibibi /mnt/usb/l $
and now without:
|bigeasy@kibibi /mnt/usb/l $ dd of=/dev/null if=basilimi
|1826864+0 records in
|1826864+0 records out
|935354368 bytes (935 MB) copied, 47.4728 s, 19.7 MB/s
|
|bigeasy@kibibi /mnt/usb/l $ time dd of=/dev/null if=basilimi
|1826864+0 records in
|1826864+0 records out
|935354368 bytes (935 MB) copied, 47.9194 s, 19.5 MB/s
|
|real 0m47.948s
|user 0m0.760s
|sys 0m4.250s
|
|bigeasy@kibibi /mnt/usb/l $ time dd of=/dev/zero if=basilimi
|1826864+0 records in
|1826864+0 records out
|935354368 bytes (935 MB) copied, 47.2979 s, 19.8 MB/s
|
|real 0m47.302s
|user 0m0.730s
|sys 0m4.540s
|bigeasy@kibibi /mnt/usb/l $ time dd of=/dev/zero if=basilimi
|1826864+0 records in
|1826864+0 records out
|935354368 bytes (935 MB) copied, 47.6229 s, 19.6 MB/s
|
|real 0m47.687s
|user 0m0.800s
|sys 0m4.230s
|bigeasy@kibibi /mnt/usb/l $
The inline and not inline performance is quite similar. I guess the
little difference here and there is due to some random ctx switches (I
had almost nothing running what includes an idle firefox on another
virtual desktop, wmnd & friends and fluxbox).
The tcrypt test which run without any kind of interruption performed
better on larger blocks and was even better on the smallest block.
Long story short, according to this numbers I'm all for the not inline
version.
[1]
diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
index 1ab8c01..a935abc 100644
--- a/crypto/tcrypt.c
+++ b/crypto/tcrypt.c
@@ -1705,6 +1705,12 @@ static void do_test(void)
test_cipher_speed("salsa20", ENCRYPT, sec, NULL, 0,
salsa20_speed_template);
break;
+ case 207:
+ test_cipher_speed("ecb(cast6)", ENCRYPT, sec, NULL, 0,
+ camellia_speed_template);
+ test_cipher_speed("ecb(cast6)", DECRYPT, sec, NULL, 0,
+ camellia_speed_template);
+ break;
case 300:
/* fall through */
>
>-Andi
Sebastian
On Sat, Jan 12, 2008 at 01:09:37AM +0100, Sebastian Siewior wrote:
>
> Long story short, according to this numbers I'm all for the not inline
> version.
Thanks for following this through!
> diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
> index 1ab8c01..a935abc 100644
> --- a/crypto/tcrypt.c
> +++ b/crypto/tcrypt.c
> @@ -1705,6 +1705,12 @@ static void do_test(void)
> test_cipher_speed("salsa20", ENCRYPT, sec, NULL, 0,
> salsa20_speed_template);
> break;
> + case 207:
> + test_cipher_speed("ecb(cast6)", ENCRYPT, sec, NULL, 0,
> + camellia_speed_template);
> + test_cipher_speed("ecb(cast6)", DECRYPT, sec, NULL, 0,
> + camellia_speed_template);
> + break;
Perhaps we should make a generic speed template that works for all
ciphers?
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
> The inline and not inline performance is quite similar. I guess the
> little difference here and there is due to some random ctx switches (I
Are you sure you were not just IO bound? It would have been better
to test in memory (e.g. using ramfs or just some direct test client)
-Andi
On Sat, Jan 12, 2008 at 03:47:41AM +0100, Andi Kleen wrote:
> > The inline and not inline performance is quite similar. I guess the
> > little difference here and there is due to some random ctx switches (I
>
> Are you sure you were not just IO bound? It would have been better
> to test in memory (e.g. using ramfs or just some direct test client)
He tested with tcrypt which is definitely not IO bound.
Cheers,
--
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
* Herbert Xu | 2008-01-12 11:57:42 [+1100]:
>
>Thanks for following this through!
np
>
>> diff --git a/crypto/tcrypt.c b/crypto/tcrypt.c
>> index 1ab8c01..a935abc 100644
>> --- a/crypto/tcrypt.c
>> +++ b/crypto/tcrypt.c
>> @@ -1705,6 +1705,12 @@ static void do_test(void)
>> test_cipher_speed("salsa20", ENCRYPT, sec, NULL, 0,
>> salsa20_speed_template);
>> break;
>> + case 207:
>> + test_cipher_speed("ecb(cast6)", ENCRYPT, sec, NULL, 0,
>> + camellia_speed_template);
>> + test_cipher_speed("ecb(cast6)", DECRYPT, sec, NULL, 0,
>> + camellia_speed_template);
>> + break;
>
>Perhaps we should make a generic speed template that works for all
>ciphers?
Most of them aren't that different, I try to make something.
>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
>-
>To unsubscribe from this list: send the line "unsubscribe linux-crypto" in
>the body of a message to [email protected]
>More majordomo info at http://vger.kernel.org/majordomo-info.html