On Tue, Jul 26, 2016 at 09:51:19AM -0400, Trond Myklebust wrote:
> We're seeing traces of the following form:
>
> [10952.396347] svc: transport ffff88042ba4a 000 dequeued, inuse=2
> [10952.396351] svc: tcp_accept ffff88042ba4 a000 sock ffff88042a6e4c80
> [10952.396362] nfsd: connect from 10.2.6.1, port=187
> [10952.396364] svc: svc_setup_socket ffff8800b99bcf00
> [10952.396368] setting up TCP socket for reading
> [10952.396370] svc: svc_setup_socket created ffff8803eb10a000 (inet ffff88042b75b800)
> [10952.396373] svc: transport ffff8803eb10a000 put into queue
> [10952.396375] svc: transport ffff88042ba4a000 put into queue
> [10952.396377] svc: server ffff8800bb0ec000 waiting for data (to = 3600000)
> [10952.396380] svc: transport ffff8803eb10a000 dequeued, inuse=2
> [10952.396381] svc_recv: found XPT_CLOSE
> [10952.396397] svc: svc_delete_xprt(ffff8803eb10a000)
> [10952.396398] svc: svc_tcp_sock_detach(ffff8803eb10a000)
> [10952.396399] svc: svc_sock_detach(ffff8803eb10a000)
> [10952.396412] svc: svc_sock_free(ffff8803eb10a000)
>
> i.e. an immediate close of the socket after initialisation.
Interesting, thanks!
So the one thing I don't understand is why this is correct behavior for
accept--I thought it wasn't supposed to return a socket until it was
fully established.
--b.
>
> The culprit appears to be the test at the end of svc_tcp_init, which
> checks if the newly created socket is in the TCP_ESTABLISHED state,
> and immediately closes it if not. The evidence appears to suggest that
> the socket might still be in the SYN_RECV state at this time.
>
> The fix is to check for both states, and then to add a check in
> svc_tcp_state_change() to ensure we don't close the socket when
> it transitions into TCP_ESTABLISHED.
>
> Signed-off-by: Trond Myklebust <trond.myklebust@xxxxxxxxxxxxxxx>
> ---
> net/sunrpc/svcsock.c | 13 ++++++++++---
> 1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/net/sunrpc/svcsock.c b/net/sunrpc/svcsock.c
> index df61a4ec21a4..38b132ff6dce 100644
> --- a/net/sunrpc/svcsock.c
> +++ b/net/sunrpc/svcsock.c
> @@ -767,8 +767,10 @@ static void svc_tcp_state_change(struct sock *sk)
> printk("svc: socket %p: no user data\n", sk);
> else {
> svsk->sk_ostate(sk);
> - set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
> - svc_xprt_enqueue(&svsk->sk_xprt);
> + if (sk->sk_state != TCP_ESTABLISHED) {
> + set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
> + svc_xprt_enqueue(&svsk->sk_xprt);
> + }
> }
> }
>
> @@ -1294,8 +1296,13 @@ static void svc_tcp_init(struct svc_sock *svsk, struct svc_serv *serv)
> tcp_sk(sk)->nonagle |= TCP_NAGLE_OFF;
>
> set_bit(XPT_DATA, &svsk->sk_xprt.xpt_flags);
> - if (sk->sk_state != TCP_ESTABLISHED)
> + switch (sk->sk_state) {
> + case TCP_SYN_RECV:
> + case TCP_ESTABLISHED:
> + break;
> + default:
> set_bit(XPT_CLOSE, &svsk->sk_xprt.xpt_flags);
> + }
> }
> }
>
> --
> 2.7.4
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
