Hi Akhil, > -----Original Message----- > From: Akhil Goyal [mailto:akhil.go...@nxp.com] > Sent: Friday, June 21, 2019 1:23 PM > To: Trahe, Fiona <fiona.tr...@intel.com>; Kusztal, ArkadiuszX > <arkadiuszx.kusz...@intel.com>; > dev@dpdk.org; shally.ve...@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.go...@nxp.com] > > > Sent: Thursday, June 20, 2019 3:17 PM > > > To: Kusztal, ArkadiuszX <arkadiuszx.kusz...@intel.com>; dev@dpdk.org > > > Cc: Trahe, Fiona <fiona.tr...@intel.com>; > > shally.ve...@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.kusz...@intel.com> > > > > --- > > > Acked-by: Akhil Goyal <akhil.go...@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 > that. [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 > > > > > > > -Akhil
