Antoine,
> -----Original Message-----
> From: linux-crypto-owner@xxxxxxxxxxxxxxx <linux-crypto-owner@xxxxxxxxxxxxxxx> On Behalf Of Antoine Tenart
> Sent: Friday, July 26, 2019 3:07 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 1/3] crypto: inside-secure - add support for authenc(hmac(sha1),cbc(des3_ede))
>
> Hi Pascal,
>
> On Fri, Jul 26, 2019 at 12:57:21PM +0000, Pascal Van Leeuwen wrote:
> > > On Fri, Jul 05, 2019 at 08:49:22AM +0200, Pascal van Leeuwen wrote:
> > > > Signed-off-by: Pascal van Leeuwen <pvanleeuwen@xxxxxxxxxxxxxx>
> > >
> > > Could you provide a commit message, explaining briefly what the patch is
> > > doing?
> > >
> > I initially figured that to be redundant if the subject already covered it completely.
> > But now that I think of it, it's possible the subject does not end up in the commit
> > at all ... if that is the case, would it work if I just copy-paste the relevant part of the
> > subject message? Or do I need to be more verbose?
>
> The subject will be the commit title. I know sometimes the commit
> message is trivial or redundant, but it's still a good practice to
> always have one (and many maintainers will ask for one). Even if it's
> only two lines :)
>
Ok, good to know. I'm still learning how this works. I'll try and remember ;-)
> > > > @@ -199,6 +201,15 @@ static int safexcel_aead_aes_setkey(struct crypto_aead *ctfm, const u8 *key,
> > > > goto badkey;
> > > >
> > > > /* Encryption key */
> > > > + if (ctx->alg == SAFEXCEL_3DES) {
> > > > + flags = crypto_aead_get_flags(ctfm);
> > > > + err = __des3_verify_key(&flags, keys.enckey);
> > > > + crypto_aead_set_flags(ctfm, flags);
> > >
> > > You could use directly des3_verify_key() which does exactly this.
> > >
> > Actually, I couldn't due to des3_verify_key expecting a struct crypto_skcipher as input,
> > and not a struct crypto_aead, that's why I had to do it this way ...
>
> I see. Maybe a good way would be to provide a function taking
> 'struct crypto_aead' as an argument so that not every single driver
> reimplement the same logic. But this can come later if needed.
>
Agree. But being a newby and all, I did not dare to touch des.h itself ...
> > > > +struct safexcel_alg_template safexcel_alg_authenc_hmac_sha1_cbc_des3_ede = {
> > > > + .type = SAFEXCEL_ALG_TYPE_AEAD,
> > >
> > > You either missed to fill .engines member of this struct, or this series
> > > is based on another one not merged yet.
> > >
> > Yes, that happened in the patchset of which v2 did not make it to the mailing list ...
>
> :)
>
> So in general if there's a dependency you should say so in the cover
> letter.
>
I'll try to do that insofar I actually realise such a dependency exists ...
> 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
