From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 6808DA0C4E; Thu, 21 Oct 2021 15:13:26 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 50895411FE; Thu, 21 Oct 2021 15:13:26 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by mails.dpdk.org (Postfix) with ESMTP id 7E0584118E for ; Thu, 21 Oct 2021 15:13:24 +0200 (CEST) X-IronPort-AV: E=McAfee;i="6200,9189,10143"; a="252528071" X-IronPort-AV: E=Sophos;i="5.87,169,1631602800"; d="scan'208";a="252528071" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Oct 2021 06:12:22 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.87,169,1631602800"; d="scan'208";a="568310304" Received: from orsmsx605.amr.corp.intel.com ([10.22.229.18]) by FMSMGA003.fm.intel.com with ESMTP; 21 Oct 2021 06:12:21 -0700 Received: from orsmsx607.amr.corp.intel.com (10.22.229.20) by ORSMSX605.amr.corp.intel.com (10.22.229.18) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Thu, 21 Oct 2021 06:12:21 -0700 Received: from orsmsx604.amr.corp.intel.com (10.22.229.17) by ORSMSX607.amr.corp.intel.com (10.22.229.20) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12; Thu, 21 Oct 2021 06:12:20 -0700 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx604.amr.corp.intel.com (10.22.229.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2242.12 via Frontend Transport; Thu, 21 Oct 2021 06:12:20 -0700 Received: from NAM11-BN8-obe.outbound.protection.outlook.com (104.47.58.171) by edgegateway.intel.com (134.134.137.102) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2242.12; Thu, 21 Oct 2021 06:11:41 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=I2g0v1+2Qv/8Tg3WfYPri9cFDu1HErMxlqQV8mWDKEiiOWhkjQQhaYF6hA7Gc6o/7XzrRgCCE5NQdRgVazy9KEKj/peRJjjXp9cT9ZqgSAafGm34SO8W5oemGsylM6fe+9T8UskSVP8i18/LDeFrdRMKE9PEpguqUTVDZG2aQhyPp3LeK3+c0cD5VKfslXsXa/yF0BSTFc8LIXXpWOs2wLL4u0I7qHqgZlMoHou9bB/RuoFKO5gm28mqAOkuO4J5aphNy9Q2a8QpvnM7FDWgO9K2YYzPVhhQaamAwhJmNRzNO3GijlLnmZj3hSQ5ZhRGd3tjgvwMIX6/1b/JtdGZdg== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=OShuRtBm9zZj/42AT5GwDCIqT4SCNKGjpXRIfnJZHg0=; b=ZWjrpYpr/+1pRqBc/BzqWWdQXcKI0OyRYuoodsIkUhcF8g6SR35GrF8UejDLFfTvO+NZR1Iill+O+laEBNgvSsOvhL5TanPRb7jRhqvceNtYMAOWN0LRwJOmTnwaafsCBIuxOvuGNQItv87Z632xCPKwaENg5OTcrLb+phjO91yqDGEEootB1c8ZIEK0XW+bietLrLzcFNgaHyJv24KEVhtQcMkJuAhcSY+0xgqb+Hf/7HPKaizz01wKyX2VhWNuQLH1p25LH1wm6zGvWLzrhMe0iYL/M7nUrPdCsVyTT37LpSks0VOP4V2vd3KnZ0tAgXm6z+g6Lgjwb4Ewpbi9OQ== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=intel.com; dmarc=pass action=none header.from=intel.com; dkim=pass header.d=intel.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel.onmicrosoft.com; s=selector2-intel-onmicrosoft-com; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=OShuRtBm9zZj/42AT5GwDCIqT4SCNKGjpXRIfnJZHg0=; b=FC/Q7P13+mHUfpKAV/6XwSxU4oJr9GqB3daqz5Hsvf3Sfz2ct8cmFJaDYCSWWv0LdtEBmsNHMG0RLckr6fUcdwew/CHt9Q1c7tkD2rl6xLo/ztM34ho/zfJpau3hp77z9g+n/NQxKmV2CYImLOWrnZ8O0uZluuBmcjlhjcD4E2c= Received: from DM6PR11MB4491.namprd11.prod.outlook.com (2603:10b6:5:204::19) by DM8PR11MB5688.namprd11.prod.outlook.com (2603:10b6:8:23::12) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4628.18; Thu, 21 Oct 2021 13:11:40 +0000 Received: from DM6PR11MB4491.namprd11.prod.outlook.com ([fe80::2c0c:5383:f814:3b4e]) by DM6PR11MB4491.namprd11.prod.outlook.com ([fe80::2c0c:5383:f814:3b4e%6]) with mapi id 15.20.4628.018; Thu, 21 Oct 2021 13:11:40 +0000 From: "Ananyev, Konstantin" To: Akhil Goyal , "dev@dpdk.org" , "Zhang, Roy Fan" CC: "thomas@monjalon.net" , "david.marchand@redhat.com" , "hemant.agrawal@nxp.com" , Anoob Joseph , "De Lara Guarch, Pablo" , "Trahe, Fiona" , "Doherty, Declan" , "matan@nvidia.com" , "g.singh@nxp.com" , "jianjay.zhou@huawei.com" , "asomalap@amd.com" , "ruifeng.wang@arm.com" , "Nicolau, Radu" , "ajit.khaparde@broadcom.com" , Nagadheeraj Rottela , Ankur Dwivedi , "Power, Ciara" , "Wang, Haiyue" , "jiawenwu@trustnetic.com" , "jianwang@trustnetic.com" , Jerin Jacob Kollanukkaran , Nithin Kumar Dabilpuram Thread-Topic: [PATCH v3 6/8] cryptodev: rework session framework Thread-Index: AQHXxGgs8uA5efxFxkuDtx/p8VXwcKvcQbSwgADGSQCAADYUkIAAJ+4AgAAHLGA= Date: Thu, 21 Oct 2021 13:11:39 +0000 Message-ID: References: <20211013192222.1582631-2-gakhil@marvell.com> <20211018213452.2734720-1-gakhil@marvell.com> <20211018213452.2734720-7-gakhil@marvell.com> In-Reply-To: Accept-Language: en-GB, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-reaction: no-action dlp-version: 11.6.200.16 authentication-results: marvell.com; dkim=none (message not signed) header.d=none;marvell.com; dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: c62dfb58-e089-4dc9-fce6-08d9949451ef x-ms-traffictypediagnostic: DM8PR11MB5688: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:7691; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: Jqy3h3B1495VG6vY8unsZ56gLRBioXnsRxZKk16VAc/IsFlazT1Lho0uFDCnNWKrdu+5uzTLbsMrkGlAXHKi7vVRIi6l1+QslvmDOplmPNicd/h7G80lFQ/9YwMbTfNoSgle5Z9+N3OwcqG0jp0Q5yLJKZgFwg3BLVGb8oOUd5NG1SyuUdYT5fR/oLncVY+DRmCPvfa0RMYqXjxvWy+oqqjmx4TmjrpgAt3/y5bHwv/ilIiQVF1EWQYCSa1+4CKjaLS+Xpnr9/NWluhDcseah/PCUW6LTM058zzjJPBrkIVkriqVAnvU5PrjoJmyFbW1hoLSmCIlQa1fhPMgjF3ts1K8+5El9RILxiGJsEImLjh/fY7YOuP3w8IzeYEbFS1Kity0FA9m1BLGFgMihxMawEzwgCR5MU3eyC9ZmeriYbgTAvtlrlgjWkD1Wm/BZv51D+Uv9au2h7XFNKPAuxyJobhmYvgyWRjOuqkaYxLNP3dmfXRa2Z3PyVyg8wjtIBp7CV9HAxIQQmjttZ7VCjb5yVPqqtJ0l5+XaTavw9oJtlMabFdGFC0WM+oZ12/gLdvIttL+KzSlm95Zt7qDT7vEKV5KRmB9ZNvN+smqZnwYffCpDn8irNdxeQTYzIvvTYSqrqzOMl3aJPQMbwzbA6giCXhi+iax79oKMNBBNhlvBUi1Zygb7IOFdhrRScuPAUxsxTIjjQRpxud1/BAX3xyNrA== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR11MB4491.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(366004)(83380400001)(186003)(26005)(6506007)(7696005)(55016002)(55236004)(9686003)(86362001)(38070700005)(7416002)(82960400001)(6636002)(64756008)(54906003)(66556008)(66946007)(66476007)(66446008)(316002)(8676002)(8936002)(110136005)(52536014)(122000001)(38100700002)(4326008)(76116006)(5660300002)(508600001)(2906002)(33656002)(71200400001); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Ru5w910QDkGR+3pWrTBFQkwfAWD2rJbktvxphpfK/5gJIH3D15qpCPONVTiR?= =?us-ascii?Q?KMYtcfQGouM0qlxXLv2YjK4bBqGC6RZBtTbfugqgGb/ffsC0hwSYsezqNvMT?= =?us-ascii?Q?p8kYfmff6WXULVUZ5ZfS3ZD3Fq0Tdyn3mLvgDqlgGphv914sLApqMuVzTtOQ?= =?us-ascii?Q?eteWejJBKOaSW/QN9zO5PQbfuZg/UeHkJt5dkNtu3ZmTq0+Tq4G9xrwc9fCr?= =?us-ascii?Q?S3Q7NZTYpKZWRZBnklXv7zevj2MdjymCpDkT7x4yrbA2EFiLHOXD6iilVHhc?= =?us-ascii?Q?D1gWJH1UlYZM96adw+dOI/OXddvz+n+WlaubqTJDEcV0eGdLXMoiyfnGKPS5?= =?us-ascii?Q?s/NozAnjl4QMZ/Uz/NR/8qH34TaeEui+ihgfadQBHoYTamQcXHIO1mKEz7Nn?= =?us-ascii?Q?FqCqQUqoM3gkU74aTmqOQGON8HhxbdK3UEOW/uBhzp2FQK3jzBsRhbmrBLis?= =?us-ascii?Q?Goo3lKdmDAkocwMMi/87Ul20hsrSlx1Z4kHBramId2up3wSZKGQ8jm6zx8m9?= =?us-ascii?Q?TLxaxWbDi1arnWS+ov5gwtojd46RxQtu6uUxTZLNncFJFDzQ+FhLlGPQIHi6?= =?us-ascii?Q?9zq7X/HzQ32iUPw08CuYw35BvITp/xhuR88H5lpKuG0EQ4CMXYhTgk/cy4C1?= =?us-ascii?Q?KIGLp/A8Q2egJxzS7IVYhA3syoBsV/p3MGGIIGkSaL3E9qz0MyTJKX19mK3e?= =?us-ascii?Q?ZvR01I9jNKlahDZwm/1xgXyDLgTJo4WcXHFtAC7FrSxOCNQQz+RcYxM9fngl?= =?us-ascii?Q?OhhUq5LkwLXbZ2J2B9d6RADf5+x9NedqQB013v0N8uR3uY3mn+9m2oUMlFJG?= =?us-ascii?Q?ELOs7AlmJuFUh1UmsfkxCHg9CiIWTb4aLUqOsUlSrnNXFkegc5ck0U5vATpX?= =?us-ascii?Q?vROR6cfZsM/lc+0U2zE7BZLKqtAntv+9xImhplPUtRM7dCIf6+uYze0pznb1?= =?us-ascii?Q?oYlcwD0hJTm/KxesO5l8uubKqWWxMoFm3y3tXH0UROdFEjM3tgR+2/OKSLq5?= =?us-ascii?Q?VBRdc+DbSQbOjZAQZU6orH1xsMydshPzxnT4tDwENet49NgN2qu0x0P/co90?= =?us-ascii?Q?6mEVwkN1vh0pV3Y1M3d5NWIh2dUKJgBSNwl57QeIsmUiDzF+lsfnC8n0axdl?= =?us-ascii?Q?/r+5wQM/pJuRyzpyHWwzI7c8qcyC2pS5UdxCCkdS6EDkWMgVyh2MQRQaRcgv?= =?us-ascii?Q?0w6R7Ass8+qUsvjfvaw8EhV+CCJNp4RjgSTQ0gqAR3hN8KaTZhBkZlM1Uurt?= =?us-ascii?Q?ahKfOydwmfv4tKdMe+6fkbhk0qI1RhEjdWexdJroHHRmZTDEIFrY0fcEQmnO?= =?us-ascii?Q?jOTF32AD0+ZokFNP2+NADmYRJC74RtBzpSE2tHvs21hnbNjDIpGw43eT/CJ8?= =?us-ascii?Q?x2WbRfrEikP9n7gvww2RiM+d44BDHcFIIN8U4GB9Yz6aKK0Wt4MfLq2TNc7k?= =?us-ascii?Q?GZOpMeyJsLbzdna9TWe90rzL3Qz4MhDQ?= Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB4491.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: c62dfb58-e089-4dc9-fce6-08d9949451ef X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Oct 2021 13:11:39.9709 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 46c98d88-e344-4ed4-8496-4ed7712e255d X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: konstantin.ananyev@intel.com X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM8PR11MB5688 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [PATCH v3 6/8] cryptodev: rework session framework X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" =20 > > The problem is that with new approach you proposed there is no simple w= ay > > for PMD to > > fulfil that requirement. > > In current version of DPDK: > > - PMD reports size of private data, note that it reports extra space ne= eded > > to align its data properly inside provided buffer. > > - Then it ss up to higher layer to allocate mempool with elements big e= nough > > to hold > > PMD private data. > > - At session init that mempool is passed to PMD sym_session_confgure() = and > > it is > > PMD responsibility to allocate buffer (from given mempool) for its pri= vate > > data > > align it properly, and update sess->sess_data[].data. > > With this patch: > > - PMD still reports size of private data, but now it is cryptodev lay= er who > > allocates > > memory for PMD private data and updates sess->sess_data[].data. > > > > So PMD simply has no way to allocate/align its private data in a way it= likes > > to. > > Of course it can simply do alignment on the fly for each operation, som= ething > > like: > > > > void *p =3D get_sym_session_private_data(sess, dev->driver_id); > > sess_priv =3D RTE_PTR_ALIGN_FLOOR(p, PMD_SES_ALIGN); > > > > But it is way too ugly and error-prone. > > > > Another potential problem with that approach (when cryptodev allocates > > memory for > > PMD private session data and updates sess->sess_data[].data for it) - i= t could > > happen > > that private data for different PMDs can endup on the same cache-line. > > If we'll ever have a case with simultaneous session processing by multi= ple- > > devices > > it can cause all sorts of performance problems. >=20 > To resolve above 2 issues(performance and pointer CEIL in PMD), can you c= heck > If following diff in library would work? > -------------------------------------------------------------------------= --------------------------- > diff --git a/lib/cryptodev/rte_cryptodev.c b/lib/cryptodev/rte_cryptodev.= c > index 9d5e08bba2..7beb5339ea 100644 > --- a/lib/cryptodev/rte_cryptodev.c > +++ b/lib/cryptodev/rte_cryptodev.c > @@ -1731,12 +1731,13 @@ rte_cryptodev_sym_session_init(uint8_t dev_id, > RTE_FUNC_PTR_OR_ERR_RET(*dev->dev_ops->sym_session_configure, -EN= OTSUP); >=20 > if (sess->sess_data[index].refcnt =3D=3D 0) { > - sess->sess_data[index].data =3D (void *)((uint8_t *)sess = + > + sess->sess_data[index].data =3D RTE_PTR_ALIGN_CEIL( > + (void *)((uint8_t *)sess + > rte_cryptodev_sym_get_header_session_size= () + > - (index * sess->priv_sz)); > - sess_iova =3D rte_mempool_virt2iova(sess) + > + (index * sess->priv_sz)), RTE_CACHE_LINE_= SIZE); > + sess_iova =3D RTE_ALIGN_CEIL(rte_mempool_virt2iova(sess) = + > rte_cryptodev_sym_get_header_session_size= () + > - (index * sess->priv_sz); > + (index * sess->priv_sz), RTE_CACHE_LINE_S= IZE); > ret =3D dev->dev_ops->sym_session_configure(dev, xforms, > sess->sess_data[index].data, sess_iova); > if (ret < 0) { > @@ -1805,7 +1806,7 @@ get_max_sym_sess_priv_sz(void) > if (sz > max_sz) > max_sz =3D sz; > } > - return max_sz; > + return RTE_ALIGN_CEIL(max_sz,RTE_CACHE_LINE_SIZE); > } >=20 > struct rte_mempool * Yep, aligning each PMD private data on CACHE_LINE will help to overcome tha= t issue. Though it means that we need to allocate extra CACHE_LINE bytes for each se= ss_data element. That could be a significant amount. Also I am still not sure that cryptodev layer should allocate/manage space= for PMD private session data. It would be really hard to predict all possible requi= rements that each PMD can have. I think better to leave it to PMD itself, as it knows be= st what it needs. > -------------------------------------------------------------------------= --------------- > > > > All in all - these changes for (remove second mempool, change the way w= e > > allocate/setup > > session private data) seems premature to me. > > So, I think to go ahead with this series (hiding rte_cryptodev_sym_sess= ion) > > for 21.11 > > we need to drop changes for sess_data[] management allocation and keep > > only changes > > directly related to hide sym_session. > > My apologies for not reviewing/testing properly that series earlier. > > >=20 > The changes are huge and will affect a lot of people. We needed help > From all the pmd owners to look into this. Agree. > We can drop this series, citing not enough review happened, but the issue= s > that were raised could have been resolved till RC2 for the cases that are= currently > broken. > However, there is one more issue that was not highlighted here was that, = in case of > Scheduler PMD there are a lot of inappropriate stuff which hampers these = changes. > Because of which we will end up reserving huge memory space which will be= unused > if scheduler PMD is compiled in. > We can have a simple single API for session creation similar to rte_secur= ity. > And let scheduler PMD manage all the memory by itself for all the PMDs wh= ich > it want to schedule. Yes, same thoughts here: if we can have just one device per session (as we have for security) it wou= ld help to come up with simple and clean approach. >From my perspective, probably better to create a simple, clean API first, and then try to re-work scheduler PMD to work with new one. =20 =20 > We can defer this series for now, and can work on Asymmetric crypto > first (probably in 22.02) which is still in experimental state. This will= help in getting > these changes matured enough for sym session which we can take up in 22.1= 1. Sounds like a good plan to me. > I believe Intel people are planning for new features in asymmetric crypto= . > It makes more sense that they can align it as per the discussed approach. >=20 > Regards, > Akhil