* [dpdk-dev] [PATCH v1 0/2] improve security instance setup @ 2020-07-16 15:35 David Coyle 2020-07-16 15:35 ` [dpdk-dev] [PATCH v1 1/2] crypto/qat: " David Coyle ` (2 more replies) 0 siblings, 3 replies; 14+ messages in thread From: David Coyle @ 2020-07-16 15:35 UTC (permalink / raw) To: akhil.goyal, declan.doherty, pablo.de.lara.guarch, fiona.trahe Cc: dev, brendan.ryan, mairtin.oloingsigh, David Coyle These patches make some minor improvements to the security instance setup for the QAT SYM and AESNI-MB PMDs. David Coyle (2): crypto/qat: improve security instance setup crypto/aesni_mb: improve security instance setup drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 13 ++++--------- drivers/crypto/qat/qat_sym_pmd.c | 9 +++------ 2 files changed, 7 insertions(+), 15 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v1 1/2] crypto/qat: improve security instance setup 2020-07-16 15:35 [dpdk-dev] [PATCH v1 0/2] improve security instance setup David Coyle @ 2020-07-16 15:35 ` David Coyle 2020-07-17 18:20 ` Trahe, Fiona 2020-07-16 15:36 ` [dpdk-dev] [PATCH v1 2/2] crypto/aesni_mb: " David Coyle 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 0/2] " David Coyle 2 siblings, 1 reply; 14+ messages in thread From: David Coyle @ 2020-07-16 15:35 UTC (permalink / raw) To: akhil.goyal, declan.doherty, pablo.de.lara.guarch, fiona.trahe Cc: dev, brendan.ryan, mairtin.oloingsigh, David Coyle This patch makes some minor improvements to the security instance setup for the QAT SYM PMD. All of this setup code is now in one '#ifdef RTE_LIBRTE_SECURITY' block. Enabling the RTE_CRYPTODEV_FF_SECURITY feature for the device is also moved to this block. Fixes: 6f0ef237404b ("crypto/qat: support DOCSIS protocol") Signed-off-by: David Coyle <david.coyle@intel.com> --- drivers/crypto/qat/qat_sym_pmd.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/drivers/crypto/qat/qat_sym_pmd.c b/drivers/crypto/qat/qat_sym_pmd.c index c7e323cce..7c760f9d2 100644 --- a/drivers/crypto/qat/qat_sym_pmd.c +++ b/drivers/crypto/qat/qat_sym_pmd.c @@ -346,10 +346,6 @@ qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, } } -#ifdef RTE_LIBRTE_SECURITY - struct rte_security_ctx *security_instance; -#endif - snprintf(name, RTE_CRYPTODEV_NAME_MAX_LEN, "%s_%s", qat_pci_dev->name, "sym"); QAT_LOG(DEBUG, "Creating QAT SYM device %s", name); @@ -381,8 +377,7 @@ qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, RTE_CRYPTODEV_FF_OOP_SGL_IN_LB_OUT | RTE_CRYPTODEV_FF_OOP_LB_IN_SGL_OUT | RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT | - RTE_CRYPTODEV_FF_DIGEST_ENCRYPTED | - RTE_CRYPTODEV_FF_SECURITY; + RTE_CRYPTODEV_FF_DIGEST_ENCRYPTED; if (rte_eal_process_type() != RTE_PROC_PRIMARY) return 0; @@ -392,6 +387,7 @@ qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, qat_pci_dev->qat_dev_gen); #ifdef RTE_LIBRTE_SECURITY + struct rte_security_ctx *security_instance; security_instance = rte_malloc("qat_sec", sizeof(struct rte_security_ctx), RTE_CACHE_LINE_SIZE); @@ -405,6 +401,7 @@ qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, security_instance->ops = &security_qat_ops; security_instance->sess_cnt = 0; cryptodev->security_ctx = security_instance; + cryptodev->feature_flags |= RTE_CRYPTODEV_FF_SECURITY; #endif internals = cryptodev->data->dev_private; -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/2] crypto/qat: improve security instance setup 2020-07-16 15:35 ` [dpdk-dev] [PATCH v1 1/2] crypto/qat: " David Coyle @ 2020-07-17 18:20 ` Trahe, Fiona 2020-07-18 21:36 ` Akhil Goyal 0 siblings, 1 reply; 14+ messages in thread From: Trahe, Fiona @ 2020-07-17 18:20 UTC (permalink / raw) To: Coyle, David, akhil.goyal, Doherty, Declan, De Lara Guarch, Pablo Cc: dev, Ryan, Brendan, O'loingsigh, Mairtin > -----Original Message----- > From: Coyle, David <david.coyle@intel.com> > Sent: Thursday, July 16, 2020 4:36 PM > To: akhil.goyal@nxp.com; Doherty, Declan <declan.doherty@intel.com>; De Lara Guarch, Pablo > <pablo.de.lara.guarch@intel.com>; Trahe, Fiona <fiona.trahe@intel.com> > Cc: dev@dpdk.org; Ryan, Brendan <brendan.ryan@intel.com>; O'loingsigh, Mairtin > <mairtin.oloingsigh@intel.com>; Coyle, David <david.coyle@intel.com> > Subject: [PATCH v1 1/2] crypto/qat: improve security instance setup > > This patch makes some minor improvements to the security instance setup > for the QAT SYM PMD. All of this setup code is now in one '#ifdef > RTE_LIBRTE_SECURITY' block. Enabling the RTE_CRYPTODEV_FF_SECURITY > feature for the device is also moved to this block. > > Fixes: 6f0ef237404b ("crypto/qat: support DOCSIS protocol") > > Signed-off-by: David Coyle <david.coyle@intel.com> Acked-by: Fiona Trahe <fiona.trahe@intel.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/2] crypto/qat: improve security instance setup 2020-07-17 18:20 ` Trahe, Fiona @ 2020-07-18 21:36 ` Akhil Goyal 2020-07-18 21:40 ` Akhil Goyal 0 siblings, 1 reply; 14+ messages in thread From: Akhil Goyal @ 2020-07-18 21:36 UTC (permalink / raw) To: Trahe, Fiona, Coyle, David, Doherty, Declan, De Lara Guarch, Pablo Cc: dev, Ryan, Brendan, O'loingsigh, Mairtin > > This patch makes some minor improvements to the security instance setup > > for the QAT SYM PMD. All of this setup code is now in one '#ifdef > > RTE_LIBRTE_SECURITY' block. Enabling the RTE_CRYPTODEV_FF_SECURITY > > feature for the device is also moved to this block. > > > > Fixes: 6f0ef237404b ("crypto/qat: support DOCSIS protocol") > > > > Signed-off-by: David Coyle <david.coyle@intel.com> > Acked-by: Fiona Trahe <fiona.trahe@intel.com> This patch is applied to dpdk-next-crypto Please send next version for 2/2 of this series. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/2] crypto/qat: improve security instance setup 2020-07-18 21:36 ` Akhil Goyal @ 2020-07-18 21:40 ` Akhil Goyal 2020-07-20 12:39 ` Coyle, David 0 siblings, 1 reply; 14+ messages in thread From: Akhil Goyal @ 2020-07-18 21:40 UTC (permalink / raw) To: Trahe, Fiona, Coyle, David, Doherty, Declan, De Lara Guarch, Pablo Cc: dev, Ryan, Brendan, O'loingsigh, Mairtin > > > > This patch makes some minor improvements to the security instance setup > > > for the QAT SYM PMD. All of this setup code is now in one '#ifdef > > > RTE_LIBRTE_SECURITY' block. Enabling the RTE_CRYPTODEV_FF_SECURITY > > > feature for the device is also moved to this block. > > > > > > Fixes: 6f0ef237404b ("crypto/qat: support DOCSIS protocol") > > > > > > Signed-off-by: David Coyle <david.coyle@intel.com> > > Acked-by: Fiona Trahe <fiona.trahe@intel.com> > > This patch is applied to dpdk-next-crypto > > Please send next version for 2/2 of this series. No this patch is pulled back. I suppose the memory leak is there in this patch also. ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v1 1/2] crypto/qat: improve security instance setup 2020-07-18 21:40 ` Akhil Goyal @ 2020-07-20 12:39 ` Coyle, David 0 siblings, 0 replies; 14+ messages in thread From: Coyle, David @ 2020-07-20 12:39 UTC (permalink / raw) To: Akhil Goyal, Trahe, Fiona, Doherty, Declan, De Lara Guarch, Pablo Cc: dev, Ryan, Brendan, O'loingsigh, Mairtin Hi Akhil, > -----Original Message----- > From: Akhil Goyal <akhil.goyal@nxp.com> > Sent: Saturday, July 18, 2020 10:41 PM > > > > This patch makes some minor improvements to the security instance > > > > setup for the QAT SYM PMD. All of this setup code is now in one > > > > '#ifdef RTE_LIBRTE_SECURITY' block. Enabling the > > > > RTE_CRYPTODEV_FF_SECURITY feature for the device is also moved to > this block. > > > > > > > > Fixes: 6f0ef237404b ("crypto/qat: support DOCSIS protocol") > > > > > > > > Signed-off-by: David Coyle <david.coyle@intel.com> > > > Acked-by: Fiona Trahe <fiona.trahe@intel.com> > > > > This patch is applied to dpdk-next-crypto > > > > Please send next version for 2/2 of this series. > > No this patch is pulled back. I suppose the memory leak is there in this patch > also. [DC] Yes, memory leak is here too and is fixed in v2 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v1 2/2] crypto/aesni_mb: improve security instance setup 2020-07-16 15:35 [dpdk-dev] [PATCH v1 0/2] improve security instance setup David Coyle 2020-07-16 15:35 ` [dpdk-dev] [PATCH v1 1/2] crypto/qat: " David Coyle @ 2020-07-16 15:36 ` David Coyle 2020-07-17 19:29 ` De Lara Guarch, Pablo 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 0/2] " David Coyle 2 siblings, 1 reply; 14+ messages in thread From: David Coyle @ 2020-07-16 15:36 UTC (permalink / raw) To: akhil.goyal, declan.doherty, pablo.de.lara.guarch, fiona.trahe Cc: dev, brendan.ryan, mairtin.oloingsigh, David Coyle This patch makes some minor improvements to the security instance setup for the AESNI-MB PMD. All of this setup code is now in one '#ifdef AESNI_MB_DOCSIS_SEC_ENABLED' block. Enabling the RTE_CRYPTODEV_FF_SECURITY feature for the device is also moved to this block. Fixes: fda5216fba55 ("crypto/aesni_mb: support DOCSIS protocol") Signed-off-by: David Coyle <david.coyle@intel.com> --- drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c index b54c57f86..171d914a3 100644 --- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c +++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c @@ -1881,9 +1881,6 @@ cryptodev_aesni_mb_create(const char *name, struct aesni_mb_private *internals; enum aesni_mb_vector_mode vector_mode; MB_MGR *mb_mgr; -#ifdef AESNI_MB_DOCSIS_SEC_ENABLED - struct rte_security_ctx *security_instance; -#endif dev = rte_cryptodev_pmd_create(name, &vdev->device, init_params); if (dev == NULL) { @@ -1912,13 +1909,10 @@ cryptodev_aesni_mb_create(const char *name, RTE_CRYPTODEV_FF_SYM_OPERATION_CHAINING | RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT | RTE_CRYPTODEV_FF_SYM_CPU_CRYPTO | - RTE_CRYPTODEV_FF_SYM_SESSIONLESS -#ifdef AESNI_MB_DOCSIS_SEC_ENABLED - | RTE_CRYPTODEV_FF_SECURITY -#endif - ; + RTE_CRYPTODEV_FF_SYM_SESSIONLESS; #ifdef AESNI_MB_DOCSIS_SEC_ENABLED + struct rte_security_ctx *security_instance; security_instance = rte_malloc("aesni_mb_sec", sizeof(struct rte_security_ctx), RTE_CACHE_LINE_SIZE); @@ -1932,6 +1926,7 @@ cryptodev_aesni_mb_create(const char *name, security_instance->ops = rte_aesni_mb_pmd_sec_ops; security_instance->sess_cnt = 0; dev->security_ctx = security_instance; + dev->feature_flags |= RTE_CRYPTODEV_FF_SECURITY; #endif /* Check CPU for support for AES instruction set */ @@ -2011,7 +2006,7 @@ cryptodev_aesni_mb_remove(struct rte_vdev_device *vdev) RTE_PER_LCORE(sync_mb_mgr) = NULL; } -#ifdef RTE_LIBRTE_SECURITY +#ifdef AESNI_MB_DOCSIS_SEC_ENABLED rte_free(cryptodev->security_ctx); #endif -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v1 2/2] crypto/aesni_mb: improve security instance setup 2020-07-16 15:36 ` [dpdk-dev] [PATCH v1 2/2] crypto/aesni_mb: " David Coyle @ 2020-07-17 19:29 ` De Lara Guarch, Pablo 2020-07-20 12:38 ` Coyle, David 0 siblings, 1 reply; 14+ messages in thread From: De Lara Guarch, Pablo @ 2020-07-17 19:29 UTC (permalink / raw) To: Coyle, David, akhil.goyal, Doherty, Declan, Trahe, Fiona Cc: dev, Ryan, Brendan, O'loingsigh, Mairtin Hi David, > -----Original Message----- > From: Coyle, David <david.coyle@intel.com> > Sent: Thursday, July 16, 2020 4:36 PM > To: akhil.goyal@nxp.com; Doherty, Declan <declan.doherty@intel.com>; De > Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona > <fiona.trahe@intel.com> > Cc: dev@dpdk.org; Ryan, Brendan <brendan.ryan@intel.com>; O'loingsigh, > Mairtin <mairtin.oloingsigh@intel.com>; Coyle, David <david.coyle@intel.com> > Subject: [PATCH v1 2/2] crypto/aesni_mb: improve security instance setup > > This patch makes some minor improvements to the security instance setup for > the AESNI-MB PMD. All of this setup code is now in one '#ifdef > AESNI_MB_DOCSIS_SEC_ENABLED' block. Enabling the > RTE_CRYPTODEV_FF_SECURITY feature for the device is also moved to this > block. > > Fixes: fda5216fba55 ("crypto/aesni_mb: support DOCSIS protocol") > > Signed-off-by: David Coyle <david.coyle@intel.com> > --- > drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 13 ++++--------- > 1 file changed, 4 insertions(+), 9 deletions(-) > > diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c > b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c > index b54c57f86..171d914a3 100644 > --- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c > +++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c > @@ -1881,9 +1881,6 @@ cryptodev_aesni_mb_create(const char *name, > struct aesni_mb_private *internals; > enum aesni_mb_vector_mode vector_mode; > MB_MGR *mb_mgr; > -#ifdef AESNI_MB_DOCSIS_SEC_ENABLED > - struct rte_security_ctx *security_instance; > -#endif > > dev = rte_cryptodev_pmd_create(name, &vdev->device, init_params); > if (dev == NULL) { > @@ -1912,13 +1909,10 @@ cryptodev_aesni_mb_create(const char *name, > RTE_CRYPTODEV_FF_SYM_OPERATION_CHAINING | > RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT | > RTE_CRYPTODEV_FF_SYM_CPU_CRYPTO | > - RTE_CRYPTODEV_FF_SYM_SESSIONLESS > -#ifdef AESNI_MB_DOCSIS_SEC_ENABLED > - | RTE_CRYPTODEV_FF_SECURITY > -#endif > - ; > + RTE_CRYPTODEV_FF_SYM_SESSIONLESS; > > #ifdef AESNI_MB_DOCSIS_SEC_ENABLED > + struct rte_security_ctx *security_instance; > security_instance = rte_malloc("aesni_mb_sec", > sizeof(struct rte_security_ctx), > RTE_CACHE_LINE_SIZE); I see that there could be a potential memory leak here. Assuming this malloc works, if alloc_init_mb_mgr() fails, this memory will not be freed. So I suggest two options: 1 - Free security_instance if alloc_init_mb_mgr() fails 2 - Move this piece of code after alloc_init_mb_mgr and free mb_mgr if this malloc fails. Thanks, Pablo ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v1 2/2] crypto/aesni_mb: improve security instance setup 2020-07-17 19:29 ` De Lara Guarch, Pablo @ 2020-07-20 12:38 ` Coyle, David 0 siblings, 0 replies; 14+ messages in thread From: Coyle, David @ 2020-07-20 12:38 UTC (permalink / raw) To: De Lara Guarch, Pablo, akhil.goyal, Doherty, Declan, Trahe, Fiona Cc: dev, Ryan, Brendan, O'loingsigh, Mairtin Hi Pablo > -----Original Message----- > From: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com> > Sent: Friday, July 17, 2020 8:29 PM > > > > #ifdef AESNI_MB_DOCSIS_SEC_ENABLED > > + struct rte_security_ctx *security_instance; > > security_instance = rte_malloc("aesni_mb_sec", > > sizeof(struct rte_security_ctx), > > RTE_CACHE_LINE_SIZE); > > I see that there could be a potential memory leak here. > Assuming this malloc works, if alloc_init_mb_mgr() fails, this memory will not > be freed. > So I suggest two options: > 1 - Free security_instance if alloc_init_mb_mgr() fails > 2 - Move this piece of code after alloc_init_mb_mgr and free mb_mgr if this > malloc fails. [DC] Good catch, disappointed I didn't spot that myself :( This is fixed in v2 coming very shortly - used option 1 above > ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v2 0/2] improve security instance setup 2020-07-16 15:35 [dpdk-dev] [PATCH v1 0/2] improve security instance setup David Coyle 2020-07-16 15:35 ` [dpdk-dev] [PATCH v1 1/2] crypto/qat: " David Coyle 2020-07-16 15:36 ` [dpdk-dev] [PATCH v1 2/2] crypto/aesni_mb: " David Coyle @ 2020-07-20 12:16 ` David Coyle 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 1/2] crypto/qat: " David Coyle ` (2 more replies) 2 siblings, 3 replies; 14+ messages in thread From: David Coyle @ 2020-07-20 12:16 UTC (permalink / raw) To: akhil.goyal, declan.doherty, pablo.de.lara.guarch, fiona.trahe Cc: dev, brendan.ryan, mairtin.oloingsigh, David Coyle These patches make some improvements to the security instance setup for the QAT SYM and AESNI-MB PMDs. David Coyle (2): crypto/qat: improve security instance setup crypto/aesni-mb: improve security instance setup drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 18 +++++----- drivers/crypto/qat/qat_sym_pmd.c | 42 ++++++++++++---------- 2 files changed, 33 insertions(+), 27 deletions(-) -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v2 1/2] crypto/qat: improve security instance setup 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 0/2] " David Coyle @ 2020-07-20 12:16 ` David Coyle 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 2/2] crypto/aesni-mb: " David Coyle 2020-07-26 19:08 ` [dpdk-dev] [PATCH v2 0/2] " Akhil Goyal 2 siblings, 0 replies; 14+ messages in thread From: David Coyle @ 2020-07-20 12:16 UTC (permalink / raw) To: akhil.goyal, declan.doherty, pablo.de.lara.guarch, fiona.trahe Cc: dev, brendan.ryan, mairtin.oloingsigh, David Coyle This patch makes some improvements to the security instance setup for the QAT SYM PMD, as follows: - fix potential memory leak where the security instance was not freed if an error occurred later in the device creation - tidy-up security instance initialization code by moving it all, including enabling the RTE_CRYPTODEV_FF_SECURITY feature, into one '#ifdef RTE_LIBRTE_SECURITY' block Fixes: 6f0ef237404b ("crypto/qat: support DOCSIS protocol") Signed-off-by: David Coyle <david.coyle@intel.com> Acked-by: Fiona Trahe <fiona.trahe@intel.com> --- drivers/crypto/qat/qat_sym_pmd.c | 42 ++++++++++++++++++-------------- 1 file changed, 24 insertions(+), 18 deletions(-) diff --git a/drivers/crypto/qat/qat_sym_pmd.c b/drivers/crypto/qat/qat_sym_pmd.c index c7e323cce..43870ac04 100644 --- a/drivers/crypto/qat/qat_sym_pmd.c +++ b/drivers/crypto/qat/qat_sym_pmd.c @@ -310,7 +310,7 @@ int qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, struct qat_dev_cmd_param *qat_dev_cmd_param __rte_unused) { - int i = 0; + int i = 0, ret = 0; struct qat_device_info *qat_dev_instance = &qat_pci_devs[qat_pci_dev->qat_dev_id]; @@ -346,10 +346,6 @@ qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, } } -#ifdef RTE_LIBRTE_SECURITY - struct rte_security_ctx *security_instance; -#endif - snprintf(name, RTE_CRYPTODEV_NAME_MAX_LEN, "%s_%s", qat_pci_dev->name, "sym"); QAT_LOG(DEBUG, "Creating QAT SYM device %s", name); @@ -381,8 +377,7 @@ qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, RTE_CRYPTODEV_FF_OOP_SGL_IN_LB_OUT | RTE_CRYPTODEV_FF_OOP_LB_IN_SGL_OUT | RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT | - RTE_CRYPTODEV_FF_DIGEST_ENCRYPTED | - RTE_CRYPTODEV_FF_SECURITY; + RTE_CRYPTODEV_FF_DIGEST_ENCRYPTED; if (rte_eal_process_type() != RTE_PROC_PRIMARY) return 0; @@ -392,19 +387,21 @@ qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, qat_pci_dev->qat_dev_gen); #ifdef RTE_LIBRTE_SECURITY + struct rte_security_ctx *security_instance; security_instance = rte_malloc("qat_sec", sizeof(struct rte_security_ctx), RTE_CACHE_LINE_SIZE); if (security_instance == NULL) { QAT_LOG(ERR, "rte_security_ctx memory alloc failed"); - rte_cryptodev_pmd_destroy(cryptodev); - return -ENOMEM; + ret = -ENOMEM; + goto error; } security_instance->device = (void *)cryptodev; security_instance->ops = &security_qat_ops; security_instance->sess_cnt = 0; cryptodev->security_ctx = security_instance; + cryptodev->feature_flags |= RTE_CRYPTODEV_FF_SECURITY; #endif internals = cryptodev->data->dev_private; @@ -428,10 +425,8 @@ qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, QAT_LOG(DEBUG, "QAT gen %d capabilities unknown", qat_pci_dev->qat_dev_gen); - rte_cryptodev_pmd_destroy(cryptodev); - memset(&qat_dev_instance->sym_rte_dev, 0, - sizeof(qat_dev_instance->sym_rte_dev)); - return -(EINVAL); + ret = -(EINVAL); + goto error; } internals->capa_mz = rte_memzone_lookup(capa_memz_name); @@ -442,12 +437,11 @@ qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, } if (internals->capa_mz == NULL) { QAT_LOG(DEBUG, - "Error allocating memzone for capabilities, destroying PMD for %s", + "Error allocating memzone for capabilities, destroying " + "PMD for %s", name); - rte_cryptodev_pmd_destroy(cryptodev); - memset(&qat_dev_instance->sym_rte_dev, 0, - sizeof(qat_dev_instance->sym_rte_dev)); - return -EFAULT; + ret = -EFAULT; + goto error; } memcpy(internals->capa_mz->addr, capabilities, capa_size); @@ -467,6 +461,17 @@ qat_sym_dev_create(struct qat_pci_device *qat_pci_dev, cryptodev->data->name, internals->sym_dev_id); return 0; + +error: +#ifdef RTE_LIBRTE_SECURITY + rte_free(cryptodev->security_ctx); + cryptodev->security_ctx = NULL; +#endif + rte_cryptodev_pmd_destroy(cryptodev); + memset(&qat_dev_instance->sym_rte_dev, 0, + sizeof(qat_dev_instance->sym_rte_dev)); + + return ret; } int @@ -485,6 +490,7 @@ qat_sym_dev_destroy(struct qat_pci_device *qat_pci_dev) cryptodev = rte_cryptodev_pmd_get_dev(qat_pci_dev->sym_dev->sym_dev_id); #ifdef RTE_LIBRTE_SECURITY rte_free(cryptodev->security_ctx); + cryptodev->security_ctx = NULL; #endif rte_cryptodev_pmd_destroy(cryptodev); qat_pci_devs[qat_pci_dev->qat_dev_id].sym_rte_dev.name = NULL; -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [dpdk-dev] [PATCH v2 2/2] crypto/aesni-mb: improve security instance setup 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 0/2] " David Coyle 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 1/2] crypto/qat: " David Coyle @ 2020-07-20 12:16 ` David Coyle 2020-07-22 7:58 ` De Lara Guarch, Pablo 2020-07-26 19:08 ` [dpdk-dev] [PATCH v2 0/2] " Akhil Goyal 2 siblings, 1 reply; 14+ messages in thread From: David Coyle @ 2020-07-20 12:16 UTC (permalink / raw) To: akhil.goyal, declan.doherty, pablo.de.lara.guarch, fiona.trahe Cc: dev, brendan.ryan, mairtin.oloingsigh, David Coyle This patch makes some improvements to the security instance setup for the AESNI-MB PMD, as follows: - fix potential memory leak where the security instance was not freed if an error occurred later in the device creation - tidy-up security instance initialization code by moving it all, including enabling the RTE_CRYPTODEV_FF_SECURITY feature, into one '#ifdef AESNI_MB_DOCSIS_SEC_ENABLED' block Fixes: fda5216fba55 ("crypto/aesni_mb: support DOCSIS protocol") Signed-off-by: David Coyle <david.coyle@intel.com> --- drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c index b54c57f86..1bddbcf74 100644 --- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c +++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c @@ -1881,9 +1881,6 @@ cryptodev_aesni_mb_create(const char *name, struct aesni_mb_private *internals; enum aesni_mb_vector_mode vector_mode; MB_MGR *mb_mgr; -#ifdef AESNI_MB_DOCSIS_SEC_ENABLED - struct rte_security_ctx *security_instance; -#endif dev = rte_cryptodev_pmd_create(name, &vdev->device, init_params); if (dev == NULL) { @@ -1912,13 +1909,10 @@ cryptodev_aesni_mb_create(const char *name, RTE_CRYPTODEV_FF_SYM_OPERATION_CHAINING | RTE_CRYPTODEV_FF_OOP_LB_IN_LB_OUT | RTE_CRYPTODEV_FF_SYM_CPU_CRYPTO | - RTE_CRYPTODEV_FF_SYM_SESSIONLESS -#ifdef AESNI_MB_DOCSIS_SEC_ENABLED - | RTE_CRYPTODEV_FF_SECURITY -#endif - ; + RTE_CRYPTODEV_FF_SYM_SESSIONLESS; #ifdef AESNI_MB_DOCSIS_SEC_ENABLED + struct rte_security_ctx *security_instance; security_instance = rte_malloc("aesni_mb_sec", sizeof(struct rte_security_ctx), RTE_CACHE_LINE_SIZE); @@ -1932,6 +1926,7 @@ cryptodev_aesni_mb_create(const char *name, security_instance->ops = rte_aesni_mb_pmd_sec_ops; security_instance->sess_cnt = 0; dev->security_ctx = security_instance; + dev->feature_flags |= RTE_CRYPTODEV_FF_SECURITY; #endif /* Check CPU for support for AES instruction set */ @@ -1944,6 +1939,10 @@ cryptodev_aesni_mb_create(const char *name, mb_mgr = alloc_init_mb_mgr(vector_mode); if (mb_mgr == NULL) { +#ifdef AESNI_MB_DOCSIS_SEC_ENABLED + rte_free(dev->security_ctx); + dev->security_ctx = NULL; +#endif rte_cryptodev_pmd_destroy(dev); return -ENOMEM; } @@ -2011,8 +2010,9 @@ cryptodev_aesni_mb_remove(struct rte_vdev_device *vdev) RTE_PER_LCORE(sync_mb_mgr) = NULL; } -#ifdef RTE_LIBRTE_SECURITY +#ifdef AESNI_MB_DOCSIS_SEC_ENABLED rte_free(cryptodev->security_ctx); + cryptodev->security_ctx = NULL; #endif return rte_cryptodev_pmd_destroy(cryptodev); -- 2.17.1 ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v2 2/2] crypto/aesni-mb: improve security instance setup 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 2/2] crypto/aesni-mb: " David Coyle @ 2020-07-22 7:58 ` De Lara Guarch, Pablo 0 siblings, 0 replies; 14+ messages in thread From: De Lara Guarch, Pablo @ 2020-07-22 7:58 UTC (permalink / raw) To: Coyle, David, akhil.goyal, Doherty, Declan, Trahe, Fiona Cc: dev, Ryan, Brendan, O'loingsigh, Mairtin > -----Original Message----- > From: Coyle, David <david.coyle@intel.com> > Sent: Monday, July 20, 2020 1:16 PM > To: akhil.goyal@nxp.com; Doherty, Declan <declan.doherty@intel.com>; De > Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona > <fiona.trahe@intel.com> > Cc: dev@dpdk.org; Ryan, Brendan <brendan.ryan@intel.com>; O'loingsigh, > Mairtin <mairtin.oloingsigh@intel.com>; Coyle, David <david.coyle@intel.com> > Subject: [PATCH v2 2/2] crypto/aesni-mb: improve security instance setup > > This patch makes some improvements to the security instance setup for the > AESNI-MB PMD, as follows: > - fix potential memory leak where the security instance was not freed if > an error occurred later in the device creation > - tidy-up security instance initialization code by moving it all, > including enabling the RTE_CRYPTODEV_FF_SECURITY feature, into one > '#ifdef AESNI_MB_DOCSIS_SEC_ENABLED' block > > Fixes: fda5216fba55 ("crypto/aesni_mb: support DOCSIS protocol") > > Signed-off-by: David Coyle <david.coyle@intel.com> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [dpdk-dev] [PATCH v2 0/2] improve security instance setup 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 0/2] " David Coyle 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 1/2] crypto/qat: " David Coyle 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 2/2] crypto/aesni-mb: " David Coyle @ 2020-07-26 19:08 ` Akhil Goyal 2 siblings, 0 replies; 14+ messages in thread From: Akhil Goyal @ 2020-07-26 19:08 UTC (permalink / raw) To: David Coyle, declan.doherty, pablo.de.lara.guarch, fiona.trahe Cc: dev, brendan.ryan, mairtin.oloingsigh > > These patches make some improvements to the security instance setup for > the QAT SYM and AESNI-MB PMDs. > Series Applied to dpdk-next-crypto Thanks. ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2020-07-26 19:08 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-07-16 15:35 [dpdk-dev] [PATCH v1 0/2] improve security instance setup David Coyle 2020-07-16 15:35 ` [dpdk-dev] [PATCH v1 1/2] crypto/qat: " David Coyle 2020-07-17 18:20 ` Trahe, Fiona 2020-07-18 21:36 ` Akhil Goyal 2020-07-18 21:40 ` Akhil Goyal 2020-07-20 12:39 ` Coyle, David 2020-07-16 15:36 ` [dpdk-dev] [PATCH v1 2/2] crypto/aesni_mb: " David Coyle 2020-07-17 19:29 ` De Lara Guarch, Pablo 2020-07-20 12:38 ` Coyle, David 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 0/2] " David Coyle 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 1/2] crypto/qat: " David Coyle 2020-07-20 12:16 ` [dpdk-dev] [PATCH v2 2/2] crypto/aesni-mb: " David Coyle 2020-07-22 7:58 ` De Lara Guarch, Pablo 2020-07-26 19:08 ` [dpdk-dev] [PATCH v2 0/2] " Akhil Goyal
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).