[dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data

Gujjar, Abhinandan S abhinandan.gujjar at intel.com
Tue Jan 16 13:29:00 CET 2018


Hi Akhil,

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
> Sent: Tuesday, January 16, 2018 5:30 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar at intel.com>; Doherty, Declan
> <declan.doherty at intel.com>; Jacob, Jerin
> <Jerin.JacobKollanukkaran at cavium.com>
> Cc: dev at dpdk.org; Vangati, Narender <narender.vangati at intel.com>; Rao,
> Nikhil <nikhil.rao at intel.com>
> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session private data
> 
> Hi Abhinandan,
> On 1/16/2018 5:06 PM, Gujjar, Abhinandan S wrote:
> > Hi Akhil,
> >
> >> -----Original Message-----
> >> From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
> >> Sent: Tuesday, January 16, 2018 2:52 PM
> >> To: Gujjar, Abhinandan S <abhinandan.gujjar at intel.com>; Doherty,
> >> Declan <declan.doherty at intel.com>; Jacob, Jerin
> >> <Jerin.JacobKollanukkaran at cavium.com>
> >> Cc: dev at dpdk.org; Vangati, Narender <narender.vangati at intel.com>;
> >> Rao, Nikhil <nikhil.rao at intel.com>
> >> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
> >> private data
> >>
> >> On 1/16/2018 2:33 PM, Gujjar, Abhinandan S wrote:
> >>> Hi Akhil,
> >>>
> >>>> -----Original Message-----
> >>>> From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
> >>>> Sent: Tuesday, January 16, 2018 12:56 PM
> >>>> To: Gujjar, Abhinandan S <abhinandan.gujjar at intel.com>; Doherty,
> >>>> Declan <declan.doherty at intel.com>; Jacob, Jerin
> >>>> <Jerin.JacobKollanukkaran at cavium.com>
> >>>> Cc: dev at dpdk.org; Vangati, Narender <narender.vangati at intel.com>;
> >>>> Rao, Nikhil <nikhil.rao at intel.com>
> >>>> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session
> >>>> private data
> >>>>
> >>>> Hi Abhinandan,
> >>>> On 1/16/2018 12:35 PM, Gujjar, Abhinandan S wrote:
> >>>>> Hi Akhil,
> >>>>>
> >>>>>> -----Original Message-----
> >>>>>> From: Akhil Goyal [mailto:akhil.goyal at nxp.com]
> >>>>>> Sent: Tuesday, January 16, 2018 11:55 AM
> >>>>>> To: Gujjar, Abhinandan S <abhinandan.gujjar at intel.com>; Doherty,
> >>>>>> Declan <declan.doherty at intel.com>
> >>>>>> Cc: dev at dpdk.org; Vangati, Narender <narender.vangati at intel.com>;
> >>>>>> Rao, Nikhil <nikhil.rao at intel.com>
> >>>>>> Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set
> >>>>>> session private data
> >>>>>>
> >>>>>> Hi Abhinandan,
> >>>>>> On 1/16/2018 11:39 AM, Gujjar, Abhinandan S wrote:
> >>>>>>>>> diff --git a/lib/librte_cryptodev/rte_crypto.h
> >>>>>>>>> b/lib/librte_cryptodev/rte_crypto.h
> >>>>>>>>> index bbc510d..3a98cbf 100644
> >>>>>>>>> --- a/lib/librte_cryptodev/rte_crypto.h
> >>>>>>>>> +++ b/lib/librte_cryptodev/rte_crypto.h
> >>>>>>>>> @@ -62,6 +62,18 @@ enum rte_crypto_op_sess_type {
> >>>>>>>>>       	RTE_CRYPTO_OP_SECURITY_SESSION	/**< Security session
> >>>> crypto
> >>>>>>>> operation */
> >>>>>>>>>       };
> >>>>>>>>>
> >>>>>>>>> +/** Private data types for cryptographic operation
> >>>>>>>>> + * @see rte_crypto_op::private_data_type */ enum
> >>>>>>>>> +rte_crypto_op_private_data_type {
> >>>>>>>>> +	RTE_CRYPTO_OP_PRIVATE_DATA_NONE,
> >>>>>>>>> +	/**< No private data */
> >>>>>>>>> +	RTE_CRYPTO_OP_PRIVATE_DATA_OP,
> >>>>>>>>> +	/**< Private data is part of rte_crypto_op and indicated by
> >>>>>>>>> +	 * private_data_offset */
> >>>>>>>>> +	RTE_CRYPTO_OP_PRIVATE_DATA_SESSION
> >>>>>>>>> +	/**< Private data is available at session */ };
> >>>>>>>>> +
> >>>>>>>> We may get away with this enum. If private_data_offset is "0",
> >>>>>>>> then we can check with the session if it has priv data or not.
> >>>>>>> Right now,  Application uses 'rte_crypto_op_private_data_type'
> >>>>>>> to indicate rte_cryptodev_sym_session_set_private_data()
> >>>>>>> was called to set the private data. Otherwise, how do you
> >>>>>>> indicate there is a
> >>>>>> private data associated with the session?
> >>>>>>> Any suggestions?
> >>>>>> For session based flows, the first choice to store the private
> >>>>>> data should be in the session. So RTE_CRYPTO_OP_WITH_SESSION or
> >>>>>> RTE_CRYPTO_OP_SECURITY_SESSION can be used to call
> >>>>>> rte_cryptodev_sym_session_set_private_data or
> >>>>>> rte_security_session_set_private_data.
> >>>>> Case 1: private_data_offset is "0" and sess_type =
> >>>>> RTE_CRYPTO_OP_WITH_SESSION -> usual case Case 2:
> >>>>> private_data_offset is "0" and sess_type =
> >>>>> RTE_CRYPTO_OP_WITH_SESSION + event case (access private data)
> >>>>> Differentiating between case 1 & 2 will be done by checking
> >>>> rte_crypto_op_private_data_type ==
> >>>> RTE_CRYPTO_OP_PRIVATE_DATA_SESSION.
> >>>>
> >>>> Consider this:
> >>>> if (sess_type == RTE_CRYPTO_OP_WITH_SESSION &&
> >>>> 		rte_cryptodev_sym_session_get_private_data == NULL)
> >>>> 	usual case.
> >>>> else if (sess_type = RTE_CRYPTO_OP_WITH_SESSION &&
> >>>> 		rte_cryptodev_sym_session_get_private_data != NULL)
> >>>> 	event case.
> >>>> else if (sess_type == RTE_CRYPTO_OP_SESSIONLESS &&
> >>>> 		private_data_offset != 0)
> >>>> 	event case for sessionless op.
> >>>>
> >>>> I hope all cases can be handled in this way.
> >>> Memory allocated for private data will be continuation of session memory.
> >>> I think, rte_cryptodev_sym_session_get_private_data() will return a
> >>> valid
> >> pointer.
> >>> It could be pointer to private data, in case application has
> >>> allocated mempool
> >> with session + private data.
> >>> It could be again a pointer to a location(may be next session),  in
> >>> case
> >> application has allocated mempool with session only.
> >>> Unless, there is a flag in the session data which will be set by
> >>> rte_cryptodev_sym_session_set_private_data()
> >>> If this flag is not set,
> >>> rte_cryptodev_sym_session_get_private_data() will
> >> return NULL.
> >>> I am not claiming, I have complete knowledge of different usage case
> >>> of
> >> mempool setup for crypto.
> >>> I am wondering, whether I am missing anything here. Please let me know.
> >>
> >> It depends on the implementation of the get/set API. We can set NULL,
> >> if it is not filled in the set API. If it is set then we have a valid pointer.
> > The plan is to store private data after "sess * nb_drivers ".
> > As you said, if it is implementation specific, flag may be again
> > required at struct rte_cryptodev_sym_session
> I think my previous statement was not clear.
> My point is that whatever we set in the
> rte_cryptodev_sym_session_set_private_data() is a valid value when we call this
> API explicitly. And before calling the set API, the values are zero or any invalid
> value. So if application calls the get API before setting it with set API, it will get
> an invalid value(may be NULL or zero or whatever).
Thanks for clarifying. At this time, without calling set API and calling get API will get some value.
How do you validating whether the data is valid or not?
Since application has called set API and same is indicated by private_data_type flag,
I thought data got by get API can be safely assume to be valid.
Not sure, if you have better way to validate the data from get API.

> 
> > OR
> > If it is planned to store at PMD's sess_private_data, it requires additional ops
> > as well in rte_cryptodev_ops.
> > We wanted to have a simple design with minimal changes to cryptodev and
> security,
> > that’s reason for existing design.
> > It will be good, if other folks chime in and share there opinion.
> > This will make the implementation part more clear.
> >>
> >> -Akhil
> > -Abhinandan
> >
Abhinandan


More information about the dev mailing list