Re: [Codel] [RFC PATCH] Codel: Enable packet drop with ECN-marked packets on a threshold

[Eric Dumazet <eric.dumazet@gmail.com>]

On Sun, 2012-06-17 at 15:30 -0700, Dave Täht wrote:
> Lossless IP networks are not possible. Dropping packets is the
> fastest way to get back to a target delay, and if that involves
> dropping ECN marked packets, so be it. When a network is in a
> more steady state, ECN is fine...
> 

Just disable ecn then ? By the way ECN is disabled by default on CoDel.

> My choice of "2 * target" as a threshold for dropping ECN
> marked packets is entirely arbitrary.
> ---
>  include/net/codel.h |    6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/include/net/codel.h b/include/net/codel.h
> index 550debf..26944a0 100644
> --- a/include/net/codel.h
> +++ b/include/net/codel.h
> @@ -280,7 +280,8 @@ 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 && INET_ECN_set_ce(skb) &&
> +					vars->ldelay <= 2 * params->target) {
>  					stats->ecn_mark++;
>  					vars->drop_next =
>  						codel_control_law(vars->drop_next,
> @@ -305,7 +306,8 @@ static struct sk_buff *codel_dequeue(struct Qdisc *sch,
>  			}
>  		}
>  	} else if (drop) {
> -		if (params->ecn && INET_ECN_set_ce(skb)) {
> +		if (params->ecn && INET_ECN_set_ce(skb) &&
> +		        vars->ldelay <= 2 * params->target) {
>  			stats->ecn_mark++;
>  		} else {
>  			qdisc_drop(skb, sch);

Hmm, while I understand the idea, patch is wrong.

if (A && B && C)

Even if (C) is false, but A is true, B is evaluated.

Also, a 2 * target value is arbitrary, why not instead provide a real
attribute, that user can set with tc command ?