[dpdk-dev] [PATCH 0/8] Remove temporary digest allocation

[dpdk-dev] [PATCH 0/8] Remove temporary digest allocation

[Pablo de Lara]

When performing authentication verification,
some crypto PMDs require extra memory where the generated
digest can be placed.
Currently, these PMDs are getting the memory from the end
of the source mbuf, which might fail if there is not enough
tailroom.

To avoid this situation, some memory is allocated
in each queue pair of the device, to store temporarily
these digests.
 
Pablo de Lara (8):
  crypto/aesni_gcm: do not append digest
  crypto/armv8: do not append digest
  crypto/openssl: do not append digest
  crypto/kasumi: do not append digest
  crypto/snow3g: do not append digest
  crypto/zuc: do not append digest
  crypto/aesni_mb: do not append digest
  test/crypto: do not allocate extra memory for digest

 drivers/crypto/aesni_gcm/aesni_gcm_pmd.c           | 31 ++++---------------
 drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h   |  7 +++++
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c         | 36 +++++++---------------
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c     |  5 +++
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h | 12 +++++++-
 drivers/crypto/armv8/rte_armv8_pmd.c               | 14 +++------
 drivers/crypto/armv8/rte_armv8_pmd_private.h       |  8 +++++
 drivers/crypto/kasumi/rte_kasumi_pmd.c             | 22 +++++--------
 drivers/crypto/kasumi/rte_kasumi_pmd_private.h     |  7 +++++
 drivers/crypto/openssl/rte_openssl_pmd.c           | 19 +++++-------
 drivers/crypto/openssl/rte_openssl_pmd_private.h   |  7 +++++
 drivers/crypto/snow3g/rte_snow3g_pmd.c             | 22 +++++--------
 drivers/crypto/snow3g/rte_snow3g_pmd_private.h     |  7 +++++
 drivers/crypto/zuc/rte_zuc_pmd.c                   | 16 +++-------
 drivers/crypto/zuc/rte_zuc_pmd_private.h           |  7 +++++
 test/test/test_cryptodev_blockcipher.c             | 29 ++---------------
 16 files changed, 112 insertions(+), 137 deletions(-)

-- 
2.9.4

