Subject: Re: [nfsv4] Delaying RELEASE_LOCKOWNER and server-side seqid state

From: Rick Macklem <[email-masked]>
Date: Tue, 24 Sep 2013 18:14:31 -0400 (EDT)

David Noveck wrote:
> >> As far as ADMIN_REVOKED and EXPIRED, I'm wondering whether these
> >> are
> >> appropriate.  If there are locks existing, as far as the client is
> >> concerned  then LOCKS_HELD seems to me the more appropriate error.
> >> 
> > If the clientid of the lock_owner argument has an expired lease,
> > I'd think NFS4ERR_EXPIRED
> > would be appropriate. I can also see the clientid being
> > administratively revoked in a server.
> 
> OK, but ....
> 
> The spec only talks about EXPIRED and ADMIN_REVOKED being returned if
> the server sees a problem
> with stateids.
I'm not sure what spec you are referring to, but RFC3530 lists
NFS4ERR_EXPIRED as a valid error for Open and Renew (both take a
clientid and not a stateid).

It defines NFS4ERR_EXPIRED as:
  A lease has expired that is being used in the current operation.

If 3530bis has changed this, then I consider 3530bis broken. A lease
is per-clientid, not per-stateid.

The FreeBSD NFSv4.0 server expires all state related to a clientid
when the lease has expired. It simply discards all the state, including
the clientid and returns NFS4ERR_EXPIRED when there is no clientid record.
I believe this is correct behaviour. (It retains the clientid, marked
admin-revoked, for the rare case of administrative revocation.)

>  The only clientid-based error is STALE_CLIENTID.
>  Right now the spec only talks about that
> being returned if the server has rebooted and doesn't say anything
> about what happens vis-à-vis clientids
> in the event of Lease expiration or administrative revocation of a
> clientid.
> 
I believe this is correct. However, I would argue that it is implied
by the fact that leases are per-clientid and that NFS4ERR_EXPIRED is
listed as a valid error for Ops that take a clientid as argument.

rick
ps: I hope you don't mind that I added the cc for [email-masked].

> -----Original Message-----
> From: Rick Macklem [mailto:[email-masked]]
> Sent: Monday, September 23, 2013 8:29 PM
> To: Noveck, David
> Cc: nfsv4 list ([email-masked]); Saul Tamari
> Subject: Re: [nfsv4] Delaying RELEASE_LOCKOWNER and server-side seqid
> state
> 
> David Noveck wrote:
> > > Would it be possible to change the bis to allow adding
> > > NFS4ERR_DELAY
> > > to the current list of allowed errors for RELEASE_LOCKOWNER?
> > 
> > I think this is a reasonable change, although  I sense that some
> > people might have trouble with it.
> > 
> > I'm not clear the issue will be resolved in time for rfc3530bis.
> > 
> > > RFC3530 lists the following allowed replies for
> > > RELEASE_LOCKOWNER:
> > >     NFS4ERR_ADMIN_REVOKED
> > >      NFS4ERR_BADXDR
> > >     NFS4ERR_EXPIRED
> > >     NFS4ERR_LEASE_MOVED
> > >      NFS4ERR_LOCKS_HELD
> > >      NFS4ERR_RESOURCE
> > >      NFS4ERR_SERVERFAULT
> > >      NFS4ERR_STALE_CLIENTID
> > 
> > In the case of RESOURCE or SERVERFAULT, I don't think that a client
> > would have any basis to assume that the operation was done
> > successfully, as seems to be the case with the Linux client and
> > DELAY.
> > In fact, I can see a client deciding that, since DELAY is
> > disallowed,
> > any server which returns it is de facto broken and treating this as
> > a
> > SERVERFAULT equivalent.
> > 
> > Given the nature of RELEASE_LOCKOWNER and its use to prune state, I
> > can see a client just continuing, leaving one owner unreleased.,
> > when
> > a RELEASE_LOCKOWNER fails
> > 
> > As far as ADMIN_REVOKED and EXPIRED, I'm wondering whether these
> > are
> > appropriate.  If there are locks existing, as far as the client is
> > concerned  then LOCKS_HELD seems to me the more appropriate error.
> > 
> If the clientid of the lock_owner argument has an expired lease, I'd
> think NFS4ERR_EXPIRED would be appropriate. I can also see the
> clientid being administratively revoked in a server.
> 
> Although I don't have a strong opinion w.r.t. whether or not
> NFS4ERR_DELAY should be allowed, the FreeBSD client will accept the
> error and try the RPC again after a delay.
> 
> rick
> 
> > 
> > -----Original Message-----
> > From: [email-masked] [mailto:[email-masked]] On
> > Behalf
> > Of Saul Tamari
> > Sent: Monday, September 23, 2013 2:36 PM
> > To: nfsv4
> > Subject: [nfsv4] Delaying RELEASE_LOCKOWNER and server-side seqid
> > state
> > 
> > Hi,
> > 
> > Currently RFC3530 does not allow NFS4ERR_DELAY to be replied for
> > RELEASE_LOCKOWNER.
> > This seems to cause an issue if the server temporarily can't
> > complete
> > the operation.
> > 
> > I've noticed that if the server replies with NFS4ERR_DELAY to
> > RELEASE_LOCKOWNER then current Linux clients will ignore this error
> > and proceed, sending a lock request with new_lock_owner,
> > lock_seqid=0 and reuse the lock_owner that was recently (not)
> > released.
> > This causes a sequence id matching failure because the server did
> > not
> > release the old lock-owner and its associated seqid.
> > 
> > Would it be possible to change the bis to allow adding
> > NFS4ERR_DELAY
> > to the current list of allowed errors for RELEASE_LOCKOWNER?
> > 
> > 
> > RFC3530 lists the following allowed replies for RELEASE_LOCKOWNER:
> >       NFS4ERR_ADMIN_REVOKED
> >       NFS4ERR_BADXDR
> >       NFS4ERR_EXPIRED
> >       NFS4ERR_LEASE_MOVED
> >       NFS4ERR_LOCKS_HELD
> >       NFS4ERR_RESOURCE
> >       NFS4ERR_SERVERFAULT
> >       NFS4ERR_STALE_CLIENTID
> > 
> > 
> > Thanks,
> > Saul
> > _______________________________________________
> > nfsv4 mailing list
> > [email-masked]
> > https://www.ietf.org/mailman/listinfo/nfsv4
> > 
> > _______________________________________________
> > nfsv4 mailing list
> > [email-masked]
> > https://www.ietf.org/mailman/listinfo/nfsv4
> > 
> 
>

