From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
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 <dev@dpdk.org>; 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 <akhil.goyal@nxp.com>
To: "Zhang, Roy Fan" <roy.fan.zhang@intel.com>, "dev@dpdk.org" <dev@dpdk.org>,
"anoobj@marvell.com" <anoobj@marvell.com>, "asomalap@amd.com"
<asomalap@amd.com>, "ruifeng.wang@arm.com" <ruifeng.wang@arm.com>,
Nagadheeraj Rottela <rnagadheeraj@marvell.com>, Michael Shamis
<michaelsh@marvell.com>, Ankur Dwivedi <adwivedi@marvell.com>, Jay Zhou
<jianjay.zhou@huawei.com>, "De Lara Guarch, Pablo"
<pablo.de.lara.guarch@intel.com>, Hemant Agrawal <hemant.agrawal@nxp.com>
CC: "Trahe, Fiona" <fiona.trahe@intel.com>, "Bronowski, PiotrX"
<piotrx.bronowski@intel.com>, "Ananyev, Konstantin"
<konstantin.ananyev@intel.com>, Thomas Monjalon <thomas@monjalon.net>
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: <VI1PR04MB3168C39E77B1B13992283038E6660@VI1PR04MB3168.eurprd04.prod.outlook.com>
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>
<VI1PR04MB316870AC08947B06B5F8C870E66B0@VI1PR04MB3168.eurprd04.prod.outlook.com>
<BL0PR11MB30438061A6C5275B004B4369B8690@BL0PR11MB3043.namprd11.prod.outlook.com>
<VI1PR04MB31680EC601207306CEBCBB53E6690@VI1PR04MB3168.eurprd04.prod.outlook.com>
<DM6PR11MB3049261536505223238E4BAEB8660@DM6PR11MB3049.namprd11.prod.outlook.com>
In-Reply-To: <DM6PR11MB3049261536505223238E4BAEB8660@DM6PR11MB3049.namprd11.prod.outlook.com>
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: <VI1PR04MB7134C41D570BFC08C7DB0532E6660@VI1PR04MB7134.eurprd04.prod.outlook.com>
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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
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