[dpdk-dev] [PATCH 0/3] AES GCM, AES CMAC fixes and addition of GCM tests for QAT.

[dpdk-dev] [PATCH 0/3] AES GCM, AES CMAC fixes and addition of GCM tests for QAT.

[John Griffin]

This patchset solves an issue in QAT driver, that was giving
invalid AES GCM results, due to incorrect IV setting.

It adds unit tests to validate AES GCM in QAT.

It also fixes the premature addition of AES CMAC support which was added to
the code in error.  AES CMAC will be added in a subsequent release
when testing completes.
AES CMAC was not advertised in the qat documentation.

This patchset depends on patches:
- aesni_gcm: PMD to support AES_GCM crypto operations
  (http://dpdk.org/dev/patchwork/patch/11201/)

John Griffin (3):
  qat: fix AES GCM decryption
  app/test: add AES GCM tests for QAT
  qat: fixes premature addition of AES_CMAC in session

 app/test/test_cryptodev.c              | 34 +++++++++++++++++++++++++++++++++-
 doc/guides/cryptodevs/qat.rst          |  1 +
 doc/guides/rel_notes/release_16_04.rst |  5 +++++
 drivers/crypto/qat/qat_crypto.c        | 24 ++++++++++++++++++++----
 4 files changed, 59 insertions(+), 5 deletions(-)

-- 
2.1.0

[dpdk-dev] [PATCH 1/3] qat: fix AES GCM decryption

[John Griffin]

AES GCM on the cryptodev API was giving invalid results
in some cases, due to an incorrect IV setting.

Added AES GCM in the QAT supported algorithms,
as encryption/decryption is fully functional.

Fixes: 1703e94ac5ce ("qat: add driver for QuickAssist devices")

Signed-off-by: John Griffin <john.griffin@intel.com>
---
 doc/guides/cryptodevs/qat.rst          |  1 +
 doc/guides/rel_notes/release_16_04.rst |  5 +++++
 drivers/crypto/qat/qat_crypto.c        | 22 +++++++++++++++++++---
 3 files changed, 25 insertions(+), 3 deletions(-)

diff --git a/doc/guides/cryptodevs/qat.rst b/doc/guides/cryptodevs/qat.rst
index af52047..ec4d6c6 100644
--- a/doc/guides/cryptodevs/qat.rst
+++ b/doc/guides/cryptodevs/qat.rst
@@ -48,6 +48,7 @@ Cipher algorithms:
 * ``RTE_CRYPTO_SYM_CIPHER_AES192_CBC``
 * ``RTE_CRYPTO_SYM_CIPHER_AES256_CBC``
 * ``RTE_CRYPTO_SYM_CIPHER_SNOW3G_UEA2``
+* ``RTE_CRYPTO_CIPHER_AES_GCM``
 
 Hash algorithms:
 
diff --git a/doc/guides/rel_notes/release_16_04.rst b/doc/guides/rel_notes/release_16_04.rst
index d7a264a..ee8d141 100644
--- a/doc/guides/rel_notes/release_16_04.rst
+++ b/doc/guides/rel_notes/release_16_04.rst
@@ -99,6 +99,11 @@ Drivers
   This made impossible the creation of more than one aesni_mb device
   from command line.
 
+* **qat: Fixed AES GCM decryption.**
+
+  Allowed AES GCM on the cryptodev API, but in some cases gave invalid results
+  due to incorrect IV setting.
+
 
 Libraries
 ~~~~~~~~~
diff --git a/drivers/crypto/qat/qat_crypto.c b/drivers/crypto/qat/qat_crypto.c
index cb16aae..48e810f 100644
--- a/drivers/crypto/qat/qat_crypto.c
+++ b/drivers/crypto/qat/qat_crypto.c
@@ -529,11 +529,27 @@ qat_write_hw_desc_entry(struct rte_crypto_op *op, uint8_t *out_msg)
 	auth_param->u1.aad_adr = op->sym->auth.aad.phys_addr;
 	/* (GCM) aad length(240 max) will be at this location after precompute */
 	if (ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_128 ||
-		ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_64) {
-		auth_param->u2.aad_sz =
-		ALIGN_POW2_ROUNDUP(ctx->cd.hash.sha.state1[
+			ctx->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_64) {
+		struct icp_qat_hw_auth_algo_blk *hash;
+
+		if (ctx->qat_cmd == ICP_QAT_FW_LA_CMD_HASH_CIPHER)
+			hash = (struct icp_qat_hw_auth_algo_blk *)((char *)&ctx->cd);
+		else
+			hash = (struct icp_qat_hw_auth_algo_blk *)((char *)&ctx->cd +
+				sizeof(struct icp_qat_hw_cipher_algo_blk));
+
+		auth_param->u2.aad_sz = ALIGN_POW2_ROUNDUP(hash->sha.state1[
 					ICP_QAT_HW_GALOIS_128_STATE1_SZ +
 					ICP_QAT_HW_GALOIS_H_SZ + 3], 16);
+		if (op->sym->cipher.iv.length == 12) {
+			/*
+			 * For GCM a 12 bit IV is allowed,
+			 * but we need to inform the f/w
+			 */
+			ICP_QAT_FW_LA_GCM_IV_LEN_FLAG_SET(
+				qat_req->comn_hdr.serv_specif_flags,
+				ICP_QAT_FW_LA_GCM_IV_LEN_12_OCTETS);
+		}
 	}
 	auth_param->hash_state_sz = (auth_param->u2.aad_sz) >> 3;
 
-- 
2.1.0

[dpdk-dev] [PATCH 2/3] app/test: add AES GCM tests for QAT

[John Griffin]

Signed-off-by: John Griffin <john.griffin@intel.com>
---
 app/test/test_cryptodev.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/app/test/test_cryptodev.c b/app/test/test_cryptodev.c
index d7e80c4..a5d4208 100644
--- a/app/test/test_cryptodev.c
+++ b/app/test/test_cryptodev.c
@@ -3420,6 +3420,39 @@ static struct unit_test_suite cryptodev_qat_testsuite  = {
 		TEST_CASE_ST(ut_setup, ut_teardown,
 				test_AES_CBC_HMAC_AES_XCBC_decrypt_digest_verify),
 		TEST_CASE_ST(ut_setup, ut_teardown, test_stats),
+
+		/** AES GCM Authenticated Encryption */
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_encryption_test_case_1),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_encryption_test_case_2),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_encryption_test_case_3),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_encryption_test_case_4),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_encryption_test_case_5),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_encryption_test_case_6),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_encryption_test_case_7),
+
+		/** AES GCM Authenticated Decryption */
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_decryption_test_case_1),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_decryption_test_case_2),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_decryption_test_case_3),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_decryption_test_case_4),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_decryption_test_case_5),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_decryption_test_case_6),
+		TEST_CASE_ST(ut_setup, ut_teardown,
+			test_mb_AES_GCM_authenticated_decryption_test_case_7),
+
 		/** Snow3G encrypt only (UEA2) */
 		TEST_CASE_ST(ut_setup, ut_teardown,
 			test_snow3g_encryption_test_case_1),
