[dpdk-dev] [PATCH] [RFC] cryptodev: crypto operation restructuring

Akhil Goyal akhil.goyal at nxp.com
Thu May 4 09:38:39 CEST 2017


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 at intel.com>
>>>>> ---
>>>>
>>>> Comments inline.
>>>>
>>>> Regards,
>>>> Akhil
>>>>
>>>>
>>>
>>>
>>
>>
>
>




More information about the dev mailing list