| Message ID | 20211013192222.1582631-1-gakhil@marvell.com (mailing list archive) |
|---|---|
| Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@inbox.dpdk.org Delivered-To: patchwork@inbox.dpdk.org Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 452BCA0C55; Wed, 13 Oct 2021 21:22:42 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 0C7DF41173; Wed, 13 Oct 2021 21:22:42 +0200 (CEST) Received: from mx0b-0016f401.pphosted.com (mx0b-0016f401.pphosted.com [67.231.156.173]) by mails.dpdk.org (Postfix) with ESMTP id 48CF64111B for <dev@dpdk.org>; Wed, 13 Oct 2021 21:22:40 +0200 (CEST) Received: from pps.filterd (m0045851.ppops.net [127.0.0.1]) by mx0b-0016f401.pphosted.com (8.16.1.2/8.16.1.2) with SMTP id 19DIvIen030386; Wed, 13 Oct 2021 12:22:34 -0700 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=marvell.com; h=from : to : cc : subject : date : message-id : in-reply-to : references : mime-version : content-transfer-encoding : content-type; s=pfpt0220; bh=FeuUp4zAOkv9OMpJok7TkUlreL/5iIuKjsGE8Sufd0g=; b=jHvil41SAoNDUqqHj2JoBb3UwOZly+6ya9KrLeoNl5+PNPfRLN+2KwxH98d7OK6tm5sQ S775rus1zsHZu3oUYbqMfFWMbi+lt0dJ0oAUzHyWt346yyeFXLstn4YUw52tpeOsDANN PBKmuzV6Z4BW8j0m8Z4Kwlx+B6a1MmeSbI5raks5y+Z/5I9LPzsbLVhstnn+Sbprv7kr 9sV1BtnkOC8NFJrp/a3Hx86Dk7jswZAjdwLcD0/4bZAxqJaO6tbJkWjKN3ZsZ6zg6w5y cTISq+4TNV6usEymbrtQ8bemMuCjXRmv6nGxof7iDTU9q0rNTNrQOF06nEYzcrvVHyMD dw== Received: from dc5-exch01.marvell.com ([199.233.59.181]) by mx0b-0016f401.pphosted.com with ESMTP id 3bp0h3sjx6-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-SHA384 bits=256 verify=NOT); Wed, 13 Oct 2021 12:22:34 -0700 Received: from DC5-EXCH01.marvell.com (10.69.176.38) by DC5-EXCH01.marvell.com (10.69.176.38) with Microsoft SMTP Server (TLS) id 15.0.1497.18; Wed, 13 Oct 2021 12:22:32 -0700 Received: from maili.marvell.com (10.69.176.80) by DC5-EXCH01.marvell.com (10.69.176.38) with Microsoft SMTP Server id 15.0.1497.18 via Frontend Transport; Wed, 13 Oct 2021 12:22:32 -0700 Received: from localhost.localdomain (unknown [10.28.36.185]) by maili.marvell.com (Postfix) with ESMTP id 38C753F7082; Wed, 13 Oct 2021 12:22:25 -0700 (PDT) From: Akhil Goyal <gakhil@marvell.com> To: <dev@dpdk.org> CC: <thomas@monjalon.net>, <david.marchand@redhat.com>, <hemant.agrawal@nxp.com>, <anoobj@marvell.com>, <pablo.de.lara.guarch@intel.com>, <fiona.trahe@intel.com>, <declan.doherty@intel.com>, <matan@nvidia.com>, <g.singh@nxp.com>, <roy.fan.zhang@intel.com>, <jianjay.zhou@huawei.com>, <asomalap@amd.com>, <ruifeng.wang@arm.com>, <konstantin.ananyev@intel.com>, <radu.nicolau@intel.com>, <ajit.khaparde@broadcom.com>, <rnagadheeraj@marvell.com>, <adwivedi@marvell.com>, <ciara.power@intel.com>, <haiyue.wang@intel.com>, <jiawenwu@trustnetic.com>, <jianwang@trustnetic.com>, Akhil Goyal <gakhil@marvell.com> Date: Thu, 14 Oct 2021 00:52:15 +0530 Message-ID: <20211013192222.1582631-1-gakhil@marvell.com> X-Mailer: git-send-email 2.25.1 In-Reply-To: <20210930145014.2476799-1-gakhil@marvell.com> References: <20210930145014.2476799-1-gakhil@marvell.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain X-Proofpoint-GUID: Om4QlXfaD1zzpSHFKLTgnUV_2TTbWmrW X-Proofpoint-ORIG-GUID: Om4QlXfaD1zzpSHFKLTgnUV_2TTbWmrW X-Proofpoint-Virus-Version: vendor=baseguard engine=ICAP:2.0.182.1,Aquarius:18.0.790,Hydra:6.0.425,FMLib:17.0.607.475 definitions=2021-10-13_07,2021-10-13_02,2020-04-07_01 Subject: [dpdk-dev] [PATCH v2 0/7] crypto/security session framework rework X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions <dev.dpdk.org> List-Unsubscribe: <https://mails.dpdk.org/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://mails.dpdk.org/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <https://mails.dpdk.org/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
| Series |
crypto/security session framework rework
|
|
Message
Akhil Goyal
Oct. 13, 2021, 7:22 p.m. UTC
As discussed in last release deprecation notice, crypto and security session framework are reworked to reduce the need of two mempool objects and remove the requirement to expose the rte_security_session and rte_cryptodev_sym_session structures. Design methodology is explained in the patch description. Similar work will need to be done for asymmetric sessions as well. Asymmetric session need another rework and is postponed to next release. Since it is still in experimental stage, we can modify the APIs in next release as well. The patches are compilable with all affected PMDs and tested with dpdk-test and ipsec-secgw app on CN9k platform. Changes in v2: - Added new parameter iova in PMD session configure APIs for session priv pointer to be used in QAT/CNXK/etc PMDs. - Hide rte_cryptodev_sym_session and rte_security_session structs. - Added compilation workaround for net PMDs(ixgbe/txgbe) for inline ipsec. Patches with actual fix is beynd the scope of this patchset. - Added inline APIs to access the opaque data and fast metadata. - Remove commented code. TODO - Release notes/deprecation notice removal. - Documentation updates. - Asym APIs - postponed for next release. Akhil Goyal (7): security: rework session framework security: hide security session struct net/cnxk: rework security session framework security: pass session iova in PMD sess create cryptodev: rework session framework cryptodev: hide sym session structure cryptodev: pass session iova in configure session app/test-crypto-perf/cperf.h | 1 - app/test-crypto-perf/cperf_ops.c | 46 ++-- app/test-crypto-perf/cperf_ops.h | 6 +- app/test-crypto-perf/cperf_test_latency.c | 5 +- app/test-crypto-perf/cperf_test_latency.h | 1 - .../cperf_test_pmd_cyclecount.c | 7 +- .../cperf_test_pmd_cyclecount.h | 1 - app/test-crypto-perf/cperf_test_throughput.c | 5 +- app/test-crypto-perf/cperf_test_throughput.h | 1 - app/test-crypto-perf/cperf_test_verify.c | 5 +- app/test-crypto-perf/cperf_test_verify.h | 1 - app/test-crypto-perf/main.c | 29 +-- app/test/test_cryptodev.c | 147 ++++--------- app/test/test_cryptodev.h | 1 - app/test/test_cryptodev_asym.c | 1 - app/test/test_cryptodev_blockcipher.c | 6 +- app/test/test_event_crypto_adapter.c | 28 +-- app/test/test_ipsec.c | 34 +-- app/test/test_ipsec_perf.c | 4 +- app/test/test_security.c | 196 ++++-------------- drivers/crypto/aesni_gcm/aesni_gcm_pmd_ops.c | 33 +-- drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c | 5 +- .../crypto/aesni_mb/rte_aesni_mb_pmd_ops.c | 65 ++---- drivers/crypto/armv8/rte_armv8_pmd_ops.c | 34 +-- drivers/crypto/bcmfs/bcmfs_sym_session.c | 36 +--- drivers/crypto/bcmfs/bcmfs_sym_session.h | 6 +- drivers/crypto/caam_jr/caam_jr.c | 65 ++---- drivers/crypto/ccp/ccp_pmd_ops.c | 32 +-- drivers/crypto/cnxk/cn10k_cryptodev_ops.c | 24 +-- drivers/crypto/cnxk/cn10k_ipsec.c | 53 +---- drivers/crypto/cnxk/cn9k_cryptodev_ops.c | 20 +- drivers/crypto/cnxk/cn9k_ipsec.c | 75 +++---- drivers/crypto/cnxk/cnxk_cryptodev_ops.c | 61 ++---- drivers/crypto/cnxk/cnxk_cryptodev_ops.h | 14 +- drivers/crypto/dpaa2_sec/dpaa2_sec_dpseci.c | 69 ++---- drivers/crypto/dpaa_sec/dpaa_sec.c | 67 ++---- drivers/crypto/kasumi/rte_kasumi_pmd_ops.c | 34 +-- drivers/crypto/mlx5/mlx5_crypto.c | 25 +-- drivers/crypto/mvsam/rte_mrvl_pmd.c | 3 +- drivers/crypto/mvsam/rte_mrvl_pmd_ops.c | 48 +---- drivers/crypto/nitrox/nitrox_sym.c | 33 +-- drivers/crypto/null/null_crypto_pmd_ops.c | 34 +-- .../crypto/octeontx/otx_cryptodev_hw_access.h | 1 - drivers/crypto/octeontx/otx_cryptodev_ops.c | 68 +++--- drivers/crypto/octeontx2/otx2_cryptodev_ops.c | 63 +++--- .../octeontx2/otx2_cryptodev_ops_helper.h | 16 +- drivers/crypto/octeontx2/otx2_cryptodev_sec.c | 77 +++---- drivers/crypto/openssl/rte_openssl_pmd_ops.c | 35 +--- drivers/crypto/qat/qat_sym.c | 3 +- drivers/crypto/qat/qat_sym.h | 8 +- drivers/crypto/qat/qat_sym_session.c | 66 ++---- drivers/crypto/qat/qat_sym_session.h | 14 +- drivers/crypto/scheduler/scheduler_pmd_ops.c | 10 +- drivers/crypto/snow3g/rte_snow3g_pmd_ops.c | 34 +-- drivers/crypto/virtio/virtio_cryptodev.c | 32 +-- drivers/crypto/zuc/rte_zuc_pmd_ops.c | 35 +--- .../octeontx2/otx2_evdev_crypto_adptr_rx.h | 3 +- drivers/net/cnxk/cn10k_ethdev_sec.c | 64 +++--- drivers/net/cnxk/cn9k_ethdev_sec.c | 59 ++---- drivers/net/cnxk/cnxk_ethdev.c | 6 +- drivers/net/cnxk/cnxk_ethdev.h | 6 - drivers/net/cnxk/cnxk_ethdev_sec.c | 21 -- drivers/net/ixgbe/ixgbe_ipsec.c | 38 +--- drivers/net/octeontx2/otx2_ethdev_sec.c | 52 ++--- drivers/net/octeontx2/otx2_ethdev_sec_tx.h | 2 +- drivers/net/txgbe/txgbe_ipsec.c | 38 +--- examples/fips_validation/fips_dev_self_test.c | 32 +-- examples/fips_validation/main.c | 20 +- examples/ipsec-secgw/ipsec-secgw.c | 40 ---- examples/ipsec-secgw/ipsec.c | 12 +- examples/ipsec-secgw/ipsec.h | 1 - examples/ipsec-secgw/ipsec_worker.c | 4 - examples/l2fwd-crypto/main.c | 41 +--- examples/vhost_crypto/main.c | 16 +- lib/cryptodev/cryptodev_pmd.h | 33 ++- lib/cryptodev/rte_crypto.h | 2 +- lib/cryptodev/rte_crypto_sym.h | 2 +- lib/cryptodev/rte_cryptodev.c | 88 +++++--- lib/cryptodev/rte_cryptodev.h | 70 +++---- lib/cryptodev/rte_cryptodev_trace.h | 16 +- lib/ipsec/rte_ipsec.h | 4 +- lib/ipsec/rte_ipsec_group.h | 13 +- lib/ipsec/ses.c | 6 +- lib/pipeline/rte_table_action.c | 8 +- lib/pipeline/rte_table_action.h | 2 +- lib/security/rte_security.c | 32 +-- lib/security/rte_security.h | 85 +++++--- lib/security/rte_security_driver.h | 31 ++- lib/vhost/rte_vhost_crypto.h | 3 - lib/vhost/vhost_crypto.c | 7 +- 90 files changed, 816 insertions(+), 1864 deletions(-)
Comments
> Subject: [PATCH v2 0/7] crypto/security session framework rework > > As discussed in last release deprecation notice, > crypto and security session framework are reworked > to reduce the need of two mempool objects and > remove the requirement to expose the rte_security_session > and rte_cryptodev_sym_session structures. > Design methodology is explained in the patch description. > > Similar work will need to be done for asymmetric sessions > as well. Asymmetric session need another rework and is > postponed to next release. Since it is still in experimental > stage, we can modify the APIs in next release as well. > > The patches are compilable with all affected PMDs > and tested with dpdk-test and ipsec-secgw app on CN9k platform. > > Changes in v2: > - Added new parameter iova in PMD session configure APIs for > session priv pointer to be used in QAT/CNXK/etc PMDs. > - Hide rte_cryptodev_sym_session and rte_security_session structs. > - Added compilation workaround for net PMDs(ixgbe/txgbe) > for inline ipsec. > Patches with actual fix is beynd the scope of this patchset. > - Added inline APIs to access the opaque data and fast metadata. > - Remove commented code. > TODO > - Release notes/deprecation notice removal. > - Documentation updates. > - Asym APIs - postponed for next release. > > Akhil Goyal (7): > security: rework session framework > security: hide security session struct > net/cnxk: rework security session framework > security: pass session iova in PMD sess create > cryptodev: rework session framework > cryptodev: hide sym session structure > cryptodev: pass session iova in configure session > The series is rebased over following to avoid merge conflicts http://patches.dpdk.org/user/todo/dpdk/?series=19519 http://patches.dpdk.org/user/todo/dpdk/?series=19467 http://patches.dpdk.org/user/todo/dpdk/?series=19551 (HEAD -> next-crypto) cryptodev: pass session iova in configure session cryptodev: hide sym session structure cryptodev: rework session framework security: pass session iova in PMD sess create net/cnxk: rework security session framework security: hide security session struct security: rework session framework cryptodev: move device specific structures cryptodev: update fast path APIs to use new flat array cryptodev: move inline APIs into separate structure cryptodev: allocate max space for internal qp array cryptodev: separate out internal structures security: add reserved bitfields security: hide internal API cryptodev: remove LIST_END enumerators test/crypto-perf: support lookaside IPsec
Hi Akhil, > -----Original Message----- > From: Akhil Goyal <gakhil@marvell.com> > Sent: Thursday, October 14, 2021 12:48 PM > To: Akhil Goyal <gakhil@marvell.com>; dev@dpdk.org > Cc: thomas@monjalon.net; david.marchand@redhat.com; > hemant.agrawal@nxp.com; Anoob Joseph <anoobj@marvell.com>; De Lara > Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona > <fiona.trahe@intel.com>; Doherty, Declan <declan.doherty@intel.com>; > matan@nvidia.com; g.singh@nxp.com; Zhang, Roy Fan > <roy.fan.zhang@intel.com>; jianjay.zhou@huawei.com; > asomalap@amd.com; ruifeng.wang@arm.com; Ananyev, Konstantin > <konstantin.ananyev@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>; > ajit.khaparde@broadcom.com; Nagadheeraj Rottela > <rnagadheeraj@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>; > Power, Ciara <ciara.power@intel.com>; Wang, Haiyue > <haiyue.wang@intel.com>; jiawenwu@trustnetic.com; > jianwang@trustnetic.com > Subject: RE: [PATCH v2 0/7] crypto/security session framework rework ... > The series is rebased over following to avoid merge conflicts > http://patches.dpdk.org/user/todo/dpdk/?series=19519 The series 19519 will make multi-process case not working for all crypto PMDs. Please provide a solution first. > http://patches.dpdk.org/user/todo/dpdk/?series=19467 > http://patches.dpdk.org/user/todo/dpdk/?series=19551 > > (HEAD -> next-crypto) cryptodev: pass session iova in configure session > cryptodev: hide sym session structure > cryptodev: rework session framework > security: pass session iova in PMD sess create > net/cnxk: rework security session framework > security: hide security session struct > security: rework session framework > cryptodev: move device specific structures > cryptodev: update fast path APIs to use new flat array > cryptodev: move inline APIs into separate structure > cryptodev: allocate max space for internal qp array > cryptodev: separate out internal structures > security: add reserved bitfields > security: hide internal API > cryptodev: remove LIST_END enumerators > test/crypto-perf: support lookaside IPsec
> Hi Akhil, > > The series 19519 will make multi-process case not working for all crypto > PMDs. > Please provide a solution first. > Yes I am working on it, will update the series asap.
Hi, Unfortunately the patches still cause seg-fault at QAT and SW PMDs. - for qat it fails at rte_security_ops->session_size_get not implemented. - for sw pmds the queue pair's session private mempools are not set. Regards, Fan > -----Original Message----- > From: Akhil Goyal <gakhil@marvell.com> > Sent: Thursday, October 14, 2021 12:48 PM > To: Akhil Goyal <gakhil@marvell.com>; dev@dpdk.org > Cc: thomas@monjalon.net; david.marchand@redhat.com; > hemant.agrawal@nxp.com; Anoob Joseph <anoobj@marvell.com>; De Lara > Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona > <fiona.trahe@intel.com>; Doherty, Declan <declan.doherty@intel.com>; > matan@nvidia.com; g.singh@nxp.com; Zhang, Roy Fan > <roy.fan.zhang@intel.com>; jianjay.zhou@huawei.com; > asomalap@amd.com; ruifeng.wang@arm.com; Ananyev, Konstantin > <konstantin.ananyev@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>; > ajit.khaparde@broadcom.com; Nagadheeraj Rottela > <rnagadheeraj@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>; > Power, Ciara <ciara.power@intel.com>; Wang, Haiyue > <haiyue.wang@intel.com>; jiawenwu@trustnetic.com; > jianwang@trustnetic.com > Subject: RE: [PATCH v2 0/7] crypto/security session framework rework > > > Subject: [PATCH v2 0/7] crypto/security session framework rework > > > > As discussed in last release deprecation notice, > > crypto and security session framework are reworked > > to reduce the need of two mempool objects and > > remove the requirement to expose the rte_security_session > > and rte_cryptodev_sym_session structures. > > Design methodology is explained in the patch description. > > > > Similar work will need to be done for asymmetric sessions > > as well. Asymmetric session need another rework and is > > postponed to next release. Since it is still in experimental > > stage, we can modify the APIs in next release as well. > > > > The patches are compilable with all affected PMDs > > and tested with dpdk-test and ipsec-secgw app on CN9k platform. > > > > Changes in v2: > > - Added new parameter iova in PMD session configure APIs for > > session priv pointer to be used in QAT/CNXK/etc PMDs. > > - Hide rte_cryptodev_sym_session and rte_security_session structs. > > - Added compilation workaround for net PMDs(ixgbe/txgbe) > > for inline ipsec. > > Patches with actual fix is beynd the scope of this patchset. > > - Added inline APIs to access the opaque data and fast metadata. > > - Remove commented code. > > TODO > > - Release notes/deprecation notice removal. > > - Documentation updates. > > - Asym APIs - postponed for next release. > > > > Akhil Goyal (7): > > security: rework session framework > > security: hide security session struct > > net/cnxk: rework security session framework > > security: pass session iova in PMD sess create > > cryptodev: rework session framework > > cryptodev: hide sym session structure > > cryptodev: pass session iova in configure session > > > The series is rebased over following to avoid merge conflicts > http://patches.dpdk.org/user/todo/dpdk/?series=19519 > http://patches.dpdk.org/user/todo/dpdk/?series=19467 > http://patches.dpdk.org/user/todo/dpdk/?series=19551 > > (HEAD -> next-crypto) cryptodev: pass session iova in configure session > cryptodev: hide sym session structure > cryptodev: rework session framework > security: pass session iova in PMD sess create > net/cnxk: rework security session framework > security: hide security session struct > security: rework session framework > cryptodev: move device specific structures > cryptodev: update fast path APIs to use new flat array > cryptodev: move inline APIs into separate structure > cryptodev: allocate max space for internal qp array > cryptodev: separate out internal structures > security: add reserved bitfields > security: hide internal API > cryptodev: remove LIST_END enumerators > test/crypto-perf: support lookaside IPsec
Hi Fan, > > Unfortunately the patches still cause seg-fault at QAT and SW PMDs. > > - for qat it fails at rte_security_ops->session_size_get not implemented. > - for sw pmds the queue pair's session private mempools are not set. > Can you check if below change works for Kasumi. I will replicate for others. diff --git a/drivers/crypto/kasumi/kasumi_pmd_private.h b/drivers/crypto/kasumi/kasumi_pmd_private.h index abedcd616d..fe0e78e516 100644 --- a/drivers/crypto/kasumi/kasumi_pmd_private.h +++ b/drivers/crypto/kasumi/kasumi_pmd_private.h @@ -38,8 +38,6 @@ struct kasumi_qp { /**< Ring for placing processed ops */ struct rte_mempool *sess_mp; /**< Session Mempool */ - struct rte_mempool *sess_mp_priv; - /**< Session Private Data Mempool */ struct rte_cryptodev_stats qp_stats; /**< Queue pair statistics */ uint8_t temp_digest[KASUMI_DIGEST_LENGTH]; diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c b/drivers/crypto/kasumi/rte_kasumi_pmd.c index d6f927417a..1fc59c8b8a 100644 --- a/drivers/crypto/kasumi/rte_kasumi_pmd.c +++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c @@ -139,27 +139,24 @@ kasumi_get_session(struct kasumi_qp *qp, struct rte_crypto_op *op) op->sym->session, cryptodev_driver_id); } else { - void *_sess = NULL; - void *_sess_private_data = NULL; + struct rte_cryptodev_sym_session *_sess = NULL; - if (rte_mempool_get(qp->sess_mp, (void **)&_sess)) + /* Create temporary session */ + _sess = rte_cryptodev_sym_session_create(qp->sess_mp); + if (_sess == NULL) return NULL; - if (rte_mempool_get(qp->sess_mp_priv, - (void **)&_sess_private_data)) - return NULL; - - sess = (struct kasumi_session *)_sess_private_data; - + _sess->sess_data[cryptodev_driver_id].data = + (void *)((uint8_t *)_sess + + rte_cryptodev_sym_get_header_session_size() + + (cryptodev_driver_id * _sess->priv_sz)); + sess = _sess->sess_data[cryptodev_driver_id].data; if (unlikely(kasumi_set_session_parameters(qp->mgr, sess, op->sym->xform) != 0)) { rte_mempool_put(qp->sess_mp, _sess); - rte_mempool_put(qp->sess_mp_priv, _sess_private_data); sess = NULL; } op->sym->session = (struct rte_cryptodev_sym_session *)_sess; - set_sym_session_private_data(op->sym->session, - cryptodev_driver_id, _sess_private_data); } if (unlikely(sess == NULL)) @@ -327,7 +324,6 @@ process_ops(struct rte_crypto_op **ops, struct kasumi_session *session, memset(ops[i]->sym->session, 0, rte_cryptodev_sym_get_existing_header_session_size( ops[i]->sym->session)); - rte_mempool_put(qp->sess_mp_priv, session); rte_mempool_put(qp->sess_mp, ops[i]->sym->session); ops[i]->sym->session = NULL; }
Hi Fan, Check for below QAT fix also > > > > Unfortunately the patches still cause seg-fault at QAT and SW PMDs. > > > > - for qat it fails at rte_security_ops->session_size_get not implemented. And for this one diff --git a/drivers/crypto/qat/qat_sym_pmd.c b/drivers/crypto/qat/qat_sym_pmd.c index efda921c05..96cd9d2eee 100644 --- a/drivers/crypto/qat/qat_sym_pmd.c +++ b/drivers/crypto/qat/qat_sym_pmd.c @@ -306,6 +306,7 @@ static struct rte_security_ops security_qat_ops = { .session_create = qat_security_session_create, .session_update = NULL, + .session_get_size = qat_security_session_get_size, .session_stats_get = NULL, .session_destroy = qat_security_session_destroy, .set_pkt_metadata = NULL, diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c index ef92f22c1a..41b5542343 100644 --- a/drivers/crypto/qat/qat_sym_session.c +++ b/drivers/crypto/qat/qat_sym_session.c @@ -2297,4 +2297,10 @@ qat_security_session_destroy(void *dev __rte_unused, void *sess_priv) } return 0; } + +static unsigned int +qat_security_session_get_size(void *device __rte_unused) +{ + return sizeof(struct qat_sym_session); +} #endif > > - for sw pmds the queue pair's session private mempools are not set. > > > Can you check if below change works for Kasumi. I will replicate for others. > > diff --git a/drivers/crypto/kasumi/kasumi_pmd_private.h > b/drivers/crypto/kasumi/kasumi_pmd_private.h > index abedcd616d..fe0e78e516 100644 > --- a/drivers/crypto/kasumi/kasumi_pmd_private.h > +++ b/drivers/crypto/kasumi/kasumi_pmd_private.h > @@ -38,8 +38,6 @@ struct kasumi_qp { > /**< Ring for placing processed ops */ > struct rte_mempool *sess_mp; > /**< Session Mempool */ > - struct rte_mempool *sess_mp_priv; > - /**< Session Private Data Mempool */ > struct rte_cryptodev_stats qp_stats; > /**< Queue pair statistics */ > uint8_t temp_digest[KASUMI_DIGEST_LENGTH]; > diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c > b/drivers/crypto/kasumi/rte_kasumi_pmd.c > index d6f927417a..1fc59c8b8a 100644 > --- a/drivers/crypto/kasumi/rte_kasumi_pmd.c > +++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c > @@ -139,27 +139,24 @@ kasumi_get_session(struct kasumi_qp *qp, struct > rte_crypto_op *op) > op->sym->session, > cryptodev_driver_id); > } else { > - void *_sess = NULL; > - void *_sess_private_data = NULL; > + struct rte_cryptodev_sym_session *_sess = NULL; > > - if (rte_mempool_get(qp->sess_mp, (void **)&_sess)) > + /* Create temporary session */ > + _sess = rte_cryptodev_sym_session_create(qp->sess_mp); > + if (_sess == NULL) > return NULL; > > - if (rte_mempool_get(qp->sess_mp_priv, > - (void **)&_sess_private_data)) > - return NULL; > - > - sess = (struct kasumi_session *)_sess_private_data; > - > + _sess->sess_data[cryptodev_driver_id].data = > + (void *)((uint8_t *)_sess + > + rte_cryptodev_sym_get_header_session_size() + > + (cryptodev_driver_id * _sess->priv_sz)); > + sess = _sess->sess_data[cryptodev_driver_id].data; > if (unlikely(kasumi_set_session_parameters(qp->mgr, sess, > op->sym->xform) != 0)) { > rte_mempool_put(qp->sess_mp, _sess); > - rte_mempool_put(qp->sess_mp_priv, _sess_private_data); > sess = NULL; > } > op->sym->session = (struct rte_cryptodev_sym_session *)_sess; > - set_sym_session_private_data(op->sym->session, > - cryptodev_driver_id, _sess_private_data); > } > > if (unlikely(sess == NULL)) > @@ -327,7 +324,6 @@ process_ops(struct rte_crypto_op **ops, struct > kasumi_session *session, > memset(ops[i]->sym->session, 0, > rte_cryptodev_sym_get_existing_header_session_size( > ops[i]->sym->session)); > - rte_mempool_put(qp->sess_mp_priv, session); > rte_mempool_put(qp->sess_mp, ops[i]->sym->session); > ops[i]->sym->session = NULL; > }
Hi Akhil, It shall work but Kasumi tests are passing :-) It is snow3g and aesni-mb/gcm that are failing. Thanks Regards, Fan > -----Original Message----- > From: Akhil Goyal <gakhil@marvell.com> > Sent: Thursday, October 14, 2021 7:24 PM > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org > Cc: thomas@monjalon.net; david.marchand@redhat.com; > hemant.agrawal@nxp.com; Anoob Joseph <anoobj@marvell.com>; De Lara > Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona > <fiona.trahe@intel.com>; Doherty, Declan <declan.doherty@intel.com>; > matan@nvidia.com; g.singh@nxp.com; jianjay.zhou@huawei.com; > asomalap@amd.com; ruifeng.wang@arm.com; Ananyev, Konstantin > <konstantin.ananyev@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>; > ajit.khaparde@broadcom.com; Nagadheeraj Rottela > <rnagadheeraj@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>; > Power, Ciara <ciara.power@intel.com>; Wang, Haiyue > <haiyue.wang@intel.com>; jiawenwu@trustnetic.com; > jianwang@trustnetic.com > Subject: RE: [PATCH v2 0/7] crypto/security session framework rework > > Hi Fan, > > > > Unfortunately the patches still cause seg-fault at QAT and SW PMDs. > > > > - for qat it fails at rte_security_ops->session_size_get not implemented. > > - for sw pmds the queue pair's session private mempools are not set. > > > Can you check if below change works for Kasumi. I will replicate for others. > > diff --git a/drivers/crypto/kasumi/kasumi_pmd_private.h > b/drivers/crypto/kasumi/kasumi_pmd_private.h > index abedcd616d..fe0e78e516 100644 > --- a/drivers/crypto/kasumi/kasumi_pmd_private.h > +++ b/drivers/crypto/kasumi/kasumi_pmd_private.h > @@ -38,8 +38,6 @@ struct kasumi_qp { > /**< Ring for placing processed ops */ > struct rte_mempool *sess_mp; > /**< Session Mempool */ > - struct rte_mempool *sess_mp_priv; > - /**< Session Private Data Mempool */ > struct rte_cryptodev_stats qp_stats; > /**< Queue pair statistics */ > uint8_t temp_digest[KASUMI_DIGEST_LENGTH]; > diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c > b/drivers/crypto/kasumi/rte_kasumi_pmd.c > index d6f927417a..1fc59c8b8a 100644 > --- a/drivers/crypto/kasumi/rte_kasumi_pmd.c > +++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c > @@ -139,27 +139,24 @@ kasumi_get_session(struct kasumi_qp *qp, struct > rte_crypto_op *op) > op->sym->session, > cryptodev_driver_id); > } else { > - void *_sess = NULL; > - void *_sess_private_data = NULL; > + struct rte_cryptodev_sym_session *_sess = NULL; > > - if (rte_mempool_get(qp->sess_mp, (void **)&_sess)) > + /* Create temporary session */ > + _sess = rte_cryptodev_sym_session_create(qp->sess_mp); > + if (_sess == NULL) > return NULL; > > - if (rte_mempool_get(qp->sess_mp_priv, > - (void **)&_sess_private_data)) > - return NULL; > - > - sess = (struct kasumi_session *)_sess_private_data; > - > + _sess->sess_data[cryptodev_driver_id].data = > + (void *)((uint8_t *)_sess + > + rte_cryptodev_sym_get_header_session_size() + > + (cryptodev_driver_id * _sess->priv_sz)); > + sess = _sess->sess_data[cryptodev_driver_id].data; > if (unlikely(kasumi_set_session_parameters(qp->mgr, sess, > op->sym->xform) != 0)) { > rte_mempool_put(qp->sess_mp, _sess); > - rte_mempool_put(qp->sess_mp_priv, _sess_private_data); > sess = NULL; > } > op->sym->session = (struct rte_cryptodev_sym_session *)_sess; > - set_sym_session_private_data(op->sym->session, > - cryptodev_driver_id, _sess_private_data); > } > > if (unlikely(sess == NULL)) > @@ -327,7 +324,6 @@ process_ops(struct rte_crypto_op **ops, struct > kasumi_session *session, > memset(ops[i]->sym->session, 0, > rte_cryptodev_sym_get_existing_header_session_size( > ops[i]->sym->session)); > - rte_mempool_put(qp->sess_mp_priv, session); > rte_mempool_put(qp->sess_mp, ops[i]->sym->session); > ops[i]->sym->session = NULL; > }
Hi Akhil, I tried to fix the problems of seg faults. The seg-faults are gone now but all asym tests are failing too. The reason is the rte_cryptodev_queue_pair_setup() checks the session mempool same for sym and asym. Since we don't have a rte_cryptodev_asym_session_pool_create() the session mempool created by test_cryptodev_asym.c with rte_mempool_create() will fail the mempool check when setting up the queue pair. If you think my fix may be useful (although not resolving asym issue) I can send it. Regards, Fan > -----Original Message----- > From: Akhil Goyal <gakhil@marvell.com> > Sent: Thursday, October 14, 2021 7:57 PM > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org > Cc: thomas@monjalon.net; david.marchand@redhat.com; > hemant.agrawal@nxp.com; Anoob Joseph <anoobj@marvell.com>; De Lara > Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona > <fiona.trahe@intel.com>; Doherty, Declan <declan.doherty@intel.com>; > matan@nvidia.com; g.singh@nxp.com; jianjay.zhou@huawei.com; > asomalap@amd.com; ruifeng.wang@arm.com; Ananyev, Konstantin > <konstantin.ananyev@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>; > ajit.khaparde@broadcom.com; Nagadheeraj Rottela > <rnagadheeraj@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>; > Power, Ciara <ciara.power@intel.com>; Wang, Haiyue > <haiyue.wang@intel.com>; jiawenwu@trustnetic.com; > jianwang@trustnetic.com > Subject: RE: [PATCH v2 0/7] crypto/security session framework rework > > Hi Fan, > Check for below QAT fix also > > > > > > Unfortunately the patches still cause seg-fault at QAT and SW PMDs. > > > > > > - for qat it fails at rte_security_ops->session_size_get not implemented. > And for this one > diff --git a/drivers/crypto/qat/qat_sym_pmd.c > b/drivers/crypto/qat/qat_sym_pmd.c > index efda921c05..96cd9d2eee 100644 > --- a/drivers/crypto/qat/qat_sym_pmd.c > +++ b/drivers/crypto/qat/qat_sym_pmd.c > @@ -306,6 +306,7 @@ static struct rte_security_ops security_qat_ops = { > > .session_create = qat_security_session_create, > .session_update = NULL, > + .session_get_size = qat_security_session_get_size, > .session_stats_get = NULL, > .session_destroy = qat_security_session_destroy, > .set_pkt_metadata = NULL, > diff --git a/drivers/crypto/qat/qat_sym_session.c > b/drivers/crypto/qat/qat_sym_session.c > index ef92f22c1a..41b5542343 100644 > --- a/drivers/crypto/qat/qat_sym_session.c > +++ b/drivers/crypto/qat/qat_sym_session.c > @@ -2297,4 +2297,10 @@ qat_security_session_destroy(void *dev > __rte_unused, void *sess_priv) > } > return 0; > } > + > +static unsigned int > +qat_security_session_get_size(void *device __rte_unused) > +{ > + return sizeof(struct qat_sym_session); > +} > #endif > > > > - for sw pmds the queue pair's session private mempools are not set. > > > > > Can you check if below change works for Kasumi. I will replicate for others. > > > > diff --git a/drivers/crypto/kasumi/kasumi_pmd_private.h > > b/drivers/crypto/kasumi/kasumi_pmd_private.h > > index abedcd616d..fe0e78e516 100644 > > --- a/drivers/crypto/kasumi/kasumi_pmd_private.h > > +++ b/drivers/crypto/kasumi/kasumi_pmd_private.h > > @@ -38,8 +38,6 @@ struct kasumi_qp { > > /**< Ring for placing processed ops */ > > struct rte_mempool *sess_mp; > > /**< Session Mempool */ > > - struct rte_mempool *sess_mp_priv; > > - /**< Session Private Data Mempool */ > > struct rte_cryptodev_stats qp_stats; > > /**< Queue pair statistics */ > > uint8_t temp_digest[KASUMI_DIGEST_LENGTH]; > > diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c > > b/drivers/crypto/kasumi/rte_kasumi_pmd.c > > index d6f927417a..1fc59c8b8a 100644 > > --- a/drivers/crypto/kasumi/rte_kasumi_pmd.c > > +++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c > > @@ -139,27 +139,24 @@ kasumi_get_session(struct kasumi_qp *qp, struct > > rte_crypto_op *op) > > op->sym->session, > > cryptodev_driver_id); > > } else { > > - void *_sess = NULL; > > - void *_sess_private_data = NULL; > > + struct rte_cryptodev_sym_session *_sess = NULL; > > > > - if (rte_mempool_get(qp->sess_mp, (void **)&_sess)) > > + /* Create temporary session */ > > + _sess = rte_cryptodev_sym_session_create(qp->sess_mp); > > + if (_sess == NULL) > > return NULL; > > > > - if (rte_mempool_get(qp->sess_mp_priv, > > - (void **)&_sess_private_data)) > > - return NULL; > > - > > - sess = (struct kasumi_session *)_sess_private_data; > > - > > + _sess->sess_data[cryptodev_driver_id].data = > > + (void *)((uint8_t *)_sess + > > + rte_cryptodev_sym_get_header_session_size() + > > + (cryptodev_driver_id * _sess->priv_sz)); > > + sess = _sess->sess_data[cryptodev_driver_id].data; > > if (unlikely(kasumi_set_session_parameters(qp->mgr, sess, > > op->sym->xform) != 0)) { > > rte_mempool_put(qp->sess_mp, _sess); > > - rte_mempool_put(qp->sess_mp_priv, _sess_private_data); > > sess = NULL; > > } > > op->sym->session = (struct rte_cryptodev_sym_session *)_sess; > > - set_sym_session_private_data(op->sym->session, > > - cryptodev_driver_id, _sess_private_data); > > } > > > > if (unlikely(sess == NULL)) > > @@ -327,7 +324,6 @@ process_ops(struct rte_crypto_op **ops, struct > > kasumi_session *session, > > memset(ops[i]->sym->session, 0, > > rte_cryptodev_sym_get_existing_header_session_size( > > ops[i]->sym->session)); > > - rte_mempool_put(qp->sess_mp_priv, session); > > rte_mempool_put(qp->sess_mp, ops[i]->sym->session); > > ops[i]->sym->session = NULL; > > }
> Hi Akhil, > > I tried to fix the problems of seg faults. > The seg-faults are gone now but all asym tests are failing too. > The reason is the rte_cryptodev_queue_pair_setup() checks the session > mempool same for sym and asym. > Since we don't have a rte_cryptodev_asym_session_pool_create() the > session mempool created by > test_cryptodev_asym.c with rte_mempool_create() will fail the mempool > check when setting up the queue pair. > > If you think my fix may be useful (although not resolving asym issue) I can > send it. > Is it a different fix than what I proposed below? If yes, you can send the diff. I already made the below changes for all the PMDs. I will try to fix the asym issue, but I suppose it can be dealt in the app Which can be fixed separately in RC2. Also, found the root cause of multi process issue, working on making the patches. Will send v3 soon with all 3 issues(docsis/mp/sessless) fixed atleast. For Asym, may send a separate patch. > > Hi Fan, > > Check for below QAT fix also > > > > > > > > Unfortunately the patches still cause seg-fault at QAT and SW PMDs. > > > > > > > > - for qat it fails at rte_security_ops->session_size_get not implemented. > > And for this one > > diff --git a/drivers/crypto/qat/qat_sym_pmd.c > > b/drivers/crypto/qat/qat_sym_pmd.c > > index efda921c05..96cd9d2eee 100644 > > --- a/drivers/crypto/qat/qat_sym_pmd.c > > +++ b/drivers/crypto/qat/qat_sym_pmd.c > > @@ -306,6 +306,7 @@ static struct rte_security_ops security_qat_ops = { > > > > .session_create = qat_security_session_create, > > .session_update = NULL, > > + .session_get_size = qat_security_session_get_size, > > .session_stats_get = NULL, > > .session_destroy = qat_security_session_destroy, > > .set_pkt_metadata = NULL, > > diff --git a/drivers/crypto/qat/qat_sym_session.c > > b/drivers/crypto/qat/qat_sym_session.c > > index ef92f22c1a..41b5542343 100644 > > --- a/drivers/crypto/qat/qat_sym_session.c > > +++ b/drivers/crypto/qat/qat_sym_session.c > > @@ -2297,4 +2297,10 @@ qat_security_session_destroy(void *dev > > __rte_unused, void *sess_priv) > > } > > return 0; > > } > > + > > +static unsigned int > > +qat_security_session_get_size(void *device __rte_unused) > > +{ > > + return sizeof(struct qat_sym_session); > > +} > > #endif > > > > > > - for sw pmds the queue pair's session private mempools are not set. > > > > > > > Can you check if below change works for Kasumi. I will replicate for > others. > > > > > > diff --git a/drivers/crypto/kasumi/kasumi_pmd_private.h > > > b/drivers/crypto/kasumi/kasumi_pmd_private.h > > > index abedcd616d..fe0e78e516 100644 > > > --- a/drivers/crypto/kasumi/kasumi_pmd_private.h > > > +++ b/drivers/crypto/kasumi/kasumi_pmd_private.h > > > @@ -38,8 +38,6 @@ struct kasumi_qp { > > > /**< Ring for placing processed ops */ > > > struct rte_mempool *sess_mp; > > > /**< Session Mempool */ > > > - struct rte_mempool *sess_mp_priv; > > > - /**< Session Private Data Mempool */ > > > struct rte_cryptodev_stats qp_stats; > > > /**< Queue pair statistics */ > > > uint8_t temp_digest[KASUMI_DIGEST_LENGTH]; > > > diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c > > > b/drivers/crypto/kasumi/rte_kasumi_pmd.c > > > index d6f927417a..1fc59c8b8a 100644 > > > --- a/drivers/crypto/kasumi/rte_kasumi_pmd.c > > > +++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c > > > @@ -139,27 +139,24 @@ kasumi_get_session(struct kasumi_qp *qp, > struct > > > rte_crypto_op *op) > > > op->sym->session, > > > cryptodev_driver_id); > > > } else { > > > - void *_sess = NULL; > > > - void *_sess_private_data = NULL; > > > + struct rte_cryptodev_sym_session *_sess = NULL; > > > > > > - if (rte_mempool_get(qp->sess_mp, (void **)&_sess)) > > > + /* Create temporary session */ > > > + _sess = rte_cryptodev_sym_session_create(qp->sess_mp); > > > + if (_sess == NULL) > > > return NULL; > > > > > > - if (rte_mempool_get(qp->sess_mp_priv, > > > - (void **)&_sess_private_data)) > > > - return NULL; > > > - > > > - sess = (struct kasumi_session *)_sess_private_data; > > > - > > > + _sess->sess_data[cryptodev_driver_id].data = > > > + (void *)((uint8_t *)_sess + > > > + rte_cryptodev_sym_get_header_session_size() + > > > + (cryptodev_driver_id * _sess->priv_sz)); > > > + sess = _sess->sess_data[cryptodev_driver_id].data; > > > if (unlikely(kasumi_set_session_parameters(qp->mgr, sess, > > > op->sym->xform) != 0)) { > > > rte_mempool_put(qp->sess_mp, _sess); > > > - rte_mempool_put(qp->sess_mp_priv, _sess_private_data); > > > sess = NULL; > > > } > > > op->sym->session = (struct rte_cryptodev_sym_session *)_sess; > > > - set_sym_session_private_data(op->sym->session, > > > - cryptodev_driver_id, _sess_private_data); > > > } > > > > > > if (unlikely(sess == NULL)) > > > @@ -327,7 +324,6 @@ process_ops(struct rte_crypto_op **ops, struct > > > kasumi_session *session, > > > memset(ops[i]->sym->session, 0, > > > rte_cryptodev_sym_get_existing_header_session_size( > > > ops[i]->sym->session)); > > > - rte_mempool_put(qp->sess_mp_priv, session); > > > rte_mempool_put(qp->sess_mp, ops[i]->sym->session); > > > ops[i]->sym->session = NULL; > > > }
> > Hi Akhil, > > > > I tried to fix the problems of seg faults. > > The seg-faults are gone now but all asym tests are failing too. > > The reason is the rte_cryptodev_queue_pair_setup() checks the session > > mempool same for sym and asym. > > Since we don't have a rte_cryptodev_asym_session_pool_create() the > > session mempool created by > > test_cryptodev_asym.c with rte_mempool_create() will fail the mempool > > check when setting up the queue pair. > > > > If you think my fix may be useful (although not resolving asym issue) I can > > send it. > > > Is it a different fix than what I proposed below? If yes, you can send the diff. > I already made the below changes for all the PMDs. > I will try to fix the asym issue, but I suppose it can be dealt in the app > Which can be fixed separately in RC2. > > Also, found the root cause of multi process issue, working on making the > patches. > Will send v3 soon with all 3 issues(docsis/mp/sessless) fixed atleast. > For Asym, may send a separate patch. > For Asym issue, it looks like the APIs are not written properly and has many Issues compared to sym. Looking at the API rte_cryptodev_queue_pair_setup(), it only support mp_session(or priv_sess_mp) for symmetric sessions even without my changes. Hence, a qp does not have mempool for sessionless Asym processing and looking at current Drivers, only QAT support asym session less and it does not use mempool stored in qp. Hence IMO, it is safe to remove the check from rte_cryptodev_queue_pair_setup() if (!qp_conf->mp_session) { CDEV_LOG_ERR("Invalid mempools\n"); return -EINVAL; } Or we can have give a CDEV_LOG_INFO (to indicate session mempool not present, session less won't work) instead of CDEV_LOG_ERR and fall through. For sym case, it is checking again in next line if session_mp is there or not. I hope, the asym cases will work once we remove the above check and pass Null in the asym app while setting up queue pairs. What say?
Hi Akhil,
I didn't work on the asym problem. As stated in the email I could think of the solution is to add new API to create asym session pool - or you may have better solution.
BTW current test_cryptodev_asym.c the function testsuite_setup() creates the queue pair before creating the session pool, which will always made the queue pair creation fail at the library layer - as the session pool cannot be empty. I don't think the session pool is mandatory when creating the queue pair as it is only needed for session-less operation even for sym crypto - this change also doesn't make sense for the crypto PMDs who don't support session-less operation.
My sym fix is as same as your proposal. Here is my diff as ref for sym crypto seg fault fix.
diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
index 330aad8157..990fc99763 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd.c
@@ -174,27 +174,25 @@ aesni_gcm_get_session(struct aesni_gcm_qp *qp, struct rte_crypto_op *op)
sym_op->session,
cryptodev_driver_id);
} else {
- void *_sess;
- void *_sess_private_data = NULL;
+ struct rte_cryptodev_sym_session *_sess =
+ rte_cryptodev_sym_session_create(qp->sess_mp);
- if (rte_mempool_get(qp->sess_mp, (void **)&_sess))
+ if (_sess == NULL)
return NULL;
- if (rte_mempool_get(qp->sess_mp_priv,
- (void **)&_sess_private_data))
- return NULL;
+ _sess->sess_data[cryptodev_driver_id].data =
+ (void *)((uint8_t *)_sess +
+ rte_cryptodev_sym_get_header_session_size() +
+ (cryptodev_driver_id * _sess->priv_sz));
- sess = (struct aesni_gcm_session *)_sess_private_data;
+ sess = _sess->sess_data[cryptodev_driver_id].data;
if (unlikely(aesni_gcm_set_session_parameters(qp->ops,
sess, sym_op->xform) != 0)) {
rte_mempool_put(qp->sess_mp, _sess);
- rte_mempool_put(qp->sess_mp_priv, _sess_private_data);
sess = NULL;
}
sym_op->session = (struct rte_cryptodev_sym_session *)_sess;
- set_sym_session_private_data(sym_op->session,
- cryptodev_driver_id, _sess_private_data);
}
if (unlikely(sess == NULL))
@@ -716,7 +714,6 @@ handle_completed_gcm_crypto_op(struct aesni_gcm_qp *qp,
memset(op->sym->session, 0,
rte_cryptodev_sym_get_existing_header_session_size(
op->sym->session));
- rte_mempool_put(qp->sess_mp_priv, sess);
rte_mempool_put(qp->sess_mp, op->sym->session);
op->sym->session = NULL;
}
diff --git a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h
index 2763d1c492..cb37fd6b29 100644
--- a/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h
+++ b/drivers/crypto/aesni_gcm/aesni_gcm_pmd_private.h
@@ -52,8 +52,6 @@ struct aesni_gcm_qp {
/**< Queue pair statistics */
struct rte_mempool *sess_mp;
/**< Session Mempool */
- struct rte_mempool *sess_mp_priv;
- /**< Session Private Data Mempool */
uint16_t id;
/**< Queue Pair Identifier */
char name[RTE_CRYPTODEV_NAME_MAX_LEN];
diff --git a/drivers/crypto/aesni_mb/aesni_mb_pmd_private.h b/drivers/crypto/aesni_mb/aesni_mb_pmd_private.h
index 11e7bf5d18..2398fdf1b8 100644
--- a/drivers/crypto/aesni_mb/aesni_mb_pmd_private.h
+++ b/drivers/crypto/aesni_mb/aesni_mb_pmd_private.h
@@ -182,8 +182,6 @@ struct aesni_mb_qp {
/**< Ring for placing operations ready for processing */
struct rte_mempool *sess_mp;
/**< Session Mempool */
- struct rte_mempool *sess_mp_priv;
- /**< Session Private Data Mempool */
struct rte_cryptodev_stats stats;
/**< Queue pair statistics */
uint8_t digest_idx;
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
index e8da9ea9e1..d9e525c86f 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd.c
@@ -1024,27 +1024,25 @@ get_session(struct aesni_mb_qp *qp, struct rte_crypto_op *op)
(op->sym->sec_session);
#endif
} else {
- void *_sess = rte_cryptodev_sym_session_create(qp->sess_mp);
- void *_sess_private_data = NULL;
+ struct rte_cryptodev_sym_session *_sess =
+ rte_cryptodev_sym_session_create(qp->sess_mp);
if (_sess == NULL)
return NULL;
- if (rte_mempool_get(qp->sess_mp_priv,
- (void **)&_sess_private_data))
- return NULL;
+ _sess->sess_data[cryptodev_driver_id].data =
+ (void *)((uint8_t *)_sess +
+ rte_cryptodev_sym_get_header_session_size() +
+ (cryptodev_driver_id * _sess->priv_sz));
- sess = (struct aesni_mb_session *)_sess_private_data;
+ sess = _sess->sess_data[cryptodev_driver_id].data;
if (unlikely(aesni_mb_set_session_parameters(qp->mb_mgr,
sess, op->sym->xform) != 0)) {
rte_mempool_put(qp->sess_mp, _sess);
- rte_mempool_put(qp->sess_mp_priv, _sess_private_data);
sess = NULL;
}
- op->sym->session = (struct rte_cryptodev_sym_session *)_sess;
- set_sym_session_private_data(op->sym->session,
- cryptodev_driver_id, _sess_private_data);
+ op->sym->session = _sess;
}
if (unlikely(sess == NULL))
@@ -1688,7 +1686,6 @@ post_process_mb_job(struct aesni_mb_qp *qp, JOB_AES_HMAC *job)
memset(op->sym->session, 0,
rte_cryptodev_sym_get_existing_header_session_size(
op->sym->session));
- rte_mempool_put(qp->sess_mp_priv, sess);
rte_mempool_put(qp->sess_mp, op->sym->session);
op->sym->session = NULL;
}
diff --git a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
index b7a806d51c..b9c0f8b9ee 100644
--- a/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
+++ b/drivers/crypto/aesni_mb/rte_aesni_mb_pmd_ops.c
@@ -1072,8 +1072,16 @@ aesni_mb_pmd_sec_capa_get(void *device __rte_unused)
return aesni_mb_pmd_security_cap;
}
+/** Returns the size of the aesni multi-buffer session structure */
+static unsigned
+aesni_mb_pmd_sec_session_get_size(void *dev __rte_unused)
+{
+ return sizeof(struct aesni_mb_session);
+}
+
static struct rte_security_ops aesni_mb_pmd_sec_ops = {
.session_create = aesni_mb_pmd_sec_sess_create,
+ .session_get_size = aesni_mb_pmd_sec_session_get_size,
.session_update = NULL,
.session_stats_get = NULL,
.session_destroy = aesni_mb_pmd_sec_sess_destroy,
diff --git a/drivers/crypto/kasumi/kasumi_pmd_private.h b/drivers/crypto/kasumi/kasumi_pmd_private.h
index abedcd616d..fe0e78e516 100644
--- a/drivers/crypto/kasumi/kasumi_pmd_private.h
+++ b/drivers/crypto/kasumi/kasumi_pmd_private.h
@@ -38,8 +38,6 @@ struct kasumi_qp {
/**< Ring for placing processed ops */
struct rte_mempool *sess_mp;
/**< Session Mempool */
- struct rte_mempool *sess_mp_priv;
- /**< Session Private Data Mempool */
struct rte_cryptodev_stats qp_stats;
/**< Queue pair statistics */
uint8_t temp_digest[KASUMI_DIGEST_LENGTH];
diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c b/drivers/crypto/kasumi/rte_kasumi_pmd.c
index d6f927417a..f130400152 100644
--- a/drivers/crypto/kasumi/rte_kasumi_pmd.c
+++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c
@@ -139,27 +139,25 @@ kasumi_get_session(struct kasumi_qp *qp, struct rte_crypto_op *op)
op->sym->session,
cryptodev_driver_id);
} else {
- void *_sess = NULL;
- void *_sess_private_data = NULL;
+ struct rte_cryptodev_sym_session *_sess =
+ rte_cryptodev_sym_session_create(qp->sess_mp);
- if (rte_mempool_get(qp->sess_mp, (void **)&_sess))
+ if (_sess == NULL)
return NULL;
- if (rte_mempool_get(qp->sess_mp_priv,
- (void **)&_sess_private_data))
- return NULL;
+ _sess->sess_data[cryptodev_driver_id].data =
+ (void *)((uint8_t *)_sess +
+ rte_cryptodev_sym_get_header_session_size() +
+ (cryptodev_driver_id * _sess->priv_sz));
- sess = (struct kasumi_session *)_sess_private_data;
+ sess = _sess->sess_data[cryptodev_driver_id].data;
if (unlikely(kasumi_set_session_parameters(qp->mgr, sess,
op->sym->xform) != 0)) {
rte_mempool_put(qp->sess_mp, _sess);
- rte_mempool_put(qp->sess_mp_priv, _sess_private_data);
sess = NULL;
}
op->sym->session = (struct rte_cryptodev_sym_session *)_sess;
- set_sym_session_private_data(op->sym->session,
- cryptodev_driver_id, _sess_private_data);
}
if (unlikely(sess == NULL))
@@ -327,7 +325,6 @@ process_ops(struct rte_crypto_op **ops, struct kasumi_session *session,
memset(ops[i]->sym->session, 0,
rte_cryptodev_sym_get_existing_header_session_size(
ops[i]->sym->session));
- rte_mempool_put(qp->sess_mp_priv, session);
rte_mempool_put(qp->sess_mp, ops[i]->sym->session);
ops[i]->sym->session = NULL;
}
diff --git a/drivers/crypto/qat/qat_sym_pmd.c b/drivers/crypto/qat/qat_sym_pmd.c
index efda921c05..a55fb4f342 100644
--- a/drivers/crypto/qat/qat_sym_pmd.c
+++ b/drivers/crypto/qat/qat_sym_pmd.c
@@ -305,6 +305,7 @@ qat_security_cap_get(void *device __rte_unused)
static struct rte_security_ops security_qat_ops = {
.session_create = qat_security_session_create,
+ .session_get_size = qat_security_session_get_private_size,
.session_update = NULL,
.session_stats_get = NULL,
.session_destroy = qat_security_session_destroy,
diff --git a/drivers/crypto/qat/qat_sym_session.c b/drivers/crypto/qat/qat_sym_session.c
index ef92f22c1a..4066230155 100644
--- a/drivers/crypto/qat/qat_sym_session.c
+++ b/drivers/crypto/qat/qat_sym_session.c
@@ -2298,3 +2298,10 @@ qat_security_session_destroy(void *dev __rte_unused, void *sess_priv)
return 0;
}
#endif
+
+unsigned int
+qat_security_session_get_private_size(void *dev __rte_unused)
+{
+ return RTE_ALIGN_CEIL(sizeof(struct qat_sym_session), 8);
+}
+
diff --git a/drivers/crypto/qat/qat_sym_session.h b/drivers/crypto/qat/qat_sym_session.h
index 4b7de4c9e7..a02ba01adf 100644
--- a/drivers/crypto/qat/qat_sym_session.h
+++ b/drivers/crypto/qat/qat_sym_session.h
@@ -169,6 +169,9 @@ qat_security_session_create(void *dev, struct rte_security_session_conf *conf,
void *sess, rte_iova_t sess_iova);
int
qat_security_session_destroy(void *dev, void *sess);
+
+unsigned int
+qat_security_session_get_private_size(void *dev);
#endif
#endif /* _QAT_SYM_SESSION_H_ */
diff --git a/drivers/crypto/snow3g/rte_snow3g_pmd.c b/drivers/crypto/snow3g/rte_snow3g_pmd.c
index 8284ac0b66..02e65393e3 100644
--- a/drivers/crypto/snow3g/rte_snow3g_pmd.c
+++ b/drivers/crypto/snow3g/rte_snow3g_pmd.c
@@ -149,27 +149,26 @@ snow3g_get_session(struct snow3g_qp *qp, struct rte_crypto_op *op)
op->sym->session,
cryptodev_driver_id);
} else {
- void *_sess = NULL;
- void *_sess_private_data = NULL;
+ struct rte_cryptodev_sym_session *_sess =
+ rte_cryptodev_sym_session_create(qp->sess_mp);
- if (rte_mempool_get(qp->sess_mp, (void **)&_sess))
+ if (_sess == NULL)
return NULL;
- if (rte_mempool_get(qp->sess_mp_priv,
- (void **)&_sess_private_data))
- return NULL;
+ _sess->sess_data[cryptodev_driver_id].data =
+ (void *)((uint8_t *)_sess +
+ rte_cryptodev_sym_get_header_session_size() +
+ (cryptodev_driver_id * _sess->priv_sz));
+
+ sess = _sess->sess_data[cryptodev_driver_id].data;
- sess = (struct snow3g_session *)_sess_private_data;
if (unlikely(snow3g_set_session_parameters(qp->mgr, sess,
op->sym->xform) != 0)) {
rte_mempool_put(qp->sess_mp, _sess);
- rte_mempool_put(qp->sess_mp_priv, _sess_private_data);
sess = NULL;
}
op->sym->session = (struct rte_cryptodev_sym_session *)_sess;
- set_sym_session_private_data(op->sym->session,
- cryptodev_driver_id, _sess_private_data);
}
if (unlikely(sess == NULL))
@@ -352,7 +351,6 @@ process_ops(struct rte_crypto_op **ops, struct snow3g_session *session,
memset(ops[i]->sym->session, 0,
rte_cryptodev_sym_get_existing_header_session_size(
ops[i]->sym->session));
- rte_mempool_put(qp->sess_mp_priv, session);
rte_mempool_put(qp->sess_mp, ops[i]->sym->session);
ops[i]->sym->session = NULL;
}
diff --git a/drivers/crypto/snow3g/snow3g_pmd_private.h b/drivers/crypto/snow3g/snow3g_pmd_private.h
index 23cf078a9c..96897d4651 100644
--- a/drivers/crypto/snow3g/snow3g_pmd_private.h
+++ b/drivers/crypto/snow3g/snow3g_pmd_private.h
@@ -39,8 +39,6 @@ struct snow3g_qp {
/**< Ring for placing processed ops */
struct rte_mempool *sess_mp;
/**< Session Mempool */
- struct rte_mempool *sess_mp_priv;
- /**< Session Private Data Mempool */
struct rte_cryptodev_stats qp_stats;
/**< Queue pair statistics */
uint8_t temp_digest[SNOW3G_DIGEST_LENGTH];
diff --git a/drivers/crypto/zuc/rte_zuc_pmd.c b/drivers/crypto/zuc/rte_zuc_pmd.c
index d4b343a7af..2ac333fc35 100644
--- a/drivers/crypto/zuc/rte_zuc_pmd.c
+++ b/drivers/crypto/zuc/rte_zuc_pmd.c
@@ -138,27 +138,25 @@ zuc_get_session(struct zuc_qp *qp, struct rte_crypto_op *op)
op->sym->session,
cryptodev_driver_id);
} else {
- void *_sess = NULL;
- void *_sess_private_data = NULL;
+ struct rte_cryptodev_sym_session *_sess =
+ rte_cryptodev_sym_session_create(qp->sess_mp);
- if (rte_mempool_get(qp->sess_mp, (void **)&_sess))
+ if (_sess == NULL)
return NULL;
- if (rte_mempool_get(qp->sess_mp_priv,
- (void **)&_sess_private_data))
- return NULL;
+ _sess->sess_data[cryptodev_driver_id].data =
+ (void *)((uint8_t *)_sess +
+ rte_cryptodev_sym_get_header_session_size() +
+ (cryptodev_driver_id * _sess->priv_sz));
- sess = (struct zuc_session *)_sess_private_data;
+ sess = _sess->sess_data[cryptodev_driver_id].data;
if (unlikely(zuc_set_session_parameters(sess,
op->sym->xform) != 0)) {
rte_mempool_put(qp->sess_mp, _sess);
- rte_mempool_put(qp->sess_mp_priv, _sess_private_data);
sess = NULL;
}
op->sym->session = (struct rte_cryptodev_sym_session *)_sess;
- set_sym_session_private_data(op->sym->session,
- cryptodev_driver_id, _sess_private_data);
}
if (unlikely(sess == NULL))
@@ -341,7 +339,6 @@ process_ops(struct rte_crypto_op **ops, enum zuc_operation op_type,
memset(ops[i]->sym->session, 0,
rte_cryptodev_sym_get_existing_header_session_size(
ops[i]->sym->session));
- rte_mempool_put(qp->sess_mp_priv, sessions[i]);
rte_mempool_put(qp->sess_mp, ops[i]->sym->session);
ops[i]->sym->session = NULL;
}
diff --git a/drivers/crypto/zuc/zuc_pmd_private.h b/drivers/crypto/zuc/zuc_pmd_private.h
index d8684891ee..23cd9dc458 100644
--- a/drivers/crypto/zuc/zuc_pmd_private.h
+++ b/drivers/crypto/zuc/zuc_pmd_private.h
@@ -38,8 +38,6 @@ struct zuc_qp {
/**< Ring for placing processed ops */
struct rte_mempool *sess_mp;
/**< Session Mempool */
- struct rte_mempool *sess_mp_priv;
- /**< Session Private Data Mempool */
struct rte_cryptodev_stats qp_stats;
/**< Queue pair statistics */
uint8_t temp_digest[ZUC_DIGEST_LENGTH];
regards,
Fan
> -----Original Message-----
> From: Akhil Goyal <gakhil@marvell.com>
> Sent: Friday, October 15, 2021 6:43 PM
> To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org
> Cc: thomas@monjalon.net; david.marchand@redhat.com;
> hemant.agrawal@nxp.com; Anoob Joseph <anoobj@marvell.com>; De Lara
> Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona
> <fiona.trahe@intel.com>; Doherty, Declan <declan.doherty@intel.com>;
> matan@nvidia.com; g.singh@nxp.com; jianjay.zhou@huawei.com;
> asomalap@amd.com; ruifeng.wang@arm.com; Ananyev, Konstantin
> <konstantin.ananyev@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>;
> ajit.khaparde@broadcom.com; Nagadheeraj Rottela
> <rnagadheeraj@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>;
> Power, Ciara <ciara.power@intel.com>; Wang, Haiyue
> <haiyue.wang@intel.com>; jiawenwu@trustnetic.com;
> jianwang@trustnetic.com
> Subject: RE: [PATCH v2 0/7] crypto/security session framework rework
>
> > Hi Akhil,
> >
> > I tried to fix the problems of seg faults.
> > The seg-faults are gone now but all asym tests are failing too.
> > The reason is the rte_cryptodev_queue_pair_setup() checks the session
> > mempool same for sym and asym.
> > Since we don't have a rte_cryptodev_asym_session_pool_create() the
> > session mempool created by
> > test_cryptodev_asym.c with rte_mempool_create() will fail the mempool
> > check when setting up the queue pair.
> >
> > If you think my fix may be useful (although not resolving asym issue) I can
> > send it.
> >
> Is it a different fix than what I proposed below? If yes, you can send the diff.
> I already made the below changes for all the PMDs.
> I will try to fix the asym issue, but I suppose it can be dealt in the app
> Which can be fixed separately in RC2.
>
> Also, found the root cause of multi process issue, working on making the
> patches.
> Will send v3 soon with all 3 issues(docsis/mp/sessless) fixed atleast.
> For Asym, may send a separate patch.
>
> > > Hi Fan,
> > > Check for below QAT fix also
> > > > >
> > > > > Unfortunately the patches still cause seg-fault at QAT and SW PMDs.
> > > > >
> > > > > - for qat it fails at rte_security_ops->session_size_get not
> implemented.
> > > And for this one
> > > diff --git a/drivers/crypto/qat/qat_sym_pmd.c
> > > b/drivers/crypto/qat/qat_sym_pmd.c
> > > index efda921c05..96cd9d2eee 100644
> > > --- a/drivers/crypto/qat/qat_sym_pmd.c
> > > +++ b/drivers/crypto/qat/qat_sym_pmd.c
> > > @@ -306,6 +306,7 @@ static struct rte_security_ops security_qat_ops = {
> > >
> > > .session_create = qat_security_session_create,
> > > .session_update = NULL,
> > > + .session_get_size = qat_security_session_get_size,
> > > .session_stats_get = NULL,
> > > .session_destroy = qat_security_session_destroy,
> > > .set_pkt_metadata = NULL,
> > > diff --git a/drivers/crypto/qat/qat_sym_session.c
> > > b/drivers/crypto/qat/qat_sym_session.c
> > > index ef92f22c1a..41b5542343 100644
> > > --- a/drivers/crypto/qat/qat_sym_session.c
> > > +++ b/drivers/crypto/qat/qat_sym_session.c
> > > @@ -2297,4 +2297,10 @@ qat_security_session_destroy(void *dev
> > > __rte_unused, void *sess_priv)
> > > }
> > > return 0;
> > > }
> > > +
> > > +static unsigned int
> > > +qat_security_session_get_size(void *device __rte_unused)
> > > +{
> > > + return sizeof(struct qat_sym_session);
> > > +}
> > > #endif
> > >
> > > > > - for sw pmds the queue pair's session private mempools are not set.
> > > > >
> > > > Can you check if below change works for Kasumi. I will replicate for
> > others.
> > > >
> > > > diff --git a/drivers/crypto/kasumi/kasumi_pmd_private.h
> > > > b/drivers/crypto/kasumi/kasumi_pmd_private.h
> > > > index abedcd616d..fe0e78e516 100644
> > > > --- a/drivers/crypto/kasumi/kasumi_pmd_private.h
> > > > +++ b/drivers/crypto/kasumi/kasumi_pmd_private.h
> > > > @@ -38,8 +38,6 @@ struct kasumi_qp {
> > > > /**< Ring for placing processed ops */
> > > > struct rte_mempool *sess_mp;
> > > > /**< Session Mempool */
> > > > - struct rte_mempool *sess_mp_priv;
> > > > - /**< Session Private Data Mempool */
> > > > struct rte_cryptodev_stats qp_stats;
> > > > /**< Queue pair statistics */
> > > > uint8_t temp_digest[KASUMI_DIGEST_LENGTH];
> > > > diff --git a/drivers/crypto/kasumi/rte_kasumi_pmd.c
> > > > b/drivers/crypto/kasumi/rte_kasumi_pmd.c
> > > > index d6f927417a..1fc59c8b8a 100644
> > > > --- a/drivers/crypto/kasumi/rte_kasumi_pmd.c
> > > > +++ b/drivers/crypto/kasumi/rte_kasumi_pmd.c
> > > > @@ -139,27 +139,24 @@ kasumi_get_session(struct kasumi_qp *qp,
> > struct
> > > > rte_crypto_op *op)
> > > > op->sym->session,
> > > > cryptodev_driver_id);
> > > > } else {
> > > > - void *_sess = NULL;
> > > > - void *_sess_private_data = NULL;
> > > > + struct rte_cryptodev_sym_session *_sess = NULL;
> > > >
> > > > - if (rte_mempool_get(qp->sess_mp, (void **)&_sess))
> > > > + /* Create temporary session */
> > > > + _sess = rte_cryptodev_sym_session_create(qp->sess_mp);
> > > > + if (_sess == NULL)
> > > > return NULL;
> > > >
> > > > - if (rte_mempool_get(qp->sess_mp_priv,
> > > > - (void **)&_sess_private_data))
> > > > - return NULL;
> > > > -
> > > > - sess = (struct kasumi_session *)_sess_private_data;
> > > > -
> > > > + _sess->sess_data[cryptodev_driver_id].data =
> > > > + (void *)((uint8_t *)_sess +
> > > > + rte_cryptodev_sym_get_header_session_size() +
> > > > + (cryptodev_driver_id * _sess->priv_sz));
> > > > + sess = _sess->sess_data[cryptodev_driver_id].data;
> > > > if (unlikely(kasumi_set_session_parameters(qp->mgr, sess,
> > > > op->sym->xform) != 0)) {
> > > > rte_mempool_put(qp->sess_mp, _sess);
> > > > - rte_mempool_put(qp->sess_mp_priv,
> _sess_private_data);
> > > > sess = NULL;
> > > > }
> > > > op->sym->session = (struct rte_cryptodev_sym_session
> *)_sess;
> > > > - set_sym_session_private_data(op->sym->session,
> > > > - cryptodev_driver_id, _sess_private_data);
> > > > }
> > > >
> > > > if (unlikely(sess == NULL))
> > > > @@ -327,7 +324,6 @@ process_ops(struct rte_crypto_op **ops, struct
> > > > kasumi_session *session,
> > > > memset(ops[i]->sym->session, 0,
> > > > rte_cryptodev_sym_get_existing_header_session_size(
> > > > ops[i]->sym->session));
> > > > - rte_mempool_put(qp->sess_mp_priv, session);
> > > > rte_mempool_put(qp->sess_mp, ops[i]->sym->session);
> > > > ops[i]->sym->session = NULL;
> > > > }
Hi, > -----Original Message----- > From: Akhil Goyal <gakhil@marvell.com> > Sent: Friday, October 15, 2021 7:47 PM > To: Zhang, Roy Fan <roy.fan.zhang@intel.com>; dev@dpdk.org > Cc: thomas@monjalon.net; david.marchand@redhat.com; > hemant.agrawal@nxp.com; Anoob Joseph <anoobj@marvell.com>; De Lara > Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Trahe, Fiona > <fiona.trahe@intel.com>; Doherty, Declan <declan.doherty@intel.com>; > matan@nvidia.com; g.singh@nxp.com; jianjay.zhou@huawei.com; > asomalap@amd.com; ruifeng.wang@arm.com; Ananyev, Konstantin > <konstantin.ananyev@intel.com>; Nicolau, Radu <radu.nicolau@intel.com>; > ajit.khaparde@broadcom.com; Nagadheeraj Rottela > <rnagadheeraj@marvell.com>; Ankur Dwivedi <adwivedi@marvell.com>; > Power, Ciara <ciara.power@intel.com>; Wang, Haiyue > <haiyue.wang@intel.com>; jiawenwu@trustnetic.com; > jianwang@trustnetic.com > Subject: RE: [PATCH v2 0/7] crypto/security session framework rework > > > > Hi Akhil, > > > > > > I tried to fix the problems of seg faults. > > > The seg-faults are gone now but all asym tests are failing too. > > > The reason is the rte_cryptodev_queue_pair_setup() checks the session > > > mempool same for sym and asym. > > > Since we don't have a rte_cryptodev_asym_session_pool_create() the > > > session mempool created by > > > test_cryptodev_asym.c with rte_mempool_create() will fail the > mempool > > > check when setting up the queue pair. > > > > > > If you think my fix may be useful (although not resolving asym issue) I can > > > send it. > > > > > Is it a different fix than what I proposed below? If yes, you can send the > diff. > > I already made the below changes for all the PMDs. > > I will try to fix the asym issue, but I suppose it can be dealt in the app > > Which can be fixed separately in RC2. > > > > Also, found the root cause of multi process issue, working on making the > > patches. > > Will send v3 soon with all 3 issues(docsis/mp/sessless) fixed atleast. > > For Asym, may send a separate patch. > > > For Asym issue, it looks like the APIs are not written properly and has many > Issues compared to sym. > Looking at the API rte_cryptodev_queue_pair_setup(), it only support > mp_session(or priv_sess_mp) for symmetric sessions even without my > changes. > > Hence, a qp does not have mempool for sessionless Asym processing and > looking at current > Drivers, only QAT support asym session less and it does not use mempool > stored in qp. > > Hence IMO, it is safe to remove the check from > rte_cryptodev_queue_pair_setup() > if (!qp_conf->mp_session) { > CDEV_LOG_ERR("Invalid mempools\n"); > return -EINVAL; > } > Or we can have give a CDEV_LOG_INFO (to indicate session mempool not > present, session less won't work) instead of CDEV_LOG_ERR and fall through. > Yes this is a valid fix. It will make queue pair setup work as before. The old code was like this: if ((qp_conf->mp_session && !qp_conf->mp_session_private) || (!qp_conf->mp_session && qp_conf->mp_session_private)) { CDEV_LOG_ERR("Invalid mempools\n"); return -EINVAL; } The requirement was either you provide 2 mempools (one for session and one for session private) or you don't provide session mempool when creating queue pair at all. Only otherwise the error is returned. > For sym case, it is checking again in next line if session_mp is there or not. > > I hope, the asym cases will work once we remove the above check and pass > Null in the asym app while setting up queue pairs. What say? It shall be working. Thanks. > > >