Subject: RE: bumping the seqid on which errors?

From: "Noveck, Dave" <[email-masked]>
Date: Wed, 21 Aug 2002 10:19:53 -0700

> A recent discussion brought up this problem with the spec.  
> Currently the last paragraph in Section 8.1.5 states:
>
> "The client MUST monotonically increment the sequence number 
> for the CLOSE, LOCK, LOCKU, OPEN, OPEN_CONFIRM, and 
> OPEN_DOWNGRADE operations.  This is true even in the event 
> that the previous operation that used the sequence number 
> received an error.  The only exception to this rule is if the 
> previous operation received one of the following errors: 
> NFS4ERR_STALE_CLIENTID, NFS4ERR_STALE_STATEID, 
> NFS4ERR_BAD_STATEID, NFS4ERR_BAD_SEQID."
> 
> I think there are a couple more errors that need to be added 
> to the list of "errors of which you do not bump the seqid":

> NFS4ERR_RESOURCE
> NFS4ERR_BADXDR
> NFS4ERR_SERVERFAULT
> NFS4ERR_NOFILEHANDLE
> NFS4ERR_BADHANDLE

> For NFS4ERR_RESOURCE, the resource error may be a temporary 
> memory resource allocation error and actually deals with the 
> incrementing of the seqid (tracking the last request of the 
> client) and the server may not be able to meet the 
> requirement.

My understanding of RESOURCE is that it is intended for use
when the resources are not available to process a given op
and that when you return it, the client is OK assuming that
nothing happened for that op.  (The spec probably doesn't say
anything of the kind but that's how I think of RESOURCE).
So if nothing has happened, not incrementing the seqid is
the right thing to do.

> The BADXDR case may happen during the attribute processing 
> which is encoded after the seqid and owner.  So the current 
> rules say that the server MUST increment seqid in the case 
> that there is an XDR decode error.  That doesn't seem 
> reasonable from an implementation perspective and isn't really 
> the intent.

BADXDR could happen before you get to the owner, in which case
there is no way you can increment the seqid.  Since the client
doesn't know where the xdr problem occurs, he needs a uniform
rule, which has to be, don't bump it on BADXDR.  The assumption
has got to be that the client doesn't change state until it has
decoded the entire op, so client and server should be OK with 
the number not being bumped.  There is no need to detect a
retransmission since nothing happened the first time.

> The same goes for SERVERFAULT.  It is another catch all error 
> for the server and may point to a resource like issue that 
> prevents the server from tracking the sequence id.

Unsure about this one.  If servers could do part of the operation
and then return SERVERFAULT, then they should record what they 
did and bump the seqid, so that we can sanely deal with
retransmissions.  If you know that when SERVERFAULT is returned,
nothing has *really* happened, then not incrementing is fine.

I guess I would lean to having this one increment the seqid.
If your server is a state in which something failed and isn't sure
what and it is unable to record information about what just happened,
but it feels perfectly able to continue processing new requests,
then somebody should put it out of its misery.  Let the client
at least see a clean reboot.

> In the cases of NOFILEHANDLE/BADHANDLE, it doesn't seem 
> reasonable to ask the server to continue processing a request 
> because the client has given it a malformed request.  In fact, 
> there may be a malicious client out 
> there that may be able to authenticate requests to the server 
> but has no access to the server's namespace.  With the current 
> requirement, it would still be able to obtain server resources 
> by the tracking of a seqid for an openowner for a request that 
> never used a PUT*FH operation before an OPEN.

If you go down this path, it is hard to know where to draw the
line.  What about case in which the current file handle for an
open is not a directory?  That seems to be a "bad" current
filehandle for an OPEN in a certain sense.

As far as the malicious client, he could do PUTROOTFH/OPEN.
That gives him a valid current fh which is a directory.  It
may fail since he can't guess the name of a file in the root
directory (and there may be only sub-directories), but it still
will increment the seqid, unless you are prepared to expand the
don't-increment-seqid-on-this-error class a whole lot more. 

> any humble opinions?

My humble opinion is that we should add BADXDR and RESOURCE and 
not the others.

---

From: Spencer Shepler <[email-masked]>
Date: Thu, 22 Aug 2002 00:09:32 -0500

> at least see a clean reboot.

I don't have a strong opinion about this case since we have the
RESOURCE error and this error is supposed to be a catchall for very
bad things on the server.  This just means that the server may be
implemented to return RESOURCE when it has an error and is too lazy to
increment the seqid.

> filehandle for an OPEN in a certain sense.

Well, for BADHANDLE, the server has the opportunity to do any
consistency checks at PUTFH, RESTOREFH so it has the choice of
erroring the COMPOUND at that point and not getting to a potential
OPEN, LOCK operation in the sequence.  So BADHANDLE can be left out.

> don't-increment-seqid-on-this-error class a whole lot more. 

Okay.  I will change it to stupid-client.  It is too stupid to build
requests correctly because of a bug and keeps sending them to the
server without an appropriate PUT*FH operation.  This consumes
resources on the server.  I suppose it is not a big deal since the
server can eventually timeout the open_owner information since the
sequence will never be confirmed in the stupid-client case but it is
still resource consumption.

> My humble opinion is that we should add BADXDR and RESOURCE and 

I agree with BADXDR and RESOURCE and am willing to give in on
BADHANDLE and SERVERFAULT.  I would like to see NOFILEHANDLE added to
the list for "no seqid increment".

-- 
Spencer