On Thu, 2018-05-10 at 16:22 +0000, Vallish Vaidyeshwara wrote:
> On Thu, May 10, 2018 at 03:25:14PM +0000, Trond Myklebust wrote:
> > On Thu, 2018-05-10 at 06:12 +0000, Vallish Vaidyeshwara wrote:
> > > Server initiated socket close can corrupt connection state
> > > tracking
> > > table in conjunction with other network middle boxes. In
> > > situations
> > > like these, client connection hangs till connection state
> > > tracking
> > > table entries age out and get purged. Client reconnection with a
> > > new
> > > port in such a situation will avoid connection hang.
> > >
> > > Reviewed-by: Jacob Strauss <jsstraus@xxxxxxxxxx>
> > > Reviewed-by: Alakesh Haloi <alakeshh@xxxxxxxxxx>
> > > Signed-off-by: Vallish Vaidyeshwara <vallish@xxxxxxxxxx>
> > > ---
> > > net/sunrpc/xprtsock.c | 5 +++++
> > > 1 file changed, 5 insertions(+)
> > >
> > > diff --git a/net/sunrpc/xprtsock.c b/net/sunrpc/xprtsock.c
> > > index 5bf75b3..d293c8d 100644
> > > --- a/net/sunrpc/xprtsock.c
> > > +++ b/net/sunrpc/xprtsock.c
> > > @@ -1629,6 +1629,8 @@ static void xs_tcp_state_change(struct sock
> > > *sk)
> > > /* The server initiated a shutdown of the socket
> > > */
> > > xprt->connect_cookie++;
> > > clear_bit(XPRT_CONNECTED, &xprt->state);
> > > + /* Server sent FIN, reconnect with a new port */
> > > + transport->srcport = 0;
> > > xs_tcp_force_close(xprt);
> > > /* fall through */
> > > case TCP_CLOSING:
> > > @@ -1650,6 +1652,9 @@ static void xs_tcp_state_change(struct sock
> > > *sk)
> > > &transport->sock_state))
> > > xprt_clear_connecting(xprt);
> > > clear_bit(XPRT_CLOSING, &xprt->state);
> > > + /* Server sent RST, reconnect with a new port */
> > > + if (sk->sk_err == ECONNRESET)
> > > + transport->srcport = 0;
> > > if (sk->sk_err)
> > > xprt_wake_pending_tasks(xprt, -sk-
> > > >sk_err);
> > > /* Trigger the socket release */
> >
> > NACK. This will utterly break NFSv2, NFSv3 and NFSv4.0 duplicate
> > replay
> > cache semantics.
> >
> > Cheers
> > Trond
> > --
> > Trond Myklebust
> > Linux NFS client maintainer, Hammerspace
> > trond.myklebust@xxxxxxxxxxxxxxx
>
> Hello Trond,
>
> The first patch in this series is actually helping restore DRC
> behavior in
> cases like network partition where packets are dropped:
> [PATCH 1/2] SUNRPC: Need to reuse non-reserved port for reconnect
> Patch 1 starts reusing port in all cases.
>
> The second patch is still not breaking DRC semantics, to quote from
> source
> code:
>
> <code snip>
> /**
> * xs_close - close a socket
> * @xprt: transport
> *
> * This is used when all requests are complete; ie, no DRC state
> remains
> * on the server we want to save.
> *
> * The caller _must_ be holding XPRT_LOCKED in order to avoid
> issues with
> * xs_reset_transport() zeroing the socket from underneath a
> writer.
> */
> static void xs_close(struct rpc_xprt *xprt)
> <code snip>
>
> If the server has closed a connection, then no DRC state remains on
> the server
> we want to use.
>
> The second patch is exploiting this semantics and using a new port in
> following
> 2 cases:
> a) RST from server implies the connection was torn down & no useful
> DRC exists
> on server
> b) FIN from server implies that server is shutting down the
> connection as part
> of close and no DRC state remains
>
> Please let me know if I have missed something obvious, I definitely
> do not want
> to break DRC as that is not the intention of this patch series. Is
> there a
> situation where server can close a connection and still keep DRC?
>
The DRC does not exist for the benefit of the server, but for the
benefit of the client. It is there to ensure that if the client replays
the request, then it gets the exact same reply as it should have
received when the first request was sent. Bearing that in mind:
1. What guarantees that all servers behave correctly w.r.t. ensuring
that they have sent all replies to any outstanding RPC call before
shutting down the connection. I'm not sure that even the Linux
server does that.
2. How would a server even know whether or not the client may need to
replay a request?
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
trond.myklebust@xxxxxxxxxxxxxxx��.n��������+%������w��{.n�����{��w��� jg��� �����ݢj����G���� ���j:+v���w�m������w�������h�����٥
