Re: [Codel] [RFC PATCH] codel: ecn mark at target

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

On Sun, 2012-08-05 at 20:40 +0200, Eric Dumazet wrote:
> On Sun, 2012-08-05 at 11:14 -0700, Yuchung Cheng wrote:
> > On Sun, Aug 5, 2012 at 10:35 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > > On Sun, 2012-08-05 at 19:26 +0200, Eric Dumazet wrote:
> > >
> > >> It could be a flaw in linux implementation, I admit we had so many bugs
> > >> that it could very well be still buggy.
> > >
> > > And at first glance, the following tcpdump seems suspect : We can see
> > > all ACK are delayed by about 40 ms
> > but RFC 3168 (sec 6.1.3) does not mandate immediate ACKs for ECE
> > marked ones? is this because ECN response is per round-trip?
> > 
> 
> We should IMHO not delay ACKS, exactly like we react to a dropped
> packet.
> 
> If not specified in RFC 3168, it seems a forgotten point.

Following patch does the trick.

Delaying ACK while sender might have a cwnd = 1 is not nice at all.

I get better behavior, but still codel with Dave patch (mark all packets
if above 'target') is giving an unfair advantage to non ECN flow.

diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 2fd2bc9..b03f429 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -237,6 +237,7 @@ static inline void TCP_ECN_check_ce(struct tcp_sock *tp, const struct sk_buff *s
 			tcp_enter_quickack_mode((struct sock *)tp);
 		break;
 	case INET_ECN_CE:
+		inet_csk((struct sock *)tp)->icsk_ack.quick |= 1;
 		tp->ecn_flags |= TCP_ECN_DEMAND_CWR;
 		/* fallinto */
 	default: