Tcrypt hmac(crc32) test can work only on Blackfin

Tcrypt hmac(crc32) test can work only on Blackfin

[Milan Broz]

Hi,

commit a482b081a2d4d74d16bc9ea8779f9f6055f95852
Author: Sonic Zhang <sonic.zhang@analog.com>
Date:   Fri May 25 17:54:13 2012 +0800
    crypto: testmgr - Add new test cases for Blackfin CRC crypto driver

added tcrypt mode=110 test for hmac(crc32)

It seems, that this mode is only directly implemented by Blackfin
driver and must fail on all other architectures because:

- nobody implements "crc32" but "crc32c"

- the block size is 1 and digest size is 4 for crc32[c], so
hmac(crc32c) must fail because ds > block_size (test in hmac_create)

And it doesn't fail here because you implement it directly and
not using hmac.c...

Just try modprobe tcrypt mode=110 on so other arch
  alg: hash: Failed to load transform for hmac(crc32): -2

Why was such one-device dependent test vector added?

IMHO either it should provide crc32c (as do generic, sparc or x86 hw drivers)
or the block size for crc32c should be 4... (and test vector is wrong then)

But now we have test vector which must fail for most of systems.

Milan

RE: Tcrypt hmac(crc32) test can work only on Blackfin

[Zhang, Sonic]

>-----Original Message-----
>From: Milan Broz [mailto:mbroz@redhat.com]
>Sent: Tuesday, November 20, 2012 10:04 PM
>To: linux-crypto@vger.kernel.org
>Cc: Herbert Xu; Zhang, Sonic
>Subject: Tcrypt hmac(crc32) test can work only on Blackfin
>
>Hi,
>
>commit a482b081a2d4d74d16bc9ea8779f9f6055f95852
>Author: Sonic Zhang <sonic.zhang@analog.com>
>Date:   Fri May 25 17:54:13 2012 +0800
>    crypto: testmgr - Add new test cases for Blackfin CRC crypto driver
>
>added tcrypt mode=110 test for hmac(crc32)
>
>It seems, that this mode is only directly implemented by Blackfin driver and must
>fail on all other architectures because:
>
>- nobody implements "crc32" but "crc32c"
>
>- the block size is 1 and digest size is 4 for crc32[c], so
>hmac(crc32c) must fail because ds > block_size (test in hmac_create)
>
>And it doesn't fail here because you implement it directly and not using hmac.c...
>
>Just try modprobe tcrypt mode=110 on so other arch
>  alg: hash: Failed to load transform for hmac(crc32): -2
>
>Why was such one-device dependent test vector added?
>

Blackfin CRC engine is flexible to support many CRC32 polynomials (include crc32c) and is able to append 0 to any block size which is not a multiple of 4. This Blackfin CRC engine specific test vector show these capabilities. Is there a policy that the CRC test vector in testmgr.h should support all CRC drivers? If so, I am fine to drop this test vector.


>IMHO either it should provide crc32c (as do generic, sparc or x86 hw drivers) or
>the block size for crc32c should be 4... (and test vector is wrong then)
>

I can set the default polynomials to that of crc32c for blackfin crc32 driver. But, it should not be limited to crc32c.

Regards,

Sonic

>But now we have test vector which must fail for most of systems.
>
>Milan

Re: Tcrypt hmac(crc32) test can work only on Blackfin

[Milan Broz]

On 11/21/2012 12:29 PM, Zhang, Sonic wrote:
> Is there a policy that the CRC test vector
> in testmgr.h should support all CRC drivers?
> If so, I am fine to drop this test vector.

Question for Herbert...
But the problem I see is that it confuses people, it simply
returns fail everytime (except Blackfin platform).

My mail originates from bug report "tcrypt tests started failing",
and it was not obvious why (for people not familiar with tcrypt
internals, they just tried all test modes available).

>> IMHO either it should provide crc32c (as do generic, sparc or x86 hw drivers) or
>> the block size for crc32c should be 4... (and test vector is wrong then)
>>
> 
> I can set the default polynomials to that of crc32c for blackfin crc32 driver.
> But, it should not be limited to crc32c.

Sure, my question was just about test vector, hw driver support
for something more is of course good thing.

But it would be nice to have "crc32c" directly supported/accelerated as well.

Thanks,
Milan

Re: Tcrypt hmac(crc32) test can work only on Blackfin

[Herbert Xu]

On Wed, Nov 21, 2012 at 01:01:49PM +0100, Milan Broz wrote:
> 
> On 11/21/2012 12:29 PM, Zhang, Sonic wrote:
> > Is there a policy that the CRC test vector
> > in testmgr.h should support all CRC drivers?
> > If so, I am fine to drop this test vector.
> 
> Question for Herbert...
> But the problem I see is that it confuses people, it simply
> returns fail everytime (except Blackfin platform).
> 
> My mail originates from bug report "tcrypt tests started failing",
> and it was not obvious why (for people not familiar with tcrypt
> internals, they just tried all test modes available).

We should just make a generic version of crc32 available.

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

RE: Tcrypt hmac(crc32) test can work only on Blackfin

[Zhang, Sonic]

