| Message ID | 1445350911-63530-1-git-send-email-aweits@rit.edu (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Tue, Oct 20, 2015 at 10:21:51AM -0400, Andrew Elble wrote: > Assuming a client has lost a delegation: Are clients really allowed to just lose a delegation? (Have you seen such a case, other than the duplicate-delegation case which you already fixed?) > If the server goes to recall > the delegation, an attempt is made to recall it twice separated by > a delay of 2 seconds. Both times, the client will state that it > doesn't have the delegation via -EBADHANDLE or -NFS4ERR_BAD_STATEID. > > 1.) Any race between a delegation grant and a recall has been > presumably avoided by the delay and second attempt. If something happened to the forechannel connection, then I believe it could take longer than 2 seconds to time out and recover. So I'm inclined to instead fix any bugs that result in servers and client disagreeing about whether there's a delegation. Another thing we could do here is finally implement the server-side support for referring triples (I think the client's done?): http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues#Referring_triples https://tools.ietf.org/html/rfc5661#section-2.10.6.3 That would eliminate the need for the recall retries. Though that would still leave open the question of how to handle those errors on a recall. We still not be able to conclude that it's safe for the server to destroy the delegation. --b. > 2.) The client doesn't have the delegation. > 3.) The backchannel is responsive. > > After these two attempts fail, the laundromat will eventually revoke > them and add these delegations to cl_revoked. This results in another > attempt to get the client to return the delegation via > TEST/FREE STATEID. This will also fail with no means > of resolution, and will cause the server and client to loop > indefinitely, as the client has nothing to give the server to satisfy it. > > The changes here are to establish a safe way to recover by: > > If the client has responded with -EBADHANDLE or -NFS4ERR_BAD_STATEID: > 1.) Not failing the backchannel after two attempts at a recall. > 2.) At the time revocation would normally occur: destroying the > delegation on the server side. > > Signed-off-by: Andrew Elble <aweits@rit.edu> > --- > fs/nfsd/nfs4state.c | 14 ++++++++++++-- > 1 file changed, 12 insertions(+), 2 deletions(-) > > diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c > index da21df673ed9..340ff365df4d 100644 > --- a/fs/nfsd/nfs4state.c > +++ b/fs/nfsd/nfs4state.c > @@ -3578,7 +3578,7 @@ static int nfsd4_cb_recall_done(struct nfsd4_callback *cb, > rpc_delay(task, 2 * HZ); > return 0; > } > - /*FALLTHRU*/ > + return 1; > default: > return -1; > } > @@ -4451,7 +4451,17 @@ nfs4_laundromat(struct nfsd_net *nn) > dp = list_first_entry(&reaplist, struct nfs4_delegation, > dl_recall_lru); > list_del_init(&dp->dl_recall_lru); > - revoke_delegation(dp); > + if ((dp->dl_recall.cb_status != -EBADHANDLE) && > + (dp->dl_recall.cb_status != -NFS4ERR_BAD_STATEID)) { > + revoke_delegation(dp); > + } else { > + dprintk("nfsd: client: %.*s is losing delegations", > + (int)dp->dl_recall.cb_clp->cl_name.len, > + dp->dl_recall.cb_clp->cl_name.data); > + put_clnt_odstate(dp->dl_clnt_odstate); > + nfs4_put_deleg_lease(dp->dl_stid.sc_file); > + nfs4_put_stid(&dp->dl_stid); > + } > } > > spin_lock(&nn->client_lock); > -- > 2.4.6 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> Are clients really allowed to just lose a delegation? (Have you seen > such a case, other than the duplicate-delegation case which you already > fixed?) In short, yes, we're still seeing it. We have also been seeing increasing stability from the work that has been done (which also increases the time between replication). The reason for v2 was I got coverage on the destruction path in testing and discovered my mistake in v1. This one is extremely frustrating to chase down (I've been 14+ hours deep in packet captures to try and find the allocation to the recall - I keep running out of disk space). The operational reason to avoid this is because a lost delegation effectively kills the mount when the state managers are looping. I have a patch that alters fault injection to recreate the scenario. >> 1.) Any race between a delegation grant and a recall has been >> presumably avoided by the delay and second attempt. > > If something happened to the forechannel connection, then I believe it > could take longer than 2 seconds to time out and recover. Hrm, yes I see. > So I'm inclined to instead fix any bugs that result in servers and > client disagreeing about whether there's a delegation. Not abandoning trying to track it down. It's quite clearly going to take a while. > Another thing we could do here is finally implement the server-side > support for referring triples (I think the client's done?): > > http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues#Referring_triples > https://tools.ietf.org/html/rfc5661#section-2.10.6.3 > > That would eliminate the need for the recall retries. > > Though that would still leave open the question of how to handle those > errors on a recall. We still not be able to conclude that it's safe for > the server to destroy the delegation. Would this be more appropriately "fixed" by supporting DELEGPURGE in a limited fashion to clear out cl_revoked? (I'm not quite sure that's a valid interpretation of RFC5561) to clear out any "lost" delegations? Thanks, Andy
On Tue, Oct 20, 2015 at 02:34:15PM -0400, Andrew W Elble wrote: > > > Are clients really allowed to just lose a delegation? (Have you seen > > such a case, other than the duplicate-delegation case which you already > > fixed?) > > In short, yes, we're still seeing it. We have also been seeing increasing > stability from the work that has been done (which also increases the > time between replication). > > The reason for v2 was I got coverage on the destruction path in > testing and discovered my mistake in v1. This one is extremely > frustrating to chase down (I've been 14+ hours deep in > packet captures to try and find the allocation to the recall - I > keep running out of disk space). Ugh. Maybe patching in some well-chosen printk's could help. Or doing some filtering as you capture. You could capture just operations that might have a delegation stateid--OPEN, DELEGRETURN, CB_RECALL. And probably SEQUENCE replies with any flags set. > > Another thing we could do here is finally implement the server-side > > support for referring triples (I think the client's done?): > > > > http://wiki.linux-nfs.org/wiki/index.php/Server_4.0_and_4.1_issues#Referring_triples > > https://tools.ietf.org/html/rfc5661#section-2.10.6.3 > > > > That would eliminate the need for the recall retries. > > > > Though that would still leave open the question of how to handle those > > errors on a recall. We still not be able to conclude that it's safe for > > the server to destroy the delegation. > > Would this be more appropriately "fixed" by supporting DELEGPURGE in a > limited fashion to clear out cl_revoked? (I'm not quite sure > that's a valid interpretation of RFC5561) to clear out any "lost" > delegations? I haven't thought it through, but that's clearly not what's intended for, so I'm pessimistic. Note also we'd need to implement it first. There may be other ways the client could better recover, but we'd still rather avoid such situations in the first place. The recovery logic is already complicated enough, it would be worse if it also needed to handle a lot of cases that could only occur due to outright bugs. --b. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c index da21df673ed9..340ff365df4d 100644 --- a/fs/nfsd/nfs4state.c +++ b/fs/nfsd/nfs4state.c @@ -3578,7 +3578,7 @@ static int nfsd4_cb_recall_done(struct nfsd4_callback *cb, rpc_delay(task, 2 * HZ); return 0; } - /*FALLTHRU*/ + return 1; default: return -1; } @@ -4451,7 +4451,17 @@ nfs4_laundromat(struct nfsd_net *nn) dp = list_first_entry(&reaplist, struct nfs4_delegation, dl_recall_lru); list_del_init(&dp->dl_recall_lru); - revoke_delegation(dp); + if ((dp->dl_recall.cb_status != -EBADHANDLE) && + (dp->dl_recall.cb_status != -NFS4ERR_BAD_STATEID)) { + revoke_delegation(dp); + } else { + dprintk("nfsd: client: %.*s is losing delegations", + (int)dp->dl_recall.cb_clp->cl_name.len, + dp->dl_recall.cb_clp->cl_name.data); + put_clnt_odstate(dp->dl_clnt_odstate); + nfs4_put_deleg_lease(dp->dl_stid.sc_file); + nfs4_put_stid(&dp->dl_stid); + } } spin_lock(&nn->client_lock);
Assuming a client has lost a delegation: If the server goes to recall the delegation, an attempt is made to recall it twice separated by a delay of 2 seconds. Both times, the client will state that it doesn't have the delegation via -EBADHANDLE or -NFS4ERR_BAD_STATEID. 1.) Any race between a delegation grant and a recall has been presumably avoided by the delay and second attempt. 2.) The client doesn't have the delegation. 3.) The backchannel is responsive. After these two attempts fail, the laundromat will eventually revoke them and add these delegations to cl_revoked. This results in another attempt to get the client to return the delegation via TEST/FREE STATEID. This will also fail with no means of resolution, and will cause the server and client to loop indefinitely, as the client has nothing to give the server to satisfy it. The changes here are to establish a safe way to recover by: If the client has responded with -EBADHANDLE or -NFS4ERR_BAD_STATEID: 1.) Not failing the backchannel after two attempts at a recall. 2.) At the time revocation would normally occur: destroying the delegation on the server side. Signed-off-by: Andrew Elble <aweits@rit.edu> --- fs/nfsd/nfs4state.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-)