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 1AD63A00BE; Tue, 7 Jul 2020 14:39:08 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 8B8031DB6E; Tue, 7 Jul 2020 14:39:07 +0200 (CEST) Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 3DDF81DA9C for ; Tue, 7 Jul 2020 14:39:06 +0200 (CEST) IronPort-SDR: 9gkV0ChJOKsWYvXvqTY1Tj5OYeewVxsMHQRB9at9QGGYsS17XH8RPq6wVXdse1ZC6l3olH47me 8B/kh6Gj9GYw== X-IronPort-AV: E=McAfee;i="6000,8403,9674"; a="212549056" X-IronPort-AV: E=Sophos;i="5.75,323,1589266800"; d="scan'208";a="212549056" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga004.jf.intel.com ([10.7.209.38]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 07 Jul 2020 05:39:05 -0700 IronPort-SDR: EcmQn8nGXjOeZckD0hsGG3crH4Hehyda7u/Dks4v+gEXXFOLl/VDQ7CJiVqsgAkbdgH3mX0m23 0O1/WLYN/QJw== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.75,323,1589266800"; d="scan'208";a="427461053" Received: from orsmsx604.amr.corp.intel.com ([10.22.229.17]) by orsmga004.jf.intel.com with ESMTP; 07 Jul 2020 05:39:05 -0700 Received: from orsmsx604.amr.corp.intel.com (10.22.229.17) 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.1713.5; Tue, 7 Jul 2020 05:37:10 -0700 Received: from ORSEDG002.ED.cps.intel.com (10.7.248.5) by orsmsx604.amr.corp.intel.com (10.22.229.17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256) id 15.1.1713.5 via Frontend Transport; Tue, 7 Jul 2020 05:37:10 -0700 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.107) by edgegateway.intel.com (134.134.137.101) with Microsoft SMTP Server (TLS) id 14.3.439.0; Tue, 7 Jul 2020 05:37:07 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=oGoXortK3J5xZ+SVHKllBGRCke385xZ8ecB5ruPHjTHD/A4x/JMBmDafFX1+jplZH2UErO5/JgvM9J8FCUSKWYyJ2KCsKXq/HjIslxBxNYFui/L2bHsCAU+sx1RCYxal+kip9hklqmIBH7VAM3/3LvI8fs65OGi5orCS3HTIUYz20b6/wy/15JImFOa2Pg4DXPklZ7LxVqgpnBBA2UmrwLnFHuI3BWM/GfBGpX2cl3aYcIFib8MuqOQUXbivr5nB00ZsRhfpJ9+HT6swYTmfvI2wfZgMP0H301j+VcQm0sMnvr8unSVRZC7lxFs7liinmcobY19cHsu/ysygNShW7w== 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=SElvOt7HY4T5d0Hnlbs+efdHswz+9cbvHBtrrzltGjs=; b=bFcdW1/FV6xlMJBpo3XSMB9gjL0u2CRLcOZMgH83hPOeXnaKeIl4nVtctX87+iI+st4pOUBULs1eO6JpITncQ8JvTEZdjgtEu2+AXTYxr7a3I1qaWYl4SFkZ9Q+L19dY0ghWLguFGdNyPu+C8h/BxQR439RzzfNdX4eSJT0MQND6eY9FnzRHAnyjigcKr8hlxe+PqLU+WNvZiX5IkRbaDQ1Kq+6GoAxB3dvj2ncm8EsWGL+FEQ4d0y5OKjdcsFWfsPQO6uTYFSBtm+PAphpMq0tZ2BxflyzipE+yWJWtb+O3mH5Q5hyBgie7/MxAb25DPth/V5oJZ7ZXuuaiazKOhw== 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=SElvOt7HY4T5d0Hnlbs+efdHswz+9cbvHBtrrzltGjs=; b=x7cp0WGVri0N7ciXEE/YTH6Z65jt9Oa64/BbOQOmFtR3e66JxfcwPMHUVZY8gpuXZrDjQXKTM3EQy5d9R54NokzY5S+FTL6CkGk5V9c/wFhUYwJfxY9XlkImzbSCPxaQD9RAm83OSAohHZUM9k/mSZun+/PoMvEtZGUrmVtvS7k= Received: from DM6PR11MB3049.namprd11.prod.outlook.com (2603:10b6:5:6a::21) by DM5PR11MB1353.namprd11.prod.outlook.com (2603:10b6:3:a::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3153.20; Tue, 7 Jul 2020 12:37:05 +0000 Received: from DM6PR11MB3049.namprd11.prod.outlook.com ([fe80::90ce:fb:2444:27f9]) by DM6PR11MB3049.namprd11.prod.outlook.com ([fe80::90ce:fb:2444:27f9%4]) with mapi id 15.20.3153.029; Tue, 7 Jul 2020 12:37:05 +0000 From: "Zhang, Roy Fan" To: Akhil Goyal , "dev@dpdk.org" , "anoobj@marvell.com" , "asomalap@amd.com" , "ruifeng.wang@arm.com" , Nagadheeraj Rottela , Michael Shamis , Ankur Dwivedi , Jay Zhou , "De Lara Guarch, Pablo" CC: "Trahe, Fiona" , "Bronowski, PiotrX" , "Ananyev, Konstantin" , Thomas Monjalon Thread-Topic: [dpdk-dev v4 1/4] cryptodev: add symmetric crypto data-path APIs Thread-Index: AQHWUTh9W23hQCo6PkeV7n4G/Cv9Iqj3u/CAgAKPEPCAADBDgIABdgEg Date: Tue, 7 Jul 2020 12:37:05 +0000 Message-ID: References: <20200703110923.26452-1-roy.fan.zhang@intel.com> <20200703124942.29171-1-roy.fan.zhang@intel.com> <20200703124942.29171-2-roy.fan.zhang@intel.com> In-Reply-To: Accept-Language: zh-Hans-HK, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZTc4OWU0YTAtYjk0OC00MmIxLWJlYTUtODFiZGUwMGUyNmJhIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiNHVyVDRaVlZud1F5Y1hPcFRwRzZKUHZDUEZQZks5R1J6RTAwNW1GQ2tnbUt1RWp6ZWg1c082Z3BxSE9YQjRpbyJ9 x-ctpclassification: CTP_NT dlp-version: 11.2.0.6 dlp-reaction: no-action dlp-product: dlpe-windows authentication-results: nxp.com; dkim=none (message not signed) header.d=none;nxp.com; dmarc=none action=none header.from=intel.com; x-originating-ip: [192.198.151.167] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 158bdfca-adcf-4401-5f3b-08d8227274ee x-ms-traffictypediagnostic: DM5PR11MB1353: x-ms-exchange-transport-forked: True x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 0457F11EAF x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: UaKxU9+sJxyzInYFZ7BsxnVDjvrJKNsiU+6dR+WKtIldTubQoGTWKFvQVH7cM+Y/AVs4cq3LjgQrWqhuQ8X3Gyz/pzc7DP3xkW5oSYWlkHm5AbOVfndSzrXA97fOd49Hi+xj1mcN4wLXwbYG/BBV1Qnc2HoCvGuPVcjdxSBWMeGQTEK7Z3PNqpPhuasrGLigFTr8nKrORNeLxlwlicw7VxcT0DuX8nuyjdNEaS+u1c3nUTg6Aa6NI6f+dYJIlWmJzRa4LEXohmeHvVyQ91np3G69B4Viqo2ffY/bGBua7PgtKXYidpyX6Y5R8HL5ZegJKN1HWuIS7XyvT2acH3En0mpqVUG2RJVB4SZdujPH1+aRoqtylPrjhEBuIvvaMP5Ft6R5Mvvsq01KRHsQjK+mj6EqiMBHgYNJLX/PndfWYhXkjISQerHnMgeGBOTbWHFy x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:DM6PR11MB3049.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFTY:; SFS:(4636009)(376002)(346002)(39860400002)(366004)(136003)(396003)(76116006)(26005)(6506007)(478600001)(966005)(53546011)(52536014)(5660300002)(64756008)(71200400001)(66446008)(66946007)(66556008)(66476007)(7696005)(83380400001)(6636002)(110136005)(86362001)(4326008)(186003)(54906003)(33656002)(7416002)(2906002)(8676002)(8936002)(55016002)(316002)(9686003)(921003); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: QInd4TMw5nBCZLzqGHqR05ehfTYKdn34jwE12gUFobT+uy9uy96RMYnJG/cKD5XfBPqHvKM/01Q8Qm25n8iYUztLOBjQ+SonEOrsCfNnsGveb+sUMLpKywlwyaoUuV3dqxid00gVX50buMvDNLJ4pKnT1V7mhGb3KgHcW5+ySi1yh8geUvPJ/A1vKDlbQHsHe06LbN1ZGc3x3LBqXb/NJ379h5/uWy3qSpZQ0eYK/WeDnAeqeODlR+uPtG6us1YPaV8DZWdE0X6siot+Ua20wPhvWFe0w3bx8tHGI/HydLe9xAfKsBY97asx7iVAtfCS+2LOB9UwWemwxzEFf1lRvF0hgpLrJl1+82Cn9778oxbuqNVniG2WQi/drJKOAvUw9w+9MpGgRi43ZS0np/3Chvg4/lNExSrAR6aGP7vj8EWkKRfiWjDm0ekV7/RaBi+7e3sNHJlcOYEdqqh41+0pkRG8HkajILi/fIAhIQcXFp97kd9WtBimva8GHpo/idrO Content-Type: text/plain; charset="Windows-1252" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-MS-Exchange-CrossTenant-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: DM6PR11MB3049.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 158bdfca-adcf-4401-5f3b-08d8227274ee X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jul 2020 12:37:05.4641 (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: XNHmFIKXtLykdh6QdKwAy2ZW2y0+SOkeyPBo4N9wjGfq2mROlJbzPGpGv9EoqFqNFKw9eoAKGoH+qf9Giq3nUQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM5PR11MB1353 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [dpdk-dev v4 1/4] cryptodev: add symmetric crypto data-path APIs 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 Akhil, > -----Original Message----- > From: Akhil Goyal > Sent: Monday, July 6, 2020 1:13 PM > To: Zhang, Roy Fan ; dev@dpdk.org; > anoobj@marvell.com; asomalap@amd.com; ruifeng.wang@arm.com; > Nagadheeraj Rottela ; Michael Shamis > ; Ankur Dwivedi ; Jay > Zhou ; De Lara Guarch, Pablo > > Cc: Trahe, Fiona ; Bronowski, PiotrX > ; Ananyev, Konstantin > ; Thomas Monjalon > > Subject: RE: [dpdk-dev v4 1/4] cryptodev: add symmetric crypto data-path > APIs >=20 ... > > > > As you may have seen the structure definition is hW centric with the > > IOVA addresses all over. Also as you will from the patch series the > operation is > > Per operation basis instead of operating in a burst. The external appli= cation > > may sooner know when a specific enqueue is failed. >=20 > You may also need to save a virtual address as well. As some hardware are > able to > Convert virtual to physical addresses on it's own giving a performance > improvement. >=20 > I do not see an issue in using enqueue burst with burst size=3D1 , but si= nce you > are doing > Optimizations, none of the hardware can perform well with burst =3D 1, I = think > it is always > Greater than 1. Shall I update the rte_crypto_sym_vec as the following - so the 2 problems = can be resolved? struct rte_crypto_sym_vec { /** array of SGL vectors */ struct rte_crypto_sgl *sgl; union { /* Supposed to be used with CPU crypto API call. */ struct { /** array of pointers to IV */ void **iv; /** array of pointers to AAD */ void **aad; /** array of pointers to digest */ void **digest; /** * array of statuses for each operation: * - 0 on success * - errno on error */ int32_t *status; }; /* Supposed to be used with HW crypto API call. */ struct { /** array of pointers to IV */ struct rte_crypto_vec *iv_hw; /** array of pointers to AAD */ struct rte_crypto_vec *aad_hw; /** array of pointers to Digest */ struct rte_crypto_vec *digest_hw; }; }; /** number of operations to perform */ uint32_t num; }; > > > > > Now for this patchset, the requirement is > > > - raw buffers > > > - asynchronous APIs > > > > > > The data structure for raw buffers and crypto related offsets are alr= eady > > > defined > > > So they should be reused. > > > And I believe with some changes in rte_crypto_op and > rte_crypto_sym_op, > > > We can support raw buffers with the same APIs. > > > Instead of m_src and m_dst, raw buffer data structures can be combine= d > in a > > > Union and some of the fields in the rte_crypto_op can be left NULL in > case of > > > raw buffers. > > > > > > > This is a good point but we still face too many unnecessary fields to b= e > NULL, > > such as > > digest pointers, I have given a lot thought to this structure. Hopefull= y it > covers > > all vendor's HW symmetric crypto needs and in the same time it well > squeeze > > the required HW addresses into 1 cacheline, instead of rte_crypto_op + > > rte_crypto_sym_op 3 cacheline footprint. Another purpose of the > structure > > design > > is the structure buffer can be taken from stack and can be used to fill= all > > jobs to the PMD HW. >=20 > Which fields you think are not useful and should be set as NULL? > Digest pointers you are anyways setting in the new structure. > Your new struct does not support session less as well as security session= s. > It does not take care of asymmetric crypto. > So whenever, a vendor need to support all these, we would end up getting > the rte_crypto_op structure. > IMO, you only need to make m_src and m_dst as union to a raw > input/output > buffers. Everything else will be relevant. >=20 Rte_crypto_op is designed to be allocated from mempool with HW address info contained so it is possible to deduct IV and AAD physical address from it. More importantly rte_crypto_op is designed to be taken from heap and=20 being freed after dequeue. So they cannot be allocated from stack - for thi= s reason I think rte_crypot_sym_vec is a better fit for the patch, do you agr= ee?=20 (the Proposed change is at above). > Have you done some profiling with using rte_crypto_op instead of this new > struct? >=20 Yes, the code are actually upstreamed in VPP https://gerrit.fd.io/r/c/vpp/+/18036, please try out. If you have a look at= the enqueue/dequeue functions you should see the struggle we had to translate ops, and creating a second software ring to make sure we only dequeue a=20 frame of data. Lucky VPP has space to store mbufs otherwise the perf will be even worse. > > > > > > > > > +/* Struct for user to perform HW specific enqueue/dequeue function > calls > > > */ > > > > +struct rte_crypto_hw_ops { > > > > + /* Driver written queue pair data pointer, should NOT be alterred= by > > > > + * the user. > > > > + */ > > > > + void *qp; > > > > + /* Function handler to enqueue AEAD job */ > > > > + rte_crypto_hw_enq_cb_fn enqueue_aead; > > > > + /* Function handler to enqueue cipher only job */ > > > > + rte_crypto_hw_enq_cb_fn enqueue_cipher; > > > > + /* Function handler to enqueue auth only job */ > > > > + rte_crypto_hw_enq_cb_fn enqueue_auth; > > > > + /* Function handler to enqueue cipher + hash chaining job */ > > > > + rte_crypto_hw_enq_cb_fn enqueue_chain; > > > > + /* Function handler to query processed jobs */ > > > > + rte_crypto_hw_query_processed query_processed; > > > > + /* Function handler to dequeue one job and return opaque data > > > stored > > > > */ > > > > + rte_crypto_hw_deq_one_cb_fn dequeue_one; > > > > + /* Function handler to dequeue many jobs */ > > > > + rte_crypto_hw_deq_many_cb_fn dequeue_many; > > > > + /* Reserved */ > > > > + void *reserved[8]; > > > > +}; > > > > > > Why do we need such callbacks in the library? > > > These should be inside the drivers, or else we do the same for > > > Legacy case as well. The pain of finding the correct enq function for > > > Appropriate crypto operation is already handled by all the drivers > > > And we can reuse that or else we modify it there as well. > > > > > > > Providing different types of enqueue functions for specific operation t= ype > > could save a lot of branches for the driver to handle. As mentioned thi= s > > data-path API is intended to be used as an advanced feature to provide > > close-to-native perf to external library/applications that are not mbuf > > centric. And I don't agree classifying choosing 1 enqueue function from > > 4 candidates as "pain". >=20 > My point is why don't we have it in the Legacy code path as well? > I think it is useful in both the paths. Branching is a pain for the drive= r. >=20 That's a good point :-) we definitely can do something about it in future r= eleases. > > > > > We should not add a lot of data paths for the user, otherwise the > > > APIs will become centric to a particular vendor and it will be very d= ifficult > > > For the user to migrate from one vendor to another and would defeat > the > > > Purpose of DPDK which provide uniform abstraction layer for all the > > > hardware > > > Vendors. > > > > > > > The purpose of adding data-path for the user is performance for non-mbu= f > > data-path centric applications/libraries, in the meantime not creating > > confusion. In this version we aim to provide a more friendly data-path = for >=20 > I do not see the new path as friendly. > Adding a parallel new datapath with create more confusion for the > application > developer. It would be convenient, if we can use the same path with minim= al > changes so that people can migrate easily. >=20 We are working on next version that is based on rte_crypto_sym_vec and sing= le Enqueue-then-dequeue API. To be honest it won't be that of friendly to appl= ication developer that the applications are mbuf-based or already built on top of c= ryptodev, however for the applications that are not mbuf based and having their own d= ata-path structures it will be surely friendlier than existing enqueue and dequeue A= PIs. No dependency to mbuf, mbuf and crypto op pool, and no need to consider how to= adapt different working methods. > > them, and aims to be useful to all vendor's PMDs. If there is any place= in > > the API that blocks a PMD please let me know. >=20 > As commented above, sessionless, rte_security sessions, asymmetric crypto > Not supported. >=20 You are right -=20 Sessionless support aims the usability and is not intended to be used in hi= gh-throughput Application. Rte_security is built on top of mbuf and ethdev and is not intended to "mbu= f-independent" applications either. >=20 > > > > > Adding other vendors to comment. > > > > > > Regards, > > > Akhil Regards, Fan