Received-SPF: pass (google.com: domain of linux-nfs-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Message-ID: <a9f18064-45d4-4346-0156-dcd74e001b82@talpey.com>
Date:   Sat, 15 Jul 2023 13:01:34 -0400
MIME-Version: 1.0
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101
 Thunderbird/102.13.0
Subject: Re: [PATCH] xprtrdma: Remap Receive buffers after a reconnect
Content-Language: en-US
To:     Chuck Lever <cel@kernel.org>, anna.schumaker@netapp.com
Cc:     Chuck Lever <chuck.lever@oracle.com>, linux-nfs@vger.kernel.org,
        linux-rdma@vger.kernel.org, kolga@netapp.com
References: <168934757954.2781502.4228890662497418497.stgit@morisot.1015granger.net>
From:   Tom Talpey <tom@talpey.com>
In-Reply-To: <168934757954.2781502.4228890662497418497.stgit@morisot.1015granger.net>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 7/14/2023 11:13 AM, Chuck Lever wrote:
> From: Chuck Lever <chuck.lever@oracle.com>
> 
> On server-initiated disconnect, rpcrdma_xprt_disconnect() DMA-unmaps
> the transport's Receive buffers, but rpcrdma_post_recvs() neglected
> to remap them after a new connection had been established. The
> result is immediate failure of the new connection with the Receives
> flushing with LOCAL_PROT_ERR.

The fix is correct, my only comment is that the failure occurs when the
first message is received, and only the first CQE is a LOCAL_PROT_ERR.
The remainder of the posted receives are simply flushed.

Same result of course! The summary is ok as-is. Important fix.

Reviewed-by: Tom Talpey <tom@talpey.com>

> Fixes: 671c450b6fe0 ("xprtrdma: Fix oops in Receive handler after device removal")
> Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> ---
>   net/sunrpc/xprtrdma/verbs.c |    9 ++++-----
>   1 file changed, 4 insertions(+), 5 deletions(-)
> 
> Hi Anna-
> 
> Can you apply this for 6.5-rc ?
> 
> 
> diff --git a/net/sunrpc/xprtrdma/verbs.c b/net/sunrpc/xprtrdma/verbs.c
> index b098fde373ab..28c0771c4e8c 100644
> --- a/net/sunrpc/xprtrdma/verbs.c
> +++ b/net/sunrpc/xprtrdma/verbs.c
> @@ -935,9 +935,6 @@ struct rpcrdma_rep *rpcrdma_rep_create(struct rpcrdma_xprt *r_xprt,
>   	if (!rep->rr_rdmabuf)
>   		goto out_free;
>   
> -	if (!rpcrdma_regbuf_dma_map(r_xprt, rep->rr_rdmabuf))
> -		goto out_free_regbuf;
> -
>   	rep->rr_cid.ci_completion_id =
>   		atomic_inc_return(&r_xprt->rx_ep->re_completion_ids);
>   
> @@ -956,8 +953,6 @@ struct rpcrdma_rep *rpcrdma_rep_create(struct rpcrdma_xprt *r_xprt,
>   	spin_unlock(&buf->rb_lock);
>   	return rep;
>   
> -out_free_regbuf:
> -	rpcrdma_regbuf_free(rep->rr_rdmabuf);
>   out_free:
>   	kfree(rep);
>   out:
> @@ -1363,6 +1358,10 @@ void rpcrdma_post_recvs(struct rpcrdma_xprt *r_xprt, int needed, bool temp)
>   			rep = rpcrdma_rep_create(r_xprt, temp);
>   		if (!rep)
>   			break;
> +		if (!rpcrdma_regbuf_dma_map(r_xprt, rep->rr_rdmabuf)) {
> +			rpcrdma_rep_put(buf, rep);
> +			break;
> +		}
>   
>   		rep->rr_cid.ci_queue_id = ep->re_attr.recv_cq->res.id;
>   		trace_xprtrdma_post_recv(rep);
> 
> 
>