Re: [nfsv4] WGLC review of delstid-02

[Rick Macklem <[e-mail]>]

The new 3.1 section looks good to me and I think it
is a useful addition to the draft. Thanks for adding it.

I do not see any problems with the draft.

rick

On Thu, Jul 6, 2023 at 2:27 PM Thomas Haynes <[e-mail]> wrote:
>
>
>
> On Jun 3, 2023, at 8:55 AM, David Noveck <[e-mail]> wrote:
>
> General Comments
>
> Overall Evaluation
>
> In pretty good shape, with a set of easily addressed issues to resolve before submission.
>
> These issues are primarily editorial.  There are a few technical/substantive issues that appear, with details, within Per-section Comments.  In order to draw special attention to these, they are listed in Substantive Issues to Address.
>
> Substantive Issues to Address
>
> Addressing the following is necessary before submission for publication:
>
> The issues regarding when the SETATTR is valid mentioned in 4. Proxying of Times.
>
> Clarification of the intent of 4.1 Use Case.
> The issues mentioned in 5. Determining OPEN Feature Support.
>
> Per-section Comments
>
> Abstract
>
> The first sentence, while true, conveys nothing of value that belongs in an abstract while these observations might be appropriate in an introduction.
>
>
> It sets up the “this” which is found objectionable.  There has to be some context established.
>
> Keeping it as is.
>
>
> In the second sentence, the referent of "this" is quite unclear.  In addition, the sentence is misleading since all clients have the right to (and do) cache metadata.
>
>
> Focused the “this” on the delegation.
>
> The key here is the  word “locally”. But I’ve tried to clear the misleading by  marking the caching as authoritative.
>
>
>
>
> With regard to the third sentence, "refinements to RFC8881" is wrong since it presents extensions to RFC7862.
>
>
> Fixed
>
>
> 1. Introduction
>
> In the first sentence, suggest a comma after the introductory prepositional phrase.  In the second sentence, suggest replacing the "of" (after authority) by either "for" or "with regard to".
>
>
> taken
>
>
>   The third sentence needs rework because it is inconsistent with the bulletted list in that the first two bullets describe extensions to GETATTR.  Suggest as a possible replacement the following:
>
> This document presents a number of extensions which enhance the functionality of opens and delegations. These allow the client to:
>
> In the first bullet, don't see the relevance of the final clause.  Suggest replacing it by "which may require significant effort to obtain".
>
>
> taken
>
>
> Feel the second bullet is not clear as written.  Suggest the following as a possible replacement.
>
> Determine what extensions to the OPEN flags (See section 18.16 of [RFC8881]) are supported by the server.
>
>
>
> Taken
>
>
> 1.1 Definitions
>
> In the definition of "delegation", do not feel recallable should be hyphenated,
>
>
> Did look weird when I saw it
>
> My spell checker does not like the proposed change.
>
> Hmm online dictionaries are okay with this change.
>
>
> Feel the definition of "proxy" needs to be clearer.  Suggest the following as a potential replacement:
>
> Proxying of attributes occurs when a client has the authority to represent the attributes normally maintained by the server.  The client having this authority is the "proxy" for those attributes.
>
>
> In the definition of "stateid", suggests replacing "defines" by "designates".
>
>
> Taken
>
>
>
> 2. Offline File
>
> In the second sentence, don't like the construction "cost ... is expensive".  Suggest saying either that the action is expensive or the cost is high.
>
>
> Taken
>
> In the third sentence you need to mention the case of not accessing the file somehow, either by saying "only accessible" or adding a clause like "and not accessing it otherwise" to the end.
>
>
>
> Went with "and not accessing it otherwise"
>
>   Don't see the point of the final sentence of the first paragraph.  This is an observation about possible implementation approaches which has no value.
>
>
>
> The “value” is showing why someone would want to implement this. And it is not “possible”, it is factual. The Mac OSX client does just this.
>
> I’m keeping this as is.
>
>
> In the second paragraph, think that the material after the first two sentences is unhelpful. Don't understand what a "filehandle to the content" might be.  In any case you will have the filehandle of the file, if only to do a GETATTR.
>
>
> Again, this explains why someone might want to implement this feature.
>
>
> In the third paragraph:
>
> In the first sentence, Suggest replacing "operating system" by "client".
>
>
> Taken
>
>
> In the second sentence, suggest replacing "adding" by "interrogating".
>
>
>
> Taken
>
> Do not see the point of the third sentence at all.
>
>
> Again, this explains why someone might want to implement this feature.
>
>
>
> 3. OPEN Grant's...
>
> In the third sentence of the first paragraph, suggest replacing "is said to be 'open' to the client" by "can be considered open for the client".
>
>
> Taken
>
> I have problems with the last sentence of the paragraph (beyond the fact that it begins with "And")  😉.  I think that the fact that the client might do gratuitous GETATTRs is not worth mentioning.
>
>
> Replace the And…
>
>
> Avoiding gratuitous GETATTRs is a major accomplishment - it is easily one of the major complaints of users when they perceive performance.
>
>
>
> 4. Proxying of Times
>
> I think this section is fundamentally sound, but the first paragraph, which tries to explain the need for the feature, is quite garbled and needs major revision. Suggest something like the following:
>
> When a client is granted a write delegation on a file, it becomes the authority for the file contents and associated attributes.  If the server queries the client as to the state of the file via a CB_GETATTR (see Section 20.1 of [RFC8881]), then, according to the unextended NFSv4 protocol, it can only determine the size of the file and the change attribute. In the case of the client holding the delegation. It has the current values of any of oher of the access, modify, or change times,   but there is no way that other clients  to have access to these values.  While it is the authority for these values, it has no way to transfer these values to the server when the delegation is being returned and the server reclaims its authority with regard to these values. Similarly, there is no current provision to obtain these values before delegation return using CB_GETATTR.   As a result, it can not pass these times up to an application expecting POSIX compliance, as is often necessary for correct operation.
>
>
>
>
> The changes do not allow the caching of the change time. My guess is that the change attribute now supersedes that for client implementations.
>
> Trond?
>
> Yes, we state later:
>
> (The change time can be derived from the modify time.)
>
> Anyway, took the changes after some editing.
>
>
>
> With regard to the last sentence of second paragraph and the third paragraph they are inconsistent and need to be combined.  Feel it is best not to consider at all which compound the request is but to key everything off whether the client doing the SETATTR, at the time of operation, possesses a delegation for the file.
>
>
>
> I don’t see the inconsistency.
>
>
> Also, somewhere, it needs to be stated that these special new attributes are invalid in GETATTR, VERIFY, and NVERIFY and can only be used in CB_GETATTR and SETATTR by a  client holding a write delegation.
>
>
>
> Done
>
>
>
>
>
> In the last sentence of the fourth paragraph, suggest replacing "if" by "when", "this policy" by "the delay approach" and "will" by "would".
>
>
> Taken
>
>
>
> 4.1 Use Case
>
> I am totally confused by this and feel that some clarification is necessary.  First of all, the rest of the document deals with clients being proxies but this section speaks of a server being a proxy.
>
>
>
> Alright, I understand why this is confusing, rewrote.
>
>
> If this is somehow pNFS-related, then it needs to be explained clearly.  Also, with regard to the supposed extra GETATTR on the wire, I can't see it.  If you are a proxy, you have the delegation and the authority to respond without asking anyone else.
>
>
> In RFC8881, the proxy is only able to be authoritative with respect to size and change attributes. It MUST not proxy the atime and mtime. If the client asks for these attributes, the proxy would have to query the NFSv4.2 server.
>
>
> 5. Determining OPEN Feature Support
>
> The following issues are not addressed and need to be:
>
> The scope of this attribute needs to be clarified.  Suggest per-fs scope.
>
>
>
> Done
>
> Since this new option is not MANDATORY,  the document needs to address somehow the question of it not being supported.  Most likely, the client would have to assume that no OPEN extensions are present.
>
>
> Good point, not so sure of the conclusion.
>
> Uggh, yes, I am.
>
>
> Added a new paragraph to support this.
>
> A nit, the new attribute is RECOMMENDED and not “not MANDATORY”.
>
>
> Most of the flags concern ordinary non-OPTIONAL elements of OPEN.  How these are treated needs to be clarified.
>
> Added a new paragraph
>
> 7. Security Considerations
>
> Would prefer a less all-encompassing statement such as "The features described in this document do not introduce new security considerations..."
>
>
> Ack
>
> 8. IANA Considerations
>
> Don't understand this.  What "new entries" are being referred to?  This document does not require any IANA action.  What's wrong with just saying that?
>
>
> Probably cut and paste from an earlier document.
>
> Does this get marked as removeInRFC?
>
>
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4
>
>
> _______________________________________________
> nfsv4 mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/nfsv4