[dpdk-dev] [RFC v3, 1/3] cryptodev: set private data for session-less mode

[dpdk-dev] [RFC v3, 1/3] cryptodev: set private data for session-less mode

[Abhinandan Gujjar]

The application may want to store private data along with the
rte_crypto_op that is transparent to the rte_cryptodev layer.
For e.g., If an eventdev based application is submitting a
crypto session-less operation and wants to indicate event
information required to construct a new event that will be
enqueued to eventdev after completion of the crypto
operation. This patch provides a mechanism for the application
to associate this information with the rte_crypto_op in
session-less mode.

Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
---
Notes:
	V3:
	1. Added separate patch for session-less private data
	2. Added more information on offset
	V2:
	1. Removed enum rte_crypto_op_private_data_type
	2. Corrected formatting

 lib/librte_cryptodev/rte_crypto.h | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/lib/librte_cryptodev/rte_crypto.h b/lib/librte_cryptodev/rte_crypto.h
index 95cf861..2540426 100644
--- a/lib/librte_cryptodev/rte_crypto.h
+++ b/lib/librte_cryptodev/rte_crypto.h
@@ -84,8 +84,14 @@ struct rte_crypto_op {
 	 */
 	uint8_t sess_type;
 	/**< operation session type */
+	uint16_t private_data_offset;
+	/**< Offset to indicate start of private data (if any). The offset
+	 * is counted from the start of the rte_crypto_op including IV.
+	 * The private data may be used by the application to store
+	 * information which should remain untouched in the library/driver
+	 */
 
-	uint8_t reserved[5];
+	uint8_t reserved[3];
 	/**< Reserved bytes to fill 64 bits for future additions */
 	struct rte_mempool *mempool;
 	/**< crypto operation mempool which operation is allocated from */
-- 
1.9.1

Re: [dpdk-dev] [RFC v3, 1/3] cryptodev: set private data for session-less mode

[Gujjar, Abhinandan S]

A gentle remainder for review :)

-Abhinandan

