Re: [PATCH 09/14] NFS: Force server to drop NFSv4 state

["J. Bruce Fields" <bfields@fieldses.org>]

On Mon, May 21, 2012 at 12:08:44PM -0400, bfields wrote:
> On Fri, May 18, 2012 at 06:06:33PM -0400, Chuck Lever wrote:
> > A SETCLIENTID boot verifier is nothing more than the client's boot
> > timestamp.  An NFSv4 server is obligated to wipe all NFSv4 state for
> > an NFS client when the client presents an updated SETCLIENTID boot
> > verifier.  This is how servers detect client reboots.
> > 
> > nfs4_reset_all_state() refreshes our client's boot verifier to trigger
> > a server to wipe this client's state.  This function is invoked when
> > an NFSv4.1 server reports that it has revoked some or all of a
> > client's NFSv4 state.
> > 
> > Soon we want to get rid of the per-nfs_client cl_boot_time field,
> 
> OK, I guess you don't need it if you never intend to send a callback
> update or anything?

(Never mind, I see in the next patch you're still remembering the
verifier, just giving it a broader scope?)

--b.

> 
> > however.  Without cl_boot_time, our NFS client will need to find a
> > different way to force the server to purge the client's NFSv4 state.
> > 
> > Because these verifiers are opaque (ie, the server doesn't know or
> > care that they happen to be timestamps), we can do force the server
> > to wipe NFSv4 state by using the same trick we use now, but then
> > immediately afterwards establish a fresh client ID using the old boot
> > verifier again.
> > 
> > Hopefully there are no extra paranoid server implementations that keep
> > track of the client's boot verifiers and prevent clients from reusing
> > a previous one.
> > 
> > Signed-off-by: Chuck Lever <chuck.lever@oracle.com>
> > ---
> > 
> >  fs/nfs/nfs4_fs.h   |    1 +
> >  fs/nfs/nfs4proc.c  |    9 +++++++--
> >  fs/nfs/nfs4state.c |   13 +++++++++++--
> >  3 files changed, 19 insertions(+), 4 deletions(-)
> > 
> > diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
> > index 03d3ff0..f164c73 100644
> > --- a/fs/nfs/nfs4_fs.h
> > +++ b/fs/nfs/nfs4_fs.h
> > @@ -24,6 +24,7 @@ enum nfs4_client_state {
> >  	NFS4CLNT_RECALL_SLOT,
> >  	NFS4CLNT_LEASE_CONFIRM,
> >  	NFS4CLNT_SERVER_SCOPE_MISMATCH,
> > +	NFS4CLNT_PURGE_STATE,
> >  };
> >  
> >  enum nfs4_session_state {
> > diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> > index 73cfd9e..c56d547 100644
> > --- a/fs/nfs/nfs4proc.c
> > +++ b/fs/nfs/nfs4proc.c
> > @@ -3937,8 +3937,13 @@ static void nfs4_construct_boot_verifier(struct nfs_client *clp,
> >  {
> >  	__be32 verf[2];
> >  
> > -	verf[0] = (__be32)clp->cl_boot_time.tv_sec;
> > -	verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
> > +	if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
> > +		verf[0] = (__be32)CURRENT_TIME.tv_sec;
> > +		verf[1] = (__be32)CURRENT_TIME.tv_nsec;
> 
> I suppose it's pretty unlikely this could happen within a jiffy of
> setting cl_boot_time?
> 
> Would it be simpler to use some special value (all-zeros?) instead?
> 
> --b.
> 
> > +	} else {
> > +		verf[0] = (__be32)clp->cl_boot_time.tv_sec;
> > +		verf[1] = (__be32)clp->cl_boot_time.tv_nsec;
> > +	}
> >  	memcpy(bootverf->data, verf, sizeof(bootverf->data));
> >  }
> >  
> > diff --git a/fs/nfs/nfs4state.c b/fs/nfs/nfs4state.c
> > index f8c06de..32cce4a 100644
> > --- a/fs/nfs/nfs4state.c
> > +++ b/fs/nfs/nfs4state.c
> > @@ -1615,7 +1615,7 @@ void nfs41_handle_recall_slot(struct nfs_client *clp)
> >  static void nfs4_reset_all_state(struct nfs_client *clp)
> >  {
> >  	if (test_and_set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state) == 0) {
> > -		clp->cl_boot_time = CURRENT_TIME;
> > +		set_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
> >  		nfs4_state_start_reclaim_nograce(clp);
> >  		nfs4_schedule_state_manager(clp);
> >  	}
> > @@ -1631,7 +1631,6 @@ static void nfs41_handle_server_reboot(struct nfs_client *clp)
> >  
> >  static void nfs41_handle_state_revoked(struct nfs_client *clp)
> >  {
> > -	/* Temporary */
> >  	nfs4_reset_all_state(clp);
> >  }
> >  
> > @@ -1652,6 +1651,10 @@ void nfs41_handle_sequence_flag_errors(struct nfs_client *clp, u32 flags)
> >  {
> >  	if (!flags)
> >  		return;
> > +
> > +	dprintk("%s: \"%s\" (client ID %llx) flags=0x%08x\n",
> > +		__func__, clp->cl_hostname, clp->cl_clientid, flags);
> > +
> >  	if (flags & SEQ4_STATUS_RESTART_RECLAIM_NEEDED)
> >  		nfs41_handle_server_reboot(clp);
> >  	if (flags & (SEQ4_STATUS_EXPIRED_ALL_STATE_REVOKED |
> > @@ -1762,6 +1765,12 @@ static void nfs4_state_manager(struct nfs_client *clp)
> >  
> >  	/* Ensure exclusive access to NFSv4 state */
> >  	do {
> > +		if (test_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state)) {
> > +			nfs4_reclaim_lease(clp);
> > +			clear_bit(NFS4CLNT_PURGE_STATE, &clp->cl_state);
> > +			set_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state);
> > +		}
> > +
> >  		if (test_and_clear_bit(NFS4CLNT_LEASE_EXPIRED, &clp->cl_state)) {
> >  			/* We're going to have to re-establish a clientid */
> >  			status = nfs4_reclaim_lease(clp);
> > 
> > --
> > 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