[v2, 1/3] cryptodev: set private data for session-less mode

[v2, 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>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 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

[v2, 2/3] cryptodev: support session private data setting

[Abhinandan Gujjar]

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().

Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
---
 lib/librte_cryptodev/rte_cryptodev.c           | 43 +++++++++++++++++++++++---
 lib/librte_cryptodev/rte_cryptodev.h           | 32 +++++++++++++++++++
 lib/librte_cryptodev/rte_cryptodev_version.map |  7 +++++
 3 files changed, 78 insertions(+), 4 deletions(-)

diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
index 8745b6b..2a95a35 100644
--- a/lib/librte_cryptodev/rte_cryptodev.c
+++ b/lib/librte_cryptodev/rte_cryptodev.c
@@ -1099,8 +1099,10 @@ struct rte_cryptodev_sym_session *
 		return NULL;
 	}
 
-	/* Clear device session pointer */
-	memset(sess, 0, (sizeof(void *) * nb_drivers));
+	/* Clear device session pointer.
+	 * Include the flag indicating presence of private data
+	 */
+	memset(sess, 0, (sizeof(void *) * nb_drivers) + sizeof(uint8_t));
 
 	return sess;
 }
@@ -1204,9 +1206,10 @@ struct rte_cryptodev_sym_session *
 {
 	/*
 	 * Header contains pointers to the private data
-	 * of all registered drivers
+	 * of all registered drivers, and a flag which
+	 * indicates presence of private data
 	 */
-	return (sizeof(void *) * nb_drivers);
+	return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
 }
 
 unsigned int
@@ -1238,6 +1241,38 @@ struct rte_cryptodev_sym_session *
 
 }
 
+int __rte_experimental
+rte_cryptodev_sym_session_set_private_data(
+					struct rte_cryptodev_sym_session *sess,
+					void *data,
+					uint16_t size)
+{
+	uint16_t off_set = sizeof(void *) * nb_drivers;
+	uint8_t *private_data_present = (uint8_t *)sess + off_set;
+
+	if (sess == NULL)
+		return -EINVAL;
+
+	*private_data_present = 1;
+	off_set += sizeof(uint8_t);
+	rte_memcpy((uint8_t *)sess + off_set, data, size);
+	return 0;
+}
+
+void * __rte_experimental
+rte_cryptodev_sym_session_get_private_data(
+					struct rte_cryptodev_sym_session *sess)
+{
+	uint16_t off_set = sizeof(void *) * nb_drivers;
+	uint8_t *private_data_present = (uint8_t *)sess + off_set;
+
+	if (sess == NULL || !*private_data_present)
+		return NULL;
+
+	off_set += sizeof(uint8_t);
+	return (uint8_t *)sess + off_set;
+}
+
 /** Initialise rte_crypto_op mempool element */
 static void
 rte_crypto_op_init(struct rte_mempool *mempool,
diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
index c8fa689..261a359 100644
--- a/lib/librte_cryptodev/rte_cryptodev.h
+++ b/lib/librte_cryptodev/rte_cryptodev.h
@@ -1037,6 +1037,38 @@ struct rte_cryptodev_sym_session *
  */
 const char *rte_cryptodev_driver_name_get(uint8_t driver_id);
 
+/**
+ * Set private data for a session.
+ *
+ * @param	sess		Session pointer allocated by
+ *				*rte_cryptodev_sym_session_create*.
+ * @param	data		Pointer to the private data.
+ * @param	size		Size of the private data.
+ *
+ * @return
+ *  - On success, zero.
+ *  - On failure, a negative value.
+ */
+int __rte_experimental
+rte_cryptodev_sym_session_set_private_data(
+					struct rte_cryptodev_sym_session *sess,
+					void *data,
+					uint16_t size);
+
+/**
+ * Get private data of a session.
+ *
+ * @param	sess		Session pointer allocated by
+ *				*rte_cryptodev_sym_session_create*.
+ *
+ * @return
+ *  - On success return pointer to private data.
+ *  - On failure returns NULL.
+ */
+void * __rte_experimental
+rte_cryptodev_sym_session_get_private_data(
+					struct rte_cryptodev_sym_session *sess);
+
 #ifdef __cplusplus
 }
 #endif
diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map
index eb47308..560e464 100644
--- a/lib/librte_cryptodev/rte_cryptodev_version.map
+++ b/lib/librte_cryptodev/rte_cryptodev_version.map
@@ -85,3 +85,10 @@ DPDK_17.11 {
 	rte_cryptodev_pmd_parse_input_args;
 
 } DPDK_17.08;
+
+EXPERIMENTAL {
+        global:
+
+	rte_cryptodev_sym_session_get_private_data;
+	rte_cryptodev_sym_session_set_private_data;
+} DPDK_17.11;
-- 
1.9.1

[v3,3/3] doc: add private data info in crypto guide

[Abhinandan Gujjar]

Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
---
 doc/guides/prog_guide/cryptodev_lib.rst | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/doc/guides/prog_guide/cryptodev_lib.rst b/doc/guides/prog_guide/cryptodev_lib.rst
index 066fe2d..b279a20 100644
--- a/doc/guides/prog_guide/cryptodev_lib.rst
+++ b/doc/guides/prog_guide/cryptodev_lib.rst
@@ -299,6 +299,33 @@ directly from the devices processed queue, and for virtual device's from a
 enqueue call.
 
 
+Private data
+~~~~~~~~~~~~
+For session-based operations, the set and get API provides a mechanism for an
+application to store and retrieve the private data information stored along with
+the crypto session.
+
+For example, suppose an application is submitting a crypto operation with a session
+associated and wants to indicate private data information which is required to be
+used after completion of the crypto operation. In this case, the application can use
+the set API to set the private data and retrieve it using get API.
+
+.. code-block:: c
+
+	int rte_cryptodev_sym_session_set_private_data(
+		struct rte_cryptodev_sym_session *sess,	void *data, uint16_t size);
+
+	void * rte_cryptodev_sym_session_get_private_data(
+		struct rte_cryptodev_sym_session *sess);
+
+
+For session-less mode, the private data information can be placed along with the
+``struct rte_crypto_op``. The ``rte_crypto_op::private_data_offset`` indicates the
+start of private data information. The offset is counted from the start of the
+rte_crypto_op including other crypto information such as the IVs (since there can
+be an IV also for authentication).
+
+
 Enqueue / Dequeue Burst APIs
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
-- 
1.9.1

Re: [v3,3/3] doc: add private data info in crypto guide

[Akhil Goyal]

Hi Abhinandan,

On 4/16/2018 12:24 PM, Abhinandan Gujjar wrote:
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
I think I acked this complete series. And this patch is v2 not v3. You 
should also mention the changelog.

Thanks,
Akhil

> ---
>  doc/guides/prog_guide/cryptodev_lib.rst | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/doc/guides/prog_guide/cryptodev_lib.rst b/doc/guides/prog_guide/cryptodev_lib.rst
> index 066fe2d..b279a20 100644
> --- a/doc/guides/prog_guide/cryptodev_lib.rst
> +++ b/doc/guides/prog_guide/cryptodev_lib.rst
> @@ -299,6 +299,33 @@ directly from the devices processed queue, and for virtual device's from a
>  enqueue call.
>
>
> +Private data
> +~~~~~~~~~~~~
> +For session-based operations, the set and get API provides a mechanism for an
> +application to store and retrieve the private data information stored along with
> +the crypto session.
> +
> +For example, suppose an application is submitting a crypto operation with a session
> +associated and wants to indicate private data information which is required to be
> +used after completion of the crypto operation. In this case, the application can use
> +the set API to set the private data and retrieve it using get API.
> +
> +.. code-block:: c
> +
> +	int rte_cryptodev_sym_session_set_private_data(
> +		struct rte_cryptodev_sym_session *sess,	void *data, uint16_t size);
> +
> +	void * rte_cryptodev_sym_session_get_private_data(
> +		struct rte_cryptodev_sym_session *sess);
> +
> +
> +For session-less mode, the private data information can be placed along with the
> +``struct rte_crypto_op``. The ``rte_crypto_op::private_data_offset`` indicates the
> +start of private data information. The offset is counted from the start of the
> +rte_crypto_op including other crypto information such as the IVs (since there can
> +be an IV also for authentication).
> +
> +
>  Enqueue / Dequeue Burst APIs
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>
>

