Subject: RE: Stateid and seqid

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 30 Jan 2001 09:36:54 -0800

Kendrick Smith wrote:
> On Mon, 29 Jan 2001, Sergey Klyushin wrote:
> 
> >     > I don't think you have to separately save the last superseded (I 
> >     > think that's what you mean here; expired has some other
> >     > connotations) stateid.
> >     >
> >     > What I do is first use the stateid to map to an entry in my table
of
> >     > state structures.  At this point, I may generate STALE_STATEID and
> >     > BAD_STATEID errors, but not OLD_STATEID.  This requires that I
know
> >     > the range of valid values of the stateid subfield which is
incremented
> >     > on each change, but I need that anyway to properly distinguish
between
> >     > BAD_STATEID and OLD_STATEID.
> >     >
> >     > Once I have the address of a state structure, I can use this to
> >     > find the corresponding owner structure which has the current seqid
> >     > and replay information.  So at this point, I can notice a replay
> >     > and respond.
> >     >
> 
> In our server implementation, we can't determine the lockowner 
> corresponding to an old stateid without retaining an entry in our stateid
> table.  (We found doing this to be impractical using 64-bit stateids.)  So
> it seems that retaining the last superseded stateid may or may not be an
> issue, depending on the server implementation.  I do think that this would
> be a useful remark for the implementor's guidelines (but probably not
> needed in the spec).
> 
> > BTW, how useful ERROR_OLD_STATEID for client? What client suppose to do
> > after receiving this error?
> 
> I was wondering this too.. On our client, we couldn't really make use of
> the distinction between NFS4ERR_OLD_STATEID and NFS4ERR_BAD_STATEID
> (although NFS4ERR_STALE_STATEID is certainly useful, as an entry point to
> the server-reboot recovery code).  Offhand, does anyone know the rationale
> for making seperate errors NFS4ERR_OLD_STATEID, NFS4ERR_BAD_STATEID?

See my answer to Sergei.

> 
> > So, you don't store stateid (or even full request) for reply message,
> > but you are based only on seqid = stored_seqid? How you could be sure,
> > that it's real replay, but not logical error? I want to be sure at
> > this point, that I understand replay correctly: for me it's real
> > retransmission of exactly the same package. Is it true? BTW, do you
> > check lease time before reply?
> 
> We only retransmit the response if the old request and new request are
> byte-by-byte identical, including rpc headers.  The client's lease is
> checked and renewed only if the seqid is correct.  Is this what everyone
> else is doing?

The entire request???

If I do LOCK followed by WRITE of 64K, are you going to save the 64K
I write so that on a subsequent replay you can compare the entire
request including all that data?  Seems like a lot of work.

I'm not doing that. 

I implement the seqid logic at the operation level.  If the seqid
matches, the saved response information becomes the response
information for that operation.  Preceding and subsequent operations
within the same request are handled normally (although they are subject 
to the ordinary replay cache so if the entire request hits there is no 
real execution of any operation in the request; the old reply is just
resent).

Your mention of the rpc headers prompts me to think that I might 
reach across levels here and save the transaction id in the per-lockowner
replay information.  Checking that when the seqid matches would not
add significant overhead. 

> 
> >     > I don't think so.  In the case of open, I don't see how you can do
> > without
> >     > a seqid.  The fact that it generates a stateid (updated or not)
> > doesn't
> >     > give you any replay protection.
> >     >
> >     > I send an open for read-write, deny=read-write.  It succeeds and I
> > send
> >     > back a stateid.  The reply is lost.  A replay of the OPEN is sent
and
> >     > it fails because open for read or write is denied, which is
definitely
> >     > not what you want.  You might fall back on various kind of ad hoc
> > replay
> >     > protection mechanisms specific to OPEN but that doesn't give me a
very
> > good
> >     > feeling.
> > I don't see any place when you need both seqid and stateid in one
request.
> > OPEN may take seqid and generate stateid, and stateid will be used later
> > without seqid
> > But, probably, changing seqid to stateid is really too late now
> > (even if it's needed only for OPEN).
> 
> It does seem that, if per-lockowner seqids were used only for OPEN, and
> per-(lockowner, file) stateids were used everywhere else, replay
> protection would still be ensured.  But this would have the side effect of
> requiring the server to cache replays on a per-(lockowner, file) basis; my
> feeling is that this would negatively impact performance.

Depends on what information you are saving for the cache replays.

---

From: "Sergey Klyushin" <[email-masked]>
Date: Tue, 30 Jan 2001 15:58:10 -0500

Kendrick Smith wrote:
> corresponding to an old stateid without retaining an entry in
> our stateid table.  (We found doing this to be impractical using 64-bit
> stateids.)  So it seems that retaining the last superseded stateid may or
> may not be an issue, depending on the server implementation.  I do think
> that this would be a useful remark for the implementor's guidelines (but
probably not
> needed in the spec).

According to spec, if you can't locate nfs_lockowner by stateid,
it means BAD_STATEID or STALE_STATEID, but not OLD_STATEID

> > > retransmission of exactly the same package. Is it true?
> >
Kendrick Smith wrote:
> > byte-by-byte identical, including rpc headers.
>
Noveck Dave wrote:
> .....

According to spec., NFS Server must store and replay last response to any
request with seqid only, but not for all requests. Looks like processing
of replay is implementation depended. It must be based on seqid, but spec.
says nothing more. Probably, I would compare full request also (at least
for the first v.4 implementation) to avoid logical bugs.

>
> > BTW, do you check lease time before reply?
>
Kendrick Smith wrote:
> The client's lease is checked and renewed only if the seqid is correct.
Is this
> what everyone else is doing?
>
Noveck Dave wrote:
>Right now my lease expiration code isn't there yet.  My inclination
>is to treat replay as merely retransmitting what already happened,
>so I wouldn't check it.

I would not check and renew lease on replay also.
It would be consistent with RPC layer cashing.
Not renewing lease on replay could cause some problems: if NFS client
finally
receives reply, the next request may get ERR_EXPIRED, but using RENEW could
help,
and my feeling, that it's better, than keeping file opened (locked,...)
forever, plus
it will be consistent with RPC layer cashing.

Another point to check lease at OPEN, if clientid OK -> nfs_lockowner
found -> seqid OK.
I would not check lease here also, but I would renew one.
OPEN is the main point where lease starts and lease is one for all
nfs_lockowners
from the same client. If lease has expired before, but NFS Server still has
nfs_lockowner in his table, checking lease at this point will never allow to
OPEN a file.

In case clientid OK -> nfs_lockowner NOT found, I would NOT check lease
also, bump seqid,
renew lease and ask for OPEN_CONFIRM.

One more note: good stateid and replay_seqid should never happened. It's
logical error.
I think BAD_SEQ_NUM should be returned.

As far as I remember, there was agreement not to bump seqid, if any error
returned
(including ERR_EXPIRED).
What about lease? I would NOT renew lease on ANY error also.

request.
> > without seqid But, probably, changing seqid to stateid is really too
late now
> protection would still be ensured.  But this would have the
> side effect of requiring the server to cache replays on a per-(lockowner,
> file) basis; my feeling is that this would negatively impact performance.
>
No, replies are stored per seqid (which is per lockowner), but it doesn't
look like
we are going to change spec. now anyway. I would look at this with more
details
for the first minor version.

Sergey