From: "Cornu, Marcel D" <marcel.d.cornu@intel.com>
To: "Karas, Krzysztof" <krzysztof.karas@intel.com>,
"Ji, Kai" <kai.ji@intel.com>,
"De Lara Guarch, Pablo" <pablo.de.lara.guarch@intel.com>,
"Power, Ciara" <ciara.power@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>, "stable@dpdk.org" <stable@dpdk.org>
Subject: RE: [PATCH] crypto/ipsec_mb: Do not dequeue ops from ring after job flush.
Date: Thu, 2 Nov 2023 17:32:07 +0000 [thread overview]
Message-ID: <CH0PR11MB54921092124277B61363C6D3DAA6A@CH0PR11MB5492.namprd11.prod.outlook.com> (raw)
In-Reply-To: <20230927124034.1092086-1-krzysztof.karas@intel.com>
> -----Original Message-----
> From: Karas, Krzysztof <krzysztof.karas@intel.com>
> Sent: Wednesday, September 27, 2023 1:41 PM
> To: Ji, Kai <kai.ji@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Cornu, Marcel D
> <marcel.d.cornu@intel.com>; Power, Ciara <ciara.power@intel.com>
> Cc: dev@dpdk.org; Karas, Krzysztof <krzysztof.karas@intel.com>;
> stable@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: stable@dpdk.org
>
> Signed-off-by: Krzysztof Karas <krzysztof.karas@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
prev parent reply other threads:[~2023-11-02 17:32 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-09-27 12:40 Krzysztof Karas
2023-10-30 7:10 ` [EXT] " Akhil Goyal
2023-11-02 17:32 ` Cornu, Marcel D [this message]
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=CH0PR11MB54921092124277B61363C6D3DAA6A@CH0PR11MB5492.namprd11.prod.outlook.com \
--to=marcel.d.cornu@intel.com \
--cc=ciara.power@intel.com \
--cc=dev@dpdk.org \
--cc=kai.ji@intel.com \
--cc=krzysztof.karas@intel.com \
--cc=pablo.de.lara.guarch@intel.com \
--cc=stable@dpdk.org \
/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).