[dpdk-dev] crypto/openssl: performance improvements
Checks
Commit Message
key and algo are added in the openssl ctx during
session initialization instead of adding it for
each packet.
Also in case of HMAC the openssl APIs HMAC_XXX give
better performance for all HMAC cases.
Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
---
Tested on arm and xeon machines
autotest passed with no issues,
perftest passed with improved performance numbers,
l2fwd-crypto passed with improved performance
drivers/crypto/openssl/rte_openssl_pmd.c | 95 +++++++++++++++---------
drivers/crypto/openssl/rte_openssl_pmd_private.h | 3 +-
2 files changed, 63 insertions(+), 35 deletions(-)
Comments
Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Friday, July 28, 2017 12:08 PM
> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> hemant.agrawal@nxp.com; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [PATCH] crypto/openssl: performance improvements
>
> key and algo are added in the openssl ctx during session initialization
> instead of adding it for each packet.
>
> Also in case of HMAC the openssl APIs HMAC_XXX give better performance
> for all HMAC cases.
>
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
I assume that this and the other patches are meant to be for 17.11, right?
Thanks,
Pablo
On 7/28/2017 5:28 PM, De Lara Guarch, Pablo wrote:
> Hi Akhil,
>
>> -----Original Message-----
>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>> Sent: Friday, July 28, 2017 12:08 PM
>> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
>> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> hemant.agrawal@nxp.com; Akhil Goyal <akhil.goyal@nxp.com>
>> Subject: [PATCH] crypto/openssl: performance improvements
>>
>> key and algo are added in the openssl ctx during session initialization
>> instead of adding it for each packet.
>>
>> Also in case of HMAC the openssl APIs HMAC_XXX give better performance
>> for all HMAC cases.
>>
>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
>
> I assume that this and the other patches are meant to be for 17.11, right?
>
yes, they are for 17.11.
But, I don't mind if these can be considered for 17.08.
-Akhil
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Akhil Goyal
> Sent: Friday, July 28, 2017 1:03 PM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
> Cc: hemant.agrawal@nxp.com
> Subject: Re: [dpdk-dev] [PATCH] crypto/openssl: performance
> improvements
>
> On 7/28/2017 5:28 PM, De Lara Guarch, Pablo wrote:
> > Hi Akhil,
> >
> >> -----Original Message-----
> >> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> >> Sent: Friday, July 28, 2017 12:08 PM
> >> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
> >> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> >> hemant.agrawal@nxp.com; Akhil Goyal <akhil.goyal@nxp.com>
> >> Subject: [PATCH] crypto/openssl: performance improvements
> >>
> >> key and algo are added in the openssl ctx during session
> >> initialization instead of adding it for each packet.
> >>
> >> Also in case of HMAC the openssl APIs HMAC_XXX give better
> >> performance for all HMAC cases.
> >>
> >> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> >
> > I assume that this and the other patches are meant to be for 17.11, right?
> >
> yes, they are for 17.11.
> But, I don't mind if these can be considered for 17.08.
Unfortunately, at this stage, it is too late to include these sorts of patches in this release.
Just wanted to double check with you.
Thanks,
Pablo
>
> -Akhil
>
>
Hi Akhil,
> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Friday, July 28, 2017 12:08 PM
> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> hemant.agrawal@nxp.com; Akhil Goyal <akhil.goyal@nxp.com>
> Subject: [PATCH] crypto/openssl: performance improvements
>
> key and algo are added in the openssl ctx during session initialization
> instead of adding it for each packet.
>
> Also in case of HMAC the openssl APIs HMAC_XXX give better performance
> for all HMAC cases.
>
> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
Thanks for the patch, nice optimization!
Could you split this into two patches, as you are doing two different things here?
One for the first sentence and another one for the second sentence.
Also, as you do that, could you rename the title to be more explicit?
Like: crypto/openssl: initialize cipher key at session init
Finally, I was looking at GCM, and I think it could benefit from this.
I will send a separate patch for it, unless you want to integrate it in this patchset yourself.
Thanks,
Pablo
On 8/14/2017 7:47 PM, De Lara Guarch, Pablo wrote:
> Hi Akhil,
>
>> -----Original Message-----
>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>> Sent: Friday, July 28, 2017 12:08 PM
>> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
>> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> hemant.agrawal@nxp.com; Akhil Goyal <akhil.goyal@nxp.com>
>> Subject: [PATCH] crypto/openssl: performance improvements
>>
>> key and algo are added in the openssl ctx during session initialization
>> instead of adding it for each packet.
>>
>> Also in case of HMAC the openssl APIs HMAC_XXX give better performance
>> for all HMAC cases.
>>
>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
>
> Thanks for the patch, nice optimization!
> Could you split this into two patches, as you are doing two different things here?
> One for the first sentence and another one for the second sentence.
> Also, as you do that, could you rename the title to be more explicit?
> Like: crypto/openssl: initialize cipher key at session init
>
> Finally, I was looking at GCM, and I think it could benefit from this.
> I will send a separate patch for it, unless you want to integrate it in this patchset yourself.
>
Ok I would split the patches.
For GCM I will try to incorporate in this patchset, if I get some
performance improvement, or I would send a different patch later if some
issue comes.
Thanks,
Akhil
Hi,
> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Tuesday, August 15, 2017 7:45 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
> Cc: hemant.agrawal@nxp.com
> Subject: Re: [PATCH] crypto/openssl: performance improvements
>
> On 8/14/2017 7:47 PM, De Lara Guarch, Pablo wrote:
> > Hi Akhil,
> >
> >> -----Original Message-----
> >> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> >> Sent: Friday, July 28, 2017 12:08 PM
> >> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
> >> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> >> hemant.agrawal@nxp.com; Akhil Goyal <akhil.goyal@nxp.com>
> >> Subject: [PATCH] crypto/openssl: performance improvements
> >>
> >> key and algo are added in the openssl ctx during session
> >> initialization instead of adding it for each packet.
> >>
> >> Also in case of HMAC the openssl APIs HMAC_XXX give better
> >> performance for all HMAC cases.
> >>
> >> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
> >
> > Thanks for the patch, nice optimization!
> > Could you split this into two patches, as you are doing two different
> things here?
> > One for the first sentence and another one for the second sentence.
> > Also, as you do that, could you rename the title to be more explicit?
> > Like: crypto/openssl: initialize cipher key at session init
> >
> > Finally, I was looking at GCM, and I think it could benefit from this.
> > I will send a separate patch for it, unless you want to integrate it in this
> patchset yourself.
> >
>
> Ok I would split the patches.
> For GCM I will try to incorporate in this patchset, if I get some performance
> improvement, or I would send a different patch later if some issue comes.
Thanks Ahkil. Since I am working on AES-CCM for this PMD, I have the change
already done. I have seen performance improvements, but it is not as straight forward
as the cipher algorithms, because GMAC is also affected, which is in a different code path,
but requires GCM to be set.
>
> Thanks,
> Akhil
>
Hi Pablo,
On 8/15/2017 12:56 PM, De Lara Guarch, Pablo wrote:
> Hi,
>
>> -----Original Message-----
>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>> Sent: Tuesday, August 15, 2017 7:45 AM
>> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>> dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
>> Cc: hemant.agrawal@nxp.com
>> Subject: Re: [PATCH] crypto/openssl: performance improvements
>>
>> On 8/14/2017 7:47 PM, De Lara Guarch, Pablo wrote:
>>> Hi Akhil,
>>>
>>>> -----Original Message-----
>>>> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
>>>> Sent: Friday, July 28, 2017 12:08 PM
>>>> To: dev@dpdk.org; Doherty, Declan <declan.doherty@intel.com>
>>>> Cc: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
>>>> hemant.agrawal@nxp.com; Akhil Goyal <akhil.goyal@nxp.com>
>>>> Subject: [PATCH] crypto/openssl: performance improvements
>>>>
>>>> key and algo are added in the openssl ctx during session
>>>> initialization instead of adding it for each packet.
>>>>
>>>> Also in case of HMAC the openssl APIs HMAC_XXX give better
>>>> performance for all HMAC cases.
>>>>
>>>> Signed-off-by: Akhil Goyal <akhil.goyal@nxp.com>
>>>
>>> Thanks for the patch, nice optimization!
>>> Could you split this into two patches, as you are doing two different
>> things here?
>>> One for the first sentence and another one for the second sentence.
>>> Also, as you do that, could you rename the title to be more explicit?
>>> Like: crypto/openssl: initialize cipher key at session init
>>>
>>> Finally, I was looking at GCM, and I think it could benefit from this.
>>> I will send a separate patch for it, unless you want to integrate it in this
>> patchset yourself.
>>>
>>
>> Ok I would split the patches.
>> For GCM I will try to incorporate in this patchset, if I get some performance
>> improvement, or I would send a different patch later if some issue comes.
>
> Thanks Ahkil. Since I am working on AES-CCM for this PMD, I have the change
> already done. I have seen performance improvements, but it is not as straight forward
> as the cipher algorithms, because GMAC is also affected, which is in a different code path,
> but requires GCM to be set.
>
If you have the change and it is working fine, then you can send your
patch, no issues in that.
Thanks,
Akhil
@@ -39,6 +39,7 @@
#include <rte_malloc.h>
#include <rte_cpuflags.h>
+#include <openssl/hmac.h>
#include <openssl/evp.h>
#include "rte_openssl_pmd_private.h"
@@ -307,6 +308,22 @@ openssl_set_session_cipher_parameters(struct openssl_session *sess,
get_cipher_key(xform->cipher.key.data, sess->cipher.key.length,
sess->cipher.key.data);
+ if (sess->cipher.direction == RTE_CRYPTO_CIPHER_OP_ENCRYPT) {
+ if (EVP_EncryptInit_ex(sess->cipher.ctx,
+ sess->cipher.evp_algo,
+ NULL, xform->cipher.key.data,
+ NULL) != 1) {
+ return -EINVAL;
+ }
+ } else if (sess->cipher.direction ==
+ RTE_CRYPTO_CIPHER_OP_DECRYPT) {
+ if (EVP_DecryptInit_ex(sess->cipher.ctx,
+ sess->cipher.evp_algo,
+ NULL, xform->cipher.key.data,
+ NULL) != 1) {
+ return -EINVAL;
+ }
+ }
break;
@@ -333,6 +350,23 @@ openssl_set_session_cipher_parameters(struct openssl_session *sess,
get_cipher_key(xform->cipher.key.data, sess->cipher.key.length,
sess->cipher.key.data);
+ if (sess->cipher.direction == RTE_CRYPTO_CIPHER_OP_ENCRYPT) {
+ if (EVP_EncryptInit_ex(sess->cipher.ctx,
+ sess->cipher.evp_algo,
+ NULL, xform->cipher.key.data,
+ NULL) != 1) {
+ return -EINVAL;
+ }
+ } else if (sess->cipher.direction ==
+ RTE_CRYPTO_CIPHER_OP_DECRYPT) {
+ if (EVP_DecryptInit_ex(sess->cipher.ctx,
+ sess->cipher.evp_algo,
+ NULL, xform->cipher.key.data,
+ NULL) != 1) {
+ return -EINVAL;
+ }
+ }
+
break;
default:
sess->cipher.algo = RTE_CRYPTO_CIPHER_NULL;
@@ -403,12 +437,16 @@ openssl_set_session_auth_parameters(struct openssl_session *sess,
case RTE_CRYPTO_AUTH_SHA384_HMAC:
case RTE_CRYPTO_AUTH_SHA512_HMAC:
sess->auth.mode = OPENSSL_AUTH_AS_HMAC;
- sess->auth.hmac.ctx = EVP_MD_CTX_create();
+ HMAC_CTX_init(&sess->auth.hmac.ctx);
if (get_auth_algo(xform->auth.algo,
&sess->auth.hmac.evp_algo) != 0)
return -EINVAL;
- sess->auth.hmac.pkey = EVP_PKEY_new_mac_key(EVP_PKEY_HMAC, NULL,
- xform->auth.key.data, xform->auth.key.length);
+
+ if (HMAC_Init_ex(&sess->auth.hmac.ctx,
+ xform->auth.key.data,
+ xform->auth.key.length,
+ sess->auth.hmac.evp_algo, NULL) != 1)
+ return -EINVAL;
break;
default:
@@ -547,7 +585,7 @@ openssl_reset_session(struct openssl_session *sess)
break;
case OPENSSL_AUTH_AS_HMAC:
EVP_PKEY_free(sess->auth.hmac.pkey);
- EVP_MD_CTX_destroy(sess->auth.hmac.ctx);
+ HMAC_CTX_cleanup(&sess->auth.hmac.ctx);
break;
default:
break;
@@ -693,12 +731,11 @@ 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, uint8_t *key, int srclen,
- EVP_CIPHER_CTX *ctx, const EVP_CIPHER *algo)
+ int offset, uint8_t *iv, int srclen, EVP_CIPHER_CTX *ctx)
{
int totlen;
- if (EVP_EncryptInit_ex(ctx, algo, NULL, key, iv) <= 0)
+ if (EVP_EncryptInit_ex(ctx, NULL, NULL, NULL, iv) <= 0)
goto process_cipher_encrypt_err;
EVP_CIPHER_CTX_set_padding(ctx, 0);
@@ -743,12 +780,11 @@ 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, uint8_t *key, int srclen,
- EVP_CIPHER_CTX *ctx, const EVP_CIPHER *algo)
+ int offset, uint8_t *iv, int srclen, EVP_CIPHER_CTX *ctx)
{
int totlen;
- if (EVP_DecryptInit_ex(ctx, algo, NULL, key, iv) <= 0)
+ if (EVP_DecryptInit_ex(ctx, NULL, NULL, NULL, iv) <= 0)
goto process_cipher_decrypt_err;
EVP_CIPHER_CTX_set_padding(ctx, 0);
@@ -971,10 +1007,9 @@ process_openssl_auth(struct rte_mbuf *mbuf_src, uint8_t *dst, int offset,
/** Process standard openssl auth algorithms with hmac */
static int
process_openssl_auth_hmac(struct rte_mbuf *mbuf_src, uint8_t *dst, int offset,
- __rte_unused uint8_t *iv, EVP_PKEY *pkey,
- int srclen, EVP_MD_CTX *ctx, const EVP_MD *algo)
+ int srclen, HMAC_CTX *ctx)
{
- size_t dstlen;
+ unsigned int dstlen;
struct rte_mbuf *m;
int l, n = srclen;
uint8_t *src;
@@ -986,19 +1021,16 @@ process_openssl_auth_hmac(struct rte_mbuf *mbuf_src, uint8_t *dst, int offset,
if (m == 0)
goto process_auth_err;
- if (EVP_DigestSignInit(ctx, NULL, algo, NULL, pkey) <= 0)
- goto process_auth_err;
-
src = rte_pktmbuf_mtod_offset(m, uint8_t *, offset);
l = rte_pktmbuf_data_len(m) - offset;
if (srclen <= l) {
- if (EVP_DigestSignUpdate(ctx, (char *)src, srclen) <= 0)
+ if (HMAC_Update(ctx, (unsigned char *)src, srclen) != 1)
goto process_auth_err;
goto process_auth_final;
}
- if (EVP_DigestSignUpdate(ctx, (char *)src, l) <= 0)
+ if (HMAC_Update(ctx, (unsigned char *)src, l) != 1)
goto process_auth_err;
n -= l;
@@ -1006,13 +1038,16 @@ process_openssl_auth_hmac(struct rte_mbuf *mbuf_src, uint8_t *dst, int offset,
for (m = m->next; (m != NULL) && (n > 0); m = m->next) {
src = rte_pktmbuf_mtod(m, uint8_t *);
l = rte_pktmbuf_data_len(m) < n ? rte_pktmbuf_data_len(m) : n;
- if (EVP_DigestSignUpdate(ctx, (char *)src, l) <= 0)
+ if (HMAC_Update(ctx, (unsigned char *)src, l) != 1)
goto process_auth_err;
n -= l;
}
process_auth_final:
- if (EVP_DigestSignFinal(ctx, dst, &dstlen) <= 0)
+ if (HMAC_Final(ctx, dst, &dstlen) != 1)
+ goto process_auth_err;
+
+ if (unlikely(HMAC_Init_ex(ctx, NULL, 0, NULL, NULL) != 1))
goto process_auth_err;
return 0;
@@ -1122,15 +1157,11 @@ 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,
- sess->cipher.key.data, srclen,
- sess->cipher.ctx,
- sess->cipher.evp_algo);
+ srclen, sess->cipher.ctx);
else
status = process_openssl_cipher_decrypt(mbuf_src, dst,
op->sym->cipher.data.offset, iv,
- sess->cipher.key.data, srclen,
- sess->cipher.ctx,
- sess->cipher.evp_algo);
+ srclen, sess->cipher.ctx);
else
status = process_openssl_cipher_des3ctr(mbuf_src, dst,
op->sym->cipher.data.offset, iv,
@@ -1174,8 +1205,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,
- sess->cipher.key.data, srclen,
- sess->cipher.ctx, sess->cipher.evp_algo);
+ srclen, sess->cipher.ctx);
if (last_block_len) {
/* Point at last block */
dst += srclen;
@@ -1225,9 +1255,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,
- sess->cipher.key.data, srclen,
- sess->cipher.ctx,
- sess->cipher.evp_algo);
+ srclen, sess->cipher.ctx);
}
}
@@ -1265,9 +1293,8 @@ process_openssl_auth_op
break;
case OPENSSL_AUTH_AS_HMAC:
status = process_openssl_auth_hmac(mbuf_src, dst,
- op->sym->auth.data.offset, NULL,
- sess->auth.hmac.pkey, srclen,
- sess->auth.hmac.ctx, sess->auth.hmac.evp_algo);
+ op->sym->auth.data.offset, srclen,
+ &sess->auth.hmac.ctx);
break;
default:
status = -1;
@@ -34,6 +34,7 @@
#define _OPENSSL_PMD_PRIVATE_H_
#include <openssl/evp.h>
+#include <openssl/hmac.h>
#include <openssl/des.h>
#define CRYPTODEV_NAME_OPENSSL_PMD crypto_openssl
@@ -164,7 +165,7 @@ struct openssl_session {
/**< pointer to EVP key */
const EVP_MD *evp_algo;
/**< pointer to EVP algorithm function */
- EVP_MD_CTX *ctx;
+ HMAC_CTX ctx;
/**< pointer to EVP context structure */
} hmac;
};