From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-bk0-f43.google.com (mail-bk0-f43.google.com [209.85.214.43]) (using TLSv1 with cipher RC4-SHA (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority" (verified OK)) by huchra.bufferbloat.net (Postfix) with ESMTPS id 6E2622022B8; Wed, 16 May 2012 07:39:16 -0700 (PDT) Received: by bkty5 with SMTP id y5so1459626bkt.16 for ; Wed, 16 May 2012 07:39:14 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:from:to:cc:content-type:date:message-id:mime-version :x-mailer:content-transfer-encoding; bh=Gt+MzkxPTToQoLdGi5GWTRFjP9BkTCUbb9a/hsNeYHA=; b=Pacu566qbjxYgpU9j+YEr6jiaKac4s+NLwdUmx2N9gnTEWMPQOzGPfu9kCFyVdXfkj CRkMX3Yq6bDW72qvp8jfcLzyuJiv7t1+0iQVfW3AMJTlcodAJFNd0qUjEvQGjgCse1sy 6xsFLTydxtyVOI3DWcJdz4SPr4hjlKVcKhM/daJX7IMidnUji38HZrfs9gnpjB0kz2Hd G2R23eeq4I7Yi1iyJK6bZL/1lRAGqQEKWzu/LeM1oC3SgX0TKWlKKjFxhWKTzfJ7/vU7 f0GitkT5ozBf8FWm3qegNYpiwh+Kje/iMmojpREPMaoTGyjeGgxSaSUsGI9QAg0X5cgt k7Ow== Received: by 10.204.157.6 with SMTP id z6mr1372173bkw.15.1337179153942; Wed, 16 May 2012 07:39:13 -0700 (PDT) Received: from [172.28.91.41] ([74.125.122.49]) by mx.google.com with ESMTPS id e20sm5640851bkw.3.2012.05.16.07.39.11 (version=SSLv3 cipher=OTHER); Wed, 16 May 2012 07:39:12 -0700 (PDT) From: Eric Dumazet To: David Miller Content-Type: text/plain; charset="UTF-8" Date: Wed, 16 May 2012 16:39:09 +0200 Message-ID: <1337179149.8512.1208.camel@edumazet-glaptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Content-Transfer-Encoding: 7bit Cc: Dave Taht , netdev , codel@lists.bufferbloat.net, bloat@lists.bufferbloat.net Subject: [Codel] [PATCH net-next] fq_codel: should use qdisc backlog as threshold X-BeenThere: codel@lists.bufferbloat.net X-Mailman-Version: 2.1.13 Precedence: list List-Id: CoDel AQM discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 16 May 2012 14:39:19 -0000 From: Eric Dumazet codel_should_drop() logic allows a packet being not dropped if queue size is under max packet size. In fq_codel, we have two possible backlogs : The qdisc global one, and the flow local one. The meaningful one for codel_should_drop() should be the global backlog, not the per flow one, so that thin flows can have a non zero drop/mark probability. Signed-off-by: Eric Dumazet Cc: Dave Taht Cc: Kathleen Nichols Cc: Van Jacobson --- include/net/codel.h | 15 +++++++-------- net/sched/sch_codel.c | 4 ++-- net/sched/sch_fq_codel.c | 5 +++-- 3 files changed, 12 insertions(+), 12 deletions(-) diff --git a/include/net/codel.h b/include/net/codel.h index 7546517..550debf 100644 --- a/include/net/codel.h +++ b/include/net/codel.h @@ -205,7 +205,7 @@ static codel_time_t codel_control_law(codel_time_t t, static bool codel_should_drop(const struct sk_buff *skb, - unsigned int *backlog, + struct Qdisc *sch, struct codel_vars *vars, struct codel_params *params, struct codel_stats *stats, @@ -219,13 +219,13 @@ static bool codel_should_drop(const struct sk_buff *skb, } vars->ldelay = now - codel_get_enqueue_time(skb); - *backlog -= qdisc_pkt_len(skb); + sch->qstats.backlog -= qdisc_pkt_len(skb); if (unlikely(qdisc_pkt_len(skb) > stats->maxpacket)) stats->maxpacket = qdisc_pkt_len(skb); if (codel_time_before(vars->ldelay, params->target) || - *backlog <= stats->maxpacket) { + sch->qstats.backlog <= stats->maxpacket) { /* went below - stay below for at least interval */ vars->first_above_time = 0; return false; @@ -249,8 +249,7 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch, struct codel_params *params, struct codel_vars *vars, struct codel_stats *stats, - codel_skb_dequeue_t dequeue_func, - u32 *backlog) + codel_skb_dequeue_t dequeue_func) { struct sk_buff *skb = dequeue_func(vars, sch); codel_time_t now; @@ -261,7 +260,7 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch, return skb; } now = codel_get_time(); - drop = codel_should_drop(skb, backlog, vars, params, stats, now); + drop = codel_should_drop(skb, sch, vars, params, stats, now); if (vars->dropping) { if (!drop) { /* sojourn time below target - leave dropping state */ @@ -292,7 +291,7 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch, qdisc_drop(skb, sch); stats->drop_count++; skb = dequeue_func(vars, sch); - if (!codel_should_drop(skb, backlog, + if (!codel_should_drop(skb, sch, vars, params, stats, now)) { /* leave dropping state */ vars->dropping = false; @@ -313,7 +312,7 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch, stats->drop_count++; skb = dequeue_func(vars, sch); - drop = codel_should_drop(skb, backlog, vars, params, + drop = codel_should_drop(skb, sch, vars, params, stats, now); } vars->dropping = true; diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c index 213ef60..2f9ab17 100644 --- a/net/sched/sch_codel.c +++ b/net/sched/sch_codel.c @@ -77,8 +77,8 @@ static struct sk_buff *codel_qdisc_dequeue(struct Qdisc *sch) struct codel_sched_data *q = qdisc_priv(sch); struct sk_buff *skb; - skb = codel_dequeue(sch, &q->params, &q->vars, &q->stats, - dequeue, &sch->qstats.backlog); + skb = codel_dequeue(sch, &q->params, &q->vars, &q->stats, dequeue); + /* We cant call qdisc_tree_decrease_qlen() if our qlen is 0, * or HTB crashes. Defer it for next round. */ diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c index 337ff20..9fc1c62 100644 --- a/net/sched/sch_fq_codel.c +++ b/net/sched/sch_fq_codel.c @@ -217,13 +217,14 @@ static int fq_codel_enqueue(struct sk_buff *skb, struct Qdisc *sch) */ static struct sk_buff *dequeue(struct codel_vars *vars, struct Qdisc *sch) { + struct fq_codel_sched_data *q = qdisc_priv(sch); struct fq_codel_flow *flow; struct sk_buff *skb = NULL; flow = container_of(vars, struct fq_codel_flow, cvars); if (flow->head) { skb = dequeue_head(flow); - sch->qstats.backlog -= qdisc_pkt_len(skb); + q->backlogs[flow - q->flows] -= qdisc_pkt_len(skb); sch->q.qlen--; } return skb; @@ -256,7 +257,7 @@ begin: prev_ecn_mark = q->cstats.ecn_mark; skb = codel_dequeue(sch, &q->cparams, &flow->cvars, &q->cstats, - dequeue, &q->backlogs[flow - q->flows]); + dequeue); flow->dropped += q->cstats.drop_count - prev_drop_count; flow->dropped += q->cstats.ecn_mark - prev_ecn_mark;