DPDK patches and discussions
 help / color / mirror / Atom feed
From: Akhil Goyal <akhil.goyal@nxp.com>
To: Abhinandan Gujjar <abhinandan.gujjar@intel.com>,
	<declan.doherty@intel.com>
Cc: <dev@dpdk.org>, <narender.vangati@intel.com>,
	Nikhil Rao <nikhil.rao@intel.com>
Subject: Re: [dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data
Date: Mon, 15 Jan 2018 17:48:40 +0530	[thread overview]
Message-ID: <a8e2a777-5a4c-23be-76ea-d8533d4872e5@nxp.com> (raw)
In-Reply-To: <1516017078-166766-1-git-send-email-abhinandan.gujjar@intel.com>

Hi Abhinandan,
On 1/15/2018 5:21 PM, Abhinandan Gujjar wrote:
> Update rte_crypto_op to indicate private data type and 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().
> 
> Signed-off-by: Abhinandan Gujjar <abhinandan.gujjar@intel.com>
> Signed-off-by: Nikhil Rao <nikhil.rao@intel.com>
Subject line should be cryptodev:....
similar comment for 2/2
> ---
>   lib/librte_cryptodev/rte_crypto.h    | 19 +++++++++++++++++--
>   lib/librte_cryptodev/rte_cryptodev.h | 30 ++++++++++++++++++++++++++++++
>   2 files changed, 47 insertions(+), 2 deletions(-)
> 
> 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.
>   /**
>    * Cryptographic Operation.
>    *
> @@ -84,8 +96,11 @@ struct rte_crypto_op {
>   	 */
>   	uint8_t sess_type;
>   	/**< operation session type */
> -
> -	uint8_t reserved[5];
> +	uint8_t private_data_type;
> +	/**< Private data type. @see enum rte_crypto_op_private_data_type */
> +	uint16_t private_data_offset;
> +	/**< Offset to indicate start of private data */
It is better to add some more information in the comments just like 
description. While viewing the code, it is not explicit that who is the 
intended user of this private data.
> +	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 */
> diff --git a/lib/librte_cryptodev/rte_cryptodev.h b/lib/librte_cryptodev/rte_cryptodev.h
> index dade554..56958a6 100644
> --- a/lib/librte_cryptodev/rte_cryptodev.h
> +++ b/lib/librte_cryptodev/rte_cryptodev.h
> @@ -1033,6 +1033,36 @@ 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_cryptodev_sym_session_set_private_data(
> +				struct rte_cryptodev_sym_session *sess,
> +				void *data,
> +				uint16_t size);
Looks like this is an RFC patch. There is no implementation of this API 
and the formatting is also incorrect. Please correct formatting while 
sending the complete patch.
Same comment for 2/2 patch
> +
> +/**
> + * 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_cryptodev_sym_session_get_private_data(
> +				const struct rte_cryptodev_sym_session *sess);
> +
same here.
>   #ifdef __cplusplus
>   }
>   #endif
> 

-Akhil

  reply	other threads:[~2018-01-15 12:18 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-01-15 11:51 Abhinandan Gujjar
2018-01-15 12:18 ` Akhil Goyal [this message]
2018-01-16  6:09   ` Gujjar, Abhinandan S
2018-01-16  6:24     ` Akhil Goyal
2018-01-16  7:05       ` Gujjar, Abhinandan S
2018-01-16  7:26         ` Akhil Goyal
2018-01-16  9:03           ` Gujjar, Abhinandan S
2018-01-16  9:21             ` Akhil Goyal
2018-01-16 11:36               ` Gujjar, Abhinandan S
2018-01-16 12:00                 ` Akhil Goyal
2018-01-16 12:29                   ` Gujjar, Abhinandan S
2018-01-16 13:02                     ` Akhil Goyal
2018-01-17  6:35                       ` Gujjar, Abhinandan S
2018-01-17  9:46                         ` De Lara Guarch, Pablo
2018-01-17 10:05                           ` Gujjar, Abhinandan S
2018-01-17 10:52                             ` Akhil Goyal
2018-01-18  6:52                               ` Gujjar, Abhinandan S
2018-01-22  6:51                                 ` Gujjar, Abhinandan S

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a8e2a777-5a4c-23be-76ea-d8533d4872e5@nxp.com \
    --to=akhil.goyal@nxp.com \
    --cc=abhinandan.gujjar@intel.com \
    --cc=declan.doherty@intel.com \
    --cc=dev@dpdk.org \
    --cc=narender.vangati@intel.com \
    --cc=nikhil.rao@intel.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).