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