CoDel AQM discussions
 help / color / mirror / Atom feed
From: "Dave Täht" <dave.taht@bufferbloat.net>
To: codel@lists.bufferbloat.net
Cc: Dave Taht <dave.taht@bufferbloat.net>
Subject: [Codel] [RFC PATCH] codel: ecn mark at target
Date: Fri,  3 Aug 2012 19:44:59 -0700	[thread overview]
Message-ID: <1344048299-26267-1-git-send-email-dave.taht@bufferbloat.net> (raw)

From: Dave Taht <dave.taht@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


             reply	other threads:[~2012-08-04  2:45 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-04  2:44 Dave Täht [this message]
2012-08-04  6:45 ` Eric Dumazet
2012-08-04 21:53   ` Kathleen Nichols
2012-08-05  3:06     ` Andrew McGregor
2012-08-05  5:30       ` Eric Dumazet
2012-08-05 16:53         ` Andrew McGregor
2012-08-05 16:58           ` Kathleen Nichols
2012-08-05 17:14             ` Andrew McGregor
2012-08-05 17:15           ` Eric Dumazet
2012-08-05 16:54         ` Richard Scheffenegger
2012-08-05 17:25           ` Eric Dumazet
2012-08-05 17:35             ` Eric Dumazet
2012-08-05 18:14               ` Yuchung Cheng
2012-08-05 18:40                 ` Eric Dumazet
2012-08-05 19:49                   ` Eric Dumazet
2012-08-06 16:22                   ` Richard Scheffenegger
2012-08-06 16:46                     ` Eric Dumazet
2012-08-06 17:50                       ` Dave Taht
2012-08-06 19:09                         ` Andrew McGregor
2012-08-06 20:01                           ` Eric Dumazet
2012-08-10 17:48                             ` Dave Taht
2012-08-04  7:00 ` Roger Jørgensen
2012-08-04 13:38   ` Richard Scheffenegger
2012-08-04 17:21     ` Eric Dumazet

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://lists.bufferbloat.net/postorius/lists/codel.lists.bufferbloat.net/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1344048299-26267-1-git-send-email-dave.taht@bufferbloat.net \
    --to=dave.taht@bufferbloat.net \
    --cc=codel@lists.bufferbloat.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox