Re: what the heck is with camellia update?
Hi, On Thu, 20 Jul 2006 17:02:37 +0200 Gisle Vanem [EMAIL PROTECTED] wrote: One other thing is that cmll_loc.h includes intypes.h for non-MSCV targets. This header is not omni-present. A patch for djgpp at least: I think you can compile on non-MSVC with Camellia code which is included in OpenSSL after Dec. 2nd. Please try with it. Best regards, -- Masashi FUJITA MS Solution Businnes Group, NTT Software __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: what the heck is with camellia update?
On Thu, Jul 20, 2006 at 03:26:55PM +0200, Andy Polyakov wrote:
The time span between original submission and update suggests that
contributors were planning the update, meaning that the code was
considered work in progress all along. If so, why it went into stable
branch?
I understand that two different implementations were developed in
parallel, and in the end it was found that the one finished later
was found to provide better performance.
Notice that while the new code indeed is in the stable branch, it is
completely excluded from compilation with the default settings
(implicit -no_camellia). Otherwise I would not have accepted the
patch for 0.9.8-stable.
Then this update quality... It's just wrong on several points.
Most notably
#ifdef L_ENDIAN
...
#else /* big endian */
The expectation is
#ifdef L_ENDIAN
little-endian
#elif defined(B_ENDIAN)
big-endian
#else
endian *neutral*!
#endif
I mean undefined L_ENDIAN does not mean big-endian, not in OpenSSL
context. Furthermore
#if (defined (__GNUC__) !defined(i386))
#define CAMELLIA_SWAP4(x) \
do{\
asm(bswap %1 : +r (x));\
}while(0)
So if you try to compile with gcc on non-x86 platform, it will insist on
injecting x86 instruction...
True, this does not make sense.
Update appears as if they were trying to
improve performance, but I observe over 30% *degradation* on x86...
The stated reason for switching to the new implementation was
performance, however I don't know what specific platforms this applies
to.
A new patch taking into account the compliation problems found so far
(including the ones pointed out by Gisle Vanem) should be on the way ...
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: what the heck is with camellia update?
The time span between original submission and update suggests that contributors were planning the update, meaning that the code was considered work in progress all along. If so, why it went into stable branch? I understand that two different implementations were developed in parallel, So they didn't know for sure if update would be required, but it still hardly makes an argument for putting it into stable branch. The work was (and as it turns out still is:-) *ongoing*. and in the end it was found that the one finished later was found to provide better performance. As mentioned earlier, I don't see improvement, rather contrary. I can imagine they benchmark on some particular platform, which might explain why I see degradation on particular different elder P4 of mine. I mean it's known fact that what appear beneficial for one platform, might turn out nearly devastating for another. We should strive for implementation which provides best *all-round* performance. Say it's 13% faster on AMD, but twice slower on PIII, what do we choose? This is real-life example from rc4-586.pl. Well, it's kind of extreme, as you're unlikely to see such swings in C, yet you *can* find yourself in similar situation. Or take size vs. performance. Say you improve benchmark by few percent by increasing footprint several times. But how would it perform in real life, when caches are cold? No, we don't really know that, but I'd say few percents on benchmark result is hardly worth size blow-up. Update appears as if they were trying to improve performance, but I observe over 30% *degradation* on x86... The stated reason for switching to the new implementation was performance, however I don't know what specific platforms this applies to. Well, I find it hard to believe it's confidential, so let's ask them to tell all about it:-) I'm under impression at least one of them is on the list, if not - please forward. Which platform and what was improvement? I observe over 30% degradation on elder 2.4GHz P4. Elder refers to value of 0xF2? in eax register after cpuid(1). A new patch taking into account the compliation problems found so far (including the ones pointed out by Gisle Vanem) should be on the way ... Once again I want to point out that I see no reason for making code so platform dependent, that say DJGPP would require special treatment:-) Cheers. A. __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
what the heck is with camellia update?
The time span between original submission and update suggests that
contributors were planning the update, meaning that the code was
considered work in progress all along. If so, why it went into stable
branch? Then this update quality... It's just wrong on several points.
Most notably
#ifdef L_ENDIAN
...
#else /* big endian */
The expectation is
#ifdef L_ENDIAN
little-endian
#elif defined(B_ENDIAN)
big-endian
#else
endian *neutral*!
#endif
I mean undefined L_ENDIAN does not mean big-endian, not in OpenSSL
context. Furthermore
#if (defined (__GNUC__) !defined(i386))
#define CAMELLIA_SWAP4(x) \
do{\
asm(bswap %1 : +r (x));\
}while(0)
So if you try to compile with gcc on non-x86 platform, it will insist on
injecting x86 instruction... Update appears as if they were trying to
improve performance, but I observe over 30% *degradation* on x86... In
other words, what the heck... A.
__
OpenSSL Project http://www.openssl.org
Development Mailing List openssl-dev@openssl.org
Automated List Manager [EMAIL PROTECTED]
Re: what the heck is with camellia update?
One other thing is that cmll_loc.h includes intypes.h for non-MSCV targets. This header is not omni-present. A patch for djgpp at least: --- crypto\camellia\cmll_loc.org2006-07-20 17:01:50 +0200 +++ crypto\camellia\cmll_loc.h 2006-07-20 16:57:54 +0200 @@ -77,6 +77,8 @@ typedef unsigned char uint8_t; typedef unsigned int uint32_t; typedef unsigned __int64 uint64_t; +#elif defined(DJGPP) +#define uint32_t unsigned int #else #include inttypes.h #endif --gv __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
Re: what the heck is with camellia update?
One other thing is that cmll_loc.h includes intypes.h for non-MSCV targets. This header is not omni-present. A patch for djgpp at least: --- crypto\camellia\cmll_loc.org2006-07-20 17:01:50 +0200 +++ crypto\camellia\cmll_loc.h 2006-07-20 16:57:54 +0200 @@ -77,6 +77,8 @@ typedef unsigned char uint8_t; typedef unsigned int uint32_t; typedef unsigned __int64 uint64_t; +#elif defined(DJGPP) +#define uint32_t unsigned int #else #include inttypes.h #endif For the record. My standpoint is that it apparently needs some more generic reconsideration first. I mean I'd be reluctant to apply such platform specific patches and address the problem by the root. For example, as far as performance goes the whole update (or most of it) can be just reverted... A. __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager [EMAIL PROTECTED]
