DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH v3] lib/librte_sched: fix update tc_credits
@ 2017-07-06 13:44 Olivier Chirossel
  2017-07-06 13:44 ` [dpdk-dev] [PATCH] " Olivier Chirossel
  0 siblings, 1 reply; 3+ messages in thread
From: Olivier Chirossel @ 2017-07-06 13:44 UTC (permalink / raw)
  To: dev; +Cc: Olivier Chirossel

---

Olivier Chirossel (1):
  lib/librte_sched: fix update tc_credits
    Actualy ( for small rate ) if tc_credits_per_period < packets length
    all packets are drop. 
    Also the credits presents before the updade are loose, because
    tc_credits is set to tc_credits_per_period. The purpose of the
    patch is to fix that.

Signed-off-by: Olivier Chirossel <olivier.chirossel@gmail.com>

 doc/guides/prog_guide/qos_framework.rst |  10 ++-
 lib/librte_sched/rte_sched.c            | 124 ++++++++++++++++++++++++++------
 2 files changed, 112 insertions(+), 22 deletions(-)

-- 
2.7.4

^ permalink raw reply	[flat|nested] 3+ messages in thread

* [dpdk-dev] [PATCH] [PATCH v3] lib/librte_sched: fix update tc_credits
  2017-07-06 13:44 [dpdk-dev] [PATCH v3] lib/librte_sched: fix update tc_credits Olivier Chirossel
@ 2017-07-06 13:44 ` Olivier Chirossel
       [not found]   ` <71309e54-c9b1-2a47-0114-a018c9047a21@intel.com>
  0 siblings, 1 reply; 3+ messages in thread
From: Olivier Chirossel @ 2017-07-06 13:44 UTC (permalink / raw)
  To: dev; +Cc: Olivier Chirossel

Signed-off-by: Olivier Chirossel <olivier.chirossel@gmail.com>
---
 doc/guides/prog_guide/qos_framework.rst |  10 ++-
 lib/librte_sched/rte_sched.c            | 124 ++++++++++++++++++++++++++------
 2 files changed, 112 insertions(+), 22 deletions(-)

diff --git a/doc/guides/prog_guide/qos_framework.rst b/doc/guides/prog_guide/qos_framework.rst
index f3f60b8..e566ff9 100644
--- a/doc/guides/prog_guide/qos_framework.rst
+++ b/doc/guides/prog_guide/qos_framework.rst
@@ -799,6 +799,9 @@ as described in :numref:`table_qos_10` and :numref:`table_qos_11`.
    | 4 | tc_credits            | Bytes | Current upper limit for the number of credits that can be consumed by |
    |   |                       |       | the current traffic class for the remainder of the current            |
    |   |                       |       | enforcement period.                                                   |
+   |   |                       |       | when The credits is update (every tc_period) the                      |
+   |   |                       |       | tc_credits_per_period is added to the value (tc_credits) if the new   |
+   |   |                       |       | value is lower than tc_rate. else the value is set to tc_rate.        |
    |   |                       |       |                                                                       |
    +---+-----------------------+-------+-----------------------------------------------------------------------+
 
@@ -819,8 +822,11 @@ as described in :numref:`table_qos_10` and :numref:`table_qos_11`.
    |   |                          |                                                                            |
    |   |                          | if (time >= tc_time) {                                                     |
    |   |                          |                                                                            |
-   |   |                          | tc_credits = tc_credits_per_period;                                        |
-   |   |                          |                                                                            |
+   |   |                          | if (tc_credits + tc_credits_per_period < tc_rate) {                        |
+   |   |                          |       tc_credits +=  tc_credits_per_period                                 |
+   |   |                          | else {                                                                     |
+   |   |                          |       tc_credits = tc_rate                                                 |
+   |   |                          | }                                                                          |
    |   |                          | tc_time = time + tc_period;                                                |
    |   |                          |                                                                            |
    |   |                          | }                                                                          |
diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
index b7cba11..732d5ef 100644
--- a/lib/librte_sched/rte_sched.c
+++ b/lib/librte_sched/rte_sched.c
@@ -85,6 +85,7 @@ struct rte_sched_subport {
 
 	/* Traffic classes (TCs) */
 	uint64_t tc_time; /* time of next update */
+	uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];  
 	uint32_t tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
 	uint32_t tc_credits[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
 	uint32_t tc_period;
@@ -109,6 +110,7 @@ struct rte_sched_pipe_profile {
 	uint32_t tb_size;
 
 	/* Pipe traffic classes */
+	uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
 	uint32_t tc_period;
 	uint32_t tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
 	uint8_t tc_ov_weight;
@@ -568,11 +570,14 @@ rte_sched_port_config_pipe_profile_table(struct rte_sched_port *port, struct rte
 		/* Traffic Classes */
 		dst->tc_period = rte_sched_time_ms_to_bytes(src->tc_period,
 							    params->rate);
-
-		for (j = 0; j < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; j++)
+	
+		for (j = 0; j < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; j++) {
+		        dst->tc_rate[j] = src->tc_rate[j];
 			dst->tc_credits_per_period[j]
 				= rte_sched_time_ms_to_bytes(src->tc_period,
 							     src->tc_rate[j]);
+		}
+	
 
 #ifdef RTE_SCHED_SUBPORT_TC_OV
 		dst->tc_ov_weight = src->tc_ov_weight;
@@ -838,6 +843,7 @@ rte_sched_subport_config(struct rte_sched_port *port,
 	/* Traffic Classes (TCs) */
 	s->tc_period = rte_sched_time_ms_to_bytes(params->tc_period, port->rate);
 	for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) {
+	        s->tc_rate[i] =  params->tc_rate[i];
 		s->tc_credits_per_period[i]
 			= rte_sched_time_ms_to_bytes(params->tc_period,
 						     params->tc_rate[i]);
@@ -1495,19 +1501,59 @@ grinder_credits_update(struct rte_sched_port *port, uint32_t pos)
 
 	/* Subport TCs */
 	if (unlikely(port->time >= subport->tc_time)) {
-		subport->tc_credits[0] = subport->tc_credits_per_period[0];
-		subport->tc_credits[1] = subport->tc_credits_per_period[1];
-		subport->tc_credits[2] = subport->tc_credits_per_period[2];
-		subport->tc_credits[3] = subport->tc_credits_per_period[3];
+	        if (likely((subport->tc_credits[0] + subport->tc_credits_per_period[0]) < subport->tc_rate[0])) {
+		        subport->tc_credits[0] += subport->tc_credits_per_period[0];
+		}
+		else {
+		        subport->tc_credits[0] = subport->tc_rate[0];
+		}
+		if (likely((subport->tc_credits[1] + subport->tc_credits_per_period[1]) < subport->tc_rate[1])) {
+		        subport->tc_credits[1] += subport->tc_credits_per_period[1];
+		}
+		else {
+		        subport->tc_credits[1] = subport->tc_rate[1];
+		}
+		if (likely((subport->tc_credits[2] + subport->tc_credits_per_period[2]) < subport->tc_rate[2])) {
+		        subport->tc_credits[2] += subport->tc_credits_per_period[2];
+		}
+		else {
+		        subport->tc_credits[2] = subport->tc_rate[2];
+		}
+		if (likely((subport->tc_credits[3] + subport->tc_credits_per_period[3]) < subport->tc_rate[3])) {
+		        subport->tc_credits[3] += subport->tc_credits_per_period[3];
+		}
+		else {
+		        subport->tc_credits[3] = subport->tc_rate[3];
+		}
 		subport->tc_time = port->time + subport->tc_period;
 	}
 
 	/* Pipe TCs */
 	if (unlikely(port->time >= pipe->tc_time)) {
-		pipe->tc_credits[0] = params->tc_credits_per_period[0];
-		pipe->tc_credits[1] = params->tc_credits_per_period[1];
-		pipe->tc_credits[2] = params->tc_credits_per_period[2];
-		pipe->tc_credits[3] = params->tc_credits_per_period[3];
+	        if (likely((pipe->tc_credits[0] + params->tc_credits_per_period[0]) < params->tc_rate[0])) {
+		        pipe->tc_credits[0] += params->tc_credits_per_period[0];
+		}
+		else {
+		        pipe->tc_credits[0] = params->tc_rate[0];
+		}
+	        if (likely((pipe->tc_credits[1] + params->tc_credits_per_period[1]) < params->tc_rate[1])) {
+		        pipe->tc_credits[1] += params->tc_credits_per_period[1];
+		}
+		else {
+		        pipe->tc_credits[1] = params->tc_rate[1];
+		}
+	        if (likely((pipe->tc_credits[2] + params->tc_credits_per_period[2]) < params->tc_rate[2])) {
+		        pipe->tc_credits[2] += params->tc_credits_per_period[2];
+		}
+		else {
+		        pipe->tc_credits[2] = params->tc_rate[2];
+		}
+	        if (likely((pipe->tc_credits[3] + params->tc_credits_per_period[3]) < params->tc_rate[3])) {
+		        pipe->tc_credits[3] += params->tc_credits_per_period[3];
+		}
+		else {
+		        pipe->tc_credits[3] = params->tc_rate[3];
+		}
 		pipe->tc_time = port->time + params->tc_period;
 	}
 }
@@ -1573,22 +1619,60 @@ grinder_credits_update(struct rte_sched_port *port, uint32_t pos)
 	/* Subport TCs */
 	if (unlikely(port->time >= subport->tc_time)) {
 		subport->tc_ov_wm = grinder_tc_ov_credits_update(port, pos);
-
-		subport->tc_credits[0] = subport->tc_credits_per_period[0];
-		subport->tc_credits[1] = subport->tc_credits_per_period[1];
-		subport->tc_credits[2] = subport->tc_credits_per_period[2];
-		subport->tc_credits[3] = subport->tc_credits_per_period[3];
-
+		if (likely((subport->tc_credits[0] + subport->tc_credits_per_period[0]) < subport->tc_rate[0])) {
+		        subport->tc_credits[0] += subport->tc_credits_per_period[0];
+		}
+		else {
+		        subport->tc_credits[0] = subport->tc_rate[0];
+		}
+		if (likely((subport->tc_credits[1] + subport->tc_credits_per_period[1]) < subport->tc_rate[1])) {
+		        subport->tc_credits[1] += subport->tc_credits_per_period[1];
+		}
+		else {
+		        subport->tc_credits[1] = subport->tc_rate[1];
+		}
+		if (likely((subport->tc_credits[2] + subport->tc_credits_per_period[2]) < subport->tc_rate[2])) {
+		        subport->tc_credits[2] += subport->tc_credits_per_period[2];
+		}
+		else {
+		       subport->tc_credits[2] = subport->tc_rate[2];
+		}
+		if (likely((subport->tc_credits[3] + subport->tc_credits_per_period[3]) < subport->tc_rate[3])) {
+		        subport->tc_credits[3] += subport->tc_credits_per_period[3];
+		}
+		else {
+		        subport->tc_credits[3] = subport->tc_rate[3];
+		}
 		subport->tc_time = port->time + subport->tc_period;
 		subport->tc_ov_period_id++;
 	}
 
 	/* Pipe TCs */
 	if (unlikely(port->time >= pipe->tc_time)) {
-		pipe->tc_credits[0] = params->tc_credits_per_period[0];
-		pipe->tc_credits[1] = params->tc_credits_per_period[1];
-		pipe->tc_credits[2] = params->tc_credits_per_period[2];
-		pipe->tc_credits[3] = params->tc_credits_per_period[3];
+	        if (likely((pipe->tc_credits[0] + params->tc_credits_per_period[0]) < params->tc_rate[0])) {
+		        pipe->tc_credits[0] += params->tc_credits_per_period[0];
+		}
+		else {
+		        pipe->tc_credits[0] = params->tc_rate[0];
+		}
+	        if (likely((pipe->tc_credits[1] + params->tc_credits_per_period[1]) < params->tc_rate[1])) {
+		        pipe->tc_credits[1] += params->tc_credits_per_period[1];
+		}
+		else {
+		        pipe->tc_credits[1] = params->tc_rate[1];
+		}
+	        if (likely((pipe->tc_credits[2] + params->tc_credits_per_period[2]) < params->tc_rate[2])) {
+		        pipe->tc_credits[2] += params->tc_credits_per_period[2];
+		}
+		else {
+		        pipe->tc_credits[2] = params->tc_rate[2];
+		}
+	        if (likely((pipe->tc_credits[3] + params->tc_credits_per_period[3]) < params->tc_rate[3])) {
+		        pipe->tc_credits[3] += params->tc_credits_per_period[3];
+		}
+		else {
+		        pipe->tc_credits[3] = params->tc_rate[3];
+	        }
 		pipe->tc_time = port->time + params->tc_period;
 	}
 
-- 
2.7.4

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [dpdk-dev] [PATCH] [PATCH v3] lib/librte_sched: fix update tc_credits
       [not found]   ` <71309e54-c9b1-2a47-0114-a018c9047a21@intel.com>
