From patchwork Wed Jan 17 06:35:05 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: "Gujjar, Abhinandan S" X-Patchwork-Id: 33899 X-Patchwork-Delegate: pablo.de.lara.guarch@intel.com Return-Path: X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 036A11B1B0; Wed, 17 Jan 2018 07:35:14 +0100 (CET) Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 40AA51B1A2 for ; Wed, 17 Jan 2018 07:35:10 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga002.jf.intel.com ([10.7.209.21]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 16 Jan 2018 22:35:10 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,371,1511856000"; d="scan'208";a="27100491" Received: from pgsmsx108.gar.corp.intel.com ([10.221.44.103]) by orsmga002.jf.intel.com with ESMTP; 16 Jan 2018 22:35:08 -0800 Received: from pgsmsx102.gar.corp.intel.com ([169.254.6.144]) by PGSMSX108.gar.corp.intel.com ([169.254.8.194]) with mapi id 14.03.0319.002; Wed, 17 Jan 2018 14:35:05 +0800 From: "Gujjar, Abhinandan S" To: Akhil Goyal , "Doherty, Declan" , "De Lara Guarch, Pablo" , "Jacob, Jerin" CC: "dev@dpdk.org" , "Vangati, Narender" , "Rao, Nikhil" Thread-Topic: [PATCH 1/2] lib/cryptodev: add support to set session private data Thread-Index: AQHTjfc6RWzdW7+9z0K/sXv6rM8+YKN0U+kAgAGhHnD//45JgIAAjwow//+COICAAJz3YP//g08AABUDxwD//4QtAP//d/uAgACZQ4D//lUYkA== Date: Wed, 17 Jan 2018 06:35:05 +0000 Message-ID: <5612CB344B05EE4F95FC5B729939F780706009F3@PGSMSX102.gar.corp.intel.com> References: <1516017078-166766-1-git-send-email-abhinandan.gujjar@intel.com> <5612CB344B05EE4F95FC5B729939F780705FED56@PGSMSX102.gar.corp.intel.com> <5612CB344B05EE4F95FC5B729939F780705FEDDD@PGSMSX102.gar.corp.intel.com> <5612CB344B05EE4F95FC5B729939F780705FEF35@PGSMSX102.gar.corp.intel.com> <5612CB344B05EE4F95FC5B729939F78070600131@PGSMSX102.gar.corp.intel.com> <0ed584fb-3524-0a52-44a4-a2b1759ecfff@nxp.com> <5612CB344B05EE4F95FC5B729939F7807060021A@PGSMSX102.gar.corp.intel.com> <1a5be561-5b9c-ae8d-e5a2-d2801973d125@nxp.com> In-Reply-To: <1a5be561-5b9c-ae8d-e5a2-d2801973d125@nxp.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ctpclassification: CTP_NT x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiMzcyOTEwMTItMjcyNC00OTU1LTk3NmEtNTdkZTUyMzU5Y2IwIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6IjJYSGppbngwcTBpTytyT1FOcGlqeVZydXNJU0Q4ajdWa0o4MmdyOXg3ejA9In0= dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [172.30.20.205] MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" Hi Akhil, > -----Original Message----- > From: Akhil Goyal [mailto:akhil.goyal@nxp.com] > Sent: Tuesday, January 16, 2018 6:32 PM > To: Gujjar, Abhinandan S ; Doherty, Declan > ; Jacob, Jerin > > Cc: dev@dpdk.org; Vangati, Narender ; Rao, > Nikhil > Subject: Re: [PATCH 1/2] lib/cryptodev: add support to set session private data > > On 1/16/2018 5:59 PM, Gujjar, Abhinandan S wrote: > > Hi Akhil, > > > >> -----Original Message----- > >> From: Akhil Goyal [mailto:akhil.goyal@nxp.com] > >> Sent: Tuesday, January 16, 2018 5:30 PM > >> To: Gujjar, Abhinandan S ; Doherty, > >> Declan ; Jacob, Jerin > >> > >> Cc: dev@dpdk.org; Vangati, Narender ; > >> Rao, Nikhil > >> 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@nxp.com] > >>>> Sent: Tuesday, January 16, 2018 2:52 PM > >>>> To: Gujjar, Abhinandan S ; Doherty, > >>>> Declan ; Jacob, Jerin > >>>> > >>>> Cc: dev@dpdk.org; Vangati, Narender ; > >>>> Rao, Nikhil > >>>> 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@nxp.com] > >>>>>> Sent: Tuesday, January 16, 2018 12:56 PM > >>>>>> To: Gujjar, Abhinandan S ; Doherty, > >>>>>> Declan ; Jacob, Jerin > >>>>>> > >>>>>> Cc: dev@dpdk.org; Vangati, Narender ; > >>>>>> Rao, Nikhil > >>>>>> 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@nxp.com] > >>>>>>>> Sent: Tuesday, January 16, 2018 11:55 AM > >>>>>>>> To: Gujjar, Abhinandan S ; > >>>>>>>> Doherty, Declan > >>>>>>>> Cc: dev@dpdk.org; Vangati, Narender > >>>>>>>> ; Rao, Nikhil > >>>>>>>> > >>>>>>>> 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. > > Got your point, we cannot validate in the library that private data is valid or not > as we do not know the values of the data. > > However, got one more option to work with. > You can have a priv_data_offset (similar to crypto_op) in the > rte_cryptodev_sym_session. In that way it will look similar in both the cases and > we do not have to make any assumption that the priv data is present after "sess > * nb_drivers ". > So in this way we can know if offset is zero, then data is not valid. > And procedure will also be same in both the cases. > If it is in crypto_op, then we set the priv_data_off in crypto_op, and if it is there > in session, then we set the priv_data_off in session. I guess, you are suggesting below changes: I am ok with this. Declan/Pablo, Is this ok? Do you see any impact on performance or anything else has to be considered? > > > > >> > >>> 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 > > Abhinandan diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h index 56958a6..057c39a 100644 --- a/lib/librte_cryptodev/rte_cryptodev.h +++ b/lib/librte_cryptodev/rte_cryptodev.h @@ -892,6 +892,8 @@ struct rte_cryptodev_data { /** Cryptodev symmetric crypto session */ struct rte_cryptodev_sym_session { + uint16_t private_data_offset; + /**< Private data offset */ __extension__ void *sess_private_data[0]; /**< Private session material */ };