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 <[email protected]>
---
Comments inline.
Regards,
Akhil
