Subject: RE: lock stateid, sequence number and lease interaction

From: "Noveck, Dave" <[email-masked]>
Date: Fri, 13 Oct 2000 11:46:10 -0700

On second thought ...

First, Andy Adamson wrote: 
>> There needs to be an agreement on the interaction between
>> the lock sequence number, lease, and stateid error checking.
>> 
>> One issue is when does the client upon receiving an error
>> on a lock sequence number mutating operation decrement
>> it's lockowner sequence number?
>> 
>> I've divided the operations into three catagories and shown
>> the server's processing logic with questions for each, and 
>> would like to hear comments....
>> 
>> In what order is everyone else doing this processing on the 
>> server?
>> 
>> 
>> Operations that have a stateid and a sequence number 
>> ---------------------------------------------------- 
>> CLOSE, LOCK, LOCKU, OPEN_DOWNGRADE, 
>> OPEN_CONFIRM (stateid in verifier otherwise server 
>> can't locate lockowner)
>> 
>> The stateid has to be checked first - it is the only way
>> to find the lockowner.  If the stateid is bad, the sequence
>> number cannot be bumped because the lockowner can't be 
>> identified. So, the client when getting a NFS4ERR_XXX_STATEID
>> error needs to decrement the lockowners sequence number otherwise
>> it will be inconsistant for other lockowner operations.

Then I wrote,
> Right.  My server does not bump the sequence number or check
> for a replay, since it can't find the lockowner structure.

The interesting question here is "What is a bad stateid (for
the purposes of checking for a duplicate sequence number)?".
I think it is very easy to get this wrong.  I know.  I got
this wrong in my server (with hardly any effort at all :-),
and I suspect others may have as well.

If you do the obvious thing and call a routine which maps a
stateid to either the address of some sort of "state" structure
or an error (NFS4ERR_{STALE,BAD,OLD}_STATEID), your server will
be as broken as mine.  (I'm working on fixing it).

The good news (if you have this problem) is that nothing we are
likely to run at the Bake-off is going to run into this problem.
Simply running the connectathon tests is not going to find this.  
Even the more aggressive option of rebooting servers while running 
the connectathon tests is not going to find this kind of issue.

Come to think of it, that's actually the *bad* news.  It's
becoming clear to me that because V4 is a more sophisticated
(and stateful) protocol than earlier nfs versions, interoperability
testing is going to have to get more sophisticated as well.

> In the case of NFS4ERR_STALE_STATEID, the client should decide that 
> the server has rebooted so it should reset the sequence numbers,
> shouldn't it.

> The cases of NFS4ERR_{BAD,OLD}_STATEID should not occur if the
> client and server are functioning properly but I agree the client
> should undo any sequnce number increment.

Suppose the client does a LOCK and and the server sends back an 
updated stateid.  And suppose that that LOCK is retried and 
the retry is not caught by a response cache.  If you then 
try to check the stateid, so as to find the associated 
structure so you can find the owner to check the sequence-id, 
you are going to determine that the state-id is old.  In
this situation, if you returned NFS4ERR_OLD_STATEID before
checking for a duplicate sequence number, the whole duplicate
sequence number logic would be defeated.  Instead of getting
the old response, you would always get back NFS4ERR_OLD_STATEID.  
So you have to check the state-id first, but not completely.  
If the stateid maps to a state object which is still valid 
but has become out of date because of a single operation,
you must not return NFS4ERR_OLD_STATEID until you find the 
owner and make sure that this is not a duplicate. 

This leads to the interesting question of whether, when it
isn't a duplicate and you return NFS4ERR_OLD_STATEID, the
server should increment the sequence number.  Before, we had
agreed that it wouldn't and the client should undo its
increment.  I'm inclined now (based on the above) to think 
that each of these three cases should be handled differently
with regards to sequence number handling.

      NFS4ERR_STALE_STATEID -- as noted before, sequence knowledge
      is reset on both client and server.

      NFS4ERR_BAD_STATEID -- Should not happen normally but the
	server should not increment the sequence id (it can't) and
	the client should undo any increment on its side.

	NFS4ERR_OLD_STATEID -- Should not happen normally on requests
	that have a sequence number, but the server must be able to
	find the owner (to check for a duplicate) so server should
	increment the sequence id and the client should *not* undo
	any increment on its side.

I've saved the best for last :-(  Now suppose we have a CLOSE.
The obvious thing to do is to take the state entry associated
with the file and mark it unused so that if someone were to 
inadvertantly use that stateid he would get NFS4ERR_BAD_STATEID
and also put that entry in a free list so that it could be
reallocated when some totally unrelated OPEN was done, it 
would be used (with a new generation number) to assign a new
stateid for that new open.  Now consider what would happen
if the CLOSE was retried.  If the state entry were deallocated,
you would not be able to find the owner to check the sequence
number to find out it was a duplicate so that you could send
back the correct saved response (from the house that Jack built).

My conclusion is that you cannot deallocate the information
associated with the CLOSE'd state until another request with
the next sequence number arrives for the same lockowner,
or the client reboots, or there is no longer any locking state
for that owner and the server receives nothing from that 
lockowner for a while and forgets the associated lockowner 
information, yada, yada, yada.