Subject: Re: [nfsv4] 5th round of typos and fixes to the NFSv4.1 specification

From: "Mike Eisler" <[email-masked]>
Date: Tue, 8 Sep 2009 11:17:30 -0700

On Tue, September 8, 2009 10:02 am, Noveck, Dave wrote:
> A couple of tiny things I noticed.
>
> In the table for ILLEGAL, you need a comma between the two errors.

OK

>
> Since, in the case of ILLEGAL and also the old ops not supported in v4.1
> (e.g. SETCLIENTID), you can't return NFS4ERR_RETRY_UNCACHED_REP, you

Interesting. Let's say we have this example:

client ->
server <-

-> COMPOUND { SEQUENCE slotid=0, seq=1, cache_this=FALSE, PUTFH, READ }

server receives and executes

client does not get reply before connection drop

-> COMPOUND { SEQUENCE slotid=0, seq=1, cache_this=FALSE, SETCLIENTID }

The server sees a retry of something the client promised to not
retry. While I agree you the server MAY return NOTSUPP, were I
to write a server, I would want to return NFS4ERR_RETRY_UNCACHED_REP

Very rarely in the NFSv4.1 spec do we REQUIRE that one error be
returned over another when multiple errors are applicable. I don't
see why we'd require it for this instance.

Now replace the retry with,

-> COMPOUND { SEQUENCE slotid=0, seq=1, cache_this=FALSE, ILLEGAL }

and there I agree with you, because NFS4ERR_RETRY_UNCACHED_REP
is not among the permitted errors for ILLEGAL, we cannot
return NFS4ERR_RETRY_UNCACHED_REP.
And that this stage in a ratififed I-D, I don't want to go there.
(Where I'd like to go, in NFSv4.2, is to deprecate
the ILLEGAL operation and replace with a new pseudo operation, called
"ERROR", which the server can substitute for any operation in
the reply, where ERROR can return any status value except NFS4_OK.)

However, NFS4ERR_RETRY_UNCACHED_REP is among the legal errors for
PUTFH (the original operation in the original request at that
position). So nothing prevents me from replying with an op
code of PUTFH, and the error NFS4ERR_RETRY_UNCACHED_REP.

And BTW, if I had this example,

-> COMPOUND { SEQUENCE slotid=0, seq=1, cache_this=TRUE, PUTFH, READ }

server receives and executes

client does not get reply before connection drop

-> COMPOUND { SEQUENCE slotid=0, seq=1, cache_this=FALSE, ILLEGAL }

my server would probably return the results of PUTFH and READ.
If I had a moment of weakness, I might return NFS4ERR_SEQ_FALSE_RETRY
from SEQUENCE.

I don't want my replier detecting retries, and then parsing what
comes after the Sequence. Requesters are supposed to be
intelligent entities.

> have to return the whole response in this case (it isn't any longer but
> you have to return the real error and not NFS4ERR_RETRY_UNCACHED_REP).
> The problem is that the first of the alternatives is written assuming
> that when one picks this option (return the whole reply), one does so
> uniformly ("to simplify implementation").  I think you need to say
> something that acknowledges that for certain weird cases such as
> SEQUENCE-SETCLIENTID, it is the only choice, even if one normally does
> the second case.

I agree we need text to address this.

