[Codel] [RFCv2 PATCH] codel: add ecn_target for when to drop rather than mark ecn packets
Eric Dumazet
eric.dumazet at gmail.com
Mon Jun 25 01:22:04 EDT 2012
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)) {
> + 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);
More information about the Codel
mailing list