Message-ID: <1550837576.6456.3.camel@redhat.com>
Subject: Re: [PATCH 1/1] SUNRPC: fix handling of half-closed connection
From:   Dave Wysochanski <dwysocha@redhat.com>
To:     Olga Kornievskaia <olga.kornievskaia@gmail.com>,
        trond.myklebust@hammerspace.com, anna.schumaker@netapp.com
Cc:     linux-nfs@vger.kernel.org
Date:   Fri, 22 Feb 2019 07:12:56 -0500
In-Reply-To: <20190220145650.21566-1-olga.kornievskaia@gmail.com>
References: <20190220145650.21566-1-olga.kornievskaia@gmail.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-nfs-owner@vger.kernel.org
Precedence: bulk

Hi Olga,

Do you have a reproducer for this?  A number of months ago I did a
significant amount of testing with half-closed connections, after we
had reports of connections stuck in FIN_WAIT2 in some older kernels. 
What I found was with kernels that had the tcp keepalives (commit
7f260e8575bf53b93b77978c1e39f8e67612759c), I could only reproduce a
hang of a few minutes, after which time the tcp keepalive code would
reset the connection.

That said it was a while ago and something subtle may have changed. 
Also I'm not not sure if your header implies an indefinite hang or just
a few minutes.

Thanks.


On Wed, 2019-02-20 at 09:56 -0500, Olga Kornievskaia wrote:
> From: Olga Kornievskaia <kolga@netapp.com>
> 
> When server replies with an ACK to client's FIN/ACK, client ends
> up stuck in a TCP_FIN_WAIT2 state and client's mount hangs.
> Instead, make sure to close and reset client's socket and transport
> when transitioned into that state.
> 
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
>  net/sunrpc/xprtsock.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> index 618e9c2..812e5e3 100644
> --- a/net/sunrpc/xprtsock.c
> +++ b/net/sunrpc/xprtsock.c
> @@ -1502,6 +1502,7 @@ static void xs_tcp_state_change(struct sock
> *sk)
>  		clear_bit(XPRT_CLOSE_WAIT, &xprt->state);
>  		smp_mb__after_atomic();
>  		break;
> +	case TCP_FIN_WAIT2:
>  	case TCP_CLOSE_WAIT:
>  		/* The server initiated a shutdown of the socket */
>  		xprt->connect_cookie++;
> @@ -2152,6 +2153,7 @@ static void xs_tcp_shutdown(struct rpc_xprt
> *xprt)
>  		kernel_sock_shutdown(sock, SHUT_RDWR);
>  		trace_rpc_socket_shutdown(xprt, sock);
>  		break;
> +	case TCP_FIN_WAIT2:
>  	case TCP_CLOSE:
>  	case TCP_TIME_WAIT:
>  		xs_reset_transport(transport);