From owner-freebsd-net@freebsd.org Sat Apr 16 03:21:59 2016 Return-Path: Delivered-To: freebsd-net@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 4A935AED706 for ; Sat, 16 Apr 2016 03:21:59 +0000 (UTC) (envelope-from daemon-user@freebsd.org) Received: from reviews.nyi.freebsd.org (reviews.nyi.freebsd.org [IPv6:2610:1c1:1:607c::16:b]) by mx1.freebsd.org (Postfix) with ESMTP id 1F7FA1FF3 for ; Sat, 16 Apr 2016 03:21:59 +0000 (UTC) (envelope-from daemon-user@freebsd.org) Received: by reviews.nyi.freebsd.org (Postfix, from userid 1346) id B72ECC17C; Sat, 16 Apr 2016 03:21:58 +0000 (UTC) Date: Sat, 16 Apr 2016 03:21:58 +0000 To: freebsd-net@freebsd.org From: "jtl (Jonathan T. Looney)" Reply-to: D5872+325+9dea0574509cdbb3@reviews.freebsd.org Subject: [Differential] [Commented On] D5872: tcp: Don't prematurely drop receiving-only connections Message-ID: <9cd7c0a26ebf84740702e39dc5d0a723@localhost.localdomain> X-Priority: 3 X-Phabricator-Sent-This-Message: Yes X-Mail-Transport-Agent: MetaMTA X-Auto-Response-Suppress: All X-Phabricator-Mail-Tags: Thread-Topic: D5872: tcp: Don't prematurely drop receiving-only connections X-Herald-Rules: <64> X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-To: X-Phabricator-Cc: X-Phabricator-Cc: Precedence: bulk In-Reply-To: References: Thread-Index: MmVmNzYzNzljOGQxMmM4MWI4MmNjYzcxMzczIFcRr9Y= MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="utf-8" X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.21 List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 16 Apr 2016 03:21:59 -0000 jtl added a comment. In https://reviews.freebsd.org/D5872#127343, @mike-karels.net wrote: > If we get an ENOBUFS when sending data, we will already be running the retransmit timer. Good point, but see below. > If we drop an ACK on ENOBUFS, either we will receive more data and > attempt another ACK, or the sender will time out and resend data. Either > will get the connection started again. True, but it may take time if we have to wait for a retransmission time out. That isn't necessarily a bad thing. > I believe lines 1552-1554 should > simply be deleted. On the surface, that seems like a reasonable alternative. However, its interesting to see why this code was originally added. It looks like it was added by jlemon in r61179. Here's what the commit message says: > When attempting to transmit a packet, if the system fails to allocate > a mbuf, it may return without setting any timers. If no more data is > scheduled to be transmitted (this was a FIN) the system will sit in > LAST_ACK state forever. > > Thus, when mbuf allocation fails, set the retransmit timer if neither > the retransmit or persist timer is already pending. Even back then, the retransmit and/or persist timers should have been running by this point in the code. It would be interesting to know if jlemon proved this code fixed the problem, or it was just speculated. We fixed a similar-sounding problem last year, and the code is different now, so these particular lines may truly no longer be necessary. But, I am a little hesitant to remove this code without knowing more about its origin. Just my 2c. REVISION DETAIL https://reviews.freebsd.org/D5872 EMAIL PREFERENCES https://reviews.freebsd.org/settings/panel/emailpreferences/ To: sepherosa_gmail.com, network, glebius, lstewart, adrian, delphij, decui_microsoft.com, honzhan_microsoft.com, howard0su_gmail.com, freebsd-net-list, transport, jtl, hiren Cc: mike-karels.net, jtl