diff mbox

[dpdk-dev] qat: addition of optimized content descriptor for AES128-SHA1-HMAC

Message ID 1466513759-55842-1-git-send-email-john.griffin@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Griffin, John June 21, 2016, 12:55 p.m. UTC 
  Adding an optimized content descriptor for AES128-SHA1-HMAC to
improve thoughput performance.

Signed-off-by: John Griffin <john.griffin@intel.com>
---
 drivers/crypto/qat/qat_adf/qat_algs.h            |  7 ++
 drivers/crypto/qat/qat_adf/qat_algs_build_desc.c | 91 ++++++++++++++++++++++++
 drivers/crypto/qat/qat_crypto.c                  | 64 +++++++++++++----
 drivers/crypto/qat/qat_crypto.h                  |  4 ++
 4 files changed, 154 insertions(+), 12 deletions(-)

Comments

Thomas Monjalon June 21, 2016, 1:49 p.m. UTC | #1 
Hi,

I'm not used to review crypto patches but I think this patch can
be improved.

2016-06-21 13:55, John Griffin:
> Adding an optimized content descriptor for AES128-SHA1-HMAC to
> improve thoughput performance.

Maybe you can explain how it improves the performance.

> +/*
> + * Function which will return if it's possible to use the
> + * optimised content descriptor.
> + */
> +int qat_crypto_sym_use_optimized_alg(struct qat_session *session)
[...]
> +/*
> + * Function to create an optimised content descriptor for AES128 SHA1.
> + */
> +int qat_crypto_create_optimzed_session(struct qat_session *session,

These function are very specific with a generic name.
Maybe that CBC AES128 SHA1 or something like that must be part of
the function name.
Otherwise you will come with yet another crypto refactoring patch in
few weeks.

>  	case ICP_QAT_FW_LA_CMD_CIPHER:
> -	session = qat_crypto_sym_configure_session_cipher(dev, xform, session);
> +		session = qat_crypto_sym_configure_session_cipher(dev, xform,
> +				session);
>  		break;
>  	case ICP_QAT_FW_LA_CMD_AUTH:
> -	session = qat_crypto_sym_configure_session_auth(dev, xform, session);
> +		session = qat_crypto_sym_configure_session_auth(dev, xform,
> +				session);
>  		break;
>  	case ICP_QAT_FW_LA_CMD_CIPHER_HASH:
> -	session = qat_crypto_sym_configure_session_cipher(dev, xform, session);
> -	session = qat_crypto_sym_configure_session_auth(dev, xform, session);
> -		break;
> +		session = qat_crypto_sym_configure_session_cipher(dev, xform,
> +				session);
> +		session = qat_crypto_sym_configure_session_auth(dev, xform,
> +				session);
> +		if (qat_crypto_sym_use_optimized_alg(session))
> +			qat_crypto_sym_configure_optimized_session(dev, xform,
> +				session);
> +	break;
>  	case ICP_QAT_FW_LA_CMD_HASH_CIPHER:
> -	session = qat_crypto_sym_configure_session_auth(dev, xform, session);
> -	session = qat_crypto_sym_configure_session_cipher(dev, xform, session);
> -		break;
> +		session = qat_crypto_sym_configure_session_auth(dev, xform,
> +				session);
> +		session = qat_crypto_sym_configure_session_cipher(dev, xform,
> +				session);
> +		if (qat_crypto_sym_use_optimized_alg(session))
> +			qat_crypto_sym_configure_optimized_session(dev, xform,
> +				session);
> +	break;

There is a lot of indent fixing mixed with the addition here.
2 patches would make things easier to understand.

> @@ -551,11 +591,11 @@ qat_crypto_sym_configure_session_auth(struct rte_cryptodev *dev,
>  				auth_xform->algo);
>  		goto error_out;
>  	}
> -	cipher_xform = qat_get_cipher_xform(xform);
>  
>  	if ((session->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_128) ||
>  			(session->qat_hash_alg ==
>  				ICP_QAT_HW_AUTH_ALGO_GALOIS_64))  {
> +		cipher_xform = qat_get_cipher_xform(xform);
>  		if (qat_alg_aead_session_create_content_desc_auth(session,

How this move is related to the patch?
Griffin, John June 21, 2016, 2:05 p.m. UTC | #2 
Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Tuesday, June 21, 2016 2:50 PM
> To: Griffin, John <john.griffin@intel.com>
> Cc: dev@dpdk.org; Jain, Deepak K <deepak.k.jain@intel.com>; De Lara
> Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: Re: [dpdk-dev] [PATCH] qat: addition of optimized content
> descriptor for AES128-SHA1-HMAC
> 
> Hi,
> 
> I'm not used to review crypto patches but I think this patch can be improved.
> 
> 2016-06-21 13:55, John Griffin:
> > Adding an optimized content descriptor for AES128-SHA1-HMAC to
> improve
> > thoughput performance.
> 
> Maybe you can explain how it improves the performance.
[JFG] Can add some text- it's though a reduction in the amount of the session information which
DMA'ed to the device - helps to reduce PCIe b/w.  I'll add some text.

> 
> > +/*
> > + * Function which will return if it's possible to use the
> > + * optimised content descriptor.
> > + */
> > +int qat_crypto_sym_use_optimized_alg(struct qat_session *session)
> [...]
> > +/*
> > + * Function to create an optimised content descriptor for AES128 SHA1.
> > + */
> > +int qat_crypto_create_optimzed_session(struct qat_session *session,
> 
> These function are very specific with a generic name.
> Maybe that CBC AES128 SHA1 or something like that must be part of the
> function name.
> Otherwise you will come with yet another crypto refactoring patch in few
> weeks.

[JFG] 
[JFG] Ok - I might not rename, but I'll make the implementation more generic to all it cope better with 
the potential addition of other algorithms.

> 
> >  	case ICP_QAT_FW_LA_CMD_CIPHER:
> > -	session = qat_crypto_sym_configure_session_cipher(dev, xform,
> session);
> > +		session = qat_crypto_sym_configure_session_cipher(dev,
> xform,
> > +				session);
> >  		break;
> >  	case ICP_QAT_FW_LA_CMD_AUTH:
> > -	session = qat_crypto_sym_configure_session_auth(dev, xform,
> session);
> > +		session = qat_crypto_sym_configure_session_auth(dev,
> xform,
> > +				session);
> >  		break;
> >  	case ICP_QAT_FW_LA_CMD_CIPHER_HASH:
> > -	session = qat_crypto_sym_configure_session_cipher(dev, xform,
> session);
> > -	session = qat_crypto_sym_configure_session_auth(dev, xform,
> session);
> > -		break;
> > +		session = qat_crypto_sym_configure_session_cipher(dev,
> xform,
> > +				session);
> > +		session = qat_crypto_sym_configure_session_auth(dev,
> xform,
> > +				session);
> > +		if (qat_crypto_sym_use_optimized_alg(session))
> > +			qat_crypto_sym_configure_optimized_session(dev,
> xform,
> > +				session);
> > +	break;
> >  	case ICP_QAT_FW_LA_CMD_HASH_CIPHER:
> > -	session = qat_crypto_sym_configure_session_auth(dev, xform,
> session);
> > -	session = qat_crypto_sym_configure_session_cipher(dev, xform,
> session);
> > -		break;
> > +		session = qat_crypto_sym_configure_session_auth(dev,
> xform,
> > +				session);
> > +		session = qat_crypto_sym_configure_session_cipher(dev,
> xform,
> > +				session);
> > +		if (qat_crypto_sym_use_optimized_alg(session))
> > +			qat_crypto_sym_configure_optimized_session(dev,
> xform,
> > +				session);
> > +	break;
> 
> There is a lot of indent fixing mixed with the addition here.
> 2 patches would make things easier to understand.

[JFG] 
[JFG] Ok let me remove from this patch.

> 
> > @@ -551,11 +591,11 @@ qat_crypto_sym_configure_session_auth(struct
> rte_cryptodev *dev,
> >  				auth_xform->algo);
> >  		goto error_out;
> >  	}
> > -	cipher_xform = qat_get_cipher_xform(xform);
> >
> >  	if ((session->qat_hash_alg ==
> ICP_QAT_HW_AUTH_ALGO_GALOIS_128) ||
> >  			(session->qat_hash_alg ==
> >  				ICP_QAT_HW_AUTH_ALGO_GALOIS_64))  {
> > +		cipher_xform = qat_get_cipher_xform(xform);
> >  		if
> (qat_alg_aead_session_create_content_desc_auth(session,
> 
> How this move is related to the patch?

 [JFG] It's not tbh - I'll remove from this patch.
diff mbox

Patch

diff --git a/drivers/crypto/qat/qat_adf/qat_algs.h b/drivers/crypto/qat/qat_adf/qat_algs.h
index b47dbc2..28fa111 100644
--- a/drivers/crypto/qat/qat_adf/qat_algs.h
+++ b/drivers/crypto/qat/qat_adf/qat_algs.h
@@ -127,4 +127,11 @@  void qat_alg_ablkcipher_init_dec(struct qat_alg_ablkcipher_cd *cd,
 int qat_alg_validate_aes_key(int key_len, enum icp_qat_hw_cipher_algo *alg);
 int qat_alg_validate_snow3g_key(int key_len, enum icp_qat_hw_cipher_algo *alg);
 
+int qat_crypto_sym_use_optimized_alg(struct qat_session *session);
+int qat_crypto_create_optimzed_session(struct qat_session *session,
+					uint8_t *cipherkey,
+					uint32_t cipherkeylen,
+					uint8_t *authkey,
+					uint32_t authkeylen,
+					uint32_t digestsize);
 #endif
diff --git a/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c b/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c
index aa108d4..8c1a801 100644
--- a/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c
+++ b/drivers/crypto/qat/qat_adf/qat_algs_build_desc.c
@@ -444,6 +444,7 @@  int qat_alg_aead_session_create_content_desc_cipher(struct qat_session *cdesc,
 	header->service_cmd_id = cdesc->qat_cmd;
 	ICP_QAT_FW_LA_DIGEST_IN_BUFFER_SET(header->serv_specif_flags,
 					ICP_QAT_FW_LA_NO_DIGEST_IN_BUFFER);
+
 	/* Configure the common header protocol flags */
 	ICP_QAT_FW_LA_PROTO_SET(header->serv_specif_flags, proto);
 	cd_pars->u.s.content_desc_addr = cdesc->cd_paddr;
@@ -749,6 +750,96 @@  int qat_alg_aead_session_create_content_desc_auth(struct qat_session *cdesc,
 	return 0;
 }
 
+/*
+ * Function which will return if it's possible to use the
+ * optimised content descriptor.
+ */
+int qat_crypto_sym_use_optimized_alg(struct qat_session *session)
+{
+	return ((session->qat_mode == ICP_QAT_HW_CIPHER_CBC_MODE)
+	&& (session->qat_cipher_alg == ICP_QAT_HW_CIPHER_ALGO_AES128)
+	&& (session->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_SHA1));
+}
+
+/*
+ * Function to create an optimised content descriptor for AES128 SHA1.
+ */
+int qat_crypto_create_optimzed_session(struct qat_session *session,
+				       uint8_t *cipherkey,
+				       uint32_t cipherkeylen,
+				       uint8_t *authkey,
+				       uint32_t authkeylen,
+				       uint32_t digestsize)
+{
+	/* CFG_CIPH_AES128_CBC_ENCRYPT, CFG_AUTH_SHA1_MODE1_NO_NESTED */
+	uint32_t icp_qat_fw_aes128_cbc_sha1_encrypt[] = {0x21120202,
+			0x422E0000, 0x14140000, 0x18031800, 0};
+	/* CFG_CIPH_AES128_CBC_DECRYPT, CFG_AUTH_SHA1_MODE1_NO_NESTED */
+	uint32_t icp_qat_fw_aes128_cbc_sha1_decrypt[] = {0x41150202,
+			0x122E0000, 0x14140000, 0x18031800, 0};
+
+	register struct icp_qat_fw_la_bulk_req *qat_req;
+	uint8_t *cd_ptr = (uint8_t *)&session->cd;
+	uint16_t state2_size;
+	struct icp_qat_fw_auth_cd_ctrl_hdr *hash_cd_ctrl;
+
+	/*
+	 * Setup the template for the request.
+	 */
+	qat_req = &session->fw_req;
+
+	/*
+	 * Setup some hard coded values for the constant config table.
+	 */
+	if (session->qat_dir == ICP_QAT_HW_CIPHER_ENCRYPT) {
+		memcpy(&qat_req->cd_ctrl, icp_qat_fw_aes128_cbc_sha1_encrypt,
+				sizeof(icp_qat_fw_aes128_cbc_sha1_encrypt));
+		qat_req->comn_hdr.serv_specif_flags = 0x2c;
+	} else {
+		memcpy(&qat_req->cd_ctrl, icp_qat_fw_aes128_cbc_sha1_decrypt,
+				sizeof(icp_qat_fw_aes128_cbc_sha1_decrypt));
+		qat_req->comn_hdr.serv_specif_flags = 0x4c;
+	}
+
+	/*
+	 * Cope with a digest size != 0x14
+	 */
+	if (digestsize != 0x14) {
+		hash_cd_ctrl = (struct icp_qat_fw_auth_cd_ctrl_hdr *)
+				&qat_req->cd_ctrl;
+		hash_cd_ctrl->inner_res_sz = digestsize;
+		hash_cd_ctrl->final_sz = digestsize;
+		struct icp_qat_fw_la_auth_req_params *auth_param =
+			(struct icp_qat_fw_la_auth_req_params *)
+			((char *)&qat_req->serv_specif_rqpars +
+			sizeof(struct icp_qat_fw_la_cipher_req_params));
+		auth_param->auth_res_sz = digestsize;
+	}
+
+	/*
+	 * Setup the size of the content descriptor.
+	 */
+
+	qat_req->cd_pars.u.s.content_desc_params_sz = 0x08;
+	qat_req->cd_pars.u.s.content_desc_addr = session->cd_paddr;
+
+	/*
+	 * Setup the content descriptor.
+	 */
+	memcpy(cd_ptr, cipherkey, cipherkeylen);
+	cd_ptr += cipherkeylen;
+
+	if (qat_alg_do_precomputes(session->qat_hash_alg,
+				authkey, authkeylen, cd_ptr,
+				&state2_size)) {
+		PMD_DRV_LOG(ERR, "(SHA) pre-compute failed");
+		return -EFAULT;
+	}
+	return 0;
+}
+
+
+
 static void qat_alg_ablkcipher_init_com(struct icp_qat_fw_la_bulk_req *req,
 					struct icp_qat_hw_cipher_algo_blk *cd,
 					const uint8_t *key, unsigned int keylen)
diff --git a/drivers/crypto/qat/qat_crypto.c b/drivers/crypto/qat/qat_crypto.c
index 940b2b6..69b63c3 100644
--- a/drivers/crypto/qat/qat_crypto.c
+++ b/drivers/crypto/qat/qat_crypto.c
@@ -361,7 +361,8 @@  qat_get_cipher_xform(struct rte_crypto_sym_xform *xform)
 }
 void *
 qat_crypto_sym_configure_session_cipher(struct rte_cryptodev *dev,
-		struct rte_crypto_sym_xform *xform, void *session_private)
+		struct rte_crypto_sym_xform *xform,
+		void *session_private)
 {
 	struct qat_pmd_private *internals = dev->data->dev_private;
 
@@ -443,9 +444,7 @@  qat_crypto_sym_configure_session(struct rte_cryptodev *dev,
 		struct rte_crypto_sym_xform *xform, void *session_private)
 {
 	struct qat_pmd_private *internals = dev->data->dev_private;
-
 	struct qat_session *session = session_private;
-
 	int qat_cmd_id;
 
 	PMD_INIT_FUNC_TRACE();
@@ -459,19 +458,31 @@  qat_crypto_sym_configure_session(struct rte_cryptodev *dev,
 	session->qat_cmd = (enum icp_qat_fw_la_cmd_id)qat_cmd_id;
 	switch (session->qat_cmd) {
 	case ICP_QAT_FW_LA_CMD_CIPHER:
-	session = qat_crypto_sym_configure_session_cipher(dev, xform, session);
+		session = qat_crypto_sym_configure_session_cipher(dev, xform,
+				session);
 		break;
 	case ICP_QAT_FW_LA_CMD_AUTH:
-	session = qat_crypto_sym_configure_session_auth(dev, xform, session);
+		session = qat_crypto_sym_configure_session_auth(dev, xform,
+				session);
 		break;
 	case ICP_QAT_FW_LA_CMD_CIPHER_HASH:
-	session = qat_crypto_sym_configure_session_cipher(dev, xform, session);
-	session = qat_crypto_sym_configure_session_auth(dev, xform, session);
-		break;
+		session = qat_crypto_sym_configure_session_cipher(dev, xform,
+				session);
+		session = qat_crypto_sym_configure_session_auth(dev, xform,
+				session);
+		if (qat_crypto_sym_use_optimized_alg(session))
+			qat_crypto_sym_configure_optimized_session(dev, xform,
+				session);
+	break;
 	case ICP_QAT_FW_LA_CMD_HASH_CIPHER:
-	session = qat_crypto_sym_configure_session_auth(dev, xform, session);
-	session = qat_crypto_sym_configure_session_cipher(dev, xform, session);
-		break;
+		session = qat_crypto_sym_configure_session_auth(dev, xform,
+				session);
+		session = qat_crypto_sym_configure_session_cipher(dev, xform,
+				session);
+		if (qat_crypto_sym_use_optimized_alg(session))
+			qat_crypto_sym_configure_optimized_session(dev, xform,
+				session);
+	break;
 	case ICP_QAT_FW_LA_CMD_TRNG_GET_RANDOM:
 	case ICP_QAT_FW_LA_CMD_TRNG_TEST:
 	case ICP_QAT_FW_LA_CMD_SSL3_KEY_DERIVE:
@@ -496,6 +507,35 @@  error_out:
 	return NULL;
 }
 
+
+struct qat_session *
+qat_crypto_sym_configure_optimized_session(struct rte_cryptodev *dev,
+				struct rte_crypto_sym_xform *xform,
+				struct qat_session *session)
+{
+
+	struct qat_pmd_private *internals = dev->data->dev_private;
+	struct rte_crypto_auth_xform *auth_xform = NULL;
+	struct rte_crypto_cipher_xform *cipher_xform = NULL;
+
+	auth_xform = qat_get_auth_xform(xform);
+	cipher_xform = qat_get_cipher_xform(xform);
+	if ((auth_xform == NULL) || (cipher_xform == NULL))
+		goto error_out;
+
+	if (qat_crypto_create_optimzed_session(session,
+			cipher_xform->key.data,
+			cipher_xform->key.length,
+			auth_xform->key.data,
+			auth_xform->key.length,
+			auth_xform->digest_length) == 0)
+	return session;
+
+error_out:
+	rte_mempool_put(internals->sess_mp, session);
+	return NULL;
+}
+
 struct qat_session *
 qat_crypto_sym_configure_session_auth(struct rte_cryptodev *dev,
 				struct rte_crypto_sym_xform *xform,
@@ -551,11 +591,11 @@  qat_crypto_sym_configure_session_auth(struct rte_cryptodev *dev,
 				auth_xform->algo);
 		goto error_out;
 	}
-	cipher_xform = qat_get_cipher_xform(xform);
 
 	if ((session->qat_hash_alg == ICP_QAT_HW_AUTH_ALGO_GALOIS_128) ||
 			(session->qat_hash_alg ==
 				ICP_QAT_HW_AUTH_ALGO_GALOIS_64))  {
+		cipher_xform = qat_get_cipher_xform(xform);
 		if (qat_alg_aead_session_create_content_desc_auth(session,
 				cipher_xform->key.data,
 				cipher_xform->key.length,
diff --git a/drivers/crypto/qat/qat_crypto.h b/drivers/crypto/qat/qat_crypto.h
index 0afe74e..79f8b7a 100644
--- a/drivers/crypto/qat/qat_crypto.h
+++ b/drivers/crypto/qat/qat_crypto.h
@@ -120,6 +120,10 @@  void *
 qat_crypto_sym_configure_session_cipher(struct rte_cryptodev *dev,
 		struct rte_crypto_sym_xform *xform, void *session_private);
 
+struct qat_session *
+qat_crypto_sym_configure_optimized_session(struct rte_cryptodev *dev,
+				struct rte_crypto_sym_xform *xform,
+				struct qat_session *session);
 
 extern void
 qat_crypto_sym_clear_session(struct rte_cryptodev *dev, void *session);