On Fri, Apr 14, 2017 at 6:53 AM, Bala Manoharan
<bala.manoha...@linaro.org> wrote:
> Regards,
> Bala
>
>
> On 14 April 2017 at 16:52, Bill Fischofer <bill.fischo...@linaro.org> wrote:
>> On Fri, Apr 14, 2017 at 5:58 AM, Dmitry Eremin-Solenikov
>> <dmitry.ereminsoleni...@linaro.org> wrote:
>>> Instead of having magic 0-1-2 numbers, let's have the special enum for
>>> feature support levels (unsupported/supported/preferred).
>>>
>>> Signed-off-by: Dmitry Eremin-Solenikov <dmitry.ereminsoleni...@linaro.org>
>>> ---
>>> include/odp/api/spec/ipsec.h | 56
>>> +++++++++++++++++++++++---------------------
>>> 1 file changed, 29 insertions(+), 27 deletions(-)
>>>
>>> diff --git a/include/odp/api/spec/ipsec.h b/include/odp/api/spec/ipsec.h
>>> index a0ceb11a..7011e3cf 100644
>>> --- a/include/odp/api/spec/ipsec.h
>>> +++ b/include/odp/api/spec/ipsec.h
>>> @@ -224,44 +224,46 @@ typedef struct odp_ipsec_outbound_config_t {
>>> } odp_ipsec_outbound_config_t;
>>>
>>> /**
>>> + * IPSEC operation mode support
>>> + */
>>> +typedef enum odp_ipsec_op_mode_support_t {
>>> + /**
>>> + * Mode is not supported
>>> + */
>>> + ODP_IPSEC_OP_MODE_UNSUPPORTED = 0,
>>
>> This looks good, but can this be shortened from
>> odp_ipsec_op_mode_support_t to something like odp_ipsec_support_t? The
>> enums could then be ODP_IPSEC_UNSUPPORTED / SUPPORTED / PREFERRED.
>> That's a lot less typing and just as clear, it seems.
>>
>>> + /**
>>> + * Mode is supported
>>> + */
>>> + ODP_IPSEC_OP_MODE_SUPPORTED,
>>> + /**
>>> + * Mode is supported and preferred
>>> + */
>>> + ODP_IPSEC_OP_MODE_PREFERRED,
>>> +} odp_ipsec_op_mode_support_t;
>
> There is a generic use for this support mode eg in cryto sync vs async
> so maybe we can create a generic enum ODP_MODE_SUPPORTED to be used in
> different modules.
>
+1 for Bala's suggestion.
>>> +
>>> +/**
>>> * IPSEC capability
>>> */
>>> typedef struct odp_ipsec_capability_t {
>>> /** Maximum number of IPSEC SAs */
>>> uint32_t max_num_sa;
>>>
>>> - /** Synchronous IPSEC operation mode (ODP_IPSEC_OP_MODE_SYNC)
>>> support
>>> - *
>>> - * 0: Synchronous mode is not supported
>>> - * 1: Synchronous mode is supported
>>> - * 2: Synchronous mode is supported and preferred
>>> - */
>>> - uint8_t op_mode_sync;
>>> + /** Synchronous IPSEC operation mode (ODP_IPSEC_OP_MODE_SYNC)
>>> support */
>>
>> If we're streamlining naming, perhaps ODP_IPSEC_SYNC / ASYNC / INLINE
>> would work for these as well? The base variables could then be
>> sync_support (or even just sync) rather than op_mode_sync, etc.
>>
>>> + odp_ipsec_op_mode_support_t op_mode_sync;
>>>
>>> - /** Asynchronous IPSEC operation mode (ODP_IPSEC_OP_MODE_ASYNC)
>>> support
>>> - *
>>> - * 0: Asynchronous mode is not supported
>>> - * 1: Asynchronous mode is supported
>>> - * 2: Asynchronous mode is supported and preferred
>>> + /**
>>> + * Asynchronous IPSEC operation mode (ODP_IPSEC_OP_MODE_ASYNC)
>>> support
>>> */
>>> - uint8_t op_mode_async;
>>> + odp_ipsec_op_mode_support_t op_mode_async;
>>>
>>> - /** Inline IPSEC operation mode (ODP_IPSEC_OP_MODE_INLINE) support
>>> - *
>>> - * 0: Inline IPSEC operation is not supported
>>> - * 1: Inline IPSEC operation is supported
>>> - * 2: Inline IPSEC operation is supported and preferred
>>> - */
>>> - uint8_t op_mode_inline;
>>> + /** Inline IPSEC operation mode (ODP_IPSEC_OP_MODE_INLINE) support
>>> */
>>> + odp_ipsec_op_mode_support_t op_mode_inline;
>>>
>>> - /** Support of pipelined classification (ODP_IPSEC_PIPELINE_CLS) of
>>> - * resulting inbound packets.
>>> - *
>>> - * 0: Classification of resulting packets is not supported
>>> - * 1: Classification of resulting packets is supported
>>> - * 2: Classification of resulting packets is supported and
>>> preferred
>>> + /**
>>> + * Support of pipelined classification (ODP_IPSEC_PIPELINE_CLS) of
>>> + * resulting inbound packets
>>> */
>>> - uint8_t pipeline_cls;
>>> + odp_ipsec_op_mode_support_t pipeline_cls;
>>>
>>> /** Soft expiry limit in seconds support
>>> *
>>> --
>>> 2.11.0
>>>
