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 6DE89A04CC; Mon, 21 Sep 2020 13:59:47 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4C6E81D95B; Mon, 21 Sep 2020 13:59:47 +0200 (CEST) Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05on2080.outbound.protection.outlook.com [40.107.20.80]) by dpdk.org (Postfix) with ESMTP id DCB271D94B for ; Mon, 21 Sep 2020 13:59:45 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=Pc0+kE1WNeGX8/trTK3jZH2Ut0VEHw/gSFTsof0Eo2jOFScvjfpjkTcm6NxsN8E+6tW9KebEhP/idjFp1vxIqbgVtb5cDvGCppHFaPoelGklseGbaDbMrz5QDomY0g3Diug7HVLXtFmnrXxECWWStesGGXj73FiJ0sYfIvPdBg3KFrNYORmozRWWMTCawl2U+RzQAPVGUlBClQr2g4RqauWjKQXNeLnGEe/6gbgn4vhXsDLqTHRqBdxPdlQrDiigE/ytRoDXBWUPf8JNmWeQ8krt5bZDoj/MLnXDwnxStz5+M2WdGCUUz1cpYfmFp7gjxrZfBPd4wU8uk+OS5PiL0w== 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=MqzovT2WN20EsiGxFHhvDz3UCL/gk73HdA+BiWJA0ds=; b=mAOrnzmWExYncvwXwGDEOnstpikLm/jQdqad/KAzZNYT76NXLYL+BbbPiT1JZxmtocZRGjB5IFfUKBTaAGYQ3Cy+8BXcNcFBo6uy2a9NMiMR2xC1c5DQi1gJS5hNbReA6rJezlmJlXL64C4nxZtg3a3gWed9P/WuFJEbOti/3Hr7olZ+6FdCmsTRDs/P8k7XmaJ0Kdknf99dLdBKsJp0XqloZoHpwxBskzInOcLwx4Zk+91zQsMvyM3qQ8YtOmPse6VFHFvvuAR4ch13oFueQCNlX5n2GEUxI7XYcKWkuLW0PuOJy1+j//xpSs0b2gbetwHLpfNGdQHz/1ZmkXdqyQ== 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=MqzovT2WN20EsiGxFHhvDz3UCL/gk73HdA+BiWJA0ds=; b=TFPgiSrScWn84MUb7CKwpR2N995wpWeL4fR4lyXduXWszZZ52A39MdPAyOvBZMlkeNWiGQrbWzw/AbTFyt5b901iEem6eW2wbZxfbb0Pnp4njda9R+ktZOGsVHxdcg1eLhsj6O2H4JQ8JQDaxpiSZyHlDHB/iQ9Yh8rC/sbmW08= Received: from VI1PR04MB3168.eurprd04.prod.outlook.com (2603:10a6:802:6::10) by VI1PR04MB5839.eurprd04.prod.outlook.com (2603:10a6:803:e0::23) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3391.11; Mon, 21 Sep 2020 11:59:43 +0000 Received: from VI1PR04MB3168.eurprd04.prod.outlook.com ([fe80::9513:3b55:931f:216e]) by VI1PR04MB3168.eurprd04.prod.outlook.com ([fe80::9513:3b55:931f:216e%4]) with mapi id 15.20.3391.026; Mon, 21 Sep 2020 11:59:43 +0000 From: Akhil Goyal To: "Zhang, Roy Fan" , "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: AQHWhbwSwlDH+pOJNkGpff/lHViImKlu7BjQgAQPvYCAAAr4EA== Date: Mon, 21 Sep 2020 11:59:42 +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: en-IN, en-US Content-Language: en-US X-Mentions: konstantin.ananyev@intel.com,thomas@monjalon.net 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=nxp.com; x-originating-ip: [122.162.67.38] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 9c0f31dc-5dc8-41b2-5a40-08d85e25d3bb x-ms-traffictypediagnostic: VI1PR04MB5839: 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: V0zAnzL/7HAwKfl557PV+npLCoukwhGlZyEoCZw4xzccD9w+QCQYJzKncM9kDwDqjdz1N1cmZuiT/59vaRo4GOrT5cqU6pGU/E2E/lLwi8cLaEY4wYuA7GSyweZ+t1Kp0JAJjs0e82pSj8CQtfE1AQgGpWXztNKtLfVJ6BQg+a4/QZiSFz4VkgAU3AJqR0xyEpXOV0Yo9ND99CQiAUe4TWeMYuAcrEQoBGQWLPEOk872aMn4baOxc0lIt+nHIhQopyYQOBfxvhjaZUqyMfqPEna24Fb2MRua0dSoygO2oYrfflUmAnhwPmPnd+k1KLHl x-forefront-antispam-report: CIP:255.255.255.255; CTRY:; LANG:en; SCL:1; SRV:; IPV:NLI; SFV:NSPM; H:VI1PR04MB3168.eurprd04.prod.outlook.com; PTR:; CAT:NONE; SFS:(4636009)(366004)(396003)(376002)(39860400002)(346002)(136003)(186003)(54906003)(83380400001)(26005)(478600001)(4326008)(44832011)(110136005)(316002)(8936002)(5660300002)(66946007)(64756008)(86362001)(7696005)(76116006)(71200400001)(9686003)(2906002)(52536014)(55016002)(66476007)(6506007)(8676002)(33656002)(66446008)(66556008); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: Sv0atCj6zrCtsl5awFNwuPmVqCKU1HFnbVsmTAq0MRckNSmxtc/FpnZGzCLKTFgxzLbmCPXpW+rAvQhXuvVYHxSVgpyGA/XIjvDhkmDT2Sloq9QJ2GflIdAhr3LVwB0h5aBNuauSiu8kgl9CaXD6udYRprKLkMmp8e+h/c4ddnHJbSncNvnLAryIWxis+B6nnPwdw0NX1hdrgjYLrOMPTXtGC2zu5qllD/dh5FrQzlYynjkvbSLoHlmBbskiJl1BFiMbJe1nL4BkVSSlJP7Ff2tKa6HXq/elbi2ZIJ22Z3Lqtf1n4vxCZeca4fd4SoZqeoevJaoNS4ckeGWd6xfwm2Ah/ySQ0s5mQr/rW/ZUvH7l1AJKzoB1Vxw8dTmtSRzBMPyzpgAvA6YhwYQvUOCiIE0swwvftaEDznz0t3gYfrCTAX8uzH6jjkCad8X6O8VKddcOXpoSS3rB+vYXso4IMamoe7ZZy0Vmcsym7vSUWP6kMthDiawUmmoNGqeXjHhBw5qjFZ49zsuzxB/MCVSki3OAyogAI2PweJh4+9NCZtYGEw5AkbA3o/MYQetmRXTNQ8e+uS/0yXW7ULPpHbVi44S4zHKfhTJrDpW17Fv1mjyj9hj/ZrCvOOLMFIic75wFxPzIeQOnIOScx3D9Wxsgeg== 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-AuthAs: Internal X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB3168.eurprd04.prod.outlook.com X-MS-Exchange-CrossTenant-Network-Message-Id: 9c0f31dc-5dc8-41b2-5a40-08d85e25d3bb X-MS-Exchange-CrossTenant-originalarrivaltime: 21 Sep 2020 11:59:43.0058 (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: OyoMDc3P81Klf87O982Ov1WAW/Ra1TghrbGsoQrq9pBjvFQVnjrKgZ8BXa2os5s4RUcluauj0iz3I7UEN7Song== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB5839 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 Fan, > > > > > > +/** 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 data = path APIs? > > > Will add comments to this enum. > Unless the driver will store xform data in certain way (in fact QAT has i= t) 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 co= rrect > handler into the service data structure fast. >=20 I believe all drivers are storing that information already in some way in t= he 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 should= be generic for the legacy Case as well. > > > > > +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 be= lieve this is > a better name for it. Agreed that this struct will be used for all 3 cases, that is what is happe= ning in Other crypto cases. We use chain for all these three cases in legacy codepa= th. Chain can be of one or two xforms and ordering can be anything - Cipher only, auth only, cipher auth and auth cipher. > > > > > + 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?] 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? > > > > > + > > > + /* 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 one = of the > > three > > would be used. > We have 2 main different uses cases, 1 for cpu crypto and 1 for data path= 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 other > doesn't. > It seems to causing too much confusion to include these many union into t= he > structure that initially was designed for cpu crypto only. > I suggest better to use different structure than squeeze all into a big u= nion. >=20 IMO, the following union can clarify all doubts. @Ananyev, Konstantin: Any suggestions from your side? /** 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>=20 /* 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; }; > > > +/** > > > + * 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 servic= e data > Size and allocate the buffer accordingly. The difference is it does not h= ave to > Be from mempool as it can be reused. So this structure is saved and filled by the lib/driver and not the applica= tion. Right? This struct is opaque to application and will be part of session private da= ta. Right? Assignment and calling appropriate driver's call backs will be hidden insid= e library and will be opaque to the application. In other words, the structure is not= exposed to the application. Please add relevant comments on top of this structure. > > > +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 start > > > + * processing until rte_cryptodev_dp_submit_done() is called. This i= s a > > > + * simplified Comment not complete. Regards, Akhil