Subject: [nfsv4] NFSv4.1 errata id 3226 (the return of return-on-close layouts)

From: Trond Myklebust <[email-masked]>
Date: Sun, 9 Feb 2014 15:37:08 -0500

Errata 3226 (see: http://www.rfc-editor.org/errata_search.php?rfc=5661&eid=3226 ) fixes an issue with return on close, but it changes the generic word ‘close’ to the operation CLOSE.

Note that the existing text does allow the client to use a byte range lock or delegation stateid as a substitute for an open stateid in the LAYOUTGET operation. In the case of a lock stateid, we should normally have a supporting open stateid, and so expecting a CLOSE is valid. However, for the case of where the client only holds a delegation stateid there is no guarantee that it will ever send anything other than a DELEGRETURN. 

Proposal: we should re-wordsmith the text in errata 3226 to talk in terms of “open state”, rather than talking explicitly about the CLOSE operation.

The question I have then is whether or not we want OPEN_DOWNGRADE to revoke any return-on-close layouts that are no longer relevant for the remaining share access mode?
IOW: If I hold a LAYOUTIOMODE4_RW layout and I do an OPEN_DOWNGRADE to OPEN4_SHARE_ACCESS_READ, should I lose that layout?
Similarly, if I hold both a LAYOUTIOMODE4_RO layout, and I OPEN_DOWNGRADE to OPEN4_SHARE_ACCESS_WRITE, should I lose that layout?

My own opinion is that we probably do not need to revoke the layout in these situations, but perhaps others disagree?

Cheers
  Trond

_________________________________
Trond Myklebust
Linux NFS client maintainer, PrimaryData
[email-masked]

---

From: Trond Myklebust <[email-masked]>
Date: Sun, 9 Feb 2014 16:24:35 -0500

> My own opinion is that we probably do not need to revoke the layout in these situations, but perhaps others disagree?

Proposal for replacement for errata 3226:

Section 18.43.3 says:

The logr_return_on_close result field is a directive to return the layout 
before closing the file.  When the metadata server sets this  return value
to TRUE, it MUST be prepared to recall the layout in the case in which the
client fails to return the layout before close.  For the metadata server 
that knows a layout must be returned before a close of the file, this 
return value can be used to communicate the desired behavior to the client
and thus remove one extra step from the client's and metadata server's 
interaction.

It should say:

The logr_return_on_close result field is a directive to return or forget 
the layout before closing the file. Once a LAYOUTGET operation returns with
logr_return_on_close set to TRUE for a given file, then all subsequent
LAYOUTGET requests by that client for the same file and layout type,
MUST reply with logr_return_on_close set to TRUE until that file is closed.

After the client has closed all open stateids and returned the delegation stateids
for that file, the server MUST invalidate all layout segments of type loga_layout_type,
that were issued with logr_return_on_close set to TRUE.

The client MUST NOT attempt to use that layout or the layout stateid once it has
closed the file.

---

From: Benny Halevy <[email-masked]>
Date: Mon, 10 Feb 2014 10:19:01 +0200

>> Proposal: we should re-wordsmith the text in errata 3226 to talk in terms of “open state”, rather than talking explicitly about the CLOSE operation.

Agreed.

>> My own opinion is that we probably do not need to revoke the layout in these situations, but perhaps others disagree?

Layouts are NOT a replacement for OPEN, ACCESS and LOCK (see RFC5661 12.5.1)
and the fact that they CAN be held across OPENs and CLOSEs make the layout state essentially
independent of the open state - and it's respective access semantics.

Also, the layout state is global to the client and can be used for different open owners,
having potentially different share access permissions.  It is the responsibility of the client
implementation to enforce the fine grained security semantics upon its open owners, similar
to cached I/O with or without holding a delegation.

However, unlike in-band I/O, the MDS cannot always control access to storage when the client
reads or writes directly from/to the DS.
In contrast to the file layout that uses a back-end mechanism to control clients' access to the DS
other layout types like the objects layout encode a token into the layout that is used by the DS
to enforce the access restriction policy set by the server.  When the client OPEN_DOWNGRADEs
a file and loses say its WRITE permissions (and this needs to be checked across all the client
open owners) the server MAY want to recall the client RW layouts as part of its security
enforcement method, and revoke them in case the client does not properly return them.
Implicitly returning them would save this step.

On the other hand this corner case seem to me layout-type specific enough that we can just
recommend that the client SHOULD explicitly return the respective layout in this case -
based on layout-type specific logic.

> The client MUST NOT attempt to use that layout or the layout stateid once it has

Ack, though as you mentioned the act of "closing the file" is still a bit confusing.
How about defining this state as the client "returning its open state"?
To take a stab at it:

	The logr_return_on_close result field is a directive to return or forget 
	the layout before retuning the open/delegation state for the file.

	Once a LAYOUTGET operation returns with logr_return_on_close set to TRUE for a given file,
	then all subsequent LAYOUTGET requests by that client for the same file and layout type,
	MUST reply with logr_return_on_close set to TRUE until the client returns its open state
	for that file via CLOSE and DELEGRETURN.

	The client MUST NOT attempt to use that layout or the layout stateid once it has
	returned the file's open state.

Now, there is another inconsistency in this erratum with regards to which layout segment
return_on_close applies.

In this sentence we say return_on_close affects only the segments that were issued with it:
	the server MUST invalidate all layout segments of type loga_layout_type,
	that were issued with logr_return_on_close set to TRUE.

and the next sentence we tell the client that it actually affected the whole layout:
	The client MUST NOT attempt to use that layout or the layout stateid once it has
	returned the file's open state.

I suggest we make up our minds and simplify both the server and the client's logic
by applying return_on_close to the whole layout.  If at all, I'd consider applying it to
all the layout segments with the same iomode as the one return with return_on_close
with the use case in mind of a server that wants all RW layouts to be return on close
while allowing read-only layouts to be kept by the client. Yet again, I think simple
is the way to go.  So this should be phrased as follows:

	The logr_return_on_close result field is a directive to return or forget 
	the layout before retuning the open/delegation state for the file.

	Once a LAYOUTGET operation returns with logr_return_on_close set to TRUE for a given file,
	then all subsequent LAYOUTGET requests by that client for the same file and layout type,
	MUST reply with logr_return_on_close set to TRUE until the client returns its open state
	for that file via CLOSE and DELEGRETURN.  Note that return_on_close also applies
	retroactively to all layout segments retrieved by the client for that file and
	layout type.
 
	After the client has closed all open stateids and returned the delegation stateids
	for that file, the server MUST invalidate all layout segments of type loga_layout_type,
	that were issued to the client for that file and the client MUST NOT attempt to use that
	layout or the layout stateid.

Benny

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

---

From: Benny Halevy <[email-masked]>
Date: Tue, 11 Feb 2014 11:10:18 +0200

>>> Proposal: we should re-wordsmith the text in errata 3226 to talk in terms of “open state”, rather than talking explicitly about the CLOSE operation.

Trond, coming to think of it, the whole concept of an "open state" that is initiated by OPEN
and extends past CLOSE via delegations may require more semantic clarifications around the
interaction of OPEN and outstanding delegations that outlived their respective OPEN.
For example, should an OPEN with share deny bits conflicting with such an outstanding
delegation be rejected with NFS4ERR_SHARE_DENIED? This case is not covered in section 9.7.

Benny

---

From: Trond Myklebust <[email-masked]>
Date: Tue, 11 Feb 2014 11:40:25 -0500

> delegation be rejected with NFS4ERR_SHARE_DENIED? This case is not covered in section 9.7.

That is really a separate issue, but normally I’d expect such an OPEN to trigger a delegation recall in order to resolve the conflict.

---

From: Benny Halevy <[email-masked]>
Date: Wed, 12 Feb 2014 12:18:29 +0200

On Tue, Feb 11, 2014 at 6:40 PM, Trond Myklebust <[email-masked]> wrote:
>>>>> Errata 3226 (see: http://www.rfc-editor.org/errata_search.php?rfc=5661&eid=3226 ) fixes an issue with return on close, but it changes the generic word 'close' to the operation CLOSE.
>>>>>
>>>>> Proposal: we should re-wordsmith the text in errata 3226 to talk in terms of "open state", rather than talking explicitly about the CLOSE operation.
>>
> That is really a separate issue,

Agreed.

> but normally I'd expect such an OPEN to trigger a delegation recall in order to resolve the conflict.

Me too, but I'm not sure it's specified anywhere.

An odd case could be OPEN for exclusive read access with

OPEN4_SHARE_ACCESS_READ
and
OPEN4_SHARE_DENY_READ or OPEN4_SHARE_DENY_BOTH

while there is an outstanding read delegation.
OPEN4_SHARE_ACCESS_READ does not conflict with the delegation
but the SHARE_DENY flags do conflict with the open state related to
the "hanging" delegation.

Benny

---

From: Trond Myklebust <[email-masked]>
Date: Tue, 18 Feb 2014 11:01:54 -0500

> 	layout or the layout stateid.

Sigh.. As usual, there is more…

1) We also want to deal with the case where the open/delegation state is revoked by the server. In that case, it really should revoke the return-on-close layout, and fence off the client.

2) If the client is sending a CLOSE/DELEGRETURN in parallel with other OPEN requests, it may need a way to test for whether or not the layout was returned. Since TEST_STATEID cannot return NFS4ERR_WRONG_CRED, I suggest that clients could use TEST_STATEID in the same COMPOUND, following the CLOSE/DELEGRETURN.