---

From: "Noveck, David" <[email-masked]>
Date: Fri, 27 Sep 2013 18:58:06 -0400

>> the server sees a problem with stateids.
> I'm not sure what spec you are referring to, but RFC3530 lists NFS4ERR_EXPIRED as a valid error for Open and Renew 
> (both take a clientid and not a stateid).

>   A lease has expired that is being used in the current operation.

I was confused about this.

> If 3530bis has changed this, then I consider 3530bis broken. A lease is per-clientid, not per-stateid.

The words have changed but not, I think, in a way you would consider broken.

> The FreeBSD NFSv4.0 server expires all state related to a clientid when the lease has expired. It simply 
> discards all the state, including the clientid and returns NFS4ERR_EXPIRED when there is no clientid record.

> I believe this is correct behaviour. 

That's valid behavior.    Courtesy locks are a  "may".

> (It retains the clientid, marked admin-revoked, for the rare case of administrative revocation.)

rfc3530bis talks about administratively getting rid of locks for particular stateids, assuming that locks 
might be revoked piecemeal.

The rfc3530bis error definition for ADMIN_REVOKED does not mention clientids but it should.

Oddly enough, the rfc3530 error definition for ADMIN_REVOKED mentions the lockowner and not clientid
or stateid.

-----Original Message-----
From: Rick Macklem [mailto:[email-masked]] 
Sent: Tuesday, September 24, 2013 6:15 PM
To: Noveck, David
Cc: [email-masked]
Subject: Re: [nfsv4] Delaying RELEASE_LOCKOWNER and server-side seqid state

David Noveck wrote:
> >> are appropriate.  If there are locks existing, as far as the client 
> >> is concerned  then LOCKS_HELD seems to me the more appropriate 
> >> error.
> >> 
> the server sees a problem with stateids.
I'm not sure what spec you are referring to, but RFC3530 lists NFS4ERR_EXPIRED as a valid error for Open and Renew (both take a clientid and not a stateid).

If 3530bis has changed this, then I consider 3530bis broken. A lease is per-clientid, not per-stateid.

The FreeBSD NFSv4.0 server expires all state related to a clientid when the lease has expired. It simply discards all the state, including the clientid and returns NFS4ERR_EXPIRED when there is no clientid record.
I believe this is correct behaviour. (It retains the clientid, marked admin-revoked, for the rare case of administrative revocation.)

>  Right now the spec only talks about that being returned if the server 
> has rebooted and doesn't say anything about what happens vis-à-vis 
> clientids in the event of Lease expiration or administrative 
> revocation of a clientid.
> 
I believe this is correct. However, I would argue that it is implied by the fact that leases are per-clientid and that NFS4ERR_EXPIRED is listed as a valid error for Ops that take a clientid as argument.

rick

> > > NFS4ERR_DELAY to the current list of allowed errors for 
> > > RELEASE_LOCKOWNER?
> > 
> > disallowed, any server which returns it is de facto broken and 
> > treating this as a SERVERFAULT equivalent.
> > 
> > when a RELEASE_LOCKOWNER fails
> > 
> > As far as ADMIN_REVOKED and EXPIRED, I'm wondering whether these are 
> > Behalf Of Saul Tamari
> > complete the operation.
> > not release the old lock-owner and its associated seqid.
> > 
> > Would it be possible to change the bis to allow adding NFS4ERR_DELAY