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

From: Mike Eisler <[email-masked]>
Date: Wed, 1 Nov 2000 18:23:11 -0800 (PST)

> 1a) Don't increment seq-id on error

Keep in mind the stateid can change with every request (that's why it is
returned), and that all requests that take a seqid also take a
stateid. Thus if client increments the seqid, what stateid would it use?
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.

> 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.

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.

> 2) Response count for COMPOUND
> There was some confusion about the op count in the response
> for COMPOUND. If the final op has an error, is that
> included? While the consensus was that the right answer is,
> "Yes.", the fact that this has come up, suggests that the
> spec should be made clearer on this issue.

Section 13.2 says:

  The results of each operation are preceded by the opcode and
  a status code (normally zero).  If an operation results in a non-zero
  status code, the status will be encoded and evaluation of the
  compound sequence will halt and the reply will be returned.

Section 14.2 says:

     [...] The
     results for all of the evaluated operations must be returned to the
     client.
[...]
     Contained within the COMPOUND results is a "status" field.  If the
     results array length is non-zero, this status must be equivalent to
     the status of the last operation that was executed within the
     COMPOUND procedure.  Therefore, if an operation incurred an error
     then the "status" value will be the same error value as is being
     returned for the operation that failed.
     
IMHO, it's clear.

> 4) Lease and deadlocks
> 
> There was some concerns expressed about leases in a situation in which
> a given lockowner obtained a lock and then hung. If other processes
> continued unaffected then the leases associated with the locks held by
> the lockowner would be periodically renewed, even when, it could be
> argued, they shouldn't be.
> 
> The general consensus was that this behavior was inherent in the lease
> design and that it was the responsibility of the client operating
> system to deal with, but perhaps the spec should discuss this issue.

This seems like a discussion that belongs to forthcoming implementation
i-d.

> 5) CREATE/OPEN/MKDIR errors

> Sergei pointed out that when OPEN-create which encounters a
> directory with the target name, the spec does not provide
> for an informative error code. NFS4ERR_EXISTS doesn't
> really tell the client what happened. A separate

This is reasonable. There's plenty of whitespace in the error section
of OPEN to add NFS4ERR_ISDIR when and if the rfc editor gives
the authors the 48 hour window to make final edits.

> NFS4ERR_ISDIR would be better. Similarly when a CREATE of a
> directory encounters a file whose type is not a directory.

what operating environment would use this code?

for example, on my Solaris desk top, I get:

frostback% touch a
frostback% mkdir a
mkdir: Failed to make directory "a"; File exists

> 6) Server references delegations by FH, not stateid
> 7) TAG, what's it good for?

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.

So how important are these issues to the bake off crowd?

> 10) Initial sequence id

> In the case in which the server is requiring confirmation,
> it is pretty clear that any value presented by the client
> is OK, but what about the case in which no confirmation is
> required?

Since it's ok for the client to use any seqid when the server
requires confirmation, and since the client has no
idea if the server requires confirmation, it follows that
any value is permissable.

> 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.

> 18) Unlimited-length opaques. Just say "No"?

> Andy points out that there are a number of unlimited-length
> opaques in the protocol (e.g. client-string, lockowner) and
> that each each such adds to server vulnerabilities to
> denial-of-service attacks. The possibility of a 32 kbyte
> lockowners appearing makes coding of the server more
> complicated and will hurt performance in the normal case,
> unless great efforts are made to optimize server
> performance.

The danger with setting limits now is that the selections
will be to low.

Again, do we want to ask iesg to rescind approval?

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.

	-mre

---

From: "Noveck, Dave" <[email-masked]>
Date: Thu, 2 Nov 2000 07:28:08 -0800

> On Thu Nov 02 04:35:28 2000, Noveck, Dave wrote:

>> 
>> Given that it appears that avoiding another trip through the
>> buracracy trumps all other considerations, I guess we can
>> just live with the problem.
>> 
>
> The problem of Open upgrade and Open downgrade/ close being closeall
> is an issue which makes the NFS server on NT not an advisable solution
> since these functions CANNOT be implemented properly. The at the
> bake-off was that there was no NEED for these functions.

The context here was the specific issue of unlimited-size variable
arrays.  This is something that I think is nasty but we can live 
with it and I guess living with it is preferable to delay.

It is true I said "trumps *all* other considerations", but I
guess I was exagerating.  If you can't implement the current spec,
that can't be trumped by all the IETF process issues, however nasty
they may be.

I think there is a need to respond to opens and close on the client
that change the open mode as seen on the server.

If you can't implement open-mode or-ing with downgrade and single
close, we need to figure out another way to do it.  After thinking
about it some, I don't think it will be easy.
 
> What now, given the above ?????

I think you need to make the case that NT servers cannot implement
this as currently specified and then make a proposal as to how you 
could specify things to keep NT and UNIX both happy.

This is going to interact with the ongoing "What is a stateid?" discussion.

     Mike Eisler says essentially that it represents (lockowner, seqid) 
     tuple.

     I was thinking it represents a (lockowner, file, count of locking
     operations for the lockowner-file pair) tuple.

     You would like it to represent a (specific open file, count of locking
     operations on the specific open file) tuple.

I think there are problems with your approach which are complementary to
the problems you are having with the current approach (in either my or
Mike Eisler's variant).  You want the LOCK requests to have a stateid whcih
allows the server to know which open file they go with so the server can
do the NT-ish thing of issuing the lock on the proper open file.  As I
rememeber UNIX VFS interfaces, they don't like to pass open file information
down through that interface at all so UNIX clients would know which of
the open files a given lock was actually for, so there would be no way
for the fs implementation layer to know which stateid to use, if we were
to change stateid to be different for each open of the same file by the
same lockowner.

---

From: Mike Eisler <[email-masked]>
Date: Thu, 2 Nov 2000 11:29:44 -0800 (PST)

> >> 1a) Don't increment 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
 	T(i+1) may not be equal to T(i)
 	the client cannot assume that T(i) is valid for use with E(i+1).
 	
Since the client gets an error, and not a stateid, using an old stateid with
a new seqid is bad form.
 
> 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.
         
> 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.

> 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.

> 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.

> 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.

> > 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.

> 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?

	-mre

---

From: "Noveck, Dave" <[email-masked]>
Date: Fri, 3 Nov 2000 14:13:52 -0800

