[1/3] crypto/mvsam: add S/G support to crypto dirver

Message ID 1535372574-18950-2-git-send-email-tdu@semihalf.com
State Superseded, archived
Delegated to: akhil goyal
Headers show
Series
  • crypto/mvsam: yet another round of features
Related show

Checks

Context Check Description
ci/Intel-compilation fail apply issues
ci/checkpatch success coding style OK

Commit Message

Tomasz Duszynski Aug. 27, 2018, 12:22 p.m.
From: Zyta Szpak <zr@semihalf.com>

The patch adds support for chained source mbufs given
to crypto operations. The crypto engine accepts source buffer
containing a number of segments. The destination buffer
stays the same - always one segment.
On decryption, EIP engine will look for digest at 'auth_icv_offset'
offset in SRC buffer.It must be placed in the last segment and the
offset must be set to reach digest in the last segment.
If application doesn't placed digest in source mbuf, driver try to
copy it to a last segment.

Signed-off-by: Zyta Szpak <zr@semihalf.com>
Signed-off-by: Natalie Samsonov <nsamsono@marvell.com>
Reviewed-by: Dmitri Epshtein <dima@marvell.com>
---
 drivers/crypto/mvsam/rte_mrvl_pmd.c         | 96 +++++++++++++++++++++--------
 drivers/crypto/mvsam/rte_mrvl_pmd_private.h |  7 +++
 2 files changed, 76 insertions(+), 27 deletions(-)

Comments

Akhil Goyal Sept. 17, 2018, 2:07 p.m. | #1
Hi Tomasz,

On 8/27/2018 5:52 PM, Tomasz Duszynski wrote:

> From: Zyta Szpak <zr@semihalf.com>
>
> The patch adds support for chained source mbufs given
> to crypto operations. The crypto engine accepts source buffer
> containing a number of segments. The destination buffer
> stays the same - always one segment.
> On decryption, EIP engine will look for digest at 'auth_icv_offset'
> offset in SRC buffer.It must be placed in the last segment and the
> offset must be set to reach digest in the last segment.
> If application doesn't placed digest in source mbuf, driver try to
> copy it to a last segment.
>
> Signed-off-by: Zyta Szpak <zr@semihalf.com>
> Signed-off-by: Natalie Samsonov <nsamsono@marvell.com>
> Reviewed-by: Dmitri Epshtein <dima@marvell.com>
> ---
>   drivers/crypto/mvsam/rte_mrvl_pmd.c         | 96 +++++++++++++++++++++--------
>   drivers/crypto/mvsam/rte_mrvl_pmd_private.h |  7 +++
>   2 files changed, 76 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd.c b/drivers/crypto/mvsam/rte_mrvl_pmd.c
> index 961802e..001aa28 100644
> --- a/drivers/crypto/mvsam/rte_mrvl_pmd.c
> +++ b/drivers/crypto/mvsam/rte_mrvl_pmd.c
> @@ -452,8 +452,10 @@ mrvl_request_prepare(struct sam_cio_op_params *request,
>   		struct rte_crypto_op *op)
>   {
>   	struct mrvl_crypto_session *sess;
> -	struct rte_mbuf *dst_mbuf;
> +	struct rte_mbuf *src_mbuf, *dst_mbuf;
> +	uint16_t segments_nb;
>   	uint8_t *digest;
> +	int i;
>   
>   	if (unlikely(op->sess_type == RTE_CRYPTO_OP_SESSIONLESS)) {
>   		MRVL_CRYPTO_LOG_ERR("MRVL CRYPTO PMD only supports session "
> @@ -469,29 +471,47 @@ mrvl_request_prepare(struct sam_cio_op_params *request,
>   		return -EINVAL;
>   	}
>   
> -	/*
> +	request->sa = sess->sam_sess;
> +	request->cookie = op;
> +
> +	src_mbuf = op->sym->m_src;
> +	segments_nb = src_mbuf->nb_segs;
> +	/* The following conditions must be met:
> +	 * - Destination buffer is required when segmented source buffer
> +	 * - Segmented destination buffer is not supported
> +	 */
> +	if ((segments_nb > 1) && (!op->sym->m_dst)) {
> +		MRVL_CRYPTO_LOG_ERR("op->sym->m_dst = NULL!\n");
> +		return -1;
> +	}
> +	/* For non SG case:
>   	 * If application delivered us null dst buffer, it means it expects
>   	 * us to deliver the result in src buffer.
>   	 */
>   	dst_mbuf = op->sym->m_dst ? op->sym->m_dst : op->sym->m_src;
>   
> -	request->sa = sess->sam_sess;
> -	request->cookie = op;
> -
> -	/* Single buffers only, sorry. */
> -	request->num_bufs = 1;
> -	request->src = src_bd;
> -	src_bd->vaddr = rte_pktmbuf_mtod(op->sym->m_src, void *);
> -	src_bd->paddr = rte_pktmbuf_iova(op->sym->m_src);
> -	src_bd->len = rte_pktmbuf_data_len(op->sym->m_src);
> -
> -	/* Empty source. */
> -	if (rte_pktmbuf_data_len(op->sym->m_src) == 0) {
> -		/* EIP does not support 0 length buffers. */
> -		MRVL_CRYPTO_LOG_ERR("Buffer length == 0 not supported!");
> +	if (!rte_pktmbuf_is_contiguous(dst_mbuf)) {
> +		MRVL_CRYPTO_LOG_ERR("Segmented destination buffer "
> +				    "not supported.\n");
>   		return -1;
>   	}
>   
> +	request->num_bufs = segments_nb;
> +	for (i = 0; i < segments_nb; i++) {
> +		/* Empty source. */
> +		if (rte_pktmbuf_data_len(src_mbuf) == 0) {
> +			/* EIP does not support 0 length buffers. */
> +			MRVL_CRYPTO_LOG_ERR("Buffer length == 0 not supported!");
> +			return -1;
> +		}
> +		src_bd[i].vaddr = rte_pktmbuf_mtod(src_mbuf, void *);
> +		src_bd[i].paddr = rte_pktmbuf_iova(src_mbuf);
> +		src_bd[i].len = rte_pktmbuf_data_len(src_mbuf);
> +
> +		src_mbuf = src_mbuf->next;
> +	}
> +	request->src = src_bd;
> +
>   	/* Empty destination. */
>   	if (rte_pktmbuf_data_len(dst_mbuf) == 0) {
>   		/* Make dst buffer fit at least source data. */
> @@ -542,7 +562,7 @@ mrvl_request_prepare(struct sam_cio_op_params *request,
>   
>   	/*
>   	 * EIP supports only scenarios where ICV(digest buffer) is placed at
> -	 * auth_icv_offset. Any other placement means risking errors.
> +	 * auth_icv_offset.
>   	 */
>   	if (sess->sam_sess_params.dir == SAM_DIR_ENCRYPT) {
>   		/*
> @@ -551,17 +571,36 @@ mrvl_request_prepare(struct sam_cio_op_params *request,
>   		 */
>   		if (rte_pktmbuf_mtod_offset(
>   				dst_mbuf, uint8_t *,
> -				request->auth_icv_offset) == digest) {
> +				request->auth_icv_offset) == digest)
>   			return 0;
> -		}
>   	} else {/* sess->sam_sess_params.dir == SAM_DIR_DECRYPT */
>   		/*
>   		 * EIP will look for digest at auth_icv_offset
> -		 * offset in SRC buffer.
> +		 * offset in SRC buffer. It must be placed in the last
> +		 * segment and the offset must be set to reach digest
> +		 * in the last segment
>   		 */
> -		if (rte_pktmbuf_mtod_offset(
> -				op->sym->m_src, uint8_t *,
> -				request->auth_icv_offset) == digest) {
> +		struct rte_mbuf *last_seg =  op->sym->m_src;
> +		uint32_t d_offset = request->auth_icv_offset;
> +		u32 d_size = sess->sam_sess_params.u.basic.auth_icv_len;
> +		unsigned char *d_ptr;
> +
> +		/* Find the last segment and the offset for the last segment */
> +		while ((last_seg->next != NULL) &&
> +				(d_offset >= last_seg->data_len)) {
> +			d_offset -= last_seg->data_len;
> +			last_seg = last_seg->next;
> +		}
> +
> +		if (rte_pktmbuf_mtod_offset(last_seg, uint8_t *,
> +					    d_offset) == digest)
> +			return 0;
> +
> +		/* copy digest to last segment */
> +		if (last_seg->buf_len >= (d_size + d_offset)) {
> +			d_ptr = (unsigned char *)last_seg->buf_addr +
> +				 d_offset;
> +			rte_memcpy(d_ptr, digest, d_size);
>   			return 0;
>   		}
>   	}
> @@ -597,11 +636,10 @@ mrvl_crypto_pmd_enqueue_burst(void *queue_pair, struct rte_crypto_op **ops,
>   	int ret;
>   	struct sam_cio_op_params requests[nb_ops];
>   	/*
> -	 * DPDK uses single fragment buffers, so we can KISS descriptors.
>   	 * SAM does not store bd pointers, so on-stack scope will be enough.
>   	 */
> -	struct sam_buf_info src_bd[nb_ops];
> -	struct sam_buf_info dst_bd[nb_ops];
> +	struct mrvl_crypto_src_table src_bd[nb_ops];
> +	struct sam_buf_info          dst_bd[nb_ops];
>   	struct mrvl_crypto_qp *qp = (struct mrvl_crypto_qp *)queue_pair;
>   
>   	if (nb_ops == 0)
> @@ -609,11 +647,14 @@ mrvl_crypto_pmd_enqueue_burst(void *queue_pair, struct rte_crypto_op **ops,
>   
>   	/* Prepare the burst. */
>   	memset(&requests, 0, sizeof(requests));
> +	memset(&src_bd, 0, sizeof(src_bd));
>   
>   	/* Iterate through */
>   	for (; iter_ops < nb_ops; ++iter_ops) {
> +		/* store the op id for debug */
> +		src_bd[iter_ops].iter_ops = iter_ops;
>   		if (mrvl_request_prepare(&requests[iter_ops],
> -					&src_bd[iter_ops],
> +					src_bd[iter_ops].src_bd,
>   					&dst_bd[iter_ops],
>   					ops[iter_ops]) < 0) {
>   			MRVL_CRYPTO_LOG_ERR(
> @@ -767,6 +808,7 @@ cryptodev_mrvl_crypto_create(const char *name,
>   
>   	sam_params.max_num_sessions = internals->max_nb_sessions;
>   
> +	/* sam_set_debug_flags(3); */
>   	return sam_init(&sam_params);
>   
>   init_error:
> diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd_private.h b/drivers/crypto/mvsam/rte_mrvl_pmd_private.h
> index c16d95b..0689fc3 100644
> --- a/drivers/crypto/mvsam/rte_mrvl_pmd_private.h
> +++ b/drivers/crypto/mvsam/rte_mrvl_pmd_private.h
> @@ -38,6 +38,8 @@
>    */
>   #define BITS2BYTES(x) ((x) >> 3)
>   
> +#define MRVL_MAX_SEGMENTS 16
> +
>   /** The operation order mode enumerator. */
>   enum mrvl_crypto_chain_order {
>   	MRVL_CRYPTO_CHAIN_CIPHER_ONLY,
> @@ -84,6 +86,11 @@ struct mrvl_crypto_session {
>   	uint16_t cipher_iv_offset;
>   } __rte_cache_aligned;
>   
> +struct mrvl_crypto_src_table {
> +	uint16_t iter_ops;
> +	struct sam_buf_info src_bd[MRVL_MAX_SEGMENTS];
> +} __rte_cache_aligned;
> +
>   /** Set and validate MRVL crypto session parameters */
>   extern int
>   mrvl_crypto_set_session_parameters(struct mrvl_crypto_session *sess,
>
Please update the doc file for pmd and also set appropriate feature flag for scatter gather support.

#define RTE_CRYPTODEV_FF_IN_PLACE_SGL                   (1ULL << 9)

/**< In-place Scatter-gather (SGL) buffers, with multiple segments,

  * are supported

  */

#define RTE_CRYPTODEV_FF_OOP_SGL_IN_SGL_OUT             (1ULL << 10)

/**< Out-of-place Scatter-gather (SGL) buffers are

  * supported in input and output

  */

#define RTE_CRYPTODEV_FF_OOP_SGL_IN_LB_OUT              (1ULL << 11)

/**< Out-of-place Scatter-gather (SGL) buffers are supported

  * in input, combined with linear buffers (LB), with a

  * single segment in output

  */

#define RTE_CRYPTODEV_FF_OOP_LB_IN_SGL_OUT              (1ULL << 12)

/**< Out-of-place Scatter-gather (SGL) buffers are supported

  * in output, combined with linear buffers (LB) in input

  */

#define RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT               (1ULL << 13)

/**< Out-of-place linear buffers (LB) are supported in input and output */
Andrzej Ostruszka Sept. 21, 2018, 1:18 p.m. | #2
Hello Akhil

I'm taking over handling of the patches submitted by Tomasz.

First of all, thank you for taking time to review them.

On 17.09.2018 16:07, Akhil Goyal wrote:
> Hi Tomasz,
[...]
> Please update the doc file for pmd and also set appropriate feature flag for scatter gather support.
[...]

This is actually already done in pending patch:
http://patches.dpdk.org/patch/44254/

I will send shortly updated version of this patch series though - there
were some issues with application of them and I aligned some Makefiles
with pending NETA driver patches to make them apply without issues.

Best regards
Andrzej

Patch

diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd.c b/drivers/crypto/mvsam/rte_mrvl_pmd.c
index 961802e..001aa28 100644
--- a/drivers/crypto/mvsam/rte_mrvl_pmd.c
+++ b/drivers/crypto/mvsam/rte_mrvl_pmd.c
@@ -452,8 +452,10 @@  mrvl_request_prepare(struct sam_cio_op_params *request,
 		struct rte_crypto_op *op)
 {
 	struct mrvl_crypto_session *sess;
-	struct rte_mbuf *dst_mbuf;
+	struct rte_mbuf *src_mbuf, *dst_mbuf;
+	uint16_t segments_nb;
 	uint8_t *digest;
+	int i;
 
 	if (unlikely(op->sess_type == RTE_CRYPTO_OP_SESSIONLESS)) {
 		MRVL_CRYPTO_LOG_ERR("MRVL CRYPTO PMD only supports session "
@@ -469,29 +471,47 @@  mrvl_request_prepare(struct sam_cio_op_params *request,
 		return -EINVAL;
 	}
 
-	/*
+	request->sa = sess->sam_sess;
+	request->cookie = op;
+
+	src_mbuf = op->sym->m_src;
+	segments_nb = src_mbuf->nb_segs;
+	/* The following conditions must be met:
+	 * - Destination buffer is required when segmented source buffer
+	 * - Segmented destination buffer is not supported
+	 */
+	if ((segments_nb > 1) && (!op->sym->m_dst)) {
+		MRVL_CRYPTO_LOG_ERR("op->sym->m_dst = NULL!\n");
+		return -1;
+	}
+	/* For non SG case:
 	 * If application delivered us null dst buffer, it means it expects
 	 * us to deliver the result in src buffer.
 	 */
 	dst_mbuf = op->sym->m_dst ? op->sym->m_dst : op->sym->m_src;
 
-	request->sa = sess->sam_sess;
-	request->cookie = op;
-
-	/* Single buffers only, sorry. */
-	request->num_bufs = 1;
-	request->src = src_bd;
-	src_bd->vaddr = rte_pktmbuf_mtod(op->sym->m_src, void *);
-	src_bd->paddr = rte_pktmbuf_iova(op->sym->m_src);
-	src_bd->len = rte_pktmbuf_data_len(op->sym->m_src);
-
-	/* Empty source. */
-	if (rte_pktmbuf_data_len(op->sym->m_src) == 0) {
-		/* EIP does not support 0 length buffers. */
-		MRVL_CRYPTO_LOG_ERR("Buffer length == 0 not supported!");
+	if (!rte_pktmbuf_is_contiguous(dst_mbuf)) {
+		MRVL_CRYPTO_LOG_ERR("Segmented destination buffer "
+				    "not supported.\n");
 		return -1;
 	}
 
+	request->num_bufs = segments_nb;
+	for (i = 0; i < segments_nb; i++) {
+		/* Empty source. */
+		if (rte_pktmbuf_data_len(src_mbuf) == 0) {
+			/* EIP does not support 0 length buffers. */
+			MRVL_CRYPTO_LOG_ERR("Buffer length == 0 not supported!");
+			return -1;
+		}
+		src_bd[i].vaddr = rte_pktmbuf_mtod(src_mbuf, void *);
+		src_bd[i].paddr = rte_pktmbuf_iova(src_mbuf);
+		src_bd[i].len = rte_pktmbuf_data_len(src_mbuf);
+
+		src_mbuf = src_mbuf->next;
+	}
+	request->src = src_bd;
+
 	/* Empty destination. */
 	if (rte_pktmbuf_data_len(dst_mbuf) == 0) {
 		/* Make dst buffer fit at least source data. */
@@ -542,7 +562,7 @@  mrvl_request_prepare(struct sam_cio_op_params *request,
 
 	/*
 	 * EIP supports only scenarios where ICV(digest buffer) is placed at
-	 * auth_icv_offset. Any other placement means risking errors.
+	 * auth_icv_offset.
 	 */
 	if (sess->sam_sess_params.dir == SAM_DIR_ENCRYPT) {
 		/*
@@ -551,17 +571,36 @@  mrvl_request_prepare(struct sam_cio_op_params *request,
 		 */
 		if (rte_pktmbuf_mtod_offset(
 				dst_mbuf, uint8_t *,
-				request->auth_icv_offset) == digest) {
+				request->auth_icv_offset) == digest)
 			return 0;
-		}
 	} else {/* sess->sam_sess_params.dir == SAM_DIR_DECRYPT */
 		/*
 		 * EIP will look for digest at auth_icv_offset
-		 * offset in SRC buffer.
+		 * offset in SRC buffer. It must be placed in the last
+		 * segment and the offset must be set to reach digest
+		 * in the last segment
 		 */
-		if (rte_pktmbuf_mtod_offset(
-				op->sym->m_src, uint8_t *,
-				request->auth_icv_offset) == digest) {
+		struct rte_mbuf *last_seg =  op->sym->m_src;
+		uint32_t d_offset = request->auth_icv_offset;
+		u32 d_size = sess->sam_sess_params.u.basic.auth_icv_len;
+		unsigned char *d_ptr;
+
+		/* Find the last segment and the offset for the last segment */
+		while ((last_seg->next != NULL) &&
+				(d_offset >= last_seg->data_len)) {
+			d_offset -= last_seg->data_len;
+			last_seg = last_seg->next;
+		}
+
+		if (rte_pktmbuf_mtod_offset(last_seg, uint8_t *,
+					    d_offset) == digest)
+			return 0;
+
+		/* copy digest to last segment */
+		if (last_seg->buf_len >= (d_size + d_offset)) {
+			d_ptr = (unsigned char *)last_seg->buf_addr +
+				 d_offset;
+			rte_memcpy(d_ptr, digest, d_size);
 			return 0;
 		}
 	}
@@ -597,11 +636,10 @@  mrvl_crypto_pmd_enqueue_burst(void *queue_pair, struct rte_crypto_op **ops,
 	int ret;
 	struct sam_cio_op_params requests[nb_ops];
 	/*
-	 * DPDK uses single fragment buffers, so we can KISS descriptors.
 	 * SAM does not store bd pointers, so on-stack scope will be enough.
 	 */
-	struct sam_buf_info src_bd[nb_ops];
-	struct sam_buf_info dst_bd[nb_ops];
+	struct mrvl_crypto_src_table src_bd[nb_ops];
+	struct sam_buf_info          dst_bd[nb_ops];
 	struct mrvl_crypto_qp *qp = (struct mrvl_crypto_qp *)queue_pair;
 
 	if (nb_ops == 0)
@@ -609,11 +647,14 @@  mrvl_crypto_pmd_enqueue_burst(void *queue_pair, struct rte_crypto_op **ops,
 
 	/* Prepare the burst. */
 	memset(&requests, 0, sizeof(requests));
+	memset(&src_bd, 0, sizeof(src_bd));
 
 	/* Iterate through */
 	for (; iter_ops < nb_ops; ++iter_ops) {
+		/* store the op id for debug */
+		src_bd[iter_ops].iter_ops = iter_ops;
 		if (mrvl_request_prepare(&requests[iter_ops],
-					&src_bd[iter_ops],
+					src_bd[iter_ops].src_bd,
 					&dst_bd[iter_ops],
 					ops[iter_ops]) < 0) {
 			MRVL_CRYPTO_LOG_ERR(
@@ -767,6 +808,7 @@  cryptodev_mrvl_crypto_create(const char *name,
 
 	sam_params.max_num_sessions = internals->max_nb_sessions;
 
+	/* sam_set_debug_flags(3); */
 	return sam_init(&sam_params);
 
 init_error:
diff --git a/drivers/crypto/mvsam/rte_mrvl_pmd_private.h b/drivers/crypto/mvsam/rte_mrvl_pmd_private.h
index c16d95b..0689fc3 100644
--- a/drivers/crypto/mvsam/rte_mrvl_pmd_private.h
+++ b/drivers/crypto/mvsam/rte_mrvl_pmd_private.h
@@ -38,6 +38,8 @@ 
  */
 #define BITS2BYTES(x) ((x) >> 3)
 
+#define MRVL_MAX_SEGMENTS 16
+
 /** The operation order mode enumerator. */
 enum mrvl_crypto_chain_order {
 	MRVL_CRYPTO_CHAIN_CIPHER_ONLY,
@@ -84,6 +86,11 @@  struct mrvl_crypto_session {
 	uint16_t cipher_iv_offset;
 } __rte_cache_aligned;
 
+struct mrvl_crypto_src_table {
+	uint16_t iter_ops;
+	struct sam_buf_info src_bd[MRVL_MAX_SEGMENTS];
+} __rte_cache_aligned;
+
 /** Set and validate MRVL crypto session parameters */
 extern int
 mrvl_crypto_set_session_parameters(struct mrvl_crypto_session *sess,