[PATCH v3 1/1] NFSD: fix error handling in NFSv4.0 callbacks

[PATCH v3 1/1] NFSD: fix error handling in NFSv4.0 callbacks

[Olga Kornievskaia]

If the server's attempts at sending a callback request fails either due
to connection or authentication issues, the server needs to set
NFS4ERR_CB_PATH_DOWN in response to RENEW so that client can recover.

Suggested-by: Bruce Fields <bfields@redhat.com>
Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
---
 fs/nfsd/nfs4callback.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
index 052be5bf9ef5..f436d2ca5223 100644
--- a/fs/nfsd/nfs4callback.c
+++ b/fs/nfsd/nfs4callback.c
@@ -1186,11 +1186,8 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
 		rpc_restart_call_prepare(task);
 		return;
 	case 1:
-		switch (task->tk_status) {
-		case -EIO:
-		case -ETIMEDOUT:
+		if (task->tk_status)
 			nfsd4_mark_cb_down(clp, task->tk_status);
-		}
 		break;
 	default:
 		BUG();
-- 
2.27.0

Re: [PATCH v3 1/1] NFSD: fix error handling in NFSv4.0 callbacks

[Chuck Lever]

> On Mar 9, 2021, at 1:23 PM, Olga Kornievskaia <olga.kornievskaia@gmail.com> wrote:
> 
> If the server's attempts at sending a callback request fails either due
> to connection or authentication issues, the server needs to set
> NFS4ERR_CB_PATH_DOWN in response to RENEW so that client can recover.
> 
> Suggested-by: Bruce Fields <bfields@redhat.com>
> Signed-off-by: Olga Kornievskaia <kolga@netapp.com>
> ---
> fs/nfsd/nfs4callback.c | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 052be5bf9ef5..f436d2ca5223 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -1186,11 +1186,8 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata)
> 		rpc_restart_call_prepare(task);
> 		return;
> 	case 1:
> -		switch (task->tk_status) {
> -		case -EIO:
> -		case -ETIMEDOUT:
> +		if (task->tk_status)

Can task->tk_status ever be positive here? Wondering if we want
a more explicit check like: (task->tk_status < 0)


> 			nfsd4_mark_cb_down(clp, task->tk_status);
> -		}
> 		break;
> 	default:
> 		BUG();
> -- 
> 2.27.0
> 

--
Chuck Lever