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
