[Codel] [RFC PATCH] codel: ecn mark at target

[Dave Täht]

From: Dave Taht <dave.taht at bufferbloat.net>

The consensus at ietf was that ecn marking should start at
target, and then the results fed into the codel drop scheduler.

While I agree with the latter, I feel that waiting an interval
before starting to mark will be more in-tune with the concept
of a sojourn time, and lead to better utilization.

As I am outnumbered and outgunned, do it at target.
---
 include/net/codel.h |   30 ++++++++++--------------------
 1 file changed, 10 insertions(+), 20 deletions(-)

diff --git a/include/net/codel.h b/include/net/codel.h
index 550debf..f06d99c 100644
--- a/include/net/codel.h
+++ b/include/net/codel.h
@@ -204,7 +204,7 @@ static codel_time_t codel_control_law(codel_time_t t,
 }
 
 
-static bool codel_should_drop(const struct sk_buff *skb,
+static bool codel_should_drop(struct sk_buff *skb,
 			      struct Qdisc *sch,
 			      struct codel_vars *vars,
 			      struct codel_params *params,
@@ -231,6 +231,9 @@ static bool codel_should_drop(const struct sk_buff *skb,
 		return false;
 	}
 	ok_to_drop = false;
+	/* Above target, start marking */
+	if (params->ecn && INET_ECN_set_ce(skb))
+		stats->ecn_mark++; 
 	if (vars->first_above_time == 0) {
 		/* just went above from below. If we stay above
 		 * for at least interval we'll say it's ok to drop
@@ -238,6 +241,7 @@ static bool codel_should_drop(const struct sk_buff *skb,
 		vars->first_above_time = now + params->interval;
 	} else if (codel_time_after(now, vars->first_above_time)) {
 		ok_to_drop = true;
+	/* dtaht: I think we should start marking here, instead */
 	}
 	return ok_to_drop;
 }
@@ -280,14 +284,6 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch,
 						* since there is no more divide
 						*/
 				codel_Newton_step(vars);
-				if (params->ecn && INET_ECN_set_ce(skb)) {
-					stats->ecn_mark++;
-					vars->drop_next =
-						codel_control_law(vars->drop_next,
-								  params->interval,
-								  vars->rec_inv_sqrt);
-					goto end;
-				}
 				qdisc_drop(skb, sch);
 				stats->drop_count++;
 				skb = dequeue_func(vars, sch);
@@ -305,16 +301,11 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch,
 			}
 		}
 	} else if (drop) {
-		if (params->ecn && INET_ECN_set_ce(skb)) {
-			stats->ecn_mark++;
-		} else {
-			qdisc_drop(skb, sch);
-			stats->drop_count++;
-
-			skb = dequeue_func(vars, sch);
-			drop = codel_should_drop(skb, sch, vars, params,
-						 stats, now);
-		}
+		qdisc_drop(skb, sch);
+		stats->drop_count++;
+		skb = dequeue_func(vars, sch);
+		drop = codel_should_drop(skb, sch, vars, params,
+					 stats, now);
 		vars->dropping = true;
 		/* if min went above target close to when we last went below it
 		 * assume that the drop rate that controlled the queue on the
@@ -336,7 +327,6 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch,
 		vars->drop_next = codel_control_law(now, params->interval,
 						    vars->rec_inv_sqrt);
 	}
-end:
 	return skb;
 }
 #endif
-- 
1.7.9.5