| Message ID | 1363923369.4431.53.camel@edumazet-glaptop |
|---|---|
| State | Accepted, archived |
| Delegated to: | David Miller |
| Headers | show |
From: Eric Dumazet <eric.dumazet@gmail.com> Date: Thu, 21 Mar 2013 20:36:09 -0700 > From: Eric Dumazet <edumazet@google.com> > > A long standing problem with TSO is the fact that tcp_tso_should_defer() > rearms the deferred timer, while it should not. ... > Signed-off-by: Eric Dumazet <edumazet@google.com> I always wondered about this, good catch. Applied and queued up for -stable, thanks! -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, 2013-03-22 at 10:34 -0400, David Miller wrote: > I always wondered about this, good catch. > > Applied and queued up for -stable, thanks! It took me a while to see the light. I was trying to reduce the time limit (2 ticks -> 1 tick), or remove tso_deferred field to use lsndtime instead. Then, before cooking the patch for net-next, I finally understood the problem. Thanks -- To unsubscribe from this list: send the line "unsubscribe netdev" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/net/ipv4/tcp_output.c b/net/ipv4/tcp_output.c index 817fbb3..5d0b438 100644 --- a/net/ipv4/tcp_output.c +++ b/net/ipv4/tcp_output.c @@ -1809,8 +1809,11 @@ static bool tcp_tso_should_defer(struct sock *sk, struct sk_buff *skb) goto send_now; } - /* Ok, it looks like it is advisable to defer. */ - tp->tso_deferred = 1 | (jiffies << 1); + /* Ok, it looks like it is advisable to defer. + * Do not rearm the timer if already set to not break TCP ACK clocking. + */ + if (!tp->tso_deferred) + tp->tso_deferred = 1 | (jiffies << 1); return true;