From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wi0-f175.google.com (mail-wi0-f175.google.com [209.85.212.175]) (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 5F68D2008E8 for ; Mon, 25 Jun 2012 07:50:35 -0700 (PDT) Received: by wibhm2 with SMTP id hm2so941285wib.10 for ; Mon, 25 Jun 2012 07:50:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=mime-version:in-reply-to:references:date:message-id:subject:from:to :cc:content-type:content-transfer-encoding; bh=UbLkJwv6ZufPj2/Ikqt7DMuQme9GBbk93SLS28+BTJY=; b=e8U9XeYEtjpA5jbcU+wt4qIUv117n1tBVhHXr5yji8VLLjRTLoaqcia/r+MFOO8v9t xXpBqFfR6Cvij8GmcFpYm0XAMH4wsOKTiqgJI0RExLQFO95yBNaUAhwdJQQ+LakKaNMl QNyD9rFKe1tl+7hWjN9kj1ypDOkN/z7t5bW9fwuRxxdmrWLH4cmUoRF5AbcHIXyQ6Sg6 4n2/QmxWEC8aGh0W10InMp9ZaW339tSdgMhH6xoJIjFQJberHgo2V2ks52zJqjbFZpX5 hPrRDKLCIyGX63UYLyPB9U6k779K8uEZAJxYWSoMuPr2r5lB1t5dvaqbzig3kly2nfCK Oaqg== MIME-Version: 1.0 Received: by 10.180.82.198 with SMTP id k6mr25602560wiy.20.1340635832846; Mon, 25 Jun 2012 07:50:32 -0700 (PDT) Received: by 10.223.103.199 with HTTP; Mon, 25 Jun 2012 07:50:32 -0700 (PDT) In-Reply-To: <1340601724.23933.16.camel@edumazet-glaptop> References: <1340600422-1806-1-git-send-email-dave.taht@bufferbloat.net> <1340600422-1806-2-git-send-email-dave.taht@bufferbloat.net> <1340601724.23933.16.camel@edumazet-glaptop> Date: Mon, 25 Jun 2012 07:50:32 -0700 Message-ID: From: Dave Taht To: Eric Dumazet Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: codel@lists.bufferbloat.net, =?ISO-8859-1?Q?Dave_T=E4ht?= 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 14:50:36 -0000 On Sun, Jun 24, 2012 at 10:22 PM, Eric Dumazet wro= te: > On Sun, 2012-06-24 at 22:00 -0700, Dave T=E4ht 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. >> --- >> =A0include/linux/pkt_sched.h | =A0 =A02 ++ >> =A0include/net/codel.h =A0 =A0 =A0 | =A0 11 +++++++++-- >> =A0net/sched/sch_codel.c =A0 =A0 | =A0 13 ++++++++++++- >> =A0net/sched/sch_fq_codel.c =A0| =A0 12 +++++++++++- >> =A04 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 { >> =A0 =A0 =A0 TCA_CODEL_LIMIT, >> =A0 =A0 =A0 TCA_CODEL_INTERVAL, >> =A0 =A0 =A0 TCA_CODEL_ECN, >> + =A0 =A0 TCA_CODEL_ECN_TARGET, >> =A0 =A0 =A0 __TCA_CODEL_MAX >> =A0}; >> >> @@ -691,6 +692,7 @@ enum { >> =A0 =A0 =A0 TCA_FQ_CODEL_ECN, >> =A0 =A0 =A0 TCA_FQ_CODEL_FLOWS, >> =A0 =A0 =A0 TCA_FQ_CODEL_QUANTUM, >> + =A0 =A0 TCA_FQ_CODEL_ECN_TARGET, >> =A0 =A0 =A0 __TCA_FQ_CODEL_MAX >> =A0}; >> >> 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 va= l) >> =A0 * @target: =A0target queue size (in time units) >> =A0 * @interval: =A0 =A0 =A0 =A0width of moving time window >> =A0 * @ecn: =A0 =A0 is Explicit Congestion Notification enabled >> + * @ecn_target: =A0 =A0 =A0ecn target for queue drop anyway (in time un= its) >> =A0 */ >> =A0struct codel_params { >> =A0 =A0 =A0 codel_time_t =A0 =A0target; >> =A0 =A0 =A0 codel_time_t =A0 =A0interval; >> =A0 =A0 =A0 bool =A0 =A0 =A0 =A0 =A0 =A0ecn; >> + =A0 =A0 codel_time_t =A0 =A0ecn_target; >> =A0}; >> >> =A0/** >> @@ -161,6 +163,7 @@ static void codel_params_init(struct codel_params *p= arams) >> =A0 =A0 =A0 params->interval =3D MS2TIME(100); >> =A0 =A0 =A0 params->target =3D MS2TIME(5); >> =A0 =A0 =A0 params->ecn =3D false; >> + =A0 =A0 params->ecn_target =3D MS2TIME(15); >> =A0} >> >> =A0static void codel_vars_init(struct codel_vars *vars) >> @@ -280,7 +283,9 @@ static struct sk_buff *codel_dequeue(struct Qdisc *s= ch, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 * since there is no more divide >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 */ >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 codel_Newton= _step(vars); >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (params->ec= n && INET_ECN_set_ce(skb)) { >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 if (params->ec= n && >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 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 && > =A0 =A0vars->ldelay <=3D params->ecn_target && > =A0 =A0INET_ECN_set_ce(skb)) { The orig is the same test, as params->ecn_target > vars->ldelay is equivalent to vars->ldelay <=3D params->ecn_target My own mental debate was whether to switch ecn from a bool to being a codel_time_t, and use a value of 0 for noecn and whatever for the ecn target value. > >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 INET_ECN_set_ce(skb)) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 stats->ecn_mark++; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 vars->drop_next =3D >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 =A0 =A0 =A0 =A0 codel_control_law(vars->drop_next, >> @@ -305,7 +310,9 @@ static struct sk_buff *codel_dequeue(struct Qdisc *s= ch, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 } >> =A0 =A0 =A0 } else if (drop) { >> - =A0 =A0 =A0 =A0 =A0 =A0 if (params->ecn && INET_ECN_set_ce(skb)) { >> + =A0 =A0 =A0 =A0 =A0 =A0 if (params->ecn && >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 params->ecn_target > vars->ldelay && >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 INET_ECN_set_ce(skb)) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 stats->ecn_mark++; >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 } else { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 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_CODE= L_MAX + 1] =3D { >> =A0 =A0 =A0 [TCA_CODEL_LIMIT] =A0 =A0 =A0 =3D { .type =3D NLA_U32 }, >> =A0 =A0 =A0 [TCA_CODEL_INTERVAL] =A0 =A0=3D { .type =3D NLA_U32 }, >> =A0 =A0 =A0 [TCA_CODEL_ECN] =A0 =A0 =A0 =A0 =3D { .type =3D NLA_U32 }, >> + =A0 =A0 [TCA_CODEL_ECN_TARGET] =A0=3D { .type =3D NLA_U32 }, >> =A0}; >> >> =A0static int codel_change(struct Qdisc *sch, struct nlattr *opt) >> @@ -133,6 +134,13 @@ static int codel_change(struct Qdisc *sch, struct n= lattr *opt) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 q->params.target =3D ((u64)target * NSEC_PER= _USEC) >> CODEL_SHIFT; >> =A0 =A0 =A0 } >> >> + =A0 =A0 if (tb[TCA_CODEL_ECN_TARGET]) { >> + =A0 =A0 =A0 =A0 =A0 =A0 u32 target =3D nla_get_u32(tb[TCA_CODEL_TARGET= ]); >> + >> + =A0 =A0 =A0 =A0 =A0 =A0 q->params.ecn_target =3D ((u64)target * NSEC_P= ER_USEC) >> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 CODEL_SHIFT; >> + =A0 =A0 } >> + >> =A0 =A0 =A0 if (tb[TCA_CODEL_INTERVAL]) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 u32 interval =3D nla_get_u32(tb[TCA_CODEL_IN= TERVAL]); >> >> @@ -145,6 +153,7 @@ static int codel_change(struct Qdisc *sch, struct nl= attr *opt) >> =A0 =A0 =A0 if (tb[TCA_CODEL_ECN]) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 q->params.ecn =3D !!nla_get_u32(tb[TCA_CODEL= _ECN]); >> >> + > > extra new line ? > >> =A0 =A0 =A0 qlen =3D sch->q.qlen; >> =A0 =A0 =A0 while (sch->q.qlen > sch->limit) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 struct sk_buff *skb =3D __skb_dequeue(&sch->= q); >> @@ -199,7 +208,9 @@ static int codel_dump(struct Qdisc *sch, struct sk_b= uff *skb) >> =A0 =A0 =A0 =A0 =A0 nla_put_u32(skb, TCA_CODEL_INTERVAL, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 codel_time_to_us(q->params.i= nterval)) || >> =A0 =A0 =A0 =A0 =A0 nla_put_u32(skb, TCA_CODEL_ECN, >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 q->params.ecn)) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 q->params.ecn) || >> + =A0 =A0 =A0 =A0 nla_put_u32(skb, TCA_CODEL_ECN_TARGET, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 codel_time_to_us(q->params.ecn= _target))) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto nla_put_failure; >> >> =A0 =A0 =A0 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_F= Q_CODEL_MAX + 1] =3D { >> =A0 =A0 =A0 [TCA_FQ_CODEL_ECN] =A0 =A0 =A0=3D { .type =3D NLA_U32 }, >> =A0 =A0 =A0 [TCA_FQ_CODEL_FLOWS] =A0 =A0=3D { .type =3D NLA_U32 }, >> =A0 =A0 =A0 [TCA_FQ_CODEL_QUANTUM] =A0=3D { .type =3D NLA_U32 }, >> + =A0 =A0 [TCA_FQ_CODEL_ECN_TARGET] =A0 =A0 =A0 =3D { .type =3D NLA_U32 = }, >> =A0}; >> >> =A0static int fq_codel_change(struct Qdisc *sch, struct nlattr *opt) >> @@ -327,6 +328,13 @@ static int fq_codel_change(struct Qdisc *sch, struc= t nlattr *opt) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 q->cparams.target =3D (target * NSEC_PER_USE= C) >> CODEL_SHIFT; >> =A0 =A0 =A0 } >> >> + =A0 =A0 if (tb[TCA_FQ_CODEL_ECN_TARGET]) { >> + =A0 =A0 =A0 =A0 =A0 =A0 u64 target =3D nla_get_u32(tb[TCA_FQ_CODEL_TAR= GET]); >> + >> + =A0 =A0 =A0 =A0 =A0 =A0 q->cparams.ecn_target =3D (target * NSEC_PER_U= SEC) >> >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 = =A0 CODEL_SHIFT; >> + =A0 =A0 } >> + >> =A0 =A0 =A0 if (tb[TCA_FQ_CODEL_INTERVAL]) { >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 u64 interval =3D nla_get_u32(tb[TCA_FQ_CODEL= _INTERVAL]); >> >> @@ -447,7 +455,9 @@ static int fq_codel_dump(struct Qdisc *sch, struct s= k_buff *skb) >> =A0 =A0 =A0 =A0 =A0 nla_put_u32(skb, TCA_FQ_CODEL_QUANTUM, >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 q->quantum) || >> =A0 =A0 =A0 =A0 =A0 nla_put_u32(skb, TCA_FQ_CODEL_FLOWS, >> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 q->flows_cnt)) >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 q->flows_cnt) || >> + =A0 =A0 =A0 =A0 nla_put_u32(skb, TCA_FQ_CODEL_ECN_TARGET, >> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 codel_time_to_us(q->cparams.ec= n_target))) >> =A0 =A0 =A0 =A0 =A0 =A0 =A0 goto nla_put_failure; >> >> =A0 =A0 =A0 nla_nest_end(skb, opts); > > > _______________________________________________ > Codel mailing list > Codel@lists.bufferbloat.net > https://lists.bufferbloat.net/listinfo/codel --=20 Dave T=E4ht http://www.bufferbloat.net/projects/cerowrt/wiki - "3.3.8-6 is out with fq_codel!"