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 3C4B7A00C3; Thu, 20 Jan 2022 11:51:13 +0100 (CET) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id F10A9426E0; Thu, 20 Jan 2022 11:51:12 +0100 (CET) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 4EC54426DF for ; Thu, 20 Jan 2022 11:51:11 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1642675871; x=1674211871; h=from:to:cc:subject:date:message-id:references: in-reply-to:content-transfer-encoding:mime-version; bh=/h3DzRnxjzxZXHHhF4/A794N1qpNde8ZqnHZHKtB1uM=; b=abAKXe1UCjvdGzl17msBFeXMTVHz8wQyirpuI5Gd7yb3QNL6/mdNv1rQ kM5SEul/XFSO7z8rzPf6VsxE8/v97eTph5AdoH60WWL09DYb1BiQGnFNt +0J9FGi7Qd6EemapGTGaLi48qL7EI5MXL2dKcu1p4ozN/UuB+pmG1HZ1O HGPMIuhGx5ljk2uTmCHjBq+KqYq3clM7mSvAky8VphI1y8gGMR2eEwjO5 jsxK++K7ZrJxm187SUPDgAvbnd3I93kMAnssNTVweWYEfnM2yUKp+yttp ror6WnHbZxwVI1B0Q18WLCgTPppkTVjA52PHIZrWKkyKpbcxQhrmXBqmV g==; X-IronPort-AV: E=McAfee;i="6200,9189,10232"; a="306055751" X-IronPort-AV: E=Sophos;i="5.88,302,1635231600"; d="scan'208";a="306055751" Received: from fmsmga003.fm.intel.com ([10.253.24.29]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 20 Jan 2022 02:51:10 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.88,302,1635231600"; d="scan'208";a="616047398" Received: from fmsmsx601.amr.corp.intel.com ([10.18.126.81]) by FMSMGA003.fm.intel.com with ESMTP; 20 Jan 2022 02:51:10 -0800 Received: from fmsmsx609.amr.corp.intel.com (10.18.126.89) by fmsmsx601.amr.corp.intel.com (10.18.126.81) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.20; Thu, 20 Jan 2022 02:51:09 -0800 Received: from fmsedg602.ED.cps.intel.com (10.1.192.136) by fmsmsx609.amr.corp.intel.com (10.18.126.89) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.2308.20 via Frontend Transport; Thu, 20 Jan 2022 02:51:09 -0800 Received: from NAM04-DM6-obe.outbound.protection.outlook.com (104.47.73.41) by edgegateway.intel.com (192.55.55.71) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.2308.20; Thu, 20 Jan 2022 02:51:09 -0800 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=KSWSEuxpPFelCpTRICOmx8rMuzDir926KvbTilGR1wM11cqnjPiFrEIveYbd25JkdWtJInYxHBU9iPaQ+YvALj8ooDD8bkzwPNuVWIoTI+jxp/h6ub9NG5rlEu9YEvcB+QmzRXGnThnSahvhX8SkT+LIMUhp6Fpi5b6XEjo2ykM3mjaqhvq11YaNw+4EtNdWGvHjiL6tujp1dQTIrggtiVOB8CmquYRVwGR6vFzDcerndPD546DM+ODTRriLJzezqlEOYKIADTAqyvD8EsRA3HEhrh+BXZqA+2AZWZ8qj40sXV6dFcVOqamCtoL/SB4l6zVU1+fDDR2SUt1QGs/1+Q== 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=srULTD4CFQUFNP7837uB5wL25gZFwI+ldp9kuzHbzIY=; b=R6XC2f3hNG0dcpiHXhG7duna+aubm+GP76ZqM9UA8wTpti61DCavDq0eYY2+gXFp0/YeU97Yaab7R9aJ9KJ94F/pcFw3AEWxWMoImGncr+mz4Wxs4dGaVx7zlu1BxCCiYP6edLDwlOoVh5NkE4YWo45kZoIHy61OvbNkcn4pzh3p3f0vXr7/wfL2nSJg53/bNq/rZ2zZ4Jmwmvt6m4rFafF7ZvIOwmnv6ZV4keRiukweTbFtXja7bHrz7fL75gdgg++kBXXCtafisJfIJRC6w/dU4pmokWiM7U2Br1SQbwnvFTX7wMRDGVkKvI5A6qmqpEh4J22dpua622x3L9PWGw== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=none; dmarc=none; dkim=none; arc=none Received: from MN2PR11MB3821.namprd11.prod.outlook.com (2603:10b6:208:f7::24) by DM4PR11MB5486.namprd11.prod.outlook.com (2603:10b6:5:39e::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.4888.11; Thu, 20 Jan 2022 10:51:08 +0000 Received: from MN2PR11MB3821.namprd11.prod.outlook.com ([fe80::4fd:a9c7:1eda:c93d]) by MN2PR11MB3821.namprd11.prod.outlook.com ([fe80::4fd:a9c7:1eda:c93d%6]) with mapi id 15.20.4909.010; Thu, 20 Jan 2022 10:51:08 +0000 From: "Power, Ciara" To: Anoob Joseph , "dev@dpdk.org" CC: "Zhang, Roy Fan" , Akhil Goyal , "Doherty, Declan" , "Ankur Dwivedi" , Tejasree Kondoj , "Griffin, John" , "Trahe, Fiona" , "Jain, Deepak K" , "Ray Kinsella" Subject: RE: [EXT] [PATCH] crypto: use single buffer for asymmetric session Thread-Topic: [EXT] [PATCH] crypto: use single buffer for asymmetric session Thread-Index: AQHX8DK0iOMULGZgskuq+JgCz08yjKwwnf+AgDtBjfA= Date: Thu, 20 Jan 2022 10:51:07 +0000 Message-ID: References: <20211213150402.3351032-1-ciara.power@intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-version: 11.6.200.16 dlp-product: dlpe-windows dlp-reaction: no-action authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=intel.com; x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: cd2c40b1-2eca-4563-796c-08d9dc02c3b8 x-ms-traffictypediagnostic: DM4PR11MB5486:EE_ x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-ms-exchange-antispam-relay: 0 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: HqZaKl1Ip9XrSKhFjl7p2+KtwUMSNuZ+K3GWNZnlhAID29vCD6U0xg86Tw72mXxy08sy8tRyYfzb92GVfwJjnydvvwQ3W4rxrgzuE5i15L9BZSTqxo0q3Bk6OX7gJ9xJXxkyW9bg5n7oFTxe8cXiGtRmMMSOdX/DCsFRBjvMzbg2IIFVTBNNU9x2jv4Uif919dssiBCa8/XR7auDQ0h/tI93nSuuXWUj9GNo0aV8WfrHCHMohE5Kl7G8KE0iYHXMlCNm8wAPV7X70CnkHuMMz2fIlH5AMBPfxROFUQs/TYDDPPQX6Pguw0gL8mvzag7V8e2/BhPT6jouI0e1VXCUUVaDvIRFCYHBBhPdm5jWgyRc8gxQXi2T0Vc2SD36wVNdLyJ3GebQbBhvyNu6us9+O3YHB7p1oa8/ZYTibWbOklvc5N0GhVMRvenN/ei+/TUM42u5f/F6RQfYULnlOAo+04Gxohw77vXVzr0kMikQRA0nC0wvnFQgMP8hAPURApzmRaLalqUhPburxDul0QOaF7UOZlI8N1krLxaOBlXtYOlhcbiQv+SjXPhNap2iZkXw6+6/yguBnPN+1BGAXc2Trc3Qkc3DMpE8YiEi8m1aMgnCC4Vd79wRmWnqsFBjEBC8M8K8a1JzxpQ5HI0pe84LKOcOBnbWrhdVY+Vrjb97PEL0g3zFuS2u8r7AlqY729dM1e8+AukqpcjFJIFNX8auUg== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MN2PR11MB3821.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(366004)(66946007)(82960400001)(55016003)(66446008)(53546011)(76116006)(71200400001)(2906002)(508600001)(66476007)(83380400001)(4326008)(9686003)(86362001)(8936002)(6506007)(7696005)(122000001)(52536014)(186003)(5660300002)(54906003)(8676002)(64756008)(110136005)(316002)(66556008)(38100700002)(38070700005)(33656002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata-chunkcount: 1 x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?Rx70pHhFoMHnktyUlm31kGzDFTXKigd2gIxmCTnWOwdr6/OINe314pq1VbnC?= =?us-ascii?Q?jtolQ0XwZOZ/cKgkywm3B7O/BjYLqU2FMzIuQ8cpjhBt0rHGZC6UHjyzeq3O?= =?us-ascii?Q?EGwsBg2pzbnDwJn836jiswVTtGIT1IczqbEhV589gdpz8IFLstxcBp/ZlTNT?= =?us-ascii?Q?IDvLqrtJpPU5r409JnHPFTol3urtV+1APOM6rb0AszLgZM7Kv7N1M8OA4vMB?= =?us-ascii?Q?jKG22zZy21oTzw33sQHG2zdNeJETMpUFD4Lu7WGZfZKewkxU3f1nK7/iLuQg?= =?us-ascii?Q?zjY/4pjf0INVRcgCQAv6jutRZBMl9jBCVVeOLpEnUHjO4ohPN6ym/SArxDkS?= =?us-ascii?Q?2peC2GxuJPrnPUhbIR3FrfaF41uMw4cVrOk9UTi5fcjLlSRsdvwiUV5Nibbw?= =?us-ascii?Q?JqGQN0jy/GgLTliKGTg9iKrWL1d8q5qMGFnp+4eRUxmQD58HJAczC5AkuIQP?= =?us-ascii?Q?PQkxzInoroLEO5Ji+FgoSc4vkUVihybRVoAbXxJn7cJdV9fD1CvxtBPX0+23?= =?us-ascii?Q?VAz3FbFDVYE6Y1zZkmCFJ6jkWFXnmVJ4waXr7QaSRtO/CPLEjz2jBPHJvUNw?= =?us-ascii?Q?AamZEbTxeu7h9LhnIgayAdT62KQ2KJhgh1Umepx1b4ytW6VoxQW0m5ocwVbj?= =?us-ascii?Q?JUdraI1sJyRXLzh0CZk4YdUMWqvuV1pmHLmFTDps/RtGzdYPoT/DFQo1KchI?= =?us-ascii?Q?eVHCR3Opo44uGivEaGmerOmxnhpOUpJGme7tRpAvC2AAV2pRHQtSlBqzu0cM?= =?us-ascii?Q?0wQqKyJ1lF6N60DO2yHsmv3w+6xt96B44NOu9VUS15RiIX42V/vcWkogCiW2?= =?us-ascii?Q?/lD2uvl0rlK0tTPNXBfskqc4csRbKVGCOy1f3OwY94F3CU3e8ZSwXseg1dJ8?= =?us-ascii?Q?nsAuF+fx3ToE5q49heiJgJxdlGoEJFTUqI+fXS12JwAlssMZQyFMmwYF/O2i?= =?us-ascii?Q?t0DK54Oawhghg2bzJxFeKhhW0XVKvyA0Rd6gU2cST99nwaEqez5RXYbrqHMb?= =?us-ascii?Q?kQxGraZErWximfMPUVGTnNhg7PT545w3pbT4RZb+/7XItMf59SanB/Kmi8ml?= =?us-ascii?Q?nqozYJqupjAKTVAl71WU2VWQmu4pkjLLklwEMdg+LfI2Ociy3HhgSG2tAAWI?= =?us-ascii?Q?TKSD2rCQ12qiRa5nkQs/ohWLlv8mtNmUyC+nY3LEbfO2bwWGFFC8lDLSgzav?= =?us-ascii?Q?tIL8VhfNk7NmgeLQiXHxGRtZmXUR5pLfYNIysx+g8uFO0aZ0/8SoaS0WNgzY?= =?us-ascii?Q?PEO4hMaQ4ksGvxiBT8cneRzuHs1sFqo79JCGmfwccGjAxaGGzsOrCgoCkgsx?= =?us-ascii?Q?PttcqaZnla86L4wJBkWY91HsnDQ7ppU067PcZFyR+ywbaz0kfbzI7UQjPlV/?= =?us-ascii?Q?SAROKx8175WqmT/S7YfOLy9SgfWEgKggxlcpk/S96o7U5w5kbhqMfw4bP8qg?= =?us-ascii?Q?+Lx1iZ0QIFmyiAmHVU6tYrFxRFC+5Idi3fDiTboDZNy+05iXJ8Ie4zzlXFT4?= =?us-ascii?Q?uSrnFnNxf6jLY6/lFjCd0s4h7W7AKLQmWZbBV2b5bdoo+qkzz4yGcZjyVT+E?= =?us-ascii?Q?Cu7ncRQC4LXJNnQdss0DUauP4FL/GdmYvnu2v5JuJemzUgE7HD9UAhVDBolH?= =?us-ascii?Q?pPuVHI9twW5FrwOfxCUfuLOwDwUMLukruXoMNbXtvAkrWky4q6ncrkebI8R4?= =?us-ascii?Q?7b7R6w2zM+VJEz0esA81NkbMikE=3D?= 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: MN2PR11MB3821.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: cd2c40b1-2eca-4563-796c-08d9dc02c3b8 X-MS-Exchange-CrossTenant-originalarrivaltime: 20 Jan 2022 10:51:08.0078 (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: ducdXa+S3SdRuZKMcPyOEI6in9Y5nVYkuYMI2LvVIRzcdgjMKPbOo1ymuQeXeONVlwnXW7NsHZM9FCzdHtntIg== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM4PR11MB5486 X-OriginatorOrg: intel.com 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 Hi Anoob, Thanks for the review, apologies for the delay in reply. Comments inline. Thanks, Ciara >-----Original Message----- >From: Anoob Joseph >Sent: Monday 13 December 2021 16:34 >To: Power, Ciara ; dev@dpdk.org >Cc: Zhang, Roy Fan ; Akhil Goyal >; Doherty, Declan ; Ankur >Dwivedi ; Tejasree Kondoj >; Griffin, John ; Trahe, >Fiona ; Jain, Deepak K ; >Ray Kinsella >Subject: RE: [EXT] [PATCH] crypto: use single buffer for asymmetric sessio= n > >Hi Ciara, > >+1 to the overall approach. Few comments inline. > >Thanks, >Anoob > >> -----Original Message----- >> From: Ciara Power >> Sent: Monday, December 13, 2021 8:34 PM >> To: dev@dpdk.org >> Cc: roy.fan.zhang@intel.com; Akhil Goyal ; Ciara >> Power ; Declan Doherty >> ; Ankur Dwivedi ; >> Anoob Joseph ; Tejasree Kondoj >> ; John Griffin ; Fiona >> Trahe ; Deepak Kumar Jain >> ; Ray Kinsella >> Subject: [EXT] [PATCH] crypto: use single buffer for asymmetric >> session >> >> External Email >> >> ---------------------------------------------------------------------- >> Rather than using a session buffer that contains pointers to private >> session data elsewhere, have a single session buffer. >> This session is created for a driver ID, and the mempool element >> contains space for the max session private data needed for any driver. >> >> Signed-off-by: Ciara Power >> >> --- >> Hiding the asym session structure by moving it to an internal header >> will be implemented in a later version of this patch. >> --- >> app/test-crypto-perf/cperf_ops.c | 14 +- >> app/test/test_cryptodev_asym.c | 204 ++++-------------- >> drivers/crypto/cnxk/cn10k_cryptodev_ops.c | 6 +- >> drivers/crypto/cnxk/cn9k_cryptodev_ops.c | 6 +- >> drivers/crypto/cnxk/cnxk_cryptodev_ops.c | 11 +- >> drivers/crypto/octeontx/otx_cryptodev_ops.c | 29 +-- >> drivers/crypto/octeontx2/otx2_cryptodev_ops.c | 25 +-- >> drivers/crypto/openssl/rte_openssl_pmd.c | 5 +- >> drivers/crypto/openssl/rte_openssl_pmd_ops.c | 23 +- >> drivers/crypto/qat/qat_asym.c | 35 +-- >> lib/cryptodev/cryptodev_pmd.h | 11 +- >> lib/cryptodev/cryptodev_trace_points.c | 3 + >> lib/cryptodev/rte_cryptodev.c | 199 +++++++++++------ >> lib/cryptodev/rte_cryptodev.h | 107 ++++++--- >> lib/cryptodev/rte_cryptodev_trace.h | 12 ++ >> lib/cryptodev/version.map | 6 +- >> 16 files changed, 302 insertions(+), 394 deletions(-) >> > >[snip] > >> diff --git a/lib/cryptodev/rte_cryptodev.h >> b/lib/cryptodev/rte_cryptodev.h index 59ea5a54df..11a62bb555 100644 >> --- a/lib/cryptodev/rte_cryptodev.h >> +++ b/lib/cryptodev/rte_cryptodev.h >> @@ -919,9 +919,15 @@ struct rte_cryptodev_sym_session { }; >> >> /** Cryptodev asymmetric crypto session */ -struct >> rte_cryptodev_asym_session { >> - __extension__ void *sess_private_data[0]; >> - /**< Private asymmetric session material */ >> +__extension__ struct rte_cryptodev_asym_session { >> + uint8_t driver_id; >> + /**< Session driver ID. */ >> + uint8_t max_priv_session_sz; >> + /**< size of private session data used when creating mempool */ >> + uint16_t user_data_sz; >> + /**< session user data will be placed after sess_data */ >> + uint8_t padding[4]; >> + uint8_t sess_private_data[0]; >> }; > >[Anoob] Should we add a uint64_t member to hold IOVA address of, may be, >rte_cryptodev_asym_session()? IOVA address could be required for >hardware PMDs. And typically rte_mempool_virt2iova() used to help in that. >Also, did you consider whether this layout of crypto session can be kept >uniform across sym, asym & security? There is no asym specific field in th= is >struct, right? > [CP]=20 Do you think mempool would not be used for session in some cases? I guess t= he IOVA address would be needed in that case. Yes, I believe this approach for session could be applicable to the sym and= security sessions also, but for now we are only doing the asym session cha= nge, but I think they should all be aligned in future releases. >> -/** >> - * Initialize asymmetric session on a device with specific asymmetric >> xform >> - * >> - * @param dev_id ID of device that we want the session to be used o= n >> - * @param sess Session to be set up on a device >> - * @param xforms Asymmetric crypto transform operations to apply on >> flow >> - * processed with this session >> - * @param mempool Mempool to be used for internal allocation. >> - * >> - * @return >> - * - On success, zero. >> - * - -EINVAL if input parameters are invalid. >> - * - -ENOTSUP if crypto device does not support the crypto transform. > >[Anoob] API rte_cryptodev_asym_session_create() returning NULL is treated >as an error. But error can be either due to -EINVAL/-ENOMEM/-ENOTSUP, in >which -ENOTSUP is typically used by PMD to declare unsupported >combinations of xforms. Should we clarify this in the API description? > >Also, none of rte_cryptodev_asym_session_create() calls in validation test= s >consider the API returning NULL due to -ENOTSUP. For sym crypto test cases= , >API returning -ENOTSUP was used to skip the test. Can you update the tests >such that returning NULL would mean test is skipped? Agreed that current >code also doesn't handle -ENOTSUP case returned by init API. > [CP]=20 Skipping for NULL return would mean it would also count as skipped for erro= r cases other than -ENOTSUP, which isn't ideal. In v2 I will change rte_cryptodev_asym_session_create() to return an int va= lue (and just pass in session to be used as input), this will allow the error returns to be treated correctly. >> - * - -ENOMEM if the private session could not be allocated. >> - */ >> -__rte_experimental >> -int >> -rte_cryptodev_asym_session_init(uint8_t dev_id, >> - struct rte_cryptodev_asym_session *sess, >> - struct rte_crypto_asym_xform *xforms, >> - struct rte_mempool *mempool); >> - >> /** >> * Frees private data for the device id, based on its device type, >> * returning it to its mempool. It is the application's >> responsibility @@ - >> 1075,14 +1088,13 @@ rte_cryptodev_sym_session_clear(uint8_t dev_id, >> struct rte_cryptodev_sym_session *sess); >> > >[snip] > >> diff --git a/lib/cryptodev/version.map b/lib/cryptodev/version.map >> index >> c50745fa8c..00b1c9ae35 100644 >> --- a/lib/cryptodev/version.map >> +++ b/lib/cryptodev/version.map >> @@ -58,7 +58,6 @@ EXPERIMENTAL { >> rte_cryptodev_asym_session_clear; >> rte_cryptodev_asym_session_create; >> rte_cryptodev_asym_session_free; >> - rte_cryptodev_asym_session_init; >> rte_cryptodev_asym_xform_capability_check_modlen; >> rte_cryptodev_asym_xform_capability_check_optype; >> rte_cryptodev_sym_cpu_crypto_process; >> @@ -104,6 +103,11 @@ EXPERIMENTAL { >> rte_cryptodev_remove_deq_callback; >> rte_cryptodev_remove_enq_callback; >> >> + # added 22.03 > >+1 for get & set user_data API. Ideally it should have been a separate ser= ies >but I agree that it's better getting addressed along with the session rewo= rk. > [CP]=20 Agreed, for v2 I have split it into its own patch in this patchset rather t= han squashing it with other session changes.