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 4AA72A04D6; Mon, 21 Sep 2020 17:27:01 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id C27E61DA62; Mon, 21 Sep 2020 17:27:00 +0200 (CEST) Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 10EBE1D6FE for ; Mon, 21 Sep 2020 17:26:58 +0200 (CEST) IronPort-SDR: r8RtfCewWk0aKDTQk3tTUpe/GQX4unc8Hx7sC9yGBVqmpFhoiqwtfponniWhqL2RhYdSjgHk2l XpBZ0EVHkV9g== X-IronPort-AV: E=McAfee;i="6000,8403,9751"; a="178476104" X-IronPort-AV: E=Sophos;i="5.77,286,1596524400"; d="scan'208";a="178476104" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga006.jf.intel.com ([10.7.209.51]) by fmsmga101.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 21 Sep 2020 08:26:55 -0700 IronPort-SDR: whZweVB2GpAzqEnm9gSIgc9b2My4SVwy+Z/fUWQ/e5OpTEfWprRRlc7UQG+usICby0coMpGLkx rrVTKOZC55ww== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.77,286,1596524400"; d="scan'208";a="309078837" Received: from orsmsx606.amr.corp.intel.com ([10.22.229.19]) by orsmga006.jf.intel.com with ESMTP; 21 Sep 2020 08:26:54 -0700 Received: from orsmsx601.amr.corp.intel.com (10.22.229.14) by ORSMSX606.amr.corp.intel.com (10.22.229.19) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256) id 15.1.1713.5; Mon, 21 Sep 2020 08:26:53 -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; Mon, 21 Sep 2020 08:26:53 -0700 Received: from NAM10-BN7-obe.outbound.protection.outlook.com (104.47.70.108) 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; Mon, 21 Sep 2020 08:26:51 -0700 ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=aKP66ho9QVBXxv1dnjXwILMyiouzKi535+/AeoobYhEvrBnpSgkNSarzUlm6vrP8f5mhPTnTep4MMqhPM7feUdv4U4XDbZT408H/qIMWaLrYk6latGNMWahDgQ7DUVT8cQ94kIPvljCfY/TksncK4qgOfJZPTjwtC6j6/jC5khsgq2WgiR7bJkK4izGy5aEAigtdf3HryBmRavF/0qi5k/XJguk7G2fgV9cYIZWaFKRsfzWHYPWSWhW6RY6wszqMkvPHv0FH3edw4q5l/NbNWOzHUkWquCDZKcNNPHRvCLBN/DpKkun3WQXW7GVFeaKoyF31pYzr+ctp0/dRfsHEkQ== 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=YnZQ8pWgmUemGTU2S3tomt5+j1l2kuyQsvLbVGHW5Gc=; b=UEIulkwsN0Mopm26j229rbizT0LtfDcqX/1E/Fcx71olT4g7OsBLQgNsPGgpvRqdKA20qkwFiTG0l4+tVV+aOGCAdz2VD8Kj3PBeehf6DkGCL9SRfMroagbbzYt6/oeJ6QSvHhQ+AYpITJz9kU2PvvlWSrgpqDug41f63gCrF7hsOHVdTd8JCwmrZLjzqm6G/YEmBGTjF8jFuUjD4ErHY90/JAgrND/fHiH7sUMWKfLsPQkpRx6RCuMpGY4e1XrAC29xh6tBXQSdpv0F4f4KPVd9XgPRvVmJZLhChNboVJP0n4LPVdG+9NC3bS2y3rVXEqY7YeghjMRHtxU795XSgg== 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=YnZQ8pWgmUemGTU2S3tomt5+j1l2kuyQsvLbVGHW5Gc=; b=nGzfGU0DOa1BXuoxJewk5V3ASVLXlPTkUHOyNzfU5SoLb1FkD+/LTKGU4/hf4KEVYXJCNDJeCgY49KwF2nmBt3zhVHXjfUs+DDcgYNYlsvQloDuOkXLPZLu2i3UOlO4l+7KwRt0ieJUtGxmRXW4rnL3gO8tYvk14NDR9pb+YDKU= Received: from BL0PR11MB3043.namprd11.prod.outlook.com (2603:10b6:208:33::19) by BL0PR11MB3042.namprd11.prod.outlook.com (2603:10b6:208:78::17) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3391.24; Mon, 21 Sep 2020 15:26:48 +0000 Received: from BL0PR11MB3043.namprd11.prod.outlook.com ([fe80::11fa:a7fe:329d:9239]) by BL0PR11MB3043.namprd11.prod.outlook.com ([fe80::11fa:a7fe:329d:9239%5]) with mapi id 15.20.3391.024; Mon, 21 Sep 2020 15:26:48 +0000 From: "Zhang, Roy Fan" To: Akhil Goyal , "dev@dpdk.org" , "Ananyev, Konstantin" , Thomas Monjalon CC: "Trahe, Fiona" , "Kusztal, ArkadiuszX" , "Dybkowski, AdamX" , "Bronowski, PiotrX" , Anoob Joseph Thread-Topic: [dpdk-dev v9 1/4] cryptodev: add crypto data-path service APIs Thread-Index: AQHWhbwaRQXVNw2ngE+5SqoPhUuyBqlu/9oAgAP1tgCAABxYAIAAOHkQ Date: Mon, 21 Sep 2020 15:26:48 +0000 Message-ID: References: <20200904152539.20608-1-roy.fan.zhang@intel.com> <20200908084253.81022-1-roy.fan.zhang@intel.com> <20200908084253.81022-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: dlp-version: 11.5.1.3 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: [95.44.220.85] x-ms-publictraffictype: Email x-ms-office365-filtering-correlation-id: 3cb5e774-ac8c-4006-a2a9-08d85e42c1ce x-ms-traffictypediagnostic: BL0PR11MB3042: 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: GxLIOP7aSvZ1dkufOMeQrTfbIPM+rYug/dDE0D5N2xdMDePZFF0ChXQO/2Qw2dZuzeB4p+c4y3Uk8jxPzSsskVL8Ro2jfqXeRB9Op3SPnpRWS7dRk/n6+yUt8tv8totn3kYx6EL0k3mxih8xWNGoHC4PHAuKduyAGayhQprEHI4jCOff12UEcFy7nM92fgW9W8cUy7/wyoP5bm4G5aldoHl6XjoJG8F0FCb+JlMyQLSymwVNpraRrkWrapNmmNk6Hj7t0SgyAmEvGzLtJ2s983MFnUZdA7Lp3SKlgUyrKqWxIAA4ZkW1CarpqQ3sdFZN x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:BL0PR11MB3043.namprd11.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(376002)(366004)(346002)(39860400002)(136003)(396003)(6506007)(478600001)(7696005)(110136005)(26005)(86362001)(53546011)(66556008)(64756008)(66476007)(66446008)(2906002)(8676002)(54906003)(66946007)(76116006)(52536014)(71200400001)(8936002)(83380400001)(55016002)(9686003)(33656002)(4326008)(316002)(5660300002)(186003); DIR:OUT; SFP:1102; x-ms-exchange-antispam-messagedata: BAJDwd+z5pnt0iwPA3r1FWbK+aStkltfifOM1Xy0aMR/JDGPOqI7mOwI9YyZeHrXznl9NukLMa0Nw7HpyY+QDBBcj9mJcN20PeRovXoVXpjP7CH1JesvDH1aH//hVq2Ma2IWi7bzTXkFsHYcKzUWgk8r715M+ODrVfDpPAF6kQtMwiTbvseUCIQW/8WY8bGQ0G4LkSOsq+ALspSG0vViirhJBP50uHrzDOpvZRdRslrDI77361qdaNeBcqFSFCsIPD93SWuqSmiRderRdP/1U0+RQyya4+hsOuf+bw53s1AXGZw0LO7mk5GuKZGThWzzG0JHi/7q9wKFtbaLDkzIfQhpbWBQC0LDultE7GQYmY/wtnXp9LuIJvGD+0CndKJsFdLFrRIgG+xnistch8f+cbJI4Hv0SrXam5GUSEGgs/emMiPkHJj7GebNkvtdAiQT44q79fNrFepnNuJISfWHy4gocNHSYu8mYlEqgEvjMQezR5FxB+6IYf7Qu5GDR3RqGSzsPaZkJ0UP1EmTihNRESt+Bkklrlgs97ipKW2INHRxrdDwvkdN6kWeo/iF5mD+HhDHKdfYbiq4UMN9qmlgl6nEKCx5luQPxSRc65cI4YKmwx3W066trAwKlrXY3rStQ0wEK20AgcK4V5kFgbYN5A== 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: BL0PR11MB3043.namprd11.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 3cb5e774-ac8c-4006-a2a9-08d85e42c1ce X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Sep 2020 15:26:48.3253 (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: yu5q9Iq4NiFihWnRKrKUDZZyRDQMtIXrLCXR1Ona+vtQtgRc470K951VGH6+N7NKMwL74ox7S1HILehYS7eqNw== X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL0PR11MB3042 X-OriginatorOrg: intel.com Subject: Re: [dpdk-dev] [dpdk-dev v9 1/4] cryptodev: add crypto data-path service 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, September 21, 2020 1:00 PM > To: Zhang, Roy Fan ; dev@dpdk.org; Ananyev, > Konstantin ; Thomas Monjalon > > Cc: Trahe, Fiona ; Kusztal, ArkadiuszX > ; Dybkowski, AdamX > ; Bronowski, PiotrX > ; Anoob Joseph > Subject: RE: [dpdk-dev v9 1/4] cryptodev: add crypto data-path service AP= Is >=20 > Hi Fan, >=20 > > > > > > > > +/** Crypto data-path service types */ > > > > +enum rte_crypto_dp_service { > > > > + RTE_CRYPTO_DP_SYM_CIPHER_ONLY =3D 0, > > > > + RTE_CRYPTO_DP_SYM_AUTH_ONLY, > > > > + RTE_CRYPTO_DP_SYM_CHAIN, > > > > + RTE_CRYPTO_DP_SYM_AEAD, > > > > + RTE_CRYPTO_DP_N_SERVICE > > > > +}; > > > > > > Comments missing for this enum. > > > Do we really need this enum? > > > Can we not have this info in the driver from the xform list? > > > And if we really want to add this, why to have it specific to raw dat= a path > APIs? > > > > > Will add comments to this enum. > > Unless the driver will store xform data in certain way (in fact QAT has= it) the > > driver may not know which data-path to choose from. > > The purpose of having this enum is that the driver knows to attach the > correct > > handler into the service data structure fast. > > > I believe all drivers are storing that information already in some way in= the > session private data. > This enum is maintained inside driver as of current implementation. This = is > not specific to raw > Data path APIs. If you are introducing this enum in library, then it shou= ld be > generic for the legacy > Case as well. > [Fan: I am not sure other drivers (that's part of the reason why it is here= ), but indeed QAT does, Ok]=20 =20 >=20 > > > > > > > +union rte_crypto_sym_additional_data { > > > > + struct { > > > > + void *cipher_iv_ptr; > > > > + rte_iova_t cipher_iv_iova; > > > > + void *auth_iv_ptr; > > > > + rte_iova_t auth_iv_iova; > > > > + void *digest_ptr; > > > > + rte_iova_t digest_iova; > > > > + } cipher_auth; > > > > > > Should be chain instead of cipher_auth > > This field is used for cipher only, auth only, or chain use-cases so I = believe > this is > > a better name for it. >=20 > Agreed that this struct will be used for all 3 cases, that is what is hap= pening in > Other crypto cases. We use chain for all these three cases in legacy code= path. > Chain can be of one or two xforms and ordering can be anything - > Cipher only, auth only, cipher auth and auth cipher. >=20 >=20 > > > > > > > + struct { > > > > + void *iv_ptr; > > > > + rte_iova_t iv_iova; > > > > + void *digest_ptr; > > > > + rte_iova_t digest_iova; > > > > + void *aad_ptr; > > > > + rte_iova_t aad_iova; > > > > + } aead; > > > > +}; > > > > + > > > > /** > > > > * Synchronous operation descriptor. > > > > * Supposed to be used with CPU crypto API call. > > > > @@ -57,12 +81,25 @@ struct rte_crypto_sgl { > > > > struct rte_crypto_sym_vec { > > > > /** array of SGL vectors */ > > > > struct rte_crypto_sgl *sgl; > > > > - /** array of pointers to IV */ > > > > - void **iv; > > > > - /** array of pointers to AAD */ > > > > - void **aad; > > > > - /** array of pointers to digest */ > > > > - void **digest; > > > > + > > > > + 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; > > > > + }; > > > > > > Can we also name this struct? > > > Probably we should split this as a separate patch. > > [Then this is an API break right?] >=20 > Since this an LTS release, I am ok to take this change. > But others can comment on this. > @Ananyev, Konstantin, @Thomas Monjalon > Can you comment on this? >=20 > > > > > > > + > > > > + /* Supposed to be used with > > > > rte_cryptodev_dp_sym_submit_vec() > > > > + * call. > > > > + */ > > > > + union rte_crypto_sym_additional_data *additional_data; > > > > + }; > > > > + > > > > > > Can we get rid of this unnecessary union > rte_crypto_sym_additional_data > > > And place chain and aead directly in the union? At any point, only on= e of > the > > > three > > > would be used. > > We have 2 main different uses cases, 1 for cpu crypto and 1 for data pa= th > APIs. > > Within each main uses case there are 4 types of algo (cipher only/auth > > only/aead/chain), one requiring HW address and virtual address, the oth= er > > doesn't. > > It seems to causing too much confusion to include these many union into > the > > structure that initially was designed for cpu crypto only. > > I suggest better to use different structure than squeeze all into a big= union. > > >=20 > IMO, the following union can clarify all doubts. > @Ananyev, Konstantin: Any suggestions from your side? >=20 > /** IV and aad information for various use cases. */ > 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; > } cpu_crypto; < or any other useful name> > /* Supposed to be used with HW raw crypto API call. */ > struct { > void *cipher_iv_ptr; > rte_iova_t cipher_iv_iova; > void *auth_iv_ptr; > rte_iova_t auth_iv_iova; > void *digest_ptr; > rte_iova_t digest_iova; > } hw_chain; > /* Supposed to be used with HW raw crypto API call. */ > struct { > void *iv_ptr; > rte_iova_t iv_iova; > void *digest_ptr; > rte_iova_t digest_iova; > void *aad_ptr; > rte_iova_t aad_iova; > } hw_aead; > }; >=20 >=20 [Structure looks good to me thanks!]=20 >=20 > > > > +/** > > > > + * Context data for asynchronous crypto process. > > > > + */ > > > > +struct rte_crypto_dp_service_ctx { > > > > + void *qp_data; > > > > + > > > > + struct { > > > > + cryptodev_dp_submit_single_job_t submit_single_job; > > > > + cryptodev_dp_sym_submit_vec_t submit_vec; > > > > + cryptodev_dp_sym_operation_done_t submit_done; > > > > + cryptodev_dp_sym_dequeue_t dequeue_opaque; > > > > + cryptodev_dp_sym_dequeue_single_job_t dequeue_single; > > > > + cryptodev_dp_sym_operation_done_t dequeue_done; > > > > + }; > > > > + > > > > + /* Driver specific service data */ > > > > + __extension__ uint8_t drv_service_data[]; > > > > +}; > > > > > > Comments missing for structure params. > > > Struct name can be rte_crypto_raw_dp_ctx. > > > > > > Who allocate and free this structure? > > Same as crypto session, the user need to query the driver specific serv= ice > data > > Size and allocate the buffer accordingly. The difference is it does not= have > to > > Be from mempool as it can be reused. >=20 > So this structure is saved and filled by the lib/driver and not the appli= cation. > Right? > This struct is opaque to application and will be part of session private = data. > Right? > Assignment and calling appropriate driver's call backs will be hidden ins= ide > library > and will be opaque to the application. In other words, the structure is n= ot > exposed > to the application. > Please add relevant comments on top of this structure. >=20 [Fan: will do]=20 >=20 > > > > +static __rte_always_inline int > > > > +_cryptodev_dp_sym_dequeue_single_job(struct > > > rte_crypto_dp_service_ctx > > > > *ctx, > > > > + void **out_opaque) > > > > +{ > > > > + return (*ctx->dequeue_single)(ctx->qp_data, ctx->drv_service_data= , > > > > + out_opaque); > > > > +} > > > > + > > > > +/** > > > > + * Submit single job into device queue but the driver will not sta= rt > > > > + * processing until rte_cryptodev_dp_submit_done() is called. This= is a > > > > + * simplified >=20 > Comment not complete. >=20 > Regards, > Akhil