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 81987A04B6; Mon, 12 Oct 2020 08:47:53 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0DA141D5FA; Mon, 12 Oct 2020 08:47:52 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 5CBC31D5AC for ; Mon, 12 Oct 2020 08:47:50 +0200 (CEST) IronPort-SDR: nii842V0+V9uVFoeZoZpa6wb8/CWIVaXuLqzSodsR0xpS8nUr8ipTJxynhaNSEc6WT1b6dlPNH 0tMDebmotGCA== X-IronPort-AV: E=McAfee;i="6000,8403,9771"; a="152623927" X-IronPort-AV: E=Sophos;i="5.77,366,1596524400"; d="scan'208";a="152623927" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga101.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 11 Oct 2020 23:47:43 -0700 IronPort-SDR: gP6zvi2x34WC+zZ7kfNwDK6idqrVQLZ3XXZnyXZCDf+7iDHanJZ4cDZLkutLmbq1MwUcGiqLPl IgZAzNRrV+tg== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,366,1596524400"; d="scan'208";a="462992607" Received: from orsmsx605.amr.corp.intel.com ([10.22.229.18]) by orsmga004.jf.intel.com with ESMTP; 11 Oct 2020 23:47:43 -0700 Received: from orsmsx611.amr.corp.intel.com (10.22.229.24) 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.1713.5; Sun, 11 Oct 2020 23:47:42 -0700 Received: from orsmsx612.amr.corp.intel.com (10.22.229.25) by ORSMSX611.amr.corp.intel.com (10.22.229.24) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Sun, 11 Oct 2020 23:47:42 -0700 Received: from ORSEDG602.ED.cps.intel.com (10.7.248.7) by orsmsx612.amr.corp.intel.com (10.22.229.25) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5 via Frontend Transport; Sun, 11 Oct 2020 23:47:42 -0700 Received: from NAM12-DM6-obe.outbound.protection.outlook.com (104.47.59.170) by edgegateway.intel.com (134.134.137.103) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.1.1713.5; Sun, 11 Oct 2020 23:47:38 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=O6Lrw5utl+5xmaVXsgs7naQlpIqIT++uCB3GUczvXuzqImy+5uefzN+JlB4ku667sOfinQ540P9AgzKTnT0pMFVWYDoQJLXYaHMxr2X7XkWkRcJPJkRPx5SUn2foytnZUt7m/HTc04JFQM1cJlT8BgWupAwPPpJwX9cKbvQRAcNKIS9QB4+6xJHVUaaOPTiQb3fZ4OH+lgNFnFUzGcfQJxNH0UiDrQe58Ypb1c7v4nZe3DPshufxe4HbMkaHH92hZz+FqPb24GhHKm0Bf83kIY3xHByOmiKKboDBc62M/PWzqeHxvUQP+XpdHMW33SFAOmxkBID0e2R7iBKDLgpRCg== 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=FpJ6iNRYaRWrPtiQ5URSPi6myE3NBddWiBtWn/xyWX8=; b=SnzXUhBPIfWpPAEsUNCp0DOg61tuOhx60DIwIpykBVnm2Tj6w8CDhPhtOsURRqFuZ7UivxfRebRCtoeDvvldVVV+S4K1OYQmjhNdVTavgTgjbIDdq/MJYQTiIo97KwLnc/6KR5KrbWluQA7dD0VxlkDfMq7m0U+w1WlquXqNz+qdHWJ5gG8ZBfn7iCXcP1dWP9/hpskthBeHALxiMYyw8LMW1Z2ddmjnjkWR4pBDBRs5ruUf3JUnTyTGWscBOBnBrpFLVKY4UIONHU9mieLwNylsinmJihnECycXsi/jgsxPXAcoRkLiPdgDKlCZGMVNbQfYSnWd5D2FJnYNiZMR4A== 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=FpJ6iNRYaRWrPtiQ5URSPi6myE3NBddWiBtWn/xyWX8=; b=wPmTwgEemLNALz9P86qQKAS4slDrn+03z6imzc5e8VWsbdtbQ9F+bgR16WwJNsrpt+BEO13KQmhNhIcbuhvRtUMXS+EAB9D7vL7eZtRaquLVGk1QTJ0DNxG5pjnpDefnLsHqUiAhbAttGxgV+fCqru3PbF5h7dDFlWiQoyY+R+g= Received: from MWHPR11MB1838.namprd11.prod.outlook.com (2603:10b6:300:10c::11) by CO1PR11MB4930.namprd11.prod.outlook.com (2603:10b6:303:9b::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3455.29; Mon, 12 Oct 2020 06:47:33 +0000 Received: from MWHPR11MB1838.namprd11.prod.outlook.com ([fe80::20fb:cc03:ce89:f0ea]) by MWHPR11MB1838.namprd11.prod.outlook.com ([fe80::20fb:cc03:ce89:f0ea%7]) with mapi id 15.20.3455.030; Mon, 12 Oct 2020 06:47:33 +0000 From: "Gujjar, Abhinandan S" To: "Ananyev, Konstantin" , 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: AQHWhsTNpueHJn2Z1E+jIJevuNB9zqlrUBQAgAARrrCAAXx3gIAGGrIAgAAS1QCAAk5BgIAA2iGAgACTrsCAAVTYAIAHE6IAgABA9YCADrHi8IABY5mAgAAjLkCAA43NAIAAf+hQ Date: Mon, 12 Oct 2020 06:47:33 +0000 Message-ID: References: <1599549024-195051-1-git-send-email-abhinandan.gujjar@intel.com> In-Reply-To: Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: 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: [103.5.135.70] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 7ea7c778-126a-4112-83fb-08d86e7ab27c x-ms-traffictypediagnostic: CO1PR11MB4930: 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: cA+A3qfmEijFzCbpSw4B7xX2N6el871bcq5OE7D24E9DXEzRfXqXvGiGEZnZZglI4WbqNESlprUuwxasTmUH8Iy6vqAjgzSFlK9iS3Lu8OxzV5rX7m/nnCQ+GrBH9kVQrj44SvqYIA2VxMLTMAasIjK+3jjeotBTUrakgdiwKwszYKzEcrzP0fnVaB0prQ+VZ2mpfepuvUqzZ94G4lZM+UMRgjkBfGKvaQhUTZ8Z1zH6x0FRw7/nRdPSIFJfcC6Sl5y7YXKD7v+gOHotkSjbVibb8bZnzOiIqHYt4bhrWOVpmI5kKlsjAlDYNksEmdRTFF3x0D0bS1oZTaayE1dvm6Gmh/Xvp8RPDJwDkpqDOcwCldDUL+0plAOj+3hpwthUxCPct9UZqMy/Ad34q/AntA== x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:MWHPR11MB1838.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(396003)(346002)(376002)(39860400002)(136003)(5660300002)(86362001)(83080400001)(55016002)(7696005)(30864003)(478600001)(55236004)(54906003)(2906002)(110136005)(4326008)(966005)(26005)(316002)(53546011)(186003)(33656002)(66446008)(66946007)(66476007)(64756008)(66556008)(6506007)(71200400001)(8936002)(6636002)(9686003)(8676002)(52536014)(83380400001)(76116006)(579004); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: jlM1BhcQHntBJ6lKVB2Kj9ut1F4vJPaqeBvjTa0WNr/JdRlXXWonoLIvBar5pH6AY/o7Zp1eWpKdgKe+dF03MJhNvFuGShgT5B7BiNTb2TrMCdoSnnFNDVEKY2EV/xo9AtcMGCd+65YwDdJ0Lckjh49caWBx5rhPxpKYiXBJyKI5mKw4Yb7nviA0LSw20EaFE6kK7UP5vDTFywtIgr9GCoY6JaTi0Rjr5fRnLpxJDZYXn+T/oJN4kdQpGlYKnK/Sfd3Tc6GK5FsxnBOVTbdFrm/gaN7dVj1z17P5g2MaIM5XgfrajD1IZNMWZx6sneRHELjBdPt4XTXBnrnEp/dWuABzVB6WpN3uVpTiCsw2czcFvoHrHpjrFN0Xoxc/QjNTA4n8MRCeyG2z7coLSpUMVwqTrso5giuPZgdcokErnOvufj5gQwaM8JbFY0ONogIWW6mWNXwpFQH/WT1VDqd+LRTp+0aXESac/xusX6Qc7FJgsUt8PuvHd6RlGfIijQxJVO/AatleLC4H9zDzj12bRPeKUsrtQvu/6vLh7Lo8jJS47Mn4/7wMUN6kSs/j9gxXvao/6Pmj8QDV1aC8HfKYtzapvGntUkzfSh1u3/Unk3akLMi8eawKFG8VLm9z5FV9KRS4oC1NQxMVGJ+jU0eV9Q== 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: MWHPR11MB1838.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 7ea7c778-126a-4112-83fb-08d86e7ab27c X-MS-Exchange-CrossTenant-originalarrivaltime: 12 Oct 2020 06:47:33.1555 (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: Ok9L1/Jbi0Mo0FiZ6ALE2zYMRk9UXFzrsPmhU+MfzaJZ0hr6gq23Z5HjX559vwlGWhhln+cee9d5AlE3HKrpcW4NXoKen3dKEh5Og+MCslY= X-MS-Exchange-Transport-CrossTenantHeadersStamped: CO1PR11MB4930 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 Konstantin, > -----Original Message----- > From: Ananyev, Konstantin > Sent: Monday, October 12, 2020 4:33 AM > To: Gujjar, Abhinandan S ; Honnappa > Nagarahalli ; dev@dpdk.org; Doherty, > Declan > Cc: jerinj@marvell.com; Akhil.goyal@nxp.com; Vangati, Narender > ; nd ; nd > Subject: RE: [dpdk-dev] [v2 1/2] cryptodev: support enqueue callback func= tions >=20 >=20 >=20 > Hi Abhinandan, >=20 > > > > Hi Konstantin, > > > > > > > -----Original Message----- > > > From: Ananyev, Konstantin > > > Sent: Friday, October 9, 2020 8:10 PM > > > To: Gujjar, Abhinandan S ; Honnappa > > > Nagarahalli ; dev@dpdk.org; Doherty, > > > Declan > > > Cc: jerinj@marvell.com; Akhil.goyal@nxp.com; Vangati, Narender > > > ; nd ; nd > > > Subject: RE: [dpdk-dev] [v2 1/2] cryptodev: support enqueue callback > > > functions > > > > > > Hi Abhinandan, > > > > > > > > > > > Hi Konstantin & Honnappa, > > > > > > > > Thanks for all the inputs and feedback. > > > > > > > > @Ananyev, Konstantin, > > > > I have measured the perf with and without callback on xeon. Here > > > > are the > > > numbers: > > > > > > > > ./app/dpdk-test-crypto-perf -l 6-7 > > > > --vdev=3D"crypto_openssl0,socket_id=3D0,max_nb_sessions=3D128" -- > > > > --ptest throughput --devtype crypto_openssl --optype > > > > cipher-then-auth --cipher-algo aes-cbc --cipher-op 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 > > > > > > > > With callback(+ RCU - totally opaque to data-plane threads) > > > > lcore id Buf Size Burst Size Enqueued Dequeued Faile= d Enq > Failed > > > Deq MOps Gbps Cycles/Buf > > > > 7 64 32 10000000 100= 00000 0 0 > > > 0.8129 0.4162 2694.09 > > > > 7 64 32 10000000 100= 00000 0 0 > > > 0.8152 0.4174 2686.31 > > > > 7 64 32 10000000 100= 00000 0 0 > > > 0.8198 0.4197 2671.48 > > > > > > > > Without callback: > > > > lcore id Buf Size Burst Size Enqueued Dequeued Faile= d Enq > Failed > > > Deq MOps Gbps Cycles/Buf > > > > > > > > 7 64 32 10000000 1000= 0000 0 0 > > > 0.8234 0.4216 2659.81 > > > > 7 64 32 10000000 1000= 0000 0 0 > > > 0.8247 0.4222 2655.63 > > > > 7 64 32 10000000 1000= 0000 0 0 > > > 0.8123 0.4159 2695.90 > > > > > > > > > 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 > > That's right. (+ RCU calling online + offline APIs inside > > rte_cryptodev_enqueue_burst()) > > > 2) callback code in place but no callbacks installed > > No callback code. i.e. Original code. >=20 > Ok, and if I get things right - difference between mean values is ~15 cyc= les? Yes. May be, number are more stable on isolated core. Let's consider worst = case too. > That's seems like very good result to me. > Can I suggest to run one more test for your new callback code in place, b= ut no > actual callbacks installed? lcore id Buf Size Burst Size Enqueued Dequeued Failed Enq F= ailed Deq MOps Gbps Cycles/Buf 7 64 32 10000000 10000000 0 = 0 0.8220 0.4209 2664.12 7 64 32 10000000 10000000 0 = 0 0.8245 0.4221 2656.14 7 64 32 10000000 10000000 0 = 0 0.8261 0.4229 2651.15 > Thanks > Konstantin >=20 > > > > > > Is my understanding correct here? > > > Thanks > > > Konstantin > > > > > > > > > > > > > > Regards > > > > Abhinandan > > > > > > > > > -----Original Message----- > > > > > From: Ananyev, Konstantin > > > > > Sent: Tuesday, September 29, 2020 2:33 PM > > > > > To: Honnappa Nagarahalli ; Gujjar, > > > > > Abhinandan S ; dev@dpdk.org; > > > > > Doherty, Declan > > > > > Cc: jerinj@marvell.com; Akhil.goyal@nxp.com; Vangati, Narender > > > > > ; nd ; nd > > > > > Subject: RE: [dpdk-dev] [v2 1/2] cryptodev: support enqueue > > > > > callback functions > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > +#ifdef RTE_CRYPTODEV_CALLBACKS int > > > > > > > > > > > > > > > > +rte_cryptodev_rcu_qsbr_add(uint8_t > > > > > > > > > > > > > > > > +dev_id, struct rte_rcu_qsbr > > > > > > > > > > > > > > > > +*qsbr) { > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > + struct rte_cryptodev *dev; > > > > > > > > > > > > > > > > + > > > > > > > > > > > > > > > > + if (!rte_cryptodev_pmd_is_valid_dev(dev_i= d)) { > > > > > > > > > > > > > > > > + 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 RCU 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 > > > > > > > > > > > > > responsibility > > > > > 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, they 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 be 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#re > > > > > > > > > > sour > > > > > > > > > > ce-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 app= lication. > > > > > > > > > > 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 anyt= hing. > > > > > > > > > > 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 the user to manage any RCU QSBR state manually. > > > > > > > > > I believe that addition of ability to add/remove > > > > > > > > > enqueue/dequeue 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 > > > > > > > > problem 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 complet= ely. > > > > > > > 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 a= ll. > > > > > > > > > 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 additional > > > logic. > > > > > > > > > > Note that we are assuming that the application has to > > > > > > > > > > report quiescent state only for using callback function= s. > > > > > > > > > > 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 ad= d 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 memo= ry. > > > > > > > > > > > > > > > > > > > > > > > > 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 should 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 wheel. > > > > > > > > > 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 single 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 data > > > > > plane. > > > > > > > > > > To avoid misunderstanding: I am talking about calling > > > > > online/offline with 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 crypt= odev: > > > > > > > > > > > > > > > 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 as 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 > > > librte_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 application to > > > > > handle. > > > > > > > > > > > > > > > > > > > > > > > > I have already tried exploring above stuffs. > > > > > > > > > > > > > > There are 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_enqueue 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 D= eclan. > > > > > > > > > > > > > > > > > > > > > > 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 much > > > modified. > > > > > > > > > > > > The changes are either pushed to CP or DP. > > > > > > > > > > > > > > > > > > > > > > > > so adding extra overhead for sync is probably ok he= re. > > > > > > > > > > > > > > > > > > > > > > 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). > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > >