From mboxrd@z Thu Jan 1 00:00:00 1970 From: Lorenzo Colitti Subject: Re: (2) FW: [Resource Leak] Suggesting patch for tcp_close Date: Tue, 27 Nov 2018 23:36:06 -0800 Message-ID: References: <20181123072258epcms1p8c41425598751be28f8cc05b91c7ac4b5@epcms1p8> <20181128010903epcms1p7ef44541625212f6af3fc2d3e0d7eb390@epcms1p7> <20181128061715epcms1p18e21f112dea8cdf67481c35af52419bd@epcms1p1> Mime-Version: 1.0 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Cc: Eric Dumazet , netdev@vger.kernel.org, =?UTF-8?B?7Jyg6riI7ZmY?= , =?UTF-8?Q?Maciej_=C5=BBenczykowski?= To: =?UTF-8?B?67Cw7ISd7KeE?= Return-path: Received: from mail-io1-f66.google.com ([209.85.166.66]:34126 "EHLO mail-io1-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1727247AbeK1ShA (ORCPT ); Wed, 28 Nov 2018 13:37:00 -0500 Received: by mail-io1-f66.google.com with SMTP id f6so19204858iob.1 for ; Tue, 27 Nov 2018 23:36:18 -0800 (PST) In-Reply-To: <20181128061715epcms1p18e21f112dea8cdf67481c35af52419bd@epcms1p1> Sender: netdev-owner@vger.kernel.org List-ID: On Tue, Nov 27, 2018 at 10:17 PM =EB=B0=B0=EC=84=9D=EC=A7=84 wrote: > >> we saw hundreds of not closed tcp session with FIN_WAIT1 and LAST_ACK. > > > > These sessions should have a timer, and eventually disappear. > > FIN_WAIT2 and TIME_WAIT have a timer. > but FIN_WAIT1 and LAST_ACK are have too? What harm is caused by these stale sessions? I thought that was the intended behaviour. If you look at the original design discussions that led to the SOCK_DESTROY and tcp_abort patch, the goal of SOCK_DESTROY was not primarily to clear TCP state, but primarily to unblock applications that were stuck in blocking socket operations such as read(), write() and connect. That is the reason why it only calls tcp_done if the SOCK_DEAD flag is not set. See in particular https://www.spinics.net/lists/netdev/msg352716.html , where opposition was voiced to being able to close sockets in TIME_WAIT_STATE. That said, I don't have a strong opinion on this: whatever works for Eric works for me. > > Do you have a test to demonstrate the issue ? > > > > I know Lorenzo wrote tests, so presumably new tests are needed. There are definitely tests that check that SOCK_DESTROY does nothing for dead sockets (i.e., the current behaviour without this patch). Those tests are part of the Android conformance test suites (VTS), so I think taking this patch will cause the device to fail the Android conformance here: https://cs.corp.google.com/android/kernel/tests/net/test/sock_diag_test.py?= l=3D663 Soukjin, you will need to modify that test if this patch is applied.