DPDK patches and discussions
 help / color / mirror / Atom feed
From: Akhil Goyal <akhil.goyal@nxp.com>
To: 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 16:31:50 +0530	[thread overview]
Message-ID: <46d732a5-3ab0-a63b-bd12-acc9372a3c57@nxp.com> (raw)
In-Reply-To: <1493402588-163123-1-git-send-email-pablo.de.lara.guarch@intel.com>

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

  reply	other threads:[~2017-05-03 11:01 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 [this message]
2017-05-03 14:18   ` Sergio Gonzalez Monroy
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=46d732a5-3ab0-a63b-bd12-acc9372a3c57@nxp.com \
    --to=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).