@@ -3432,7 +3465,6 @@ static struct unit_test_suite cryptodev_qat_testsuite  = {
 		TEST_CASE_ST(ut_setup, ut_teardown,
 			test_snow3g_encryption_test_case_5),
 
-
 		/** Snow3G decrypt only (UEA2) */
 		TEST_CASE_ST(ut_setup, ut_teardown,
 			test_snow3g_decryption_test_case_1),
-- 
2.1.0

[dpdk-dev] [PATCH 3/3] qat: fixes premature addition of AES_CMAC in session creation

[John Griffin]

Remove support for AES CMAC support for which was added to
the code in error.  AES CMAC will be added in a subsequent release
when testing completes.

Fixes: 1703e94ac5ce ("qat: add driver for QuickAssist devices")

Signed-off-by: John Griffin <john.griffin@intel.com>
---
 drivers/crypto/qat/qat_crypto.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/crypto/qat/qat_crypto.c b/drivers/crypto/qat/qat_crypto.c
index 48e810f..366a064 100644
--- a/drivers/crypto/qat/qat_crypto.c
+++ b/drivers/crypto/qat/qat_crypto.c
@@ -295,7 +295,6 @@ qat_crypto_sym_configure_session_auth(struct rte_cryptodev *dev,
 		session->qat_hash_alg = ICP_QAT_HW_AUTH_ALGO_AES_XCBC_MAC;
 		break;
 	case RTE_CRYPTO_AUTH_AES_GCM:
-	case RTE_CRYPTO_AUTH_AES_GMAC:
 		session->qat_hash_alg = ICP_QAT_HW_AUTH_ALGO_GALOIS_128;
 		break;
 	case RTE_CRYPTO_AUTH_SNOW3G_UIA2:
@@ -312,6 +311,7 @@ qat_crypto_sym_configure_session_auth(struct rte_cryptodev *dev,
 	case RTE_CRYPTO_AUTH_MD5:
 	case RTE_CRYPTO_AUTH_MD5_HMAC:
 	case RTE_CRYPTO_AUTH_AES_CCM:
+	case RTE_CRYPTO_AUTH_AES_GMAC:
 	case RTE_CRYPTO_AUTH_KASUMI_F9:
 	case RTE_CRYPTO_AUTH_AES_CMAC:
 	case RTE_CRYPTO_AUTH_AES_CBC_MAC:
-- 
2.1.0

Re: [dpdk-dev] [PATCH 3/3] qat: fixes premature addition of AES_CMAC in session creation

[Thomas Monjalon]

2016-03-08 16:22, John Griffin:
> Remove support for AES CMAC support for which was added to
> the code in error.  AES CMAC will be added in a subsequent release
> when testing completes.
[...]
>  	case RTE_CRYPTO_AUTH_AES_GCM:
> -	case RTE_CRYPTO_AUTH_AES_GMAC:

CMAC or GMAC?

>  		session->qat_hash_alg = ICP_QAT_HW_AUTH_ALGO_GALOIS_128;
>  		break;
>  	case RTE_CRYPTO_AUTH_SNOW3G_UIA2:
> @@ -312,6 +311,7 @@ qat_crypto_sym_configure_session_auth(struct rte_cryptodev *dev,
>  	case RTE_CRYPTO_AUTH_MD5:
>  	case RTE_CRYPTO_AUTH_MD5_HMAC:
>  	case RTE_CRYPTO_AUTH_AES_CCM:
> +	case RTE_CRYPTO_AUTH_AES_GMAC:

So it is added? What should we have later?

Re: [dpdk-dev] [PATCH 3/3] qat: fixes premature addition of AES_CMAC in session creation

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Friday, March 11, 2016 12:17 AM
> To: Griffin, John
> Cc: dev@dpdk.org; De Lara Guarch, Pablo
> Subject: Re: [dpdk-dev] [PATCH 3/3] qat: fixes premature addition of
> AES_CMAC in session creation
> 
> 2016-03-08 16:22, John Griffin:
> > Remove support for AES CMAC support for which was added to
> > the code in error.  AES CMAC will be added in a subsequent release
> > when testing completes.
> [...]
> >  	case RTE_CRYPTO_AUTH_AES_GCM:
> > -	case RTE_CRYPTO_AUTH_AES_GMAC:
> 
> CMAC or GMAC?

It is AES_GMAC, I commented that title was wrong.

> 
> >  		session->qat_hash_alg =
> ICP_QAT_HW_AUTH_ALGO_GALOIS_128;
> >  		break;
> >  	case RTE_CRYPTO_AUTH_SNOW3G_UIA2:
> > @@ -312,6 +311,7 @@ qat_crypto_sym_configure_session_auth(struct
> rte_cryptodev *dev,
> >  	case RTE_CRYPTO_AUTH_MD5:
> >  	case RTE_CRYPTO_AUTH_MD5_HMAC:
> >  	case RTE_CRYPTO_AUTH_AES_CCM:
> > +	case RTE_CRYPTO_AUTH_AES_GMAC:
> 
> So it is added? What should we have later?

Re: [dpdk-dev] [PATCH 3/3] qat: fixes premature addition of AES_CMAC in session creation

[John Griffin]

On 11/03/16 00:16, Thomas Monjalon wrote:
> 2016-03-08 16:22, John Griffin:
>> Remove support for AES CMAC support for which was added to
>> the code in error.  AES CMAC will be added in a subsequent release
>> when testing completes.
> [...]
>>   	case RTE_CRYPTO_AUTH_AES_GCM:
>> -	case RTE_CRYPTO_AUTH_AES_GMAC:
>
> CMAC or GMAC?

Yes GMAC - thanks Pablo and Thomas.

Re: [dpdk-dev] [PATCH 0/3] AES GCM, AES CMAC fixes and addition of GCM tests for QAT.

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of John Griffin
> Sent: Tuesday, March 08, 2016 4:22 PM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH 0/3] AES GCM, AES CMAC fixes and addition of
> GCM tests for QAT.
> 
> This patchset solves an issue in QAT driver, that was giving
> invalid AES GCM results, due to incorrect IV setting.
> 
> It adds unit tests to validate AES GCM in QAT.
> 
> It also fixes the premature addition of AES CMAC support which was added
> to
> the code in error.  AES CMAC will be added in a subsequent release
> when testing completes.
> AES CMAC was not advertised in the qat documentation.
> 
> This patchset depends on patches:
> - aesni_gcm: PMD to support AES_GCM crypto operations
>   (http://dpdk.org/dev/patchwork/patch/11201/)
> 
> John Griffin (3):
>   qat: fix AES GCM decryption
>   app/test: add AES GCM tests for QAT
>   qat: fixes premature addition of AES_CMAC in session
> 
>  app/test/test_cryptodev.c              | 34
> +++++++++++++++++++++++++++++++++-
>  doc/guides/cryptodevs/qat.rst          |  1 +
>  doc/guides/rel_notes/release_16_04.rst |  5 +++++
>  drivers/crypto/qat/qat_crypto.c        | 24 ++++++++++++++++++++----
>  4 files changed, 59 insertions(+), 5 deletions(-)
> 
> --
> 2.1.0

Series-acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Just a small comment: there is a type in the title of last patch,
It should be "premature addition of AES_GMAC in session"

Re: [dpdk-dev] [PATCH 0/3] AES GCM, AES CMAC fixes and addition of GCM tests for QAT.

[Thomas Monjalon]

> > John Griffin (3):
> >   qat: fix AES GCM decryption
> >   app/test: add AES GCM tests for QAT
> >   qat: fixes premature addition of AES_CMAC in session
> 
> Series-acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> 
> Just a small comment: there is a type in the title of last patch,
> It should be "premature addition of AES_GMAC in session"

Applied, thanks