>
> -----Original Message-----
> From: Mike Eisler [mailto:[email-masked]]
> Sent: Sunday, September 06, 2009 12:26 PM
> To: [email-masked]
> Subject: [nfsv4] 5th round of typos and fixes to the NFSv4.1
> specification
>
>
> We've had a number of issues of clarification come up in the past
> several months, and these are now addressed. I suggest looking very
> soon at what the changes as it appears the RFC Editor's engagement with
> editing of the NFSv4.1 drafts is imminent.
>
> The changes are at:
>
> http://eisler.com/nfsv41/0_README.html
>
> Search for PAv5
>
> Here is the summary of the what has changed:
>
> PAv5: Typos. Clarify the use of the NFS4ERR_RETRY_UNCACHED_REP and add
> it
> to every operation in the error table. Several attributes were misnamed.
> Fixed a critical copy/paste error in the MDS recovery section. Clarify
> the
> NFS4ERR_SEQ_FALSE_RETRY error and what happens when a client uses a
> different credential. Clarify the use of loca_stateid in LAYOUTCOMMIT
> when
> doing a reclaim.
>
> In the dot x file I-D, several attributes from earlier versions of the
> I-D
> were still in place, so they were removed. Several attributes were
> misnamed. The change politcy attribute was missing its fattr4_ data
> type.
> Some #endif's did not have a comment around the argument to #endif, and
> this breaks some C compilers.
>
> --
> Mike Eisler, Senior Technical Director, NetApp, 719 599 9026,
> http://blogs.netapp.com/eislers_nfs_blog/
>
>
>
> _______________________________________________
> nfsv4 mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/nfsv4
>

-- 
Mike Eisler, Senior Technical Director, NetApp, 719 599 9026,
http://blogs.netapp.com/eislers_nfs_blog/

---

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 8 Sep 2009 15:18:05 -0400

> The server sees a retry of something the client promised 
> to not retry. While I agree you the server MAY return 
> NOTSUPP, were I to write a server, I would want to return 
> NFS4ERR_RETRY_UNCACHED_REP

> Very rarely in the NFSv4.1 spec do we REQUIRE that one 
> error be returned over another when multiple errors are 
> applicable. I don't see why we'd require it for this 
> instance.

Me either but we're not requiring it for this case.  The only error that
is allowed for SETCLIENTID is NFS4ERR_NOTSUPP.  As it stands
NFS4ERR_RETRY_UNCACHED_REP is not allowed for SETCLIENTID and I don't
see the point of changing that.

> and there I agree with you, because 
> NFS4ERR_RETRY_UNCACHED_REP is not among the permitted 
> errors for ILLEGAL, we cannot return 
> NFS4ERR_RETRY_UNCACHED_REP.

Exactly.  (ILLEGAL, NFS4ERR_OP_ILLEGAL) is exactly the same as the case
of (SETCLIENTID, NFS4ERR_NOTSUPP).  Except that SETCLIENTID can't return
NFS4ERR_BADXDR but let's not get into that.

> And that this stage in a ratififed I-D, I don't want to 

Me either.

> the ILLEGAL operation and replace with a new pseudo 
> operation, called "ERROR", which the server can 
> substitute for any operation in the reply, where 
> ERROR can return any status value except NFS4_OK.)

Fine with me.

> However, NFS4ERR_RETRY_UNCACHED_REP is among the 
> legal errors for PUTFH (the original operation in 
> the original request at that position). So nothing 
> prevents me from replying with an op code of PUTFH, 
> and the error NFS4ERR_RETRY_UNCACHED_REP.

Right, but I was talking about the case in which the retries used the
ops as the original requests.

> my server would probably return the results of PUTFH 
> and READ.  If I had a moment of weakness, I might 
> return NFS4ERR_SEQ_FALSE_RETRY from SEQUENCE.

OK.

> I don't want my replier detecting retries, and then 
> parsing what comes after the Sequence. Requesters 
> are supposed to be intelligent entities.

Sure but the spec has to deal with all the cases in which they aren't.

> > have to return the whole response in this case 
> (it isn't any longer but you have to return the 
> real error and not NFS4ERR_RETRY_UNCACHED_REP).
> The problem is that the first of the alternatives 
> is written assuming that when one picks this option 
> (return the whole reply), one does so uniformly 
> ("to simplify implementation").  I think you need 
> to say something that acknowledges that for certain 
> weird cases such as SEQUENCE-SETCLIENTID, it is 
> the only choice, even if one normally does
> the second case.

> I agree we need text to address this.

