Re: [nfsv4] AD review: draft-ietf-nfsv4-delstid-03

[David Noveck <[e-mail]>]

On Thu, Oct 5, 2023 at 6:43 AM Zaheduzzaman Sarker <
[e-mail]> wrote:

> Hi all,
>
> Now I have done my AD review of draft-ietf-nfsv4-delstid-03. I think this
> document needs some rewrites and clarifications before we can proceed
> further. See my comments below.
>
> Comments :
>
> # We need a section describing what are we updating in a nutshell. The way
> it is now written makes it is extremely difficult to understand what has
> been modified/amended/updated compared to RFC 8881.
>

Since this is an extension to NFSv4.2,, the comparison should be to
RFC7862/


> # Please clearly mention that this specification will update RFC8881 in
> the abstract.
>

It does not update really RFC8881, even though the header says it does.
Although the title says "NFSv4.2", it does not need to update RFC7862
either.  Please see RFC 8178  regarding this issue.


>
> # Why are we defining delegation and stateid again here? What is the
> change in the definition that requires the redefinition ? Can't we use what
> is defined in RFC8881?
>
> # Proxy definition : does this mean the client is proxying to itself? Can
> this proxy be somewhere in the network between the client and server.
> RFC8881 states server could also act as proxy hence this proxy definition
> needs to be clear about who can be proxy. I would also like to understand
> the difference between delegation and proxying.
>
> # I would strongly suggest to define/describe "offline files". I have hard
> time understanding of a file being offline from whos perspective? Will a
> file be considered offline if the client for some reason cannot access the
> cloud storage or is this only about archived files?
>
> # Please explain or describe the code blocks, otherwise they seems to come
> out of nowhere and does not fit themselves well in the sections.
>
> # I think we should gather all the new flags defined in one section or
> sub-section and describe them. This will also help to see what are we
> updating.
>
> # As we are defining new procedure and new flag, how are we handling the
> errors (we not have number of MUSTs, RECOMMENDs and SHOULDs)?
>
> # Section 3:
>     - It says -
>          "If the client gets both a open and a delegation stateid as part
> of the OPEN, then it has to return them both."
>     Return to where?
>     - Where is OPEN4_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION and
> OPEN4_RESULT_NO_OPEN_STATEID  delegation flags defined ?
>
> # Section 3.1:
>    - Put a reference to the "CLOSE operation".
>    - Where is OPEN_ARGS_SHARE_ACCESS_WANT_OPEN_XOR_DELEGATION flag
> defined?
>    - What is DELEGRETURN? yes, refer to RFC8881.
>
> # Section 4:
>    - I am not sure I understand the statement - "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."
>      I thought DELEGRETURN is a way to transfer or return those values.
> RFC 8881 says - "Delegations may be returned voluntarily (i.e., before
> the server has recalled them) or when recalled. In either case, the client
> must properly propagate state changed under the context of the delegation
> to the server before returning the delegation". Can we please explain it
> in a better way, like what values we are taking about and why they cant be
> transferred?.
>     - "These new attributes are invalid GETATTR, VERIFY, and NVERIFY" -
> there is something missing in this statement. "invalid to be used with" ?
>     - Please describe/explain/reference - "old times", "past the current
> time", and "original time". also put references for "access time", "change
> time", "modify time".
>     - Where is FATTR4_TIME_DELEG_ACCESS and FATTR4_TIME_DELEG_MODIFY
> defined?
>     - Give a ref to NFS4ERR_DELAY.
>
> # Section 4.1 seems to me a good motivation for this extension, may be we
> should uplift if to a separate motivation section.
>
> # Section 4.2: If we want someone to review this document, how would one
> read this section? please describe the code blocks, what they do or mean or
> related to the sections where these blocks are described.
>
> # May be section 5 is misplaced, may be we should have it before section 3
> and 4, so we define them and then show how the are used. the alternative
> would be to refer them from section 3 and section 4.
>
> # Section 5:
>     - It says - " client MUST determine that the additional new OPTIONAL functionality
> to OPEN is also not supported". what is the procedure for the client to
> determine that?
>
> # Section 6:
>     - It says - "While the XDR can be appended to that from [RFC7863
> <https://www.ietf.org/archive/id/draft-ietf-nfsv4-delstid-03.html#RFC7863>
> ], the various code snippets belong in their respective areas of the that
> XDR.". Does this mean this document also should update RFC7863?
>

I don't think it should do so.  That hasn't been the practice with other
extensions to NFSv4.2.


> # Section 7: in this specification we are extending some capabilities for
> client delegation. I think we need more convincing description that there
> is not security concerns here.
>
> //Zahed
>
>
>
>
>
>
>