Subject: FW: [nfsv4] Re: [pnfs] what error should be returned toanimpatientclient?

From: "Iyer, Rahul" <[email-masked]>
Date: Wed, 11 Apr 2007 14:59:10 -0700

Forwarding it to the lit as my alias caused a bounce.
Regards
Rahul

> -----Original Message-----
> From: Iyer, Rahul 
> Sent: Wednesday, April 11, 2007 2:24 PM
> To: 'Halevy, Benny'; Noveck, Dave; [email-masked]; 
> [email-masked]
> Subject: RE: [nfsv4] Re: [pnfs] what error should be returned 
> toanimpatientclient?
> 
> 
> 
> > -----Original Message-----
> > From: Halevy, Benny [mailto:[email-masked]]
> > Sent: Wednesday, April 11, 2007 12:02 PM
> > To: Noveck, Dave; [email-masked]; [email-masked]
> > Subject: RE: [nfsv4] Re: [pnfs] what error should be returned 
> > toanimpatientclient?
> > 
> > Dave Noveck said:
> > > 
> > > > - Return a new error, say: NFS4ERR_INPROGRESS. I don't 
> really like
> > > >   this because it is accomodating a broken client.
> > > 
> > > The accommodation consists of giving him an error so he can
> > note that
> > > he has done something wrong and fix it.  I'm not against
> > that sort of
> > > accommodation.
> > > 
> > > > - Return NFS4ERR_DELAY. I like this because it uses an existing
> > > >   error and so is not providing accomodation to brokeness.
> > > 
> > > So the broken client will not get fixed.  DELAY is a normal and 
> > > expected error which indicates the server is not ready to do 
> > > something, and that it isn't the client's fault.  So the
> > problem won't
> > > get fixed as easily.
> > > 
> > > Why is this better?
> > > 
> > > Would you still object if the error were NFS4ERR_BROKEN_CLIENT or 
> > > somethng else that made it absolutely clear the client is
> > not supposed
> > > to do this and we are only providing the error in the 
> interests of 
> > > getting the client fixed.
> > 
> > I agree with Dave on all points above.  First, we better return an 
> > error before tearing down the session.  If the bug on the client 
> > implementation is rare enough it could still be useful for 
> end users 
> > and returning an error, logging it, and handling it properly (by 
> > closing and recreating the session on the client side) will give 
> > developers a better chance to fix the bug.
> > 
> > Errors must be unique per client error recovery procedure. I.e.
> > different error conditions can return the same error code if the 
> > client is expected to recover for these conditions in the 
> same way.  
> > Recovering from this particular case is unique therefore it needs a 
> > different error code from the ones already defined for 
> SEQUENCE.  To 
> > avoid introducing a new error code, how about returning 
> NFS4ERR_INVAL 
> > in this case?
> > (although NFS4ERR_DAMN_YOU_CLIENTS does seem nicer :)
> >
> 
> Just to add to Benny's original question, our current 
> implementation has a curious side effect of "allowing" the 
> slot to be used by 2 RPCs simultaneously. In the current 
> implementation, if the aforementioned scenario does occur, 
> the sequenceid on the slot isn't bumped up and the client 
> eventually sees NFSERR_SEQ_MISORDERED errors on the next 
> request in that slot. So, despite the server "allowing" it, 
> from a pure client side view, NFSERR_SEQ_MISORDERED is what 
> the client currently sees.
> Thanks
> Regards
> Rahul
> 
> 
> 
> > > 
> > > -----Original Message-----
> > > From: Mike Eisler [mailto:[email-masked]]
> > > Sent: Wednesday, April 11, 2007 11:11 AM
> > > To: [email-masked]
> > > Subject: Re: [nfsv4] Re: [pnfs] what error should be 
> returned to an 
> > > impatientclient?
> > > 
> > > 
> > > 
> > > --- "Talpey, Thomas" <[email-masked]> wrote:
> > > 
> > > > Well, since the client violated the no-retransmit 
> principle, the 
> > > > server could simply close the connection. In fact on an RDMA 
> > > > transport, this may happen from the hardware itself. In
> > my opinion
> > > > this is what the server should do. But, if the server 
> is going to 
> > > > return an error, it should be NFS4ERR_SEQ_MISORDERED.
> > > 
> > > _MISORDERED is for when the client sends a sequenceid 
> that is less 
> > > than the sequencedid in the reply cache or greater (by two or
> > > more)
> > > than the sequenceid in reply cache.
> > > 
> > > I think there are four approaches:
> > > 
> > > - disconnect. One can look at my 2006 connectathon presentation as
> > >   to why that is a bad idea. Even though the spec allows it, don't
> > >   do it.
> > > 
> > > - The server just processes the request as if it it a new 
> one. This
> > >   is the Rick Macklem approach (see his presentation from 2006
> > >   connectathon). So there are two or more requests outstanding
> > >   on the same slot, sequenceid pair. The first request
> > >   that completes on the server side gets to update the cache.
> > >   The client gets what it deserves. However I suspect this is
> > >   more complex logic for the v4.1 server than for the v4.0
> > >   server.
> > > 
> > > - Return a new error, say: NFS4ERR_INPROGRESS. I don't really like
> > >   this because it is accomodating a broken client.
> > > 
> > > - Return NFS4ERR_DELAY. I like this because it uses an existing
> > >   error and so is not providing accomodation to brokeness.
> > > 
> > > >
> > > > Remember, in this kind of situation, the slot and sequence are 
> > > > completely invalid, and the exchange is basically outside the 
> > > > session. There is no replay protection, and the client
> > will have to
> > > > use XID matching to detect the server's reply. Basically,
> > > 
> > > To make it clear to everyone, there is no slot table replay
> > protection
> > > because SEQUENCE is returning an error, versus a sequenceid and 
> > > slotid. Hence that is why the xid is needed.
> > > 
> > > > it's an invalid protocol request, almost as if the client
> > had sent
> > > > RPC garbage.
> > > >
> > > > Tom.
> > > >
> > > > At 10:19 AM 4/11/2007, Benny Halevy wrote:
> > > > >while reviewing the sessions text in draft-10 I realized
> > that it is
> > > > >not clear what error the server should return if the
> > client sent a
> > > > >new request on a slot that's still in use.
> > > > >
> > > > >In the currently linux implementation (in Andy's 
> latest patches)
> > > > >nfsd4_sequence() just allows that
> > > > >	status = nfs_ok;
> > > > >	if (nfs41_get_slot_state(slot) != NFS4_SLOT_AVAILABLE)
> > > > >		goto out;
> > > > >...
> > > > >out:
> > > > >	dprintk("%s returns %d\n", __FUNCTION__, status);
> > > > >	nfs4_unlock_state();
> > > > >	return status;
> > > > >
> > > > >I suggest returning NFS4ERR_BADSLOT or add a new error code
> > > > NFS4ERR_BUSYSLOT
> > > > >(other alternatives could be NFS4ERR_INVALID or
> > > > NFS4ERR_SEQ_MISORDERED).
> > > > >
> > > > >Benny
> > > > >_______________________________________________
> > > > >pNFS mailing list
> > > > >[email-masked]
> > > > >http://linux-nfs.org/cgi-bin/mailman/listinfo/pnfs
> > > >
> > > >
> > > > _______________________________________________
> > > > nfsv4 mailing list
> > > > [email-masked]
> > > > https://www1.ietf.org/mailman/listinfo/nfsv4
> > > >
> > > 
> > > 
> > > _______________________________________________
> > > nfsv4 mailing list
> > > [email-masked]
> > > https://www1.ietf.org/mailman/listinfo/nfsv4
> > > 
> > > _______________________________________________
> > > nfsv4 mailing list
> > > [email-masked]
> > > https://www1.ietf.org/mailman/listinfo/nfsv4
> > 
> > _______________________________________________
> > nfsv4 mailing list
> > [email-masked]
> > https://www1.ietf.org/mailman/listinfo/nfsv4
> > 