I think your examples point out the need to be very specific about which
op is returned in the response in cases in which the retry and the
original request are not different.

-----Original Message-----
From: Mike Eisler [mailto:[email-masked]] 
Sent: Tuesday, September 08, 2009 2:18 PM
To: Noveck, Dave; [email-masked]
Subject: RE: [nfsv4] 5th round of typos and fixes to the NFSv4.1
specification

OK

client ->
server <-

---

From: "Mike Eisler" <[email-masked]>
Date: Tue, 8 Sep 2009 13:11:14 -0700

Here is what I am thinking of putting into PAv6:

***************
*** 2606,2625 ****
--- 2606,2647 ----
          NFS4ERR_RETRY_UNCACHED_REP in reply to
          a Sequence operation if the Sequence
          operation is the first operation.

          </t>

          </list>

         </t>

+        <t>
+         If the second operation is an illegal operation, or an
+         operation that was legal in a previous minor version of
+         NFSv4 and MUST NOT
+         be supported in current minor version (e.g. SETCLIENTID), the
+         replier MUST NOT ever return NFS4ERR_RETRY_UNCACHED_REP.
+         Instead the replier MUST return NFS4ERR_OP_ILLEGAL, or
+         NFS4ERR_BADXDR, or NFS4ERR_NOTSUPP as appropriate.
+        </t>
+
+        <t>
+         If the second operation can result in another error status,
+         the replier MAY return a status other than
NFS4ERR_RETRY_UNCACHED_REP,
+         provided the operation is not executed in such a way that the state
+         of the replier is changed. Examples of such
+         an error status include: NFS4ERR_NOTSUP returned for an
+         operation that is legal but not REQUIRED in the current
+         minor versions, and thus not supported by the replier;
+         NFS4ERR_SEQUENCE_POS; and NFS4ERR_REQ_TOO_BIG.
+        </t>
+
+
         </list>
        </t>
       </section> <!-- Optional Reply Caching -->
      </section> <!-- Slot Identifiers and Server Reply Cache -->

      <section anchor="Retry and Replay" title="Retry and Replay of Reply">
      <t>
       A requester MUST NOT retry a request, unless
       the connection it used to send the request
       disconnects. The requester can then reconnect

---

From: Trond Myklebust <[email-masked]>
Date: Tue, 08 Sep 2009 16:54:51 -0400

> Here is what I am thinking of putting into PAv6:

This looks extremely ugly. What if it is the third operation, or the
fourth? IOW, why are we special-casing the error return value for the
second operation but not all the rest?

Also, is there a similar issue in the case where the client is caching,
and the client does a FALSE_RETRY with an illegal op?

---

From: Trond Myklebust <[email-masked]>
Date: Tue, 08 Sep 2009 16:57:13 -0400

> and the client does a FALSE_RETRY with an illegal op?

I meant to say 'is there a similar issue in the case where the client
requests caching and subsequently does a FALSE_RETRY with an illegal
op?'

---

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 8 Sep 2009 18:03:24 -0400

> This looks extremely ugly. 

I'd be surprised if we were able to do this in a aesthetically desirable
way.  After all, Mike is working under the constraint that the change
must be minimal.  

> What if it is the third operation, or the fourth? 
> IOW, why are we special-casing the error return value 
> for the second operation but not all the rest? 

Because that is the op for whose error code we substitute
NFS_RETRY_UNCACHED_REP.  That is what is the source of the special
casing.

> Also, is there a similar issue in the case where 
> the client is caching, and the client does a 
> FALSE_RETRY with an illegal op?

FALSE_RETRY is reported as an error for the SEQUENCE.

I don't think that is an option for RETRY_UNCACHED_REP because we need
the OK response for SEQUENCE and then need to say that you have to retry
this (with a new sequence) on your own.  You shouldn't get FALSE_RETRY
unless you are broken, while valid client (if it ever uses cache_this ==
FALSE), will have to be prepared to get RETRY_UNCACHED_REP.

-----Original Message-----
From: Trond Myklebust [mailto:[email-masked]] 
Sent: Tuesday, September 08, 2009 4:55 PM
To: Mike Eisler
Cc: Noveck, Dave; [email-masked]
Subject: Re: [nfsv4] 5th round of typos and fixes to the NFSv4.1
specification

---

From: "Noveck, Dave" <[email-masked]>
Date: Tue, 8 Sep 2009 18:15:46 -0400

How about adding something like:

The discussion above assumes that the retried request matches the
original one.  Since the server may only cache a small amount of the
information that would be required to determine whether this is a case
of a false retry, the server may send to the client, any of the
following responses:

* The response for the original request.

* A response that consists only of the SEQUENCE op with the error
NFS4ERR_FALSE_RETRY.

* A response that consists of the response for SEQUENCE with the error
NFS4_OK, together with the second op as it appeared in the retried
request with a response of NFS4ERR_RETRY_UNCACHED_REP or other error as
described above.

* A response that consists of the response for SEQUENCE with the error
NFS4_OK, together with the second op as it appeared in the original
request with a response of NFS4ERR_RETRY_UNCACHED_REP or other error as
described above.

Does this make sense? [Not to be added. :-]

-----Original Message-----
From: Mike Eisler [mailto:[email-masked]] 
Sent: Tuesday, September 08, 2009 4:11 PM
To: Noveck, Dave
Cc: [email-masked]
Subject: RE: [nfsv4] 5th round of typos and fixes to the NFSv4.1
specification

***************
*** 2606,2625 ****
--- 2606,2647 ----

          </t>

          </list>

         </t>

+        <t>
+         provided the operation is not executed in such a way that the
state

      <section anchor="Retry and Replay" title="Retry and Replay of
Reply">
      <t>
       A requester MUST NOT retry a request, unless
       the connection it used to send the request
       disconnects. The requester can then reconnect

On Tue, September 8, 2009 12:18 pm, Noveck, Dave wrote:
>> retry. While I agree you the server MAY return NOTSUPP, were I to 
>> write a server, I would want to return NFS4ERR_RETRY_UNCACHED_REP
>
> Me either but we're not requiring it for this case.  The only error 
> that is allowed for SETCLIENTID is NFS4ERR_NOTSUPP.  As it stands 
>> NFS4ERR_RETRY_UNCACHED_REP is not among the permitted errors for 
>> ILLEGAL, we cannot return NFS4ERR_RETRY_UNCACHED_REP.
>
> Exactly.  (ILLEGAL, NFS4ERR_OP_ILLEGAL) is exactly the same as the 
> case of (SETCLIENTID, NFS4ERR_NOTSUPP).  Except that SETCLIENTID can't

> return NFS4ERR_BADXDR but let's not get into that.
>
>> (Where I'd like to go, in NFSv4.2, is to deprecate the ILLEGAL 
>> operation and replace with a new pseudo operation, called "ERROR", 
>> which the server can substitute for any operation in the reply, where

>> position). So nothing prevents me from replying with an op code of 
>> PUTFH, and the error NFS4ERR_RETRY_UNCACHED_REP.
>
>> -> COMPOUND { SEQUENCE slotid=0, seq=1, cache_this=TRUE, PUTFH, READ 
>> -> }
>
>> my server would probably return the results of PUTFH and READ.  If I 
>> had a moment of weakness, I might return NFS4ERR_SEQ_FALSE_RETRY from

>> SEQUENCE.
>> comes after the Sequence. Requesters are supposed to be intelligent 
>> entities.
>
>> (it isn't any longer but you have to return the real error and not 
>> NFS4ERR_RETRY_UNCACHED_REP).

