Subject: RE: issue with LOCK being able to return an update open_stateid

From: "Noveck, Dave" <[email-masked]>
Date: Wed, 4 Sep 2002 20:03:03 -0700

> In reviewing the state transitions in the protocol, I have found a
> problem with the LOCK operation in the case that a new lock_owner is
> being used.

> The general issue is that the server does not have a method of
> returning a new open_owner stateid at the transition from open_owner
> to lock_owner. 

That's true but I don't think this is a real problem in practice
(or even in theory, assuming you have a suitable theory :-).

> Since one of the purposes of the stateids and their
> respective transitions is to prevent things like out-of-order
> processing of READs and WRITEs,

They don't force strict in-order processing, but they do provide
ordering constraints within a set of IO and locking/open/close
operations.

So the question is whether this results in the inability to 
enforce an important ordering constraint.

The basic issue that the ordering constraints are intending to 
prevent is for an IO operation to be processed in a context in
which the locking environment has changed to make it no longer
valid to process.  In this connection, locking operations can be
divided into two classes: those that increase the rights of a 
given owner and those that reduce them.  If an IO operation is
processed after a locking operation of the first class, there is
really no issue.  If it was valid to issue the IO when I didn't
have a lock, getting one will not make it invalid for the IO
to be processed.  On the other hand, releasing a lock is different.
If an operation was valid to issue while I hold a lock, and it is
processed when I no longer hold it (for example), then this is the
kind of sequencing problem, the stateid logic is intended to prevent.
In fact, in order to be more uniform and simple, the existing 
stateid logic enforces more sequencing constraints than are absolutely
necessary, but the motivation derives from the cases when locking 
rights are reduced.

Typically, the operations which reduce locking rights are open-
downgrade and LOCKU.  However, since LOCK can act as a demote
(under POSIX locking), this too can in some cases reduce locking
rights.  However, in the case we are considering here, the first
LOCK for a given owner within an open, the demote case cannot occur,
so I think this is case in which we cannot check the sequencing but 
we really have no pressing need to. 

> the server is unable to protect
> against a scenario like the following:

> Client						Server
> ------						------
> OPEN(open_owner,seqid[1])	->		
> 				<-		OPENres(open_stateid[1])
> READ(open_stateid[1])		->		
>				<-		READres
> WRITE(open_stateid[1])		->
>	|response delayed|	<-		WRITEres
> WRITE(open_stateid[1]) [retrans]->
>
>	|response received to initial WRITE|
>
> LOCK(open_stateid[1],seqid[2],
>     lock_owner,lock_seqid[1])	->
>				<-		LOCKres(lock_stateid[1])
>				|retransmitted WRITE processed|
>				<-		WRITEres (of retrans request)

> So at the point of processing the retransmitted WRITE, the server has
> no method to determine if the retransmitted WRITE request (in this
> case) was sent before or after the LOCK operation was processed.
> It is true that the servers duplicate request cache may catch this
> scenario but it is not a guarantee.

But for this case to occur, the application must issue a WRITE while
the process (i.e. owner) in question, has no lock.  When the WRITE
is received with the open stateid, it is treated as issued by a process
which does not have any byte-range locks, and that is really valid
handling whether the process that will get the lock issued the IO,
or it was issued by a different process which never got a lock.

Compare this to the following case:

     After the open, process A and process B, each get a lock on the
     file and release it.  The result would be two independent 
     lock stateid sequences.  

     In that context if, process A issued an IO and process B issued
     a LOCK, they would be processed with no sequencing constraints 
     them.  The IO issued by A is treated as having no associated 
     locks (other than the open) and that is true whether it is 
     processed after or before the lock done by B.

Now in your case, we are assuming that all of this is done by a single
process so the tranposition of the lock and IO is not quite so obviously
harmless, but I still argue that this is a case of "No harm, no foul"
since the fact that you (validly) issued the IO with no lock implies 
to me that the server is perfectly justified as treating this issued
by an anonymous null lockowner (one that has no additional locks). 

> For the client and server to interact appropriately, the server must
> be able to return an open_stateid as well as a lock_stateid on the
> initial LOCK request.

> To correct this, I propose the following changes be made to the
> protocol description file: nfs4_prot.x
> 
> CHANGE:
> 
> struct LOCK4resok {
> 	stateid4	lock_stateid;
> };

> TO:
> 
> struct new_lock_owner_res4 {
>	stateid4	open_stateid;
>	stateid4	lock_stateid;
> };

> union LOCK4resok switch (bool new_lock_owner) {
> case TRUE:
> 	new_lock_owner_res4	open_lock_stateids;
> case FALSE:
> 	stateid4	lock_stateid;
> };

> As stated above, this allows the server to transition the open_stateid
> at the first LOCK for a lock_owner.  This therefore extends the
> coverage of stateid checking the server can do for I/O operations.

Yes.

> The client does not have any additional serialization beyond what is
> needed today since the client must serialize its requests via the
> open_seqid.  Therefore, no additional burden for the client.

There is no additional serialization among LOCK requests, but there
is additional serialization between LOCK requests and IO's.  If one
process is doing IO's and another process gets its first lock, then
you have to serialize there, where otherwise you wouldn't have to.
In particular, IO's that were validly issued may come, when processed
to have old (open) stateids, because another process using the same open
go a lock.  So you might have to reissue such IO's.  This isn't a very
common case, but you have to deal with it.

> This change is easy to make in implementations because the LOCK
> processing in this case will have access to the necessary data
> structures for checking the open_seqid and generating the new
> lock_owner data structures.

The big issue for me is that it's another over-the-wire protocol change
at a time when each such change complicates development and testing.
Having the July protocol, the August protocol, and now the September
protocol just adds confusion tracking down mismatched clients and
servers.  It pushes off the time when one can assemble a set of
multiple clients implementing the same protocol and test against
your server implementing that same protocol.  (Transpose for those
implementing and testing clients).

---

From: Spencer Shepler <[email-masked]>
Date: Thu, 5 Sep 2002 09:27:54 -0500

On Wed, Noveck, Dave wrote:

<deleted>

> by an anonymous null lockowner (one that has no additional locks). 

Yes. I agree with your assessment if we have a single process that is
opening and locking a file.  As an example of where a problem may
occur would be if a process were to open a file and initialize it with
some amount of data.  It then would fork a number of children that
would each then use file locking to interact amongst themselves for
access to the data.  In this one case, the protocol will not provide
the appropriate protection for the instance mentioned above.

> common case, but you have to deal with it.

When I mentioned serialization, I should have made it clear that I was
speaking of client serialization the lock sequence id.  As we have
discussed before, the client doesn't need to serialize the sending of
these requests if it is prepared to handle errors returned from the
server in the event that the requests are processed "out of order"
with respect to their sequence ids.  In any event, this really doesn't
have a impact on the decision to change the LOCK response.

> implementing and testing clients).

I agree that we need to be prudent with any potential protocol changes
but if we know something is broken and likely to cause problems then I
would hope that we would fix it.

So the question to the WG is: is this particular scenario broken
enough to fix it now?  Or should we leave as is and when/if this ever
breaks in practice we fix it in a minor version.

-- 
Spencer