lkml.org 
[lkml]   [2018]   [Sep]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.18 004/123] xprtrdma: Fix disconnect regression
    Date
    4.18-stable review patch.  If anyone has any objections, please let me know.

    ------------------

    From: Chuck Lever <chuck.lever@oracle.com>

    commit 8d4fb8ff427a23e573c9373b2bb3d1d6e8ea4399 upstream.

    I found that injecting disconnects with v4.18-rc resulted in
    random failures of the multi-threaded git regression test.

    The root cause appears to be that, after a reconnect, the
    RPC/RDMA transport is waking pending RPCs before the transport has
    posted enough Receive buffers to receive the Replies. If a Reply
    arrives before enough Receive buffers are posted, the connection
    is dropped. A few connection drops happen in quick succession as
    the client and server struggle to regain credit synchronization.

    This regression was introduced with commit 7c8d9e7c8863 ("xprtrdma:
    Move Receive posting to Receive handler"). The client is supposed to
    post a single Receive when a connection is established because
    it's not supposed to send more than one RPC Call before it gets
    a fresh credit grant in the first RPC Reply [RFC 8166, Section
    3.3.3].

    Unfortunately there appears to be a longstanding bug in the Linux
    client's credit accounting mechanism. On connect, it simply dumps
    all pending RPC Calls onto the new connection. It's possible it has
    done this ever since the RPC/RDMA transport was added to the kernel
    ten years ago.

    Servers have so far been tolerant of this bad behavior. Currently no
    server implementation ever changes its credit grant over reconnects,
    and servers always repost enough Receives before connections are
    fully established.

    The Linux client implementation used to post a Receive before each
    of these Calls. This has covered up the flooding send behavior.

    I could try to correct this old bug so that the client sends exactly
    one RPC Call and waits for a Reply. Since we are so close to the
    next merge window, I'm going to instead provide a simple patch to
    post enough Receives before a reconnect completes (based on the
    number of credits granted to the previous connection).

    The spurious disconnects will be gone, but the client will still
    send multiple RPC Calls immediately after a reconnect.

    Addressing the latter problem will wait for a merge window because
    a) I expect it to be a large change requiring lots of testing, and
    b) obviously the Linux client has interoperated successfully since
    day zero while still being broken.

    Fixes: 7c8d9e7c8863 ("xprtrdma: Move Receive posting to ... ")
    Cc: stable@vger.kernel.org # v4.18+
    Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
    Signed-off-by: Anna Schumaker <Anna.Schumaker@Netapp.com>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    net/sunrpc/xprtrdma/verbs.c | 5 ++---
    1 file changed, 2 insertions(+), 3 deletions(-)

    --- a/net/sunrpc/xprtrdma/verbs.c
    +++ b/net/sunrpc/xprtrdma/verbs.c
    @@ -280,7 +280,6 @@ rpcrdma_conn_upcall(struct rdma_cm_id *i
    ++xprt->rx_xprt.connect_cookie;
    connstate = -ECONNABORTED;
    connected:
    - xprt->rx_buf.rb_credits = 1;
    ep->rep_connected = connstate;
    rpcrdma_conn_func(ep);
    wake_up_all(&ep->rep_connect_wait);
    @@ -755,6 +754,7 @@ retry:
    }

    ep->rep_connected = 0;
    + rpcrdma_post_recvs(r_xprt, true);

    rc = rdma_connect(ia->ri_id, &ep->rep_remote_cma);
    if (rc) {
    @@ -773,8 +773,6 @@ retry:

    dprintk("RPC: %s: connected\n", __func__);

    - rpcrdma_post_recvs(r_xprt, true);
    -
    out:
    if (rc)
    ep->rep_connected = rc;
    @@ -1171,6 +1169,7 @@ rpcrdma_buffer_create(struct rpcrdma_xpr
    list_add(&req->rl_list, &buf->rb_send_bufs);
    }

    + buf->rb_credits = 1;
    buf->rb_posted_receives = 0;
    INIT_LIST_HEAD(&buf->rb_recv_bufs);


    \
     
     \ /
      Last update: 2018-09-03 19:49    [W:6.987 / U:0.036 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site