Subject: RE: Spec-related discussions at the Bake-off

From: "Noveck, Dave" <[email-masked]>
Date: Thu, 2 Nov 2000 13:52:09 -0800

> > >> 1a) Don't increment seq-id on error
[..]
> > > The previous stateid is valid only with seqid X, not X+1. So that's
> > > why we don't increment the seq-id on error.
> > 
> > I don't understand your point.  

> A sequence consists of a series of
>  { E0, T0 }, { E1, T1 } ... { En, Tn }
>  
>  where 
>  	Ei is a sequence number for the i'th lock state changing operation
>  	Ti is a state id fr i'th lock state changeing operation
>  such that
>  	E(i+1) = E(i) + 1

But we are saying precisely that E(i+1) = E(i), if the operation i fails.

>  	T(i+1) may not be equal to T(i)

Does the spec actually say that anywhere?
 
> 	the client cannot assume that T(i) is valid for use with E(i+1).

Well if I say that E(i+1) = E(i), if operation i+1 fails, then if T(i)
isn't valid with E(i+1) = E(i), then the client is dead in the water.

I suppose your version of this would be that when operation i fails,
we simply edit it out of the operation history (It's not a state-chainging
operation since it failed to change state), so that when we execute the 
next state-changing operation (which I would call operation i+2 and you
would call operation i+1), then yes E(i+1) = E(i) + 1.

But why can the client not assume that T(i) is valid for use with a new
sequence id?  To maintain the {En, Tn} sequence?  But the spec doesn't
talk about that sequence or such a tie-in between seqid and state-id.
It might be better if it did.  Remember that state-id's are also used
in requests that don't have sequence id's.  When does the stateid become
invalid?  When you get a new one that replaces it, or actually when the
server executes an operation that changes it.  Since we have a model
of when stateid become invalid (get OLD_STATEID when used), it makes
sense for that to also apply when they are used with sequence id's.
I don't see that we have a right to create this {En, Tn} rule which
I don't see in the spec, although I might have missed it.
 	
> Since the client gets an error, and not a stateid, using an old stateid with
> a new seqid is bad form.

I think we agree on the conclusion, but your reasoning seems to me
circular.
 
> > >> 1c) Remembrance of state-id's past
> > >> 
> > >> During the discussion of item 1, Dave brought up the issue of a
> > >> retransmitted CLOSE. To respond correctly, it is necessary to compare
> > >> the sequence id to the sequence id for the lockowner. This necessitates
> > >> knowing the lockowner. The problem is that the lockowner can only be
> > >> found using the stateid passed in the CLOSE. If the CLOSE succeeded,
> > >> then you would expect the state entry to be deallocated, which means
> > >> that you would get a BAD_STATEID error and the server will be unable to
> > >> find the lockowner necessary to treat this as a duplicate.
> > 
> > > A stateid is associated with an nfs_lockowner, not an open
> > > file. If there are multiple open files associated with the
> > > nfs_lockowner, then removing the nfs_lockowner is not allowed.
> > 
> > Well there is some difference of opinion on this.  My understanding
> > is that it designates a given lockowner-file pair.  I think I got this
> > impression in a discussion with Robinson, but at this point I can't
> > be sure.  Anyway, this is the way my server is implemented.

> Well, I think your server is wrong. Section 8.1.5:
> 
>    Note that for requests that contain a sequence number, the client,     |
>    for each nfs_lockowner, there should be no more than one outstanding   |
>    request.          
>   
> Awkwardly written, but clear.

Yes, but I'm not sure why that indicates my server is wrong.  As far
as sequence id's, that is precisely my understanding and my server
does nothing that contradicts that.
         
> > I could change thing if we decide that is the right way to do 
> > things.  One worry about doing it this way is it increases the
> > chances of running into an OLD_STATEID error sicne a lock and
> > an IO will conclict even if they are on different files.
> > 
> > There is some stuff in the spec that refers to it as designating a
> > lockowner-file pair, which I think I wrote.
>
> Yes, I see that. It is unfortunate that the spec contradicts itself
> in that manner. However, the fact that CLOSE returns a stateid is
> pretty compelling.

I think you have made your case on a more-proabbale-than-not standard.

I'm not ready to concede on a clear-and-convincing-evidence standard,
just yet.  I need to think about this some more.

A big issue now is that my server interoperates with a number of
clients, and I don't want to have it stop inter-operating.  So it seems
that a number of implementations have to make this change together.

What do my co-conspirators from the bake-off think about this?

> > > If this is the last open file associated with the nfs_lockowner, then
> > > there are issues, which are discussed in section 8.1.7 of the i-d.
> > > In a nutshell, removing an nfs_lockowner before a time period
> > > corresponding to the lease time is not advisable.
> > 
> > Gievn stateid=lockowner, yes, although I don't see why the timeout
> > is necessarily the lease time.

> A client cannot expect lock operations to work after the lease expires.

