Re: [lng-odp] [RFC] api: classification: add random early discard

2017-06-12 Thread Bill Fischofer 
On Mon, Jun 12, 2017 at 6:41 AM, Balasubramanian Manoharan
 wrote:
> Add random early discard configuration to class-of-service
>
> Signed-off-by: Balasubramanian Manoharan 
> ---
>  include/odp/api/spec/classification.h | 44 
> +++
>  1 file changed, 44 insertions(+)
>
> diff --git a/include/odp/api/spec/classification.h 
> b/include/odp/api/spec/classification.h
> index 39831b2..badadff 100644
> --- a/include/odp/api/spec/classification.h
> +++ b/include/odp/api/spec/classification.h
> @@ -128,6 +128,13 @@ typedef struct odp_cls_capability_t {
>
> /** A Boolean to denote support of PMR range */
> odp_bool_t pmr_range_supported;
> +
> +   /** Support for Random Early Discard */
> +   odp_support_t random_early_discard;

The correct expansion of the RED acronym, per RFC 2309 [1], is Random
Early Detection. This should be updated throughout.

---
[1] https://tools.ietf.org/html/rfc2309

> +
> +   /** Support for Back Pressure to the remote peer */
> +   odp_support_t back_pressure;
> +
>  } odp_cls_capability_t;
>
>  /**
> @@ -167,6 +174,43 @@ typedef struct odp_cls_cos_param {
> odp_queue_t queue;  /**< Queue associated with CoS */
> odp_pool_t pool;/**< Pool associated with CoS */
> odp_cls_drop_t drop_policy; /**< Drop policy associated with CoS 
> */
> +
> +   /* Random Early Discard (RED)
> +* Random Early discard is enabled to initiate a drop probability
> +* for the incoming packet when the packets in the queue/pool reaches
> +* a specified threshold.
> +* When RED is enabled for a particular flow then further incoming
> +* packets are assigned a drop probability based on the size of the
> +* pool/queue and the drop probability becomes 1 when the queue/pool
> +* is full.
> +* RED is logically configured in the CoS and could be implemented
> +* in either pool or queue linked to the CoS depending on
> +* platform capabilities.
> +* RED is controlled using maximum and minimum threshold values
> +* which are defined as percentage of the system resource.
> +* RED is enabled when the resource limit is equal to or greater than
> +* the maximum threshold value and is disabled when resource limit
> +* is less than or equal to minimum threshold value. */

Is this sufficient? RED is a family of algorithms, including Weighted
RED (WRED), Adaptive RED (ARED), Robust RED (RRED), etc. [2]. Each of
these require a number of parameters to control it. All RED-type
processing requires some estimation of average queue size or marks, so
has ties both to HW as well as higher-level protocols like TCP with
ECN support. So the question is how will applications, or subsystems
like OFP, make use of this feature?

---
[2] https://en.wikipedia.org/wiki/Random_early_detection

> +
> +   /* A boolean to enable RED parameters */
> +   odp_bool_t red_enable;
> +
> +   /* Maximum threshold percentage for RED */
> +   uint16_t max_red_threshold;
> +
> +   /* Minimum threshold percentage for RED */
> +   uint16_t min_red_threshold;

Need to be more precise about what minimum and maximum refer to here.
Needs to be sufficiently specific to enable a validation test to be
written to determine whether an ODP implementation conforms to the
spec. Also, if these are percentages, shouldn't a uint8_t be
sufficient since the range should only be between 0-100?

> +
> +   /* Back pressure
> +* When back pressure is enabled for a particular flow, the HW can 
> send
> +* back pressure information to the remote peer indicating a network
> +* congestion */
> +   odp_bool_t bp_enable;

Again, requires a bit more precision and possibly additional
parameters. For example, if PFC is used as a back pressure mechanism,
we need to know the PFC class to use.

> +
> +   /* Threshold for enabling back pressure. BP is enabled when pool/queue
> +* size is equal to or greater than this backpressure threshold.
> +* BP threshold is expressed as a percentage of the resource size. */
> +   uint16_t bp_threshold;

Why is this a size when RED is specified in terms of percentages?
Shouldn't these be consistent?

