> -----Original Message-----
> From: Karas, Krzysztof <krzysztof.ka...@intel.com>
> Sent: Wednesday, September 27, 2023 1:41 PM
> To: Ji, Kai <kai...@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.gua...@intel.com>; Cornu, Marcel D
> <marcel.d.co...@intel.com>; Power, Ciara <ciara.po...@intel.com>
> Cc: dev@dpdk.org; Karas, Krzysztof <krzysztof.ka...@intel.com>;
> sta...@dpdk.org
> Subject: [PATCH] crypto/ipsec_mb: Do not dequeue ops from ring after job
> flush.
>
> Previously it was possible to increment `processed_jobs` to a value greater
> than
> requested `nb_ops`, because after flushing at most `nb_ops` jobs the while
> loop
> continued, so `processed_jobs` could still be incremented and it was possible
> for
> this variable to be greater than `nb_ops`. If `ops` provided to the function
> were
> only `nb_ops` long, then the `aesni_mb_dequeue_burst()` would write to the
> memory outside of `ops` array.
>
> Fixes: b50b8b5b38f8 ("crypto/ipsec_mb: use burst API in AESNI")
> Cc: sta...@dpdk.org
>
> Signed-off-by: Krzysztof Karas <krzysztof.ka...@intel.com>
>
> ---
> drivers/crypto/ipsec_mb/pmd_aesni_mb.c | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
> b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
> index 9e298023d7..ff52bc85a4 100644
> --- a/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
> +++ b/drivers/crypto/ipsec_mb/pmd_aesni_mb.c
> @@ -2056,7 +2056,7 @@ aesni_mb_dequeue_burst(void *queue_pair, struct
> rte_crypto_op **ops,
> uint16_t n = (nb_ops / burst_sz) ?
> burst_sz : nb_ops;
>
> - while (unlikely((IMB_GET_NEXT_BURST(mb_mgr, n, jobs)) < n)) {
> + if (unlikely((IMB_GET_NEXT_BURST(mb_mgr, n, jobs)) < n)) {
> /*
> * Not enough free jobs in the queue
> * Flush n jobs until enough jobs available @@ -2074,6
> +2074,8 @@ aesni_mb_dequeue_burst(void *queue_pair, struct rte_crypto_op
> **ops,
> break;
> }
> }
> + nb_ops -= nb_jobs;
This assumes the loop above completes without errors.
If post_process_mb_job() returns with an error it will break out of the loop
and nb_ops will be decremented by the wrong value.
Maybe decrementing by 'i' would work better here.
> + continue;
> }
>
> /*
> --
> 2.34.1
Hi Krzysztof, I noticed that when I run the dpdk-test-crypto-perf application
testing with imix, the number of failed enqueue ops increases vs without the
patch.
Example command: dpdk-test-crypto-perf -l 4,5 --no-huge
--vdev="crypto_aesni_mb" -- --pool-sz 8192 --cipher-algo aes-cbc
--cipher-key-sz 16 --optype cipher-only --cipher-iv-sz 16 --cipher-op encrypt
--silent --buffer-sz 16,6144 --imix 99,1 --burst-sz 32
Could you try it and see if you get the same result?
Regards,
Marcel