Re: [v3,3/3] doc: add private data info in crypto guide

[Gujjar, Abhinandan S]

Hi Akhil,

Missed out the *series* and thought the ack was only on the doc patch.
Regarding change log, Just before posting I looked at recent patches.
Most of them, didn't have change log! So, I didn't add it.
Yes, there is a typo in the subject of this patch. Does this require newer version of patches?

Regards
Abhinandan

> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Monday, April 16, 2018 2:47 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; jerin.jacob@caviumnetworks.com;
> hemant.agrawal@nxp.com; dev@dpdk.org
> Cc: Vangati, Narender <narender.vangati@intel.com>; Rao, Nikhil
> <nikhil.rao@intel.com>
> Subject: Re: [v3,3/3] doc: add private data info in crypto guide
> 
> Hi Abhinandan,
> 
> On 4/16/2018 12:24 PM, Abhinandan Gujjar wrote:
> > Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> > Acked-by: Akhil Goyal <akhil.goyal@nxp.com>
> I think I acked this complete series. And this patch is v2 not v3. You should also
> mention the changelog.
> 
> Thanks,
> Akhil
> 
> > ---
> >  doc/guides/prog_guide/cryptodev_lib.rst | 27
> > +++++++++++++++++++++++++++
> >  1 file changed, 27 insertions(+)
> >
> > diff --git a/doc/guides/prog_guide/cryptodev_lib.rst
> > b/doc/guides/prog_guide/cryptodev_lib.rst
> > index 066fe2d..b279a20 100644
> > --- a/doc/guides/prog_guide/cryptodev_lib.rst
> > +++ b/doc/guides/prog_guide/cryptodev_lib.rst
> > @@ -299,6 +299,33 @@ directly from the devices processed queue, and
> > for virtual device's from a  enqueue call.
> >
> >
> > +Private data
> > +~~~~~~~~~~~~
> > +For session-based operations, the set and get API provides a
> > +mechanism for an application to store and retrieve the private data
> > +information stored along with the crypto session.
> > +
> > +For example, suppose an application is submitting a crypto operation
> > +with a session associated and wants to indicate private data
> > +information which is required to be used after completion of the
> > +crypto operation. In this case, the application can use the set API to set the
> private data and retrieve it using get API.
> > +
> > +.. code-block:: c
> > +
> > +	int rte_cryptodev_sym_session_set_private_data(
> > +		struct rte_cryptodev_sym_session *sess,	void *data, uint16_t
> size);
> > +
> > +	void * rte_cryptodev_sym_session_get_private_data(
> > +		struct rte_cryptodev_sym_session *sess);
> > +
> > +
> > +For session-less mode, the private data information can be placed
> > +along with the ``struct rte_crypto_op``. The
> > +``rte_crypto_op::private_data_offset`` indicates the start of private
> > +data information. The offset is counted from the start of the
> > +rte_crypto_op including other crypto information such as the IVs (since there
> can be an IV also for authentication).
> > +
> > +
> >  Enqueue / Dequeue Burst APIs
> >  ~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >
> >

Re: [v3,3/3] doc: add private data info in crypto guide

[De Lara Guarch, Pablo]

Hi,

