From: Anoob Joseph <anoobj@marvell.com>
To: Akhil Goyal <akhil.goyal@nxp.com>, "dev@dpdk.org" <dev@dpdk.org>
Cc: "hemant.agrawal@nxp.com" <hemant.agrawal@nxp.com>,
"Jerin Jacob Kollanukkaran" <jerinj@marvell.com>,
"declan.doherty@intel.com" <declan.doherty@intel.com>,
"konstantin.ananyev@intel.com" <konstantin.ananyev@intel.com>,
Narayana Prasad Raju Athreya <pathreya@marvell.com>
Subject: Re: [dpdk-dev] [EXT] [PATCH] crypto/openssl: support SG for inplace buffers
Date: Fri, 13 Sep 2019 05:58:38 +0000 [thread overview]
Message-ID: <BN8PR18MB2868580967166945A272E9E2DFB30@BN8PR18MB2868.namprd18.prod.outlook.com> (raw)
In-Reply-To: <20190902124234.2314-1-akhil.goyal@nxp.com>
Hi Akhil,
Please see inline.
Thanks,
Anoob
> -----Original Message-----
> From: Akhil Goyal <akhil.goyal@nxp.com>
> Sent: Monday, September 2, 2019 6:13 PM
> To: dev@dpdk.org
> Cc: hemant.agrawal@nxp.com; Anoob Joseph <anoobj@marvell.com>; Jerin
> Jacob Kollanukkaran <jerinj@marvell.com>; declan.doherty@intel.com;
> konstantin.ananyev@intel.com; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [EXT] [PATCH] crypto/openssl: support SG for inplace buffers
>
> External Email
>
> ----------------------------------------------------------------------
> As per current support, scatter Gather is only supported for out of place input
[Anoob] s/scatter/Scatter
> and output buffers.
> This patch add support for scatter gather for inplace buffers.
>
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> ---
> doc/guides/cryptodevs/features/openssl.ini | 1 +
> drivers/crypto/openssl/rte_openssl_pmd.c | 82 +++++++++++++++++-----
> 2 files changed, 64 insertions(+), 19 deletions(-)
>
> diff --git a/doc/guides/cryptodevs/features/openssl.ini
> b/doc/guides/cryptodevs/features/openssl.ini
> index 6ddca39e7..30ffb111d 100644
> --- a/doc/guides/cryptodevs/features/openssl.ini
> +++ b/doc/guides/cryptodevs/features/openssl.ini
> @@ -6,6 +6,7 @@
> [Features]
> Symmetric crypto = Y
> Sym operation chaining = Y
> +In Place SGL = Y
> OOP SGL In LB Out = Y
> OOP LB In LB Out = Y
> Asymmetric crypto = Y
> diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c
> b/drivers/crypto/openssl/rte_openssl_pmd.c
> index 2f5552840..304ebc6ff 100644
> --- a/drivers/crypto/openssl/rte_openssl_pmd.c
> +++ b/drivers/crypto/openssl/rte_openssl_pmd.c
> @@ -798,12 +798,12 @@ get_session(struct openssl_qp *qp, struct
> rte_crypto_op *op)
> */
> static inline int
> process_openssl_encryption_update(struct rte_mbuf *mbuf_src, int offset,
> - uint8_t **dst, int srclen, EVP_CIPHER_CTX *ctx)
> + uint8_t **dst, int srclen, EVP_CIPHER_CTX *ctx, uint8_t inplace)
> {
> struct rte_mbuf *m;
> int dstlen;
> int l, n = srclen;
> - uint8_t *src;
> + uint8_t *src, temp[128];
[Anoob] What is 128? Wouldn't a macro be better here?
>
> for (m = mbuf_src; m != NULL && offset > rte_pktmbuf_data_len(m);
> m = m->next)
> @@ -813,6 +813,8 @@ process_openssl_encryption_update(struct rte_mbuf
> *mbuf_src, int offset,
> return -1;
>
> src = rte_pktmbuf_mtod_offset(m, uint8_t *, offset);
> + if (inplace)
> + *dst = src;
>
> l = rte_pktmbuf_data_len(m) - offset;
> if (srclen <= l) {
> @@ -829,8 +831,24 @@ process_openssl_encryption_update(struct rte_mbuf
> *mbuf_src, int offset,
> n -= l;
>
> for (m = m->next; (m != NULL) && (n > 0); m = m->next) {
> + uint8_t diff = l - dstlen, rem;
> +
> src = rte_pktmbuf_mtod(m, uint8_t *);
> l = rte_pktmbuf_data_len(m) < n ? rte_pktmbuf_data_len(m) : n;
[Anoob] Can you explain the logic of below lines? I was trying to understand the two 'rte_memcpy's and how dst is being used there.
> + if (diff && inplace) {
> + rem = l > (EVP_CIPHER_CTX_block_size(ctx) - diff) ?
> + (EVP_CIPHER_CTX_block_size(ctx) - diff) : l;
[Anoob] Can't we use RTE_MIN here?
> + if (EVP_EncryptUpdate(ctx, temp,
> + &dstlen, src, rem) <= 0)
> + return -1;
> + n -= rem;
> + rte_memcpy(*dst, temp, diff);
> + rte_memcpy(src, temp + diff, rem);
> + src += rem;
> + l -= rem;
> + }
> + if (inplace)
> + *dst = src;
> if (EVP_EncryptUpdate(ctx, *dst, &dstlen, src, l) <= 0)
> return -1;
> *dst += dstlen;
> @@ -842,12 +860,12 @@ process_openssl_encryption_update(struct rte_mbuf
> *mbuf_src, int offset,
>
> static inline int
> process_openssl_decryption_update(struct rte_mbuf *mbuf_src, int offset,
> - uint8_t **dst, int srclen, EVP_CIPHER_CTX *ctx)
> + uint8_t **dst, int srclen, EVP_CIPHER_CTX *ctx, uint8_t inplace)
> {
> struct rte_mbuf *m;
> int dstlen;
> int l, n = srclen;
> - uint8_t *src;
> + uint8_t *src, temp[128];
>
> for (m = mbuf_src; m != NULL && offset > rte_pktmbuf_data_len(m);
> m = m->next)
> @@ -857,6 +875,8 @@ process_openssl_decryption_update(struct rte_mbuf
> *mbuf_src, int offset,
> return -1;
>
> src = rte_pktmbuf_mtod_offset(m, uint8_t *, offset);
> + if (inplace)
> + *dst = src;
>
> l = rte_pktmbuf_data_len(m) - offset;
> if (srclen <= l) {
> @@ -873,8 +893,24 @@ process_openssl_decryption_update(struct rte_mbuf
> *mbuf_src, int offset,
> n -= l;
>
> for (m = m->next; (m != NULL) && (n > 0); m = m->next) {
> + uint8_t diff = l - dstlen, rem;
> +
> src = rte_pktmbuf_mtod(m, uint8_t *);
> l = rte_pktmbuf_data_len(m) < n ? rte_pktmbuf_data_len(m) : n;
> + if (diff && inplace) {
> + rem = l > (EVP_CIPHER_CTX_block_size(ctx) - diff) ?
> + (EVP_CIPHER_CTX_block_size(ctx) - diff) : l;
> + if (EVP_EncryptUpdate(ctx, temp,
> + &dstlen, src, rem) <= 0)
[Anoob] Shouldn't this be EVP_DecryptUpdate()?
> + return -1;
> + n -= rem;
> + rte_memcpy(*dst, temp, diff);
> + rte_memcpy(src, temp + diff, rem);
> + src += rem;
> + l -= rem;
> + }
> + if (inplace)
> + *dst = src;
> if (EVP_DecryptUpdate(ctx, *dst, &dstlen, src, l) <= 0)
> return -1;
> *dst += dstlen;
> @@ -887,7 +923,8 @@ process_openssl_decryption_update(struct rte_mbuf
> *mbuf_src, int offset,
> /** Process standard openssl cipher encryption */ static int
> process_openssl_cipher_encrypt(struct rte_mbuf *mbuf_src, uint8_t *dst,
> - int offset, uint8_t *iv, int srclen, EVP_CIPHER_CTX *ctx)
> + int offset, uint8_t *iv, int srclen, EVP_CIPHER_CTX *ctx,
> + uint8_t inplace)
> {
> int totlen;
>
> @@ -897,7 +934,7 @@ process_openssl_cipher_encrypt(struct rte_mbuf
> *mbuf_src, uint8_t *dst,
> EVP_CIPHER_CTX_set_padding(ctx, 0);
>
> if (process_openssl_encryption_update(mbuf_src, offset, &dst,
> - srclen, ctx))
> + srclen, ctx, inplace))
> goto process_cipher_encrypt_err;
>
> if (EVP_EncryptFinal_ex(ctx, dst, &totlen) <= 0) @@ -936,7 +973,8 @@
> process_openssl_cipher_bpi_encrypt(uint8_t *src, uint8_t *dst,
> /** Process standard openssl cipher decryption */ static int
> process_openssl_cipher_decrypt(struct rte_mbuf *mbuf_src, uint8_t *dst,
> - int offset, uint8_t *iv, int srclen, EVP_CIPHER_CTX *ctx)
> + int offset, uint8_t *iv, int srclen, EVP_CIPHER_CTX *ctx,
> + uint8_t inplace)
> {
> int totlen;
>
> @@ -946,7 +984,7 @@ process_openssl_cipher_decrypt(struct rte_mbuf
> *mbuf_src, uint8_t *dst,
> EVP_CIPHER_CTX_set_padding(ctx, 0);
>
> if (process_openssl_decryption_update(mbuf_src, offset, &dst,
> - srclen, ctx))
> + srclen, ctx, inplace))
> goto process_cipher_decrypt_err;
>
> if (EVP_DecryptFinal_ex(ctx, dst, &totlen) <= 0) @@ -1033,7 +1071,7
> @@ process_openssl_auth_encryption_gcm(struct rte_mbuf *mbuf_src, int
> offset,
>
> if (srclen > 0)
> if (process_openssl_encryption_update(mbuf_src, offset, &dst,
> - srclen, ctx))
> + srclen, ctx, 0))
> goto process_auth_encryption_gcm_err;
>
> /* Workaround open ssl bug in version less then 1.0.1f */ @@ -1078,7
> +1116,7 @@ process_openssl_auth_encryption_ccm(struct rte_mbuf
> *mbuf_src, int offset,
>
> if (srclen > 0)
> if (process_openssl_encryption_update(mbuf_src, offset, &dst,
> - srclen, ctx))
> + srclen, ctx, 0))
> goto process_auth_encryption_ccm_err;
>
> if (EVP_EncryptFinal_ex(ctx, dst, &len) <= 0) @@ -1115,7 +1153,7 @@
> process_openssl_auth_decryption_gcm(struct rte_mbuf *mbuf_src, int offset,
>
> if (srclen > 0)
> if (process_openssl_decryption_update(mbuf_src, offset, &dst,
> - srclen, ctx))
> + srclen, ctx, 0))
> goto process_auth_decryption_gcm_err;
>
> /* Workaround open ssl bug in version less then 1.0.1f */ @@ -1161,7
> +1199,7 @@ process_openssl_auth_decryption_ccm(struct rte_mbuf
> *mbuf_src, int offset,
>
> if (srclen > 0)
> if (process_openssl_decryption_update(mbuf_src, offset, &dst,
> - srclen, ctx))
> + srclen, ctx, 0))
> return -EFAULT;
>
> return 0;
> @@ -1372,12 +1410,15 @@ process_openssl_cipher_op {
> uint8_t *dst, *iv;
> int srclen, status;
> + uint8_t inplace = (mbuf_src == mbuf_dst) ? 1 : 0;
>
> /*
> - * Segmented destination buffer is not supported for
> - * encryption/decryption
> + * Segmented OOP destination buffer is not supported for encryption/
> + * decryption. In case of des3ctr, even inplace segmented buffers are
> + * not supported.
> */
> - if (!rte_pktmbuf_is_contiguous(mbuf_dst)) {
> + if (!rte_pktmbuf_is_contiguous(mbuf_dst) &&
> + (!inplace || sess->cipher.mode !=
> OPENSSL_CIPHER_LIB)) {
> op->status = RTE_CRYPTO_OP_STATUS_ERROR;
> return;
> }
> @@ -1393,11 +1434,13 @@ process_openssl_cipher_op
> if (sess->cipher.direction ==
> RTE_CRYPTO_CIPHER_OP_ENCRYPT)
> status = process_openssl_cipher_encrypt(mbuf_src, dst,
> op->sym->cipher.data.offset, iv,
> - srclen, sess->cipher.ctx);
> + srclen, sess->cipher.ctx,
> + inplace);
> else
> status = process_openssl_cipher_decrypt(mbuf_src, dst,
> op->sym->cipher.data.offset, iv,
> - srclen, sess->cipher.ctx);
> + srclen, sess->cipher.ctx,
> + inplace);
> else
> status = process_openssl_cipher_des3ctr(mbuf_src, dst,
> op->sym->cipher.data.offset, iv,
> @@ -1441,7 +1484,7 @@ process_openssl_docsis_bpi_op(struct rte_crypto_op
> *op,
> /* Encrypt with the block aligned stream with CBC
> mode */
> status = process_openssl_cipher_encrypt(mbuf_src, dst,
> op->sym->cipher.data.offset, iv,
> - srclen, sess->cipher.ctx);
> + srclen, sess->cipher.ctx, 0);
> if (last_block_len) {
> /* Point at last block */
> dst += srclen;
> @@ -1491,7 +1534,7 @@ process_openssl_docsis_bpi_op(struct rte_crypto_op
> *op,
> /* Decrypt with CBC mode */
> status |= process_openssl_cipher_decrypt(mbuf_src,
> dst,
> op->sym->cipher.data.offset, iv,
> - srclen, sess->cipher.ctx);
> + srclen, sess->cipher.ctx, 0);
> }
> }
>
> @@ -2121,6 +2164,7 @@ cryptodev_openssl_create(const char *name,
> dev->feature_flags = RTE_CRYPTODEV_FF_SYMMETRIC_CRYPTO |
> RTE_CRYPTODEV_FF_SYM_OPERATION_CHAINING |
> RTE_CRYPTODEV_FF_CPU_AESNI |
> + RTE_CRYPTODEV_FF_IN_PLACE_SGL |
> RTE_CRYPTODEV_FF_OOP_SGL_IN_LB_OUT |
> RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT |
> RTE_CRYPTODEV_FF_ASYMMETRIC_CRYPTO |
> --
> 2.17.1
next prev parent reply other threads:[~2019-09-13 5:58 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-09-02 12:42 [dpdk-dev] " Akhil Goyal
2019-09-13 5:58 ` Anoob Joseph [this message]
2019-09-19 15:57 ` [dpdk-dev] [EXT] " Akhil Goyal
2019-11-18 13:36 ` [dpdk-dev] [PATCH v2] " Akhil Goyal
2019-11-19 14:19 ` Akhil Goyal
2019-11-20 4:31 ` [dpdk-dev] [EXT] " Anoob Joseph
2019-11-20 6:11 ` Akhil Goyal
2019-11-20 5:58 ` [dpdk-dev] [PATCH v3] " Akhil Goyal
2019-11-20 6:23 ` Anoob Joseph
2019-11-20 11:50 ` Akhil Goyal
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=BN8PR18MB2868580967166945A272E9E2DFB30@BN8PR18MB2868.namprd18.prod.outlook.com \
--to=anoobj@marvell.com \
--cc=akhil.goyal@nxp.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
--cc=jerinj@marvell.com \
--cc=konstantin.ananyev@intel.com \
--cc=pathreya@marvell.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).