On Mon, Jul 29, 2019 at 10:16:48PM +0000, Pascal Van Leeuwen wrote: > > > > Note that the "empty test suite" message shouldn't be printed (especially not at > > > > KERN_ERR level!) if it's working as intended. > > > > > > > That's not my code, that was already there. I already got these messages before my > > > modifications, for some ciphersuites. Of course if we don't want that, we can make > > > it a pr_warn pr_dbg? > > > > I didn't get these error messages before this patch. They start showing up > > because this patch changes alg_test_null to alg_test_aead for algorithms with no > > test vectors. > > > Ok, I guess I caused it for some additional ciphersuites by forcing them > to be at least fuzz tested. But there were some ciphersuites without test > vectors already reporting this in my situation because they did not point > to alg_test_null in the first place. Are you sure? I don't see anything that had no test vectors but didn't use alg_test_null. > So it wasn't entirely clear what the > whole intention was in the first place, as it wasn't consistent. > If we all agree on the logging level we want for this message, then I can > make that change. I suggest at least downgrading it to KERN_INFO, since that's the level used for logging that there wasn't any test description found at all: printk(KERN_INFO "alg: No test for %s (%s)\n", alg, driver); > > > > > Why not put these new fields in the existing 'struct aead_test_suite'? > > > > > > > > I don't see the point of the separate 'params' struct. It just confuses things. > > > > > > > Mostly because I'm not that familiar with C datastructures (I'm not a programmer > > > and this is pretty much my first serious experience with C), so I didn't know how > > > to do that / didn't want to break anything else :-) > > > > > > So if you can provide some example on how to do that ... > > > > I'm simply suggesting adding the fields of 'struct aead_test_params' to > > 'struct aead_test_suite'. > > > My next mail tried to explain why that's not so simple ... The only actual issue is that you can't reuse the __VECS() macro because it adds an extra level of braces, right? > Actually, the patch *should* (didn't try yet) make it work for both: if both > alen and clen are valid (>=0) then it creates a key blob from those ranges. > If only clen is valid (>=0) but a alen is not (i.e., -1), then it will just > generate a random key the "normal" way with length clen. > So, for authenc you define both ranges, for other AEAD you define only a > cipher key length range with the auth key range count at 0. > Okay, I guess that makes sense. It wasn't obvious to me though. - Eric
