On 19 April 2017 at 10:42, Savolainen, Petri (Nokia - FI/Espoo) < petri.savolai...@nokia-bell-labs.com> wrote:
> Maxim, > > Why this API change is now in api-next *without* by review? > > We discussed that on meeting and everybody said that it is good improvement and we need to apply that. > I did OBJECT to it yesterday. See under. Please revert it. > > > -Petri > > Yes, will be reverted shortly. Sorry Petri, I did not understand that it was objection. I read it as that you will have plans to do future work. If possible be more exact saying "NACK. ....". Thank you, Maxim. > > > -----Original Message----- > > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of > > Savolainen, Petri (Nokia - FI/Espoo) > > Sent: Tuesday, April 18, 2017 2:46 PM > > To: Dmitry Eremin-Solenikov <dmitry.ereminsoleni...@linaro.org>; lng- > > o...@lists.linaro.org > > Subject: Re: [lng-odp] [API-NEXT v4] api: ipsec: factor out definitions > > for feature support levels > > > > > > > > > -----Original Message----- > > > From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of > > > Dmitry Eremin-Solenikov > > > Sent: Tuesday, April 18, 2017 12:03 AM > > > To: lng-odp@lists.linaro.org > > > Subject: [lng-odp] [API-NEXT v4] api: ipsec: factor out definitions for > > > feature support levels > > > > > > 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/feature.h | 53 > > > ++++++++++++++++++++++++ > > > include/odp/api/spec/ipsec.h | 39 > ++++++----------- > > > include/odp_api.h | 1 + > > > platform/Makefile.inc | 1 + > > > platform/linux-generic/Makefile.am | 1 + > > > platform/linux-generic/include/odp/api/feature.h | 34 +++++++++++++++ > > > 6 files changed, 102 insertions(+), 27 deletions(-) > > > create mode 100644 include/odp/api/spec/feature.h > > > create mode 100644 platform/linux-generic/include/odp/api/feature.h > > > > > > diff --git a/include/odp/api/spec/feature.h > > > b/include/odp/api/spec/feature.h > > > new file mode 100644 > > > index 00000000..7ee2ae04 > > > --- /dev/null > > > +++ b/include/odp/api/spec/feature.h > > > @@ -0,0 +1,53 @@ > > > +/* Copyright (c) 2017, Linaro Limited > > > + * All rights reserved. > > > + * > > > + * SPDX-License-Identifier: BSD-3-Clause > > > + */ > > > + > > > +/** > > > + * @file > > > + * > > > + * ODP feature API > > > + */ > > > + > > > +#ifndef ODP_API_FEATURE_H_ > > > +#define ODP_API_FEATURE_H_ > > > +#include <odp/visibility_begin.h> > > > + > > > +#ifdef __cplusplus > > > +extern "C" { > > > +#endif > > > + > > > +/** @defgroup odp_feature ODP feature > > > + * Common API > > > + * @{ > > > + */ > > > + > > > +/** > > > + * ODP feature support > > > + */ > > > +typedef enum odp_feature_t { > > > + /** > > > + * Feature is not supported > > > + */ > > > + ODP_FEATURE_UNSUPPORTED, > > > + /** > > > + * Feature is supported > > > + */ > > > + ODP_FEATURE_SUPPORTED, > > > + /** > > > + * Feature is supported and preferred > > > + */ > > > + ODP_FEATURE_PREFERRED > > > +} odp_feature_t; > > > > > > I'm going to introduce odp_feature_t for list of ODP features - e.g. a > bit > > field listing all major ODP features (timer, tm, plain queue, scheduled > > queued, etc). It can be used e.g. in global init phase to list which APIs > > / API features the application is going to use. > > > > > > This one is support level. No support should be zero (good default for > > memset 0). The relative order must be documented, so that application can > > simply compare between levels. > > > > /** > > * ODP feature support level > > * > > * Support levels are specified in the relative order, where > > * ODP_SUPPORT_NO is the lowest level. E.g. if the examined support > > * level is greater than ODP_SUPPORT_NO, the feature is supported > > * in some form. > > */ > > typedef enum odp_support_t { > > /** > > * Feature is not supported > > */ > > ODP_SUPPORT_NO = 0, > > > > /** > > * Feature is supported > > */ > > ODP_SUPPORT_YES, > > > > /** > > * Feature is supported and preferred > > */ > > ODP_SUPPORT_PREFERRED > > > > } odp_support_t; > > > > > > > > > > > > > > @@ -230,38 +231,22 @@ 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_feature_t op_mode_sync; > > > > > > Enum takes 4x the space of uint8_t. Maybe that's not a problem on these > > structs. > > > > > > -Petri > > > >
