From patchwork Tue Apr 24 08:40:27 2012 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Ilpo_J=C3=A4rvinen?= X-Patchwork-Id: 154623 X-Patchwork-Delegate: davem@davemloft.net Return-Path: X-Original-To: patchwork-incoming@ozlabs.org Delivered-To: patchwork-incoming@ozlabs.org Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 461EEB6FDD for ; Tue, 24 Apr 2012 18:40:35 +1000 (EST) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756266Ab2DXIkb (ORCPT ); Tue, 24 Apr 2012 04:40:31 -0400 Received: from courier.cs.helsinki.fi ([128.214.9.1]:43081 "EHLO mail.cs.helsinki.fi" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755991Ab2DXIk3 (ORCPT ); Tue, 24 Apr 2012 04:40:29 -0400 Received: from wel-95.cs.helsinki.fi (wel-95.cs.helsinki.fi [128.214.10.211]) (TLS: TLSv1/SSLv3,256bits,AES256-SHA) by mail.cs.helsinki.fi with esmtp; Tue, 24 Apr 2012 11:40:27 +0300 id 00068A4F.4F9666FB.00007599 Date: Tue, 24 Apr 2012 11:40:27 +0300 (EEST) From: "=?ISO-8859-15?Q?Ilpo_J=E4rvinen?=" X-X-Sender: ijjarvin@wel-95.cs.helsinki.fi To: David Miller cc: Eric Dumazet , rick.jones2@hp.com, Netdev , therbert@google.com, ncardwell@google.com, maze@google.com, Yuchung Cheng Subject: Re: [PATCH 2/2 net-next] tcp: sk_add_backlog() is too agressive for TCP In-Reply-To: <20120424.042546.2154907239944513463.davem@davemloft.net> Message-ID: References: <20120424.041018.1514311596818654005.davem@davemloft.net> <20120424.042546.2154907239944513463.davem@davemloft.net> User-Agent: Alpine 2.00 (DEB 1167 2008-08-23) MIME-Version: 1.0 Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Tue, 24 Apr 2012, David Miller wrote: > From: "Ilpo Järvinen" > Date: Tue, 24 Apr 2012 11:21:18 +0300 (EEST) > > > On Tue, 24 Apr 2012, David Miller wrote: > > > >> That makes this a non-starter since we must therefore remember all of > >> the SACK boundaries in the original packets. > > > > GRO works because TCP tends to use rather constant MSS, right? ...Since > > ACKs and SACKs are nothing more than reflection of those MSS boundaries of > > the opposite direction I don't find that as impossible as you do because > > the same kind of "mss" assumption can be applied there. But GRO has made > > this somewhat messier now because the receiver doesn't any more generate > > ACK per MSS or ACK per 2*MSS but that could be "fixed" by offloading the > > ACK sending when responding to a GROed packet. > > We're talking about accumulating ACKs on GRO not data packets. So am I... :-). ...Code speaks more than thousands of words: ...Obviously Data and ACK couldn't be GROed at the same time practically (would allow reusing the gso_size field for ack_size). ...But why exactly you think this is not possible or viable solution if fully implemented? And the problem I mentioned in the previous mail (in the terms of this code fragment) is that ackdiff is no longer MSS or 2*MSS because of GRO for the opposite direction doesn't trigger all those ACKs a non-GRO receiver would. diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c index 8bb6ade..33b87b2 100644 --- a/net/ipv4/tcp.c +++ b/net/ipv4/tcp.c @@ -2820,7 +2820,11 @@ found: flush |= (__force int)(flags & TCP_FLAG_CWR); flush |= (__force int)((flags ^ tcp_flag_word(th2)) & ~(TCP_FLAG_CWR | TCP_FLAG_FIN | TCP_FLAG_PSH)); - flush |= (__force int)(th->ack_seq ^ th2->ack_seq); + + ackgap = skb_shinfo(p)->ack_size; + ackdiff = th2->ack_seq - th->ack_seq; + flush |= (ackdiff - 1) >= ackgap; + for (i = sizeof(*th); i < thlen; i += 4) flush |= *(u32 *)((u8 *)th + i) ^ *(u32 *)((u8 *)th2 + i);