From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-we0-f171.google.com (mail-we0-f171.google.com [74.125.82.171]) (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 A6E082022BC for ; Sun, 24 Jun 2012 22:22:09 -0700 (PDT) Received: by wejx9 with SMTP id x9so4953845wej.16 for ; Sun, 24 Jun 2012 22:22:07 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=subject:from:to:cc:in-reply-to:references:content-type:date :message-id:mime-version:x-mailer:content-transfer-encoding; bh=GDk5EYgxxyIKQ8d2I9UEe4Zw5ANc6eMd5etwiCYfSxw=; b=XY6s7huWHEhBY8dBiMwo7ki4vdgnN7w6GyH920QCuxKvFPVrmB8KAn1NlOA3V2vl5E jQVZrWqrMRhT+8DjlSgzQnOgRfJ54KX7oITLNPix8+gvMINOB+z/2UNJ+31roStKa65U uhoVmH69U5X7aj+EF9Mdiwawdhzm1ZqgefaJ9BVgbf5M5sHVFwSHYHW4ZvuR5OiQgCzX IJmEUXrzAo6uOIJMeG1E6+QAVjTXwu07qf7W+LImL/Wfo7xMAiBUhzD27eoqVmjXW0dx OTEyJD9cHdNCAyC2pW453DkCU6142+oX0Z0mqCl2NgdV6eTvcrm1LlAdRxCYuTHRCudd kETw== Received: by 10.180.8.41 with SMTP id o9mr21594471wia.0.1340601726655; Sun, 24 Jun 2012 22:22:06 -0700 (PDT) Received: from [192.168.1.37] (122.237.66.86.rev.sfr.net. [86.66.237.122]) by mx.google.com with ESMTPS id eb8sm14847837wib.11.2012.06.24.22.22.04 (version=SSLv3 cipher=OTHER); Sun, 24 Jun 2012 22:22:05 -0700 (PDT) From: Eric Dumazet To: Dave =?ISO-8859-1?Q?T=E4ht?= In-Reply-To: <1340600422-1806-2-git-send-email-dave.taht@bufferbloat.net> References: <1340600422-1806-1-git-send-email-dave.taht@bufferbloat.net> <1340600422-1806-2-git-send-email-dave.taht@bufferbloat.net> Content-Type: text/plain; charset="UTF-8" Date: Mon, 25 Jun 2012 07:22:04 +0200 Message-ID: <1340601724.23933.16.camel@edumazet-glaptop> Mime-Version: 1.0 X-Mailer: Evolution 2.28.3 Content-Transfer-Encoding: 8bit Cc: codel@lists.bufferbloat.net Subject: Re: [Codel] [RFCv2 PATCH] codel: add ecn_target for when to drop rather than mark ecn packets 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: Mon, 25 Jun 2012 05:22:10 -0000 On Sun, 2012-06-24 at 22:00 -0700, Dave Täht wrote: > From: Dave Taht > > ECN can be gamed, and it is generally faster under overload > to drop rather than mark packets, to get back to a target. > > This patch adds support for ecn_target which controls when > codel and fq_codel will start dropping rather than marking > ecn tagged packets. > > The default is 15ms. > --- > include/linux/pkt_sched.h | 2 ++ > include/net/codel.h | 11 +++++++++-- > net/sched/sch_codel.c | 13 ++++++++++++- > net/sched/sch_fq_codel.c | 12 +++++++++++- > 4 files changed, 34 insertions(+), 4 deletions(-) > > diff --git a/include/linux/pkt_sched.h b/include/linux/pkt_sched.h > index 32aef0a..f0dcf8c 100644 > --- a/include/linux/pkt_sched.h > +++ b/include/linux/pkt_sched.h > @@ -663,6 +663,7 @@ enum { > TCA_CODEL_LIMIT, > TCA_CODEL_INTERVAL, > TCA_CODEL_ECN, > + TCA_CODEL_ECN_TARGET, > __TCA_CODEL_MAX > }; > > @@ -691,6 +692,7 @@ enum { > TCA_FQ_CODEL_ECN, > TCA_FQ_CODEL_FLOWS, > TCA_FQ_CODEL_QUANTUM, > + TCA_FQ_CODEL_ECN_TARGET, > __TCA_FQ_CODEL_MAX > }; > > diff --git a/include/net/codel.h b/include/net/codel.h > index 550debf..8a6a2b0 100644 > --- a/include/net/codel.h > +++ b/include/net/codel.h > @@ -111,11 +111,13 @@ static inline u32 codel_time_to_us(codel_time_t val) > * @target: target queue size (in time units) > * @interval: width of moving time window > * @ecn: is Explicit Congestion Notification enabled > + * @ecn_target: ecn target for queue drop anyway (in time units) > */ > struct codel_params { > codel_time_t target; > codel_time_t interval; > bool ecn; > + codel_time_t ecn_target; > }; > > /** > @@ -161,6 +163,7 @@ static void codel_params_init(struct codel_params *params) > params->interval = MS2TIME(100); > params->target = MS2TIME(5); > params->ecn = false; > + params->ecn_target = MS2TIME(15); > } > > static void codel_vars_init(struct codel_vars *vars) > @@ -280,7 +283,9 @@ 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)) { > + if (params->ecn && > + params->ecn_target > vars->ldelay && Wrong test ? We want ECN if delay < ecn_target, not if delay > ecn_target (unresponsive flows will make delay being above ecn_target, while responsive ones should make delay more like target ( < en_target) if (params->ecn && vars->ldelay <= params->ecn_target && INET_ECN_set_ce(skb)) { > + INET_ECN_set_ce(skb)) { > stats->ecn_mark++; > vars->drop_next = > codel_control_law(vars->drop_next, > @@ -305,7 +310,9 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch, > } > } > } else if (drop) { > - if (params->ecn && INET_ECN_set_ce(skb)) { > + if (params->ecn && > + params->ecn_target > vars->ldelay && > + INET_ECN_set_ce(skb)) { > stats->ecn_mark++; > } else { > qdisc_drop(skb, sch); > diff --git a/net/sched/sch_codel.c b/net/sched/sch_codel.c > index 2f9ab17..b008a35 100644 > --- a/net/sched/sch_codel.c > +++ b/net/sched/sch_codel.c > @@ -109,6 +109,7 @@ static const struct nla_policy codel_policy[TCA_CODEL_MAX + 1] = { > [TCA_CODEL_LIMIT] = { .type = NLA_U32 }, > [TCA_CODEL_INTERVAL] = { .type = NLA_U32 }, > [TCA_CODEL_ECN] = { .type = NLA_U32 }, > + [TCA_CODEL_ECN_TARGET] = { .type = NLA_U32 }, > }; > > static int codel_change(struct Qdisc *sch, struct nlattr *opt) > @@ -133,6 +134,13 @@ static int codel_change(struct Qdisc *sch, struct nlattr *opt) > q->params.target = ((u64)target * NSEC_PER_USEC) >> CODEL_SHIFT; > } > > + if (tb[TCA_CODEL_ECN_TARGET]) { > + u32 target = nla_get_u32(tb[TCA_CODEL_TARGET]); > + > + q->params.ecn_target = ((u64)target * NSEC_PER_USEC) >> > + CODEL_SHIFT; > + } > + > if (tb[TCA_CODEL_INTERVAL]) { > u32 interval = nla_get_u32(tb[TCA_CODEL_INTERVAL]); > > @@ -145,6 +153,7 @@ static int codel_change(struct Qdisc *sch, struct nlattr *opt) > if (tb[TCA_CODEL_ECN]) > q->params.ecn = !!nla_get_u32(tb[TCA_CODEL_ECN]); > > + extra new line ? > qlen = sch->q.qlen; > while (sch->q.qlen > sch->limit) { > struct sk_buff *skb = __skb_dequeue(&sch->q); > @@ -199,7 +208,9 @@ static int codel_dump(struct Qdisc *sch, struct sk_buff *skb) > nla_put_u32(skb, TCA_CODEL_INTERVAL, > codel_time_to_us(q->params.interval)) || > nla_put_u32(skb, TCA_CODEL_ECN, > - q->params.ecn)) > + q->params.ecn) || > + nla_put_u32(skb, TCA_CODEL_ECN_TARGET, > + codel_time_to_us(q->params.ecn_target))) > goto nla_put_failure; > > return nla_nest_end(skb, opts); > diff --git a/net/sched/sch_fq_codel.c b/net/sched/sch_fq_codel.c > index 9fc1c62..d974e07 100644 > --- a/net/sched/sch_fq_codel.c > +++ b/net/sched/sch_fq_codel.c > @@ -297,6 +297,7 @@ static const struct nla_policy fq_codel_policy[TCA_FQ_CODEL_MAX + 1] = { > [TCA_FQ_CODEL_ECN] = { .type = NLA_U32 }, > [TCA_FQ_CODEL_FLOWS] = { .type = NLA_U32 }, > [TCA_FQ_CODEL_QUANTUM] = { .type = NLA_U32 }, > + [TCA_FQ_CODEL_ECN_TARGET] = { .type = NLA_U32 }, > }; > > static int fq_codel_change(struct Qdisc *sch, struct nlattr *opt) > @@ -327,6 +328,13 @@ static int fq_codel_change(struct Qdisc *sch, struct nlattr *opt) > q->cparams.target = (target * NSEC_PER_USEC) >> CODEL_SHIFT; > } > > + if (tb[TCA_FQ_CODEL_ECN_TARGET]) { > + u64 target = nla_get_u32(tb[TCA_FQ_CODEL_TARGET]); > + > + q->cparams.ecn_target = (target * NSEC_PER_USEC) >> > + CODEL_SHIFT; > + } > + > if (tb[TCA_FQ_CODEL_INTERVAL]) { > u64 interval = nla_get_u32(tb[TCA_FQ_CODEL_INTERVAL]); > > @@ -447,7 +455,9 @@ static int fq_codel_dump(struct Qdisc *sch, struct sk_buff *skb) > nla_put_u32(skb, TCA_FQ_CODEL_QUANTUM, > q->quantum) || > nla_put_u32(skb, TCA_FQ_CODEL_FLOWS, > - q->flows_cnt)) > + q->flows_cnt) || > + nla_put_u32(skb, TCA_FQ_CODEL_ECN_TARGET, > + codel_time_to_us(q->cparams.ecn_target))) > goto nla_put_failure; > > nla_nest_end(skb, opts);