From: Sergio Gonzalez Monroy <sergio.gonzalez.monroy@intel.com>
To: Akhil Goyal <akhil.goyal@nxp.com>,
Pablo de Lara <pablo.de.lara.guarch@intel.com>,
dev@dpdk.org
Cc: declan.doherty@intel.com, hemant.agrawal@nxp.com,
zbigniew.bodek@caviumnetworks.com,
jerin.jacob@caviumnetworks.com
Subject: Re: [dpdk-dev] [PATCH] [RFC] cryptodev: crypto operation restructuring
Date: Wed, 3 May 2017 15:18:03 +0100 [thread overview]
Message-ID: <058e6ed7-9404-1333-31be-c389fe08d246@intel.com> (raw)
In-Reply-To: <46d732a5-3ab0-a63b-bd12-acc9372a3c57@nxp.com>
On 03/05/2017 12:01, Akhil Goyal wrote:
> Hi Pablo,
>
> On 4/28/2017 11:33 PM, Pablo de Lara wrote:
>> This is a proposal to correct and improve the current crypto
>> operation (rte_crypto_op)
>> and symmetric crypto operation (rte_crypto_sym_op) structures, shrinking
>> their sizes to fit both structures into two 64-byte cache lines as
>> one of the goals.
>>
>> The following changes are proposed:
>>
>> In rte_crypto_op:
>>
>> - Move session type (with session/sessionless) from symmetric op to
>> crypto op,
>> as this could be used for other types
>>
>> - Combine operation type, operation status and session type into a
>> 64-bit flag (each one taking 1 byte),
>> instead of having enums taking 4 bytes each
> [Akhil] wouldn't this be a problem? Bit fields create endianness
> issues. Can we have uint8_t for each of the field.
Sure, as it is proposed it would be the same as having 3 uint8_t fields.
The idea was to possibly compact those fields (ie. we do not need 8 bits
for sess_type) to make better use of the bits and add asym fields there
if needed.
I don't think bitfields would be a problem in this case. Agree, we
should not use both bitmask and bitfields, but we would use just bitfields.
Can you elaborate on the issue you see?
Regards,
Sergio
>>
>> - Remove opaque data from crypto operation, as private data can be
>> allocated
>> just after the symmetric (or other type) crypto operation
>>
>> - Modify symmetric operation pointer to zero-array, as the symmetric
>> op should be always after the crypto operation
>>
>> In rte_crypto_sym_xform:
>>
>> - Remove AAD length from sym_xform (will be taken from operation only)
>>
>> - Add IV length in sym_xform, so this length will be fixed for all
>> the operations in a session
> A much needed change. This would remove hard codings for iv length
> while configuring sessions.
>>
>> In rte_crypto_sym_op:
>>
>> - Separate IV from cipher structure in symmetric crypto operation, as
>> it is also used in authentication, for some algorithms
>>
>> - Remove IV pointer and length from sym crypto op, and leave just the
>> offset (from the beginning of the crypto operation),
>> as the IV can reside after the crypto operation
>>
>> - Create union with authentication data and AAD, as these two values
>> cannot be used at the same time
> [Akhil] Does this mean, in case of AEAD, additional authentication
> data and auth data are contiguous as we do not have explicit auth data
> offset here.
>>
>> - Remove digest length from sym crypto op, so this length will be
>> fixed for all the operations in a session
>>
>> - Add zero-array at the end of sym crypto op to be used to get extra
>> allocated memory (IV + other user data)
>>
>> Previous rte_crypto_op (40 bytes) and rte_crypto_sym_op (114 bytes)
>> structures:
>>
>> struct rte_crypto_op {
>> enum rte_crypto_op_type type;
>>
>> enum rte_crypto_op_status status;
>>
>> struct rte_mempool *mempool;
>>
>> phys_addr_t phys_addr;
>>
>> void *opaque_data;
>>
>> union {
>> struct rte_crypto_sym_op *sym;
>> };
>> } __rte_cache_aligned;
>>
>> struct rte_crypto_sym_op {
>> struct rte_mbuf *m_src;
>> struct rte_mbuf *m_dst;
>>
>> enum rte_crypto_sym_op_sess_type sess_type;
>>
>> RTE_STD_C11
>> union {
>> struct rte_cryptodev_sym_session *session;
>> struct rte_crypto_sym_xform *xform;
>> };
>>
>> struct {
>> struct {
>> uint32_t offset;
>> uint32_t length;
>> } data;
>>
>> struct {
>> uint8_t *data;
>> phys_addr_t phys_addr;
>> uint16_t length;
>> } iv;
>> } cipher;
>>
>> struct {
>> struct {
>> uint32_t offset;
>> uint32_t length;
>> } data;
>> struct {
>> uint8_t *data;
>> phys_addr_t phys_addr;
>> uint16_t length;
>> } digest; /**< Digest parameters */
>>
>> struct {
>> uint8_t *data;
>> phys_addr_t phys_addr;
>> uint16_t length;
>> } aad;
>>
>> } auth;
>> } __rte_cache_aligned;
>>
>> New rte_crypto_op (24 bytes) and rte_crypto_sym_op (72 bytes)
>> structures:
>>
>> struct rte_crypto_op {
>> uint64_t type: 8;
>> uint64_t status: 8;
>> uint64_t sess_type: 8;
>>
>> struct rte_mempool *mempool;
>>
>> phys_addr_t phys_addr;
>>
>> RTE_STD_C11
>> union {
>> struct rte_crypto_sym_op sym[0];
>> };
>> } __rte_cache_aligned;
>>
>> struct rte_crypto_sym_op {
>> struct rte_mbuf *m_src;
>> struct rte_mbuf *m_dst;
>>
>> RTE_STD_C11
>> union {
>> struct rte_cryptodev_sym_session *session;
>> struct rte_crypto_sym_xform *xform;
>> };
>>
>> struct {
>> uint8_t offset;
>> } iv;
>>
>> struct {
>> union {
>> struct {
>> uint32_t offset;
>> uint32_t length;
>> } data;
>> struct {
>> uint32_t length;
>> uint8_t *data;
>> phys_addr_t phys_addr;
>> } aad;
>> };
>>
>> struct {
>> uint8_t *data;
>> phys_addr_t phys_addr;
>> } digest;
>>
>> } auth;
>> struct {
>> struct {
>> uint32_t offset;
>> uint32_t length;
>> } data;
>>
>> } cipher;
>>
>> __extension__ char _private[0];
>> };
>>
>> Signed-off-by: Pablo de Lara <pablo.de.lara.guarch@intel.com>
>> ---
>
> Comments inline.
>
> Regards,
> Akhil
>
>
next prev parent reply other threads:[~2017-05-03 14:18 UTC|newest]
Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-04-28 18:03 Pablo de Lara
2017-05-03 11:01 ` Akhil Goyal
2017-05-03 14:18 ` Sergio Gonzalez Monroy [this message]
2017-05-04 6:09 ` Akhil Goyal
2017-05-04 7:31 ` Sergio Gonzalez Monroy
2017-05-04 7:38 ` Akhil Goyal
2017-05-04 8:19 ` Sergio Gonzalez Monroy
2017-05-04 11:23 ` Akhil Goyal
2017-05-04 16:20 ` De Lara Guarch, Pablo
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=058e6ed7-9404-1333-31be-c389fe08d246@intel.com \
--to=sergio.gonzalez.monroy@intel.com \
--cc=akhil.goyal@nxp.com \
--cc=declan.doherty@intel.com \
--cc=dev@dpdk.org \
--cc=hemant.agrawal@nxp.com \
--cc=jerin.jacob@caviumnetworks.com \
--cc=pablo.de.lara.guarch@intel.com \
--cc=zbigniew.bodek@caviumnetworks.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).