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 80D85A00BE; Tue, 7 Jul 2020 22:37:06 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 4F72F1DD46; Tue, 7 Jul 2020 22:37:05 +0200 (CEST) Received: from EUR04-VI1-obe.outbound.protection.outlook.com (mail-eopbgr80082.outbound.protection.outlook.com [40.107.8.82]) by dpdk.org (Postfix) with ESMTP id 736291DD39 for ; Tue, 7 Jul 2020 22:37:03 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=TxG4GWESmixFF8q2HHvHKd74FTrEvj8KnS/XUDwr9MFxP8QVQMJhXuAlELS/HFQ6GvEUlkfyRpc3TuLzaD/mE7ELXPgWCYn5IhVbYckH55V/K0UhI/GmFUxm6kKVPqFwgDEf56FwgkhvGjAS8dBnx3BBuGrjXJXKHXKu5EHoYWOJkjKnuErRh8WRJw42oD7pNDNUWpMko3+lcwbv63NfMKEnFG6v04v9FwVcoeEK6/U9iO0Ib+LV3A1T4jR0T3uWl27O58cIX03gevFqMNkRneIpgqSJ24rFvrRNWOOzHRbJQzkTYj4F1tAWXyEeQa8c5HTEIXk7bK2YpfAAjjQmhA== 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=QRVxs7fey2/+NSScxquZN8x2w6yigulz4ic/TooLH2w=; b=jyDGDUpP9KXobGLuyRhEpIAKck77L2DLA+GcUoPx2Ca0Rx7ndzBGwgEbZAnUfB0vFj/l8nidZqE8aciQNyQd8B4zJmxNnd5IYkyOBdmrO+B4imiGtGThJ7RpyBt/hbuLQWGDuuok2ljCshnFKVAb5mvszv9r3RV79IWuL5u9qTocf/NaORjexUHIOZEeI6WXyXu888Omy8fbaOz5xTDfW3YNsXvyN3jKdBWiJKr59uZ2aIa+VXSw6Y/NygWOqCIwI03cxzY5JCeuV2DsWOJT2ReNGYLrz7nlYHMQ+ftWyqKoNLjTRV9bCiOOIhAfkx28au4uRO6YJPWUGjqG7z4TXQ== 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=QRVxs7fey2/+NSScxquZN8x2w6yigulz4ic/TooLH2w=; b=X7FQT4fOzZHteifvLJBKLnLv2coqGMDwsz3lApwYTS79q+At8i4wsaBegDE8hsUjDY7GvJigbBoc20rII5QDGOgEg+EW3h72W3mnXLGHiIYsZaLwLOrWxwgChn0hF9Des0HxQb40GEUvjp6+XwmumH6fz4XtqnXFvH6dRBUmek4= Received: from VI1PR04MB3168.eurprd04.prod.outlook.com (2603:10a6:802:6::10) by VI1PR04MB7134.eurprd04.prod.outlook.com (2603:10a6:800:12e::11) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3174.21; Tue, 7 Jul 2020 20:37:02 +0000 Received: from VI1PR04MB3168.eurprd04.prod.outlook.com ([fe80::b077:1fe4:d352:b464]) by VI1PR04MB3168.eurprd04.prod.outlook.com ([fe80::b077:1fe4:d352:b464%7]) with mapi id 15.20.3153.029; Tue, 7 Jul 2020 20:37:02 +0000 From: Akhil Goyal 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" , Hemant Agrawal 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: AQHWUThwVrYw4m7l8kKR81UTdxPeb6j3tmWggAKgRYCAAByaIIABoO+AgAB+EqA= Date: Tue, 7 Jul 2020 20:37:02 +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: en-IN, 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=nxp.com; x-originating-ip: [45.118.167.74] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 084c2d91-8e9c-4a23-257d-08d822b58117 x-ms-traffictypediagnostic: VI1PR04MB7134: 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: rXqecBWv12iwgdKG4acOV34CSIbNRDYvYG3kMt42p8+2SuluOZdhhKB0sLH3Xw1qgu2tCGh5dnGIDpR1VaYln8rJVjJsjHvIBAz/cuOOImFIuUWO+6EABqVObZHdHIUJKSTmI4WWrbKj3NrowmoPNrfv242Dx3qe2aNjZv6SA51zchJ7vUdE9EzH9JD4fQnQi2oUb4NxGdGRP9SJc7rv7M3tbhguXeFr62k3zBeTQw0yiQwxDXi2peHQ8oNhd4tdfsPJFaGt2M0rPBEwrZeFePXhoU/zUhoyLO85iL46/HZWA3ysSMPj6lsW8Lkw9hpoqVHBBeDSO0GSkzVFb2V5SO67VjUpsG6OMqnZPHe80p85P8ayz0hiM1Itw3vXX7s5rpw6y38WXSI58Yu9J2d0cvb2t99VwqTJB4BOYRtifvVFSCM4rgxkK+z27MySljRr 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; SFTY:; SFS:(4636009)(39860400002)(136003)(376002)(366004)(346002)(396003)(110136005)(33656002)(54906003)(86362001)(2906002)(966005)(6636002)(5660300002)(9686003)(26005)(83380400001)(7416002)(55016002)(44832011)(8676002)(6506007)(7696005)(66446008)(64756008)(66556008)(66476007)(4326008)(8936002)(186003)(316002)(52536014)(478600001)(71200400001)(76116006)(66946007)(921003); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: DPZcXk84h1qBCecc50pkOT6mSMfGY7uOf89bHZL/xOAju3AiPUJXLyhBStnX3r9xgeFur6VQX7+LiCIe7Nvytkkv22IUqEIWzhOg79YnhYj8KspAaEHGRb0Ob7/RZjGVV2UYMxSOjqlpIrDLj3yl2ytu4Rdp1bAkazsvow/xt1k/RoLwQhhSF6rH/uHZPk6Y+cmQxCa5Zn4fIlIQ3XfaaYV7fN4rNeBigp0bfd3b3TIQ1FHONyuu/nudk4fNABriruKQL+rMcx254NK0/1T1+S2Ukcfr1p41xlPY+1Tsr5Yo0jRY3E63wv7evqB/8VeyVkH46SvuhgbsN9OBQBPow5/lWxwGffWiUeluEba77ejKFawyTjK6dkw/OC+SQwZpA6Jtsw6B7uEbbELpc3ZKC5G6CUUVfKFb3S/224THBnj+l/s1TRBZ4t5BkvxGbUG4v72Hp1H8r3MQWLFOKUnA7LgI7KXmQMNtwWgNnmucMEM= 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: 084c2d91-8e9c-4a23-257d-08d822b58117 X-MS-Exchange-CrossTenant-originalarrivaltime: 07 Jul 2020 20:37:02.1139 (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: RVVvlMHlbuDqzSXGal7TVOx+PYnNojoy2Ds0XTiiXqSkN0fjT7ZWbZ++RbyUTwdympYWG22JJFIUSr1/9agA3g== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB7134 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 Fan, >=20 > Hi Akhil, >=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 app= lication > > > may sooner know when a specific enqueue is failed. > > > > You may also need to save a virtual address as well. As some hardware a= re > > able to > > Convert virtual to physical addresses on it's own giving a performance > > improvement. > > > > I do not see an issue in using enqueue burst with burst size=3D1 , but = since you > > are doing > > Optimizations, none of the hardware can perform well with burst =3D 1, = I think > > it is always > > Greater than 1. >=20 > Shall I update the rte_crypto_sym_vec as the following - so the 2 problem= s can > be > resolved? >=20 > 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; > }; >=20 > /* 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; > }; >=20 > }; > /** number of operations to perform */ > uint32_t num; > }; Yes something of that sort can work.=20 The current use case was also discussed in the CPU crypto mail chain About the need of non-mbuf use case for async enq/deq APIs. http://patches.dpdk.org/patch/58528/#101995 >=20 > > > > > > > Now for this patchset, the requirement is > > > > - raw buffers > > > > - asynchronous APIs > > > > > > > > The data structure for raw buffers and crypto related offsets are a= lready > > > > 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 combi= ned > > 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= be > > NULL, > > > such as > > > digest pointers, I have given a lot thought to this structure. Hopefu= lly 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 fi= ll all > > > jobs to the PMD HW. > > > > 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 sessi= ons. > > It does not take care of asymmetric crypto. > > So whenever, a vendor need to support all these, we would end up gettin= g > > 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 fr= om > it. More importantly rte_crypto_op is designed to be taken from heap and > being freed after dequeue. So they cannot be allocated from stack - for t= his > reason I think rte_crypot_sym_vec is a better fit for the patch, do you a= gree? > (the Proposed change is at above). Agreed. >=20 > > Have you done some profiling with using rte_crypto_op instead of this n= ew > > struct? > > > 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 > frame of data. Lucky VPP has space to store mbufs otherwise the perf will > be even worse. What is the performance gap do you see after making m_src and m_dst as Raw buffers? >=20 > > > > > > > > > > > > +/* Struct for user to perform HW specific enqueue/dequeue functi= on > > 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 f= or > > > > 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= type > > > could save a lot of branches for the driver to handle. As mentioned t= his > > > data-path API is intended to be used as an advanced feature to provid= e > > > close-to-native perf to external library/applications that are not mb= uf > > > centric. And I don't agree classifying choosing 1 enqueue function fr= om > > > 4 candidates as "pain". > > > > 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 dri= ver. > > >=20 > That's a good point :-) we definitely can do something about it in future= releases. >=20 > > > > > > > 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= difficult > > > > 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-m= buf > > > data-path centric applications/libraries, in the meantime not creatin= g > > > confusion. In this version we aim to provide a more friendly data-pat= h for > > > > 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 min= imal > > changes so that people can migrate easily. > > >=20 > We are working on next version that is based on rte_crypto_sym_vec and si= ngle > Enqueue-then-dequeue API. To be honest it won't be that of friendly to > application > developer that the applications are mbuf-based or already built on top of > cryptodev, > however for the applications that are not mbuf based and having their own > data-path > structures it will be surely friendlier than existing enqueue and dequeue= APIs. No > dependency to mbuf, mbuf and crypto op pool, and no need to consider how = to > adapt > different working methods. Agreed with your point. The intention is just not to create multiple copies= of Same information in multiple structures. >=20 > > > them, and aims to be useful to all vendor's PMDs. If there is any pla= ce in > > > the API that blocks a PMD please let me know. > > > > As commented above, sessionless, rte_security sessions, asymmetric cryp= to > > Not supported. > > > You are right - > Sessionless support aims the usability and is not intended to be used in = high- > throughput > Application. There may be some cases where a limited amount of control pkts can be sent Which may be session less. We cannot ask people to use a different data pat= h For such traffic. So we may need to support that too. > Rte_security is built on top of mbuf and ethdev and is not intended to "m= buf- > independent" > applications either. Rte_security for lookaside protocol can be mbuf independent and NXP may Support it in future especially in case of PDCP. Regards, Akhil