Date: Fri, 29 Apr 2016 01:40:44 +0000 From: "sepherosa_gmail.com (Sepherosa Ziehau)" <phabric-noreply@FreeBSD.org> To: freebsd-net@freebsd.org Subject: [Differential] D5872: tcp: Don't prematurely drop receiving-only connections Message-ID: <61203690be93262c73401e765d755a38@localhost.localdomain> In-Reply-To: <differential-rev-PHID-DREV-5vu7wrfibtoing77xfvt-req@FreeBSD.org> References: <differential-rev-PHID-DREV-5vu7wrfibtoing77xfvt-req@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
sepherosa_gmail.com added a comment. In https://reviews.freebsd.org/D5872#130805, @lstewart wrote: > In https://reviews.freebsd.org/D5872#130179, @sepherosa_gmail.com wrote: > > > We probably can leave the cwnd resetting to later rexmt timeout or possible later fast retransmit (I think fast retransmit could kick in under some cases, if ENOBUFS happened); instead of resetting the cwnd immediately upon ENOBUFS. > > > Please leave the manipulation of cwnd as is so as to avoid conflating two different changes. The manipulation of cwnd on local drop has nothing to do with the subject of this particular change. Yep, I am not going to delete the cwnd reset in this patch. > Also, the patch we're reviewing here should be the commit candidate i.e. what will actually land in the tree. I understand that you need to test with something different than the commit candidate, but that should just be noted in the review description. We all need to see the final code as there are many subtleties here that require close attention from as many sets of eyes as possible. This is what I am testing now. Since this path is not on a hot code path and we obviously don't want to have timers unset for data/FIN/SYN, can we just use the "if() panic" here? And another thing is abstract it as a macro. Maybe we just leave the macro abstraction to the next step? So, if both of following is true: - We agree to use "if() panic" here, instead of KASSERT - Do the macro abstraction in the next step Then the current patch is exactly what I want to put into the tree. REVISION DETAIL https://reviews.freebsd.org/D5872 EMAIL PREFERENCES https://reviews.freebsd.org/settings/panel/emailpreferences/ To: sepherosa_gmail.com, network, glebius, adrian, delphij, decui_microsoft.com, honzhan_microsoft.com, howard0su_gmail.com, freebsd-net-list, lstewart, hiren, jtl, transport Cc: gnn, mike-karels.net, jtl
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?61203690be93262c73401e765d755a38>