On Mon, Jul 29, 2019 at 04:10:06PM +0000, Pascal Van Leeuwen wrote:
> Hi Eric,
>
> > -----Original Message-----
> > From: linux-crypto-ow...@vger.kernel.org
> > <linux-crypto-ow...@vger.kernel.org> On Behalf Of
> > Pascal Van Leeuwen
> > Sent: Monday, July 29, 2019 11:11 AM
> > To: Eric Biggers <ebigg...@kernel.org>; Pascal van Leeuwen
> > <pascalv...@gmail.com>
> > Cc: linux-crypto@vger.kernel.org; herb...@gondor.apana.org.au;
> > da...@davemloft.net
> > Subject: RE: [PATCH] crypto: testmgr - Improve randomization of params for
> > AEAD fuzz
> > testing
> >
> > Hi Eric,
> >
> > Thanks for your feedback!
> >
> > > -----Original Message-----
> > > From: Eric Biggers <ebigg...@kernel.org>
> > > Sent: Sunday, July 28, 2019 7:31 PM
> > > To: Pascal van Leeuwen <pascalv...@gmail.com>
> > > Cc: linux-crypto@vger.kernel.org; herb...@gondor.apana.org.au;
> > > da...@davemloft.net;
> > Pascal Van Leeuwen
> > > <pvanleeu...@verimatrix.com>
> > > Subject: Re: [PATCH] crypto: testmgr - Improve randomization of params
> > > for AEAD fuzz
> > testing
> > > >
> > > > +struct len_range_set {
> > > > + const struct len_range_sel *lensel;
> > > > + unsigned int count;
> > > > +};
> > > > +
> > > > struct aead_test_suite {
> > > > const struct aead_testvec *vecs;
> > > > unsigned int count;
> > > > };
> > > >
> > > > +struct aead_test_params {
> > > > + struct len_range_set ckeylensel;
> > > > + struct len_range_set akeylensel;
> > > > + struct len_range_set authsizesel;
> > > > + struct len_range_set aadlensel;
> > > > + struct len_range_set ptxtlensel;
> > > > +};
> > > > +
> > > > struct cipher_test_suite {
> > > > const struct cipher_testvec *vecs;
> > > > unsigned int count;
> > > > @@ -143,6 +156,10 @@ struct alg_test_desc {
> > > > struct akcipher_test_suite akcipher;
> > > > struct kpp_test_suite kpp;
> > > > } suite;
> > > > +
> > > > + union {
> > > > + struct aead_test_params aead;
> > > > + } params;
> > > > };
> > >
> > > 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 ...
> >
> Actually, while looking into some way to combine these fields into
> 'struct aead_test_suite': I really can't think of a way to do that that
> would be as convenient as the current approach which allows me to:
>
> - NOT have these params for the other types (cipher, comp, hash etc.), at
> least for now
> - NOT have to touch any declarations in the alg_test_desc assignment that
> do not need this
> - conveniently use a macro line __LENS (idea shamelessly borrowed from
> __VECS) to assign the struct ptr / list length fields pairs
>
> If you know of a better way to achieve all that, then feel free to teach
> me. But, frankly I do not see why having 1 entry defining the testsuite
> and a seperate entry defining the fuzz test parameters would necessarily
> be confusing? Apart from 'params' perhaps not being a really good name,
> being too generic and all, 'fuzz_params' would probably be better?
>
Doesn't simply putting the fields in 'struct aead_test_suite' work?
The reason the current approach confuses me is that it's unclear what should go
in the aead_test_suite and what should go in the aead_test_params, both now and
in the future as people add new stuff. They seem like the same thing to me.
- Eric