>  } odp_cls_cos_param_t;
>
>  /**
> --
> 1.9.1
>

[lng-odp] [API-NEXT PATCHv5 5/5] doc: userguide: add odp_init_global() documentation for unused features

2017-06-12 Thread Bill Fischofer 
Update User Guide startup section to include current parameters for
odp_init_global() and odp_init_local() as well as optimization hints for
unused features.

Signed-off-by: Bill Fischofer 
---
 doc/users-guide/users-guide.adoc | 36 ++--
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/doc/users-guide/users-guide.adoc b/doc/users-guide/users-guide.adoc
index ead8da5e..d78e0777 100755
--- a/doc/users-guide/users-guide.adoc
+++ b/doc/users-guide/users-guide.adoc
@@ -530,20 +530,44 @@ calling the terminate functions should only be done when 
the application is
 sure it has closed the ingress and subsequently drained all queues, etc.
 
 === Startup
-The first API that must be called by an ODP application is 'odp_init_global()'.
+The first API that must be called by an ODP application is `odp_init_global()`:
+[source,c]
+-
+int odp_init_global(odp_instance_t *instance,
+   const odp_init_t *param,
+   const odp_platform_init_t *platform_param);
+-
 This takes two pointers. The first, `odp_init_t`, contains ODP initialization
 data that is platform independent and portable, while the second,
 `odp_platform_init_t`, is passed unparsed to the implementation
 to be used for platform specific data that is not yet, or may never be
-suitable for the ODP API.
+suitable for the ODP API. Each of these parameters is optional and may be
+specified as NULL to accept the implementation-defined default initialization
+values.
 
-Calling odp_init_global() establishes the ODP API framework and MUST be
+Calling `odp_init_global()` establishes the ODP API framework and MUST be
 called before any other ODP API may be called. Note that it is only called
-once per application. Following global initialization, each thread in turn
+once per application. A successful call to `odp_init_global()` returns rc = 0
+and sets the `instance` variable supplied as input to the call to an handle
+representing this unique ODP instance.
+
+The `odp_init_t` parameter is used to specify various customizations to the
+ODP environment being established by this call. For example, the caller can
+specify the maximum number of worker threads it will use, the thread masks
+associated with these threads, as well as whether the default logging or
+abort functions are to be overridden with an application-supplied handler.
+
+The application may also provide optimization hints to the ODP implementation
+if it knows that it will never use specific ODP feature sets, such as the
+packet classifier or traffic manager. Implementations may use such hints to
+provide optimized behavior to applications that are known not to need these
+features.
+
+Following global initialization, each thread in turn
 calls 'odp_init_local()'. This establishes the local ODP thread
 context for that thread and MUST be called before other ODP APIs may be
-called by that thread. The sole argument to this call is the _thread type_,
-which is either `ODP_THREAD_WORKER` or `ODP_THREAD_CONTROL`.
+called by that thread. The sole argument to this call is the `instance`
+variable returned by `odp_init_global()`.
 
 === Shutdown
 Shutdown is the logical reverse of the initialization procedure, with
-- 
2.11.0

[lng-odp] [API-NEXT PATCHv5 4/5] validation: init: use odp_init_param_init() in init tests

2017-06-12 Thread Bill Fischofer 
Provide test coverage for odp_init_param_init() API.

Signed-off-by: Bill Fischofer 
---
 test/common_plat/validation/api/init/init.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/common_plat/validation/api/init/init.c 
b/test/common_plat/validation/api/init/init.c
index 61055fad..6f9556d7 100644
--- a/test/common_plat/validation/api/init/init.c
+++ b/test/common_plat/validation/api/init/init.c
@@ -24,10 +24,10 @@ static int odp_init_log(odp_log_level_t level, const char 
*fmt, ...);
 void init_test_odp_init_global_replace_abort(void)
 {
int status;
-   struct odp_init_t init_data;
+   odp_init_t init_data;
odp_instance_t instance;
 
-   memset(_data, 0, sizeof(init_data));
+   odp_init_param_init(_data);
init_data.abort_fn = _init_abort;
 
status = odp_init_global(, _data, NULL);
@@ -77,10 +77,10 @@ int init_main_abort(int argc, char *argv[])
 void init_test_odp_init_global_replace_log(void)
 {
int status;
-   struct odp_init_t init_data;
+   odp_init_t init_data;
odp_instance_t instance;
 
-   memset(_data, 0, sizeof(init_data));
+   odp_init_param_init(_data);
init_data.log_fn = _init_log;
 
replacement_logging_used = 0;
-- 
2.11.0

[lng-odp] [API-NEXT PATCHv5 3/5] linux-generic: init: implement odp_init_param_init()

2017-06-12 Thread Bill Fischofer 
Signed-off-by: Bill Fischofer 
---
 platform/linux-generic/odp_init.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/platform/linux-generic/odp_init.c 
b/platform/linux-generic/odp_init.c
index 31bd7c73..62a1fbc2 100644
--- a/platform/linux-generic/odp_init.c
+++ b/platform/linux-generic/odp_init.c
@@ -147,6 +147,11 @@ static int read_configfile(void)
return 0;
 }
 
+void odp_init_param_init(odp_init_t *param)
+{
+   memset(param, 0, sizeof(odp_init_t));
+}
+
 int odp_init_global(odp_instance_t *instance,
const odp_init_t *params,
const odp_platform_init_t *platform_params ODP_UNUSED)
-- 
2.11.0

[lng-odp] [API-NEXT PATCHv5 2/5] api: init: add support for unused features

2017-06-12 Thread Bill Fischofer 
Add the not_used field to odp_init_t to permit applications to
specify that they will not use various ODP features. This may
allow implementations to provide optimized behavior.

Also add the odp_init_param_init() API to initialize odp_init_t
to default values.

Signed-off-by: Bill Fischofer 
---
 include/odp/api/spec/init.h | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/include/odp/api/spec/init.h b/include/odp/api/spec/init.h
index 154cdf8f..e8ec4113 100644
--- a/include/odp/api/spec/init.h
+++ b/include/odp/api/spec/init.h
@@ -29,6 +29,7 @@ extern "C" {
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -153,9 +154,23 @@ typedef struct odp_init_t {
odp_log_func_t log_fn;
/** Replacement for the default abort fn */
odp_abort_func_t abort_fn;
+   /** Unused features. These are hints to the ODP implementation that
+* the application will not use any APIs associated with these
+* features. Implementations may use this information to provide
+* optimized behavior. Results are undefined if applications assert
+* that a feature will not be used and it is used anyway.
+*/
+   odp_feature_t not_used;
 } odp_init_t;
 
 /**
+ * Initialize the odp_init_t to default values for all fields
+ *
+ * @param[out] param Address of the odp_init_t to be initialized
+ */
+void odp_init_param_init(odp_init_t *param);
+
+/**
  * @typedef odp_platform_init_t
  * ODP platform initialization data
  *
-- 
2.11.0

[lng-odp] [API-NEXT PATCHv5 1/5] api: feature: add odp feature bits

2017-06-12 Thread Bill Fischofer 
Add new odp_feature_t bits that permit other APIs/components to
refer to various ODP features.

Signed-off-by: Bill Fischofer 
---
 include/odp/api/spec/feature.h   | 65 
 include/odp_api.h|  1 +
 platform/Makefile.inc|  1 +
 platform/linux-generic/Makefile.am   |  3 +-
 platform/linux-generic/include/odp/api/feature.h | 34 +
 5 files changed, 103 insertions(+), 1 deletion(-)
 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 ..dd8db780
--- /dev/null
+++ b/include/odp/api/spec/feature.h
@@ -0,0 +1,65 @@
+/* Copyright (c) 2017, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * ODP features.
+ * Define various ODP feature sets that can be referenced by other
+ * components.
+ */
+
+#ifndef ODP_API_FEATURE_H_
+#define ODP_API_FEATURE_H_
+#include 
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include 
+
+/** @defgroup odp_features ODP_FEATURE
+ *  ODP feature definitions
+ *  @{
+ */
+
+/** Definition of ODP features */
+typedef union odp_feature_t {
+   /** All features */
+   uint32_t all_feat;
+
+   /** Individual feature bits */
+   struct {
+   /** Classifier APIs are not used, e.g., odp_cls_xxx(),
+* odp_cos_xxx()  */
+   uint32_t cls:1;
+   /** Crypto APIs are not used, e.g., odp_crypto_xxx() */
+   uint32_t crypto:1;
+   /** IPsec APIs are not used, e.g., odp_ipsec_xxx() */
+   uint32_t ipsec:1;
+   /** Scheduler APIs are not used, e.g., odp_schedule_xxx() */
+   uint32_t schedule:1;
+   /** Time APIs are not used, e.g., odp_time_xxx(),
+* odp_timeout_xxx() */
+   uint32_t time:1;
+   /** Timer APIs are not used, e.g., odp_timer_xxx() */
+   uint32_t timer:1;
+   /** Traffic Manager APIs are not used, e.g., odp_tm_xxx() */
+   uint32_t tm:1;
+   } feat;
+} odp_feature_t;
+
+/**
+ * @}
+ */
+
+#ifdef __cplusplus
+}
+#endif
+
+#include 
+#endif
diff --git a/include/odp_api.h b/include/odp_api.h
index 8146e024..86232ee1 100644
--- a/include/odp_api.h
+++ b/include/odp_api.h
@@ -32,6 +32,7 @@ extern "C" {
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/platform/Makefile.inc b/platform/Makefile.inc
index 3d609aa7..9f856a15 100644
--- a/platform/Makefile.inc
+++ b/platform/Makefile.inc
@@ -32,6 +32,7 @@ odpapispecinclude_HEADERS = \
  $(top_srcdir)/include/odp/api/spec/deprecated.h \
  $(top_srcdir)/include/odp/api/spec/errno.h \
  $(top_srcdir)/include/odp/api/spec/event.h \
+ $(top_srcdir)/include/odp/api/spec/feature.h \
  $(top_srcdir)/include/odp/api/spec/support.h \
  $(top_srcdir)/include/odp/api/spec/hash.h \
  $(top_srcdir)/include/odp/api/spec/hints.h \
diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index 58c73767..b4bbd07e 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -38,7 +38,7 @@ odpapiinclude_HEADERS = \
  $(srcdir)/include/odp/api/deprecated.h \
  $(srcdir)/include/odp/api/errno.h \
  $(srcdir)/include/odp/api/event.h \
- $(srcdir)/include/odp/api/support.h \
+ $(srcdir)/include/odp/api/feature.h \
  $(srcdir)/include/odp/api/hash.h \
  $(srcdir)/include/odp/api/hints.h \
  $(srcdir)/include/odp/api/init.h \
@@ -59,6 +59,7 @@ odpapiinclude_HEADERS = \
  $(srcdir)/include/odp/api/spinlock_recursive.h \
  $(srcdir)/include/odp/api/std_clib.h \
  $(srcdir)/include/odp/api/std_types.h \
+ $(srcdir)/include/odp/api/support.h \
  $(srcdir)/include/odp/api/sync.h \
  $(srcdir)/include/odp/api/system_info.h \
  $(srcdir)/include/odp/api/thread.h \
diff --git a/platform/linux-generic/include/odp/api/feature.h 
b/platform/linux-generic/include/odp/api/feature.h
new file mode 100644
index ..55a86a83
--- /dev/null
+++ b/platform/linux-generic/include/odp/api/feature.h
@@ -0,0 +1,34 @@
+/* Copyright (c) 2017, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * ODP features.
+ */
+
+#ifndef ODP_PLAT_FEATURE_H_
+#define ODP_PLAT_FEATURE_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/** @ingroup odp_feature
+ *  @{
+ */
+
+/**
+ * @}
+ */
+

[lng-odp] [API-NEXT PATCHv4 5/5] doc: feature: add odp_init_global() documentation for unused features

2017-06-12 Thread Bill Fischofer 
Signed-off-by: Bill Fischofer 
---
 doc/users-guide/users-guide.adoc | 36 ++--
 1 file changed, 30 insertions(+), 6 deletions(-)

diff --git a/doc/users-guide/users-guide.adoc b/doc/users-guide/users-guide.adoc
index ead8da5e..d78e0777 100755
--- a/doc/users-guide/users-guide.adoc
+++ b/doc/users-guide/users-guide.adoc
@@ -530,20 +530,44 @@ calling the terminate functions should only be done when 
the application is
 sure it has closed the ingress and subsequently drained all queues, etc.
 
 === Startup
-The first API that must be called by an ODP application is 'odp_init_global()'.
+The first API that must be called by an ODP application is `odp_init_global()`:
+[source,c]
+-
+int odp_init_global(odp_instance_t *instance,
+   const odp_init_t *param,
+   const odp_platform_init_t *platform_param);
+-
 This takes two pointers. The first, `odp_init_t`, contains ODP initialization
 data that is platform independent and portable, while the second,
 `odp_platform_init_t`, is passed unparsed to the implementation
 to be used for platform specific data that is not yet, or may never be
-suitable for the ODP API.
+suitable for the ODP API. Each of these parameters is optional and may be
+specified as NULL to accept the implementation-defined default initialization
+values.
 
-Calling odp_init_global() establishes the ODP API framework and MUST be
+Calling `odp_init_global()` establishes the ODP API framework and MUST be
 called before any other ODP API may be called. Note that it is only called
-once per application. Following global initialization, each thread in turn
+once per application. A successful call to `odp_init_global()` returns rc = 0
+and sets the `instance` variable supplied as input to the call to an handle
+representing this unique ODP instance.
+
+The `odp_init_t` parameter is used to specify various customizations to the
+ODP environment being established by this call. For example, the caller can
+specify the maximum number of worker threads it will use, the thread masks
+associated with these threads, as well as whether the default logging or
+abort functions are to be overridden with an application-supplied handler.
+
+The application may also provide optimization hints to the ODP implementation
+if it knows that it will never use specific ODP feature sets, such as the
+packet classifier or traffic manager. Implementations may use such hints to
+provide optimized behavior to applications that are known not to need these
+features.
+
+Following global initialization, each thread in turn
 calls 'odp_init_local()'. This establishes the local ODP thread
 context for that thread and MUST be called before other ODP APIs may be
-called by that thread. The sole argument to this call is the _thread type_,
-which is either `ODP_THREAD_WORKER` or `ODP_THREAD_CONTROL`.
+called by that thread. The sole argument to this call is the `instance`
+variable returned by `odp_init_global()`.
 
 === Shutdown
 Shutdown is the logical reverse of the initialization procedure, with
-- 
2.11.0

[lng-odp] [API-NEXT PATCHv4 4/5] validation: init: use odp_init_param_init() in init tests

2017-06-12 Thread Bill Fischofer 
Provide test coverage for odp_init_param_init() API.

Signed-off-by: Bill Fischofer 
---
 test/common_plat/validation/api/init/init.c | 8 
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/test/common_plat/validation/api/init/init.c 
b/test/common_plat/validation/api/init/init.c
index 61055fad..6f9556d7 100644
--- a/test/common_plat/validation/api/init/init.c
+++ b/test/common_plat/validation/api/init/init.c
@@ -24,10 +24,10 @@ static int odp_init_log(odp_log_level_t level, const char 
*fmt, ...);
 void init_test_odp_init_global_replace_abort(void)
 {
int status;
-   struct odp_init_t init_data;
+   odp_init_t init_data;
odp_instance_t instance;
 
-   memset(_data, 0, sizeof(init_data));
+   odp_init_param_init(_data);
init_data.abort_fn = _init_abort;
 
status = odp_init_global(, _data, NULL);
@@ -77,10 +77,10 @@ int init_main_abort(int argc, char *argv[])
 void init_test_odp_init_global_replace_log(void)
 {
int status;
-   struct odp_init_t init_data;
+   odp_init_t init_data;
odp_instance_t instance;
 
-   memset(_data, 0, sizeof(init_data));
+   odp_init_param_init(_data);
init_data.log_fn = _init_log;
 
replacement_logging_used = 0;
-- 
2.11.0

[lng-odp] [API-NEXT PATCHv4 3/5] linux-generic: init: implement odp_init_param_init()

2017-06-12 Thread Bill Fischofer 
Signed-off-by: Bill Fischofer 
---
 platform/linux-generic/odp_init.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/platform/linux-generic/odp_init.c 
b/platform/linux-generic/odp_init.c
index 31bd7c73..62a1fbc2 100644
--- a/platform/linux-generic/odp_init.c
+++ b/platform/linux-generic/odp_init.c
@@ -147,6 +147,11 @@ static int read_configfile(void)
return 0;
 }
 
+void odp_init_param_init(odp_init_t *param)
+{
+   memset(param, 0, sizeof(odp_init_t));
+}
+
 int odp_init_global(odp_instance_t *instance,
const odp_init_t *params,
const odp_platform_init_t *platform_params ODP_UNUSED)
-- 
2.11.0

[lng-odp] [API-NEXT PATCHv4 2/5] api: init: add support for unused features

2017-06-12 Thread Bill Fischofer 
Add the not_used field to odp_init_t to permit applications to
specify that they will not use various ODP features. This may
allow implementations to provide optimized behavior.

Also add the odp_init_param_init() API to initialize odp_init_t
to default values.

Signed-off-by: Bill Fischofer 
---
 include/odp/api/spec/init.h | 15 +++
 1 file changed, 15 insertions(+)

diff --git a/include/odp/api/spec/init.h b/include/odp/api/spec/init.h
index 154cdf8f..e8ec4113 100644
--- a/include/odp/api/spec/init.h
+++ b/include/odp/api/spec/init.h
@@ -29,6 +29,7 @@ extern "C" {
 
 #include 
 #include 
+#include 
 #include 
 #include 
 
@@ -153,9 +154,23 @@ typedef struct odp_init_t {
odp_log_func_t log_fn;
/** Replacement for the default abort fn */
odp_abort_func_t abort_fn;
+   /** Unused features. These are hints to the ODP implementation that
+* the application will not use any APIs associated with these
+* features. Implementations may use this information to provide
+* optimized behavior. Results are undefined if applications assert
+* that a feature will not be used and it is used anyway.
+*/
+   odp_feature_t not_used;
 } odp_init_t;
 
 /**
+ * Initialize the odp_init_t to default values for all fields
+ *
+ * @param[out] param Address of the odp_init_t to be initialized
+ */
+void odp_init_param_init(odp_init_t *param);
+
+/**
  * @typedef odp_platform_init_t
  * ODP platform initialization data
  *
-- 
2.11.0

[lng-odp] [API-NEXT PATCHv4 1/5] api: feature: add odp feature bits

2017-06-12 Thread Bill Fischofer 
Add new odp_feature_t bits that permit other APIs/components to
refer to various ODP features.

Signed-off-by: Bill Fischofer 
---
 include/odp/api/spec/feature.h   | 65 
 include/odp_api.h|  1 +
 platform/Makefile.inc|  1 +
 platform/linux-generic/Makefile.am   |  3 +-
 platform/linux-generic/include/odp/api/feature.h | 34 +
 5 files changed, 103 insertions(+), 1 deletion(-)
 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 ..dd8db780
--- /dev/null
+++ b/include/odp/api/spec/feature.h
@@ -0,0 +1,65 @@
+/* Copyright (c) 2017, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * ODP features.
+ * Define various ODP feature sets that can be referenced by other
+ * components.
+ */
+
+#ifndef ODP_API_FEATURE_H_
+#define ODP_API_FEATURE_H_
+#include 
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+#include 
+
+/** @defgroup odp_features ODP_FEATURE
+ *  ODP feature definitions
+ *  @{
+ */
+
+/** Definition of ODP features */
+typedef union odp_feature_t {
+   /** All features */
+   uint32_t all_feat;
+
+   /** Individual feature bits */
+   struct {
+   /** Classifier APIs are not used, e.g., odp_cls_xxx(),
+* odp_cos_xxx()  */
+   uint32_t cls:1;
+   /** Crypto APIs are not used, e.g., odp_crypto_xxx() */
+   uint32_t crypto:1;
+   /** IPsec APIs are not used, e.g., odp_ipsec_xxx() */
+   uint32_t ipsec:1;
+   /** Scheduler APIs are not used, e.g., odp_schedule_xxx() */
+   uint32_t schedule:1;
+   /** Time APIs are not used, e.g., odp_time_xxx(),
+* odp_timeout_xxx() */
+   uint32_t time:1;
+   /** Timer APIs are not used, e.g., odp_timer_xxx() */
+   uint32_t timer:1;
+   /** Traffic Manager APIs are not used, e.g., odp_tm_xxx() */
+   uint32_t tm:1;
+   } feat;
+} odp_feature_t;
+
+/**
+ * @}
+ */
+
+#ifdef __cplusplus
+}
+#endif
+
+#include 
+#endif
diff --git a/include/odp_api.h b/include/odp_api.h
index 8146e024..86232ee1 100644
--- a/include/odp_api.h
+++ b/include/odp_api.h
@@ -32,6 +32,7 @@ extern "C" {
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
diff --git a/platform/Makefile.inc b/platform/Makefile.inc
index 3d609aa7..9f856a15 100644
--- a/platform/Makefile.inc
+++ b/platform/Makefile.inc
@@ -32,6 +32,7 @@ odpapispecinclude_HEADERS = \
  $(top_srcdir)/include/odp/api/spec/deprecated.h \
  $(top_srcdir)/include/odp/api/spec/errno.h \
  $(top_srcdir)/include/odp/api/spec/event.h \
+ $(top_srcdir)/include/odp/api/spec/feature.h \
  $(top_srcdir)/include/odp/api/spec/support.h \
  $(top_srcdir)/include/odp/api/spec/hash.h \
  $(top_srcdir)/include/odp/api/spec/hints.h \
diff --git a/platform/linux-generic/Makefile.am 
b/platform/linux-generic/Makefile.am
index 58c73767..b4bbd07e 100644
--- a/platform/linux-generic/Makefile.am
+++ b/platform/linux-generic/Makefile.am
@@ -38,7 +38,7 @@ odpapiinclude_HEADERS = \
  $(srcdir)/include/odp/api/deprecated.h \
  $(srcdir)/include/odp/api/errno.h \
  $(srcdir)/include/odp/api/event.h \
- $(srcdir)/include/odp/api/support.h \
+ $(srcdir)/include/odp/api/feature.h \
  $(srcdir)/include/odp/api/hash.h \
  $(srcdir)/include/odp/api/hints.h \
  $(srcdir)/include/odp/api/init.h \
@@ -59,6 +59,7 @@ odpapiinclude_HEADERS = \
  $(srcdir)/include/odp/api/spinlock_recursive.h \
  $(srcdir)/include/odp/api/std_clib.h \
  $(srcdir)/include/odp/api/std_types.h \
+ $(srcdir)/include/odp/api/support.h \
  $(srcdir)/include/odp/api/sync.h \
  $(srcdir)/include/odp/api/system_info.h \
  $(srcdir)/include/odp/api/thread.h \
diff --git a/platform/linux-generic/include/odp/api/feature.h 
b/platform/linux-generic/include/odp/api/feature.h
new file mode 100644
index ..55a86a83
--- /dev/null
+++ b/platform/linux-generic/include/odp/api/feature.h
@@ -0,0 +1,34 @@
+/* Copyright (c) 2017, Linaro Limited
+ * All rights reserved.
+ *
+ * SPDX-License-Identifier: BSD-3-Clause
+ */
+
+/**
+ * @file
+ *
+ * ODP features.
+ */
+
+#ifndef ODP_PLAT_FEATURE_H_
+#define ODP_PLAT_FEATURE_H_
+
+#ifdef __cplusplus
+extern "C" {
+#endif
+
+/** @ingroup odp_feature
+ *  @{
+ */
+
+/**
+ * @}
+ */
+

Re: [lng-odp] [API-NEXT PATCHv3 1/4] api: feature: add odp feature bits

2017-06-12 Thread Bill Fischofer 
Thanks. I'll fix this in v4.

On Mon, Jun 12, 2017 at 7:28 AM, Savolainen, Petri (Nokia - FI/Espoo)
 wrote:
>
>
>> -Original Message-
>> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Bill
>> Fischofer
>> Sent: Monday, June 12, 2017 2:59 AM
>> To: lng-odp@lists.linaro.org
>> Subject: [lng-odp] [API-NEXT PATCHv3 1/4] api: feature: add odp feature
>> bits
>>
>> Add new odp_feature_t bits that permit other APIs/components to
>> refer to various ODP features.
>>
>> Signed-off-by: Bill Fischofer 
>> ---
>>  include/odp/api/spec/feature.h   | 60
>> 
>>  include/odp_api.h|  1 +
>>  platform/Makefile.inc|  1 +
>>  platform/linux-generic/Makefile.am   |  3 +-
>>  platform/linux-generic/include/odp/api/feature.h | 34 ++
>>  5 files changed, 98 insertions(+), 1 deletion(-)
>>  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 ..a1cf4505
>> --- /dev/null
>> +++ b/include/odp/api/spec/feature.h
>> @@ -0,0 +1,60 @@
>> +/* Copyright (c) 2017, Linaro Limited
>> + * All rights reserved.
>> + *
>> + * SPDX-License-Identifier: BSD-3-Clause
>> + */
>> +
>> +
>> +/**
>> + * @file
>> + *
>> + * ODP features.
>> + * Define various ODP feature sets that can be referenced by other
>> + * components.
>> + */
>> +
>> +#ifndef ODP_API_FEATURE_H_
>> +#define ODP_API_FEATURE_H_
>> +#include 
>> +
>> +#ifdef __cplusplus
>> +extern "C" {
>> +#endif
>> +
>> +#include 
>> +#include 
>> +#include 
>> +#include 
>
> These three includes above are not needed.
>
>
>> +
>> +/** @defgroup odp_features ODP_FEATURES
>> + *  ODP feature definitions
>> + *  @{
>> + */
>> +
>> +/** Definition of ODP features */
>> +typedef union odp_feature_t {
>> + /** All features */
>> + uint32_t all_feat;
>> +
>> + /** Individual feature bits */
>> + struct {
>> + uint32_t classification:1;
>
> "classifier" or "cls" instead of classification. Packet IO API has 
> "classifier_enable" parameter. IPSEC API has "ODP_IPSEC_PIPELINE_CLS" 
> enumeration.
>
>> + uint32_t crypto:1;
>> + uint32_t ipsec:1;
>> + uint32_t schedule:1;
>> + uint32_t time:1;
>> + uint32_t timer:1;
>> + uint32_t traffic_mngr:1;
>> + } feat;
>> +} odp_feature_t;
>
> Each feature bit would need some documentation about it. Application does not 
> include spec directory header files, but odp_api.h. So, a "feature" needs to 
> be defined other means than with file names. For example, what feature "cls" 
> means ... calls to odp_cls_xxx() / odp_cos_xxx() functions.
>
>
> -Petri
>
>
>

Re: [lng-odp] [API-NEXT PATCH 1/2] api: ipsec: change IPSEC result to packet

2017-06-12 Thread Bill Fischofer 
On Mon, Jun 12, 2017 at 2:34 AM, Savolainen, Petri (Nokia - FI/Espoo)
 wrote:
>> >> >  /**
>> >> >   * Get IPSEC status information from an ODP_EVENT_IPSEC_STATUS event
>> >> > diff --git a/include/odp/arch/default/api/abi/event.h
>> >> b/include/odp/arch/default/api/abi/event.h
>> >> > index 87220d63..d807425b 100644
>> >> > --- a/include/odp/arch/default/api/abi/event.h
>> >> > +++ b/include/odp/arch/default/api/abi/event.h
>> >> > @@ -27,9 +27,9 @@ typedef _odp_abi_event_t *odp_event_t;
>> >> >  typedef enum odp_event_type_t {
>> >> > ODP_EVENT_BUFFER   = 1,
>> >> > ODP_EVENT_PACKET   = 2,
>> >> > -   ODP_EVENT_TIMEOUT  = 3,
>> >> > -   ODP_EVENT_CRYPTO_COMPL = 4,
>> >> > -   ODP_EVENT_IPSEC_RESULT = 5,
>> >> > +   ODP_EVENT_PACKET_IPSEC = 3,
>> >> > +   ODP_EVENT_TIMEOUT  = 4,
>> >> > +   ODP_EVENT_CRYPTO_COMPL = 5,
>> >> > ODP_EVENT_IPSEC_STATUS = 6
>> >> >  } odp_event_type_t;
>> >>
>> >> Any reason for rearrangement of existing values?
>> >
>> > Yes, it's now
>> >
>> > ODP_EVENT_BUFFER   = 1,
>> > ODP_EVENT_PACKET   = 2,
>> > ODP_EVENT_PACKET_IPSEC = 3,
>> >
>> > which may turn in couple of weeks into
>> >
>> > ODP_EVENT_BUFFER   = 1,
>> > ODP_EVENT_PACKET   = 2,
>> > ODP_EVENT_PACKET_IPSEC = 3,
>> > ODP_EVENT_PACKET_CRYPTO = 4,
>> > ODP_EVENT_PACKET_COMP = 5,
>> >
>> > ... normal packet and special ones.
>>
>> The problem with trying to to this as an ever-growing list of event
>> types, is that every time we introduce a new one every application
>> needs to change. ODP_EVENT_PACKET_IPSEC is simply an ODP_EVENT_PACKET
>> with some additional metadata that may or may not be of interest to a
>> given application processing stage. So the basic architecture should
>> reflect this fact.
>
> Actually, the idea is that existing application *does not* have to change. 
> EVENT_PACKET is still what it is today. So, current applications do not need 
> to change anything: e.g. those do not need to check if a packet is coming 
> from IPSEC and if the IPSEC operation has failed, etc.

EVENT_PACKET remains the same after the addition of packet subtypes
since every subtype is fully an odp_packet_t. The subtypes simply
qualify that by noting what additional metadata is available for this
packet. Applications (or more properly individual routines within
applications) need only be concerned with subtypes when they want to
process this additional metadata.

>
>
>>
>> The typical event loop for a worker thread would look like:
>>
>> while (1) {
>> odp_event_t ev = odp_schedule(, ODP_SCHED_WAIT);
>> switch (odp_event_type(ev)) {
>> case ODP_EVENT_BUFFER:
>> do_buffer_processing(odp_buffer_from_event(ev),
>> queue);
>> break;
>> case ODP_EVENT_TIMEOUT:
>>
>> do_timeout_processing(odp_timeout_from_event(ev), queue);
>> break;
>> case ODP_EVENT_PACKET:
>> do_packet_processing(odp_packet_from_event(ev),
>> queue);
>> break;
>> ...
>> default:
>> // Unknown event type, log and discard
>> }
>> }
>>
>> So now that processing loop needs to be updated even if this worker
>> thread doesn't care about that optional IPsec metadata.
>
>
> No, by default nothing changes. Only if application itself *enabled* IPSEC 
> processing, it would see PACKET_IPSEC events and would need to handle 
> those... but then application was already changed a lot (decided to use IPSEC 
> offload).
>

The application enabled IPsec processing in one, likely small, section
of the application code. If I have an existing large body of code it's
best not to have to comb through it updating every reference to packet
events to now have to consider other packet events that don't present
themselves as ODP_EVENT_PACKETs.

>
>>
>> A more flexible approach would be to define a new odp_packet_type_t
>> and associated odp_packet_type() API that identifies these subtypes.
>> That way the worker packet processing routine can be written as:
>>
>> void do_packet_processing(odp_packet_t pkt, odp_queue_t queue)
>> {
>> 
>> switch (odp_packet_type(pkt)) {
>> case ODP_PACKET_IPSEC:
>>   // do processing of interest with associated
>> IPsec metadata
>>   break;
>> case ODP_PACKET_COMP:
>>   // do processing of interest with associated
>> COMP metadata
>>   break;
>> default:
>> }
>>
>> ... process the basic packet itself
>> }
>
>
> Subtype is not needed - event type is the subtype already.
>
> We have a plan of adding odp_event_class_t instead (ODP_EVENT_CLASS_PACKET, 
> ...) as a super-type, but decided that it can wait for

Re: [lng-odp] [API-NEXT PATCHv3 1/4] api: feature: add odp feature bits

2017-06-12 Thread Savolainen, Petri (Nokia - FI/Espoo) 


> -Original Message-
> From: lng-odp [mailto:lng-odp-boun...@lists.linaro.org] On Behalf Of Bill
> Fischofer
> Sent: Monday, June 12, 2017 2:59 AM
> To: lng-odp@lists.linaro.org
> Subject: [lng-odp] [API-NEXT PATCHv3 1/4] api: feature: add odp feature
> bits
> 
> Add new odp_feature_t bits that permit other APIs/components to
> refer to various ODP features.
> 
> Signed-off-by: Bill Fischofer 
> ---
>  include/odp/api/spec/feature.h   | 60
> 
>  include/odp_api.h|  1 +
>  platform/Makefile.inc|  1 +
>  platform/linux-generic/Makefile.am   |  3 +-
>  platform/linux-generic/include/odp/api/feature.h | 34 ++
>  5 files changed, 98 insertions(+), 1 deletion(-)
>  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 ..a1cf4505
> --- /dev/null
> +++ b/include/odp/api/spec/feature.h
> @@ -0,0 +1,60 @@
> +/* Copyright (c) 2017, Linaro Limited
> + * All rights reserved.
> + *
> + * SPDX-License-Identifier: BSD-3-Clause
> + */
> +
> +
> +/**
> + * @file
> + *
> + * ODP features.
> + * Define various ODP feature sets that can be referenced by other
> + * components.
> + */
> +
> +#ifndef ODP_API_FEATURE_H_
> +#define ODP_API_FEATURE_H_
> +#include 
> +
> +#ifdef __cplusplus
> +extern "C" {
> +#endif
> +
> +#include 
> +#include 
> +#include 
> +#include 

These three includes above are not needed.


> +
> +/** @defgroup odp_features ODP_FEATURES
> + *  ODP feature definitions
> + *  @{
> + */
> +
> +/** Definition of ODP features */
> +typedef union odp_feature_t {
> + /** All features */
> + uint32_t all_feat;
> +
> + /** Individual feature bits */
> + struct {
> + uint32_t classification:1;

"classifier" or "cls" instead of classification. Packet IO API has 
"classifier_enable" parameter. IPSEC API has "ODP_IPSEC_PIPELINE_CLS" 
enumeration.

> + uint32_t crypto:1;
> + uint32_t ipsec:1;
> + uint32_t schedule:1;
> + uint32_t time:1;
> + uint32_t timer:1;
> + uint32_t traffic_mngr:1;
> + } feat;
> +} odp_feature_t;

Each feature bit would need some documentation about it. Application does not 
include spec directory header files, but odp_api.h. So, a "feature" needs to be 
defined other means than with file names. For example, what feature "cls" means 
... calls to odp_cls_xxx() / odp_cos_xxx() functions.


-Petri

[lng-odp] [RFC] api: classification: add random early discard

2017-06-12 Thread Balasubramanian Manoharan 
Add random early discard configuration to class-of-service

Signed-off-by: Balasubramanian Manoharan 
---
 include/odp/api/spec/classification.h | 44 +++
 1 file changed, 44 insertions(+)

diff --git a/include/odp/api/spec/classification.h 
b/include/odp/api/spec/classification.h
index 39831b2..badadff 100644
--- a/include/odp/api/spec/classification.h
+++ b/include/odp/api/spec/classification.h
@@ -128,6 +128,13 @@ typedef struct odp_cls_capability_t {
 
/** A Boolean to denote support of PMR range */
odp_bool_t pmr_range_supported;
+
+   /** Support for Random Early Discard */
+   odp_support_t random_early_discard;
+
+   /** Support for Back Pressure to the remote peer */
+   odp_support_t back_pressure;
+
 } odp_cls_capability_t;
 
 /**
@@ -167,6 +174,43 @@ typedef struct odp_cls_cos_param {
odp_queue_t queue;  /**< Queue associated with CoS */
odp_pool_t pool;/**< Pool associated with CoS */
odp_cls_drop_t drop_policy; /**< Drop policy associated with CoS */
+
+   /* Random Early Discard (RED)
+* Random Early discard is enabled to initiate a drop probability
+* for the incoming packet when the packets in the queue/pool reaches
+* a specified threshold.
+* When RED is enabled for a particular flow then further incoming
+* packets are assigned a drop probability based on the size of the
+* pool/queue and the drop probability becomes 1 when the queue/pool
+* is full.
+* RED is logically configured in the CoS and could be implemented
+* in either pool or queue linked to the CoS depending on
+* platform capabilities.
+* RED is controlled using maximum and minimum threshold values
+* which are defined as percentage of the system resource.
+* RED is enabled when the resource limit is equal to or greater than
+* the maximum threshold value and is disabled when resource limit
+* is less than or equal to minimum threshold value. */
+
+   /* A boolean to enable RED parameters */
+   odp_bool_t red_enable;
+
+   /* Maximum threshold percentage for RED */
+   uint16_t max_red_threshold;
+
+   /* Minimum threshold percentage for RED */
+   uint16_t min_red_threshold;
+
+   /* Back pressure
+* When back pressure is enabled for a particular flow, the HW can send
+* back pressure information to the remote peer indicating a network
+* congestion */
+   odp_bool_t bp_enable;
+
+   /* Threshold for enabling back pressure. BP is enabled when pool/queue
+* size is equal to or greater than this backpressure threshold.
+* BP threshold is expressed as a percentage of the resource size. */
+   uint16_t bp_threshold;
 } odp_cls_cos_param_t;
 
 /**
-- 
1.9.1

[lng-odp] [API-NEXT PATCH] linux-gen: queue: clean up after modular interface

2017-06-12 Thread Petri Savolainen 
Clean up function and parameter naming after modular interface
patch. Queue_t type is referred as "queue internal": queue_int or
q_int. Term "handle" is reserved for API level handles (e.g.
odp_queue_t, odp_pktio_t, etc) through out linux-gen implementation.

Signed-off-by: Petri Savolainen 
---
 platform/linux-generic/include/odp_queue_if.h  |  28 +--
 .../linux-generic/include/odp_queue_internal.h |   4 +-
 platform/linux-generic/include/odp_schedule_if.h   |   2 +-
 platform/linux-generic/odp_queue.c | 218 +++--
 platform/linux-generic/odp_schedule.c  |   4 +-
 platform/linux-generic/odp_schedule_iquery.c   |   4 +-
 platform/linux-generic/odp_schedule_sp.c   |   4 +-
 7 files changed, 133 insertions(+), 131 deletions(-)

diff --git a/platform/linux-generic/include/odp_queue_if.h 
b/platform/linux-generic/include/odp_queue_if.h
index 4359435f..168d0e9e 100644
--- a/platform/linux-generic/include/odp_queue_if.h
+++ b/platform/linux-generic/include/odp_queue_if.h
@@ -53,24 +53,24 @@ typedef int (*queue_term_global_fn_t)(void);
 typedef int (*queue_init_local_fn_t)(void);
 typedef int (*queue_term_local_fn_t)(void);
 typedef queue_t (*queue_from_ext_fn_t)(odp_queue_t handle);
-typedef odp_queue_t (*queue_to_ext_fn_t)(queue_t handle);
-typedef int (*queue_enq_fn_t)(queue_t handle, odp_buffer_hdr_t *);
-typedef int (*queue_enq_multi_fn_t)(queue_t handle, odp_buffer_hdr_t **, int);
-typedef odp_buffer_hdr_t *(*queue_deq_fn_t)(queue_t handle);
-typedef int (*queue_deq_multi_fn_t)(queue_t handle, odp_buffer_hdr_t **, int);
-typedef odp_pktout_queue_t (*queue_get_pktout_fn_t)(queue_t handle);
-typedef void (*queue_set_pktout_fn_t)(queue_t handle, odp_pktio_t pktio,
+typedef odp_queue_t (*queue_to_ext_fn_t)(queue_t q_int);
+typedef int (*queue_enq_fn_t)(queue_t q_int, odp_buffer_hdr_t *);
+typedef int (*queue_enq_multi_fn_t)(queue_t q_int, odp_buffer_hdr_t **, int);
+typedef odp_buffer_hdr_t *(*queue_deq_fn_t)(queue_t q_int);
+typedef int (*queue_deq_multi_fn_t)(queue_t q_int, odp_buffer_hdr_t **, int);
+typedef odp_pktout_queue_t (*queue_get_pktout_fn_t)(queue_t q_int);
+typedef void (*queue_set_pktout_fn_t)(queue_t q_int, odp_pktio_t pktio,
  int index);
-typedef odp_pktin_queue_t (*queue_get_pktin_fn_t)(queue_t handle);
-typedef void (*queue_set_pktin_fn_t)(queue_t handle, odp_pktio_t pktio,
+typedef odp_pktin_queue_t (*queue_get_pktin_fn_t)(queue_t q_int);
+typedef void (*queue_set_pktin_fn_t)(queue_t q_int, odp_pktio_t pktio,
 int index);
-typedef void (*queue_set_enq_fn_t)(queue_t handle, queue_enq_fn_t func);
-typedef void (*queue_set_enq_multi_fn_t)(queue_t handle,
+typedef void (*queue_set_enq_fn_t)(queue_t q_int, queue_enq_fn_t func);
+typedef void (*queue_set_enq_multi_fn_t)(queue_t q_int,
 queue_enq_multi_fn_t func);
-typedef void (*queue_set_deq_fn_t)(queue_t handle, queue_deq_fn_t func);
-typedef void (*queue_set_deq_multi_fn_t)(queue_t handle,
+typedef void (*queue_set_deq_fn_t)(queue_t q_int, queue_deq_fn_t func);
+typedef void (*queue_set_deq_multi_fn_t)(queue_t q_int,
 queue_deq_multi_fn_t func);
-typedef void (*queue_set_type_fn_t)(queue_t handle, odp_queue_type_t type);
+typedef void (*queue_set_type_fn_t)(queue_t q_int, odp_queue_type_t type);
 
 /* Queue functions towards other internal components */
 typedef struct {
diff --git a/platform/linux-generic/include/odp_queue_internal.h 
b/platform/linux-generic/include/odp_queue_internal.h
index 0c31ce8a..d79abd23 100644
--- a/platform/linux-generic/include/odp_queue_internal.h
+++ b/platform/linux-generic/include/odp_queue_internal.h
@@ -78,9 +78,9 @@ static inline uint32_t queue_to_id(odp_queue_t handle)
return _odp_typeval(handle) - 1;
 }
 
-static inline queue_entry_t *qentry_from_int(queue_t handle)
+static inline queue_entry_t *qentry_from_int(queue_t q_int)
 {
-   return (queue_entry_t *)(void *)(handle);
+   return (queue_entry_t *)(void *)(q_int);
 }
 
 static inline queue_t qentry_to_int(queue_entry_t *qentry)
diff --git a/platform/linux-generic/include/odp_schedule_if.h 
b/platform/linux-generic/include/odp_schedule_if.h
index 9adacef7..5d10cd37 100644
--- a/platform/linux-generic/include/odp_schedule_if.h
+++ b/platform/linux-generic/include/odp_schedule_if.h
@@ -26,7 +26,7 @@ typedef int (*schedule_init_queue_fn_t)(uint32_t queue_index,
 typedef void (*schedule_destroy_queue_fn_t)(uint32_t queue_index);
 typedef int (*schedule_sched_queue_fn_t)(uint32_t queue_index);
 typedef int (*schedule_unsched_queue_fn_t)(uint32_t queue_index);
-typedef int (*schedule_ord_enq_multi_fn_t)(queue_t handle,
+typedef int (*schedule_ord_enq_multi_fn_t)(queue_t q_int,
   void *buf_hdr[], int num, int *ret);
 typedef int (*schedule_init_global_fn_t)(void);

Re: [lng-odp] [API-NEXT PATCH 0/2] IPSEC packet event

2017-06-12 Thread Savolainen, Petri (Nokia - FI/Espoo) 


> -Original Message-
> From: Bill Fischofer [mailto:bill.fischo...@linaro.org]
> Sent: Friday, June 09, 2017 6:27 PM
> To: Savolainen, Petri (Nokia - FI/Espoo) 
> Cc: Dmitry Eremin-Solenikov ; lng-
> o...@lists.linaro.org
> Subject: Re: [lng-odp] [API-NEXT PATCH 0/2] IPSEC packet event
> 
> On Fri, Jun 9, 2017 at 7:06 AM, Savolainen, Petri (Nokia - FI/Espoo)
>  wrote:
> >
> >
> >> -Original Message-
> >> From: Dmitry Eremin-Solenikov
> [mailto:dmitry.ereminsoleni...@linaro.org]
> >> Sent: Friday, June 09, 2017 2:49 PM
> >> To: Petri Savolainen ; lng-
> >> o...@lists.linaro.org
> >> Subject: Re: [lng-odp] [API-NEXT PATCH 0/2] IPSEC packet event
> >>
> >> Petri,
> >>
> >> On 09.06.2017 13:51, Petri Savolainen wrote:
> >> > Requires "[API-NEXT PATCH v2 0/2] IPsec API update".
> >> >
> >> > Input and output of IPSEC operations are packets. Parameter and
> >> > result structures are cleaner when packet arrays are direct
> >> > parameters to functions. Also API is more flexible for
> >> > application or API pipelining when output is packets with
> >> > additional metadata. Application or API pipeline stages which
> >> > do not care about IPSEC results may work on basic packet metadata.
> >>
> >> Two generic issues:
> >>
> >> - I thought that basic idea was to generate ODP_EVENT_PACKET and then
> >> use API call to check if there is IPsec metadata associated with it?
> >>
> >
> > We ended up with defining odp_event_type() to be the call to check if
> there's extra metadata. The reason is that when ipsec decrypt fails, data
> is garbage, so application needs to check ipsec status always for those
> packets. With single event type, application would need to do the ipsec
> check also for non-ipsec packets. This API keeps normal packets as is.
> 
> It's sufficient for this to be reflected in the general packet error
> bits so application code that doesn't care about IPsec details simply
> knows that this packet has an error. Please see my earlier post about
> odp_packet_type().
>

Packet type would result two step check for every packet. At least for every 
ipsec/crypto/comp packet:

type = event_type(ev)

if (type == packet)
   pkt = packet_from_event(ev)  // generic
   subtype = packet_type(pkt)
   if (subtype == packet_ipsec) // 2nd branch
   foo();
   else
   bar();


Direct event type is more efficient for both application and implementation. 
Also implementation can e.g. finalize IPSEC processing during the IPSEC 
specific conversion function:

type = event_type(ev)

if (type == packet_ipsec)
   pkt = packet_ipsec_from_event(ev) // ipsec speficic
   foo();
else (type == packet_ipsec)
   pkt = packet_from_event(ev)   // basic pkt specific
   bar();


Class is a backward compatible addition later on:

class = event_class(ev)

if (class == packet)
   pkt = packet_from_class(ev)   // generic
   bar();
else



-Petri

Re: [lng-odp] [API-NEXT PATCH 1/2] api: ipsec: change IPSEC result to packet

2017-06-12 Thread Savolainen, Petri (Nokia - FI/Espoo) 
> >> >  /**
> >> >   * Get IPSEC status information from an ODP_EVENT_IPSEC_STATUS event
> >> > diff --git a/include/odp/arch/default/api/abi/event.h
> >> b/include/odp/arch/default/api/abi/event.h
> >> > index 87220d63..d807425b 100644
> >> > --- a/include/odp/arch/default/api/abi/event.h
> >> > +++ b/include/odp/arch/default/api/abi/event.h
> >> > @@ -27,9 +27,9 @@ typedef _odp_abi_event_t *odp_event_t;
> >> >  typedef enum odp_event_type_t {
> >> > ODP_EVENT_BUFFER   = 1,
> >> > ODP_EVENT_PACKET   = 2,
> >> > -   ODP_EVENT_TIMEOUT  = 3,
> >> > -   ODP_EVENT_CRYPTO_COMPL = 4,
> >> > -   ODP_EVENT_IPSEC_RESULT = 5,
> >> > +   ODP_EVENT_PACKET_IPSEC = 3,
> >> > +   ODP_EVENT_TIMEOUT  = 4,
> >> > +   ODP_EVENT_CRYPTO_COMPL = 5,
> >> > ODP_EVENT_IPSEC_STATUS = 6
> >> >  } odp_event_type_t;
> >>
> >> Any reason for rearrangement of existing values?
> >
> > Yes, it's now
> >
> > ODP_EVENT_BUFFER   = 1,
> > ODP_EVENT_PACKET   = 2,
> > ODP_EVENT_PACKET_IPSEC = 3,
> >
> > which may turn in couple of weeks into
> >
> > ODP_EVENT_BUFFER   = 1,
> > ODP_EVENT_PACKET   = 2,
> > ODP_EVENT_PACKET_IPSEC = 3,
> > ODP_EVENT_PACKET_CRYPTO = 4,
> > ODP_EVENT_PACKET_COMP = 5,
> >
> > ... normal packet and special ones.
> 
> The problem with trying to to this as an ever-growing list of event
> types, is that every time we introduce a new one every application
> needs to change. ODP_EVENT_PACKET_IPSEC is simply an ODP_EVENT_PACKET
> with some additional metadata that may or may not be of interest to a
> given application processing stage. So the basic architecture should
> reflect this fact.

Actually, the idea is that existing application *does not* have to change. 
EVENT_PACKET is still what it is today. So, current applications do not need to 
change anything: e.g. those do not need to check if a packet is coming from 
IPSEC and if the IPSEC operation has failed, etc.


> 
> The typical event loop for a worker thread would look like:
> 
> while (1) {
> odp_event_t ev = odp_schedule(, ODP_SCHED_WAIT);
> switch (odp_event_type(ev)) {
> case ODP_EVENT_BUFFER:
> do_buffer_processing(odp_buffer_from_event(ev),
> queue);
> break;
> case ODP_EVENT_TIMEOUT:
> 
> do_timeout_processing(odp_timeout_from_event(ev), queue);
> break;
> case ODP_EVENT_PACKET:
> do_packet_processing(odp_packet_from_event(ev),
> queue);
> break;
> ...
> default:
> // Unknown event type, log and discard
> }
> }
> 
> So now that processing loop needs to be updated even if this worker
> thread doesn't care about that optional IPsec metadata.


No, by default nothing changes. Only if application itself *enabled* IPSEC 
processing, it would see PACKET_IPSEC events and would need to handle those... 
but then application was already changed a lot (decided to use IPSEC offload).


> 
> A more flexible approach would be to define a new odp_packet_type_t
> and associated odp_packet_type() API that identifies these subtypes.
> That way the worker packet processing routine can be written as:
> 
> void do_packet_processing(odp_packet_t pkt, odp_queue_t queue)
> {
> 
> switch (odp_packet_type(pkt)) {
> case ODP_PACKET_IPSEC:
>   // do processing of interest with associated
> IPsec metadata
>   break;
> case ODP_PACKET_COMP:
>   // do processing of interest with associated
> COMP metadata
>   break;
> default:
> }
> 
> ... process the basic packet itself
> }


Subtype is not needed - event type is the subtype already.

We have a plan of adding odp_event_class_t instead (ODP_EVENT_CLASS_PACKET, 
...) as a super-type, but decided that it can wait for the actual need. The 
need is not yet there today as all packets are basic packets. After 
IPSEC/crypto/comp packet types are merged, it could be needed by an application 
pipeline stage which needs to do simple processing for all type packets. But 
even with that only successfully decrypted packets would go through that stage, 
otherwise failed decrypt would leave inner packet as garbage ...


> 
> Note that this means that anytime we add a new packet subtype nothing
> has to change at the application layer. It only needs to add code to
> process that optional metadata in stages that specifically want to do
> something with it. Otherwise they are treated as regular packets. So
> the fact that this packet was IPsec decrypted before it got to this
> stage is something the stage need not be aware of or care about unless
> it wants to take special action because the packet arrived via that
> offload path.