>> SEQUENCE-SETCLIENTID, it is the only choice, even if one normally 
>> does the second case.
>
> I think your examples point out the need to be very specific about 
> which op is returned in the response in cases in which the retry and 
> the original request are not different.
>
>
> -> COMPOUND { SEQUENCE slotid=0, seq=1, cache_this=FALSE, PUTFH, READ 
> -> }
>
> -> COMPOUND { SEQUENCE slotid=0, seq=1, cache_this=FALSE, SETCLIENTID 
> -> }
>
> The server sees a retry of something the client promised to not retry.

> While I agree you the server MAY return NOTSUPP, were I to write a 
> server, I would want to return NFS4ERR_RETRY_UNCACHED_REP
>
> returned over another when multiple errors are applicable. I don't see

> why we'd require it for this instance.
>
> and there I agree with you, because NFS4ERR_RETRY_UNCACHED_REP is not 
> among the permitted errors for ILLEGAL, we cannot return 
> (Where I'd like to go, in NFSv4.2, is to deprecate the ILLEGAL 
> operation and replace with a new pseudo operation, called "ERROR", 
> which the server can substitute for any operation in the reply, where 
> position). So nothing prevents me from replying with an op code of 
> PUTFH, and the error NFS4ERR_RETRY_UNCACHED_REP.
>
> I don't want my replier detecting retries, and then parsing what comes

> after the Sequence. Requesters are supposed to be intelligent 
> entities.
>

>> SEQUENCE-SETCLIENTID, it is the only choice, even if one normally 
>> does the second case.
>
>> PAv5: Typos. Clarify the use of the NFS4ERR_RETRY_UNCACHED_REP and 
>> add it to every operation in the error table. Several attributes were
> misnamed.
>> Fixed a critical copy/paste error in the MDS recovery section. 
>> Clarify the NFS4ERR_SEQ_FALSE_RETRY error and what happens when a 
>> client uses a different credential. Clarify the use of loca_stateid 
>> in LAYOUTCOMMIT when doing a reclaim.
>>
>> In the dot x file I-D, several attributes from earlier versions of 
>> the I-D were still in place, so they were removed. Several attributes

>> were misnamed. The change politcy attribute was missing its fattr4_ 
>> data type.

---

From: "Mike Eisler" <[email-masked]>
Date: Tue, 8 Sep 2009 15:40:20 -0700

inline ...
>> second operation but not all the rest?

Because the second operation is a special case:

"On a per-request basis the requester can choose to direct the replier
   to cache the reply to all operations after the first operation
   (SEQUENCE or CB_SEQUENCE) via the sa_cachethis or csa_cachethis
   fields of the arguments to SEQUENCE or CB_SEQUENCE. "

"after the first operation" -> ergo, the second operation and beyond
are what [c]sa_cachethis control.

If [c]sa_cachethis is FALSE and the replier agrees, then the replier still
has to cache the reply to Sequence, and it has to return an error
for the second operation. The text I'm proposing lists the
conditions where that second operation won't get a returned error
code of  NFS4ERR_RETRY_UNCACHED_REP.

I don't see what the third and fourth operation have to do with
anything. If the requester sent,

  SEQUENCE (sa_cache_this=FALSE), SETCLIENTID, RENEW }

then we never get to the third operation on the original
request or the retry because SETCLIENTID produces
an error (NOTSUPP).

If the requester sent,

  SEQUENCE (sa_cache_this=FALSE), PUTFH, RENEW }

then we get to the RENEW on the original request, but not
not on the retry, because PUTFH returns NFS4ERR_RETRY_UNCACHED_REP .

> requests caching and subsequently does a FALSE_RETRY with an illegal

Yes. I'll make sure the discussion on FALSE_RETRY covers this, or refers
to the discussion on NFS4ERR_RETRY_UNCACHED_REP .

---

From: "Mike Eisler" <[email-masked]>
Date: Tue, 8 Sep 2009 15:48:05 -0700

> Does this make sense? [Not to be added. :-]

Ah, looks like Trond and you are thinking along the same lines with
respect to _FALSE_RETRY, and I like your treatment of the issues.

>> }
>> }