diff mbox series

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

Message ID 20210225182716.1402449-4-thara.gopinath@linaro.org
State Superseded
Headers show
Series Add support for AEAD algorithms in Qualcomm Crypto Engine driver | expand

Commit Message

Thara Gopinath Feb. 25, 2021, 6:27 p.m. UTC
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@linaro.org>

---
 drivers/crypto/qce/common.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

-- 
2.25.1

Comments

Bjorn Andersson April 5, 2021, 10:32 p.m. UTC | #1
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@linaro.org>

> ---

>  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?

>  

>  #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?

Regards,
Bjorn

>  

>  #define IS_ENCRYPT(dir)			(dir & QCE_ENCRYPT)

>  #define IS_DECRYPT(dir)			(dir & QCE_DECRYPT)

> -- 

> 2.25.1

>
Thara Gopinath April 13, 2021, 7:30 p.m. UTC | #2
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@linaro.org>

>> ---

>>   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
Bjorn Andersson April 13, 2021, 8:38 p.m. UTC | #3
On Tue 13 Apr 14:30 CDT 2021, Thara Gopinath wrote:

> 

> 

> 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@linaro.org>

> > > ---

> > >   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 ?

> 


I don't have a problem with this approach.

Regards,
Bjorn

> > 

> > Regards,

> > Bjorn

> > 

> > >   #define IS_ENCRYPT(dir)			(dir & QCE_ENCRYPT)

> > >   #define IS_DECRYPT(dir)			(dir & QCE_DECRYPT)

> > > -- 

> > > 2.25.1

> > > 

> 

> -- 

> Warm Regards

> Thara
diff mbox series

Patch

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)
 
 #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)
 
 #define IS_ENCRYPT(dir)			(dir & QCE_ENCRYPT)
 #define IS_DECRYPT(dir)			(dir & QCE_DECRYPT)