diff mbox series

[01/14] crypto/dpaa2_sec: fix fle buffer leak

Message ID 20220422035100.3180870-1-g.singh@nxp.com (mailing list archive)
State Superseded, archived
Delegated to: akhil goyal
Headers
Series [01/14] crypto/dpaa2_sec: fix fle buffer leak |

Checks

Context Check Description
ci/checkpatch warning coding style issues

Commit Message

Gagandeep Singh April 22, 2022, 3:50 a.m. UTC 
  Driver allocates a fle buffer for each packet
before enqueue and free the buffer on dequeue. But in case if
there are enqueue failures, then code should free the fle buffers.

Fixes: b15cbf5b2d88 ("crypto/dpaa2_sec: fix fle buffer leak")
Cc: stable@dpdk.org

Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
---
 drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 35 ++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

Comments

Akhil Goyal April 28, 2022, 7:15 a.m. UTC | #1 
> Driver allocates a fle buffer for each packet
> before enqueue and free the buffer on dequeue. But in case if
> there are enqueue failures, then code should free the fle buffers.
> 
> Fixes: b15cbf5b2d88 ("crypto/dpaa2_sec: fix fle buffer leak")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> ---

You should use check-git-log
$ ./devtools/check-git-log.sh -14
Wrong headline format:
        crypto/dpaa_sec : fix secondary process probe
Wrong headline prefix:
        dpaax/caamflib: remove obsolete code
        crypto/dpaa2_sec: per queue pair fle pool
        crypto/dpaa_sec: remove unused thread specific variables
Wrong headline case:
                        "crypto/dpaa_sec: fix length for chain fd in raw sec driver": fd --> FD
Wrong headline case:
                        "crypto/dpaa2_sec: fix length for chain fd in raw sec driver": fd --> FD
Wrong headline case:
                        "crypto/dpaa2_sec: fix operation status for simple fd": fd --> FD
Headline too long:
        crypto/dpaa2_sec: fix crypto op pointer for atomic and ordered queues
Wrong 'Fixes' reference:
        Fixes: b15cbf5b2d88 ("crypto/dpaa2_sec: fix fle buffer leak")

Invalid patch(es) found - checked 14 patches
Gagandeep Singh April 28, 2022, 9:23 a.m. UTC | #2 
Hi,

> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Thursday, April 28, 2022 12:46 PM
> To: Gagandeep Singh <G.Singh@nxp.com>; dev@dpdk.org
> Cc: stable@dpdk.org
> Subject: RE: [EXT] [PATCH 01/14] crypto/dpaa2_sec: fix fle buffer leak
> 
> > Driver allocates a fle buffer for each packet before enqueue and free
> > the buffer on dequeue. But in case if there are enqueue failures, then
> > code should free the fle buffers.
> >
> > Fixes: b15cbf5b2d88 ("crypto/dpaa2_sec: fix fle buffer leak")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Gagandeep Singh <g.singh@nxp.com>
> > ---
> 
> You should use check-git-log
> $ ./devtools/check-git-log.sh -14
> Wrong headline format:
>         crypto/dpaa_sec : fix secondary process probe Wrong headline prefix:
>         dpaax/caamflib: remove obsolete code
>         crypto/dpaa2_sec: per queue pair fle pool
>         crypto/dpaa_sec: remove unused thread specific variables Wrong headline
> case:
>                         "crypto/dpaa_sec: fix length for chain fd in raw sec driver": fd -->
> FD Wrong headline case:
>                         "crypto/dpaa2_sec: fix length for chain fd in raw sec driver": fd -->
> FD Wrong headline case:
>                         "crypto/dpaa2_sec: fix operation status for simple fd": fd --> FD
> Headline too long:
>         crypto/dpaa2_sec: fix crypto op pointer for atomic and ordered queues
> Wrong 'Fixes' reference:
>         Fixes: b15cbf5b2d88 ("crypto/dpaa2_sec: fix fle buffer leak")
> 
> Invalid patch(es) found - checked 14 patches

In two of the patches check-git-log is giving below error:

Wrong headline prefix:
        crypto/dpaa2_sec: per queue pair fle pool
        crypto/dpaa_sec: remove unused thread specific variables

Invalid patch(es) found - checked 14 patches

