Antoine,
> -----Original Message-----
> From: linux-crypto-owner@xxxxxxxxxxxxxxx <linux-crypto-owner@xxxxxxxxxxxxxxx> On Behalf Of Antoine Tenart
> Sent: Friday, July 26, 2019 3:47 PM
> To: Pascal Van Leeuwen <pvanleeuwen@xxxxxxxxxxxxxx>
> Cc: Antoine Tenart <antoine.tenart@xxxxxxxxxxx>; Pascal van Leeuwen <pascalvanl@xxxxxxxxx>; linux-crypto@xxxxxxxxxxxxxxx;
> herbert@xxxxxxxxxxxxxxxxxxx; davem@xxxxxxxxxxxxx
> Subject: Re: [PATCH 2/3] crypto: inside-secure - added support for rfc3686(ctr(aes))
>
> Hi Pascal,
>
> On Fri, Jul 26, 2019 at 01:28:13PM +0000, Pascal Van Leeuwen wrote:
> > > On Fri, Jul 05, 2019 at 08:49:23AM +0200, Pascal van Leeuwen wrote:
> >
> > > > @@ -62,9 +63,9 @@ static void safexcel_skcipher_token(struct safexcel_cipher_ctx *ctx, u8 *iv,
> > > > u32 length)
> > > > - if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC) {
> > > > + if (ctx->mode != CONTEXT_CONTROL_CRYPTO_MODE_ECB) {
> > >
> > > I think it's better for maintenance and readability to have something
> > > like:
> > >
> > > if (ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CBC ||
> > > ctx->mode == CONTEXT_CONTROL_CRYPTO_MODE_CTR_LOAD)
> > >
> > Not really. I *really* want to execute that for any mode other than ECB,
> > ECB being the *only* mode that does not require an IV (which I know
> > for a fact, being the architect and all :-).
> > And I don't believe a long list of modes that *do* require an IV would
> > be more readable or easy to maintain than this single compare ...
>
> That's where I disagree as you need extra knowledge to be aware of this.
> Being explicit removes any possible question one may ask. But that's a
> small point really :)
>
Well, while we're disagreeing ... I disagree with your assertion that you
would need more knowledge to know which modes do NOT need an IV
than to know which modes DO need an IV. There's really no fundamental
difference, it's two sides of the exact same coin ... you need that
knowledge either way.
That being said:
1) This code is executed for each individual cipher call, i.e. it's in the
critical path. Having just 1 compare-and-branch there is better for
performance than having many.
2) Generally, all else being equal, having less code is easier to maintain
than having more code.
3) Stuffing an IV in the token while you don't need it is harmless (apart
from wasting some cycles). NOT stuffing an IV in the token while you DO
need it is fatal. i.e. the single compare always errs on the safe side ...
4) If there is anything unclear about an otherwise fine code construct,
then you clarify it by adding a comment, not by rewriting it to be inefficient
and redundant ;-)
> Thanks!
> Antoine
>
> --
> Antoine Ténart, Bootlin
> Embedded Linux and Kernel engineering
> https://bootlin.com
Regards,
Pascal van Leeuwen
Silicon IP Architect, Multi-Protocol Engines @ Verimatrix
www.insidesecure.com