Note that the NFS4ERR_WRONG_CRED thingy (i.e. state protection) prevents the client from using an explicit LAYOUTRETURN in the same COMPOUND as a CLOSE/DELEGRETURN.

----------------

Section 18.43.3 says:

It should say:

The logr_return_on_close result field is a directive to return or forget
the layout when the client returns all open/delegation state for that file.

Once a LAYOUTGET operation returns with logr_return_on_close set to TRUE for a given file,
then all subsequent LAYOUTGET requests by that client for the same file and layout type,
MUST reply with logr_return_on_close set to TRUE until the client returns all its open state
for that file using CLOSE and DELEGRETURN.  Note that return_on_close also applies
retroactively to all layout segments retrieved by the client for that file and
layout type.

After the client has closed all open stateids and returned the delegation stateids
for a file for which logr_return_on_close was set to TRUE, the server MUST 
invalidate all layout segments that were issued to the client for that file.
The client MUST NOT attempt to use that layout or the layout stateid.

If the server needs to revoke all open stateids and delegation stateids owned by the client
for a file for which logr_return_on_close was set to TRUE, then it MUST also revoke all
layout segments of  type loga_layout_type that were issued for that file to that client, and
take action to fence the access to the DSes.

Cheers
  Trond

---

From: Benny Halevy <[email-masked]>
Date: Sun, 23 Feb 2014 18:53:11 +0200

> Note that the NFS4ERR_WRONG_CRED thingy (i.e. state protection) prevents the client from using an explicit LAYOUTRETURN in the same COMPOUND as a CLOSE/DELEGRETURN.

Ack.  Looks good.

Benny