On Fri, Oct 11, 2019 at 10:57:40AM -0700, Eric Biggers wrote: > The choice of data lengths seems a bit unusual, as they include every length in > two ranges but nothing in between. Also, none of the lengths except 0 is a > multiple of the blake2b block size. Instead, maybe use something like > [0, 1, 7, 15, 64, 247, 256]? Just to clarify, do you mean the block size defined by BLAKE2B_BLOCKBYTES? That's 128, so that makes 0 and 256 the multiples. > Also, since the 4 variants share nearly all their code, it seems the tests would > be just as effective in practice if we cut the test vectors down by 4x by > distributing the key lengths among each variant. Like: > > blake2b-160 blake2b-256 blake2b-384 blake2b-512 > --------------------------------------------------- > len=0 | klen=0 klen=1 klen=16 klen=32 > len=1 | klen=16 klen=32 klen=0 klen=1 > len=7 | klen=32 klen=0 klen=1 klen=16 > len=15 | klen=1 klen=16 klen=32 klen=0 > len=64 | klen=0 klen=1 klen=16 klen=32 > len=247 | klen=16 klen=32 klen=0 klen=1 > len=256 | klen=32 klen=0 klen=1 klen=16 That's clever. I assume the 32 key length refers to the default key, right? That's 64 bytes (BLAKE2B_KEYBYTES), so I'll use that value. > > Testing performed: > > > > - compiled with SLUB_DEBUG and KASAN, plus crypto selftests > > CONFIG_CRYPTO_MANAGER2=y > > CONFIG_CRYPTO_MANAGER_DISABLE_TESTS=n > > CONFIG_CRYPTO_MANAGER_EXTRA_TESTS=y > > - module loaded, no errors reported from the tessuite > > - (un)intentionally broken test values were detected > > > > The test values were produced by b2sum, compiled from the reference > > implementation. The generated values were cross-checked by pyblake2 > > based script (ie. not the same sources, built by distro). > > > > The .h portion of testmgr is completely generated, so in case somebody feels > > like reducing it in size, adding more keys, changing the formatting, it's easy > > to do. > > > > > In case the patches don't make it to the mailinglist, it's in git > > > > git://git.kernel.org/pub/scm/linux/kernel/git/kdave/linux.git dev/blake2b-v4 > > Can you please rebase this onto cryptodev/master? Will do. Thanks for the comments.
