Re: patch 'crypto/openssl: set cipher padding once' has been queued to stable release 21.11.8

[Kevin Traynor <ktraynor@redhat.com>]

On 23/08/2024 17:18, Kevin Traynor wrote:
> Hi,
> 
> FYI, your patch has been queued to stable release 21.11.8
> 
> Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet.
> It will be pushed if I get no objections before 08/28/24. So please
> shout if anyone has objections.
> 
> Also note that after the patch there's a diff of the upstream commit vs the
> patch applied to the branch. This will indicate if there was any rebasing
> needed to apply to the stable branch. If there were code changes for rebasing
> (ie: not only metadata diffs), please double check that the rebase was
> correctly done.
> 
> Queued patches are on a temporary branch at:
> https://github.com/kevintraynor/dpdk-stable
> 
> This queued commit can be viewed at:
> https://github.com/kevintraynor/dpdk-stable/commit/d6048dac1e75d310ad2dc6b89278567c8c925b56
> 
> Thanks.
> 
> Kevin
> 

Hi, this patch was causing unit test failures in the CI.

CI run without patch applied:
https://dpdkdashboard.iol.unh.edu/results/dashboard/tarballs/30439/

CI run with this patch applied. See extra unit test fails.

1/2 DPDK:driver-tests / cryptodev_openssl_autotest  FAIL     0.84 s
(killed by signal 11 SIGSEGV)

https://dpdkdashboard.iol.unh.edu/results/dashboard/tarballs/30433/
You can get full logs from the relevant platforms.

I've removed it from 21.11.8. I can re-add for 21.11.9 if it is rebased
and passing CI.

thanks,
Kevin.

> ---
> From d6048dac1e75d310ad2dc6b89278567c8c925b56 Mon Sep 17 00:00:00 2001
> From: Jack Bond-Preston <jack.bond-preston@foss.arm.com>
> Date: Wed, 3 Jul 2024 13:45:51 +0000
> Subject: [PATCH] crypto/openssl: set cipher padding once
> 
> [ upstream commit d2bf59017315dc18eb6c9f2d7acd10dfb8d7758e ]
> 
> Setting the cipher padding has a noticeable performance footprint,
> and it doesn't need to be done for every call to
> process_openssl_cipher_{en,de}crypt(). Setting it causes OpenSSL to set
> it on every future context re-init. Thus, for every buffer after the
> first one, the padding is being set twice.
> 
> Instead, just set the cipher padding once - when configuring the session
> parameters - avoiding the unnecessary double setting behaviour. This is
> skipped for AEAD ciphers, where disabling padding is not necessary.
> 
> Throughput performance uplift measurements for AES-CBC-128 encrypt on
> Ampere Altra Max platform:
> 1 worker lcore
> |   buffer sz (B) |   prev (Gbps) |   optimised (Gbps) |   uplift |
> |-----------------+---------------+--------------------+----------|
> |              64 |          2.97 |               3.72 |    25.2% |
> |             256 |          8.10 |               9.42 |    16.3% |
> |            1024 |         14.22 |              15.18 |     6.8% |
> |            2048 |         16.28 |              16.93 |     4.0% |
> |            4096 |         17.58 |              17.97 |     2.2% |
> 
> 8 worker lcores
> |   buffer sz (B) |   prev (Gbps) |   optimised (Gbps) |   uplift |
> |-----------------+---------------+--------------------+----------|
> |              64 |         21.27 |              29.85 |    40.3% |
> |             256 |         60.05 |              75.53 |    25.8% |
> |            1024 |        110.11 |             121.56 |    10.4% |
> |            2048 |        128.05 |             135.40 |     5.7% |
> |            4096 |        139.45 |             143.76 |     3.1% |
> 
> Signed-off-by: Jack Bond-Preston <jack.bond-preston@foss.arm.com>
> Acked-by: Kai Ji <kai.ji@intel.com>
> Reviewed-by: Wathsala Vithanage <wathsala.vithanage@arm.com>
> ---
>  drivers/crypto/openssl/rte_openssl_pmd.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/crypto/openssl/rte_openssl_pmd.c b/drivers/crypto/openssl/rte_openssl_pmd.c
> index a321258980..b82f6939a5 100644
> --- a/drivers/crypto/openssl/rte_openssl_pmd.c
> +++ b/drivers/crypto/openssl/rte_openssl_pmd.c
> @@ -615,4 +615,6 @@ openssl_set_session_auth_parameters(struct openssl_session *sess,
>  	}
>  
> +	EVP_CIPHER_CTX_set_padding(sess->cipher.ctx, 0);
> +
>  	return 0;
>  }
> @@ -943,6 +945,4 @@ process_openssl_cipher_encrypt(struct rte_mbuf *mbuf_src, uint8_t *dst,
>  		goto process_cipher_encrypt_err;
>  
> -	EVP_CIPHER_CTX_set_padding(ctx, 0);
> -
>  	if (process_openssl_encryption_update(mbuf_src, offset, &dst,
>  			srclen, ctx, inplace))
> @@ -993,6 +993,4 @@ process_openssl_cipher_decrypt(struct rte_mbuf *mbuf_src, uint8_t *dst,
>  		goto process_cipher_decrypt_err;
>  
> -	EVP_CIPHER_CTX_set_padding(ctx, 0);
> -
>  	if (process_openssl_decryption_update(mbuf_src, offset, &dst,
>  			srclen, ctx, inplace))