These patches have changes in bus as well in crypto drivers. What would be the correct headline prefix for these patches? Please advise.
Akhil Goyal April 28, 2022, 9:29 a.m. UTC | #3 
> > You should use check-git-log
> > $ ./devtools/check-git-log.sh -14
> > Wrong headline format:
> >         crypto/dpaa_sec : fix secondary process probe Wrong headline prefix:
> >         dpaax/caamflib: remove obsolete code
> >         crypto/dpaa2_sec: per queue pair fle pool
> >         crypto/dpaa_sec: remove unused thread specific variables Wrong
> headline
> > case:
> >                         "crypto/dpaa_sec: fix length for chain fd in raw sec driver": fd -->
> > FD Wrong headline case:
> >                         "crypto/dpaa2_sec: fix length for chain fd in raw sec driver": fd --
> >
> > FD Wrong headline case:
> >                         "crypto/dpaa2_sec: fix operation status for simple fd": fd --> FD
> > Headline too long:
> >         crypto/dpaa2_sec: fix crypto op pointer for atomic and ordered queues
> > Wrong 'Fixes' reference:
> >         Fixes: b15cbf5b2d88 ("crypto/dpaa2_sec: fix fle buffer leak")
> >
> > Invalid patch(es) found - checked 14 patches
> 
> In two of the patches check-git-log is giving below error:
> 
> Wrong headline prefix:
>         crypto/dpaa2_sec: per queue pair fle pool
>         crypto/dpaa_sec: remove unused thread specific variables
> 
> Invalid patch(es) found - checked 14 patches
> 
> These patches have changes in bus as well in crypto drivers. What would be the
> correct headline prefix for these patches? Please advise.

If the major/main change is in crypto driver, then it should be crypto/xxx or else bus/xxx
diff mbox series

Patch

diff --git a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
index e62d04852b..03fef5e500 100644
--- a/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
+++ b/drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c
@@ -1,7 +1,7 @@ 
 /* SPDX-License-Identifier: BSD-3-Clause
  *
  *   Copyright (c) 2016 Freescale Semiconductor, Inc. All rights reserved.
- *   Copyright 2016-2021 NXP
+ *   Copyright 2016-2022 NXP
  *
  */
 
@@ -64,6 +64,27 @@  enum dpaa2_sec_dump_levels {
 uint8_t cryptodev_driver_id;
 uint8_t dpaa2_sec_dp_dump = DPAA2_SEC_DP_ERR_DUMP;
 
+static inline void
+free_fle(const struct qbman_fd *fd)
+{
+	struct qbman_fle *fle;
+	struct rte_crypto_op *op;
+	struct ctxt_priv *priv;
+
+#ifdef RTE_LIB_SECURITY
+	if (DPAA2_FD_GET_FORMAT(fd) == qbman_fd_single)
+		return;
+#endif
+	fle = (struct qbman_fle *)DPAA2_IOVA_TO_VADDR(DPAA2_GET_FD_ADDR(fd));
+	op = (struct rte_crypto_op *)DPAA2_GET_FLE_ADDR((fle - 1));
+	/* free the fle memory */
+	if (likely(rte_pktmbuf_is_contiguous(op->sym->m_src))) {
+		priv = (struct ctxt_priv *)(size_t)DPAA2_GET_FLE_CTXT(fle - 1);
+		rte_mempool_put(priv->fle_pool, (void *)(fle-1));
+	} else
+		rte_free((void *)(fle-1));
+}
+
 #ifdef RTE_LIB_SECURITY
 static inline int
 build_proto_compound_sg_fd(dpaa2_sec_session *sess,
@@ -1513,6 +1534,12 @@  dpaa2_sec_enqueue_burst(void *qp, struct rte_crypto_op **ops,
 				if (retry_count > DPAA2_MAX_TX_RETRY_COUNT) {
 					num_tx += loop;
 					nb_ops -= loop;
+					DPAA2_SEC_DP_DEBUG("Enqueue fail\n");
+					/* freeing the fle buffers */
+					while (loop < frames_to_send) {
+						free_fle(&fd_arr[loop]);
+						loop++;
+					}
 					goto skip_tx;
 				}
 			} else {
@@ -1854,6 +1881,12 @@  dpaa2_sec_enqueue_burst_ordered(void *qp, struct rte_crypto_op **ops,
 				if (retry_count > DPAA2_MAX_TX_RETRY_COUNT) {
 					num_tx += loop;
 					nb_ops -= loop;
+					DPAA2_SEC_DP_DEBUG("Enqueue fail\n");
+					/* freeing the fle buffers */
+					while (loop < frames_to_send) {
+						free_fle(&fd_arr[loop]);
+						loop++;
+					}
 					goto skip_tx;
 				}
 			} else {