> -----Original Message-----
> From: Gujjar, Abhinandan S
> Sent: Monday, April 16, 2018 10:36 AM
> To: Akhil Goyal <akhil.goyal@nxp.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; jerin.jacob@caviumnetworks.com;
> hemant.agrawal@nxp.com; dev@dpdk.org
> Cc: Vangati, Narender <narender.vangati@intel.com>; Rao, Nikhil
> <nikhil.rao@intel.com>
> Subject: RE: [v3,3/3] doc: add private data info in crypto guide
> 
> Hi Akhil,
> 
> Missed out the *series* and thought the ack was only on the doc patch.
> Regarding change log, Just before posting I looked at recent patches.
> Most of them, didn't have change log! So, I didn't add it.
> Yes, there is a typo in the subject of this patch. Does this require newer version
> of patches?

Unless there is something else to change, there is no need to send another patchset
just for this typo.
About changelog, it is a good practice to add it. Also, for patchsets, it is good to
add a cover letter, and there you could add the changelog for the entire patchset.

Thanks,
Pablo

Re: [v2, 1/3] cryptodev: set private data for session-less mode

[De Lara Guarch, Pablo]

> -----Original Message-----
> From: Gujjar, Abhinandan S
> Sent: Monday, April 16, 2018 7:55 AM
> To: De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; jerin.jacob@caviumnetworks.com;
> hemant.agrawal@nxp.com; akhil.goyal@nxp.com; dev@dpdk.org
> Cc: Vangati, Narender <narender.vangati@intel.com>; Gujjar, Abhinandan S
> <abhinandan.gujjar@intel.com>; Rao, Nikhil <nikhil.rao@intel.com>
> Subject: [v2,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>
> Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>

Applied to dpdk-next-crypto.
Thanks,

Pablo

Re: [v2, 2/3] cryptodev: support session private data setting

[Verma, Shally]

>-----Original Message-----
>From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Abhinandan Gujjar
>Sent: 16 April 2018 12:25
>To: pablo.de.lara.guarch@intel.com; declan.doherty@intel.com; Jacob, Jerin <Jerin.JacobKollanukkaran@cavium.com>;
>hemant.agrawal@nxp.com; akhil.goyal@nxp.com; dev@dpdk.org
>Cc: narender.vangati@intel.com; abhinandan.gujjar@intel.com; nikhil.rao@intel.com
>Subject: [dpdk-dev] [v2, 2/3] cryptodev: support session private data setting
>
>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().
>
>Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
>Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
>Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
>---
> lib/librte_cryptodev/rte_cryptodev.c           | 43 +++++++++++++++++++++++---
> lib/librte_cryptodev/rte_cryptodev.h           | 32 +++++++++++++++++++
> lib/librte_cryptodev/rte_cryptodev_version.map |  7 +++++
> 3 files changed, 78 insertions(+), 4 deletions(-)
>
>diff --git a/lib/librte_cryptodev/rte_cryptodev.c b/lib/librte_cryptodev/rte_cryptodev.c
>index 8745b6b..2a95a35 100644
>--- a/lib/librte_cryptodev/rte_cryptodev.c
>+++ b/lib/librte_cryptodev/rte_cryptodev.c
>@@ -1099,8 +1099,10 @@ struct rte_cryptodev_sym_session *
> 		return NULL;
> 	}
>
>-	/* Clear device session pointer */
>-	memset(sess, 0, (sizeof(void *) * nb_drivers));
>+	/* Clear device session pointer.
>+	 * Include the flag indicating presence of private data
>+	 */
>+	memset(sess, 0, (sizeof(void *) * nb_drivers) + sizeof(uint8_t));
>
> 	return sess;
> }
>@@ -1204,9 +1206,10 @@ struct rte_cryptodev_sym_session *
> {
> 	/*
> 	 * Header contains pointers to the private data
>-	 * of all registered drivers
>+	 * of all registered drivers, and a flag which
>+	 * indicates presence of private data
> 	 */
>-	return (sizeof(void *) * nb_drivers);
>+	return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
> }
>
> unsigned int
>@@ -1238,6 +1241,38 @@ struct rte_cryptodev_sym_session *
>
> }
>
>+int __rte_experimental
>+rte_cryptodev_sym_session_set_private_data(
>+					struct rte_cryptodev_sym_session *sess,
>+					void *data,
>+					uint16_t size)
>+{
>+	uint16_t off_set = sizeof(void *) * nb_drivers;
>+	uint8_t *private_data_present = (uint8_t *)sess + off_set;

[Shally]  I was going through this in context of crypto event adapter and realize probably it is safer to set priv_data_size after (sess == NULL) check.
Same is applicable in get_private_data().

>+
>+	if (sess == NULL)
>+		return -EINVAL;
>+
>+	*private_data_present = 1;
>+	off_set += sizeof(uint8_t);
>+	rte_memcpy((uint8_t *)sess + off_set, data, size);
>+	return 0;
>+}
>+
>+void * __rte_experimental
>+rte_cryptodev_sym_session_get_private_data(
>+					struct rte_cryptodev_sym_session *sess)
>+{
>+	uint16_t off_set = sizeof(void *) * nb_drivers;
>+	uint8_t *private_data_present = (uint8_t *)sess + off_set;
>+
>+	if (sess == NULL || !*private_data_present)
>+		return NULL;
>+
>+	off_set += sizeof(uint8_t);
>+	return (uint8_t *)sess + off_set;
>+}
>+
> /** Initialise rte_crypto_op mempool element */
> static void
> rte_crypto_op_init(struct rte_mempool *mempool,
>diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
>index c8fa689..261a359 100644
>--- a/lib/librte_cryptodev/rte_cryptodev.h
>+++ b/lib/librte_cryptodev/rte_cryptodev.h
>@@ -1037,6 +1037,38 @@ struct rte_cryptodev_sym_session *
>  */
> const char *rte_cryptodev_driver_name_get(uint8_t driver_id);
>
>+/**
>+ * Set private data for a session.
>+ *
>+ * @param	sess		Session pointer allocated by
>+ *				*rte_cryptodev_sym_session_create*.
>+ * @param	data		Pointer to the private data.
>+ * @param	size		Size of the private data.
>+ *
>+ * @return
>+ *  - On success, zero.
>+ *  - On failure, a negative value.
>+ */
>+int __rte_experimental
>+rte_cryptodev_sym_session_set_private_data(
>+					struct rte_cryptodev_sym_session *sess,
>+					void *data,
>+					uint16_t size);
>+
>+/**
>+ * Get private data of a session.
>+ *
>+ * @param	sess		Session pointer allocated by
>+ *				*rte_cryptodev_sym_session_create*.
>+ *
>+ * @return
>+ *  - On success return pointer to private data.
>+ *  - On failure returns NULL.
>+ */
>+void * __rte_experimental
>+rte_cryptodev_sym_session_get_private_data(
>+					struct rte_cryptodev_sym_session *sess);
>+
> #ifdef __cplusplus
> }
> #endif
>diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map b/lib/librte_cryptodev/rte_cryptodev_version.map
>index eb47308..560e464 100644
>--- a/lib/librte_cryptodev/rte_cryptodev_version.map
>+++ b/lib/librte_cryptodev/rte_cryptodev_version.map
>@@ -85,3 +85,10 @@ DPDK_17.11 {
> 	rte_cryptodev_pmd_parse_input_args;
>
> } DPDK_17.08;
>+
>+EXPERIMENTAL {
>+        global:
>+
>+	rte_cryptodev_sym_session_get_private_data;
>+	rte_cryptodev_sym_session_set_private_data;
>+} DPDK_17.11;
>--
>1.9.1

