From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id 5C433A04BC; Fri, 9 Oct 2020 16:40:45 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 38FF71D6FA; Fri, 9 Oct 2020 16:40:44 +0200 (CEST) Received: from mga03.intel.com (mga03.intel.com [134.134.136.65]) by dpdk.org (Postfix) with ESMTP id 6701B1D705 for ; Fri, 9 Oct 2020 16:40:42 +0200 (CEST) IronPort-SDR: MuxOaqi0TblM7uqYfSy39Sco4nUiTy04Oh/T3fHJi/xdMSkFLosn9ND0J9LrmnVE3nbJ2U8O+q h4glrGoeePHQ== X-IronPort-AV: E=McAfee;i="6000,8403,9768"; a="165553438" X-IronPort-AV: E=Sophos;i="5.77,355,1596524400"; d="scan'208";a="165553438" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga005.fm.intel.com ([10.253.24.32]) by orsmga103.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 09 Oct 2020 07:40:33 -0700 IronPort-SDR: Rb4rbire7lBAKV/CqyVJfBGf3Tashemdpwx+OSqmxEAf0HdSHGGv8IvlmdqJjlnvIhvwGwoh7E 9bFrj17+Frpw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,355,1596524400"; d="scan'208";a="519740517" Received: from orsmsx601.amr.corp.intel.com ([10.22.229.14]) by fmsmga005.fm.intel.com with ESMTP; 09 Oct 2020 07:40:33 -0700 Received: from orsmsx608.amr.corp.intel.com (10.22.229.21) by ORSMSX601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 9 Oct 2020 07:40:32 -0700 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) by ORSMSX608.amr.corp.intel.com (10.22.229.21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Fri, 9 Oct 2020 07:40:32 -0700 Received: from ORSEDG601.ED.cps.intel.com (10.7.248.6) by orsmsx601.amr.corp.intel.com (10.22.229.14) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Fri, 9 Oct 2020 07:40:32 -0700 Received: from NAM12-BN8-obe.outbound.protection.outlook.com (104.47.55.176) 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.1713.5; Fri, 9 Oct 2020 07:40:32 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=WfpWmvGEX8fPIDnJPxozXBjbANQHqVoieLVKV7Rseg/Z+x8lJJjG5kDHXW1jzCpMYbSI7cwVdQ2Ni21/CFm9CnAwTzskvhxR61XdMqG58BF5l80Jf7Xt12tbKg9Qpf/jPO7lu+9L0L9u1qhmSPLdqpTNMU0EUADoQmbv/UWojUXXWwVbPad8KX/xr47KXPAbp0+D17+nan0ezsBa10nSj1RrHkcxzaOR8XWQV+BuQeU2Ls35MBfu3jUEJo19pUhZx4X0I9Q2oVsvRN2T3XndqxZl2cY2tiuHB3g6nqCPuJZ0tojnTLjQsTpOhJdqpVT/53W4i8meL9mUcANNa+zfWg== 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-SenderADCheck; bh=Fmtb8TPxfVrPYAvbQ4uYBmTLPrJxxVNhD9wbwy6kGso=; b=eqpkiNCHscZXgpI5+3Luiei4fVLmvtusuiJIVS4Y+41uEe5bK110AwX0Ij22VJJ+KtxR6vuRB4MoYOijy0wvhiMuiVarkUpt7a2rXUIhdOfchHb6KOdr5vcgskU9/qaECaLH2/4ZVqsFXF2JL2PUYeEEnE50n29d9cmGChYqz4EuIP7Gd+VvVv5d0r1/4f9EGlgqaH3SwJmGRUFeCZE+xkxVCD4eXXYfiYLmqSHIhSPZhnZgA6ABLtnzssxWHnbkXjSRWXJqU3qdh0Cx3RpSp18ZjAqqoW4NNjpek9VgdAJNguP0AYDpcVcIyrLTZOZfh6gnpRH++Faz0sMV0ZDhZg== 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=Fmtb8TPxfVrPYAvbQ4uYBmTLPrJxxVNhD9wbwy6kGso=; b=XIwN4vU4jKMvbyuYxkypCEpI9SBFClxekAXwcpfyhuuQ78anVnafZV58eaz/5G9+Xnjcu5s0JSU8ee4jE/s2vIMba4J59b+3G/Gme1/N7D7VOjlLxAA9haZJtlaNU8hR9//4aDJo0rYhNfJfCQScI5kvNBtcXIVzjVYs9Pp8KIU= Received: from BYAPR11MB3301.namprd11.prod.outlook.com (2603:10b6:a03:7f::26) by BYAPR11MB2680.namprd11.prod.outlook.com (2603:10b6:a02:c9::21) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3455.21; Fri, 9 Oct 2020 14:40:29 +0000 Received: from BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f5a4:3f6b:ade3:296b]) by BYAPR11MB3301.namprd11.prod.outlook.com ([fe80::f5a4:3f6b:ade3:296b%3]) with mapi id 15.20.3455.027; Fri, 9 Oct 2020 14:40:29 +0000 From: "Ananyev, Konstantin" To: "Gujjar, Abhinandan S" , Honnappa Nagarahalli , "dev@dpdk.org" , "Doherty, Declan" CC: "jerinj@marvell.com" , "Akhil.goyal@nxp.com" , "Vangati, Narender" , nd , nd Thread-Topic: [dpdk-dev] [v2 1/2] cryptodev: support enqueue callback functions Thread-Index: AQHWhsTbY+pAzzKww0euN2orABmUValrRWzAgAAl9gCAAW0pMIAGJnUAgAAH/lCAAp3uAIAAgn/ggADASgCAASzNQIAHHMUAgAA/FzCADrUggIABX7xw Date: Fri, 9 Oct 2020 14:40:29 +0000 Message-ID: References: <1599549024-195051-1-git-send-email-abhinandan.gujjar@intel.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.5.1.3 authentication-results: intel.com; dkim=none (message not signed) header.d=none;intel.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [46.7.39.127] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: c2e18115-ae90-4d6a-eb86-08d86c6144d3 x-ms-traffictypediagnostic: BYAPR11MB2680: x-ld-processed: 46c98d88-e344-4ed4-8496-4ed7712e255d,ExtAddr x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: fBXs5sECC60BdeTeuGwRVzrqTX/SsVUNJBq+QzxxfhzSVknZNWkZHDXpLgjDivy/f0+yPusQyYRmTXMnvxeQDuK9Gb4SGnPT5XZyRKWRi698kjdy+uvJBxK3x9A86WiUPBhqm6i8BwAEmJqcYLs0W7eicT33pxhuofQSZrFbudSk+DWmsjkFmerPzC4/OQvYsYd2QNywRTIhge/6iROb6Yknkv8xdXQEq7IOBbFBP0WOV8Seyto5xBeQ5KCq+Ga8YvOuzql6b2RqMMABoTaeU2In7tiB1jIvpKppJwyxmYsRZVP2TPg6lzBMHT+Go2PU1L8aYi52qc5DFixv7bGgEBU3X9GP3eiIftEnrPQ7rrDqefj3x/6yyWfz04o/NrL40RaZOiDiUfMuDWI19351cg== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BYAPR11MB3301.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(39860400002)(346002)(396003)(136003)(376002)(83380400001)(110136005)(83080400001)(6636002)(54906003)(26005)(186003)(5660300002)(316002)(8676002)(71200400001)(66946007)(55016002)(76116006)(30864003)(66556008)(66476007)(64756008)(8936002)(966005)(66446008)(9686003)(52536014)(7696005)(2906002)(6506007)(86362001)(478600001)(4326008)(53546011)(33656002); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: JTQyC2M/7Gfk9zYg3hxA+SwMzBcSqJK+mtwVsJ9qCxXtsVBrfsgVOjw8/ZhzIIyKRM2a38O7+atSFTXTepcdsTSa21V0Ko9jE8oZWUXWKfqKiTNO2k9DKIi5t1afgPydD0WgSGUwOAJ9EJ7dG93y3NpntbYidGXX956eHQF2biMSzOmVHk4KnG/ZZs5mFo5Xux1ntrrjnORrjNe+VxFU3mdnIWQ0iWE4V1V+u0AYKDayIvNE87ydlqV8O5xwK8tMC2HV3nVL2nXnKeho3yOB1vxpJjmN/G2pWHPsAlD5LWhT9AFTQgZUIMsK6Xc3K5TJXNsm+j7jY67D6Ek/p9UTIh+fvL8EFumUBERZGMw4YjKU8oWUBxeV1yryXsT08z4Fo6mAfX9P9kU5tNoNnH+zRGuqoKSDw9/TOXtnDjtmdDxrsrK9VT7htQNxmiXwRA2v4StfCtSiU2q8wvZJilCcWvbmQz2uLB3yxJQGVe+PPyN40uJJk/WTh+Tf76dZRuFptn8MbzcVzBxdhUnwlGqaPcdyEl/akdr3S/Y4hALjwjHz5ZkCN9LiQTOn3aQtUMKskZB0Y2b/bLLQW3XzvsBfKd8wyRL+WZ/PbRzXx/CIlagcX7vUdSIF121GyIvDlG5o31OwvSccc0sbqvu2+FIqfg== 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: BYAPR11MB3301.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: c2e18115-ae90-4d6a-eb86-08d86c6144d3 X-MS-Exchange-CrossTenant-originalarrivaltime: 09 Oct 2020 14:40:29.3003 (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: OlmXd0vqPp61VdcudQbxMaZdCIPZhQnvZbe0joxFjZYrha+CxPUE12RsnHdIOUOCCDP1MINMt8FkHs4V2NQVZY101aH/IdTIzI0Zj8B+31w= X-MS-Exchange-Transport-CrossTenantHeadersStamped: BYAPR11MB2680 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [v2 1/2] cryptodev: support enqueue callback functions X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Hi Abhinandan, >=20 > Hi Konstantin & Honnappa, >=20 > Thanks for all the inputs and feedback. >=20 > @Ananyev, Konstantin, > I have measured the perf with and without callback on xeon. Here are the = numbers: >=20 > ./app/dpdk-test-crypto-perf -l 6-7 --vdev=3D"crypto_openssl0,socket_id=3D= 0,max_nb_sessions=3D128" -- --ptest throughput --devtype > crypto_openssl --optype cipher-then-auth --cipher-algo aes-cbc --cipher-o= p encrypt --cipher-key-sz 16 --auth-algo sha1-hmac --auth-op > generate --auth-key-sz 64 --digest-sz 12 --total-ops 10000000 --burst-sz = 32 --buffer-sz 64 >=20 > With callback(+ RCU - totally opaque to data-plane threads) > lcore id Buf Size Burst Size Enqueued Dequeued Failed Enq = Failed Deq MOps Gbps Cycles/Buf > 7 64 32 10000000 10000000 = 0 0 0.8129 0.4162 2694.0= 9 > 7 64 32 10000000 10000000 = 0 0 0.8152 0.4174 2686.3= 1 > 7 64 32 10000000 10000000 = 0 0 0.8198 0.4197 2671.4= 8 >=20 > Without callback: > lcore id Buf Size Burst Size Enqueued Dequeued Failed Enq = Failed Deq MOps Gbps Cycles/Buf >=20 > 7 64 32 10000000 10000000 = 0 0 0.8234 0.4216 2659.8= 1 > 7 64 32 10000000 10000000 = 0 0 0.8247 0.4222 2655.6= 3 > 7 64 32 10000000 10000000 = 0 0 0.8123 0.4159 2695.9= 0 Just to cofirm: You implemented crypto enqueue callbacks using RCU QSBR online /offline (as suggested below) and numbers above are for: 1) callback code in place and some dummy callback installed 2) callback code in place but no callbacks installed=20 =20 Is my understanding correct here? Thanks Konstantin =20 >=20 > Regards > Abhinandan >=20 > > -----Original Message----- > > From: Ananyev, Konstantin > > Sent: Tuesday, September 29, 2020 2:33 PM > > To: Honnappa Nagarahalli ; Gujjar, > > Abhinandan S ; dev@dpdk.org; Doherty, Decl= an > > > > Cc: jerinj@marvell.com; Akhil.goyal@nxp.com; Vangati, Narender > > ; nd ; nd > > Subject: RE: [dpdk-dev] [v2 1/2] cryptodev: support enqueue callback fu= nctions > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#ifdef RTE_CRYPTODEV_CALLBACKS int > > > > > > > > > > > > > +rte_cryptodev_rcu_qsbr_add(uint8_t dev_id, struc= t > > > > > > > > > > > > > +rte_rcu_qsbr > > > > > > > > > > > > > +*qsbr) { > > > > > > > > > > > > > + > > > > > > > > > > > > > + struct rte_cryptodev *dev; > > > > > > > > > > > > > + > > > > > > > > > > > > > + if (!rte_cryptodev_pmd_is_valid_dev(dev_id)) { > > > > > > > > > > > > > + CDEV_LOG_ERR("Invalid dev_id=3D%" PRIu8, > > > > dev_id); > > > > > > > > > > > > > + return -EINVAL; > > > > > > > > > > > > > + } > > > > > > > > > > > > > + > > > > > > > > > > > > > + dev =3D &rte_crypto_devices[dev_id]; > > > > > > > > > > > > > + dev->qsbr =3D qsbr; > > > > > > > > > > > > > + return 0; > > > > > > > > > > > > > +} > > > > > > > > > > > > > > > > > > > > > > > > So if I understand your patch correctly you propose > > > > > > > > > > > > a new working model for > > > > > > > > > > > > crypto-devs: > > > > > > > > > > > > 1. Control-plane has to allocate/setup rcu_qsbr and > > > > > > > > > > > > do rte_cryptodev_rcu_qsbr_add(). > > > > > > > > > > > > 2. Data-plane has somehow to obtain pointer to that > > > > > > > > > > > > rcu_qsbr and wrap > > > > > > > > > > > > cryptodev_enqueue() > > > > > > > > > > > > with rcu_qsbr_quiescent() or > > > > > > rcu_qsbr_online()/rcu_qsbr_offline(). > > > > > > > > > > > Yes. I think, it is not a new model. It is same as RC= U > > > > > > > > > > > integration with > > > > > > > > LPM. > > > > > > > > > > > Please refer: https://patches.dpdk.org/cover/73673/ > > > > > > > > > > > > > > > > > > > > I am talking about new working model for crypto-dev > > > > > > enqueue/dequeue. > > > > > > > > > > As I said above now it becomes data-plane thread respon= sibility > > to: > > > > > > > > > > -somehow to obtain pointer to that rcu_qsbr for each > > > > > > > > > > cryptodev it is > > > > > > > > using. > > > > > > > > > > -call rcu sync functions (quiescent/online/offline) on > > > > > > > > > > a regular > > > > basis. > > > > > > > > > It is not on regular basis. When data plane comes up, the= y > > > > > > > > > report > > > > online. > > > > > > > > > They report quiescent when they are done with critical > > > > > > > > > section or shared > > > > > > > > structure. > > > > > > > > > > > > > > > > I understand that, but it means all existing apps have to b= e > > > > > > > > changed that > > > > > > way. > > > > > > > > > > > > > > > > > All though, there is some dataplane changes involved here= , > > > > > > > > > I don't think, it > > > > > > > > is major. > > > > > > > > > > > > > > > > I still think our goal here should be to make no visible > > > > > > > > changes to the dataplane. > > > > > > > > I.E. all necessary data-plane changes need to be hidden > > > > > > > > inside CB invocation part. > > > > > > > Please note that this is being implemented using the memory > > > > > > > reclamation framework documented at > > > > > > > https://doc.dpdk.org/guides/prog_guide/rcu_lib.html#resource-= r > > > > > > > ecla > > > > > > > mati > > > > > > > on-framework-for-dpdk > > > > > > > > > > > > > > While using RCU there are couple of trade-offs that > > > > > > > applications have to > > > > > > consider: > > > > > > > 1) Performance - reporting the quiescent state too often > > > > > > > results in performance impact on data plane > > > > > > > 2) Amount of outstanding memory to reclaim - reporting less > > > > > > > often results in more outstanding memory to reclaim > > > > > > > > > > > > > > Hence, the quiescent state reporting is left to the applicati= on. > > > > > > > The application decides how often it reports the quiescent > > > > > > > state and has control > > > > > > over the data plane performance and the outstanding memory to > > reclaim. > > > > > > > > > > > > > > When you say "new working model for crypto-dev > > > > > > > enqueue/dequeue", > > > > > > > > > > > > > > 1) are you comparing these with existing crypto-dev > > > > > > > enqueue/dequeue > > > > > > APIs? If yes, these are new APIs, it is not breaking anything. > > > > > > > 2) are you comparing these with existing call back functions > > > > > > > in ethdev enqueue/dequeue APIs? If yes, agree that this is a > > > > > > > new model. But, it is > > > > > > possible to support what ethdev supports along with the RCU > > > > > > method used in this patch. > > > > > > > > > > > > What I am talking about: > > > > > > > > > > > > Existing cryptodev enqueue/dequeue model doesn't require for th= e > > > > > > user to manage any RCU QSBR state manually. > > > > > > I believe that addition of ability to add/remove enqueue/dequeu= e > > > > > > callbacks shouldn't change existing working model. > > > > > > I think that adding/removing such callbacks has to be opaque to > > > > > > the user DP code and shouldn't require user to change it. Same > > > > > > as we have now for ethdev callback implementation. > > > > > The ethdev callback implementation conveniently leaves the proble= m > > > > > of > > > > freeing memory to the user to resolve, it does not handle the issue= . > > > > > Hence, it "looks" to be opaque to the DP code. However, if the > > > > > application has to implement a safe way to free the call back > > > > > memory, its > > > > DP is affected based on call backs are being used or not. > > > > > > > > Yes, I think that's big drawback in initial ethdev callback > > > > implementation - it simply ignores DP/CP sync problem completely. > > > > Though I think it is possible to have both here: > > > > keep callback "opaque" to DP code and provide some sync mechanism > > > > between DP/CP. > > > > Hopefully one day we can fix ethdev callbacks too. > > > The solution we develop can be used in ethdev too. > > > > > > > > > > > > > I think that forcing DP code to be aware that callbacks are > > > > > > present or not and to modify its behaviour depending on that > > > > > > nearly voids the purpose of having callbacks at all. > > > > > > In that case DP can just invoke callback function directly from > > > > > > it's > > > > codepath . > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > Note that now data-plane thread would have to do that > > > > > > > > > > always > > > > > > > > > > - even if there are now callbacks installed for that > > > > > > > > > > cryptodev queue > > > > > > right now. > > > > > > > > > > All that changes behaviour of existing apps and I > > > > > > > > > > presume would reduce adoption of that fature. > > > > > > > If I understand this correct, you are talking about a case > > > > > > > where in the application might be registering/unregistering > > > > > > > multiple times during its lifetime. In this case, yes, the > > > > > > > application might be reporting the > > > > > > quiescent state even when it has not registered the call backs. > > > > > > But, it has the flexibility to not report it if it implements a= dditional logic. > > > > > > > Note that we are assuming that the application has to report > > > > > > > quiescent state only for using callback functions. Most > > > > > > > probably the application has > > > > > > other requirements to use RCU. > > > > > > > Why not support what is done for ethdev call back functions > > > > > > > along with > > > > > > providing RCU method? > > > > > > > > > > > > > > > > There is always trade off involved! > > > > > > > > > In the previous patch, you suggested that some lazy app > > > > > > > > > may not free up the memory allocated by add cb. For such > > > > > > > > > apps, this patch has sync mechanism with some additional > > > > > > > > > cost of CP & DP > > > > changes. > > > > > > > > > > > > > > > > Sigh, it is not about laziness of the app. > > > > > > > > The problem with current ethedev cb mechanism and yours V1 > > > > > > > > (which was just a clone of it) - CP doesn't know when it is > > > > > > > > safe after CB removal to free related memory. > > > > > > > > > > > > > > > > > > I still think all this callback mechanism should be > > > > > > > > > > totally opaque to data-plane threads - user shouldn't > > > > > > > > > > change his app code depending on would some > > > > > > > > > > enqueue/dequeue callbacks be > > > > > > installed or not. > > > > > > > > > I am not sure, how that can be implemented with existing > > > > > > > > > RCU > > > > design. > > > > > > > > > > > > > > > > As I said below the simplest way - with calling rcu > > > > > > > > onine/offline inside CB invocation block. > > > > > > > > That's why I asked you - did you try that approach and what > > > > > > > > is the perf numbers? > > > > > > > > I presume with no callbacks installed the perf change shoul= d > > > > > > > > be nearly > > > > > > zero. > > > > > > > > > > > > > > > > > @Honnappa Nagarahalli, Do you have any suggestions? > > > > > > > Reporting quiescent state in the call back functions has > > > > > > > several > > > > > > disadvantages: > > > > > > > 1) it will have performance impacts and the impacts will > > > > > > > increase as the > > > > > > number of data plane threads increase. > > > > > > > 2) It will require additional configuration parameters to > > > > > > > control how often the quiescent state is reported to control > > > > > > > the performance > > > > impact. > > > > > > > 3) Does not take advantage of the fact that most probably the > > > > > > > application is using RCU already > > > > > > > 4) There are few difficulties as well, please see below. > > > > > > > > > > > > I suggested Abhinandan to use RCU library because it is already > > > > > > there, and I thought it would be good not to re-implement the w= heel. > > > > > > Though if you feel librte_rcu doesn't match that task - fine, > > > > > > let's do it without librte_rcu. > > > > > > After all, what we need here - just an atomic ref count per > > > > > > queue that we are going to increment at entering and leaving > > > > > > list of callbacks inside enqueue/dequeue. > > > > > Ok, looks like I missed the point that a queue is used by a singl= e > > > > > data plane > > > > thread. > > > > > Along with ref count increment you need the memory orderings to > > > > > avoid > > > > race conditions. These will be the same ones used in RCU. > > > > > On the control plane, you need to read this counter and poll for > > > > > the > > > > counter updates. All this is same cost as in RCU. > > > > > > > > Agree. > > > > > > > > > To control the cost, you > > > > > will have to control the rate of quiescent state reporting and > > > > > might have to > > > > expose this as a configuration parameter. > > > > > > > > > > The other important information you have to consider is if the > > > > > thread is making any blocking calls, which may be in some other > > > > > library. The thread is supposed to call rcu_qsbr_thread_offline > > > > > API before calling a > > > > blocking call. This allows the RCU to know that this particular > > > > thread will not report quiescent state. The cryptodev library might= not have > > that information. > > > > > > > > > > If you want to go ahead with this design, you can still use RCU > > > > > with single thread configuration (like you have mentioned below) > > > > > and hide the > > > > details from the application. > > > > > > > > Yes, same thought here - use rcu_qsbr online/offline for DP part > > > > and hide actual sync details inside callback code. > > > We can give it a try. If we can have the performance numbers, we can > > > decide about how to control how often these APIs are called on the da= ta > > plane. > > > > To avoid misunderstanding: I am talking about calling online/offline wi= th every > > cryptodev_enqueue() traversal over CB list. > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > That seems quite a big change and I don't think it > > > > > > > > > > > > is acceptable for most users. > > > > > > > > > > > > From my perspective adding/installing call-backs to > > > > > > > > > > > > the dev has to be opaque to the data-plane code. > > > > > > > > > > > > Also note that different callbacks can be installed > > > > > > > > > > > > by different entities (libs) and might have no idea > > > > > > > > > > > > about each > > > > other. > > > > > > > > > > > > That's why I thought it would be better to make all > > > > > > > > > > > > this RCU stuff internal inside cryptodev: > > > > > > > > > > > > hide all this rcu_qsbr allocation/setup inside > > > > > > > > > > > > cryptod somehow to > > > > > > > > > > obtain pointer to that rcu_qsbr ev init/queue setup > > > > > > > > > > > > invoke rcu_qsbr_online()/rcu_qsbr_offline() > > > > > > > > > > > > inside > > > > > > > > > > cryptodev_enqueue(). > > > > > > > This will bring in the application related information such a= s > > > > > > > the thread ID > > > > > > into the library. > > > > > > > > > > > > I don't think it would. > > > > > > Cryptodev enqueue/dequeue functions are not supposed to be > > > > > > thread safe (same as rx/tx burst). > > > > > > So we can always use RCU with just one thread(thread_id =3D 0). > > > > > Agree, the memory that needs to be freed is accessed by a single > > > > > thread > > > > on the data plane. RCU with one thread would suffice. > > > > > > > > > > > But as I said above - if you feel RCU lib is an overhead here, > > > > > > that's fine - I think it would be easy enough to do without lib= rte_rcu. > > > > > > > > > > > > > If the same API calls are being made from multiple data plane > > > > > > > threads, you need a way to configure that information to the > > > > > > > library. So, it is better to leave those details for the appl= ication to > > handle. > > > > > > > > > > > > > > > > > > I have already tried exploring above stuffs. There ar= e > > > > > > > > > > > too many > > > > > > > > constraints. > > > > > > > > > > > The changes don't fit in, as per RCU design. > > > > > > > > > > > > > > > > > > > > Hmm could you be more specific here - what constraints > > > > > > > > > > are you referring to? > > > > > > > > > > > > > > > > > > > > > Moreover, having rcu api under enqueue_burst() will > > > > > > > > > > > affect the > > > > > > > > > > performance. > > > > > > > > > > > > > > > > > > > > It most likely will. Though my expectation it will > > > > > > > > > > affect performance only when some callbacks are > > > > > > > > > > installed. My thought > > > > > > here: > > > > > > > > > > callback function by itself will affect cryptdev_enqueu= e > > > > > > > > > > performance anyway, > > > > > > > > > With existing callback design, I have measured the > > > > > > > > > performance(with > > > > > > > > crypto perf test) on xeon. > > > > > > > > > It was almost negligible and same was shared with Declan. > > > > > > > > > > > > > > > > I am asking about different thing: did you try alternate > > > > > > > > approach I described, that wouldn't require changes in the > > > > > > > > user data- > > > > plane code. > > > > > > > > > > > > > > > > > That is one of the reasons, I didn't want to add to many > > > > > > > > > stuffs in to the > > > > > > > > callback. > > > > > > > > > The best part of existing design is crypto lib is not muc= h modified. > > > > > > > > > The changes are either pushed to CP or DP. > > > > > > > > > > > > > > > > > > so adding extra overhead for sync is probably ok here. > > > > > > > > > > > > > > > > I think that extra overhead when callbacks are present is > > > > > > > > expected and probably acceptable. > > > > > > > > Changes in the upper-layer data-plane code - probably not. > > > > > > > > > > > > > > > > > > Though for situation when no callbacks are installed - > > > > > > > > > > perfomance should be left unaffected (or impact should > > > > > > > > > > be as small > > > > > > as possible). > > > > > > > > > > > > > > > > > > > > > The changes are more on control plane side, which is = one > > time. > > > > > > > > > > > The data plane changes are minimal. > > > > > > > > > > > > > > > > > > > > I still think upper layer data-plane code should stay > > > > > > > > > > unaffected (zero changes). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >