diff mbox

[API-NEXT] api: ipsec: factor out definitions for feature support levels

Message ID 20170414105813.26652-1-dmitry.ereminsolenikov@linaro.org
State Superseded
Headers show

Commit Message

Dmitry Eremin-Solenikov April 14, 2017, 10:58 a.m. UTC 
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.ereminsolenikov@linaro.org>

---
 include/odp/api/spec/ipsec.h | 56 +++++++++++++++++++++++---------------------
 1 file changed, 29 insertions(+), 27 deletions(-)

-- 
2.11.0

Comments

Bill Fischofer April 14, 2017, 11:22 a.m. UTC | #1 
On Fri, Apr 14, 2017 at 5:58 AM, Dmitry Eremin-Solenikov
<dmitry.ereminsolenikov@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.ereminsolenikov@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;

> +

> +/**

>   * 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

>
Balasubramanian Manoharan April 14, 2017, 11:53 a.m. UTC | #2 
Regards,
Bala


On 14 April 2017 at 16:52, Bill Fischofer <bill.fischofer@linaro.org> wrote:
> On Fri, Apr 14, 2017 at 5:58 AM, Dmitry Eremin-Solenikov

> <dmitry.ereminsolenikov@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.ereminsolenikov@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.

>> +

>> +/**

>>   * 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

>>
Bill Fischofer April 14, 2017, 1:25 p.m. UTC | #3 
On Fri, Apr 14, 2017 at 6:53 AM, Bala Manoharan
<bala.manoharan@linaro.org> wrote:
> Regards,

> Bala

>

>

> On 14 April 2017 at 16:52, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>> On Fri, Apr 14, 2017 at 5:58 AM, Dmitry Eremin-Solenikov

>> <dmitry.ereminsolenikov@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.ereminsolenikov@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

>>>
Dmitry Eremin-Solenikov April 14, 2017, 1:49 p.m. UTC | #4 
On 14.04.2017 16:25, Bill Fischofer wrote:
> On Fri, Apr 14, 2017 at 6:53 AM, Bala Manoharan

> <bala.manoharan@linaro.org> wrote:

>> Regards,

>> Bala

>>

>>

>> On 14 April 2017 at 16:52, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>>> On Fri, Apr 14, 2017 at 5:58 AM, Dmitry Eremin-Solenikov

>>> <dmitry.ereminsolenikov@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.ereminsolenikov@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.


Any suggestion for the header? I can't find one generic enough.


-- 
With best wishes
Dmitry
Bill Fischofer April 14, 2017, 2:35 p.m. UTC | #5 
Perhaps something like:

typedef enum odp_feature_t {
        ODP_FEATURE_UNSUPPORTED = 0,
        ODP_FEATURE_SUPPORTED = 1,
        ODP_FEATURE_PREFERRED = 2,
} odp_feature_t;

On Fri, Apr 14, 2017 at 8:49 AM, Dmitry Eremin-Solenikov
<dmitry.ereminsolenikov@linaro.org> wrote:
> On 14.04.2017 16:25, Bill Fischofer wrote:

>> On Fri, Apr 14, 2017 at 6:53 AM, Bala Manoharan

>> <bala.manoharan@linaro.org> wrote:

>>> Regards,

>>> Bala

>>>

>>>

>>> On 14 April 2017 at 16:52, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>>>> On Fri, Apr 14, 2017 at 5:58 AM, Dmitry Eremin-Solenikov

>>>> <dmitry.ereminsolenikov@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.ereminsolenikov@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.

>

> Any suggestion for the header? I can't find one generic enough.

>

>

> --

> With best wishes

> Dmitry
Dmitry Eremin-Solenikov April 14, 2017, 2:39 p.m. UTC | #6 
On 14.04.2017 17:35, Bill Fischofer wrote:
> Perhaps something like:

> 

> typedef enum odp_feature_t {

>         ODP_FEATURE_UNSUPPORTED = 0,

>         ODP_FEATURE_SUPPORTED = 1,

>         ODP_FEATURE_PREFERRED = 2,

> } odp_feature_t;


I've asked for the advice about the header file, where to place this
definition.

> 

> On Fri, Apr 14, 2017 at 8:49 AM, Dmitry Eremin-Solenikov

> <dmitry.ereminsolenikov@linaro.org> wrote:

>> On 14.04.2017 16:25, Bill Fischofer wrote:

>>> On Fri, Apr 14, 2017 at 6:53 AM, Bala Manoharan

>>> <bala.manoharan@linaro.org> wrote:

>>>> Regards,

>>>> Bala

>>>>

>>>>

>>>> On 14 April 2017 at 16:52, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>>>>> On Fri, Apr 14, 2017 at 5:58 AM, Dmitry Eremin-Solenikov

>>>>> <dmitry.ereminsolenikov@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.ereminsolenikov@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.

>>

>> Any suggestion for the header? I can't find one generic enough.

>>

>>

>> --

>> With best wishes

>> Dmitry



-- 
With best wishes
Dmitry
Bill Fischofer April 14, 2017, 2:44 p.m. UTC | #7 
On Fri, Apr 14, 2017 at 9:39 AM, Dmitry Eremin-Solenikov
<dmitry.ereminsolenikov@linaro.org> wrote:
> On 14.04.2017 17:35, Bill Fischofer wrote:

>> Perhaps something like:

>>

>> typedef enum odp_feature_t {

>>         ODP_FEATURE_UNSUPPORTED = 0,

>>         ODP_FEATURE_SUPPORTED = 1,

>>         ODP_FEATURE_PREFERRED = 2,

>> } odp_feature_t;

>

> I've asked for the advice about the header file, where to place this

> definition.


Perhaps a new feature.h file that can be included by ipsec, crypto, or
similar things that will need these sort of definitions?

>

>>

>> On Fri, Apr 14, 2017 at 8:49 AM, Dmitry Eremin-Solenikov

>> <dmitry.ereminsolenikov@linaro.org> wrote:

>>> On 14.04.2017 16:25, Bill Fischofer wrote:

>>>> On Fri, Apr 14, 2017 at 6:53 AM, Bala Manoharan

>>>> <bala.manoharan@linaro.org> wrote:

>>>>> Regards,

>>>>> Bala

>>>>>

>>>>>

>>>>> On 14 April 2017 at 16:52, Bill Fischofer <bill.fischofer@linaro.org> wrote:

>>>>>> On Fri, Apr 14, 2017 at 5:58 AM, Dmitry Eremin-Solenikov

>>>>>> <dmitry.ereminsolenikov@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.ereminsolenikov@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.

>>>

>>> Any suggestion for the header? I can't find one generic enough.

>>>

>>>

>>> --

>>> With best wishes

>>> Dmitry

>

>

> --

> With best wishes

> Dmitry
diff mbox

Patch

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,
+	/**
+	 * Mode is supported
+	 */
+	ODP_IPSEC_OP_MODE_SUPPORTED,
+	/**
+	 * Mode is supported and preferred
+	 */
+	ODP_IPSEC_OP_MODE_PREFERRED,
+} odp_ipsec_op_mode_support_t;
+
+/**
  * 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 */
+	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
 	 *