[dpdk-dev,RFC] cryptodev: crypto operation restructuring
Checks
Commit Message
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
- 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
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
- 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>
---
lib/librte_cryptodev/rte_crypto.h | 36 ++--
lib/librte_cryptodev/rte_crypto_sym.h | 375 ++++++++++++++--------------------
2 files changed, 176 insertions(+), 235 deletions(-)
Comments
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
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
>
>
Hi Sergio,
On 5/3/2017 7:48 PM, Sergio Gonzalez Monroy wrote:
> 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
>
The problem will come when we run on systems with different endianness.
The bit field positioning will be different for LE and BE.
It would be like in LE
uint64_t type:8;
uint64_t status:8;
uint64_t sess_type:8;
uint64_t reserved:40;
and on BE it would be
uint64_t reserved:40;
uint64_t sess_type:8;
uint64_t status:8;
uint64_t type:8;
So it would be better to use 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
>>
>>
>
>
On 04/05/2017 07:09, Akhil Goyal wrote:
> Hi Sergio,
>
> On 5/3/2017 7:48 PM, Sergio Gonzalez Monroy wrote:
>> 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
>>
>
> The problem will come when we run on systems with different
> endianness. The bit field positioning will be different for LE and BE.
> It would be like in LE
> uint64_t type:8;
> uint64_t status:8;
> uint64_t sess_type:8;
> uint64_t reserved:40;
>
> and on BE it would be
> uint64_t reserved:40;
> uint64_t sess_type:8;
> uint64_t status:8;
> uint64_t type:8;
>
> So it would be better to use uint8_t for each of the field.
Understood, but why is that an issue? Those fields are used by
application code and PMD, same system.
Do you have a use case where you are offloading crypto ops to a
different arch/system?
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
>>>
>>>
>>
>>
>
>
On 5/4/2017 1:01 PM, Sergio Gonzalez Monroy wrote:
> On 04/05/2017 07:09, Akhil Goyal wrote:
>> Hi Sergio,
>>
>> On 5/3/2017 7:48 PM, Sergio Gonzalez Monroy wrote:
>>> 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
>>>
>>
>> The problem will come when we run on systems with different
>> endianness. The bit field positioning will be different for LE and BE.
>> It would be like in LE
>> uint64_t type:8;
>> uint64_t status:8;
>> uint64_t sess_type:8;
>> uint64_t reserved:40;
>>
>> and on BE it would be
>> uint64_t reserved:40;
>> uint64_t sess_type:8;
>> uint64_t status:8;
>> uint64_t type:8;
>>
>> So it would be better to use uint8_t for each of the field.
>
> Understood, but why is that an issue? Those fields are used by
> application code and PMD, same system.
> Do you have a use case where you are offloading crypto ops to a
> different arch/system?
>
> Sergio
same application may run on LE or BE machines. So if we use masks for
accessing these fields and take the complete field as uint64_t, then LE
and BE machine would interpret it differently as the code is same.
Akhil
>
>>
>>>>>
>>>>> - 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
>>>>
>>>>
>>>
>>>
>>
>>
>
>
On 04/05/2017 08:38, Akhil Goyal wrote:
> On 5/4/2017 1:01 PM, Sergio Gonzalez Monroy wrote:
>> On 04/05/2017 07:09, Akhil Goyal wrote:
>>> Hi Sergio,
>>>
>>> On 5/3/2017 7:48 PM, Sergio Gonzalez Monroy wrote:
>>>> 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
>>>>
>>>
>>> The problem will come when we run on systems with different
>>> endianness. The bit field positioning will be different for LE and BE.
>>> It would be like in LE
>>> uint64_t type:8;
>>> uint64_t status:8;
>>> uint64_t sess_type:8;
>>> uint64_t reserved:40;
>>>
>>> and on BE it would be
>>> uint64_t reserved:40;
>>> uint64_t sess_type:8;
>>> uint64_t status:8;
>>> uint64_t type:8;
>>>
>>> So it would be better to use uint8_t for each of the field.
>>
>> Understood, but why is that an issue? Those fields are used by
>> application code and PMD, same system.
>> Do you have a use case where you are offloading crypto ops to a
>> different arch/system?
>>
>> Sergio
> same application may run on LE or BE machines. So if we use masks for
> accessing these fields and take the complete field as uint64_t, then
> LE and BE machine would interpret it differently as the code is same.
>
Right, either you use bitfields or you define macros and work on the
complete uint64_t. The proposal here was to just use bitfields and for
the given use case it would not be an issue while IMHO allowing better
packing and readability than defining each field as a Byte.
Anyway, if you feel strongly against bitfields, we can just define it as
uint8_t as you suggest or single uint64_t with macros.
Sergio
> Akhil
>>
>>>
>>>>>>
>>>>>> - 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
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
On 5/4/2017 1:49 PM, Sergio Gonzalez Monroy wrote:
> On 04/05/2017 08:38, Akhil Goyal wrote:
>> On 5/4/2017 1:01 PM, Sergio Gonzalez Monroy wrote:
>>> On 04/05/2017 07:09, Akhil Goyal wrote:
>>>> Hi Sergio,
>>>>
>>>> On 5/3/2017 7:48 PM, Sergio Gonzalez Monroy wrote:
>>>>> 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
>>>>>
>>>>
>>>> The problem will come when we run on systems with different
>>>> endianness. The bit field positioning will be different for LE and BE.
>>>> It would be like in LE
>>>> uint64_t type:8;
>>>> uint64_t status:8;
>>>> uint64_t sess_type:8;
>>>> uint64_t reserved:40;
>>>>
>>>> and on BE it would be
>>>> uint64_t reserved:40;
>>>> uint64_t sess_type:8;
>>>> uint64_t status:8;
>>>> uint64_t type:8;
>>>>
>>>> So it would be better to use uint8_t for each of the field.
>>>
>>> Understood, but why is that an issue? Those fields are used by
>>> application code and PMD, same system.
>>> Do you have a use case where you are offloading crypto ops to a
>>> different arch/system?
>>>
>>> Sergio
>> same application may run on LE or BE machines. So if we use masks for
>> accessing these fields and take the complete field as uint64_t, then
>> LE and BE machine would interpret it differently as the code is same.
>>
>
> Right, either you use bitfields or you define macros and work on the
> complete uint64_t. The proposal here was to just use bitfields and for
> the given use case it would not be an issue while IMHO allowing better
> packing and readability than defining each field as a Byte.
>
> Anyway, if you feel strongly against bitfields, we can just define it as
> uint8_t as you suggest or single uint64_t with macros.
>
> Sergio
>
I am not against bitfields. But we should take care of the endianness
using the compile time flags for byte ordering or we can use the uint8_t.
I am ok with both.
Thanks,
Akhil
>> Akhil
>>>
>>>>
>>>>>>>
>>>>>>> - 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.
Pablo/Sergio,
Is my understanding correct here?
Akhil
>>>>>>>
>>>>>>> - 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
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>
>>>>
>>>
>>>
>>
>>
>
>
> -----Original Message-----
> From: Akhil Goyal [mailto:akhil.goyal@nxp.com]
> Sent: Thursday, May 04, 2017 12:23 PM
> To: Gonzalez Monroy, Sergio; De Lara Guarch, Pablo; dev@dpdk.org
> Cc: Doherty, Declan; hemant.agrawal@nxp.com;
> zbigniew.bodek@caviumnetworks.com; jerin.jacob@caviumnetworks.com
> Subject: Re: [PATCH] [RFC] cryptodev: crypto operation restructuring
>
> On 5/4/2017 1:49 PM, Sergio Gonzalez Monroy wrote:
> > On 04/05/2017 08:38, Akhil Goyal wrote:
> >> On 5/4/2017 1:01 PM, Sergio Gonzalez Monroy wrote:
> >>> On 04/05/2017 07:09, Akhil Goyal wrote:
> >>>> Hi Sergio,
> >>>>
> >>>> On 5/3/2017 7:48 PM, Sergio Gonzalez Monroy wrote:
> >>>>> 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
> >>>>>
> >>>>
> >>>> The problem will come when we run on systems with different
> >>>> endianness. The bit field positioning will be different for LE and BE.
> >>>> It would be like in LE
> >>>> uint64_t type:8;
> >>>> uint64_t status:8;
> >>>> uint64_t sess_type:8;
> >>>> uint64_t reserved:40;
> >>>>
> >>>> and on BE it would be
> >>>> uint64_t reserved:40;
> >>>> uint64_t sess_type:8;
> >>>> uint64_t status:8;
> >>>> uint64_t type:8;
> >>>>
> >>>> So it would be better to use uint8_t for each of the field.
> >>>
> >>> Understood, but why is that an issue? Those fields are used by
> >>> application code and PMD, same system.
> >>> Do you have a use case where you are offloading crypto ops to a
> >>> different arch/system?
> >>>
> >>> Sergio
> >> same application may run on LE or BE machines. So if we use masks for
> >> accessing these fields and take the complete field as uint64_t, then
> >> LE and BE machine would interpret it differently as the code is same.
> >>
> >
> > Right, either you use bitfields or you define macros and work on the
> > complete uint64_t. The proposal here was to just use bitfields and for
> > the given use case it would not be an issue while IMHO allowing better
> > packing and readability than defining each field as a Byte.
> >
> > Anyway, if you feel strongly against bitfields, we can just define it as
> > uint8_t as you suggest or single uint64_t with macros.
> >
> > Sergio
> >
>
> I am not against bitfields. But we should take care of the endianness
> using the compile time flags for byte ordering or we can use the uint8_t.
> I am ok with both.
>
> Thanks,
> Akhil
>
> >> Akhil
> >>>
> >>>>
> >>>>>>>
> >>>>>>> - 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.
>
> Pablo/Sergio,
>
> Is my understanding correct here?
Hi Akhil,
Thanks for your comments!
No, AAD can be anywhere, as we are passing a pointer to the data:
union {
struct {
uint32_t offset;
uint32_t length;
} data;
struct {
uint32_t length;
uint8_t *data;
phys_addr_t phys_addr;
} aad;
};
We don't need auth data offset/length, because data to authenticate and cipher
is passed on cipher structure, plus the aad structure for additional data to authenticate.
Another proposal would be creating an structure in that crypto operation for AEAD algorithms,
and have the union with that structure and the other two structures (auth and cipher).
In this case, it is more explicit that AAD cannot be used in authentication algorithms such as SHA1.
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 {
uint32_t offset;
} iv;
union {
struct {
struct {
uint32_t offset;
uint32_t length;
} data;
/**< Data offset and length for encryption/authentication */
struct {
uint32_t length;
uint8_t *data;
phys_addr_t phys_addr;
} aad;
/**< Additional authentication parameters */
struct {
uint8_t *data;
phys_addr_t phys_addr;
} digest;
/**< Digest parameters (length is fixed in session) */
} aead;
struct {
struct {
struct {
uint32_t offset;
uint32_t length;
} data;
/**< Data offset and length for authentication */
struct {
uint8_t *data;
phys_addr_t phys_addr;
} digest;
/**< Digest parameters (length is fixed in session) */
} auth;
struct {
struct {
uint32_t offset;
uint32_t length;
} data;
/**< Data offset and length for ciphering */
} cipher;
};
};
__extension__ char _private[0];
/**< Private crypto operation material */
};
Thanks,
Pablo
@@ -82,6 +82,16 @@ enum rte_crypto_op_status {
};
/**
+ * Crypto operation session type. This is used to specify whether a crypto
+ * operation has session structure attached for immutable parameters or if all
+ * operation information is included in the operation data structure.
+ */
+enum rte_crypto_op_sess_type {
+ RTE_CRYPTO_OP_WITH_SESSION, /**< Session based crypto operation */
+ RTE_CRYPTO_OP_SESSIONLESS /**< Session-less crypto operation */
+};
+
+/**
* Cryptographic Operation.
*
* This structure contains data relating to performing cryptographic
@@ -92,31 +102,28 @@ enum rte_crypto_op_status {
* rte_cryptodev_enqueue_burst() / rte_cryptodev_dequeue_burst() .
*/
struct rte_crypto_op {
- enum rte_crypto_op_type type;
+ uint64_t type: 8;
/**< operation type */
-
- enum rte_crypto_op_status status;
+ uint64_t status: 8;
/**<
* operation status - this is reset to
* RTE_CRYPTO_OP_STATUS_NOT_PROCESSED on allocation from mempool and
* will be set to RTE_CRYPTO_OP_STATUS_SUCCESS after crypto operation
* is successfully processed by a crypto PMD
*/
-
+ uint64_t sess_type: 8;
+ /**< operation session type */
struct rte_mempool *mempool;
/**< crypto operation mempool which operation is allocated from */
phys_addr_t phys_addr;
/**< physical address of crypto operation */
- void *opaque_data;
- /**< Opaque pointer for user data */
-
RTE_STD_C11
union {
- struct rte_crypto_sym_op *sym;
+ struct rte_crypto_sym_op sym[0];
/**< Symmetric operation parameters */
- }; /**< operation specific parameters */
+ } /**< operation specific parameters */ __rte_aligned(8);
} __rte_cache_aligned;
/**
@@ -130,22 +137,15 @@ __rte_crypto_op_reset(struct rte_crypto_op *op, enum rte_crypto_op_type type)
{
op->type = type;
op->status = RTE_CRYPTO_OP_STATUS_NOT_PROCESSED;
+ op->sess_type = RTE_CRYPTO_OP_SESSIONLESS;
switch (type) {
case RTE_CRYPTO_OP_TYPE_SYMMETRIC:
- /** Symmetric operation structure starts after the end of the
- * rte_crypto_op structure.
- */
- op->sym = (struct rte_crypto_sym_op *)(op + 1);
- op->type = type;
-
__rte_crypto_sym_op_reset(op->sym);
break;
default:
break;
}
-
- op->opaque_data = NULL;
}
/**
@@ -407,6 +407,8 @@ rte_crypto_op_attach_sym_session(struct rte_crypto_op *op,
if (unlikely(op->type != RTE_CRYPTO_OP_TYPE_SYMMETRIC))
return -1;
+ op->sess_type = RTE_CRYPTO_OP_WITH_SESSION;
+
return __rte_crypto_sym_op_attach_sym_session(op->sym, sess);
}
@@ -316,35 +316,6 @@ struct rte_crypto_auth_xform {
* by *rte_cryptodev_sym_session_create* or by the
* *rte_cryptodev_sym_enqueue_burst* if using session-less APIs.
*/
-
- uint32_t add_auth_data_length;
- /**< The length of the additional authenticated data (AAD) in bytes.
- * The maximum permitted value is 240 bytes, unless otherwise specified
- * below.
- *
- * This field must be specified when the hash algorithm is one of the
- * following:
- *
- * - For SNOW 3G (@ref RTE_CRYPTO_AUTH_SNOW3G_UIA2), this is the
- * length of the IV (which should be 16).
- *
- * - For GCM (@ref RTE_CRYPTO_AUTH_AES_GCM). In this case, this is
- * the length of the Additional Authenticated Data (called A, in NIST
- * SP800-38D).
- *
- * - For CCM (@ref RTE_CRYPTO_AUTH_AES_CCM). In this case, this is
- * the length of the associated data (called A, in NIST SP800-38C).
- * Note that this does NOT include the length of any padding, or the
- * 18 bytes reserved at the start of the above field to store the
- * block B0 and the encoded length. The maximum permitted value in
- * this case is 222 bytes.
- *
- * @note
- * For AES-GMAC (@ref RTE_CRYPTO_AUTH_AES_GMAC) mode of operation
- * this field is not used and should be set to 0. Instead the length
- * of the AAD data is specified in additional authentication data
- * length field of the rte_crypto_sym_op_data structure
- */
};
/** Crypto transformation types */
@@ -366,8 +337,8 @@ enum rte_crypto_sym_xform_type {
struct rte_crypto_sym_xform {
struct rte_crypto_sym_xform *next;
/**< next xform in chain */
- enum rte_crypto_sym_xform_type type
- ; /**< xform type */
+ enum rte_crypto_sym_xform_type type;
+ /**< xform type */
RTE_STD_C11
union {
struct rte_crypto_auth_xform auth;
@@ -375,19 +346,9 @@ struct rte_crypto_sym_xform {
struct rte_crypto_cipher_xform cipher;
/**< Cipher xform */
};
+ uint32_t iv_length;
};
-/**
- * Crypto operation session type. This is used to specify whether a crypto
- * operation has session structure attached for immutable parameters or if all
- * operation information is included in the operation data structure.
- */
-enum rte_crypto_sym_op_sess_type {
- RTE_CRYPTO_SYM_OP_WITH_SESSION, /**< Session based crypto operation */
- RTE_CRYPTO_SYM_OP_SESSIONLESS /**< Session-less crypto operation */
-};
-
-
struct rte_cryptodev_sym_session;
/**
@@ -424,8 +385,6 @@ struct rte_crypto_sym_op {
struct rte_mbuf *m_src; /**< source mbuf */
struct rte_mbuf *m_dst; /**< destination mbuf */
- enum rte_crypto_sym_op_sess_type sess_type;
-
RTE_STD_C11
union {
struct rte_cryptodev_sym_session *session;
@@ -435,6 +394,157 @@ struct rte_crypto_sym_op {
};
struct {
+ uint8_t offset;
+ /**< Initialisation Vector or Counter.
+ *
+ * - For block ciphers in CBC or F8 mode, or for KASUMI
+ * in F8 mode, or for SNOW 3G in UEA2 mode, this is the
+ * Initialisation Vector (IV) value.
+ *
+ * - For block ciphers in CTR mode, this is the counter.
+ *
+ * - For GCM mode, this is either the IV (if the length
+ * is 96 bits) or J0 (for other sizes), where J0 is as
+ * defined by NIST SP800-38D. Regardless of the IV
+ * length, a full 16 bytes needs to be allocated.
+ *
+ * - For CCM mode, the first byte is reserved, and the
+ * nonce should be written starting at &iv[1] (to allow
+ * space for the implementation to write in the flags
+ * in the first byte). Note that a full 16 bytes should
+ * be allocated, even though the length field will
+ * have a value less than this.
+ *
+ * - For AES-XTS, this is the 128bit tweak, i, from
+ * IEEE Std 1619-2007.
+ *
+ * This offset is in bytes from the start of the crypto op.
+ * For optimum performance, the data pointed to SHOULD
+ * be 8-byte aligned.
+ */
+ } iv; /**< Initialisation vector parameters (length is fixed in session) */
+
+ struct {
+ union {
+ struct {
+ uint32_t offset;
+ /**< Starting point for hash processing, specified as
+ * number of bytes from start of packet in source
+ * buffer.
+ *
+ * @note
+ * For CCM and GCM modes of operation, this field is
+ * ignored. The field @ref aad field
+ * should be set instead.
+ *
+ * @note For AES-GMAC (@ref RTE_CRYPTO_AUTH_AES_GMAC)
+ * mode of operation, this field is set to 0. aad data
+ * pointer of rte_crypto_sym_op_data structure is
+ * used instead
+ *
+ * @note
+ * For SNOW 3G @ RTE_CRYPTO_AUTH_SNOW3G_UIA2,
+ * KASUMI @ RTE_CRYPTO_AUTH_KASUMI_F9
+ * and ZUC @ RTE_CRYPTO_AUTH_ZUC_EIA3,
+ * this field should be in bits.
+ */
+
+ uint32_t length;
+ /**< The message length, in bytes, of the source
+ * buffer that the hash will be computed on.
+ *
+ * @note
+ * For CCM and GCM modes of operation, this field is
+ * ignored. The field @ref aad field should be set
+ * instead.
+ *
+ * @note
+ * For AES-GMAC @ref RTE_CRYPTO_AUTH_AES_GMAC mode
+ * of operation, this field is set to 0.
+ * Auth.aad.length is used instead.
+ *
+ * @note
+ * For SNOW 3G @ RTE_CRYPTO_AUTH_SNOW3G_UIA2,
+ * KASUMI @ RTE_CRYPTO_AUTH_KASUMI_F9
+ * and ZUC @ RTE_CRYPTO_AUTH_ZUC_EIA3,
+ * this field should be in bits.
+ */
+ } data; /**< Data offsets and length for authentication */
+ struct {
+ uint32_t length; /**< Length of AAD */
+ uint8_t *data;
+ /**< Pointer to Additional Authenticated Data (AAD)
+ * needed for authenticated cipher mechanisms (CCM and
+ * GCM)
+ *
+ * The length of the data pointed to by this field is
+ * set up for the session in the @ref
+ * rte_crypto_auth_xform structure as part of the @ref
+ * rte_cryptodev_sym_session_create function call.
+ * This length must not exceed 240 bytes.
+ *
+ * Specifically for CCM (@ref RTE_CRYPTO_AUTH_AES_CCM),
+ * the caller should setup this field as follows:
+ *
+ * - the nonce should be written starting at an offset
+ * of one byte into the array, leaving room for the
+ * implementation to write in the flags to the first
+ * byte.
+ *
+ * - the additional authentication data itself should
+ * be written starting at an offset of 18 bytes into
+ * the array, leaving room for the length encoding in
+ * the first two bytes of the second block.
+ *
+ * - the array should be big enough to hold the above
+ * fields, plus any padding to round this up to the
+ * nearest multiple of the block size (16 bytes).
+ * Padding will be added by the implementation.
+ *
+ * Finally, for GCM (@ref RTE_CRYPTO_AUTH_AES_GCM), the
+ * caller should setup this field as follows:
+ *
+ * - the AAD is written in starting at byte 0
+ * - the array must be big enough to hold the AAD, plus
+ * any space to round this up to the nearest multiple
+ * of the block size (16 bytes).
+ *
+ * @note
+ * For AES-GMAC (@ref RTE_CRYPTO_AUTH_AES_GMAC) mode of
+ * operation, this field is used to pass plaintext.
+ */
+ phys_addr_t phys_addr; /**< physical address */
+ } aad;
+ /**< Additional authentication parameters (for AEAD operations only) */
+ };
+
+ struct {
+ uint8_t *data;
+ /**< This points to the location where the digest result
+ * should be inserted (in the case of digest generation)
+ * or where the purported digest exists (in the case of
+ * digest verification).
+ *
+ * At session creation time, the client specified the
+ * digest result length with the digest_length member
+ * of the @ref rte_crypto_auth_xform structure. For
+ * physical crypto devices the caller must allocate at
+ * least digest_length of physically contiguous memory
+ * at this location.
+ *
+ * For digest generation, the digest result will
+ * overwrite any data at this location.
+ *
+ * @note
+ * For GCM (@ref RTE_CRYPTO_AUTH_AES_GCM), for
+ * "digest result" read "authentication tag T".
+ */
+ phys_addr_t phys_addr;
+ /**< Physical address of digest */
+ } digest; /**< Digest parameters (length is fixed in session) */
+
+ } auth;
+ struct {
struct {
uint32_t offset;
/**< Starting point for cipher processing, specified
@@ -477,179 +587,11 @@ struct rte_crypto_sym_op {
*/
} data; /**< Data offsets and length for ciphering */
- struct {
- uint8_t *data;
- /**< Initialisation Vector or Counter.
- *
- * - For block ciphers in CBC or F8 mode, or for KASUMI
- * in F8 mode, or for SNOW 3G in UEA2 mode, this is the
- * Initialisation Vector (IV) value.
- *
- * - For block ciphers in CTR mode, this is the counter.
- *
- * - For GCM mode, this is either the IV (if the length
- * is 96 bits) or J0 (for other sizes), where J0 is as
- * defined by NIST SP800-38D. Regardless of the IV
- * length, a full 16 bytes needs to be allocated.
- *
- * - For CCM mode, the first byte is reserved, and the
- * nonce should be written starting at &iv[1] (to allow
- * space for the implementation to write in the flags
- * in the first byte). Note that a full 16 bytes should
- * be allocated, even though the length field will
- * have a value less than this.
- *
- * - For AES-XTS, this is the 128bit tweak, i, from
- * IEEE Std 1619-2007.
- *
- * For optimum performance, the data pointed to SHOULD
- * be 8-byte aligned.
- */
- phys_addr_t phys_addr;
- uint16_t length;
- /**< Length of valid IV data.
- *
- * - For block ciphers in CBC or F8 mode, or for KASUMI
- * in F8 mode, or for SNOW 3G in UEA2 mode, this is the
- * length of the IV (which must be the same as the
- * block length of the cipher).
- *
- * - For block ciphers in CTR mode, this is the length
- * of the counter (which must be the same as the block
- * length of the cipher).
- *
- * - For GCM mode, this is either 12 (for 96-bit IVs)
- * or 16, in which case data points to J0.
- *
- * - For CCM mode, this is the length of the nonce,
- * which can be in the range 7 to 13 inclusive.
- */
- } iv; /**< Initialisation vector parameters */
} cipher;
- struct {
- struct {
- uint32_t offset;
- /**< Starting point for hash processing, specified as
- * number of bytes from start of packet in source
- * buffer.
- *
- * @note
- * For CCM and GCM modes of operation, this field is
- * ignored. The field @ref aad field
- * should be set instead.
- *
- * @note For AES-GMAC (@ref RTE_CRYPTO_AUTH_AES_GMAC)
- * mode of operation, this field is set to 0. aad data
- * pointer of rte_crypto_sym_op_data structure is
- * used instead
- *
- * @note
- * For SNOW 3G @ RTE_CRYPTO_AUTH_SNOW3G_UIA2,
- * KASUMI @ RTE_CRYPTO_AUTH_KASUMI_F9
- * and ZUC @ RTE_CRYPTO_AUTH_ZUC_EIA3,
- * this field should be in bits.
- */
-
- uint32_t length;
- /**< The message length, in bytes, of the source
- * buffer that the hash will be computed on.
- *
- * @note
- * For CCM and GCM modes of operation, this field is
- * ignored. The field @ref aad field should be set
- * instead.
- *
- * @note
- * For AES-GMAC @ref RTE_CRYPTO_AUTH_AES_GMAC mode
- * of operation, this field is set to 0.
- * Auth.aad.length is used instead.
- *
- * @note
- * For SNOW 3G @ RTE_CRYPTO_AUTH_SNOW3G_UIA2,
- * KASUMI @ RTE_CRYPTO_AUTH_KASUMI_F9
- * and ZUC @ RTE_CRYPTO_AUTH_ZUC_EIA3,
- * this field should be in bits.
- */
- } data; /**< Data offsets and length for authentication */
-
- struct {
- uint8_t *data;
- /**< This points to the location where the digest result
- * should be inserted (in the case of digest generation)
- * or where the purported digest exists (in the case of
- * digest verification).
- *
- * At session creation time, the client specified the
- * digest result length with the digest_length member
- * of the @ref rte_crypto_auth_xform structure. For
- * physical crypto devices the caller must allocate at
- * least digest_length of physically contiguous memory
- * at this location.
- *
- * For digest generation, the digest result will
- * overwrite any data at this location.
- *
- * @note
- * For GCM (@ref RTE_CRYPTO_AUTH_AES_GCM), for
- * "digest result" read "authentication tag T".
- */
- phys_addr_t phys_addr;
- /**< Physical address of digest */
- uint16_t length;
- /**< Length of digest */
- } digest; /**< Digest parameters */
-
- struct {
- uint8_t *data;
- /**< Pointer to Additional Authenticated Data (AAD)
- * needed for authenticated cipher mechanisms (CCM and
- * GCM), and to the IV for SNOW 3G authentication
- * (@ref RTE_CRYPTO_AUTH_SNOW3G_UIA2). For other
- * authentication mechanisms this pointer is ignored.
- *
- * The length of the data pointed to by this field is
- * set up for the session in the @ref
- * rte_crypto_auth_xform structure as part of the @ref
- * rte_cryptodev_sym_session_create function call.
- * This length must not exceed 240 bytes.
- *
- * Specifically for CCM (@ref RTE_CRYPTO_AUTH_AES_CCM),
- * the caller should setup this field as follows:
- *
- * - the nonce should be written starting at an offset
- * of one byte into the array, leaving room for the
- * implementation to write in the flags to the first
- * byte.
- *
- * - the additional authentication data itself should
- * be written starting at an offset of 18 bytes into
- * the array, leaving room for the length encoding in
- * the first two bytes of the second block.
- *
- * - the array should be big enough to hold the above
- * fields, plus any padding to round this up to the
- * nearest multiple of the block size (16 bytes).
- * Padding will be added by the implementation.
- *
- * Finally, for GCM (@ref RTE_CRYPTO_AUTH_AES_GCM), the
- * caller should setup this field as follows:
- *
- * - the AAD is written in starting at byte 0
- * - the array must be big enough to hold the AAD, plus
- * any space to round this up to the nearest multiple
- * of the block size (16 bytes).
- *
- * @note
- * For AES-GMAC (@ref RTE_CRYPTO_AUTH_AES_GMAC) mode of
- * operation, this field is used to pass plaintext.
- */
- phys_addr_t phys_addr; /**< physical address */
- uint16_t length; /**< Length of digest */
- } aad;
- /**< Additional authentication parameters */
- } auth;
-} __rte_cache_aligned;
+ __extension__ char _private[0];
+ /**< Private crypto operation material */
+};
/**
@@ -661,8 +603,6 @@ static inline void
__rte_crypto_sym_op_reset(struct rte_crypto_sym_op *op)
{
memset(op, 0, sizeof(*op));
-
- op->sess_type = RTE_CRYPTO_SYM_OP_SESSIONLESS;
}
@@ -704,7 +644,6 @@ __rte_crypto_sym_op_attach_sym_session(struct rte_crypto_sym_op *sym_op,
struct rte_cryptodev_sym_session *sess)
{
sym_op->session = sess;
- sym_op->sess_type = RTE_CRYPTO_SYM_OP_WITH_SESSION;
return 0;
}