DPDK-dev Archive on lore.kernel.org
 help / color / Atom feed
* [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
@ 2019-06-03 19:44 Arek Kusztal
  2019-06-05 12:16 ` [dpdk-dev] [EXT] " Shally Verma
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Arek Kusztal @ 2019-06-03 19:44 UTC (permalink / raw)
  To: dev; +Cc: akhil.goyal, fiona.trahe, shally.verma, Arek Kusztal

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

Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
---
 lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/lib/librte_cryptodev/rte_crypto_asym.h b/lib/librte_cryptodev/rte_crypto_asym.h
index 8672f21..5d69692 100644
--- a/lib/librte_cryptodev/rte_crypto_asym.h
+++ b/lib/librte_cryptodev/rte_crypto_asym.h
@@ -503,6 +503,8 @@ struct rte_crypto_dsa_op_param {
 struct rte_crypto_asym_op {
 	struct rte_cryptodev_asym_session *session;
 	/**< Handle for the initialised session context */
+	struct rte_crypto_asym_xform *xform;
+	/**< Session-less API crypto operation parameters */
 
 	__extension__
 	union {
-- 
2.7.4


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-03 19:44 [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless Arek Kusztal
@ 2019-06-05 12:16 ` " Shally Verma
  2019-06-14 10:21   ` Kusztal, ArkadiuszX
  2019-06-14 10:24   ` Kusztal, ArkadiuszX
  2019-06-20 14:17 ` [dpdk-dev] " Akhil Goyal
  2019-08-12  9:26 ` [dpdk-dev] [EXT] " Anoob Joseph
  2 siblings, 2 replies; 17+ messages in thread
From: Shally Verma @ 2019-06-05 12:16 UTC (permalink / raw)
  To: Arek Kusztal, dev; +Cc: akhil.goyal, fiona.trahe, shally.verma



> -----Original Message-----
> From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> Sent: Tuesday, June 4, 2019 1:14 AM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com;
> shally.verma@caviumnetworks.com; Arek Kusztal
> <arkadiuszx.kusztal@intel.com>
> Subject: [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by
> sessionless
> 
> External Email
> 
> ----------------------------------------------------------------------
> Asymmetric cryptography algorithms may more likely use sessionless API so
> there is need to extend API.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> b/lib/librte_cryptodev/rte_crypto_asym.h
> index 8672f21..5d69692 100644
> --- a/lib/librte_cryptodev/rte_crypto_asym.h
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> @@ -503,6 +503,8 @@ struct rte_crypto_dsa_op_param {  struct
> rte_crypto_asym_op {
>  	struct rte_cryptodev_asym_session *session;
>  	/**< Handle for the initialised session context */
> +	struct rte_crypto_asym_xform *xform;
> +	/**< Session-less API crypto operation parameters */

[Shally] Ack to this change. But is this all that is needed to support sessionless? Do you have working poc with sessionless?

Thanks
Shally

> 
>  	__extension__
>  	union {
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-05 12:16 ` [dpdk-dev] [EXT] " Shally Verma
@ 2019-06-14 10:21   ` Kusztal, ArkadiuszX
  2019-06-14 10:24   ` Kusztal, ArkadiuszX
  1 sibling, 0 replies; 17+ messages in thread
From: Kusztal, ArkadiuszX @ 2019-06-14 10:21 UTC (permalink / raw)
  To: Shally Verma, dev; +Cc: akhil.goyal, Trahe, Fiona, shally.verma

Hi Shally,

Thanks for your feedback.

> -----Original Message-----
> From: Shally Verma [mailto:shallyv@marvell.com]
> Sent: Wednesday, June 5, 2019 2:17 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>;
> shally.verma@caviumnetworks.com
> Subject: RE: [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by
> sessionless
> 
> 
> 
> > -----Original Message-----
> > From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > Sent: Tuesday, June 4, 2019 1:14 AM
> > To: dev@dpdk.org
> > Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com;
> > shally.verma@caviumnetworks.com; Arek Kusztal
> > <arkadiuszx.kusztal@intel.com>
> > Subject: [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by
> > sessionless
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> > Asymmetric cryptography algorithms may more likely use sessionless API
> > so there is need to extend API.
> >
> > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > ---
> >  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > b/lib/librte_cryptodev/rte_crypto_asym.h
> > index 8672f21..5d69692 100644
> > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > @@ -503,6 +503,8 @@ struct rte_crypto_dsa_op_param {  struct
> > rte_crypto_asym_op {
> >  	struct rte_cryptodev_asym_session *session;
> >  	/**< Handle for the initialised session context */
> > +	struct rte_crypto_asym_xform *xform;
> > +	/**< Session-less API crypto operation parameters */
> 
> [Shally] Ack to this change. But is this all that is needed to support
> sessionless? Do you have working poc with sessionless?
> 

[AK]
xform holds to get working. Crypto_op holds sess_type
From our side for now we not intend to store any user information in session at all.
For sure not private keys, any other information is small enough comparing to asymmetric crypto computation time that it has no gain to allocate session for it.


> Thanks
> Shally
> 
> >
> >  	__extension__
> >  	union {
> > --
> > 2.7.4


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-05 12:16 ` [dpdk-dev] [EXT] " Shally Verma
  2019-06-14 10:21   ` Kusztal, ArkadiuszX
@ 2019-06-14 10:24   ` Kusztal, ArkadiuszX
  2019-06-14 11:23     ` Shally Verma
  1 sibling, 1 reply; 17+ messages in thread
From: Kusztal, ArkadiuszX @ 2019-06-14 10:24 UTC (permalink / raw)
  To: Shally Verma, dev; +Cc: akhil.goyal, Trahe, Fiona, shally.verma



> -----Original Message-----
> From: Kusztal, ArkadiuszX
> Sent: Friday, June 14, 2019 12:21 PM
> To: 'Shally Verma' <shallyv@marvell.com>; dev@dpdk.org
> Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>;
> shally.verma@caviumnetworks.com
> Subject: RE: [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by
> sessionless
> 
> Hi Shally,
> 
> Thanks for your feedback.
> 
> > -----Original Message-----
> > From: Shally Verma [mailto:shallyv@marvell.com]
> > Sent: Wednesday, June 5, 2019 2:17 PM
> > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> > Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>;
> > shally.verma@caviumnetworks.com
> > Subject: RE: [EXT] [PATCH] cryptodev: extend api of asymmetric crypto
> > by sessionless
> >
> >
> >
> > > -----Original Message-----
> > > From: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > Sent: Tuesday, June 4, 2019 1:14 AM
> > > To: dev@dpdk.org
> > > Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com;
> > > shally.verma@caviumnetworks.com; Arek Kusztal
> > > <arkadiuszx.kusztal@intel.com>
> > > Subject: [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by
> > > sessionless
> > >
> > > External Email
> > >
> > > --------------------------------------------------------------------
> > > -- Asymmetric cryptography algorithms may more likely use
> > > sessionless API so there is need to extend API.
> > >
> > > Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> > > ---
> > >  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
> > >  1 file changed, 2 insertions(+)
> > >
> > > diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> > > b/lib/librte_cryptodev/rte_crypto_asym.h
> > > index 8672f21..5d69692 100644
> > > --- a/lib/librte_cryptodev/rte_crypto_asym.h
> > > +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> > > @@ -503,6 +503,8 @@ struct rte_crypto_dsa_op_param {  struct
> > > rte_crypto_asym_op {
> > >  	struct rte_cryptodev_asym_session *session;
> > >  	/**< Handle for the initialised session context */
> > > +	struct rte_crypto_asym_xform *xform;
> > > +	/**< Session-less API crypto operation parameters */
> >
> > [Shally] Ack to this change. But is this all that is needed to support
> > sessionless? Do you have working poc with sessionless?
> >
> 
> [AK]
> xform holds to get working. Crypto_op holds sess_type From our side for
> now we not intend to store any user information in session at all.
> For sure not private keys, any other information is small enough comparing
> to asymmetric crypto computation time that it has no gain to allocate session
> for it.
> 
[AK] Sorry, I had to fix bad writing.
rte_crypto_asym_xform holds enough information, and rte_crypto_op holds sess_type.
> 
> > Thanks
> > Shally
> >
> > >
> > >  	__extension__
> > >  	union {
> > > --
> > > 2.7.4


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-14 10:24   ` Kusztal, ArkadiuszX
@ 2019-06-14 11:23     ` Shally Verma
  2019-06-14 12:12       ` Kusztal, ArkadiuszX
  0 siblings, 1 reply; 17+ messages in thread
From: Shally Verma @ 2019-06-14 11:23 UTC (permalink / raw)
  To: Kusztal, ArkadiuszX, dev; +Cc: akhil.goyal, Trahe, Fiona, shally.verma



> -----Original Message-----
> From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> Sent: Friday, June 14, 2019 3:55 PM
> To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org
> Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>;
> shally.verma@caviumnetworks.com
> Subject: RE: [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by
> sessionless
> 
...
> > > [Shally] Ack to this change. But is this all that is needed to
> > > support sessionless? Do you have working poc with sessionless?
> > >
> >
> > [AK]
> > xform holds to get working. Crypto_op holds sess_type From our side
> > for now we not intend to store any user information in session at all.
> > For sure not private keys, any other information is small enough
> > comparing to asymmetric crypto computation time that it has no gain to
> > allocate session for it.
> >
> [AK] Sorry, I had to fix bad writing.
> rte_crypto_asym_xform holds enough information, and rte_crypto_op holds
> sess_type.

Can you submit example app working on sessionless ?

> >
> > > Thanks
> > > Shally
> > >
> > > >
> > > >  	__extension__
> > > >  	union {
> > > > --
> > > > 2.7.4


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-14 11:23     ` Shally Verma
@ 2019-06-14 12:12       ` Kusztal, ArkadiuszX
  0 siblings, 0 replies; 17+ messages in thread
From: Kusztal, ArkadiuszX @ 2019-06-14 12:12 UTC (permalink / raw)
  To: Shally Verma, dev; +Cc: akhil.goyal, Trahe, Fiona, shally.verma



> -----Original Message-----
> From: Shally Verma [mailto:shallyv@marvell.com]
> Sent: Friday, June 14, 2019 1:23 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>;
> shally.verma@caviumnetworks.com
> Subject: RE: [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by
> sessionless
> 
> 
> 
> > -----Original Message-----
> > From: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>
> > Sent: Friday, June 14, 2019 3:55 PM
> > To: Shally Verma <shallyv@marvell.com>; dev@dpdk.org
> > Cc: akhil.goyal@nxp.com; Trahe, Fiona <fiona.trahe@intel.com>;
> > shally.verma@caviumnetworks.com
> > Subject: RE: [EXT] [PATCH] cryptodev: extend api of asymmetric crypto
> > by sessionless
> >
> ...
> > > > [Shally] Ack to this change. But is this all that is needed to
> > > > support sessionless? Do you have working poc with sessionless?
> > > >
> > >
> > > [AK]
> > > xform holds to get working. Crypto_op holds sess_type From our side
> > > for now we not intend to store any user information in session at all.
> > > For sure not private keys, any other information is small enough
> > > comparing to asymmetric crypto computation time that it has no gain
> > > to allocate session for it.
> > >
> > [AK] Sorry, I had to fix bad writing.
> > rte_crypto_asym_xform holds enough information, and rte_crypto_op
> > holds sess_type.
> 
> Can you submit example app working on sessionless ?

[AK] Sure, I will.
> 
> > >
> > > > Thanks
> > > > Shally
> > > >
> > > > >
> > > > >  	__extension__
> > > > >  	union {
> > > > > --
> > > > > 2.7.4


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-03 19:44 [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless Arek Kusztal
  2019-06-05 12:16 ` [dpdk-dev] [EXT] " Shally Verma
@ 2019-06-20 14:17 ` " Akhil Goyal
  2019-06-21 11:58   ` Trahe, Fiona
  2019-08-12  9:26 ` [dpdk-dev] [EXT] " Anoob Joseph
  2 siblings, 1 reply; 17+ messages in thread
From: Akhil Goyal @ 2019-06-20 14:17 UTC (permalink / raw)
  To: Arek Kusztal, dev; +Cc: fiona.trahe, shally.verma


> 
> Asymmetric cryptography algorithms may more likely use
> sessionless API so there is need to extend API.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-20 14:17 ` [dpdk-dev] " Akhil Goyal
@ 2019-06-21 11:58   ` Trahe, Fiona
  2019-06-21 12:22     ` Akhil Goyal
  0 siblings, 1 reply; 17+ messages in thread
From: Trahe, Fiona @ 2019-06-21 11:58 UTC (permalink / raw)
  To: Akhil Goyal, Kusztal, ArkadiuszX, dev, shally.verma; +Cc: Trahe, Fiona

Hi Akhil, Arek, Shally,

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Thursday, June 20, 2019 3:17 PM
> To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>; shally.verma@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@intel.com>
> > ---
> Acked-by: Akhil Goyal <akhil.goyal@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.
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.
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.
 - fix comment in rte_crypto.h under STATUS_INVALID_SESSION
 - release note




^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-21 11:58   ` Trahe, Fiona
@ 2019-06-21 12:22     ` Akhil Goyal
  2019-06-21 14:18       ` Trahe, Fiona
  0 siblings, 1 reply; 17+ messages in thread
From: Akhil Goyal @ 2019-06-21 12:22 UTC (permalink / raw)
  To: Trahe, Fiona, Kusztal, ArkadiuszX, dev, shally.verma

Hi Fiona,

> 
> Hi Akhil, Arek, Shally,
> 
> > -----Original Message-----
> > From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> > Sent: Thursday, June 20, 2019 3:17 PM
> > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> > Cc: Trahe, Fiona <fiona.trahe@intel.com>;
> shally.verma@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@intel.com>
> > > ---
> > Acked-by: Akhil Goyal <akhil.goyal@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.
> 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.

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


-Akhil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-21 12:22     ` Akhil Goyal
@ 2019-06-21 14:18       ` Trahe, Fiona
  2019-06-24  7:05         ` Akhil Goyal
  0 siblings, 1 reply; 17+ messages in thread
From: Trahe, Fiona @ 2019-06-21 14:18 UTC (permalink / raw)
  To: Akhil Goyal, Kusztal, ArkadiuszX, dev, shally.verma; +Cc: Trahe, Fiona

Hi Akhil,

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-21 14:18       ` Trahe, Fiona
@ 2019-06-24  7:05         ` Akhil Goyal
  2019-06-27 11:54           ` Trahe, Fiona
  0 siblings, 1 reply; 17+ messages in thread
From: Akhil Goyal @ 2019-06-24  7:05 UTC (permalink / raw)
  To: Trahe, Fiona, Kusztal, ArkadiuszX, dev, Shally Verma

Hi Fiona

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

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.

> 
> 
> > > 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?

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.

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-24  7:05         ` Akhil Goyal
@ 2019-06-27 11:54           ` Trahe, Fiona
  2019-06-28 16:10             ` Shally Verma
  0 siblings, 1 reply; 17+ messages in thread
From: Trahe, Fiona @ 2019-06-27 11:54 UTC (permalink / raw)
  To: Akhil Goyal, Kusztal, ArkadiuszX, dev, Shally Verma; +Cc: Trahe, Fiona

Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Monday, June 24, 2019 8:05 AM
> To: Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>;
> dev@dpdk.org; Shally Verma <shallyv@marvell.com>
> Subject: RE: [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
> 
> Hi Fiona
> 
> >
> > Hi Akhil,
> >
> > > -----Original Message-----
> > > From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> > > Sent: Friday, June 21, 2019 1:23 PM
> > > To: Trahe, Fiona <fiona.trahe@intel.com>; Kusztal, ArkadiuszX
> > <arkadiuszx.kusztal@intel.com>;
> > > dev@dpdk.org; shally.verma@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@nxp.com]
> > > > > Sent: Thursday, June 20, 2019 3:17 PM
> > > > > To: Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> > > > > Cc: Trahe, Fiona <fiona.trahe@intel.com>;
> > > > shally.verma@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@intel.com>
> > > > > > ---
> > > > > Acked-by: Akhil Goyal <akhil.goyal@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.
> 
> 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.
[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, applications 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 path, 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 the API:

For asymmetric crypto operations, if an op fails to enqueue, the op.status 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 enqueued 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.

Note - it's out of the scope of this thread whether the same should be true for symmetric crypto ops.
> 
> >
> >
> > > > 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?
> 
> 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.
[Fiona] ok

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

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-27 11:54           ` Trahe, Fiona
@ 2019-06-28 16:10             ` Shally Verma
  2019-06-28 17:27               ` Trahe, Fiona
  0 siblings, 1 reply; 17+ messages in thread
From: Shally Verma @ 2019-06-28 16:10 UTC (permalink / raw)
  To: Trahe, Fiona, Akhil Goyal, Kusztal, ArkadiuszX, dev

Hi Finoa, Akhil

> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: Thursday, June 27, 2019 5:25 PM
> To: Akhil Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>; dev@dpdk.org; Shally Verma
> <shallyv@marvell.com>
> Cc: Trahe, Fiona <fiona.trahe@intel.com>
> Subject: [EXT] RE: [PATCH] cryptodev: extend api of asymmetric crypto by
> sessionless
> 
> External Email
> 
> ----------------------------------------------------------------------
...

> > > > > 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.
[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.


> >
> > > 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.
> [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, applications
> 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 path, 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 the API:
> 
> For asymmetric crypto operations, if an op fails to enqueue, the op.status
> 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 enqueued 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.
> 

[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 INVALID_OP_SESSION, or NOT_SUPPORTED does not give
clear reason for failure. Assuming we agree on feature flag, then next question comes if PMD set SESSIONLESS feature flag, then does that mean it support *only* sessionless 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

Does this sounds okay?

Thanks
Shally

> Note - it's out of the scope of this thread whether the same should be true
> for symmetric crypto ops.
> >
> > >
> > >
> > > > > 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?
> >
> > 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.
> [Fiona] ok
> 
> >
> > >
> > > >
> > > > >  - fix comment in rte_crypto.h under STATUS_INVALID_SESSION
> > > > >  - release note
> > > > >
> > > > >
> > > >
> > > >
> > > > -Akhil

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-28 16:10             ` Shally Verma
@ 2019-06-28 17:27               ` Trahe, Fiona
  2019-06-30  8:37                 ` Shally Verma
  0 siblings, 1 reply; 17+ messages in thread
From: Trahe, Fiona @ 2019-06-28 17:27 UTC (permalink / raw)
  To: Shally Verma, Akhil Goyal, Kusztal, ArkadiuszX, dev; +Cc: Trahe, Fiona

Hi Shally,

> -----Original Message-----
> From: Shally Verma [mailto:shallyv@marvell.com]
> Sent: Friday, June 28, 2019 5:11 PM
> To: Trahe, Fiona <fiona.trahe@intel.com>; Akhil Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Subject: RE: [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
> 
> Hi Finoa, Akhil
> 
> > -----Original Message-----
> > From: Trahe, Fiona <fiona.trahe@intel.com>
> > Sent: Thursday, June 27, 2019 5:25 PM
> > To: Akhil Goyal <akhil.goyal@nxp.com>; Kusztal, ArkadiuszX
> > <arkadiuszx.kusztal@intel.com>; dev@dpdk.org; Shally Verma
> > <shallyv@marvell.com>
> > Cc: Trahe, Fiona <fiona.trahe@intel.com>
> > Subject: [EXT] RE: [PATCH] cryptodev: extend api of asymmetric crypto by
> > sessionless
> >
> > External Email
> >
> > ----------------------------------------------------------------------
> ...
> 
> > > > > > 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.
> [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.
> 
> 
> > >
> > > > 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.
> > [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, applications
> > 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 path, 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 the API:
> >
> > For asymmetric crypto operations, if an op fails to enqueue, the op.status
> > 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 enqueued 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.
> >
> 
> [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 INVALID_OP_SESSION, or
> NOT_SUPPORTED does not give
> clear reason for failure. Assuming we agree on feature flag, then next question comes if PMD set
> SESSIONLESS feature flag, then does that mean it support *only* sessionless 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
> 
> Does this sounds okay?
> 
> 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 not. 
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-28 17:27               ` Trahe, Fiona
@ 2019-06-30  8:37                 ` Shally Verma
  0 siblings, 0 replies; 17+ messages in thread
From: Shally Verma @ 2019-06-30  8:37 UTC (permalink / raw)
  To: Trahe, Fiona, Akhil Goyal, Kusztal, ArkadiuszX, dev



> -----Original Message-----
> From: Trahe, Fiona <fiona.trahe@intel.com>
> Sent: Friday, June 28, 2019 10:58 PM
> To: Shally Verma <shallyv@marvell.com>; Akhil Goyal <akhil.goyal@nxp.com>;
> Kusztal, ArkadiuszX <arkadiuszx.kusztal@intel.com>; dev@dpdk.org
> Cc: Trahe, Fiona <fiona.trahe@intel.com>
> Subject: [EXT] RE: [PATCH] cryptodev: extend api of asymmetric crypto by
> sessionless
> 
> External Email
> 
> ----------------------------------------------------------------------
> Hi Shally,
....

> > >
> >
> > [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
> > INVALID_OP_SESSION, or NOT_SUPPORTED does not give clear reason for
> > failure. Assuming we agree on feature flag, then next question comes
> > if PMD set SESSIONLESS feature flag, then does that mean it support
> > *only* sessionless 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
> >
> > Does this sounds okay?
> >
> > 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 not.
>
That sounds perfect. So, we will have v2 for this? 

Thanks
Shally
 

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-06-03 19:44 [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless Arek Kusztal
  2019-06-05 12:16 ` [dpdk-dev] [EXT] " Shally Verma
  2019-06-20 14:17 ` [dpdk-dev] " Akhil Goyal
@ 2019-08-12  9:26 ` " Anoob Joseph
  2019-08-12 10:00   ` Kusztal, ArkadiuszX
  2 siblings, 1 reply; 17+ messages in thread
From: Anoob Joseph @ 2019-08-12  9:26 UTC (permalink / raw)
  To: Arek Kusztal, dev
  Cc: akhil.goyal, fiona.trahe, Shally Verma, Sunila Sahu,
	Narayana Prasad Raju Athreya

Hi Arek,

Please see inline.

Thanks,
Anoob

> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Arek Kusztal
> Sent: Tuesday, June 4, 2019 1:14 AM
> To: dev@dpdk.org
> Cc: akhil.goyal@nxp.com; fiona.trahe@intel.com;
> shally.verma@caviumnetworks.com; Arek Kusztal
> <arkadiuszx.kusztal@intel.com>
> Subject: [EXT] [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric
> crypto by sessionless
> 
> External Email
> 
> ----------------------------------------------------------------------
> Asymmetric cryptography algorithms may more likely use sessionless API so
> there is need to extend API.
> 
> Signed-off-by: Arek Kusztal <arkadiuszx.kusztal@intel.com>
> ---
>  lib/librte_cryptodev/rte_crypto_asym.h | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto_asym.h
> b/lib/librte_cryptodev/rte_crypto_asym.h
> index 8672f21..5d69692 100644
> --- a/lib/librte_cryptodev/rte_crypto_asym.h
> +++ b/lib/librte_cryptodev/rte_crypto_asym.h
> @@ -503,6 +503,8 @@ struct rte_crypto_dsa_op_param {  struct
> rte_crypto_asym_op {
>  	struct rte_cryptodev_asym_session *session;
>  	/**< Handle for the initialised session context */
> +	struct rte_crypto_asym_xform *xform;
> +	/**< Session-less API crypto operation parameters */

[Anoob] Shouldn't we make this a union? In symmetric mode, it is done that way and it makes sense also.

Something like,

       RTE_STD_C11
       union {
               struct rte_cryptodev_asym_session *session;
               /**< Handle for the initialised session context */
               struct rte_crypto_asym_xform *xform;
               /**< Session-less API crypto operation parameters */
       };
 
> 
>  	__extension__
>  	union {
> --
> 2.7.4


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: [dpdk-dev] [EXT] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless
  2019-08-12  9:26 ` [dpdk-dev] [EXT] " Anoob Joseph
@ 2019-08-12 10:00   ` Kusztal, ArkadiuszX
  0 siblings, 0 replies; 17+ messages in thread
From: Kusztal, ArkadiuszX @ 2019-08-12 10:00 UTC (permalink / raw)
  To: Anoob Joseph, dev
  Cc: akhil.goyal, Trahe, Fiona, Shally Verma, Sunila Sahu,
	Narayana Prasad Raju Athreya

Hi Anoob,
 
> [Anoob] Shouldn't we make this a union? In symmetric mode, it is done that
> way and it makes sense also.
> 
> Something like,
> 
>        RTE_STD_C11
>        union {
>                struct rte_cryptodev_asym_session *session;
>                /**< Handle for the initialised session context */
>                struct rte_crypto_asym_xform *xform;
>                /**< Session-less API crypto operation parameters */
>        };
[AK] - Agreed. I will send updated version.
> 
> >
> >  	__extension__
> >  	union {
> > --
> > 2.7.4


^ permalink raw reply	[flat|nested] 17+ messages in thread

end of thread, back to index

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-03 19:44 [dpdk-dev] [PATCH] cryptodev: extend api of asymmetric crypto by sessionless Arek Kusztal
2019-06-05 12:16 ` [dpdk-dev] [EXT] " Shally Verma
2019-06-14 10:21   ` Kusztal, ArkadiuszX
2019-06-14 10:24   ` Kusztal, ArkadiuszX
2019-06-14 11:23     ` Shally Verma
2019-06-14 12:12       ` Kusztal, ArkadiuszX
2019-06-20 14:17 ` [dpdk-dev] " Akhil Goyal
2019-06-21 11:58   ` Trahe, Fiona
2019-06-21 12:22     ` Akhil Goyal
2019-06-21 14:18       ` Trahe, Fiona
2019-06-24  7:05         ` Akhil Goyal
2019-06-27 11:54           ` Trahe, Fiona
2019-06-28 16:10             ` Shally Verma
2019-06-28 17:27               ` Trahe, Fiona
2019-06-30  8:37                 ` Shally Verma
2019-08-12  9:26 ` [dpdk-dev] [EXT] " Anoob Joseph
2019-08-12 10:00   ` Kusztal, ArkadiuszX

DPDK-dev Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/dpdk-dev/0 dpdk-dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dpdk-dev dpdk-dev/ https://lore.kernel.org/dpdk-dev \
		dev@dpdk.org dpdk-dev@archiver.kernel.org
	public-inbox-index dpdk-dev

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/ public-inbox