Subject: [nfsv4] Review of draft-haynes-nfsv4-delstid-00

From: Chuck Lever <[email-masked]>
Date: Thu, 5 Jul 2018 17:15:16 -0400

Hi-

In the interest of preparing for our meeting at IETF 102, I've read
Tom's draft-haynes-nfsv4-delstid-00. Here are some brief thoughts
and review comments.

General comments:

I support all three ideas. I appreciate that the new protocol
elements have already been prototyped.

It appears to need further polish but I have no objection to allowing
this to move forward and eventually become a WG document.

Title:

I recommend "Extending the NFSv4.2 OPEN operation".

Abstract:

Consider shortening it to just the last sentence:

"This document presents several OPTIONAL refinements to the NFSv4.2
OPEN operation that improve the efficiency of managing open files."

Section 1:

I would like to see the Introduction include a problem statement and
short discussions of use cases for all three extensions. Some of
that language already appears at later points in the document, but
it is perhaps more in keeping with precedent to keep rationale here,
and leave protocol specification to subsequent sections.

It needs a more thorough discussion of how these extensions help.

Section 3:

Has there been any analysis of how well this feature will work on
multi-protocol storage targets that support the very similar Offline
attribute in the SMB protocol? In general these seem like the same
thing, but I wonder if there are subtleties that could bite us.

Section 4:

The use of RFC 2119 terms is sometimes strange in this section.

"it [the server] MUST be able to query the client via a CB_GETATTR"

I'm not sure what is required of the server here.

"it MUST accept those FATTR4_TIME_DELEG_ACCESS attribute and
FATTR4_TIME_DELEG_MODIFY attribute changes and derive the change
time or reject the changes with NFS4ERR_DELAY."

It's unusual (or perhaps not meaningful) to say "the server MUST
do this _or_ that." That makes the implementation requirement
unclear. Would this be better:

"it accepts those ... attribute changes and derives the change
time. If it cannot, it MUST respond with NFS4ERR_DELAY."

Do you expect cases where the server has to express a permanent
error (not just "wait and try again") ?

"If the time presented is in the future, the server can either clamp
the new time to the current time, or it may return NFS4ERR_DELAY to
the client, allowing it to retry."

This seems to allow other possible responses. Is that what you
want?

"A change in the access time MUST not advance the change time".
Should that be MUST NOT ?

Section 5:

The title of this section is "XDR Description of the Flexible File
Layout Type", perhaps a copy-and-paste oversight.

Recent RFCs that extend NFSv4.2 (such as RFC 8275) and include an
XDR definition do not appear to have a citation of "Legal Provisions
Relation to IETF Documents".

Do you need additional boilerplate in the derived .x file, for
example:

/// /*
///  * Copyright (c) 2018 IETF Trust and the person identified
///  * as author of the code.  All rights reserved.
///  *
///  * The author of the code is: T. Haynes
///  */

Appendix B:

This section appears to be unneeded.

--
Chuck Lever
[email-masked]