On Sun, 2012-06-24 at 22:00 -0700, Dave Täht wrote:
> From: Dave Taht <[email protected]>
> 
> 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);


_______________________________________________
Codel mailing list
[email protected]
https://lists.bufferbloat.net/listinfo/codel

Reply via email to