> -----Original Message----- > From: Karas, Krzysztof <[email protected]> > Sent: Wednesday, September 27, 2023 1:41 PM > To: Ji, Kai <[email protected]>; De Lara Guarch, Pablo > <[email protected]>; Cornu, Marcel D > <[email protected]>; Power, Ciara <[email protected]> > Cc: [email protected]; Karas, Krzysztof <[email protected]>; > [email protected] > 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: [email protected] > > Signed-off-by: Krzysztof Karas <[email protected]> > > --- > 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
