Re: [dpdk-dev] [PATCH 1/3] crypto/mvsam: add S/G support to crypto dirver

[Akhil Goyal <akhil.goyal@nxp.com>]

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 */