[Codel] [RFCv2 PATCH] codel: add ecn_target for when to drop rather than mark ecn packets
Dave Taht
dave.taht at gmail.com
Mon Jun 25 10:50:32 EDT 2012
On Sun, Jun 24, 2012 at 10:22 PM, Eric Dumazet <eric.dumazet at gmail.com> wrote:
> On Sun, 2012-06-24 at 22:00 -0700, Dave Täht wrote:
>> From: Dave Taht <dave.taht at bufferbloat.net>
>>
>> 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)) {
The orig is the same test, as params->ecn_target > vars->ldelay
is equivalent to vars->ldelay <= 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.
>
>> + 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);
>
>
> _______________________________________________
> Codel mailing list
> Codel at lists.bufferbloat.net
> https://lists.bufferbloat.net/listinfo/codel
--
Dave Täht
http://www.bufferbloat.net/projects/cerowrt/wiki - "3.3.8-6 is out
with fq_codel!"
More information about the Codel
mailing list