@ 2017-09-22 15:52     ` Dumitrescu, Cristian
  0 siblings, 0 replies; 3+ messages in thread
From: Dumitrescu, Cristian @ 2017-09-22 15:52 UTC (permalink / raw)
  To: Olivier Chirossel; +Cc: dev

> 
> Signed-off-by: Olivier Chirossel <olivier.chirossel@gmail.com>
> ---
>  doc/guides/prog_guide/qos_framework.rst |  10 ++-
>  lib/librte_sched/rte_sched.c            | 124
> ++++++++++++++++++++++++++------
>  2 files changed, 112 insertions(+), 22 deletions(-)
> 
> diff --git a/doc/guides/prog_guide/qos_framework.rst
> b/doc/guides/prog_guide/qos_framework.rst
> index f3f60b8..e566ff9 100644
> --- a/doc/guides/prog_guide/qos_framework.rst
> +++ b/doc/guides/prog_guide/qos_framework.rst
> @@ -799,6 +799,9 @@ as described in :numref:`table_qos_10` and
> :numref:`table_qos_11`.
>     | 4 | tc_credits            | Bytes | Current upper limit for the
> number of credits that can be consumed by |
>     |   |                       |       | the current traffic class for
> the remainder of the current            |
>     |   |                       |       | enforcement period.
>                                        |
> +   |   |                       |       | when The credits is update
> (every tc_period) the                      |
> +   |   |                       |       | tc_credits_per_period is added
> to the value (tc_credits) if the new   |
> +   |   |                       |       | value is lower than tc_rate.
> else the value is set to tc_rate.        |
>     |   |                       |       |
>                                        |
> 
> +---+-----------------------+-------+------------------------------------------------------
> -----------------+
>  @@ -819,8 +822,11 @@ as described in :numref:`table_qos_10` and
> :numref:`table_qos_11`.
>     |   |                          |
>                                        |
>     |   |                          | if (time >= tc_time) {
>                                        |
>     |   |                          |
>                                        |
> -   |   |                          | tc_credits = tc_credits_per_period;
>                                        |
> -   |   |                          |
>                                        |
> +   |   |                          | if (tc_credits +
> tc_credits_per_period < tc_rate) {                        |
> +   |   |                          |       tc_credits +=
> tc_credits_per_period                                 |
> +   |   |                          | else {
>                                        |
> +   |   |                          |       tc_credits = tc_rate
>                                        |
> +   |   |                          | }
>                                        |
>     |   |                          | tc_time = time + tc_period;
>                                        |
>     |   |                          |
>                                        |
>     |   |                          | }
>                                        |
> diff --git a/lib/librte_sched/rte_sched.c b/lib/librte_sched/rte_sched.c
> index b7cba11..732d5ef 100644
> --- a/lib/librte_sched/rte_sched.c
> +++ b/lib/librte_sched/rte_sched.c
> @@ -85,6 +85,7 @@ struct rte_sched_subport {
>   	/* Traffic classes (TCs) */
>  	uint64_t tc_time; /* time of next update */
> +	uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
> 	uint32_t
> tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
>  	uint32_t tc_credits[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
>  	uint32_t tc_period;
> @@ -109,6 +110,7 @@ struct rte_sched_pipe_profile {
>  	uint32_t tb_size;
>   	/* Pipe traffic classes */
> +	uint32_t tc_rate[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
>  	uint32_t tc_period;
>  	uint32_t
> tc_credits_per_period[RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE];
>  	uint8_t tc_ov_weight;
> @@ -568,11 +570,14 @@ rte_sched_port_config_pipe_profile_table(struct
> rte_sched_port *port, struct rte
>  		/* Traffic Classes */
>  		dst->tc_period = rte_sched_time_ms_to_bytes(src-
> >tc_period,
>  							    params->rate);
> -
> -		for (j = 0; j < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; j++)
> +
> +		for (j = 0; j < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; j++) {
> +		        dst->tc_rate[j] = src->tc_rate[j];
>  			dst->tc_credits_per_period[j]
>  				= rte_sched_time_ms_to_bytes(src-
> >tc_period,
>  							     src->tc_rate[j]);
> +		}
> +
>   #ifdef RTE_SCHED_SUBPORT_TC_OV
>  		dst->tc_ov_weight = src->tc_ov_weight;
> @@ -838,6 +843,7 @@ rte_sched_subport_config(struct rte_sched_port
> *port,
>  	/* Traffic Classes (TCs) */
>  	s->tc_period = rte_sched_time_ms_to_bytes(params->tc_period,
> port->rate);
>  	for (i = 0; i < RTE_SCHED_TRAFFIC_CLASSES_PER_PIPE; i++) {
> +	        s->tc_rate[i] =  params->tc_rate[i];
>  		s->tc_credits_per_period[i]
>  			= rte_sched_time_ms_to_bytes(params->tc_period,
>  						     params->tc_rate[i]);
> @@ -1495,19 +1501,59 @@ grinder_credits_update(struct rte_sched_port
> *port, uint32_t pos)
>   	/* Subport TCs */
>  	if (unlikely(port->time >= subport->tc_time)) {
> -		subport->tc_credits[0] = subport->tc_credits_per_period[0];
> -		subport->tc_credits[1] = subport->tc_credits_per_period[1];
> -		subport->tc_credits[2] = subport->tc_credits_per_period[2];
> -		subport->tc_credits[3] = subport->tc_credits_per_period[3];
> +	        if (likely((subport->tc_credits[0] +
> subport->tc_credits_per_period[0]) < subport->tc_rate[0])) {
> +		        subport->tc_credits[0] += subport-
> >tc_credits_per_period[0];
> +		}
> +		else {
> +		        subport->tc_credits[0] = subport->tc_rate[0];
> +		}
> +		if (likely((subport->tc_credits[1] +
> subport->tc_credits_per_period[1]) < subport->tc_rate[1])) {
> +		        subport->tc_credits[1] += subport-
> >tc_credits_per_period[1];
> +		}
> +		else {
> +		        subport->tc_credits[1] = subport->tc_rate[1];
> +		}
> +		if (likely((subport->tc_credits[2] +
> subport->tc_credits_per_period[2]) < subport->tc_rate[2])) {
> +		        subport->tc_credits[2] += subport-
> >tc_credits_per_period[2];
> +		}
> +		else {
> +		        subport->tc_credits[2] = subport->tc_rate[2];
> +		}
> +		if (likely((subport->tc_credits[3] +
> subport->tc_credits_per_period[3]) < subport->tc_rate[3])) {
> +		        subport->tc_credits[3] += subport-
> >tc_credits_per_period[3];
> +		}
> +		else {
> +		        subport->tc_credits[3] = subport->tc_rate[3];
> +		}
>  		subport->tc_time = port->time + subport->tc_period;
>  	}
>   	/* Pipe TCs */
>  	if (unlikely(port->time >= pipe->tc_time)) {
> -		pipe->tc_credits[0] = params->tc_credits_per_period[0];
> -		pipe->tc_credits[1] = params->tc_credits_per_period[1];
> -		pipe->tc_credits[2] = params->tc_credits_per_period[2];
> -		pipe->tc_credits[3] = params->tc_credits_per_period[3];
> +	        if (likely((pipe->tc_credits[0] +
> params->tc_credits_per_period[0]) < params->tc_rate[0])) {
> +		        pipe->tc_credits[0] += params-
> >tc_credits_per_period[0];
> +		}
> +		else {
> +		        pipe->tc_credits[0] = params->tc_rate[0];
> +		}
> +	        if (likely((pipe->tc_credits[1] +
> params->tc_credits_per_period[1]) < params->tc_rate[1])) {
> +		        pipe->tc_credits[1] += params-
> >tc_credits_per_period[1];
> +		}
> +		else {
> +		        pipe->tc_credits[1] = params->tc_rate[1];
> +		}
> +	        if (likely((pipe->tc_credits[2] +
> params->tc_credits_per_period[2]) < params->tc_rate[2])) {
> +		        pipe->tc_credits[2] += params-
> >tc_credits_per_period[2];
> +		}
> +		else {
> +		        pipe->tc_credits[2] = params->tc_rate[2];
> +		}
> +	        if (likely((pipe->tc_credits[3] +
> params->tc_credits_per_period[3]) < params->tc_rate[3])) {
> +		        pipe->tc_credits[3] += params-
> >tc_credits_per_period[3];
> +		}
> +		else {
> +		        pipe->tc_credits[3] = params->tc_rate[3];
> +		}
>  		pipe->tc_time = port->time + params->tc_period;
>  	}
>  }
> @@ -1573,22 +1619,60 @@ grinder_credits_update(struct rte_sched_port
> *port, uint32_t pos)
>  	/* Subport TCs */
>  	if (unlikely(port->time >= subport->tc_time)) {
>  		subport->tc_ov_wm = grinder_tc_ov_credits_update(port,
> pos);
> -
> -		subport->tc_credits[0] = subport->tc_credits_per_period[0];
> -		subport->tc_credits[1] = subport->tc_credits_per_period[1];
> -		subport->tc_credits[2] = subport->tc_credits_per_period[2];
> -		subport->tc_credits[3] = subport->tc_credits_per_period[3];
> -
> +		if (likely((subport->tc_credits[0] +
> subport->tc_credits_per_period[0]) < subport->tc_rate[0])) {
> +		        subport->tc_credits[0] += subport-
> >tc_credits_per_period[0];
> +		}
> +		else {
> +		        subport->tc_credits[0] = subport->tc_rate[0];
> +		}
> +		if (likely((subport->tc_credits[1] +
> subport->tc_credits_per_period[1]) < subport->tc_rate[1])) {
> +		        subport->tc_credits[1] += subport-
> >tc_credits_per_period[1];
> +		}
> +		else {
> +		        subport->tc_credits[1] = subport->tc_rate[1];
> +		}
> +		if (likely((subport->tc_credits[2] +
> subport->tc_credits_per_period[2]) < subport->tc_rate[2])) {
> +		        subport->tc_credits[2] += subport-
> >tc_credits_per_period[2];
> +		}
> +		else {
> +		       subport->tc_credits[2] = subport->tc_rate[2];
> +		}
> +		if (likely((subport->tc_credits[3] +
> subport->tc_credits_per_period[3]) < subport->tc_rate[3])) {
> +		        subport->tc_credits[3] += subport-
> >tc_credits_per_period[3];
> +		}
> +		else {
> +		        subport->tc_credits[3] = subport->tc_rate[3];
> +		}
>  		subport->tc_time = port->time + subport->tc_period;
>  		subport->tc_ov_period_id++;
>  	}
>   	/* Pipe TCs */
>  	if (unlikely(port->time >= pipe->tc_time)) {
> -		pipe->tc_credits[0] = params->tc_credits_per_period[0];
> -		pipe->tc_credits[1] = params->tc_credits_per_period[1];
> -		pipe->tc_credits[2] = params->tc_credits_per_period[2];
> -		pipe->tc_credits[3] = params->tc_credits_per_period[3];
> +	        if (likely((pipe->tc_credits[0] +
> params->tc_credits_per_period[0]) < params->tc_rate[0])) {
> +		        pipe->tc_credits[0] += params-
> >tc_credits_per_period[0];
> +		}
> +		else {
> +		        pipe->tc_credits[0] = params->tc_rate[0];
> +		}
> +	        if (likely((pipe->tc_credits[1] +
> params->tc_credits_per_period[1]) < params->tc_rate[1])) {
> +		        pipe->tc_credits[1] += params-
> >tc_credits_per_period[1];
> +		}
> +		else {
> +		        pipe->tc_credits[1] = params->tc_rate[1];
> +		}
> +	        if (likely((pipe->tc_credits[2] +
> params->tc_credits_per_period[2]) < params->tc_rate[2])) {
> +		        pipe->tc_credits[2] += params-
> >tc_credits_per_period[2];
> +		}
> +		else {
> +		        pipe->tc_credits[2] = params->tc_rate[2];
> +		}
> +	        if (likely((pipe->tc_credits[3] +
> params->tc_credits_per_period[3]) < params->tc_rate[3])) {
> +		        pipe->tc_credits[3] += params-
> >tc_credits_per_period[3];
> +		}
> +		else {
> +		        pipe->tc_credits[3] = params->tc_rate[3];
> +	        }
>  		pipe->tc_time = port->time + params->tc_period;
>  	}
>  -- 2.7.4
> 

Hi Olivier,

Thanks for your patch, I have reviewed it in detail, but I have to NAK it.

This patch might help for some particular case related to low rate pipe configuration, but it breaks everything else.

Right now, to avoid the MTU problem that you are describing, the solution for low rate pipe is to pick the tc_period in such a way that tc_credits_per_period is bigger than MTU.
Examples: tc_period = 40 ms, MTU = 1500 bytes
a) pipe rate = tc rate = 1 MB/s (high) => tc_credits_per_period = 40 kB > MTU (OK)
b) pipe rate = tc rate = 1 kB/s => tc_credits_per_period = 40 B < MTU (not OK)
c) pipe rate = tc rate = 40 kB/s => tc_credits_per_period = 1600 B ~= MTU (this is the threshold for minimum pipe rate for given tc_period)

I agree this could be improved, but your proposed fix does not achieve this; it only disables the tc rate enforcement mechanism by setting tc credits to a very high rate (i.e. tc_rate) that is not correlated with tc_period in any way.

So, NAK for now.

Regards,
Cristian


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-09-22 15:52 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-07-06 13:44 [dpdk-dev] [PATCH v3] lib/librte_sched: fix update tc_credits Olivier Chirossel
2017-07-06 13:44 ` [dpdk-dev] [PATCH] " Olivier Chirossel
     [not found]   ` <71309e54-c9b1-2a47-0114-a018c9047a21@intel.com>
2017-09-22 15:52     ` Dumitrescu, Cristian

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).