2023-10-10 17:23:55

by Chuck Lever

Subject: [PATCH v1] svcrdma: Drop connection after an RDMA Read error

From: Chuck Lever <[email protected]>

When an RPC Call message cannot be pulled from the client, that
is a message loss, by definition. Close the connection to trigger
the client to resend.

Cc: <[email protected]>
Signed-off-by: Chuck Lever <[email protected]>
---
net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
index 85c8bcaebb80..3b05f90a3e50 100644
--- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
+++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
@@ -852,7 +852,8 @@ int svc_rdma_recvfrom(struct svc_rqst *rqstp)
if (ret == -EINVAL)
svc_rdma_send_error(rdma_xprt, ctxt, ret);
svc_rdma_recv_ctxt_put(rdma_xprt, ctxt);
- return ret;
+ svc_xprt_deferred_close(xprt);
+ return -ENOTCONN;

out_backchannel:
svc_rdma_handle_bc_reply(rqstp, ctxt);

2023-10-13 00:23:57

by Tom Talpey

Subject: Re: [PATCH v1] svcrdma: Drop connection after an RDMA Read error

On 10/10/2023 1:23 PM, Chuck Lever wrote:
> From: Chuck Lever <[email protected]>
>
> When an RPC Call message cannot be pulled from the client, that
> is a message loss, by definition. Close the connection to trigger
> the client to resend.

This looks correct, but it seems there are actually two changes here,
it's initiating the close but it's also unconditionally returning
-ENOTCONN. Other similar code paths do this so it's ok but the
altered return value is a bit mysterious.

Reviewed-by: Tom Talpey <[email protected]>

>
> Cc: <[email protected]>
> Signed-off-by: Chuck Lever <[email protected]>
> ---
> net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> index 85c8bcaebb80..3b05f90a3e50 100644
> --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
> @@ -852,7 +852,8 @@ int svc_rdma_recvfrom(struct svc_rqst *rqstp)
> if (ret == -EINVAL)
> svc_rdma_send_error(rdma_xprt, ctxt, ret);
> svc_rdma_recv_ctxt_put(rdma_xprt, ctxt);
> - return ret;
> + svc_xprt_deferred_close(xprt);
> + return -ENOTCONN;
>
> out_backchannel:
> svc_rdma_handle_bc_reply(rqstp, ctxt);
>
>
>

2023-10-13 12:02:51

by Chuck Lever III

Subject: Re: [PATCH v1] svcrdma: Drop connection after an RDMA Read error

> On Oct 12, 2023, at 8:23 PM, Tom Talpey <[email protected]> wrote:
>
> On 10/10/2023 1:23 PM, Chuck Lever wrote:
>> From: Chuck Lever <[email protected]>
>> When an RPC Call message cannot be pulled from the client, that
>> is a message loss, by definition. Close the connection to trigger
>> the client to resend.
>
> This looks correct, but it seems there are actually two changes here,
> it's initiating the close but it's also unconditionally returning
> -ENOTCONN. Other similar code paths do this so it's ok but the
> altered return value is a bit mysterious.

It's conventional for transport top level methods to return
-ENOTCONN if the connection is closing or closed.


> Reviewed-by: Tom Talpey <[email protected]>

Thanks!


>> Cc: <[email protected]>
>> Signed-off-by: Chuck Lever <[email protected]>
>> ---
>> net/sunrpc/xprtrdma/svc_rdma_recvfrom.c | 3 ++-
>> 1 file changed, 2 insertions(+), 1 deletion(-)
>> diff --git a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
>> index 85c8bcaebb80..3b05f90a3e50 100644
>> --- a/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
>> +++ b/net/sunrpc/xprtrdma/svc_rdma_recvfrom.c
>> @@ -852,7 +852,8 @@ int svc_rdma_recvfrom(struct svc_rqst *rqstp)
>> if (ret == -EINVAL)
>> svc_rdma_send_error(rdma_xprt, ctxt, ret);
>> svc_rdma_recv_ctxt_put(rdma_xprt, ctxt);
>> - return ret;
>> + svc_xprt_deferred_close(xprt);
>> + return -ENOTCONN;
>> out_backchannel:
>> svc_rdma_handle_bc_reply(rqstp, ctxt);

--
Chuck Lever