From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 905915A6A for ; Wed, 3 May 2017 16:18:07 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by orsmga105.jf.intel.com with ESMTP; 03 May 2017 07:18:06 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.38,284,1491289200"; d="scan'208";a="1143136852" Received: from smonroyx-mobl.ger.corp.intel.com (HELO [10.237.221.113]) ([10.237.221.113]) by fmsmga001.fm.intel.com with ESMTP; 03 May 2017 07:18:03 -0700 To: Akhil Goyal , Pablo de Lara , dev@dpdk.org References: <1493402588-163123-1-git-send-email-pablo.de.lara.guarch@intel.com> <46d732a5-3ab0-a63b-bd12-acc9372a3c57@nxp.com> Cc: declan.doherty@intel.com, hemant.agrawal@nxp.com, zbigniew.bodek@caviumnetworks.com, jerin.jacob@caviumnetworks.com From: Sergio Gonzalez Monroy Message-ID: <058e6ed7-9404-1333-31be-c389fe08d246@intel.com> Date: Wed, 3 May 2017 15:18:03 +0100 User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:45.0) Gecko/20100101 Thunderbird/45.1.1 MIME-Version: 1.0 In-Reply-To: <46d732a5-3ab0-a63b-bd12-acc9372a3c57@nxp.com> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] [RFC] cryptodev: crypto operation restructuring X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 03 May 2017 14:18:09 -0000 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 >> --- > > Comments inline. > > Regards, > Akhil > >