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 3BB65A2EFC for ; Tue, 17 Sep 2019 08:02:52 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id F3ACD1BEE2; Tue, 17 Sep 2019 08:02:51 +0200 (CEST) Received: from EUR02-VE1-obe.outbound.protection.outlook.com (mail-eopbgr20060.outbound.protection.outlook.com [40.107.2.60]) by dpdk.org (Postfix) with ESMTP id BCA7A37A2 for ; Tue, 17 Sep 2019 08:02:50 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Sqx0peJ9JOpIOfHrxsWkb7lStuqD7xKRHcxS1YGBDQD4Pk6YKoqMm9KdST95zGqajcgec5Sh9KfWJl8NF03+9KDweiUVchZS/SBEj1bdX1aI1pnGvv5dsfyLhalLueG8ofsQCKT3CaFEk8ZY1IDCuam3ZpGEIvxVKmRcrlKVOhzug+l1fJyOtKVBM0Crrny7goNVGup0cLRA/3Mm2ktunDJAQpjvslaIlxamSvnmlQ8zAEZZhDvevWMIYq3kohvjTz4nqP5hw9lnxB5wH49APaqSuBeb4iJTd3rYt7hQxvZYoYPkhSSNSZY/hOhvU7NGOXGjDltVyGLVlswF2ASWHQ== 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=HzTLDy8I9nzC2ulC48ya5L+NrNsvAyOo5GB3dVmC92g=; b=Z+jKtd9LiiUHJsQoPrfMi/b6WW53zIKPKycux5llUL08fX7kAor/EGE4Uqa7kcbJydZ1/mZzpTRttc057EqwI5t+1iJLcvcSiyHuBoB8tlqa0oYIdW8raLd9OL4Fa+NX7w/Jkn8m2X1AQnbJP0V9p1tYnnLilN2VogYFRTJikWui3PpVEIawqsDKhyH9WJdXvNwnYBgezPX23RxfV256KaDL+9Z3mIKymPsIBa9qFwpo+TarJCxbf9xNKg7a7SclMGmEXRoLOtraVV9t39HnHyG16KfvWfyHIpdOkLxkfYMfYcq5qbRzisIZQwtQOjOs2Bho7t/zbMsuk0+/5Pj/ig== ARC-Authentication-Results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=nxp.com; dmarc=pass action=none header.from=nxp.com; dkim=pass header.d=nxp.com; arc=none DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=nxp.com; s=selector2; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=HzTLDy8I9nzC2ulC48ya5L+NrNsvAyOo5GB3dVmC92g=; b=fQBFvuD4o7EShac0meHsDT9OiYvBFKwOYB1MyLJxMoStXYoUftosSNbR5A056PB7GlvDb+YZkYyOwKPUNRT2Q9GKiKpPxG2KO0HVN7YBL9AkpZ84oakIvYVE2jeby+gJ0WFMf3RNfOEe1P7CpNSOqCcczQKuTT1pYbuzEPnsnSM= Received: from VE1PR04MB6639.eurprd04.prod.outlook.com (20.179.235.82) by VE1PR04MB6750.eurprd04.prod.outlook.com (20.179.235.215) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.2263.26; Tue, 17 Sep 2019 06:02:49 +0000 Received: from VE1PR04MB6639.eurprd04.prod.outlook.com ([fe80::7092:d719:df0b:2b70]) by VE1PR04MB6639.eurprd04.prod.outlook.com ([fe80::7092:d719:df0b:2b70%7]) with mapi id 15.20.2263.023; Tue, 17 Sep 2019 06:02:49 +0000 From: Akhil Goyal To: "Ananyev, Konstantin" , "dev@dpdk.org" , "De Lara Guarch, Pablo" , Thomas Monjalon CC: "Zhang, Roy Fan" , "Doherty, Declan" , Anoob Joseph Thread-Topic: [RFC PATCH 1/9] security: introduce CPU Crypto action type and API Thread-Index: AQHVYm4LqyJkewM9NkuUWAfAmrqx1acbUiZggAAsN4CAAtsIgIAAT02AgAYXC5CAAbSDgIABbRGggAaWxgCAAPjG4A== Date: Tue, 17 Sep 2019 06:02:49 +0000 Message-ID: References: <20190903154046.55992-1-roy.fan.zhang@intel.com> <20190903154046.55992-2-roy.fan.zhang@intel.com> <9F7182E3F746AB4EA17801C148F3C6043369D686@IRSMSX101.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772580191926A17@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772580191962CD5@irsmsx105.ger.corp.intel.com> <2601191342CEEE43887BDE71AB9772580191966116@irsmsx105.ger.corp.intel.com> In-Reply-To: <2601191342CEEE43887BDE71AB9772580191966116@irsmsx105.ger.corp.intel.com> Accept-Language: en-IN, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: authentication-results: spf=none (sender IP is ) smtp.mailfrom=akhil.goyal@nxp.com; x-originating-ip: [92.120.1.65] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 60f3a990-c313-4c1b-5938-08d73b34ab67 x-ms-office365-filtering-ht: Tenant x-microsoft-antispam: BCL:0; PCL:0; RULEID:(2390118)(7020095)(4652040)(8989299)(5600167)(711020)(4605104)(1401327)(4618075)(4534185)(4627221)(201703031133081)(201702281549075)(8990200)(2017052603328)(7193020); SRVR:VE1PR04MB6750; x-ms-traffictypediagnostic: VE1PR04MB6750: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:6430; x-forefront-prvs: 01630974C0 x-forefront-antispam-report: SFV:NSPM; SFS:(10009020)(4636009)(376002)(396003)(136003)(39860400002)(366004)(346002)(13464003)(189003)(199004)(6506007)(76116006)(52536014)(64756008)(86362001)(66476007)(66556008)(446003)(11346002)(14454004)(14444005)(25786009)(476003)(256004)(4326008)(15650500001)(186003)(229853002)(2906002)(6436002)(66066001)(478600001)(6116002)(3846002)(102836004)(7736002)(54906003)(44832011)(6246003)(81156014)(110136005)(81166006)(66446008)(5660300002)(30864003)(55016002)(66946007)(316002)(26005)(9686003)(7696005)(71200400001)(8676002)(33656002)(76176011)(2501003)(71190400001)(74316002)(8936002)(99286004)(305945005)(561944003)(486006)(156664002); DIR:OUT; SFP:1101; SCL:1; SRVR:VE1PR04MB6750; H:VE1PR04MB6639.eurprd04.prod.outlook.com; FPR:; SPF:None; LANG:en; PTR:InfoNoRecords; MX:1; A:1; received-spf: None (protection.outlook.com: nxp.com does not designate permitted sender hosts) x-ms-exchange-senderadcheck: 1 x-microsoft-antispam-message-info: ck2a4pcmCSvWmz6luNY/eCIppKrinW1PY9rG9pI1x7j9CbqS+uDr47U/7f93C+54JGN2PnuWBIvEciELirHr1SPXAam7leugcCJODqXGxvJWVYst1yFz3r8CSMw9k4KNSvK9dTR+pcrL4TVczjvcxMB3oNHKLyhzeMsMjh/RlB6uVBCy+154lFRRZATsK6guiY9680VXMiYf4+atRjCZ+XbIk6QR5fr2vJ8x3/zPyVwvKtlF/3zVaDJrn+nikywAew7IEoKmabQLKvnyfT5H9acAiSCubsoEw8Dc2Z/OW0QtIQId+Y0ouru5NVW6x/2e1kwjLjTe+WaePNlkvloLhOcmIxXqt7HqxUlFqow/rays4MqH1R9LbHaLqAxkMbk4jUbRLeakP+lVmwn9XuvEwf4/CcNycwavsSWvXgq/Ftg= x-ms-exchange-transport-forked: True Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 X-OriginatorOrg: nxp.com X-MS-Exchange-CrossTenant-Network-Message-Id: 60f3a990-c313-4c1b-5938-08d73b34ab67 X-MS-Exchange-CrossTenant-originalarrivaltime: 17 Sep 2019 06:02:49.4560 (UTC) X-MS-Exchange-CrossTenant-fromentityheader: Hosted X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635 X-MS-Exchange-CrossTenant-mailboxtype: HOSTED X-MS-Exchange-CrossTenant-userprincipalname: UVaWI8LtB5Tti+fcBxQ0UVV0qrNS2wSpplcK8ftVRUKr9cNhWiBbMl4yo/i07IXVri+6XLWkkBA85k2nqdGgoA== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VE1PR04MB6750 Subject: Re: [dpdk-dev] [RFC PATCH 1/9] security: introduce CPU Crypto action type and API 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, >=20 > Hi Akhil, >=20 > > > > > > > This action type allows the burst of symmetric crypto workloa= d using > the > > > > > same > > > > > > > algorithm, key, and direction being processed by CPU cycles > > > synchronously. > > > > > > > This flexible action type does not require external hardware > involvement, > > > > > > > having the crypto workload processed synchronously, and is mo= re > > > > > performant > > > > > > > than Cryptodev SW PMD due to the saved cycles on removed "asy= nc > > > mode > > > > > > > simulation" as well as 3 cacheline access of the crypto ops. > > > > > > > > > > > > Does that mean application will not call the cryptodev_enqueue_= burst > and > > > > > corresponding dequeue burst. > > > > > > > > > > Yes, instead it just call rte_security_process_cpu_crypto_bulk(..= .) > > > > > > > > > > > It would be a new API something like process_packets and it wil= l have > the > > > > > crypto processed packets while returning from the API? > > > > > > > > > > Yes, though the plan is that API will operate on raw data buffers= , not > mbufs. > > > > > > > > > > > > > > > > > I still do not understand why we cannot do with the conventiona= l > crypto lib > > > > > only. > > > > > > As far as I can understand, you are not doing any protocol proc= essing > or > > > any > > > > > value add > > > > > > To the crypto processing. IMO, you just need a synchronous cryp= to > > > processing > > > > > API which > > > > > > Can be defined in cryptodev, you don't need to re-create a cryp= to > session > > > in > > > > > the name of > > > > > > Security session in the driver just to do a synchronous process= ing. > > > > > > > > > > I suppose your question is why not to have > > > > > rte_crypot_process_cpu_crypto_bulk(...) instead? > > > > > The main reason is that would require disruptive changes in exist= ing > > > cryptodev > > > > > API > > > > > (would cause ABI/API breakage). > > > > > Session for RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO need some > extra > > > > > information > > > > > that normal crypto_sym_xform doesn't contain > > > > > (cipher offset from the start of the buffer, might be something e= xtra in > > > future). > > > > > > > > Cipher offset will be part of rte_crypto_op. > > > > > > fill/read (+ alloc/free) is one of the main things that slowdown curr= ent > crypto-op > > > approach. > > > That's why the general idea - have all data that wouldn't change from= packet > to > > > packet > > > included into the session and setup it once at session_init(). > > > > I agree that you cannot use crypto-op. > > You can have the new API in crypto. > > As per the current patch, you only need cipher_offset which you can hav= e it as > a parameter until > > You get it approved in the crypto xform. I believe it will be beneficia= l in case of > other crypto cases as well. > > We can have cipher offset at both places(crypto-op and cipher_xform). I= t will > give flexibility to the user to > > override it. >=20 > After having another thought on your proposal: > Probably we can introduce new rte_crypto_sym_xform_types for CPU related > stuff here? I also thought of adding new xforms, but that wont serve the purpose for ma= y be all the cases. You would be needing all information currently available in the current xfo= rms. So if you are adding new fields in the new xform, the size will be more tha= n that of the union of xforms. ABI breakage would still be there.=20 If you think a valid compression of the AEAD xform can be done, then that c= an be done for each of the Xforms and we can have a solution to this issue. > Let say we can have : > num rte_crypto_sym_xform_type { > RTE_CRYPTO_SYM_XFORM_NOT_SPECIFIED =3D 0, /**< No xform specified > */ > RTE_CRYPTO_SYM_XFORM_AUTH, /**< Authentication xform= */ > RTE_CRYPTO_SYM_XFORM_CIPHER, /**< Cipher xform */ > RTE_CRYPTO_SYM_XFORM_AEAD /**< AEAD xform */ > + RTE_CRYPTO_SYM_XFORM_CPU =3D INT32_MIN, > + RTE_CRYPTO_SYM_XFORM_CPU_AEAD =3D (RTE_CRYPTO_SYM_XFORM_CPU | > RTE_CRYPTO_SYM_XFORM_CPU), Instead of CPU I believe SYNC would be better. > /* same for auth and crypto xforms */ > }; >=20 > Then we either can re-define some values in struct rte_crypto_aead_xform = (via > unions), > or even have new struct rte_crypto_cpu_aead_xform (same for crypto and a= uth > xforms). > Then if PMD wants to support new sync API it would need to recognize new > xform types > and internally it might end up with different session structure (one for= sync, > another for async mode). > That I think should allow us to introduce cpu_crypto as part of crypto-de= v API > without ABI breakage. > What do you think? > Konstantin >=20 > > > > > > > > > If you intend not to use rte_crypto_op > > > > You can pass this as an argument in the new cryptodev API. > > > > > > You mean extra parameter in rte_security_process_cpu_crypto_bulk()? > > > It can be in theory, but that solution looks a bit ugly: > > > why to pass for each call something that would be constant per sessi= on? > > > Again having that value constant per session might allow some extra > > > optimisations > > > That would be hard to achieve for dynamic case. > > > and not extendable: > > > Suppose tomorrow will need to add something extra (some new algorithm > > > support or so). > > > With what you proposing will need to new parameter to the function, > > > which means API breakage. > > > > > > > Something extra will also cause ABI breakage in security as well. > > > > So it will be same. > > > > > > I don't think it would. > > > AFAIK, right now this patch doesn't introduce any API/ABI breakage. > > > Iinside struct rte_security_session_conf we have a union of xforms > > > depending on session type. > > > So as long as cpu_crypto_xform wouldn't exceed sizes of other xform - > > > I believe no ABI breakage will appear. > > Agreed, it will not break ABI in case of security till we do not exceed= current > size. > > > > Saving an ABI/API breakage is more important or placing the code at the > correct place. > > We need to find a tradeoff. Others can comment on this. > > @Thomas Monjalon, @De Lara Guarch, Pablo Any comments? > > > > > > > > > > > > > > > > > Also right now there is no way to add new type of crypto_sym_sess= ion > > > without > > > > > either breaking existing crypto-dev ABI/API or introducing new st= ructure > > > > > (rte_crypto_sym_cpu_session or so) for that. > > > > > > > > What extra info is required in rte_cryptodev_sym_session to get the > > > rte_crypto_sym_cpu_session. > > > > > > Right now - just cipher_offset (see above). > > > What else in future (if any) - don't know. > > > > > > > I don't think there is any. > > > > I believe the same crypto session will be able to work synchronousl= y as well. > > > > > > Exactly the same - problematically, see above. > > > > > > > We would only need a new API to perform synchronous actions. > > > > That will reduce the duplication code significantly > > > > in the driver to support 2 different kind of APIs with similar code= inside. > > > > Please correct me in case I am missing something. > > > > > > To add new API into crypto-dev would also require changes in the PMD, > > > it wouldn't come totally free and I believe would require roughly the= same > > > amount of changes. > > > > It will be required only in the PMDs which support it and would be mini= mal. > > You would need a feature flag, support for that synchronous API. Sessi= on > information will > > already be there in the session. The changes wrt cipher_offset need to = be > added > > but with some default value to identify override will be done or not. > > > > > > > > > > > > > > > > > > While rte_security is designed in a way that we can add new sessi= on > types > > > and > > > > > related parameters without causing API/ABI breakage. > > > > > > > > Yes the intent is to add new sessions based on various protocols th= at can > be > > > supported by the driver. > > > > > > Various protocols and different types of sessions (and devices they b= elong > to). > > > Let say right now we have INLINE_CRYPTO, INLINE_PROTO, > LOOKASIDE_PROTO, > > > etc. > > > Here we introduce new type of session. > > > > What is the new value add to the existing sessions. The changes that we= are > doing > > here is just to avoid an API/ABI breakage. The synchronous processing c= an > happen on both > > crypto and security session. This would mean, only the processing API s= hould > be defined, > > rest all should be already there in the sessions. > > In All other cases, INLINE - eth device was not having any format to pe= rform > crypto op > > LOOKASIDE - PROTO - add protocol specific sessions which is not availab= le in > crypto. > > > > > > > > > It is not that we should find it as an alternative to cryptodev and= using it > just > > > because it will not cause > > > > ABI/API breakage. > > > > > > I am considering this new API as an alternative to existing ones, but= as an > > > extension. > > > Existing crypto-op API has its own advantages (generic), and I think = we > should > > > keep it supported by all crypto-devs. > > > From other side rte_security is an extendable framework that suits th= e > purpose: > > > allows easily (and yes without ABI breakage) introduce new API for sp= ecial > type > > > of crypto-dev (SW based). > > > > > > > > > > Adding a synchronous processing API is understandable and can be added = in > both > > Crypto as well as Security, but a new action type for it is not require= d. > > Now whether to support that, we have ABI/API breakage, that is a differ= ent > issue. > > And we may have to deal with it if no other option is there. > > > > > > > > > > > > > > > IMO the code should be placed where its intent is. > > > > > > > > > > > > > > BTW, what is your concern with proposed approach (via rte_securit= y)? > > > > > From my perspective it is a lightweight change and it is totally = optional > > > > > for the crypto PMDs to support it or not. > > > > > Konstantin > > > > > > > > > > > > > > > > > > > AESNI-GCM and AESNI-MB PMDs are updated with this support. > There is > > > a > > > > > small > > > > > > > performance test app under > app/test/security_aesni_gcm(mb)_perftest > > > to > > > > > > > prove. > > > > > > > > > > > > > > For the new API > > > > > > > The packet is sent to the crypto device for symmetric crypto > > > > > > > processing. The device will encrypt or decrypt the buffer bas= ed on the > > > > > session > > > > > > > data specified and preprocessed in the security session. Diff= erent > > > > > > > than the inline or lookaside modes, when the function exits, = the user > will > > > > > > > expect the buffers are either processed successfully, or havi= ng the > error > > > > > number > > > > > > > assigned to the appropriate index of the status array. > > > > > > > > > > > > > > Will update the program's guide in the v1 patch. > > > > > > > > > > > > > > Regards, > > > > > > > Fan > > > > > > > > > > > > > > > -----Original Message----- > > > > > > > > From: Akhil Goyal [mailto:akhil.goyal@nxp.com] > > > > > > > > Sent: Wednesday, September 4, 2019 11:33 AM > > > > > > > > To: Zhang, Roy Fan ; dev@dpdk.org > > > > > > > > Cc: Ananyev, Konstantin ; Doh= erty, > > > > > Declan > > > > > > > > ; De Lara Guarch, Pablo > > > > > > > > > > > > > > > > Subject: RE: [RFC PATCH 1/9] security: introduce CPU Crypto= action > > > type > > > > > and > > > > > > > > API > > > > > > > > > > > > > > > > Hi Fan, > > > > > > > > > > > > > > > > > > > > > > > > > > This patch introduce new > > > RTE_SECURITY_ACTION_TYPE_CPU_CRYPTO > > > > > > > > action > > > > > > > > > type to security library. The type represents performing = crypto > > > > > > > > > operation with CPU cycles. The patch also includes a new = API to > > > > > > > > > process crypto operations in bulk and the function pointe= rs for > PMDs. > > > > > > > > > > > > > > > > > I am not able to get the flow of execution for this action = type. > Could > > > you > > > > > > > > please elaborate the flow in the documentation. If not in > > > documentation > > > > > > > > right now, then please elaborate the flow in cover letter. > > > > > > > > Also I see that there are new APIs for processing crypto op= erations > in > > > bulk. > > > > > > > > What does that mean. How are they different from the existi= ng APIs > > > which > > > > > > > > are also handling bulk crypto ops depending on the budget. > > > > > > > > > > > > > > > > > > > > > > > > -Akhil