From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by dpdk.org (Postfix) with ESMTP id 0C4F81B784 for ; Wed, 31 Jan 2018 14:41:01 +0100 (CET) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga105.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 31 Jan 2018 05:41:01 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.46,440,1511856000"; d="scan'208";a="15857021" Received: from irsmsx106.ger.corp.intel.com ([163.33.3.31]) by fmsmga002.fm.intel.com with ESMTP; 31 Jan 2018 05:41:00 -0800 Received: from irsmsx108.ger.corp.intel.com ([169.254.11.9]) by IRSMSX106.ger.corp.intel.com ([169.254.8.62]) with mapi id 14.03.0319.002; Wed, 31 Jan 2018 13:40:39 +0000 From: "De Lara Guarch, Pablo" To: "Gujjar, Abhinandan S" , "Doherty, Declan" , "akhil.goyal@nxp.com" , "Jerin.JacobKollanukkaran@cavium.com" CC: "dev@dpdk.org" , "Vangati, Narender" , "Rao, Nikhil" Thread-Topic: [RFC v2, 1/2] cryptodev: add support to set session private data Thread-Index: AQHTlCfSfwsKZqjS8U+rsKI3MTSvZKODUHbwgAFsJACACUylQA== Date: Wed, 31 Jan 2018 13:40:39 +0000 Message-ID: References: <1516697659-44375-1-git-send-email-abhinandan.gujjar@intel.com> <5612CB344B05EE4F95FC5B729939F780706043AC@PGSMSX102.gar.corp.intel.com> In-Reply-To: <5612CB344B05EE4F95FC5B729939F780706043AC@PGSMSX102.gar.corp.intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiNWIwZTQ0ZmEtNTIwMi00NDA0LTg4YjAtNGU4MDU1YTIxYmIzIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6InlFcnBOZ0xPUzJKcGtLeDJNTDVUSzVCQkVqSG9TZURRTFFXMWlGcW53eWc9In0= x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [RFC v2, 1/2] 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: , X-List-Received-Date: Wed, 31 Jan 2018 13:41:02 -0000 Hi Abhinandan, > -----Original Message----- > From: Gujjar, Abhinandan S > Sent: Thursday, January 25, 2018 3:38 PM > To: De Lara Guarch, Pablo ; Doherty, > Declan ; akhil.goyal@nxp.com; > Jerin.JacobKollanukkaran@cavium.com > Cc: dev@dpdk.org; Vangati, Narender ; Rao, > Nikhil > Subject: RE: [RFC v2, 1/2] cryptodev: add support to set session private > data >=20 > Hi Pablo & Declan, >=20 > > -----Original Message----- > > From: De Lara Guarch, Pablo > > Sent: Thursday, January 25, 2018 1:17 AM > > To: Gujjar, Abhinandan S ; Doherty, > > Declan ; akhil.goyal@nxp.com; > > Jerin.JacobKollanukkaran@cavium.com > > Cc: dev@dpdk.org; Vangati, Narender ; > Rao, > > Nikhil > > Subject: RE: [RFC v2, 1/2] cryptodev: add support to set session > > private data > > > > > > > > > -----Original Message----- > > > From: Gujjar, Abhinandan S > > > Sent: Tuesday, January 23, 2018 8:54 AM > > > To: Doherty, Declan ; > akhil.goyal@nxp.com; > > > De Lara Guarch, Pablo ; > > > Jerin.JacobKollanukkaran@cavium.com > > > Cc: dev@dpdk.org; Vangati, Narender ; > > > Gujjar, Abhinandan S ; Rao, Nikhil > > > > > > Subject: [RFC v2, 1/2] cryptodev: add support to set session private > > > data > > > > > > Update rte_crypto_op to indicate private data offset. > > > > > > The application may want to store private data along with the > > > rte_cryptodev that is transparent to the rte_cryptodev layer. > > > For e.g., If an eventdev based application is submitting a > > > rte_cryptodev_sym_session operation and wants to indicate event > > > information required to construct a new event that will be enqueued > > > to eventdev after completion of the rte_cryptodev_sym_session > operation. > > > This patch provides a mechanism for the application to associate > > > this information with the rte_cryptodev_sym_session session. > > > The application can set the private data using > > > rte_cryptodev_sym_session_set_private_data() and retrieve it using > > > rte_cryptodev_sym_session_get_private_data(). > > > > Hi Abhinandan, > > > > > > > > Signed-off-by: Abhinandan Gujjar > > > Signed-off-by: Nikhil Rao > > > --- > > > Notes: > > > V2: > > > 1. Removed enum rte_crypto_op_private_data_type > > > 2. Corrected formatting > > > > > > lib/librte_cryptodev/rte_crypto.h | 8 ++++++-- > > > lib/librte_cryptodev/rte_cryptodev.h | 32 > > > ++++++++++++++++++++++++++++++++ > > > 2 files changed, 38 insertions(+), 2 deletions(-) > > > > > > diff --git a/lib/librte_cryptodev/rte_crypto.h > > > b/lib/librte_cryptodev/rte_crypto.h > > > index 95cf861..14c87c8 100644 > > > --- a/lib/librte_cryptodev/rte_crypto.h > > > +++ b/lib/librte_cryptodev/rte_crypto.h > > > @@ -84,8 +84,12 @@ struct rte_crypto_op { > > > */ > > > uint8_t sess_type; > > > /**< operation session type */ > > > - > > > - uint8_t reserved[5]; > > > + uint16_t private_data_offset; > > > + /**< Offset to indicate start of private data (if any). The private > > > + * data may be used by the application to store information which > > > + * should remain untouched in the library/driver > > > > Is this the offset for the private data after the crypto operation? > Yes. This is private date is meant for sessionless case. > > From your title, it looks like it is for the session private data, but > > then, this shouldn't be here. > Agree. > > If it is for the crypto operation, I suggest you to separate it in anot= her > patch. > > Also, you should indicate where the offset starts from. For the IV, > > the offset is counted from the start of the rte_crypto_op, so I think > > it should be the same, to keep consistency. > Sure. I will make a separate patch for this changes. Add some more > information to make it clear. > > > > For the session private data, we see two options: > > > > 1 - Add a "valid" private data field in the rte_cryptodev_sym_session > > structure, so when it is set, it indicates that the session contains > > private data (a single bit would be enough, 1 to indicate there is, and= 0 to > indicate there is not). > > This would go into the beginning of the structure, so this would > > require an ABI deprecation notice. > > This also assumes that the private data starts just after the session > > header > > > > 2 - Do not add an extra "valid" private data field in > > rte_cryptodev_sym_session structure, and add a small header in the > private data, which contains the "valid" > > bit. > > Then, when calling sym_session_get_private_data, this bit should be > checked. > > Note that the object that holds the session structure needs to be big > > enough to hold this value. > > If the object has only space for the sess_private_data array, then the > > session has no private data. > > Therefore, this approach might be less performant, but with no ABI > > deprecation required. > I am with option 2 with slight changes as below: > rte_cryptodev_sym_session_create() will have a flag as below indicating > private data exits or not. > { > - memset(sess, 0, (sizeof(void *) * nb_drivers)); > +memset(sess, 0, (sizeof(void *) * nb_drivers ) + > +sizeof(private_data_flag)); > } > and in > rte_cryptodev_get_header_session_size(void) > { > =A0=A0/* > =A0=A0=A0* Header contains pointers to the private data > =A0=A0=A0* of all registered drivers > =A0=A0=A0*/ > -return (sizeof(void *) * nb_drivers); > =A0=A0+return ((sizeof(void *) * nb_drivers) + sizeof(private_data_flag))= ; } With > this, a flag indicating private data exists or not will always have valid= value. Sure, this should work. Go ahead and send a v3 with this change, separating= the changes made in the session from the changes made in the crypto operation (so you w= ill have 3 patches in total). Pablo >=20 > > > > I would recommend you to send a deprecation notice for option 1, then > > check the performance of both option, and if needed, make the change > > in the structure, in 18.05. > > > > Regards, > > Pablo