Hi Pablo & Declan, > -----Original Message----- > From: De Lara Guarch, Pablo > Sent: Thursday, January 25, 2018 1:17 AM > To: Gujjar, Abhinandan S <[email protected]>; Doherty, Declan > <[email protected]>; [email protected]; > [email protected] > Cc: [email protected]; Vangati, Narender <[email protected]>; Rao, > Nikhil <[email protected]> > 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 <[email protected]>; [email protected]; > > De Lara Guarch, Pablo <[email protected]>; > > [email protected] > > Cc: [email protected]; Vangati, Narender <[email protected]>; > > Gujjar, Abhinandan S <[email protected]>; Rao, Nikhil > > <[email protected]> > > 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 <[email protected]> > > Signed-off-by: Nikhil Rao <[email protected]> > > --- > > 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 another > 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) { /* * Header contains pointers to the private data * of all registered drivers */ -return (sizeof(void *) * nb_drivers); +return ((sizeof(void *) * nb_drivers) + sizeof(private_data_flag)); } With this, a flag indicating private data exists or not will always have valid value.
> > 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