>-----Original Message-----
>From: Herbert Xu [mailto:herbert@gondor.hengli.com.au]
>Sent: Thursday, November 22, 2012 10:33 AM
>To: Milan Broz
>Cc: Zhang, Sonic; linux-crypto@vger.kernel.org
>Subject: Re: Tcrypt hmac(crc32) test can work only on Blackfin
>
>On Wed, Nov 21, 2012 at 01:01:49PM +0100, Milan Broz wrote:
>>
>> On 11/21/2012 12:29 PM, Zhang, Sonic wrote:
>> > Is there a policy that the CRC test vector in testmgr.h should
>> > support all CRC drivers?
>> > If so, I am fine to drop this test vector.
>>
>> Question for Herbert...
>> But the problem I see is that it confuses people, it simply returns
>> fail everytime (except Blackfin platform).
>>
>> My mail originates from bug report "tcrypt tests started failing", and
>> it was not obvious why (for people not familiar with tcrypt internals,
>> they just tried all test modes available).
>
>We should just make a generic version of crc32 available.
>

Hi Herbert and Broz,

Unfortunately, after investigating the hardware manual of Blackfin CRC engine and the CRC algorithm, I have to say the Blackfin CRC driver can't use current crc32c test vector in testmgr.h. Blackfin CRC engine only supports big endian CRC algorithm, while current crypto crc32c driver uses little endian CRC algorithm. The big endian CRC digest is different from that of the little endian.

There are 2 solutions. One is to add a big endian crc32c test vector. The other is to keep the Blackfin specific test vector. However, neither meets Broz's expectation of a generic crc32 test vector.

Any idea?

Regard,

Sonic

Re: Tcrypt hmac(crc32) test can work only on Blackfin

[Herbert Xu]

On Thu, Nov 22, 2012 at 03:38:45AM -0500, Zhang, Sonic wrote:
> 
> >On Wed, Nov 21, 2012 at 01:01:49PM +0100, Milan Broz wrote:
> >>
> >> On 11/21/2012 12:29 PM, Zhang, Sonic wrote:
> >> > Is there a policy that the CRC test vector in testmgr.h should
> >> > support all CRC drivers?
> >> > If so, I am fine to drop this test vector.
> >>
> >> Question for Herbert...
> >> But the problem I see is that it confuses people, it simply returns
> >> fail everytime (except Blackfin platform).
> >>
> >> My mail originates from bug report "tcrypt tests started failing", and
> >> it was not obvious why (for people not familiar with tcrypt internals,
> >> they just tried all test modes available).
> >
> >We should just make a generic version of crc32 available.
> >
> 
> Hi Herbert and Broz,
> 
> Unfortunately, after investigating the hardware manual of Blackfin CRC engine and the CRC algorithm, I have to say the Blackfin CRC driver can't use current crc32c test vector in testmgr.h. Blackfin CRC engine only supports big endian CRC algorithm, while current crypto crc32c driver uses little endian CRC algorithm. The big endian CRC digest is different from that of the little endian.
> 
> There are 2 solutions. One is to add a big endian crc32c test vector. The other is to keep the Blackfin specific test vector. However, neither meets Broz's expectation of a generic crc32 test vector.
> 
> Any idea?

I think we should just remove the blackfin crc driver altogether
as nothing in the kernel ever uses hmac(crc32).

Cheers,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

RE: Tcrypt hmac(crc32) test can work only on Blackfin

[Zhang, Sonic]

Hi Herbert,

>-----Original Message-----
>From: Herbert Xu [mailto:herbert@gondor.hengli.com.au]
>Sent: Thursday, December 06, 2012 5:07 PM
>To: Zhang, Sonic
>Cc: Milan Broz; linux-crypto@vger.kernel.org
>Subject: Re: Tcrypt hmac(crc32) test can work only on Blackfin
>
>On Thu, Nov 22, 2012 at 03:38:45AM -0500, Zhang, Sonic wrote:
>>
>> >On Wed, Nov 21, 2012 at 01:01:49PM +0100, Milan Broz wrote:
>> >>
>> >> On 11/21/2012 12:29 PM, Zhang, Sonic wrote:
>> >> > Is there a policy that the CRC test vector in testmgr.h should
>> >> > support all CRC drivers?
>> >> > If so, I am fine to drop this test vector.
>> >>
>> >> Question for Herbert...
>> >> But the problem I see is that it confuses people, it simply returns
>> >> fail everytime (except Blackfin platform).
>> >>
>> >> My mail originates from bug report "tcrypt tests started failing",
>> >> and it was not obvious why (for people not familiar with tcrypt
>> >> internals, they just tried all test modes available).
>> >
>> >We should just make a generic version of crc32 available.
>> >
>>
>> Hi Herbert and Broz,
>>
>> Unfortunately, after investigating the hardware manual of Blackfin CRC engine
>and the CRC algorithm, I have to say the Blackfin CRC driver can't use current
>crc32c test vector in testmgr.h. Blackfin CRC engine only supports big endian
>CRC algorithm, while current crypto crc32c driver uses little endian CRC
>algorithm. The big endian CRC digest is different from that of the little endian.
>>
>> There are 2 solutions. One is to add a big endian crc32c test vector. The other
>is to keep the Blackfin specific test vector. However, neither meets Broz's
>expectation of a generic crc32 test vector.
>>
>> Any idea?
>
>I think we should just remove the blackfin crc driver altogether as nothing in the
>kernel ever uses hmac(crc32).
>

Blackfin Linux applications may use the crypto bfin_crc driver. I wish you could remove the blackfin crc test patch from testmgr, while keeping the driver as it is.


Thanks,

Sonic

>Cheers,
>--
>Email: Herbert Xu <herbert@gondor.apana.org.au> Home Page:
>http://gondor.apana.org.au/~herbert/
>PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt