On Tue, Aug 29, 2017 at 5:02 PM, Savolainen, Petri (Nokia - FI/Espoo)
<[email protected]> wrote:
>
>
>> -----Original Message-----
>> From: shally verma [mailto:[email protected]]
>> Sent: Tuesday, August 29, 2017 10:26 AM
>> To: Narayana Prasad Athreya <[email protected]>
>> Cc: Savolainen, Petri (Nokia - FI/Espoo) <[email protected]>;
>> Github ODP bot <[email protected]>; [email protected]; Narayana,
>> Prasad Athreya <[email protected]>; Mahipal Challa
>> <[email protected]>; Verma, Shally <[email protected]>
>> Subject: Re: [lng-odp] [PATCH API-NEXT v8 1/1] comp: compression spec
>>
>> Based on last discussion, I was reworking to add odp_comp_op_pkt ()
>> API based on Crypto design. Help me to answer with these questions:
>>
>> 1. Current crypto packet base API is not giving Error code as an
>> output to its sync version i.e. in int odp_crypto_op(const
>> odp_packet_t pkt_in[],......), I do not see where it is returning
>> odp_crypto_packet_result_t. Can anyone help?
>
> Error codes are part of operation results:
>
> /**
> * Get crypto operation results from an crypto processed packet
> *
> * Successful crypto operations of all types (SYNC and ASYNC) produce packets
> * which contain crypto result metadata. This function copies the operation
> * results from an crypto processed packet. Event subtype of this kind of
> * packet is ODP_EVENT_PACKET_crypto. Results are undefined if a non-crypto
> * processed packet is passed as input.
> *
> * @param packet An crypto processed packet (ODP_EVENT_PACKET_CRYPTO)
> * @param[out] result Pointer to operation result for output
> *
> * @retval 0 On success
> * @retval <0 On failure
> */
> int odp_crypto_result(odp_crypto_packet_result_t *result,
> odp_packet_t packet);
Ok. That seems user need to make explicit call to this API to get
result, if he want.
So this is optional call in crypto context?
>
> /**
> * Crypto packet API operation result
> */
> typedef struct odp_crypto_packet_result_t {
> /** Request completed successfully */
> odp_bool_t ok;
>
> /** Cipher status */
> odp_crypto_op_status_t cipher_status;
>
> /** Authentication status */
> odp_crypto_op_status_t auth_status;
>
> } odp_crypto_packet_result_t;
>
> /**
> * Cryto API per packet operation completion status
> */
> typedef struct odp_crypto_op_status {
> /** Algorithm specific return code */
> odp_crypto_alg_err_t alg_err;
>
> /** Hardware specific return code */
> odp_crypto_hw_err_t hw_err;
>
> } odp_crypto_op_status_t;
>
> /**
> * Crypto API algorithm return code
> */
> typedef enum {
> /** Algorithm successful */
> ODP_CRYPTO_ALG_ERR_NONE,
> /** Invalid data block size */
> ODP_CRYPTO_ALG_ERR_DATA_SIZE,
> /** Key size invalid for algorithm */
> ODP_CRYPTO_ALG_ERR_KEY_SIZE,
> /** Computed ICV value mismatch */
> ODP_CRYPTO_ALG_ERR_ICV_CHECK,
> /** IV value not specified */
> ODP_CRYPTO_ALG_ERR_IV_INVALID,
> } odp_crypto_alg_err_t;
>
>
>>
>> 2. Current crypto version of odp_crypto_op(odp_pkt_t pkt_in[] ...)
>> does not have two separate version for encryption and decryption where
>> as in Compression, we added two. One for compress and another for
>> decompress.
>> So do we want to retain two separate flavor or unify like crypto
>> packet based api? Ex.
>> odp_comp_op_pkt ( ... ) OR
>> odp_comp_compress_pkt( ...),
>> odp_comp_decompress_pkt(),
>> odp_comp_compress_pkt_enq() and so on...?
>
> Crypto has single operation, IPSEC has two operations (inbound vs outbound).
> So, both styles are used today. Benefits of an operation per direction are:
> * more readable code: odp_comp_compress() vs odp_comp_op()
> * possibility to have different set of arguments (parameters) for each
> direction. E.g. IPSEC does IP fragmentation on output direction and thus
> needs extra parameters for that, those params are not defined on inbound
> direction.
> * cleaner specification of different operations e.g. "... output of
> odp_comp_compress()..." vs "... output of odp_comp_op() in compress mode ...."
> * easier to extend since a new feature can be added to one side without
> changing the spec for the other side
>
>
> BTW, since most of our operations process packets, we don't need to highlight
> it with "pkt". I'd name odp_comp_compress() for packets, and then later on
> add odp_comp_compress_mem(), odp_comp_compress_from_mem(), etc for mem ->
> mem, pkt -> mem operations.
Ok. no issues. I will retain separate flavor and keep API name in sync
to crypto.
Thanks
Shally
>
> -Petri