[dpdk-dev] [PATCH 1/8] crypto/aesni_gcm: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/aesni_gcm/aesni_gcm_pmd.c         | 31 +++++-------------------
 drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h |  7 ++++++
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
index d9c91d0..ae670a7 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
@@ -298,14 +298,7 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp, struct rte_crypto_op *op,
 				sym_op->aead.digest.data,
 				(uint64_t)session->digest_length);
 	} else if (session->op == AESNI_GCM_OP_AUTHENTICATED_DECRYPTION) {
-		uint8_t *auth_tag = (uint8_t *)rte_pktmbuf_append(sym_op->m_dst ?
-				sym_op->m_dst : sym_op->m_src,
-				session->digest_length);
-
-		if (!auth_tag) {
-			GCM_LOG_ERR("auth_tag");
-			return -1;
-		}
+		uint8_t *auth_tag = (uint8_t *)&qp->temp_digest;
 
 		qp->ops[session->key].init(&session->gdata_key,
 				&qp->gdata_ctx,
@@ -350,14 +343,7 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp, struct rte_crypto_op *op,
 				sym_op->auth.digest.data,
 				(uint64_t)session->digest_length);
 	} else { /* AESNI_GMAC_OP_VERIFY */
-		uint8_t *auth_tag = (uint8_t *)rte_pktmbuf_append(sym_op->m_dst ?
-				sym_op->m_dst : sym_op->m_src,
-				session->digest_length);
-
-		if (!auth_tag) {
-			GCM_LOG_ERR("auth_tag");
-			return -1;
-		}
+		uint8_t *auth_tag = (uint8_t *)&qp->temp_digest;
 
 		qp->ops[session->key].init(&session->gdata_key,
 				&qp->gdata_ctx,
@@ -385,11 +371,10 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp, struct rte_crypto_op *op,
  * - Returns NULL on invalid job
  */
 static void
-post_process_gcm_crypto_op(struct rte_crypto_op *op,
+post_process_gcm_crypto_op(struct aesni_gcm_qp *qp,
+		struct rte_crypto_op *op,
 		struct aesni_gcm_session *session)
 {
-	struct rte_mbuf *m = op->sym->m_dst ? op->sym->m_dst : op->sym->m_src;
-
 	op->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
 
 	/* Verify digest if required */
@@ -397,8 +382,7 @@ post_process_gcm_crypto_op(struct rte_crypto_op *op,
 			session->op == AESNI_GMAC_OP_VERIFY) {
 		uint8_t *digest;
 
-		uint8_t *tag = rte_pktmbuf_mtod_offset(m, uint8_t *,
-				m->data_len - session->digest_length);
+		uint8_t *tag = (uint8_t *)&qp->temp_digest;
 
 		if (session->op == AESNI_GMAC_OP_VERIFY)
 			digest = op->sym->auth.digest.data;
@@ -414,9 +398,6 @@ post_process_gcm_crypto_op(struct rte_crypto_op *op,
 
 		if (memcmp(tag, digest,	session->digest_length) != 0)
 			op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
-
-		/* trim area used for digest from mbuf */
-		rte_pktmbuf_trim(m, session->digest_length);
 	}
 }
 
@@ -435,7 +416,7 @@ handle_completed_gcm_crypto_op(struct aesni_gcm_qp *qp,
 		struct rte_crypto_op *op,
 		struct aesni_gcm_session *sess)
 {
-	post_process_gcm_crypto_op(op, sess);
+	post_process_gcm_crypto_op(qp, op, sess);
 
 	/* Free session if a session-less crypto op */
 	if (op->sess_type == RTE_CRYPTO_OP_SESSIONLESS) {
diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h
index 7e15572..1c8835b 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h
@@ -58,6 +58,8 @@
 #define GCM_LOG_DBG(fmt, args...)
 #endif
 
+/* Maximum length for digest */
+#define DIGEST_LENGTH_MAX 16
 
 /** private data structure for each virtual AESNI GCM device */
 struct aesni_gcm_private {
@@ -84,6 +86,11 @@ struct aesni_gcm_qp {
 	/**< Queue Pair Identifier */
 	char name[RTE_CRYPTODEV_NAME_LEN];
 	/**< Unique Queue Pair Name */
+	uint8_t temp_digest[DIGEST_LENGTH_MAX];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 
-- 
2.9.4

[dpdk-dev] [PATCH 2/8] crypto/armv8: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/armv8/rte_armv8_pmd.c         | 14 +++++---------
 drivers/crypto/armv8/rte_armv8_pmd_private.h |  8 ++++++++
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/armv8/rte_armv8_pmd.c b/drivers/crypto/armv8/rte_armv8_pmd.c
index a5c39c9..6f6d053 100644
--- a/drivers/crypto/armv8/rte_armv8_pmd.c
+++ b/drivers/crypto/armv8/rte_armv8_pmd.c
@@ -575,8 +575,8 @@ get_session(struct armv8_crypto_qp *qp, struct rte_crypto_op *op)
 
 /** Process cipher operation */
 static inline void
-process_armv8_chained_op
-		(struct rte_crypto_op *op, struct armv8_crypto_session *sess,
+process_armv8_chained_op(struct armv8_crypto_qp *qp, struct rte_crypto_op *op,
+		struct armv8_crypto_session *sess,
 		struct rte_mbuf *mbuf_src, struct rte_mbuf *mbuf_dst)
 {
 	crypto_func_t crypto_func;
@@ -633,8 +633,7 @@ process_armv8_chained_op
 					op->sym->auth.data.length);
 		}
 	} else {
-		adst = (uint8_t *)rte_pktmbuf_append(m_asrc,
-				sess->auth.digest_length);
+		adst = (uint8_t *)&qp->temp_digest;
 	}
 
 	arg.cipher.iv = rte_crypto_op_ctod_offset(op, uint8_t *,
@@ -655,15 +654,12 @@ process_armv8_chained_op
 				sess->auth.digest_length) != 0) {
 			op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
 		}
-		/* Trim area used for digest from mbuf. */
-		rte_pktmbuf_trim(m_asrc,
-				sess->auth.digest_length);
 	}
 }
 
 /** Process crypto operation for mbuf */
 static inline int
-process_op(const struct armv8_crypto_qp *qp, struct rte_crypto_op *op,
+process_op(struct armv8_crypto_qp *qp, struct rte_crypto_op *op,
 		struct armv8_crypto_session *sess)
 {
 	struct rte_mbuf *msrc, *mdst;
@@ -676,7 +672,7 @@ process_op(const struct armv8_crypto_qp *qp, struct rte_crypto_op *op,
 	switch (sess->chain_order) {
 	case ARMV8_CRYPTO_CHAIN_CIPHER_AUTH:
 	case ARMV8_CRYPTO_CHAIN_AUTH_CIPHER: /* Fall through */
-		process_armv8_chained_op(op, sess, msrc, mdst);
+		process_armv8_chained_op(qp, op, sess, msrc, mdst);
 		break;
 	default:
 		op->status = RTE_CRYPTO_OP_STATUS_ERROR;
diff --git a/drivers/crypto/armv8/rte_armv8_pmd_private.h b/drivers/crypto/armv8/rte_armv8_pmd_private.h
index d02992a..fa31f0a 100644
--- a/drivers/crypto/armv8/rte_armv8_pmd_private.h
+++ b/drivers/crypto/armv8/rte_armv8_pmd_private.h
@@ -69,6 +69,9 @@ do {								\
 #define NBBY		8		/* Number of bits in a byte */
 #define BYTE_LENGTH(x)	((x) / NBBY)	/* Number of bytes in x (round down) */
 
+/* Maximum length for digest (SHA-256 needs 32 bytes) */
+#define DIGEST_LENGTH_MAX 32
+
 /** ARMv8 operation order mode enumerator */
 enum armv8_crypto_chain_order {
 	ARMV8_CRYPTO_CHAIN_CIPHER_AUTH,
@@ -147,6 +150,11 @@ struct armv8_crypto_qp {
 	/**< Queue pair statistics */
 	char name[RTE_CRYPTODEV_NAME_LEN];
 	/**< Unique Queue Pair Name */
+	uint8_t temp_digest[DIGEST_LENGTH_MAX];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 /** ARMv8 crypto private session structure */
-- 
2.9.4

[dpdk-dev] [PATCH 3/8] crypto/openssl: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/openssl/rte_openssl_pmd.c         | 19 ++++++++-----------
 drivers/crypto/openssl/rte_openssl_pmd_private.h |  7 +++++++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index 0bd5f98..b72d1f4 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -1237,9 +1237,9 @@ process_openssl_docsis_bpi_op(struct rte_crypto_op *op,
 
 /** Process auth operation */
 static void
-process_openssl_auth_op
-		(struct rte_crypto_op *op, struct openssl_session *sess,
-		struct rte_mbuf *mbuf_src, struct rte_mbuf *mbuf_dst)
+process_openssl_auth_op(struct openssl_qp *qp, struct rte_crypto_op *op,
+		struct openssl_session *sess, struct rte_mbuf *mbuf_src,
+		struct rte_mbuf *mbuf_dst)
 {
 	uint8_t *dst;
 	int srclen, status;
@@ -1247,8 +1247,7 @@ process_openssl_auth_op
 	srclen = op->sym->auth.data.length;
 
 	if (sess->auth.operation == RTE_CRYPTO_AUTH_OP_VERIFY)
-		dst = (uint8_t *)rte_pktmbuf_append(mbuf_src,
-				sess->auth.digest_length);
+		dst = (uint8_t *)&qp->temp_digest;
 	else {
 		dst = op->sym->auth.digest.data;
 		if (dst == NULL)
@@ -1279,8 +1278,6 @@ process_openssl_auth_op
 				sess->auth.digest_length) != 0) {
 			op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
 		}
-		/* Trim area used for digest from mbuf. */
-		rte_pktmbuf_trim(mbuf_src, sess->auth.digest_length);
 	}
 
 	if (status != 0)
@@ -1289,7 +1286,7 @@ process_openssl_auth_op
 
 /** Process crypto operation for mbuf */
 static int
-process_op(const struct openssl_qp *qp, struct rte_crypto_op *op,
+process_op(struct openssl_qp *qp, struct rte_crypto_op *op,
 		struct openssl_session *sess)
 {
 	struct rte_mbuf *msrc, *mdst;
@@ -1305,14 +1302,14 @@ process_op(const struct openssl_qp *qp, struct rte_crypto_op *op,
 		process_openssl_cipher_op(op, sess, msrc, mdst);
 		break;
 	case OPENSSL_CHAIN_ONLY_AUTH:
-		process_openssl_auth_op(op, sess, msrc, mdst);
+		process_openssl_auth_op(qp, op, sess, msrc, mdst);
 		break;
 	case OPENSSL_CHAIN_CIPHER_AUTH:
 		process_openssl_cipher_op(op, sess, msrc, mdst);
-		process_openssl_auth_op(op, sess, mdst, mdst);
+		process_openssl_auth_op(qp, op, sess, mdst, mdst);
 		break;
 	case OPENSSL_CHAIN_AUTH_CIPHER:
-		process_openssl_auth_op(op, sess, msrc, mdst);
+		process_openssl_auth_op(qp, op, sess, msrc, mdst);
 		process_openssl_cipher_op(op, sess, msrc, mdst);
 		break;
 	case OPENSSL_CHAIN_COMBINED:
diff --git a/drivers/crypto/openssl/rte_openssl_pmd_private.h b/drivers/crypto/openssl/rte_openssl_pmd_private.h
index b7f7475..93937d5 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd_private.h
+++ b/drivers/crypto/openssl/rte_openssl_pmd_private.h
@@ -59,6 +59,8 @@
 #define OPENSSL_LOG_DBG(fmt, args...)
 #endif
 
+/* Maximum length for digest (SHA-512 needs 64 bytes) */
+#define DIGEST_LENGTH_MAX 64
 
 /** OPENSSL operation order mode enumerator */
 enum openssl_chain_order {
@@ -103,6 +105,11 @@ struct openssl_qp {
 	/**< Session Mempool */
 	struct rte_cryptodev_stats stats;
 	/**< Queue pair statistics */
+	uint8_t temp_digest[DIGEST_LENGTH_MAX];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 /** OPENSSL crypto private session structure */
-- 
2.9.4

[dpdk-dev] [PATCH 4/8] crypto/kasumi: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/kasumi/rte_kasumi_pmd.c         | 22 ++++++++--------------
 drivers/crypto/kasumi/rte_kasumi_pmd_private.h |  7 +++++++
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c b/drivers/crypto/kasumi/rte_kasumi_pmd.c
index 38cd8a9..27d2104 100644
--- a/drivers/crypto/kasumi/rte_kasumi_pmd.c
+++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c
@@ -44,7 +44,6 @@
 
 #define KASUMI_KEY_LENGTH 16
 #define KASUMI_IV_LENGTH 8
-#define KASUMI_DIGEST_LENGTH 4
 #define KASUMI_MAX_BURST 4
 #define BYTE_LEN 8
 
@@ -261,7 +260,7 @@ process_kasumi_cipher_op_bit(struct rte_crypto_op *op,
 
 /** Generate/verify hash from mbufs with same hash key. */
 static int
-process_kasumi_hash_op(struct rte_crypto_op **ops,
+process_kasumi_hash_op(struct kasumi_qp *qp, struct rte_crypto_op **ops,
 		struct kasumi_session *session,
 		uint8_t num_ops)
 {
@@ -287,8 +286,7 @@ process_kasumi_hash_op(struct rte_crypto_op **ops,
 		num_bytes = length_in_bits >> 3;
 
 		if (session->auth_op == RTE_CRYPTO_AUTH_OP_VERIFY) {
-			dst = (uint8_t *)rte_pktmbuf_append(ops[i]->sym->m_src,
-					KASUMI_DIGEST_LENGTH);
+			dst = (uint8_t *)&qp->temp_digest;
 			sso_kasumi_f9_1_buffer(&session->pKeySched_hash, src,
 					num_bytes, dst);
 
@@ -296,10 +294,6 @@ process_kasumi_hash_op(struct rte_crypto_op **ops,
 			if (memcmp(dst, ops[i]->sym->auth.digest.data,
 					KASUMI_DIGEST_LENGTH) != 0)
 				ops[i]->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
-
-			/* Trim area used for digest from mbuf. */
-			rte_pktmbuf_trim(ops[i]->sym->m_src,
-					KASUMI_DIGEST_LENGTH);
 		} else  {
 			dst = ops[i]->sym->auth.digest.data;
 
@@ -327,16 +321,16 @@ process_ops(struct rte_crypto_op **ops, struct kasumi_session *session,
 				session, num_ops);
 		break;
 	case KASUMI_OP_ONLY_AUTH:
-		processed_ops = process_kasumi_hash_op(ops, session,
+		processed_ops = process_kasumi_hash_op(qp, ops, session,
 				num_ops);
 		break;
 	case KASUMI_OP_CIPHER_AUTH:
 		processed_ops = process_kasumi_cipher_op(ops, session,
 				num_ops);
-		process_kasumi_hash_op(ops, session, processed_ops);
+		process_kasumi_hash_op(qp, ops, session, processed_ops);
 		break;
 	case KASUMI_OP_AUTH_CIPHER:
-		processed_ops = process_kasumi_hash_op(ops, session,
+		processed_ops = process_kasumi_hash_op(qp, ops, session,
 				num_ops);
 		process_kasumi_cipher_op(ops, session, processed_ops);
 		break;
@@ -384,15 +378,15 @@ process_op_bit(struct rte_crypto_op *op, struct kasumi_session *session,
 				session);
 		break;
 	case KASUMI_OP_ONLY_AUTH:
-		processed_op = process_kasumi_hash_op(&op, session, 1);
+		processed_op = process_kasumi_hash_op(qp, &op, session, 1);
 		break;
 	case KASUMI_OP_CIPHER_AUTH:
 		processed_op = process_kasumi_cipher_op_bit(op, session);
 		if (processed_op == 1)
-			process_kasumi_hash_op(&op, session, 1);
+			process_kasumi_hash_op(qp, &op, session, 1);
 		break;
 	case KASUMI_OP_AUTH_CIPHER:
-		processed_op = process_kasumi_hash_op(&op, session, 1);
+		processed_op = process_kasumi_hash_op(qp, &op, session, 1);
 		if (processed_op == 1)
 			process_kasumi_cipher_op_bit(op, session);
 		break;
diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd_private.h b/drivers/crypto/kasumi/rte_kasumi_pmd_private.h
index 0ce2a2e..5f7044b 100644
--- a/drivers/crypto/kasumi/rte_kasumi_pmd_private.h
+++ b/drivers/crypto/kasumi/rte_kasumi_pmd_private.h
@@ -58,6 +58,8 @@
 #define KASUMI_LOG_DBG(fmt, args...)
 #endif
 
+#define KASUMI_DIGEST_LENGTH 4
+
 /** private data structure for each virtual KASUMI device */
 struct kasumi_private {
 	unsigned max_nb_queue_pairs;
@@ -78,6 +80,11 @@ struct kasumi_qp {
 	/**< Session Mempool */
 	struct rte_cryptodev_stats qp_stats;
 	/**< Queue pair statistics */
+	uint8_t temp_digest[KASUMI_DIGEST_LENGTH];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 enum kasumi_operation {
-- 
2.9.4

[dpdk-dev] [PATCH 5/8] crypto/snow3g: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/snow3g/rte_snow3g_pmd.c         | 22 ++++++++--------------
 drivers/crypto/snow3g/rte_snow3g_pmd_private.h |  7 +++++++
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/crypto/snow3g/rte_snow3g_pmd.c b/drivers/crypto/snow3g/rte_snow3g_pmd.c
index dad4506..2083699 100644
--- a/drivers/crypto/snow3g/rte_snow3g_pmd.c
+++ b/drivers/crypto/snow3g/rte_snow3g_pmd.c
@@ -43,7 +43,6 @@
 #include "rte_snow3g_pmd_private.h"
 
 #define SNOW3G_IV_LENGTH 16
-#define SNOW3G_DIGEST_LENGTH 4
 #define SNOW3G_MAX_BURST 8
 #define BYTE_LEN 8
 
@@ -263,7 +262,7 @@ process_snow3g_cipher_op_bit(struct rte_crypto_op *op,
 
 /** Generate/verify hash from mbufs with same hash key. */
 static int
-process_snow3g_hash_op(struct rte_crypto_op **ops,
+process_snow3g_hash_op(struct snow3g_qp *qp, struct rte_crypto_op **ops,
 		struct snow3g_session *session,
 		uint8_t num_ops)
 {
@@ -289,8 +288,7 @@ process_snow3g_hash_op(struct rte_crypto_op **ops,
 				session->auth_iv_offset);
 
 		if (session->auth_op == RTE_CRYPTO_AUTH_OP_VERIFY) {
-			dst = (uint8_t *)rte_pktmbuf_append(ops[i]->sym->m_src,
-					SNOW3G_DIGEST_LENGTH);
+			dst = (uint8_t *)&qp->temp_digest;
 
 			sso_snow3g_f9_1_buffer(&session->pKeySched_hash,
 					iv, src,
@@ -299,10 +297,6 @@ process_snow3g_hash_op(struct rte_crypto_op **ops,
 			if (memcmp(dst, ops[i]->sym->auth.digest.data,
 					SNOW3G_DIGEST_LENGTH) != 0)
 				ops[i]->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
-
-			/* Trim area used for digest from mbuf. */
-			rte_pktmbuf_trim(ops[i]->sym->m_src,
-					SNOW3G_DIGEST_LENGTH);
 		} else  {
 			dst = ops[i]->sym->auth.digest.data;
 
@@ -346,16 +340,16 @@ process_ops(struct rte_crypto_op **ops, struct snow3g_session *session,
 				session, num_ops);
 		break;
 	case SNOW3G_OP_ONLY_AUTH:
-		processed_ops = process_snow3g_hash_op(ops, session,
+		processed_ops = process_snow3g_hash_op(qp, ops, session,
 				num_ops);
 		break;
 	case SNOW3G_OP_CIPHER_AUTH:
 		processed_ops = process_snow3g_cipher_op(ops, session,
 				num_ops);
-		process_snow3g_hash_op(ops, session, processed_ops);
+		process_snow3g_hash_op(qp, ops, session, processed_ops);
 		break;
 	case SNOW3G_OP_AUTH_CIPHER:
-		processed_ops = process_snow3g_hash_op(ops, session,
+		processed_ops = process_snow3g_hash_op(qp, ops, session,
 				num_ops);
 		process_snow3g_cipher_op(ops, session, processed_ops);
 		break;
@@ -403,15 +397,15 @@ process_op_bit(struct rte_crypto_op *op, struct snow3g_session *session,
 				session);
 		break;
 	case SNOW3G_OP_ONLY_AUTH:
-		processed_op = process_snow3g_hash_op(&op, session, 1);
+		processed_op = process_snow3g_hash_op(qp, &op, session, 1);
 		break;
 	case SNOW3G_OP_CIPHER_AUTH:
 		processed_op = process_snow3g_cipher_op_bit(op, session);
 		if (processed_op == 1)
-			process_snow3g_hash_op(&op, session, 1);
+			process_snow3g_hash_op(qp, &op, session, 1);
 		break;
 	case SNOW3G_OP_AUTH_CIPHER:
-		processed_op = process_snow3g_hash_op(&op, session, 1);
+		processed_op = process_snow3g_hash_op(qp, &op, session, 1);
 		if (processed_op == 1)
 			process_snow3g_cipher_op_bit(op, session);
 		break;
diff --git a/drivers/crypto/snow3g/rte_snow3g_pmd_private.h b/drivers/crypto/snow3g/rte_snow3g_pmd_private.h
index fba3cb8..7b9729f 100644
--- a/drivers/crypto/snow3g/rte_snow3g_pmd_private.h
+++ b/drivers/crypto/snow3g/rte_snow3g_pmd_private.h
@@ -58,6 +58,8 @@
 #define SNOW3G_LOG_DBG(fmt, args...)
 #endif
 
+#define SNOW3G_DIGEST_LENGTH 4
+
 /** private data structure for each virtual SNOW 3G device */
 struct snow3g_private {
 	unsigned max_nb_queue_pairs;
@@ -78,6 +80,11 @@ struct snow3g_qp {
 	/**< Session Mempool */
 	struct rte_cryptodev_stats qp_stats;
 	/**< Queue pair statistics */
+	uint8_t temp_digest[SNOW3G_DIGEST_LENGTH];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 enum snow3g_operation {
-- 
2.9.4

[dpdk-dev] [PATCH 6/8] crypto/zuc: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/zuc/rte_zuc_pmd.c         | 16 +++++-----------
 drivers/crypto/zuc/rte_zuc_pmd_private.h |  7 +++++++
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/zuc/rte_zuc_pmd.c b/drivers/crypto/zuc/rte_zuc_pmd.c
index b301711..2d8d641 100644
--- a/drivers/crypto/zuc/rte_zuc_pmd.c
+++ b/drivers/crypto/zuc/rte_zuc_pmd.c
@@ -42,7 +42,6 @@
 
 #include "rte_zuc_pmd_private.h"
 
-#define ZUC_DIGEST_LENGTH 4
 #define ZUC_MAX_BURST 8
 #define BYTE_LEN 8
 
@@ -258,7 +257,7 @@ process_zuc_cipher_op(struct rte_crypto_op **ops,
 
 /** Generate/verify hash from mbufs with same hash key. */
 static int
-process_zuc_hash_op(struct rte_crypto_op **ops,
+process_zuc_hash_op(struct zuc_qp *qp, struct rte_crypto_op **ops,
 		struct zuc_session *session,
 		uint8_t num_ops)
 {
@@ -285,8 +284,7 @@ process_zuc_hash_op(struct rte_crypto_op **ops,
 				session->auth_iv_offset);
 
 		if (session->auth_op == RTE_CRYPTO_AUTH_OP_VERIFY) {
-			dst = (uint32_t *)rte_pktmbuf_append(ops[i]->sym->m_src,
-					ZUC_DIGEST_LENGTH);
+			dst = (uint32_t *)&qp->temp_digest;
 
 			sso_zuc_eia3_1_buffer(session->pKey_hash,
 					iv, src,
@@ -295,10 +293,6 @@ process_zuc_hash_op(struct rte_crypto_op **ops,
 			if (memcmp(dst, ops[i]->sym->auth.digest.data,
 					ZUC_DIGEST_LENGTH) != 0)
 				ops[i]->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
-
-			/* Trim area used for digest from mbuf. */
-			rte_pktmbuf_trim(ops[i]->sym->m_src,
-					ZUC_DIGEST_LENGTH);
 		} else  {
 			dst = (uint32_t *)ops[i]->sym->auth.digest.data;
 
@@ -327,16 +321,16 @@ process_ops(struct rte_crypto_op **ops, struct zuc_session *session,
 				session, num_ops);
 		break;
 	case ZUC_OP_ONLY_AUTH:
-		processed_ops = process_zuc_hash_op(ops, session,
+		processed_ops = process_zuc_hash_op(qp, ops, session,
 				num_ops);
 		break;
 	case ZUC_OP_CIPHER_AUTH:
 		processed_ops = process_zuc_cipher_op(ops, session,
 				num_ops);
-		process_zuc_hash_op(ops, session, processed_ops);
+		process_zuc_hash_op(qp, ops, session, processed_ops);
 		break;
 	case ZUC_OP_AUTH_CIPHER:
-		processed_ops = process_zuc_hash_op(ops, session,
+		processed_ops = process_zuc_hash_op(qp, ops, session,
 				num_ops);
 		process_zuc_cipher_op(ops, session, processed_ops);
 		break;
diff --git a/drivers/crypto/zuc/rte_zuc_pmd_private.h b/drivers/crypto/zuc/rte_zuc_pmd_private.h
index b706e0a..a57b8cd 100644
--- a/drivers/crypto/zuc/rte_zuc_pmd_private.h
+++ b/drivers/crypto/zuc/rte_zuc_pmd_private.h
@@ -59,6 +59,8 @@
 #endif
 
 #define ZUC_IV_KEY_LENGTH 16
+#define ZUC_DIGEST_LENGTH 4
+
 /** private data structure for each virtual ZUC device */
 struct zuc_private {
 	unsigned max_nb_queue_pairs;
@@ -79,6 +81,11 @@ struct zuc_qp {
 	/**< Session Mempool */
 	struct rte_cryptodev_stats qp_stats;
 	/**< Queue pair statistics */
+	uint8_t temp_digest[ZUC_DIGEST_LENGTH];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 enum zuc_operation {
-- 
2.9.4

[dpdk-dev] [PATCH 7/8] crypto/aesni_mb: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c         | 36 +++++++---------------
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c     |  5 +++
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h | 12 +++++++-
 3 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index 16e1451..2ec5a71 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -407,7 +407,7 @@ get_session(struct aesni_mb_qp *qp, struct rte_crypto_op *op)
  */
 static inline int
 set_mb_job_params(JOB_AES_HMAC *job, struct aesni_mb_qp *qp,
-		struct rte_crypto_op *op)
+		struct rte_crypto_op *op, uint8_t *digest_idx)
 {
 	struct rte_mbuf *m_src = op->sym->m_src, *m_dst;
 	struct aesni_mb_session *session;
@@ -466,19 +466,8 @@ set_mb_job_params(JOB_AES_HMAC *job, struct aesni_mb_qp *qp,
 	/* Set digest output location */
 	if (job->hash_alg != NULL_HASH &&
 			session->auth.operation == RTE_CRYPTO_AUTH_OP_VERIFY) {
-		job->auth_tag_output = (uint8_t *)rte_pktmbuf_append(m_dst,
-				get_digest_byte_length(job->hash_alg));
-
-		if (job->auth_tag_output == NULL) {
-			MB_LOG_ERR("failed to allocate space in output mbuf "
-					"for temp digest");
-			op->status = RTE_CRYPTO_OP_STATUS_ERROR;
-			return -1;
-		}
-
-		memset(job->auth_tag_output, 0,
-				sizeof(get_digest_byte_length(job->hash_alg)));
-
+		job->auth_tag_output = (uint8_t *)&qp->temp_digests[*digest_idx];
+		*digest_idx = (*digest_idx + 1) % MAX_JOBS;
 	} else {
 		job->auth_tag_output = op->sym->auth.digest.data;
 	}
@@ -507,22 +496,17 @@ set_mb_job_params(JOB_AES_HMAC *job, struct aesni_mb_qp *qp,
 
 	/* Set user data to be crypto operation data struct */
 	job->user_data = op;
-	job->user_data2 = m_dst;
 
 	return 0;
 }
 
 static inline void
-verify_digest(JOB_AES_HMAC *job, struct rte_crypto_op *op) {
-	struct rte_mbuf *m_dst = (struct rte_mbuf *)job->user_data2;
-
+verify_digest(struct aesni_mb_qp *qp __rte_unused, JOB_AES_HMAC *job,
+		struct rte_crypto_op *op) {
 	/* Verify digest if required */
 	if (memcmp(job->auth_tag_output, op->sym->auth.digest.data,
 			job->auth_tag_output_len_in_bytes) != 0)
 		op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
-
-	/* trim area used for digest from mbuf */
-	rte_pktmbuf_trim(m_dst,	get_digest_byte_length(job->hash_alg));
 }
 
 /**
@@ -532,8 +516,7 @@ verify_digest(JOB_AES_HMAC *job, struct rte_crypto_op *op) {
  * @param job	JOB_AES_HMAC job to process
  *
  * @return
- * - Returns processed crypto operation which mbuf is trimmed of output digest
- *   used in verification of supplied digest.
+ * - Returns processed crypto operation.
  * - Returns NULL on invalid job
  */
 static inline struct rte_crypto_op *
@@ -552,7 +535,7 @@ post_process_mb_job(struct aesni_mb_qp *qp, JOB_AES_HMAC *job)
 			if (job->hash_alg != NULL_HASH) {
 				if (sess->auth.operation ==
 						RTE_CRYPTO_AUTH_OP_VERIFY)
-					verify_digest(job, op);
+					verify_digest(qp, job, op);
 			}
 			break;
 		default:
@@ -650,6 +633,7 @@ aesni_mb_pmd_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
 	if (unlikely(nb_ops == 0))
 		return 0;
 
+	uint8_t digest_idx = qp->digest_idx;
 	do {
 		/* Get next operation to process from ingress queue */
 		retval = rte_ring_dequeue(qp->ingress_queue, (void **)&op);
@@ -667,7 +651,7 @@ aesni_mb_pmd_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
 			job = (*qp->op_fns->job.get_next)(&qp->mb_mgr);
 		}
 
-		retval = set_mb_job_params(job, qp, op);
+		retval = set_mb_job_params(job, qp, op, &digest_idx);
 		if (unlikely(retval != 0)) {
 			qp->stats.dequeue_err_count++;
 			set_job_null_op(job);
@@ -687,6 +671,8 @@ aesni_mb_pmd_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
 
 	} while (processed_jobs < nb_ops);
 
+	qp->digest_idx = digest_idx;
+
 	if (processed_jobs < 1)
 		processed_jobs += flush_mb_mgr(qp,
 				&ops[processed_jobs],
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
index 692b354..4be9b80 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
@@ -430,6 +430,11 @@ aesni_mb_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 
 	memset(&qp->stats, 0, sizeof(qp->stats));
 
+	char mp_name[RTE_MEMPOOL_NAMESIZE];
+
+	snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+				"digest_mp_%u_%u", dev->data->dev_id, qp_id);
+
 	/* Initialise multi-buffer manager */
 	(*qp->op_fns->job.init_mgr)(&qp->mb_mgr);
 	return 0;
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h
index 6676948..fe3bd73 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h
@@ -61,6 +61,8 @@
 #define HMAC_IPAD_VALUE			(0x36)
 #define HMAC_OPAD_VALUE			(0x5C)
 
+/* Maximum length for digest (SHA-512 truncated needs 32 bytes) */
+#define DIGEST_LENGTH_MAX 32
 static const unsigned auth_blocksize[] = {
 		[MD5]		= 64,
 		[SHA1]		= 64,
@@ -164,9 +166,17 @@ struct aesni_mb_qp {
 	/**< Session Mempool */
 	struct rte_cryptodev_stats stats;
 	/**< Queue pair statistics */
+	uint8_t digest_idx;
+	/**< Index of the next slot to be used in temp_digests,
+	 * to store the digest for a given operation
+	 */
+	uint8_t temp_digests[MAX_JOBS][DIGEST_LENGTH_MAX];
+	/**< Buffers used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
-
 /** AES-NI multi-buffer private session structure */
 struct aesni_mb_session {
 	JOB_CHAIN_ORDER chain_order;
-- 
2.9.4

[dpdk-dev] [PATCH 8/8] test/crypto: do not allocate extra memory for digest

[Pablo de Lara]

Now that PMDs do not need extra space in the mbuf
to store temporarily the digest when verifying
an authentication tag, it is not required to allocate
more memory in the mbufs passed to cryptodev.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 test/test/test_cryptodev_blockcipher.c | 29 ++---------------------------
 1 file changed, 2 insertions(+), 27 deletions(-)

diff --git a/test/test/test_cryptodev_blockcipher.c b/test/test/test_cryptodev_blockcipher.c
index 6089af4..f8222bd 100644
--- a/test/test/test_cryptodev_blockcipher.c
+++ b/test/test/test_cryptodev_blockcipher.c
@@ -452,25 +452,13 @@ test_blockcipher_one_case(const struct blockcipher_test_case *t,
 	if (t->feature_mask & BLOCKCIPHER_TEST_FEATURE_OOP) {
 		struct rte_mbuf *mbuf;
 		uint8_t value;
-		uint32_t head_unchanged_len = 0, changed_len = 0;
+		uint32_t head_unchanged_len, changed_len = 0;
 		uint32_t i;
 
 		mbuf = sym_op->m_src;
-		if (t->op_mask & BLOCKCIPHER_TEST_OP_AUTH_VERIFY) {
-			/* white-box test: PMDs use some of the
-			 * tailroom as temp storage in verify case
-			 */
-			head_unchanged_len = rte_pktmbuf_headroom(mbuf)
-					+ rte_pktmbuf_data_len(mbuf);
-			changed_len = digest_len;
-		} else {
-			head_unchanged_len = mbuf->buf_len;
-			changed_len = 0;
-		}
+		head_unchanged_len = mbuf->buf_len;
 
 		for (i = 0; i < mbuf->buf_len; i++) {
-			if (i == head_unchanged_len)
-				i += changed_len;
 			value = *((uint8_t *)(mbuf->buf_addr)+i);
 			if (value != tmp_src_buf[i]) {
 				snprintf(test_msg, BLOCKCIPHER_TEST_MSG_LEN,
@@ -531,19 +519,6 @@ test_blockcipher_one_case(const struct blockcipher_test_case *t,
 		if (t->op_mask & BLOCKCIPHER_TEST_OP_AUTH_GEN)
 			changed_len += digest_len;
 
-		if (t->op_mask & BLOCKCIPHER_TEST_OP_AUTH_VERIFY) {
-			/* white-box test: PMDs use some of the
-			 * tailroom as temp storage in verify case
-			 */
-			if (t->op_mask & BLOCKCIPHER_TEST_OP_CIPHER) {
-				/* This is simplified, not checking digest*/
-				changed_len += digest_len*2;
-			} else {
-				head_unchanged_len += digest_len;
-				changed_len += digest_len;
-			}
-		}
-
 		for (i = 0; i < mbuf->buf_len; i++) {
 			if (i == head_unchanged_len)
 				i += changed_len;
-- 
2.9.4

Re: [dpdk-dev] [PATCH 1/8] crypto/aesni_gcm: do not append digest

[Zhang, Roy Fan]

Hi Pablo,

Thanks for the patch. It is very good idea of allocating only necessary buffer for digests in the operation.
Comments inline:

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pablo de Lara
> Sent: Friday, August 18, 2017 8:21 AM
> To: Doherty, Declan <declan.doherty@intel.com>;
> jerin.jacob@caviumnetworks.com
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: [dpdk-dev] [PATCH 1/8] crypto/aesni_gcm: do not append digest
> 
> When performing an authentication verification, the PMD was using memory
> at the end of the input buffer, to store temporarily the digest.
> This operation requires the buffer to have enough tailroom unnecessarily.
> Instead, memory is allocated for each queue pair, to store temporarily the
> digest generated by the driver, so it can be compared with the one provided
> in the crypto operation, without needing to touch the input buffer.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> ---
>  drivers/crypto/aesni_gcm/aesni_gcm_pmd.c         | 31 +++++-------------------
>  drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h |  7 ++++++
>  2 files changed, 13 insertions(+), 25 deletions(-)
> 
> diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> index d9c91d0..ae670a7 100644
> --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> @@ -298,14 +298,7 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp,
> struct rte_crypto_op *op,
>  				sym_op->aead.digest.data,
>  				(uint64_t)session->digest_length);
>  	} else if (session->op ==
> AESNI_GCM_OP_AUTHENTICATED_DECRYPTION) {
> -		uint8_t *auth_tag = (uint8_t
> *)rte_pktmbuf_append(sym_op->m_dst ?
> -				sym_op->m_dst : sym_op->m_src,
> -				session->digest_length);
> -
> -		if (!auth_tag) {
> -			GCM_LOG_ERR("auth_tag");
> -			return -1;
> -		}

qp->tmp_digest has already the data type of uint8_t*, the casting is not necessary here. Although the "&" here seems to be wrong. auth_tag didn't point to qp->tmp_digest but a buffer with the address as &qp->tmp_digest.

> +		uint8_t *auth_tag = (uint8_t *)&qp->temp_digest;
>  		qp->ops[session->key].init(&session->gdata_key,
>  				&qp->gdata_ctx,
> @@ -350,14 +343,7 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp,
> struct rte_crypto_op *op,
>  				sym_op->auth.digest.data,
>  				(uint64_t)session->digest_length);
>  	} else { /* AESNI_GMAC_OP_VERIFY */
> -		uint8_t *auth_tag = (uint8_t
> *)rte_pktmbuf_append(sym_op->m_dst ?
> -				sym_op->m_dst : sym_op->m_src,
> -				session->digest_length);
> -
> -		if (!auth_tag) {
> -			GCM_LOG_ERR("auth_tag");
> -			return -1;
> -		}

Same here. 

> +		uint8_t *auth_tag = (uint8_t *)&qp->temp_digest;
> 
>  		qp->ops[session->key].init(&session->gdata_key,
>  				&qp->gdata_ctx,
> @@ -385,11 +371,10 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp,
> struct rte_crypto_op *op,
>   * - Returns NULL on invalid job
>   */
>  static void
> -post_process_gcm_crypto_op(struct rte_crypto_op *op,
> +post_process_gcm_crypto_op(struct aesni_gcm_qp *qp,
> +		struct rte_crypto_op *op,
>  		struct aesni_gcm_session *session)
>  {
> -	struct rte_mbuf *m = op->sym->m_dst ? op->sym->m_dst : op-
> >sym->m_src;
> -
>  	op->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
> 
>  	/* Verify digest if required */
> @@ -397,8 +382,7 @@ post_process_gcm_crypto_op(struct rte_crypto_op
> *op,
>  			session->op == AESNI_GMAC_OP_VERIFY) {
>  		uint8_t *digest;
> 

Same here.

> -		uint8_t *tag = rte_pktmbuf_mtod_offset(m, uint8_t *,
> -				m->data_len - session->digest_length);
> +		uint8_t *tag = (uint8_t *)&qp->temp_digest;
> 

...

> 
> --
> 2.9.4

The same problem lies the rest of your patches in the patchset.

Regards,
Fan

[dpdk-dev] [PATCH v2 0/8] Remove temporary digest allocation

[Pablo de Lara]

When performing authentication verification,
some crypto PMDs require extra memory where the generated
digest can be placed.
Currently, these PMDs are getting the memory from the end
of the source mbuf, which might fail if there is not enough
tailroom.

To avoid this situation, some memory is allocated
in each queue pair of the device, to store temporarily
these digests.

Changes in v2:
- Removed incorrect indirection when getting the memory
  to store the generated digest (i.e. removed "&" in &temp_digest) 

Pablo de Lara (8):
  crypto/aesni_gcm: do not append digest
  crypto/armv8: do not append digest
  crypto/openssl: do not append digest
  crypto/kasumi: do not append digest
  crypto/snow3g: do not append digest
  crypto/zuc: do not append digest
  crypto/aesni_mb: do not append digest
  test/crypto: do not allocate extra memory for digest

 drivers/crypto/aesni_gcm/aesni_gcm_pmd.c           | 31 ++++---------------
 drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h   |  7 +++++
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c         | 36 +++++++---------------
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c     |  5 +++
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h | 12 +++++++-
 drivers/crypto/armv8/rte_armv8_pmd.c               | 14 +++------
 drivers/crypto/armv8/rte_armv8_pmd_private.h       |  8 +++++
 drivers/crypto/kasumi/rte_kasumi_pmd.c             | 22 +++++--------
 drivers/crypto/kasumi/rte_kasumi_pmd_private.h     |  7 +++++
 drivers/crypto/openssl/rte_openssl_pmd.c           | 19 +++++-------
 drivers/crypto/openssl/rte_openssl_pmd_private.h   |  7 +++++
 drivers/crypto/snow3g/rte_snow3g_pmd.c             | 22 +++++--------
 drivers/crypto/snow3g/rte_snow3g_pmd_private.h     |  7 +++++
 drivers/crypto/zuc/rte_zuc_pmd.c                   | 16 +++-------
 drivers/crypto/zuc/rte_zuc_pmd_private.h           |  7 +++++
 test/test/test_cryptodev_blockcipher.c             | 29 ++---------------
 16 files changed, 112 insertions(+), 137 deletions(-)

-- 
2.9.4

[dpdk-dev] [PATCH v2 1/8] crypto/aesni_gcm: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/aesni_gcm/aesni_gcm_pmd.c         | 31 +++++-------------------
 drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h |  7 ++++++
 2 files changed, 13 insertions(+), 25 deletions(-)

diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
index d9c91d0..8c9c211 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
@@ -298,14 +298,7 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp, struct rte_crypto_op *op,
 				sym_op->aead.digest.data,
 				(uint64_t)session->digest_length);
 	} else if (session->op == AESNI_GCM_OP_AUTHENTICATED_DECRYPTION) {
-		uint8_t *auth_tag = (uint8_t *)rte_pktmbuf_append(sym_op->m_dst ?
-				sym_op->m_dst : sym_op->m_src,
-				session->digest_length);
-
-		if (!auth_tag) {
-			GCM_LOG_ERR("auth_tag");
-			return -1;
-		}
+		uint8_t *auth_tag = qp->temp_digest;
 
 		qp->ops[session->key].init(&session->gdata_key,
 				&qp->gdata_ctx,
@@ -350,14 +343,7 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp, struct rte_crypto_op *op,
 				sym_op->auth.digest.data,
 				(uint64_t)session->digest_length);
 	} else { /* AESNI_GMAC_OP_VERIFY */
-		uint8_t *auth_tag = (uint8_t *)rte_pktmbuf_append(sym_op->m_dst ?
-				sym_op->m_dst : sym_op->m_src,
-				session->digest_length);
-
-		if (!auth_tag) {
-			GCM_LOG_ERR("auth_tag");
-			return -1;
-		}
+		uint8_t *auth_tag = qp->temp_digest;
 
 		qp->ops[session->key].init(&session->gdata_key,
 				&qp->gdata_ctx,
@@ -385,11 +371,10 @@ process_gcm_crypto_op(struct aesni_gcm_qp *qp, struct rte_crypto_op *op,
  * - Returns NULL on invalid job
  */
 static void
-post_process_gcm_crypto_op(struct rte_crypto_op *op,
+post_process_gcm_crypto_op(struct aesni_gcm_qp *qp,
+		struct rte_crypto_op *op,
 		struct aesni_gcm_session *session)
 {
-	struct rte_mbuf *m = op->sym->m_dst ? op->sym->m_dst : op->sym->m_src;
-
 	op->status = RTE_CRYPTO_OP_STATUS_SUCCESS;
 
 	/* Verify digest if required */
@@ -397,8 +382,7 @@ post_process_gcm_crypto_op(struct rte_crypto_op *op,
 			session->op == AESNI_GMAC_OP_VERIFY) {
 		uint8_t *digest;
 
-		uint8_t *tag = rte_pktmbuf_mtod_offset(m, uint8_t *,
-				m->data_len - session->digest_length);
+		uint8_t *tag = (uint8_t *)&qp->temp_digest;
 
 		if (session->op == AESNI_GMAC_OP_VERIFY)
 			digest = op->sym->auth.digest.data;
@@ -414,9 +398,6 @@ post_process_gcm_crypto_op(struct rte_crypto_op *op,
 
 		if (memcmp(tag, digest,	session->digest_length) != 0)
 			op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
-
-		/* trim area used for digest from mbuf */
-		rte_pktmbuf_trim(m, session->digest_length);
 	}
 }
 
@@ -435,7 +416,7 @@ handle_completed_gcm_crypto_op(struct aesni_gcm_qp *qp,
 		struct rte_crypto_op *op,
 		struct aesni_gcm_session *sess)
 {
-	post_process_gcm_crypto_op(op, sess);
+	post_process_gcm_crypto_op(qp, op, sess);
 
 	/* Free session if a session-less crypto op */
 	if (op->sess_type == RTE_CRYPTO_OP_SESSIONLESS) {
diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h
index 7e15572..1c8835b 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h
@@ -58,6 +58,8 @@
 #define GCM_LOG_DBG(fmt, args...)
 #endif
 
+/* Maximum length for digest */
+#define DIGEST_LENGTH_MAX 16
 
 /** private data structure for each virtual AESNI GCM device */
 struct aesni_gcm_private {
@@ -84,6 +86,11 @@ struct aesni_gcm_qp {
 	/**< Queue Pair Identifier */
 	char name[RTE_CRYPTODEV_NAME_LEN];
 	/**< Unique Queue Pair Name */
+	uint8_t temp_digest[DIGEST_LENGTH_MAX];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 
-- 
2.9.4

[dpdk-dev] [PATCH v2 2/8] crypto/armv8: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/armv8/rte_armv8_pmd.c         | 14 +++++---------
 drivers/crypto/armv8/rte_armv8_pmd_private.h |  8 ++++++++
 2 files changed, 13 insertions(+), 9 deletions(-)

diff --git a/drivers/crypto/armv8/rte_armv8_pmd.c b/drivers/crypto/armv8/rte_armv8_pmd.c
index a5c39c9..d5da02b 100644
--- a/drivers/crypto/armv8/rte_armv8_pmd.c
+++ b/drivers/crypto/armv8/rte_armv8_pmd.c
@@ -575,8 +575,8 @@ get_session(struct armv8_crypto_qp *qp, struct rte_crypto_op *op)
 
 /** Process cipher operation */
 static inline void
-process_armv8_chained_op
-		(struct rte_crypto_op *op, struct armv8_crypto_session *sess,
+process_armv8_chained_op(struct armv8_crypto_qp *qp, struct rte_crypto_op *op,
+		struct armv8_crypto_session *sess,
 		struct rte_mbuf *mbuf_src, struct rte_mbuf *mbuf_dst)
 {
 	crypto_func_t crypto_func;
@@ -633,8 +633,7 @@ process_armv8_chained_op
 					op->sym->auth.data.length);
 		}
 	} else {
-		adst = (uint8_t *)rte_pktmbuf_append(m_asrc,
-				sess->auth.digest_length);
+		adst = qp->temp_digest;
 	}
 
 	arg.cipher.iv = rte_crypto_op_ctod_offset(op, uint8_t *,
@@ -655,15 +654,12 @@ process_armv8_chained_op
 				sess->auth.digest_length) != 0) {
 			op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
 		}
-		/* Trim area used for digest from mbuf. */
-		rte_pktmbuf_trim(m_asrc,
-				sess->auth.digest_length);
 	}
 }
 
 /** Process crypto operation for mbuf */
 static inline int
-process_op(const struct armv8_crypto_qp *qp, struct rte_crypto_op *op,
+process_op(struct armv8_crypto_qp *qp, struct rte_crypto_op *op,
 		struct armv8_crypto_session *sess)
 {
 	struct rte_mbuf *msrc, *mdst;
@@ -676,7 +672,7 @@ process_op(const struct armv8_crypto_qp *qp, struct rte_crypto_op *op,
 	switch (sess->chain_order) {
 	case ARMV8_CRYPTO_CHAIN_CIPHER_AUTH:
 	case ARMV8_CRYPTO_CHAIN_AUTH_CIPHER: /* Fall through */
-		process_armv8_chained_op(op, sess, msrc, mdst);
+		process_armv8_chained_op(qp, op, sess, msrc, mdst);
 		break;
 	default:
 		op->status = RTE_CRYPTO_OP_STATUS_ERROR;
diff --git a/drivers/crypto/armv8/rte_armv8_pmd_private.h b/drivers/crypto/armv8/rte_armv8_pmd_private.h
index d02992a..fa31f0a 100644
--- a/drivers/crypto/armv8/rte_armv8_pmd_private.h
+++ b/drivers/crypto/armv8/rte_armv8_pmd_private.h
@@ -69,6 +69,9 @@ do {								\
 #define NBBY		8		/* Number of bits in a byte */
 #define BYTE_LENGTH(x)	((x) / NBBY)	/* Number of bytes in x (round down) */
 
+/* Maximum length for digest (SHA-256 needs 32 bytes) */
+#define DIGEST_LENGTH_MAX 32
+
 /** ARMv8 operation order mode enumerator */
 enum armv8_crypto_chain_order {
 	ARMV8_CRYPTO_CHAIN_CIPHER_AUTH,
@@ -147,6 +150,11 @@ struct armv8_crypto_qp {
 	/**< Queue pair statistics */
 	char name[RTE_CRYPTODEV_NAME_LEN];
 	/**< Unique Queue Pair Name */
+	uint8_t temp_digest[DIGEST_LENGTH_MAX];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 /** ARMv8 crypto private session structure */
-- 
2.9.4

[dpdk-dev] [PATCH v2 3/8] crypto/openssl: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/openssl/rte_openssl_pmd.c         | 19 ++++++++-----------
 drivers/crypto/openssl/rte_openssl_pmd_private.h |  7 +++++++
 2 files changed, 15 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
index 0bd5f98..6e01669 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd.c
+++ b/drivers/crypto/openssl/rte_openssl_pmd.c
@@ -1237,9 +1237,9 @@ process_openssl_docsis_bpi_op(struct rte_crypto_op *op,
 
 /** Process auth operation */
 static void
-process_openssl_auth_op
-		(struct rte_crypto_op *op, struct openssl_session *sess,
-		struct rte_mbuf *mbuf_src, struct rte_mbuf *mbuf_dst)
+process_openssl_auth_op(struct openssl_qp *qp, struct rte_crypto_op *op,
+		struct openssl_session *sess, struct rte_mbuf *mbuf_src,
+		struct rte_mbuf *mbuf_dst)
 {
 	uint8_t *dst;
 	int srclen, status;
@@ -1247,8 +1247,7 @@ process_openssl_auth_op
 	srclen = op->sym->auth.data.length;
 
 	if (sess->auth.operation == RTE_CRYPTO_AUTH_OP_VERIFY)
-		dst = (uint8_t *)rte_pktmbuf_append(mbuf_src,
-				sess->auth.digest_length);
+		dst = qp->temp_digest;
 	else {
 		dst = op->sym->auth.digest.data;
 		if (dst == NULL)
@@ -1279,8 +1278,6 @@ process_openssl_auth_op
 				sess->auth.digest_length) != 0) {
 			op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
 		}
-		/* Trim area used for digest from mbuf. */
-		rte_pktmbuf_trim(mbuf_src, sess->auth.digest_length);
 	}
 
 	if (status != 0)
@@ -1289,7 +1286,7 @@ process_openssl_auth_op
 
 /** Process crypto operation for mbuf */
 static int
-process_op(const struct openssl_qp *qp, struct rte_crypto_op *op,
+process_op(struct openssl_qp *qp, struct rte_crypto_op *op,
 		struct openssl_session *sess)
 {
 	struct rte_mbuf *msrc, *mdst;
@@ -1305,14 +1302,14 @@ process_op(const struct openssl_qp *qp, struct rte_crypto_op *op,
 		process_openssl_cipher_op(op, sess, msrc, mdst);
 		break;
 	case OPENSSL_CHAIN_ONLY_AUTH:
-		process_openssl_auth_op(op, sess, msrc, mdst);
+		process_openssl_auth_op(qp, op, sess, msrc, mdst);
 		break;
 	case OPENSSL_CHAIN_CIPHER_AUTH:
 		process_openssl_cipher_op(op, sess, msrc, mdst);
-		process_openssl_auth_op(op, sess, mdst, mdst);
+		process_openssl_auth_op(qp, op, sess, mdst, mdst);
 		break;
 	case OPENSSL_CHAIN_AUTH_CIPHER:
-		process_openssl_auth_op(op, sess, msrc, mdst);
+		process_openssl_auth_op(qp, op, sess, msrc, mdst);
 		process_openssl_cipher_op(op, sess, msrc, mdst);
 		break;
 	case OPENSSL_CHAIN_COMBINED:
diff --git a/drivers/crypto/openssl/rte_openssl_pmd_private.h b/drivers/crypto/openssl/rte_openssl_pmd_private.h
index b7f7475..93937d5 100644
--- a/drivers/crypto/openssl/rte_openssl_pmd_private.h
+++ b/drivers/crypto/openssl/rte_openssl_pmd_private.h
@@ -59,6 +59,8 @@
 #define OPENSSL_LOG_DBG(fmt, args...)
 #endif
 
+/* Maximum length for digest (SHA-512 needs 64 bytes) */
+#define DIGEST_LENGTH_MAX 64
 
 /** OPENSSL operation order mode enumerator */
 enum openssl_chain_order {
@@ -103,6 +105,11 @@ struct openssl_qp {
 	/**< Session Mempool */
 	struct rte_cryptodev_stats stats;
 	/**< Queue pair statistics */
+	uint8_t temp_digest[DIGEST_LENGTH_MAX];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 /** OPENSSL crypto private session structure */
-- 
2.9.4

[dpdk-dev] [PATCH v2 4/8] crypto/kasumi: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/kasumi/rte_kasumi_pmd.c         | 22 ++++++++--------------
 drivers/crypto/kasumi/rte_kasumi_pmd_private.h |  7 +++++++
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c b/drivers/crypto/kasumi/rte_kasumi_pmd.c
index 38cd8a9..4177c3a 100644
--- a/drivers/crypto/kasumi/rte_kasumi_pmd.c
+++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c
@@ -44,7 +44,6 @@
 
 #define KASUMI_KEY_LENGTH 16
 #define KASUMI_IV_LENGTH 8
-#define KASUMI_DIGEST_LENGTH 4
 #define KASUMI_MAX_BURST 4
 #define BYTE_LEN 8
 
@@ -261,7 +260,7 @@ process_kasumi_cipher_op_bit(struct rte_crypto_op *op,
 
 /** Generate/verify hash from mbufs with same hash key. */
 static int
-process_kasumi_hash_op(struct rte_crypto_op **ops,
+process_kasumi_hash_op(struct kasumi_qp *qp, struct rte_crypto_op **ops,
 		struct kasumi_session *session,
 		uint8_t num_ops)
 {
@@ -287,8 +286,7 @@ process_kasumi_hash_op(struct rte_crypto_op **ops,
 		num_bytes = length_in_bits >> 3;
 
 		if (session->auth_op == RTE_CRYPTO_AUTH_OP_VERIFY) {
-			dst = (uint8_t *)rte_pktmbuf_append(ops[i]->sym->m_src,
-					KASUMI_DIGEST_LENGTH);
+			dst = qp->temp_digest;
 			sso_kasumi_f9_1_buffer(&session->pKeySched_hash, src,
 					num_bytes, dst);
 
@@ -296,10 +294,6 @@ process_kasumi_hash_op(struct rte_crypto_op **ops,
 			if (memcmp(dst, ops[i]->sym->auth.digest.data,
 					KASUMI_DIGEST_LENGTH) != 0)
 				ops[i]->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
-
-			/* Trim area used for digest from mbuf. */
-			rte_pktmbuf_trim(ops[i]->sym->m_src,
-					KASUMI_DIGEST_LENGTH);
 		} else  {
 			dst = ops[i]->sym->auth.digest.data;
 
@@ -327,16 +321,16 @@ process_ops(struct rte_crypto_op **ops, struct kasumi_session *session,
 				session, num_ops);
 		break;
 	case KASUMI_OP_ONLY_AUTH:
-		processed_ops = process_kasumi_hash_op(ops, session,
+		processed_ops = process_kasumi_hash_op(qp, ops, session,
 				num_ops);
 		break;
 	case KASUMI_OP_CIPHER_AUTH:
 		processed_ops = process_kasumi_cipher_op(ops, session,
 				num_ops);
-		process_kasumi_hash_op(ops, session, processed_ops);
+		process_kasumi_hash_op(qp, ops, session, processed_ops);
 		break;
 	case KASUMI_OP_AUTH_CIPHER:
-		processed_ops = process_kasumi_hash_op(ops, session,
+		processed_ops = process_kasumi_hash_op(qp, ops, session,
 				num_ops);
 		process_kasumi_cipher_op(ops, session, processed_ops);
 		break;
@@ -384,15 +378,15 @@ process_op_bit(struct rte_crypto_op *op, struct kasumi_session *session,
 				session);
 		break;
 	case KASUMI_OP_ONLY_AUTH:
-		processed_op = process_kasumi_hash_op(&op, session, 1);
+		processed_op = process_kasumi_hash_op(qp, &op, session, 1);
 		break;
 	case KASUMI_OP_CIPHER_AUTH:
 		processed_op = process_kasumi_cipher_op_bit(op, session);
 		if (processed_op == 1)
-			process_kasumi_hash_op(&op, session, 1);
+			process_kasumi_hash_op(qp, &op, session, 1);
 		break;
 	case KASUMI_OP_AUTH_CIPHER:
-		processed_op = process_kasumi_hash_op(&op, session, 1);
+		processed_op = process_kasumi_hash_op(qp, &op, session, 1);
 		if (processed_op == 1)
 			process_kasumi_cipher_op_bit(op, session);
 		break;
diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd_private.h b/drivers/crypto/kasumi/rte_kasumi_pmd_private.h
index 0ce2a2e..5f7044b 100644
--- a/drivers/crypto/kasumi/rte_kasumi_pmd_private.h
+++ b/drivers/crypto/kasumi/rte_kasumi_pmd_private.h
@@ -58,6 +58,8 @@
 #define KASUMI_LOG_DBG(fmt, args...)
 #endif
 
+#define KASUMI_DIGEST_LENGTH 4
+
 /** private data structure for each virtual KASUMI device */
 struct kasumi_private {
 	unsigned max_nb_queue_pairs;
@@ -78,6 +80,11 @@ struct kasumi_qp {
 	/**< Session Mempool */
 	struct rte_cryptodev_stats qp_stats;
 	/**< Queue pair statistics */
+	uint8_t temp_digest[KASUMI_DIGEST_LENGTH];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 enum kasumi_operation {
-- 
2.9.4

[dpdk-dev] [PATCH v2 5/8] crypto/snow3g: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/snow3g/rte_snow3g_pmd.c         | 22 ++++++++--------------
 drivers/crypto/snow3g/rte_snow3g_pmd_private.h |  7 +++++++
 2 files changed, 15 insertions(+), 14 deletions(-)

diff --git a/drivers/crypto/snow3g/rte_snow3g_pmd.c b/drivers/crypto/snow3g/rte_snow3g_pmd.c
index dad4506..15f9381 100644
--- a/drivers/crypto/snow3g/rte_snow3g_pmd.c
+++ b/drivers/crypto/snow3g/rte_snow3g_pmd.c
@@ -43,7 +43,6 @@
 #include "rte_snow3g_pmd_private.h"
 
 #define SNOW3G_IV_LENGTH 16
-#define SNOW3G_DIGEST_LENGTH 4
 #define SNOW3G_MAX_BURST 8
 #define BYTE_LEN 8
 
@@ -263,7 +262,7 @@ process_snow3g_cipher_op_bit(struct rte_crypto_op *op,
 
 /** Generate/verify hash from mbufs with same hash key. */
 static int
-process_snow3g_hash_op(struct rte_crypto_op **ops,
+process_snow3g_hash_op(struct snow3g_qp *qp, struct rte_crypto_op **ops,
 		struct snow3g_session *session,
 		uint8_t num_ops)
 {
@@ -289,8 +288,7 @@ process_snow3g_hash_op(struct rte_crypto_op **ops,
 				session->auth_iv_offset);
 
 		if (session->auth_op == RTE_CRYPTO_AUTH_OP_VERIFY) {
-			dst = (uint8_t *)rte_pktmbuf_append(ops[i]->sym->m_src,
-					SNOW3G_DIGEST_LENGTH);
+			dst = qp->temp_digest;
 
 			sso_snow3g_f9_1_buffer(&session->pKeySched_hash,
 					iv, src,
@@ -299,10 +297,6 @@ process_snow3g_hash_op(struct rte_crypto_op **ops,
 			if (memcmp(dst, ops[i]->sym->auth.digest.data,
 					SNOW3G_DIGEST_LENGTH) != 0)
 				ops[i]->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
-
-			/* Trim area used for digest from mbuf. */
-			rte_pktmbuf_trim(ops[i]->sym->m_src,
-					SNOW3G_DIGEST_LENGTH);
 		} else  {
 			dst = ops[i]->sym->auth.digest.data;
 
@@ -346,16 +340,16 @@ process_ops(struct rte_crypto_op **ops, struct snow3g_session *session,
 				session, num_ops);
 		break;
 	case SNOW3G_OP_ONLY_AUTH:
-		processed_ops = process_snow3g_hash_op(ops, session,
+		processed_ops = process_snow3g_hash_op(qp, ops, session,
 				num_ops);
 		break;
 	case SNOW3G_OP_CIPHER_AUTH:
 		processed_ops = process_snow3g_cipher_op(ops, session,
 				num_ops);
-		process_snow3g_hash_op(ops, session, processed_ops);
+		process_snow3g_hash_op(qp, ops, session, processed_ops);
 		break;
 	case SNOW3G_OP_AUTH_CIPHER:
-		processed_ops = process_snow3g_hash_op(ops, session,
+		processed_ops = process_snow3g_hash_op(qp, ops, session,
 				num_ops);
 		process_snow3g_cipher_op(ops, session, processed_ops);
 		break;
@@ -403,15 +397,15 @@ process_op_bit(struct rte_crypto_op *op, struct snow3g_session *session,
 				session);
 		break;
 	case SNOW3G_OP_ONLY_AUTH:
-		processed_op = process_snow3g_hash_op(&op, session, 1);
+		processed_op = process_snow3g_hash_op(qp, &op, session, 1);
 		break;
 	case SNOW3G_OP_CIPHER_AUTH:
 		processed_op = process_snow3g_cipher_op_bit(op, session);
 		if (processed_op == 1)
-			process_snow3g_hash_op(&op, session, 1);
+			process_snow3g_hash_op(qp, &op, session, 1);
 		break;
 	case SNOW3G_OP_AUTH_CIPHER:
-		processed_op = process_snow3g_hash_op(&op, session, 1);
+		processed_op = process_snow3g_hash_op(qp, &op, session, 1);
 		if (processed_op == 1)
 			process_snow3g_cipher_op_bit(op, session);
 		break;
diff --git a/drivers/crypto/snow3g/rte_snow3g_pmd_private.h b/drivers/crypto/snow3g/rte_snow3g_pmd_private.h
index fba3cb8..7b9729f 100644
--- a/drivers/crypto/snow3g/rte_snow3g_pmd_private.h
+++ b/drivers/crypto/snow3g/rte_snow3g_pmd_private.h
@@ -58,6 +58,8 @@
 #define SNOW3G_LOG_DBG(fmt, args...)
 #endif
 
+#define SNOW3G_DIGEST_LENGTH 4
+
 /** private data structure for each virtual SNOW 3G device */
 struct snow3g_private {
 	unsigned max_nb_queue_pairs;
@@ -78,6 +80,11 @@ struct snow3g_qp {
 	/**< Session Mempool */
 	struct rte_cryptodev_stats qp_stats;
 	/**< Queue pair statistics */
+	uint8_t temp_digest[SNOW3G_DIGEST_LENGTH];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 enum snow3g_operation {
-- 
2.9.4

[dpdk-dev] [PATCH v2 6/8] crypto/zuc: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/zuc/rte_zuc_pmd.c         | 16 +++++-----------
 drivers/crypto/zuc/rte_zuc_pmd_private.h |  7 +++++++
 2 files changed, 12 insertions(+), 11 deletions(-)

diff --git a/drivers/crypto/zuc/rte_zuc_pmd.c b/drivers/crypto/zuc/rte_zuc_pmd.c
index b301711..dd882c4 100644
--- a/drivers/crypto/zuc/rte_zuc_pmd.c
+++ b/drivers/crypto/zuc/rte_zuc_pmd.c
@@ -42,7 +42,6 @@
 
 #include "rte_zuc_pmd_private.h"
 
-#define ZUC_DIGEST_LENGTH 4
 #define ZUC_MAX_BURST 8
 #define BYTE_LEN 8
 
@@ -258,7 +257,7 @@ process_zuc_cipher_op(struct rte_crypto_op **ops,
 
 /** Generate/verify hash from mbufs with same hash key. */
 static int
-process_zuc_hash_op(struct rte_crypto_op **ops,
+process_zuc_hash_op(struct zuc_qp *qp, struct rte_crypto_op **ops,
 		struct zuc_session *session,
 		uint8_t num_ops)
 {
@@ -285,8 +284,7 @@ process_zuc_hash_op(struct rte_crypto_op **ops,
 				session->auth_iv_offset);
 
 		if (session->auth_op == RTE_CRYPTO_AUTH_OP_VERIFY) {
-			dst = (uint32_t *)rte_pktmbuf_append(ops[i]->sym->m_src,
-					ZUC_DIGEST_LENGTH);
+			dst = (uint32_t *)qp->temp_digest;
 
 			sso_zuc_eia3_1_buffer(session->pKey_hash,
 					iv, src,
@@ -295,10 +293,6 @@ process_zuc_hash_op(struct rte_crypto_op **ops,
 			if (memcmp(dst, ops[i]->sym->auth.digest.data,
 					ZUC_DIGEST_LENGTH) != 0)
 				ops[i]->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
-
-			/* Trim area used for digest from mbuf. */
-			rte_pktmbuf_trim(ops[i]->sym->m_src,
-					ZUC_DIGEST_LENGTH);
 		} else  {
 			dst = (uint32_t *)ops[i]->sym->auth.digest.data;
 
@@ -327,16 +321,16 @@ process_ops(struct rte_crypto_op **ops, struct zuc_session *session,
 				session, num_ops);
 		break;
 	case ZUC_OP_ONLY_AUTH:
-		processed_ops = process_zuc_hash_op(ops, session,
+		processed_ops = process_zuc_hash_op(qp, ops, session,
 				num_ops);
 		break;
 	case ZUC_OP_CIPHER_AUTH:
 		processed_ops = process_zuc_cipher_op(ops, session,
 				num_ops);
-		process_zuc_hash_op(ops, session, processed_ops);
+		process_zuc_hash_op(qp, ops, session, processed_ops);
 		break;
 	case ZUC_OP_AUTH_CIPHER:
-		processed_ops = process_zuc_hash_op(ops, session,
+		processed_ops = process_zuc_hash_op(qp, ops, session,
 				num_ops);
 		process_zuc_cipher_op(ops, session, processed_ops);
 		break;
diff --git a/drivers/crypto/zuc/rte_zuc_pmd_private.h b/drivers/crypto/zuc/rte_zuc_pmd_private.h
index b706e0a..a57b8cd 100644
--- a/drivers/crypto/zuc/rte_zuc_pmd_private.h
+++ b/drivers/crypto/zuc/rte_zuc_pmd_private.h
@@ -59,6 +59,8 @@
 #endif
 
 #define ZUC_IV_KEY_LENGTH 16
+#define ZUC_DIGEST_LENGTH 4
+
 /** private data structure for each virtual ZUC device */
 struct zuc_private {
 	unsigned max_nb_queue_pairs;
@@ -79,6 +81,11 @@ struct zuc_qp {
 	/**< Session Mempool */
 	struct rte_cryptodev_stats qp_stats;
 	/**< Queue pair statistics */
+	uint8_t temp_digest[ZUC_DIGEST_LENGTH];
+	/**< Buffer used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
 enum zuc_operation {
-- 
2.9.4

[dpdk-dev] [PATCH v2 7/8] crypto/aesni_mb: do not append digest

[Pablo de Lara]

When performing an authentication verification,
the PMD was using memory at the end of the input buffer,
to store temporarily the digest.
This operation requires the buffer to have enough
tailroom unnecessarily.
Instead, memory is allocated for each queue pair, to store
temporarily the digest generated by the driver, so it can
be compared with the one provided in the crypto operation,
without needing to touch the input buffer.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c         | 36 +++++++---------------
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c     |  5 +++
 drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h | 12 +++++++-
 3 files changed, 27 insertions(+), 26 deletions(-)

diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index 16e1451..529f469 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -407,7 +407,7 @@ get_session(struct aesni_mb_qp *qp, struct rte_crypto_op *op)
  */
 static inline int
 set_mb_job_params(JOB_AES_HMAC *job, struct aesni_mb_qp *qp,
-		struct rte_crypto_op *op)
+		struct rte_crypto_op *op, uint8_t *digest_idx)
 {
 	struct rte_mbuf *m_src = op->sym->m_src, *m_dst;
 	struct aesni_mb_session *session;
@@ -466,19 +466,8 @@ set_mb_job_params(JOB_AES_HMAC *job, struct aesni_mb_qp *qp,
 	/* Set digest output location */
 	if (job->hash_alg != NULL_HASH &&
 			session->auth.operation == RTE_CRYPTO_AUTH_OP_VERIFY) {
-		job->auth_tag_output = (uint8_t *)rte_pktmbuf_append(m_dst,
-				get_digest_byte_length(job->hash_alg));
-
-		if (job->auth_tag_output == NULL) {
-			MB_LOG_ERR("failed to allocate space in output mbuf "
-					"for temp digest");
-			op->status = RTE_CRYPTO_OP_STATUS_ERROR;
-			return -1;
-		}
-
-		memset(job->auth_tag_output, 0,
-				sizeof(get_digest_byte_length(job->hash_alg)));
-
+		job->auth_tag_output = qp->temp_digests[*digest_idx];
+		*digest_idx = (*digest_idx + 1) % MAX_JOBS;
 	} else {
 		job->auth_tag_output = op->sym->auth.digest.data;
 	}
@@ -507,22 +496,17 @@ set_mb_job_params(JOB_AES_HMAC *job, struct aesni_mb_qp *qp,
 
 	/* Set user data to be crypto operation data struct */
 	job->user_data = op;
-	job->user_data2 = m_dst;
 
 	return 0;
 }
 
 static inline void
-verify_digest(JOB_AES_HMAC *job, struct rte_crypto_op *op) {
-	struct rte_mbuf *m_dst = (struct rte_mbuf *)job->user_data2;
-
+verify_digest(struct aesni_mb_qp *qp __rte_unused, JOB_AES_HMAC *job,
+		struct rte_crypto_op *op) {
 	/* Verify digest if required */
 	if (memcmp(job->auth_tag_output, op->sym->auth.digest.data,
 			job->auth_tag_output_len_in_bytes) != 0)
 		op->status = RTE_CRYPTO_OP_STATUS_AUTH_FAILED;
-
-	/* trim area used for digest from mbuf */
-	rte_pktmbuf_trim(m_dst,	get_digest_byte_length(job->hash_alg));
 }
 
 /**
@@ -532,8 +516,7 @@ verify_digest(JOB_AES_HMAC *job, struct rte_crypto_op *op) {
  * @param job	JOB_AES_HMAC job to process
  *
  * @return
- * - Returns processed crypto operation which mbuf is trimmed of output digest
- *   used in verification of supplied digest.
+ * - Returns processed crypto operation.
  * - Returns NULL on invalid job
  */
 static inline struct rte_crypto_op *
@@ -552,7 +535,7 @@ post_process_mb_job(struct aesni_mb_qp *qp, JOB_AES_HMAC *job)
 			if (job->hash_alg != NULL_HASH) {
 				if (sess->auth.operation ==
 						RTE_CRYPTO_AUTH_OP_VERIFY)
-					verify_digest(job, op);
+					verify_digest(qp, job, op);
 			}
 			break;
 		default:
@@ -650,6 +633,7 @@ aesni_mb_pmd_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
 	if (unlikely(nb_ops == 0))
 		return 0;
 
+	uint8_t digest_idx = qp->digest_idx;
 	do {
 		/* Get next operation to process from ingress queue */
 		retval = rte_ring_dequeue(qp->ingress_queue, (void **)&op);
@@ -667,7 +651,7 @@ aesni_mb_pmd_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
 			job = (*qp->op_fns->job.get_next)(&qp->mb_mgr);
 		}
 
-		retval = set_mb_job_params(job, qp, op);
+		retval = set_mb_job_params(job, qp, op, &digest_idx);
 		if (unlikely(retval != 0)) {
 			qp->stats.dequeue_err_count++;
 			set_job_null_op(job);
@@ -687,6 +671,8 @@ aesni_mb_pmd_dequeue_burst(void *queue_pair, struct rte_crypto_op **ops,
 
 	} while (processed_jobs < nb_ops);
 
+	qp->digest_idx = digest_idx;
+
 	if (processed_jobs < 1)
 		processed_jobs += flush_mb_mgr(qp,
 				&ops[processed_jobs],
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
index 692b354..4be9b80 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
@@ -430,6 +430,11 @@ aesni_mb_pmd_qp_setup(struct rte_cryptodev *dev, uint16_t qp_id,
 
 	memset(&qp->stats, 0, sizeof(qp->stats));
 
+	char mp_name[RTE_MEMPOOL_NAMESIZE];
+
+	snprintf(mp_name, RTE_MEMPOOL_NAMESIZE,
+				"digest_mp_%u_%u", dev->data->dev_id, qp_id);
+
 	/* Initialise multi-buffer manager */
 	(*qp->op_fns->job.init_mgr)(&qp->mb_mgr);
 	return 0;
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h
index 6676948..fe3bd73 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_private.h
@@ -61,6 +61,8 @@
 #define HMAC_IPAD_VALUE			(0x36)
 #define HMAC_OPAD_VALUE			(0x5C)
 
+/* Maximum length for digest (SHA-512 truncated needs 32 bytes) */
+#define DIGEST_LENGTH_MAX 32
 static const unsigned auth_blocksize[] = {
 		[MD5]		= 64,
 		[SHA1]		= 64,
@@ -164,9 +166,17 @@ struct aesni_mb_qp {
 	/**< Session Mempool */
 	struct rte_cryptodev_stats stats;
 	/**< Queue pair statistics */
+	uint8_t digest_idx;
+	/**< Index of the next slot to be used in temp_digests,
+	 * to store the digest for a given operation
+	 */
+	uint8_t temp_digests[MAX_JOBS][DIGEST_LENGTH_MAX];
+	/**< Buffers used to store the digest generated
+	 * by the driver when verifying a digest provided
+	 * by the user (using authentication verify operation)
+	 */
 } __rte_cache_aligned;
 
-
 /** AES-NI multi-buffer private session structure */
 struct aesni_mb_session {
 	JOB_CHAIN_ORDER chain_order;
-- 
2.9.4

[dpdk-dev] [PATCH v2 8/8] test/crypto: do not allocate extra memory for digest

[Pablo de Lara]

Now that PMDs do not need extra space in the mbuf
to store temporarily the digest when verifying
an authentication tag, it is not required to allocate
more memory in the mbufs passed to cryptodev.

Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 test/test/test_cryptodev_blockcipher.c | 29 ++---------------------------
 1 file changed, 2 insertions(+), 27 deletions(-)

diff --git a/test/test/test_cryptodev_blockcipher.c b/test/test/test_cryptodev_blockcipher.c
index 6089af4..f8222bd 100644
--- a/test/test/test_cryptodev_blockcipher.c
+++ b/test/test/test_cryptodev_blockcipher.c
@@ -452,25 +452,13 @@ test_blockcipher_one_case(const struct blockcipher_test_case *t,
 	if (t->feature_mask & BLOCKCIPHER_TEST_FEATURE_OOP) {
 		struct rte_mbuf *mbuf;
 		uint8_t value;
-		uint32_t head_unchanged_len = 0, changed_len = 0;
+		uint32_t head_unchanged_len, changed_len = 0;
 		uint32_t i;
 
 		mbuf = sym_op->m_src;
-		if (t->op_mask & BLOCKCIPHER_TEST_OP_AUTH_VERIFY) {
-			/* white-box test: PMDs use some of the
-			 * tailroom as temp storage in verify case
-			 */
-			head_unchanged_len = rte_pktmbuf_headroom(mbuf)
-					+ rte_pktmbuf_data_len(mbuf);
-			changed_len = digest_len;
-		} else {
-			head_unchanged_len = mbuf->buf_len;
-			changed_len = 0;
-		}
+		head_unchanged_len = mbuf->buf_len;
 
 		for (i = 0; i < mbuf->buf_len; i++) {
-			if (i == head_unchanged_len)
-				i += changed_len;
 			value = *((uint8_t *)(mbuf->buf_addr)+i);
 			if (value != tmp_src_buf[i]) {
 				snprintf(test_msg, BLOCKCIPHER_TEST_MSG_LEN,
@@ -531,19 +519,6 @@ test_blockcipher_one_case(const struct blockcipher_test_case *t,
 		if (t->op_mask & BLOCKCIPHER_TEST_OP_AUTH_GEN)
 			changed_len += digest_len;
 
-		if (t->op_mask & BLOCKCIPHER_TEST_OP_AUTH_VERIFY) {
-			/* white-box test: PMDs use some of the
-			 * tailroom as temp storage in verify case
-			 */
-			if (t->op_mask & BLOCKCIPHER_TEST_OP_CIPHER) {
-				/* This is simplified, not checking digest*/
-				changed_len += digest_len*2;
-			} else {
-				head_unchanged_len += digest_len;
-				changed_len += digest_len;
-			}
-		}
-
 		for (i = 0; i < mbuf->buf_len; i++) {
 			if (i == head_unchanged_len)
 				i += changed_len;
-- 
2.9.4

Re: [dpdk-dev] [PATCH 1/8] crypto/aesni_gcm: do not append digest

[De Lara Guarch, Pablo]

Hi Fan,

> -----Original Message-----
> From: Zhang, Roy Fan
> Sent: Monday, September 4, 2017 11:08 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org; Zhang, Roy Fan <roy.fan.zhang@intel.com>
> Subject: RE: [dpdk-dev] [PATCH 1/8] crypto/aesni_gcm: do not append
> digest
> 
> Hi Pablo,
> 
> Thanks for the patch. It is very good idea of allocating only necessary buffer
> for digests in the operation.
> Comments inline:
> 
> > -----Original Message-----
> > From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Pablo de Lara
> > Sent: Friday, August 18, 2017 8:21 AM
> > To: Doherty, Declan <declan.doherty@intel.com>;
> > jerin.jacob@caviumnetworks.com
> > Cc: dev@dpdk.org; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>
> > Subject: [dpdk-dev] [PATCH 1/8] crypto/aesni_gcm: do not append digest
> >
> > When performing an authentication verification, the PMD was using
> > memory at the end of the input buffer, to store temporarily the digest.
> > This operation requires the buffer to have enough tailroom unnecessarily.
> > Instead, memory is allocated for each queue pair, to store temporarily
> > the digest generated by the driver, so it can be compared with the one
> > provided in the crypto operation, without needing to touch the input
> buffer.
> >
> > Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> > ---
> >  drivers/crypto/aesni_gcm/aesni_gcm_pmd.c         | 31 +++++----------------
> ---
> >  drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h |  7 ++++++
> >  2 files changed, 13 insertions(+), 25 deletions(-)
> >
> > diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > index d9c91d0..ae670a7 100644
> > --- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > +++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
> > @@ -298,14 +298,7 @@ process_gcm_crypto_op(struct aesni_gcm_qp
> *qp,
> > struct rte_crypto_op *op,
> >  				sym_op->aead.digest.data,
> >  				(uint64_t)session->digest_length);
> >  	} else if (session->op ==
> > AESNI_GCM_OP_AUTHENTICATED_DECRYPTION) {
> > -		uint8_t *auth_tag = (uint8_t
> > *)rte_pktmbuf_append(sym_op->m_dst ?
> > -				sym_op->m_dst : sym_op->m_src,
> > -				session->digest_length);
> > -
> > -		if (!auth_tag) {
> > -			GCM_LOG_ERR("auth_tag");
> > -			return -1;
> > -		}
> 
> qp->tmp_digest has already the data type of uint8_t*, the casting is not
> necessary here. Although the "&" here seems to be wrong. auth_tag didn't
> point to qp->tmp_digest but a buffer with the address as &qp->tmp_digest.

Thanks for spotting this! Will send a v2 soon.

Pablo

Re: [dpdk-dev] [PATCH v2 0/8] Remove temporary digest allocation

[Zhang, Roy Fan]

Hi Pablo,

Thanks, looks great!

Regards,
Fan

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Tuesday, September 5, 2017 3:20 AM
> To: Doherty, Declan <declan.doherty@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; jerin.jacob@caviumnetworks.com
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH v2 0/8] Remove temporary digest allocation
> 
> When performing authentication verification, some crypto PMDs require
> extra memory where the generated digest can be placed.
> Currently, these PMDs are getting the memory from the end of the source
> mbuf, which might fail if there is not enough tailroom.
> 
> To avoid this situation, some memory is allocated in each queue pair of the
> device, to store temporarily these digests.
> 
> Changes in v2:
> - Removed incorrect indirection when getting the memory
>   to store the generated digest (i.e. removed "&" in &temp_digest)

Series Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

Re: [dpdk-dev] [PATCH v2 2/8] crypto/armv8: do not append digest

[De Lara Guarch, Pablo]

Hi Jerin,

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Tuesday, September 5, 2017 3:20 AM
> To: Doherty, Declan <declan.doherty@intel.com>; Zhang, Roy Fan
> <roy.fan.zhang@intel.com>; jerin.jacob@caviumnetworks.com
> Cc: dev@dpdk.org; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH v2 2/8] crypto/armv8: do not append digest
> 
> When performing an authentication verification, the PMD was using
> memory at the end of the input buffer, to store temporarily the digest.
> This operation requires the buffer to have enough tailroom unnecessarily.
> Instead, memory is allocated for each queue pair, to store temporarily the
> digest generated by the driver, so it can be compared with the one provided
> in the crypto operation, without needing to touch the input buffer.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Could you take a look at this patch?
The series have been acked by Fan Zhang, but this patch affects the ARMv8 PMD,
So I would prefer applying it after your review.

Thanks,
Pablo

Re: [dpdk-dev] [PATCH 2/8] crypto/armv8: do not append digest

[Jerin Jacob]

-----Original Message-----
> Date: Fri, 18 Aug 2017 08:20:57 +0100
> From: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> To: declan.doherty@intel.com, jerin.jacob@caviumnetworks.com
> CC: dev@dpdk.org, Pablo de Lara <pablo.de.lara.guarch@intel.com>
> Subject: [PATCH 2/8] crypto/armv8: do not append digest
> X-Mailer: git-send-email 2.9.4
> 
> When performing an authentication verification,
> the PMD was using memory at the end of the input buffer,
> to store temporarily the digest.
> This operation requires the buffer to have enough
> tailroom unnecessarily.
> Instead, memory is allocated for each queue pair, to store
> temporarily the digest generated by the driver, so it can
> be compared with the one provided in the crypto operation,
> without needing to touch the input buffer.
> 
> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Acked-by: Jerin Jacob <jerin.jacob@caviumnetworks.com>

Re: [dpdk-dev] [PATCH v2 0/8] Remove temporary digest allocation

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Zhang, Roy Fan
> Sent: Tuesday, September 5, 2017 1:39 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH v2 0/8] Remove temporary digest allocation
> 
> Hi Pablo,
> 
> Thanks, looks great!
> 
> Regards,
> Fan
> 
> > -----Original Message-----
> > From: De Lara Guarch, Pablo
> > Sent: Tuesday, September 5, 2017 3:20 AM
> > To: Doherty, Declan <declan.doherty@intel.com>; Zhang, Roy Fan
> > <roy.fan.zhang@intel.com>; jerin.jacob@caviumnetworks.com
> > Cc: dev@dpdk.org; De Lara Guarch, Pablo
> > <pablo.de.lara.guarch@intel.com>
> > Subject: [PATCH v2 0/8] Remove temporary digest allocation
> >
> > When performing authentication verification, some crypto PMDs require
> > extra memory where the generated digest can be placed.
> > Currently, these PMDs are getting the memory from the end of the
> > source mbuf, which might fail if there is not enough tailroom.
> >
> > To avoid this situation, some memory is allocated in each queue pair
> > of the device, to store temporarily these digests.
> >
> > Changes in v2:
> > - Removed incorrect indirection when getting the memory
> >   to store the generated digest (i.e. removed "&" in &temp_digest)
> 
> Series Acked-by: Fan Zhang <roy.fan.zhang@intel.com>

Applied to dpdk-next-crypto.

Pablo