diff mbox

[RFC] api: classification: add random early discard

Message ID 1497267686-3590-1-git-send-email-bala.manoharan@linaro.org
State New
Headers show

Commit Message

Balasubramanian Manoharan June 12, 2017, 11:41 a.m. UTC
Add random early discard configuration to class-of-service

Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>

---
 include/odp/api/spec/classification.h | 44 +++++++++++++++++++++++++++++++++++
 1 file changed, 44 insertions(+)

-- 
1.9.1

Comments

Bill Fischofer June 12, 2017, 10:40 p.m. UTC | #1
On Mon, Jun 12, 2017 at 6:41 AM, Balasubramanian Manoharan
<bala.manoharan@linaro.org> wrote:
> Add random early discard configuration to class-of-service

>

> Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>

> ---

>  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

>
Balasubramanian Manoharan June 13, 2017, 2:38 p.m. UTC | #2
On 13 June 2017 at 04:10, Bill Fischofer <bill.fischofer@linaro.org> wrote:

> On Mon, Jun 12, 2017 at 6:41 AM, Balasubramanian Manoharan

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

> > Add random early discard configuration to class-of-service

> >

> > Signed-off-by: Balasubramanian Manoharan <bala.manoharan@linaro.org>

> > ---

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

>


Okay.


>

> ---

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

>

>

I would like to keep the first version simple and limit the scope to only
discard which is supported in most HWs. It would be better to get this
feature to ODP upstream ASAP. If some HW supports the different types of
RED then we could add those features with additional parameters as required
in the future update.


> > +

> > +       /* 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?

>


I have added the description in the above documentation, I can add more
description and yes it should be changed to uin8_t


> > +

> > +       /* 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.

>


I would prefer to leave those to implementation specific, If application
would like to have more control then it could be done at application level.


> > +

> > +       /* 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?

>


I believe all the parameters are specified as a percentage only.

Regards,
Bala

>

> >  } odp_cls_cos_param_t;

> >

> >  /**

> > --

> > 1.9.1

> >

>
diff mbox

Patch

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