_______________________________________________
nfsv4 mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/nfsv4

---

From: Spencer Shepler <[email-masked]>
Date: Thu, 12 Apr 2007 00:42:23 -0500

>> the client currently sees.

So you have interpreted the processing such that the sequence
number in the slot table is not incremented until the
COMPOUND processing is complete.  Therefore, the second
request is SEQ_MISORDERED by definition?

Spencer

---

From: "Iyer, Rahul" <[email-masked]>
Date: Wed, 11 Apr 2007 22:49:39 -0700

> From: Spencer Shepler [mailto:[email-masked]] 
> Sent: Wednesday, April 11, 2007 10:42 PM
> To: Iyer, Rahul
> Cc: [email-masked]
> Subject: Re: [nfsv4] Re: [pnfs] what error should be returned 
> toanimpatientclient?
> 
> 
> implementation 
> >> has a curious side effect of "allowing" the slot to be 
> used by 2 RPCs 
> >> simultaneously. In the current implementation, if the 
> aforementioned 
> >> scenario does occur, the sequenceid on the slot isn't 
> bumped up and 
> >> the client eventually sees NFSERR_SEQ_MISORDERED errors on 
> the next 
> >> request in that slot. So, despite the server "allowing" it, from a 
> >> pure client side view, NFSERR_SEQ_MISORDERED is what the client 
> >> currently sees.
> 

Not exactly... The sequence number is incremented on sucessful
processing of a SEQUENCE op. It should probably be on successful
compound processing though. And because of this, the next request is
SEQ_MISORDERED.

Regards
Rahul

---

From: Benny Halevy <[email-masked]>
Date: Thu, 12 Apr 2007 12:09:04 +0300

Iyer, Rahul wrote:
> compound processing though.

I disagree.  The slot sequence number must be incremented
even if the compound operation failed later.

> And because of this, the next request is
> SEQ_MISORDERED.
>

---

From: "Iyer, Rahul" <[email-masked]>
Date: Thu, 12 Apr 2007 02:50:25 -0700

> From: Benny Halevy [mailto:[email-masked]] 
> Sent: Thursday, April 12, 2007 2:09 AM
> To: Iyer, Rahul
> Cc: Spencer Shepler; [email-masked]

Oh yeah! Sorry, my bad. This screws up the normal case...
Regards
Rahul
 
> > And because of this, the next request is SEQ_MISORDERED.
> > 
>

---

From: Spencer Shepler <[email-masked]>
Date: Thu, 12 Apr 2007 09:01:29 -0500

> even if the compound operation failed later.

Except in the case that the SEQUENCE op itself returns an error.

Spencer