On Tue, 09 Mar 2021, J. Bruce Fields wrote: > On Tue, Mar 09, 2021 at 09:41:27AM -0500, Olga Kornievskaia wrote: > > From: Olga Kornievskaia <kolga@xxxxxxxxxx> > > > > When the server tries to do a callback and a client fails it due to > > authentication problems, we need the server to set callback down > > flag in RENEW so that client can recover. > > I was looking at this. It looks to me like this should really be just: > > case 1: > if (task->tk_status) > nfsd4_mark_cb_down(clp, task->tk_status); > > If tk_status showed an error, and the ->done method doesn't return 0 to > tell us it something worth retrying, then the callback failed > permanently, so we should mark the callback path down, regardless of the > exact error. That switch was added because the server was erroneously setting SEQ4_STATUS_CB_PATH_DOWN in the event that a client returned an NFS-level error for a CB_RECALL that the client had already done a FREE_STATEID for. Removing the switch will cause the server to go back to that behaviour, won't it? -Scott > > --b. > > > > > Signed-off-by: Olga Kornievskaia <kolga@xxxxxxxxxx> > > --- > > fs/nfsd/nfs4callback.c | 1 + > > 1 file changed, 1 insertion(+) > > > > diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c > > index 052be5bf9ef5..7325592b456e 100644 > > --- a/fs/nfsd/nfs4callback.c > > +++ b/fs/nfsd/nfs4callback.c > > @@ -1189,6 +1189,7 @@ static void nfsd4_cb_done(struct rpc_task *task, void *calldata) > > switch (task->tk_status) { > > case -EIO: > > case -ETIMEDOUT: > > + case -EACCES: > > nfsd4_mark_cb_down(clp, task->tk_status); > > } > > break; > > -- > > 2.27.0 > > >