Re: It's back! (Re: [REGRESSION] NFS is creating a hidden port (left over from xs_bind() ))

[Trond Myklebust <trondmy@primarydata.com>]

[-- Attachment #1: Type: text/plain, Size: 3799 bytes --]

On Tue, 2018-02-06 at 10:20 +0100, Daniel Reichelt wrote:
> On 02/06/2018 01:24 AM, Trond Myklebust wrote:
> > Does the following fix the issue?
> > 
> > 8<-----------------------------------------------
> > From 9b30889c548a4d45bfe6226e58de32504c1d682f Mon Sep 17 00:00:00
> > 2001
> > From: Trond Myklebust <trond.myklebust@primarydata.com>
> > Date: Mon, 5 Feb 2018 10:20:06 -0500
> > Subject: [PATCH] SUNRPC: Ensure we always close the socket after a
> > connection
> >  shuts down
> > 
> > Ensure that we release the TCP socket once it is in the TCP_CLOSE
> > or
> > TCP_TIME_WAIT state (and only then) so that we don't confuse
> > rkhunter
> > and its ilk.
> > 
> > Signed-off-by: Trond Myklebust <trond.myklebust@primarydata.com>
> > ---
> >  net/sunrpc/xprtsock.c | 23 ++++++++++-------------
> >  1 file changed, 10 insertions(+), 13 deletions(-)
> > 
> > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> > index 18803021f242..5d0108172ed3 100644
> > --- a/net/sunrpc/xprtsock.c
> > +++ b/net/sunrpc/xprtsock.c
> > @@ -807,13 +807,6 @@ static void
> > xs_sock_reset_connection_flags(struct rpc_xprt *xprt)
> >  	smp_mb__after_atomic();
> >  }
> >  
> > -static void xs_sock_mark_closed(struct rpc_xprt *xprt)
> > -{
> > -	xs_sock_reset_connection_flags(xprt);
> > -	/* Mark transport as closed and wake up all pending tasks
> > */
> > -	xprt_disconnect_done(xprt);
> > -}
> > -
> >  /**
> >   * xs_error_report - callback to handle TCP socket state errors
> >   * @sk: socket
> > @@ -833,9 +826,6 @@ static void xs_error_report(struct sock *sk)
> >  	err = -sk->sk_err;
> >  	if (err == 0)
> >  		goto out;
> > -	/* Is this a reset event? */
> > -	if (sk->sk_state == TCP_CLOSE)
> > -		xs_sock_mark_closed(xprt);
> >  	dprintk("RPC:       xs_error_report client %p,
> > error=%d...\n",
> >  			xprt, -err);
> >  	trace_rpc_socket_error(xprt, sk->sk_socket, err);
> > @@ -1655,9 +1645,11 @@ static void xs_tcp_state_change(struct sock
> > *sk)
> >  		if (test_and_clear_bit(XPRT_SOCK_CONNECTING,
> >  					&transport->sock_state))
> >  			xprt_clear_connecting(xprt);
> > +		clear_bit(XPRT_CLOSING, &xprt->state);
> >  		if (sk->sk_err)
> >  			xprt_wake_pending_tasks(xprt, -sk-
> > >sk_err);
> > -		xs_sock_mark_closed(xprt);
> > +		/* Trigger the socket release */
> > +		xs_tcp_force_close(xprt);
> >  	}
> >   out:
> >  	read_unlock_bh(&sk->sk_callback_lock);
> > @@ -2265,14 +2257,19 @@ static void xs_tcp_shutdown(struct rpc_xprt
> > *xprt)
> >  {
> >  	struct sock_xprt *transport = container_of(xprt, struct
> > sock_xprt, xprt);
> >  	struct socket *sock = transport->sock;
> > +	int skst = transport->inet ? transport->inet->sk_state :
> > TCP_CLOSE;
> >  
> >  	if (sock == NULL)
> >  		return;
> > -	if (xprt_connected(xprt)) {
> > +	switch (skst) {
> > +	default:
> >  		kernel_sock_shutdown(sock, SHUT_RDWR);
> >  		trace_rpc_socket_shutdown(xprt, sock);
> > -	} else
> > +		break;
> > +	case TCP_CLOSE:
> > +	case TCP_TIME_WAIT:
> >  		xs_reset_transport(transport);
> > +	}
> >  }
> >  
> >  static void xs_tcp_set_socket_timeouts(struct rpc_xprt *xprt,
> > 
> 
> 
> Previously, I've seen hidden ports within 5-6 minutes after re-
> starting
> the nfsd and re-mounting nfs-exports on clients.
> 
> With this patch applied, I don't see any hidden ports after 15mins. I
> guess it's a valid fix.

For the record, the intention of the patch is not to adjust or correct
any connection timeout values. Merely to ensure that once the
connection breakage is detected by the socket layer, so that is it no
longer usable by the RPC client, we release the socket.

-- 
Trond Myklebust
Linux NFS client maintainer, PrimaryData
trond.myklebust@primarydata.com

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 833 bytes --]