[dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
fiona.trahe at intel.com
Fri Jun 21 16:18:25 CEST 2019
> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
> Sent: Friday, June 21, 2019 1:23 PM
> To: Trahe, Fiona <fiona.trahe at intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal at intel.com>;
> dev at dpdk.org; shally.verma at caviumnetworks.com
> Subject: RE: [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
> Hi Fiona,
> > Hi Akhil, Arek, Shally,
> > > -----Original Message-----
> > > From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
> > > Sent: Thursday, June 20, 2019 3:17 PM
> > > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal at intel.com>; dev at dpdk.org
> > > Cc: Trahe, Fiona <fiona.trahe at intel.com>;
> > shally.verma at caviumnetworks.com
> > > Subject: RE: [PATCH] cryptodev: extend api of asymmetric crypto by
> > sessionless
> > > >
> > > > Asymmetric cryptography algorithms may more likely use
> > > > sessionless API so there is need to extend API.
> > > >
> > > > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal at intel.com>
> > > > ---
> > > Acked-by: Akhil Goyal <akhil.goyal at nxp.com>
> > [Fiona] The code is ok but I think a little more is needed.
> > As all PMDs don't support sessionless, this needs to be handled as an optional
> > capability.
> > And in future some PMDs may only support SESSIONLESS and some only support
> > WITH_SESSION.
> I believe this holds true for symmetric crypto as well. But adding a feature flag for everything may beat
> the purpose
> Of adding a feature flag. Sessionless crypto operations in symmetric crypto is being used without any
> issue for a long
> And nobody feel the need of that as of today. So my question is how asymmetric crypto pmds are
> different that they
> Need feature flag?
> If the driver does not support sessionless, then it may give an error while creating it. I don't think that is
> an issue. It is
> Already being handled in the rte_crypto_op by an enum which denote that the 'op' need to be
> processed with some
> Session or with xform.
> > 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 gets.
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.
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 having a feature flag.
However if a PMD doesn't support SESSIONLESS, then the fail will only occur on the op_enqueue_burst.
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 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.
> > We'll follow up with SESSIONLESS QAT implementation and UTs in a separate
> > patchset.
> > Also documentation updates should go with this API patch, i.e.
> > - update section 16.7.2 in the cryptodev programmers guide - and review that
> > doc in case other sections need updating.
> Yes this needs to be updated if the implementation is complete and we have some PMD supporting
[Fiona] Are you saying we need to submit the PMD changes with this API patch? Or can we do
separately as we'd planned?
> > - fix comment in rte_crypto.h under STATUS_INVALID_SESSION
> > - release note
More information about the dev