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 82F01A00C5; Mon, 6 Jul 2020 14:13:30 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D0E2E1D66D; Mon, 6 Jul 2020 14:13:29 +0200 (CEST) Received: from EUR05-DB8-obe.outbound.protection.outlook.com (mail-db8eur05on2055.outbound.protection.outlook.com [40.107.20.55]) by dpdk.org (Postfix) with ESMTP id 2C7B11D669 for ; Mon, 6 Jul 2020 14:13:28 +0200 (CEST) ARC-Seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=bdz9q/zVN+N/QGCh6x4UNzOpRMW/Cy8nBxLM0G+Hea/QuQeTwj3nxy/rwgfjIlF+o47j0TgtO+yDg1/L6y7ezmnrZOFEzcrq7Cfc5rxqslN1a4JEDyK2cETP2J0J9wjUHudwh6pjgmXrvmV9WDSGXTfC6339+y4RwyLQPlaWNbpyxBzutqvj36HTWwswGKmcCq2xrC9g1gc8cRtMC/swtD8tKQFfthp+YEEF7Ls2UQITN4M4qndFl2+bwceG49XvMp2VnYX246bJcr/5MaVQNKgDjTrYiARFcVgPbw0UZWapCp6eLX4yLGNiltoLCdWgjpOX+0V9F0BO8wOcq14G5A== 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=Z1ab2TDNi5e3hxZGG7i9405Xep0Hm94lXaxuCm/tERE=; b=li4PF/m/87pw8KmSPNnGmGMRmwDerePltLinyFLibEGcltx8+BowjA858vEMQHuMEjw/dviJn4MMAY6SvrbZtN5tMO0smr6/li8wFay250YsXkf/SYsj9J2cfT2ajalBf5GU5kjJx4dqHLhOpkAUwSwtC8wvHy3HdAn2p1gR/tDX58neZOC/mZzVQ83BQw2k1imcWmezGbJjWkC+BLxGcSnR3e0MxkripWJawiLVumXA7wUaEqTrLjYbnahxYa0RUhzRoN1fdcvR+nmE4WO1LH43neDNdY4VEq5JTQKJkmk2XNTROn+hRNIyTRP04IpWn1sOfNj0wRBQNgLKk15+Gg== 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=Z1ab2TDNi5e3hxZGG7i9405Xep0Hm94lXaxuCm/tERE=; b=ob2dWynPfu3BEm4NGg3IuJC75mC+t77kniGD6F17FQbLYPk9nde3DwiHSFVi9dqpdIOGj/e7jj6oiPLlDo3QVZmMUyuAlxA0jaojKbUOYubBriIRUzE6E2N3xbdm4gwDAAcpFxX9Tt/bWwakOnUubrjYmbkgI5gKhlOz7abG4xg= Received: from VI1PR04MB3168.eurprd04.prod.outlook.com (2603:10a6:802:6::10) by VI1PR0402MB3567.eurprd04.prod.outlook.com (2603:10a6:803:c::30) with Microsoft SMTP Server (version=TLS1_2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id 15.20.3153.21; Mon, 6 Jul 2020 12:13:27 +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; Mon, 6 Jul 2020 12:13:27 +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" 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: AQHWUThwVrYw4m7l8kKR81UTdxPeb6j3tmWggAKgRYCAAByaIA== Date: Mon, 6 Jul 2020 12:13:27 +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.64] x-ms-publictraffictype: Email x-ms-office365-filtering-ht: Tenant x-ms-office365-filtering-correlation-id: 6bfc7aaf-cf94-45bf-09f0-08d821a5fd1e x-ms-traffictypediagnostic: VI1PR0402MB3567: x-microsoft-antispam-prvs: x-ms-oob-tlc-oobclassifiers: OLM:10000; x-forefront-prvs: 04569283F9 x-ms-exchange-senderadcheck: 1 x-microsoft-antispam: BCL:0; x-microsoft-antispam-message-info: 27AEJAEdLYdSyC2NhoXQRbisWUSpQoDqfZyV3sTqqh7PL9UQ1gsIkAJwYUIW1/eZWLnGfVahydSUZPiPLPYxPmdRQm/t+lx7YT/5+XGfHuNqwugzuMy/QP5DV5IZWRJST8vRB4BBQkv1nimQJAEqIDHcK8GXSZUaMFvKEBbWtTNvdqlSkRrsGcUQCPwhdPiVLxkCmyrbBqGG5kCBY8TePEIV9tZCoWn7pvS4bTA8UeobTSKePghG7AqhzDpMLv9VC/0+nfwnuoJcdTO4Iw4s9MHXDZp7+/L+QJ9iycgbhKKQKfspA2jk+d7y4BGzGjgnBsSQsyeY/alzgIvJlQRPAUBYt2RSUncT2GTSaezcsIZ96ec6Bp32urC5tInPT3PH 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)(346002)(396003)(376002)(136003)(39860400002)(366004)(6506007)(76116006)(54906003)(9686003)(66946007)(66446008)(64756008)(66556008)(66476007)(7416002)(55016002)(478600001)(316002)(110136005)(19627235002)(33656002)(71200400001)(44832011)(7696005)(52536014)(2906002)(26005)(8676002)(4326008)(5660300002)(186003)(83380400001)(8936002)(86362001)(921003); DIR:OUT; SFP:1101; x-ms-exchange-antispam-messagedata: t8tQ7mPjt1SX8D9OCX81jMqpPoJhPK+N1O3HbPbtFVMnxlixxT2L2AoFklJ8sbfbwxuy20UcER/xrKnyizQeZ1LFRDOL6HDU28+faPYhmFQseKAsc7QGwaUBkMJsvP/yeUN+itnTQlrn5mkqOR3teJJBIr0sYY19duz+wpD6tcGPQeTXqwSYg4O+60N/+h8L2Vy8+pqkbwFQrq4MFdAkDQPd8qIi0sKscPSqss+qLcOV9RW0GtmCDzaL1W/azSB50hgMv/ucfV4lLj55gj9xFqeOXfnSYZ91K47QIrY2EScLJX43w7AExWMREhybzYuKfp6SZ2xVkanZkADvmEr9eMs1+ywtGyxP8JeXlAAl48Z5fAlGWILsLrU0PtfGiRSpxKy0yYAxEtRxpvwnlX3D4VroefJ4XlqWxuoSYw9hd/jPsO+5TaJJWjAkYEKoilaqhGO4jK/94hmjRUSYhHS7Ov10z+gG2fPxgjUyySa1PAo= 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: 6bfc7aaf-cf94-45bf-09f0-08d821a5fd1e X-MS-Exchange-CrossTenant-originalarrivaltime: 06 Jul 2020 12:13:27.1370 (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: sMHUOfVUznb8xKZP5H2wLNnV07GaDGtsOBi7TQTGqxPzfBTWksufycd7tCDGjx4tsjiPwE03tNOrDv7Q09/CDQ== X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR0402MB3567 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, > Hi Akhil, >=20 > > > + > > > +/** > > > + * Asynchronous operation job descriptor. > > > + * Used by HW crypto devices direct API call that supports such acti= vity > > > + **/ > > > +struct rte_crypto_sym_job { > > > + union { > > > + /** > > > + * When RTE_CRYPTO_HW_ENQ_FLAG_IS_SGL bit is set in > > flags, > > > sgl > > > + * field is used as input data. Otherwise data_iova is > > > + * used. > > > + **/ > > > + rte_iova_t data_iova; > > > + struct rte_crypto_sgl *sgl; > > > + }; > > > + union { > > > + /** > > > + * Different than cryptodev ops, all ofs and len fields have > > > + * the unit of bytes (including Snow3G/Kasumi/Zuc. > > > + **/ > > > + struct { > > > + uint32_t cipher_ofs; > > > + uint32_t cipher_len; > > > + } cipher_only; > > > + struct { > > > + uint32_t auth_ofs; > > > + uint32_t auth_len; > > > + rte_iova_t digest_iova; > > > + } auth_only; > > > + struct { > > > + uint32_t aead_ofs; > > > + uint32_t aead_len; > > > + rte_iova_t tag_iova; > > > + uint8_t *aad; > > > + rte_iova_t aad_iova; > > > + } aead; > > > + struct { > > > + uint32_t cipher_ofs; > > > + uint32_t cipher_len; > > > + uint32_t auth_ofs; > > > + uint32_t auth_len; > > > + rte_iova_t digest_iova; > > > + } chain; > > > + }; > > > + uint8_t *iv; > > > + rte_iova_t iv_iova; > > > +}; > > > > NACK, > > Why do you need this structure definitions again when you have similar = ones > > (the ones used in CPU crypto) available for the same purpose. > > In case of CPU crypto, there were 2 main requirements > > - synchronous API instead of enq +deq > > - raw buffers. > > >=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 opera= tion is > Per operation basis instead of operating in a burst. The external applica= tion > may sooner know when a specific enqueue is failed. You may also need to save a virtual address as well. As some hardware are a= ble to Convert virtual to physical addresses on it's own giving a performance impr= ovement. I do not see an issue in using enqueue burst with burst size=3D1 , but sinc= e you are doing Optimizations, none of the hardware can perform well with burst =3D 1, I th= ink it is always Greater than 1. >=20 > > Now for this patchset, the requirement is > > - raw buffers > > - asynchronous APIs > > > > The data structure for raw buffers and crypto related offsets are alrea= dy > > 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 combined = in a > > Union and some of the fields in the rte_crypto_op can be left NULL in c= ase of > > raw buffers. > > >=20 > 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. Hopefully = it covers > all vendor's HW symmetric crypto needs and in the same time it well squee= ze > 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 a= ll > 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 sessions. 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. Have you done some profiling with using rte_crypto_op instead of this new s= truct? >=20 > > > > > +/* Struct for user to perform HW specific enqueue/dequeue function c= alls > > */ > > > +struct rte_crypto_hw_ops { > > > + /* Driver written queue pair data pointer, should NOT be alterred b= y > > > + * 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. > > >=20 > Providing different types of enqueue functions for specific operation typ= e > could save a lot of branches for the driver to handle. As mentioned this > 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". 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 driver. >=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 dif= ficult > > 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. > > >=20 > The purpose of adding data-path for the user is performance for non-mbuf > data-path centric applications/libraries, in the meantime not creating > confusion. In this version we aim to provide a more friendly data-path fo= r I do not see the new path as friendly. Adding a parallel new datapath with create more confusion for the applicati= on developer. It would be convenient, if we can use the same path with minimal changes so that people can migrate easily. > them, and aims to be useful to all vendor's PMDs. If there is any place i= n > the API that blocks a PMD please let me know. As commented above, sessionless, rte_security sessions, asymmetric crypto Not supported. >=20 > > Adding other vendors to comment. > > > > Regards, > > Akhil