Re: [v2, 2/3] cryptodev: support session private data setting

[Gujjar, Abhinandan S]

> -----Original Message-----
> From: Verma, Shally [mailto:Shally.Verma@cavium.com]
> Sent: Thursday, April 19, 2018 5:47 PM
> To: Gujjar, Abhinandan S <abhinandan.gujjar@intel.com>; De Lara Guarch, Pablo
> <pablo.de.lara.guarch@intel.com>; Doherty, Declan
> <declan.doherty@intel.com>; Jacob, Jerin
> <Jerin.JacobKollanukkaran@cavium.com>; hemant.agrawal@nxp.com;
> akhil.goyal@nxp.com; dev@dpdk.org
> Cc: Vangati, Narender <narender.vangati@intel.com>; Rao, Nikhil
> <nikhil.rao@intel.com>
> Subject: RE: [dpdk-dev] [v2, 2/3] cryptodev: support session private data setting
> 
> 
> 
> >-----Original Message-----
> >From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Abhinandan Gujjar
> >Sent: 16 April 2018 12:25
> >To: pablo.de.lara.guarch@intel.com; declan.doherty@intel.com; Jacob,
> >Jerin <Jerin.JacobKollanukkaran@cavium.com>;
> >hemant.agrawal@nxp.com; akhil.goyal@nxp.com; dev@dpdk.org
> >Cc: narender.vangati@intel.com; abhinandan.gujjar@intel.com;
> >nikhil.rao@intel.com
> >Subject: [dpdk-dev] [v2, 2/3] cryptodev: support session private data
> >setting
> >
> >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().
> >
> >Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> >Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
> >Acked-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
> >---
> > lib/librte_cryptodev/rte_cryptodev.c           | 43 +++++++++++++++++++++++---
> > lib/librte_cryptodev/rte_cryptodev.h           | 32 +++++++++++++++++++
> > lib/librte_cryptodev/rte_cryptodev_version.map |  7 +++++
> > 3 files changed, 78 insertions(+), 4 deletions(-)
> >
> >diff --git a/lib/librte_cryptodev/rte_cryptodev.c
> >b/lib/librte_cryptodev/rte_cryptodev.c
> >index 8745b6b..2a95a35 100644
> >--- a/lib/librte_cryptodev/rte_cryptodev.c
> >+++ b/lib/librte_cryptodev/rte_cryptodev.c
> >@@ -1099,8 +1099,10 @@ struct rte_cryptodev_sym_session *
> > 		return NULL;
> > 	}
> >
> >-	/* Clear device session pointer */
> >-	memset(sess, 0, (sizeof(void *) * nb_drivers));
> >+	/* Clear device session pointer.
> >+	 * Include the flag indicating presence of private data
> >+	 */
> >+	memset(sess, 0, (sizeof(void *) * nb_drivers) + sizeof(uint8_t));
> >
> > 	return sess;
> > }
> >@@ -1204,9 +1206,10 @@ struct rte_cryptodev_sym_session *  {
> > 	/*
> > 	 * Header contains pointers to the private data
> >-	 * of all registered drivers
> >+	 * of all registered drivers, and a flag which
> >+	 * indicates presence of private data
> > 	 */
> >-	return (sizeof(void *) * nb_drivers);
> >+	return ((sizeof(void *) * nb_drivers) + sizeof(uint8_t));
> > }
> >
> > unsigned int
> >@@ -1238,6 +1241,38 @@ struct rte_cryptodev_sym_session *
> >
> > }
> >
> >+int __rte_experimental
> >+rte_cryptodev_sym_session_set_private_data(
> >+					struct rte_cryptodev_sym_session
> *sess,
> >+					void *data,
> >+					uint16_t size)
> >+{
> >+	uint16_t off_set = sizeof(void *) * nb_drivers;
> >+	uint8_t *private_data_present = (uint8_t *)sess + off_set;
> 
> [Shally]  I was going through this in context of crypto event adapter and realize
> probably it is safer to set priv_data_size after (sess == NULL) check.
> Same is applicable in get_private_data().
[Abhinandan] I guess you are pointing *private_data_present* which is calculated prior, it will not be used if the sess is NULL.

> 
> >+
> >+	if (sess == NULL)
> >+		return -EINVAL;
> >+
> >+	*private_data_present = 1;
> >+	off_set += sizeof(uint8_t);
> >+	rte_memcpy((uint8_t *)sess + off_set, data, size);
> >+	return 0;
> >+}
> >+
> >+void * __rte_experimental
> >+rte_cryptodev_sym_session_get_private_data(
> >+					struct rte_cryptodev_sym_session
> *sess) {
> >+	uint16_t off_set = sizeof(void *) * nb_drivers;
> >+	uint8_t *private_data_present = (uint8_t *)sess + off_set;
> >+
> >+	if (sess == NULL || !*private_data_present)
> >+		return NULL;
> >+
> >+	off_set += sizeof(uint8_t);
> >+	return (uint8_t *)sess + off_set;
> >+}
> >+
> > /** Initialise rte_crypto_op mempool element */  static void
> >rte_crypto_op_init(struct rte_mempool *mempool, diff --git
> >a/lib/librte_cryptodev/rte_cryptodev.h
> >b/lib/librte_cryptodev/rte_cryptodev.h
> >index c8fa689..261a359 100644
> >--- a/lib/librte_cryptodev/rte_cryptodev.h
> >+++ b/lib/librte_cryptodev/rte_cryptodev.h
> >@@ -1037,6 +1037,38 @@ struct rte_cryptodev_sym_session *
> >  */
> > const char *rte_cryptodev_driver_name_get(uint8_t driver_id);
> >
> >+/**
> >+ * Set private data for a session.
> >+ *
> >+ * @param	sess		Session pointer allocated by
> >+ *				*rte_cryptodev_sym_session_create*.
> >+ * @param	data		Pointer to the private data.
> >+ * @param	size		Size of the private data.
> >+ *
> >+ * @return
> >+ *  - On success, zero.
> >+ *  - On failure, a negative value.
> >+ */
> >+int __rte_experimental
> >+rte_cryptodev_sym_session_set_private_data(
> >+					struct rte_cryptodev_sym_session
> *sess,
> >+					void *data,
> >+					uint16_t size);
> >+
> >+/**
> >+ * Get private data of a session.
> >+ *
> >+ * @param	sess		Session pointer allocated by
> >+ *				*rte_cryptodev_sym_session_create*.
> >+ *
> >+ * @return
> >+ *  - On success return pointer to private data.
> >+ *  - On failure returns NULL.
> >+ */
> >+void * __rte_experimental
> >+rte_cryptodev_sym_session_get_private_data(
> >+					struct rte_cryptodev_sym_session
> *sess);
> >+
> > #ifdef __cplusplus
> > }
> > #endif
> >diff --git a/lib/librte_cryptodev/rte_cryptodev_version.map
> >b/lib/librte_cryptodev/rte_cryptodev_version.map
> >index eb47308..560e464 100644
> >--- a/lib/librte_cryptodev/rte_cryptodev_version.map
> >+++ b/lib/librte_cryptodev/rte_cryptodev_version.map
> >@@ -85,3 +85,10 @@ DPDK_17.11 {
> > 	rte_cryptodev_pmd_parse_input_args;
> >
> > } DPDK_17.08;
> >+
> >+EXPERIMENTAL {
> >+        global:
> >+
> >+	rte_cryptodev_sym_session_get_private_data;
> >+	rte_cryptodev_sym_session_set_private_data;
> >+} DPDK_17.11;
> >--
> >1.9.1

Re: [v2, 2/3] cryptodev: support session private data setting

[Verma, Shally]

//snip
>> >+int __rte_experimental
>> >+rte_cryptodev_sym_session_set_private_data(
>> >+					struct rte_cryptodev_sym_session
>> *sess,
>> >+					void *data,
>> >+					uint16_t size)
>> >+{
>> >+	uint16_t off_set = sizeof(void *) * nb_drivers;
>> >+	uint8_t *private_data_present = (uint8_t *)sess + off_set;
>>
>> [Shally]  I was going through this in context of crypto event adapter and realize
>> probably it is safer to set priv_data_size after (sess == NULL) check.
>> Same is applicable in get_private_data().
>[Abhinandan] I guess you are pointing *private_data_present* which is calculated prior, it will not be used if the sess is NULL.
>
Oh. Ya . sorry I missed that.

//snip

Thanks
Shally