> -----Original Message-----
> From: Gujjar, Abhinandan S
> Sent: Thursday, February 1, 2018 3:00 PM
> To: Doherty, Declan <declan.doherty@intel.com>; akhil.goyal@nxp.com; De
> Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> Jerin.JacobKollanukkaran@cavium.com
> Cc: dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>; Gujjar,
> Abhinandan S <abhinandan.gujjar@intel.com>; Rao, Nikhil
> <nikhil.rao@intel.com>
> Subject: [RFC v3, 1/3] cryptodev: set private data for session-less mode
> 
> The application may want to store private data along with the rte_crypto_op
> that is transparent to the rte_cryptodev layer.
> For e.g., If an eventdev based application is submitting a crypto session-less
> operation and wants to indicate event information required to construct a new
> event that will be enqueued to eventdev after completion of the crypto
> operation. This patch provides a mechanism for the application to associate this
> information with the rte_crypto_op in session-less mode.
> 
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> ---
> Notes:
> 	V3:
> 	1. Added separate patch for session-less private data
> 	2. Added more information on offset
> 	V2:
> 	1. Removed enum rte_crypto_op_private_data_type
> 	2. Corrected formatting
> 
>  lib/librte_cryptodev/rte_crypto.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto.h b/lib/librte_cryptodev/rte_crypto.h
> index 95cf861..2540426 100644
> --- a/lib/librte_cryptodev/rte_crypto.h
> +++ b/lib/librte_cryptodev/rte_crypto.h
> @@ -84,8 +84,14 @@ struct rte_crypto_op {
>  	 */
>  	uint8_t sess_type;
>  	/**< operation session type */
> +	uint16_t private_data_offset;
> +	/**< Offset to indicate start of private data (if any). The offset
> +	 * is counted from the start of the rte_crypto_op including IV.
> +	 * The private data may be used by the application to store
> +	 * information which should remain untouched in the library/driver
> +	 */
> 
> -	uint8_t reserved[5];
> +	uint8_t reserved[3];
>  	/**< Reserved bytes to fill 64 bits for future additions */
>  	struct rte_mempool *mempool;
>  	/**< crypto operation mempool which operation is allocated from */
> --
> 1.9.1

Re: [dpdk-dev] [RFC v3, 1/3] cryptodev: set private data for session-less mode

[De Lara Guarch, Pablo]

Hi,

> -----Original Message-----
> From: Gujjar, Abhinandan S
> Sent: Thursday, February 1, 2018 9:30 AM
> To: Doherty, Declan <declan.doherty@intel.com>; akhil.goyal@nxp.com; De
> Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> Jerin.JacobKollanukkaran@cavium.com
> Cc: dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>;
> Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Rao, Nikhil
> <nikhil.rao@intel.com>
> Subject: [RFC v3, 1/3] cryptodev: set private data for session-less mode
> 
> The application may want to store private data along with the
> rte_crypto_op that is transparent to the rte_cryptodev layer.
> For e.g., If an eventdev based application is submitting a crypto session-less
> operation and wants to indicate event information required to construct a
> new event that will be enqueued to eventdev after completion of the crypto
> operation. This patch provides a mechanism for the application to associate
> this information with the rte_crypto_op in session-less mode.
> 
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> ---
> Notes:
> 	V3:
> 	1. Added separate patch for session-less private data
> 	2. Added more information on offset
> 	V2:
> 	1. Removed enum rte_crypto_op_private_data_type
> 	2. Corrected formatting
> 
>  lib/librte_cryptodev/rte_crypto.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/librte_cryptodev/rte_crypto.h
> b/lib/librte_cryptodev/rte_crypto.h
> index 95cf861..2540426 100644
> --- a/lib/librte_cryptodev/rte_crypto.h
> +++ b/lib/librte_cryptodev/rte_crypto.h
> @@ -84,8 +84,14 @@ struct rte_crypto_op {
>  	 */
>  	uint8_t sess_type;
>  	/**< operation session type */
> +	uint16_t private_data_offset;
> +	/**< Offset to indicate start of private data (if any). The offset
> +	 * is counted from the start of the rte_crypto_op including IV.

I assume that if this offset is 0, it means that there is no private data?

Also, I would reword the Doxygen comment a bit (here is my suggestion, which looks a bit long, but I hope it is descriptive).

" Offset to indicate start of private data (if any). The offset is counted from the start of the rte_crypto_op structure.
The private data may be used by the application to store information which should remain untouched in the library/driver.
Note that an Initialization Vector (IV) for the operation may be placed at the end of the structure as well, so caution to avoid overwrites has to be taken."


> +	 * The private data may be used by the application to store
> +	 * information which should remain untouched in the library/driver
> +	 */
> 
> -	uint8_t reserved[5];
> +	uint8_t reserved[3];
>  	/**< Reserved bytes to fill 64 bits for future additions */
>  	struct rte_mempool *mempool;
>  	/**< crypto operation mempool which operation is allocated from
> */
> --
> 1.9.1

Apart from the comments above, this overall looks good to me.
I would say you could submit a v1 with some implementation.
Could you also modify the Cryptodev section in the Programmer's Guide document, to reflect these changes?

Thanks,
Pablo

Re: [dpdk-dev] [RFC v3, 1/3] cryptodev: set private data for session-less mode

[Gujjar, Abhinandan S]

Hi Pablo,

> -----Original Message-----
> From: De Lara Guarch, Pablo
> Sent: Tuesday, February 13, 2018 8:24 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; akhil.goyal@nxp.com;
> Jerin.JacobKollanukkaran@cavium.com
> Cc: dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>; Rao,
> Nikhil <nikhil.rao@intel.com>
> Subject: RE: [RFC v3, 1/3] cryptodev: set private data for session-less mode
> 
> Hi,
> 
> > -----Original Message-----
> > From: Gujjar, Abhinandan S
> > Sent: Thursday, February 1, 2018 9:30 AM
> > To: Doherty, Declan <declan.doherty@intel.com>; akhil.goyal@nxp.com;
> > De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> > Jerin.JacobKollanukkaran@cavium.com
> > Cc: dev@dpdk.org; Vangati, Narender <narender.vangati@intel.com>;
> > Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; Rao, Nikhil
> > <nikhil.rao@intel.com>
> > Subject: [RFC v3, 1/3] cryptodev: set private data for session-less
> > mode
> >
> > The application may want to store private data along with the
> > rte_crypto_op that is transparent to the rte_cryptodev layer.
> > For e.g., If an eventdev based application is submitting a crypto
> > session-less operation and wants to indicate event information
> > required to construct a new event that will be enqueued to eventdev
> > after completion of the crypto operation. This patch provides a
> > mechanism for the application to associate this information with the
> rte_crypto_op in session-less mode.
> >
> > Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> > Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> > ---
> > Notes:
> > 	V3:
> > 	1. Added separate patch for session-less private data
> > 	2. Added more information on offset
> > 	V2:
> > 	1. Removed enum rte_crypto_op_private_data_type
> > 	2. Corrected formatting
> >
> >  lib/librte_cryptodev/rte_crypto.h | 8 +++++++-
> >  1 file changed, 7 insertions(+), 1 deletion(-)
> >
> > diff --git a/lib/librte_cryptodev/rte_crypto.h
> > b/lib/librte_cryptodev/rte_crypto.h
> > index 95cf861..2540426 100644
> > --- a/lib/librte_cryptodev/rte_crypto.h
> > +++ b/lib/librte_cryptodev/rte_crypto.h
> > @@ -84,8 +84,14 @@ struct rte_crypto_op {
> >  	 */
> >  	uint8_t sess_type;
> >  	/**< operation session type */
> > +	uint16_t private_data_offset;
> > +	/**< Offset to indicate start of private data (if any). The offset
> > +	 * is counted from the start of the rte_crypto_op including IV.
> 
> I assume that if this offset is 0, it means that there is no private data?
Yes
> 
> Also, I would reword the Doxygen comment a bit (here is my suggestion, which
> looks a bit long, but I hope it is descriptive).
> 
> " Offset to indicate start of private data (if any). The offset is counted from the
> start of the rte_crypto_op structure.
> The private data may be used by the application to store information which
> should remain untouched in the library/driver.
> Note that an Initialization Vector (IV) for the operation may be placed at the end
> of the structure as well, so caution to avoid overwrites has to be taken."
> 
> 
> > +	 * The private data may be used by the application to store
> > +	 * information which should remain untouched in the library/driver
> > +	 */
> >
> > -	uint8_t reserved[5];
> > +	uint8_t reserved[3];
> >  	/**< Reserved bytes to fill 64 bits for future additions */
> >  	struct rte_mempool *mempool;
> >  	/**< crypto operation mempool which operation is allocated from */
> > --
> > 1.9.1
> 
> Apart from the comments above, this overall looks good to me.
> I would say you could submit a v1 with some implementation.
Ok.
> Could you also modify the Cryptodev section in the Programmer's Guide
> document, to reflect these changes?
Sure.

Thanks
Abhinandan
> 
> Thanks,
> Pablo