On Wed, 16 Sep 2015, Trond Myklebust wrote:
> On Wed, Sep 16, 2015 at 9:58 AM, Benjamin Coddington
> <bcodding@xxxxxxxxxx> wrote:
> > After a network segmentation that times out the socket, we can get stuck in
> > a loop where the client repeatedly sends a SYN, then a RST, with the result
> > that the client will never successfully reconnect to the server.
> >
> > When there's a need to re-establish a TCP connection, the scheduled
> > connect_worker does xs_tcp_setup_socket(), which sends a SYN, then
> > immediately wakes the waiting task to call_connect_status with a status of
> > EAGAIN, which proceeds to xprt_connect() and kernel_sock_shutdown() which
> > will send a RST.
> >
> > Fix this loop by deferring xprt_clear_connecting() until the TCP state
> > change of ESTALISHED or CLOSED, thereby ensuring that we do not race into
> > xs_connect() while waiting for SYN,ACK from the server.
> >
> > Fixes: 099392048c (SUNRPC: Prevent SYN+SYNACK+RST storms)
> > Signed-off-by: Benjamin Coddington <bcodding@xxxxxxxxxx>
> > ---
> > net/sunrpc/xprtsock.c | 6 ++++--
> > 1 files changed, 4 insertions(+), 2 deletions(-)
> >
> > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> > index 7be90bc..6df51df 100644
> > --- a/net/sunrpc/xprtsock.c
> > +++ b/net/sunrpc/xprtsock.c
> > @@ -1450,6 +1450,7 @@ static void xs_tcp_state_change(struct sock *sk)
> > switch (sk->sk_state) {
> > case TCP_ESTABLISHED:
> > spin_lock(&xprt->transport_lock);
> > + xprt_clear_connecting(xprt);
> > if (!xprt_test_and_set_connected(xprt)) {
> > struct sock_xprt *transport = container_of(xprt,
> > struct sock_xprt, xprt);
> > @@ -1496,6 +1497,7 @@ static void xs_tcp_state_change(struct sock *sk)
> > smp_mb__after_atomic();
> > break;
> > case TCP_CLOSE:
> > + xprt_clear_connecting(xprt);
> > xs_sock_mark_closed(xprt);
> > }
> > out:
> > @@ -2237,10 +2239,10 @@ static void xs_tcp_setup_socket(struct work_struct *work)
> > xs_tcp_force_close(xprt);
> > break;
> > case 0:
> > - case -EINPROGRESS:
> > + xprt_clear_connecting(xprt);
> > case -EALREADY:
> > + case -EINPROGRESS:
> > xprt_unlock_connect(xprt, transport);
> > - xprt_clear_connecting(xprt);
> > return;
> > case -EINVAL:
> > /* Happens, for instance, if the user specified a link
> >
>
> This introduces races. The call to xprt_clear_connecting in TCP_CLOSE
> can trigger pretty much at any time, and so could interfere with a
> subsequent connection attempt.
Oh, yeah. I see that now.
> I have an alternative patch which addresses this problem and which is
> undergoing testing.
Great!
--
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
