Hi Fiona

Hi Akhil,

quoted

-----Original Message-----
From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
Sent: Friday, June 21, 2019 1:23 PM
To: Trahe, Fiona <redacted>; Kusztal, ArkadiuszX

[off-list ref];

quoted

dev@dpdk.org; shally.verma@caviumnetworks.com
Subject: RE: [PATCH] cryptodev: extend api of asymmetric crypto by

sessionless

quoted

Hi Fiona,

quoted

Hi Akhil, Arek, Shally,

quoted

-----Original Message-----
From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
Sent: Thursday, June 20, 2019 3:17 PM
To: Kusztal, ArkadiuszX <redacted>; dev@dpdk.org
Cc: Trahe, Fiona <redacted>;

shally.verma@caviumnetworks.com

quoted

Subject: RE: [PATCH] cryptodev: extend api of asymmetric crypto by

sessionless

quoted

quoted

Asymmetric cryptography algorithms may more likely use
sessionless API so there is need to extend API.

Signed-off-by: Arek Kusztal <redacted>
---

Acked-by: Akhil Goyal <redacted>

[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

quoted

quoted

capability.
And in future some PMDs may only support SESSIONLESS and some only

support

quoted

quoted

WITH_SESSION.

I believe this holds true for symmetric crypto as well. But adding a feature flag

for everything may beat

quoted

the purpose
Of adding a feature flag. Sessionless crypto operations in symmetric crypto is

being used without any

quoted

issue for a long
And nobody feel the need of that as of today. So my question is how

asymmetric crypto pmds are

quoted

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

quoted

an issue. It is
Already being handled in the rte_crypto_op by an enum which denote that the

'op' need to be

quoted

processed with some
Session or with xform.

quoted

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.

Agreed, if Asymmetric is not likely to have sessions, it should not call that API.

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.

Yes on the first enqueue, before actually submitting to the hardware, in the driver itself
Before sending the request to hardware and return 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 set it if it support sessionless or not.

I believe that would be simple and will not bother the data path or the busy hardware.
Anyways in case of sessions also in sym, we make session on arrival of 1st packet. That same logic
Can be applied here also. I don't think that will be an issue.

quoted

quoted

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

quoted

quoted

doc in case other sections need updating.

Yes this needs to be updated if the implementation is complete and we have

some PMD supporting

quoted

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?

I believe you should update the documentation when some PMD support sessionless.
I think we should hold back this patch, until you have a sample PMD/app ready for reference.

You may end up adding a few more updates to the lib while actually supporting the functionality.

quoted

quoted

 - fix comment in rte_crypto.h under STATUS_INVALID_SESSION
 - release note

-Akhil