> > But we are saying precisely that E(i+1) = E(i), if the operation i fails.
> > 
> > Does the spec actually say that anywhere?
> 
> The example of how a stateid can be built includes a sequence value
> which changes. Aside from that, because lock state modifying operations
> return a stateid, it is implicit that T(i+1) is not necessarily equal
> to T(i), otherwise why return T(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.
> 
> Exactly. So don't increment i when you get a failure.

The context here is that you are referring to "the i'th lcok state
changing operation".  "i" is not a variable on the client or server.
It is part of the description of what is happening.  It is not
something you choose to increment or not.  Otherwise, you are in a
"Here's my i'th state-changing operation; Here's my other i'th
state-changing operation"-situation.  I'm confused already.  That
makes things impossible. 

> 
> > 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
> 
> Because new stateids get returned on each operation.

They get returned on each operatin that does not result in error.

> 
> > talk about that sequence or such a tie-in between seqid and state-id.
> 
> Section 8.1.3:
> 
>    The server must be able to distinguish the following situations and
>    return the error as specified:
> [...]
>    o    The stateid was generated by the current server instance but the
>         stateid no longer designates the current locking state for the
>         lockowner-file pair in question (i.e. one or more locking
>         operations has occurred).  The error NFS4ERR_OLD_STATEID should
>         be returned.
> 
>         This error condition will only occur when the client issues a
>         locking request which changes a stateid while an I/O request
>         that uses that stateid is outstanding.
> 
> Despite the reference to "lockowner-file pair", given that lock
> operations take a sequence number, and given that stateids are
> associated with lock state, if follows that stateids and seqids have
> a tie in. Of course, it is a permissable thing for a server to
> not change the stateid on any or every lock state modifying operation,
> but a client cannot assume that.

Well they have some sort of tie in.  But it is really a stretch to
construct the one-to-one correspondence you do and then argue that
correspondence requires you to do things a certain way to maintain
it.  I think you need to argue independently about when you 
increment sequence id's.  Then if this correspondence appears as
a consequence, so be it.  We differ on what is cause and what is
effect here.

> 
> > 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
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> 
> No requests that change the lock state.

True, but the issue of whether a stateid is valid is posed every time
the server receives one.  The paragraphs you quote above govern what 
the server does with a stateid presented to it.

Your model of when stateid are valid is one that seems only applicable
to request that receive a stateid and seqid together (which happen
to be those that send a new stateid back).  I don't think it makes
sense to have two models of stateid validity, one that refers to
seqid (for those that have it), and one that applies otherwise
(when the request has no seqid).  
 
> 
> > 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
> 
> What's an example of an operation that returns a new stateid without
> changing lock state?

There isn't one.

> 
> > 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.
> 
> The rule is quite implicit, and in any case is the most conservative
> interpretation. If a client assumes anything more, then it won't work
> against some servers.

As far as it being implicit, I'll agree that it is not explicit.

I don't think there is a conservative inetrpretation here.  The clients
and servers have to agree.  If the server considers the seqid incremented
when the client does not, they won't work together.  If the client
increments the seqid when the server does not, they won't work together.
They must agree.

Fortunately, we agree on when this should done.  The only thing we
disagree about (with regard to seqid's) is the reason to not increment
the seqid on error.  There is no reason we have to agree on that,
hwoever.

I propose we adjourn this particular aspect of our discussion to the
next time we meet over a beer.

> > 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 don't see how you can implement a server that associates a sequence
> of seqids with a given nfs_lockowner, but has stateids associated with
> a particular nfs_lockowner-file pair. E.g.
> 
> r1: OPEN { seq # 0, lockowner 0, file1 } ->
> <- fh1, stateid 1, OPEN4_RESULT_CONFIRM, confirm_verifier 
> 
> r2: OPEN_CONFIRM { fh1, seq # 1, confirm_verifier } ->
> <- stateid 2
> 
> r3: LOCK { fh1, seq # 2, stateid 2 } ->
> <- stateid 3
> 
> r4: OPEN { seq # 3, lockowner 0, file2 } ->
> <- stateid 4
> 
> r5: UNLOCK { fh1, seq # 4, stateid 4 } ->
> <- stateid 5
> 
> If your server doesn't return stateid errors for RPC r5,
> then this is wonderful.

My server would return BAD_STATEID for r5.  It checks the current
file ahndle and determines if the stateid makes sense in that contextt.

It wants the client to have the same stateid interpretation as it done:
stateid=owner+file.  Such a client will send stateid 3 and everything 
will work.

If clients and servers agree that stateid=owner, they will work together
also.

We just have to decide which interpretation will be adopted as the "right"
one.

> > Who said a lease expired here?
> 
> The scenario in mind is the client sends the CLOSE, which is the CLOSE
> for the last open file for this nfs_lockowner. The server returns
> NFS4_Ok (and a stateid that is uselss sicne there are no open files ...
> the server implementor would probably want to return a special stateid
> for this occasion).
> 
> However, the client doesn't get the response; it re-sends its CLOSE.
> The spec mandates that the server keep a lock state duplicate request
> cache consisting of the last response sent for given nfs_lockowner. If
> the client never gets a response after N seconds from the last
> successful response it got for that nfs_lockowner, where N is the lease
> time, then the client cannot reasonably expect his lease to be around,
> nor the lock state duplicate request cache. Thus the client has to be
> prepared to get back a stateid error on his retry, instead of the expected
> NFS4_OK, when resending after N seconds.
> 
> Hence, section 8.1.7 suggests lease expiration as a heuristic for when
> to delete nfs_lockowner state after close of the last file opened by
> the nfs_lockowner.
> 
> BTW, fodder for the implementation RFC is what sequence number does an
> nfs_lockowner use for an OPEN subsequent to a successful CLOSE of its
> last OPEN file. I claim it should be the sequence number used in that
> CLOSE, plus 1. If the OPEN is issued before the lease expires, the
> server won't return the confirm flag.

I think most servers will want to keep it far longer.  If memory is not
tight, it can avoid the confirm interaction almost all the time.
 
> > Exactly.  The value is wrong.  What you do not do is adjust some
> 
> Today. 14 years from now, I can imagine there will be at least
> as many desktop computers with 0.5 TB of ram, as there were desktops
> with 4 MB in 1986. And many of those boxes then ran NFS with 8192
> byte buffers, which happens to be 1/512 of 4 MB. 1/512 of 0.5TB is
> one GByte. As a server implementor, I'd rather provide a knob to
> allow the field to keep up with the times, versus forcing customers to 
> spend big $$$ for an upgrade.

I can see an expansion of write sizes with increasing memory.  Howver,
we have a mechanism to deal with that without creating an interoperability
nightmare, the maxwrite attribute.

With regard to stuff like lockowner, I just don't see it.  If I have 16 
exabuytes of RAM (=2**64), I can why I would want to do Terabyte transfers 
(oops, we have 4 Gig limit) but I don't see why I would have 32KB lockowners.

You can go the "More things in Heaven and Earth" route, but we are going 
to need transfers beyond 4 Gig far before we would need even 4K lockowners
(i.e. never).

I give up.  I'm obviously in the minority of those who are going to 
respond on this issue.

---

From: "Noveck, Dave" <[email-masked]>
Date: Mon, 6 Nov 2000 06:48:58 -0800

> > 
> > 3) Multi-component pathname issues
> > 
> > A number of proposals were made to more accurately report errors that
> > occur in handling multi-component pathnames for OPEN and LOOKUP. The
> > difficulties in recovering from a situation in which one of the
> > directories in a multi-component path name turned out to be a symlink
> > were discussed, with much gnashing of teeth.
> > 
> > The consensus was that these difficulties make use of multi-component
> > pathnames undesirable with a better alternative being COMPOUNDed
> > LOOKUP's. This makes it desirable to remove multi-component pathnames
> > from OPEN and LOOKU

> The issue of recovering from an error with the use of a pathname4 on
> OPEN and LOOKUP is one that the client needs to deal with.  The server
> processing should be straightforward.

The client only needs to deal with it if it uses the multi-component
pathname.  The question is why would it do so?  I can see no reason
that it should do.  To write, debug, and spend time executing 
comlicated error code normally requires a reason.  And I would
say, that unless you are writing a conformance test, simply
using the multi-component pathname feature, doesn't count as a
reason.

> Therefore, if the client doesn't want to deal with the error recovery
> it doesn't need to generate an OPEN or LOOKUP with a multi-component
> pathname4.  It can generate the appropriate COMPOUND.  If the client
> is smart enough to deal with the error recovery, then it has the
> option.

If it is smart (=clever) enough to do the error recovery and not smart
(=wise) enough to avoid doing it when it serves no purpose, then it
has that option, if we provide it.  But why should we provide it?

If you need to recover from an error in a multi-componenent pathanme,
what do you do?

     You issue lookup's on each component until you run into the
     problem.

     That is high-latency done on a per-request basis so you instead
     issue COMPOUND'ed single-component lookups.

     But wait.  If you had done that in the first place (instead of
     a multi-component lookup), you would have both,
  
          a) The correct result if there is no error (e.g. symlink)
             in the path, just as you would have gotten from a 
             multi-component lookup.

          b) Knowledge of where the problem is so you can do your
             READLINK.

So why would any client write a multi-component lookup which only
gives a) and then write separate recovery code to get b) and debug
both and double the latency to recover from a problem?  I conclude
that no client would do that and that multi-component lookup is not
a useful feature. 

> I don't see a need for any change.

I suppose it is better to have a useless feature than to lack one
that we really need.  It still bothers me however and it would be 
nice if we had a chance to change it.  I realize that might happen
though :-(