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 706B2A046B for ; Fri, 28 Jun 2019 19:27:50 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id BE5893798; Fri, 28 Jun 2019 19:27:48 +0200 (CEST) Received: from mga04.intel.com (mga04.intel.com [192.55.52.120]) by dpdk.org (Postfix) with ESMTP id 605FB34F0 for ; Fri, 28 Jun 2019 19:27:46 +0200 (CEST) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by fmsmga104.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 28 Jun 2019 10:27:45 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.63,428,1557212400"; d="scan'208";a="153430236" Received: from irsmsx109.ger.corp.intel.com ([163.33.3.23]) by orsmga007.jf.intel.com with ESMTP; 28 Jun 2019 10:27:43 -0700 Received: from irsmsx101.ger.corp.intel.com ([169.254.1.80]) by IRSMSX109.ger.corp.intel.com ([169.254.13.220]) with mapi id 14.03.0439.000; Fri, 28 Jun 2019 18:27:42 +0100 From: "Trahe, Fiona" To: Shally Verma , Akhil Goyal , "Kusztal, ArkadiuszX" , "dev@dpdk.org" CC: "Trahe, Fiona" Thread-Topic: [PATCH] cryptodev: extend api of asymmetric crypto by sessionless Thread-Index: AQHVGkUHWWASMFfof0mYfFRjYZRUS6akoF0AgAF30JD///qlgIAAHWVggARAxgCABRFeYIAB0HqAgAAlJEA= Date: Fri, 28 Jun 2019 17:27:42 +0000 Message-ID: <348A99DA5F5B7549AA880327E580B435897A7E73@IRSMSX101.ger.corp.intel.com> References: <20190603194411.8352-1-arkadiuszx.kusztal@intel.com> <348A99DA5F5B7549AA880327E580B435897A26ED@IRSMSX101.ger.corp.intel.com> <348A99DA5F5B7549AA880327E580B435897A28B6@IRSMSX101.ger.corp.intel.com> <348A99DA5F5B7549AA880327E580B435897A68FE@IRSMSX101.ger.corp.intel.com> In-Reply-To: Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiZGY3MWUzZGYtOWZmNy00OTgxLWE1YzEtMGZlOWU5NmU3OGZmIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoicWN6emxCN2xrdHR3MTZmUFNBK2ZwZzVWMDlVREdlT0tSWVFaZzd5dXRSRUpwaXVxZGdEWDJKOHVOcGxqaEVvZiJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.600.7 dlp-reaction: no-action x-originating-ip: [163.33.239.182] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless 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 Shally, > -----Original Message----- > From: Shally Verma [mailto:shallyv@marvell.com] > Sent: Friday, June 28, 2019 5:11 PM > To: Trahe, Fiona ; Akhil Goyal ; Kusztal, ArkadiuszX > ; dev@dpdk.org > Subject: RE: [PATCH] cryptodev: extend api of asymmetric crypto by sessio= nless >=20 > Hi Finoa, Akhil >=20 > > -----Original Message----- > > From: Trahe, Fiona > > Sent: Thursday, June 27, 2019 5:25 PM > > To: Akhil Goyal ; Kusztal, ArkadiuszX > > ; dev@dpdk.org; Shally Verma > > > > Cc: Trahe, Fiona > > Subject: [EXT] RE: [PATCH] cryptodev: extend api of asymmetric crypto b= y > > sessionless > > > > External Email > > > > ---------------------------------------------------------------------- > ... >=20 > > > > > > So I propose adding 2 feature flags to the API > > > > > > RTE_CRYPTODEV_FF_ASYM_WTH_SESSION > > > > > > RTE_CRYPTODEV_FF_ASYM_SESSIONLESS and including in this patch > > > > > > the PMD and UT changes to set and test the first > > > > flag. > > > > [Fiona] symmetric crypto is inherently session-based, so all PMDs > > > > support this. I don't know how much real use SESSIONLESS actually g= ets. > > > > For asymmetric, my understanding is that sessionless is more likely= to be > > used. > > > > Sequences of ops using the same params/keys are an unlikely > > > > use-case, so there's no advantage to setting up a session and it's > > > > an extra API call so preferable to avoid. > > > > > > Agreed, if Asymmetric is not likely to have sessions, it should not c= all that > > API. > [Shally] I would rather say, asymmetric are using session based calls but= those are not so useful as unlike > symmetric, session params doesn't say same for larger amount of data. > So, its instead useful to have sessionless support for same. >=20 >=20 > > > > > > > That said, I think it would be ok with one feature flag. > > > > If a PMD doesn't support WITH_SESSION, the session_init API will > > > > fail with - ENOTSUP, so giving the app the information it needs. > > > > This can be documented as a PMD limitation and I'm ok with it not h= aving > > a feature flag. > > > > However if a PMD doesn't support SESSIONLESS, then the fail will > > > > only occur on the op_enqueue_burst. > > > > > > Yes on the first enqueue, before actually submitting to the hardware, > > > in the driver itself Before sending the request to hardware and retur= n > > OP_INVALID_SESSION. > > > > > > > Failure to enqueue the next op is a typical outcome on a busy > > > > hardware device, and the app will likely assume the device is busy = and try > > again with same result. > > > > > > The PMD will not be sending the request to hardware if sessionless is= not > > supported. > > > And app will not enqueue the op again if the previous error is > > OP_INVALID_SESSION. > > > > > > > The PMD could change the op.status to OP_STATUS_ERROR or > > > > OP_INVALID_SESSION but it would still require the app to check the > > > > status of the next op which failed to enqueue. I think it better to > > > > detect this before the op_enqueue by providing a > > > > RTE_CRYPTODEV_FF_ASYM_SESSIONLESS feature flag. > > > > > > On second thought, we can have another value in the enum(op->status) > > > to say sessionless Is not supported if OP_INVALID_SESSION looks > > > ambiguous instead of setting a feature flag and making each driver se= t it if it > > support sessionless or not. > > > > > > I believe that would be simple and will not bother the data path or t= he busy > > hardware. > > > Anyways in case of sessions also in sym, we make session on arrival o= f > > > 1st packet. That same logic Can be applied here also. I don't think t= hat will > > be an issue. > > [Fiona] The issue for me isn't that OP_INVALID_SESSION is ambiguous - > > although that's also true - it's that if an op fails on the enqueue, ap= plications > > may not check the op.status and so not notice the fail and would likely > > resubmit, assuming the op didn't enqueue because of a busy device. > > This could result in an infinite loop. > > > > Also in my understanding the enqueue_burst() call is part of the data p= ath, in > > which I'd include the PMD processing as well as the hardware processing= , so I > > think adding a check for this case DOES affect the data-path. > > But a few extra cycles on the data-path to check the op.status is not a= big > > performance impact for asymmetric crypto. > > So I'd suggest adding a generic RTE_CRYPTO_OP_STATUS_NOT_SUPPORTED > > and using for this case as long as we also document the following on th= e API: > > > > For asymmetric crypto operations, if an op fails to enqueue, the op.sta= tus > > must be set appropriately and the PMD should return without enqueuing > > any subsequent ops in that burst. > > It's up to the application to check if less than the full burst is enqu= eued and in > > this case to check the status of the first unenqueued op. If still > > NOT_PROCESSED, it's likely due to a busy device and a later retry with = the > > same op can be expected to succeed, for any other error the application > > should not resubmit the same op unless the error has been rectified. > > >=20 > [Shally] I would favor to have feature flag instead, to keep it simple. > We're relying too much on documentation here. Any op status, be it INVALI= D_OP_SESSION, or > NOT_SUPPORTED does not give > clear reason for failure. Assuming we agree on feature flag, then next qu= estion comes if PMD set > SESSIONLESS feature flag, then does that mean it support *only* sessionle= ss OR both "session" and > "sessionless" ? > To solve this, we can define it like this: > 1. if PMD does not set _SESSIONLESS feature flag, that implicitly mean it= support session based only > (which is current case) > 2. if PMD does set _SESSIONLESS feature flag, then app can certainly use = sessionless, but if it invokes > session init API, then > - if PMD don't have session support, it should return NOT_SUPPORTED in = session_init() > - if PMD do have session support (which will be our case), then it will= allow session APIs. Then its app > discretion to choose either of these >=20 > Does this sounds okay? >=20 > Thanks > Shally [Fiona] Yes, this is ok for me. Or to paraphrase: sessionless feature flag just informs about sessionless. The response to session_init() informs whether sessions are supported or no= t.=20 =20