Re: [PATCH 3/7] crypto: qce: Add mode for rfc4309

[Thara Gopinath]

On 4/5/21 6:32 PM, Bjorn Andersson wrote:
> On Thu 25 Feb 12:27 CST 2021, Thara Gopinath wrote:
>
> > rf4309 is the specification that uses aes ccm algorithms with IPsec
> > security packets. Add a submode to identify rfc4309 ccm(aes) algorithm
> > in the crypto driver.
> >
> > Signed-off-by: Thara Gopinath <thara.gopinath@xxxxxxxxxx>
> > ---
> >   drivers/crypto/qce/common.h | 7 +++++--
> >   1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/crypto/qce/common.h b/drivers/crypto/qce/common.h
> > index 3bc244bcca2d..3ffe719b79e4 100644
> > --- a/drivers/crypto/qce/common.h
> > +++ b/drivers/crypto/qce/common.h
> > @@ -51,9 +51,11 @@
> >   #define QCE_MODE_CCM			BIT(12)
> >   #define QCE_MODE_MASK			GENMASK(12, 8)
> >   
> > +#define QCE_MODE_CCM_RFC4309		BIT(13)
> > +
> >   /* cipher encryption/decryption operations */
> > -#define QCE_ENCRYPT			BIT(13)
> > -#define QCE_DECRYPT			BIT(14)
> > +#define QCE_ENCRYPT			BIT(14)
> > +#define QCE_DECRYPT			BIT(15)
>
> Can't we move these further up, so that next time we want to add
> something it doesn't require that we also move the ENC/DEC bits?

Yes I will change it to BIT(30) and BIT(31)

> >   
> >   #define IS_DES(flags)			(flags & QCE_ALG_DES)
> >   #define IS_3DES(flags)			(flags & QCE_ALG_3DES)
> > @@ -73,6 +75,7 @@
> >   #define IS_CTR(mode)			(mode & QCE_MODE_CTR)
> >   #define IS_XTS(mode)			(mode & QCE_MODE_XTS)
> >   #define IS_CCM(mode)			(mode & QCE_MODE_CCM)
> > +#define IS_CCM_RFC4309(mode)		((mode) & QCE_MODE_CCM_RFC4309)
>
> While leaving room for the typical macro issues, none of the other
> macros wrap the argument in parenthesis. Please follow the style of the
> driver, and perhaps follow up with a cleanup patch that just wraps them
> all in parenthesis?

This does throw up a checkpatch warning if I don't wrap "mode" in 
parenthesis. How about I keep this for now and I will follow up with a 
clean up for rest of the macros later ?

> Regards,
> Bjorn
>
> >   
> >   #define IS_ENCRYPT(dir)			(dir & QCE_ENCRYPT)
> >   #define IS_DECRYPT(dir)			(dir & QCE_DECRYPT)
> > --
> > 2.25.1

--
Warm Regards
Thara