Who said a lease expired here?

When the lockowner does a CLOSE, that may be the last open file
for this client in which case there is no lease to expire or to
renew.  The question is for how long should the server be able
to deal correctly with the retransmission of non-idempotent
rqeuests, of which CLOSE is one.  The idea is that the normal
reply cache is not sufficient.  What is the basis for saying that
lease interval must be long enough?

If the CLOSE closes the last file for this lockowner and there
are open files for other lockowner, then these lease may be 
periodicaly renewed by those other lcokowners.  If they are,
what about the retransmitted close.  How long should the
sequence mechanism be able to seal with that?  If the answer 
is "for the lease period", why is that?

> > > Is there a proposal to alter version 4.0 of spec, already
> > > approved by iesg, with publication imminent? This amounts
> > > asking iesg to reverse its decision.
> > 
> > Don't know about all this IETF esoterica.  I thought it was
> > a Proposed Standard, but that changes could be made, based on
> > implementation experience, after Proposed Standard on the 
> > way to it becoming a Draft Standard.
>
> In theory. In practice, people will implement to the first RFC.

> > >> 16) Stateid returned on CLOSE. Why?
> > >
> > >> There seems to be no valid use for the stateid returned on
> > >> CLOSE, since the file is no longer opened by that
> > >> lockowner. What is it there for?
> > 
> > > Stateids are per nfs_lockowner not per open file.  Thus the CLOSE
> > > causes the sequence number to be incremented on the server's and
> > > client's nfs_lockowner state structure.  The new stateid is used on the
next
> > > operation that takes the stateid, and so, the seqid and stateid better
> > > match what the server expects.
> > 
> > Most likely you are going to do an OPEN with doesn't take a stateid

> Just as likely you'll do an operation on another file the same nfs_lockowner
> alreayd has open, and it will need to use the stateid from CLOSE.

Yes.  Both are possible.

> > > Keep in mind that that this issue exists already in the
> > > record marking header that prefixes every rpc message.  If
> > > there is a 32 kbyte nfs lockowner, then over TCP, the
> > > record marking lengths will sum to over 32 kb. Implementors
> > > *are* making sure that the record marking length is
> > > reasonable before further processing right?
> > 
> > > Thus, TCP is your friend.
> 
> > > If using UDP, and if unreasonable (as compared to tunable
> > > values) lengths are passed, why not return an error or
> > > treat it as an XDR decode error, and log it in the
> > > implementation. Since the limits will be tunable, they can
> > > be upped if necessary. Practically speaking,
> > > implementors will select limits that work in most cases, because
> > > they've tested with each other.
> 
> Actually, I realized that RPC over UDP has a prefixed length, it's
> simply in UDP transport header instead of an RPC header. So, the denial
> of service vulnerabilities seem rather limited. Yes, a client could
> establish an nfs_lockowner that is 32KB or more, and this information
> would have to be cached on the server (especially if one accepts my
> interpretation of nfs_lockowner, seqid, and stateid correspondence).
> Were I implementing a server, I'd cut off nfs_lockowner size at 1 KByte
> or less, and provide a knob to tune it higher if needed.

If I did that, I would get beat up by a bunch of my colleagues shouting,
"No knobs."  Knobs --> Service calls --> Money wasted + unhappy
customers.  This stuff is not going to be run in the main by UNIX
hackers (and even UNIX hackers will probably think dealing with this
stuff is kind of annoying).

> > Asking customers to tune various variables when they get 
> > XDR errors, is job security for somebody, but it doesn't
> > really make sense.
> 
> So, what do NFS/TCP implementations do today when presented with a
> record marking header that is say, one gigabyte? Surely they break the
> connection?

Exactly.  The value is wrong.  What you do not do is adjust some
value to make this valid.  I have no objection in giving errors to those
who generate invalid requests (and disconecting those when the nature
of things make resyncing impossible).  What I object is to create a
class of requests that may or may not be valid depending on somebody's
setting of an option.  We should agree on appropriate limits.  If we
can't have those in the spec, then they should be in the implementation
RFC.

---

From: Spencer Shepler <[email-masked]>
Date: Thu, 2 Nov 2000 16:51:01 -0600

> can't have those in the spec, then they should be in the implementation

Interesting discussion.  It is hard for me to guess about what the
appropriate limits would be for some of the opaque<> or <> types in
the protocol.  For example, what is the appropriate limit for the
"data" field of WRITE4args?  What will it be 5 or 10 years from now?
How about "attrlist4"?  If it contains an "fattr4_acl" attribute, what
is the limit on the number of "nfsace4" entries for that attribute.

I agree that we should make suggestions in the implementation RFC but
to dictate some of these in the protocol specification seems short
sighted.

If there is a denial of service concern, NFSv4 does provide for
authentication so that the source can be identified and actually
denied access to the server's resources if it continues to be a
problem.

-- 

- Spencer -