From: Akhil Goyal <akhil.goyal@nxp.com>
To: "Gujjar, Abhinandan S" <abhinandan.gujjar@intel.com>,
"Doherty, Declan" <declan.doherty@intel.com>
Cc: "dev@dpdk.org" <dev@dpdk.org>,
"Vangati, Narender" <narender.vangati@intel.com>,
"Rao, Nikhil" <nikhil.rao@intel.com>
Subject: Re: [dpdk-dev] [PATCH 1/2] lib/cryptodev: add support to set session private data
Date: Tue, 16 Jan 2018 11:54:35 +0530 [thread overview]
Message-ID: <f0232518-7576-ce8f-9c8f-65dfc978da8d@nxp.com> (raw)
In-Reply-To: <5612CB344B05EE4F95FC5B729939F780705FED56@PGSMSX102.gar.corp.intel.com>
Hi Abhinandan,
On 1/16/2018 11:39 AM, Gujjar, Abhinandan S wrote:
>>> 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.
> Right now, Application uses 'rte_crypto_op_private_data_type' to indicate rte_cryptodev_sym_session_set_private_data()
> was called to set the private data. Otherwise, how do you indicate there is a private data associated with the session?
> Any suggestions?
For session based flows, the first choice to store the private data
should be in the session. So RTE_CRYPTO_OP_WITH_SESSION or
RTE_CRYPTO_OP_SECURITY_SESSION can be used to call
rte_cryptodev_sym_session_set_private_data or
rte_security_session_set_private_data.
>>> /**
>>> * 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.
> The propose APIs are generic, that’s that reason eventdev was not mentioned in the comments of this patch &
> mentioned only in the description.
it may be written as, "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"
>>> + 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 */
<snip>
>>> +
>>> +/**
>>> + * 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.
> This doesn’t fit into a single line or in the next line aligned with bracket.
void * should be in a separate line.
-Akhil
next prev parent reply other threads:[~2018-01-16 6:24 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
2018-01-16 6:09 ` Gujjar, Abhinandan S
2018-01-16 6:24 ` Akhil Goyal [this message]
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=f0232518-7576-ce8f-9c8f-65dfc978da8d@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).