Hi Martin, In reviewing the Gen-ART reviews of 3530bis, I realized I had not responded to your review. We had been very focused on S12.
I am sorry for the oversight and want to thank you for your review. I have answers in-line and have revised the draft with the necessary changes. Tom On 4/15/13, 5:41 PM, "Martin Thomson" <[email protected]> wrote: >I am the assigned Gen-ART reviewer for this draft. For background on >Gen-ART, please see the FAQ at > ><http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>. > >Please resolve these comments along with any other Last Call comments >you may receive. > >Document: draft-ietf-nfsv4-rfc3530bis-25 >Reviewer: Martin Thomson >Review Date: 2013-04 >IETF LC End Date: too soon >IESG Telechat date: ditto > >Summary: I have some confidence that this document is ready for >publication. What I have seen is sufficiently well specified that it >could be implemented, mostly. There are large parts of this document >that I simply haven't read. I can't justify spending more time. >(Sorry Jari, I tried.) > >Reviewing this document is especially difficult because the it relies >heavily on context. A lot of this context is only gained by reading on >and stumbling upon that context. Much of this could be addressed >with appropriate forward references. I agree with you here, the focus is so narrow that it is assumed that everyone knows all. > >It's clear that Section 12 is largely new, or at least significantly >changed from RFC 3530. Section 12 on its own would constitute a >larger than average RFC. Yes and we have whacked it again to reflect reality. Going forward, I envision we no longer write these large drafts and instead break down the sections which are minor version independent. Both S12 and the minor versioning rules themselves are candidate for such RFCs. The work on the minor versioning is underway. > >If accessibility is a goal, the document could have been split and it >probably should have been. However, I suspect that accessibility >isn't that important to those working on this. No, it is. But it wasn¹t the mandate put forth at the start of this document. The goal was to get the document in tune with working code without breaking the minor versioning rules. And one interpretation of that was to not do anything which would impact RFC 5661. Believe me, I never want to do such a re-edit of this large a document again. The way forward is to break it up opportunistically. > >The document talks quite frequently about "before", with reference to >NFSv2 or NFSv3. This is almost always useless to a reader without >context. I don't expect you to do anything about this, but I'd >request that any future revision consider removing this. For one, in >reviewing a -bis draft, I personally think of RFC3530 as the "before" >to which is refered. On the whole, fewer history lessons and more >real context would have made this a lot easier to read. > >General: The document uses lowercase "must" and "may" a lot. It's not >clear whether these are intended to be interpreted as normative >statements or not. To my eyes, most of these are. On the other hand, >there are places where 2119 language is used to effectively specify >implementation details that aren't observable at a protocol level. >If the document weren't so large, I'd suggest that the editors go through >and make sure that every 2119 keyword is in uppercase and then to >check each and every instance. > >Nits: > >1.5.3 - I realize that it might be premature to explain details when the >draft hasn't yet described how to identify locations (that's a truly >unfortunate choice of word). Maybe a short primer on that could be >included. At this point, I'm relying on assumed knowledge about >filesystems to understand concepts like: hierarchical ACLs, pseudo >filesystems, attributes. > >Even the fact that a filesystem is a tree with terminal nodes that >contain sequences of bytes. Non-terminal nodes (directories) and >terminal nodes (files) are named (hence the UTF-8) and have >attributes. Where it gets fuzzy is when it uses "path name". Is >"path name" an abstraction that encapsulates "directory name" and >"file name"? Is a "file name" the name of the node, or is it the >entire locator including the names of the directory above it (and some >unspecified separator)? Going back to your earlier comments about context, the assumptions here are that the reader is familiar with Unix filesystems. > >1.5.3.1 > [...] The server provides multiple > filesystems by gluing them together with pseudo filesystems. These > pseudo filesystems provide for potential gaps in the path names > between real filesystems. > >I don't know how to interpret this last statement. I'm using a fair >bit of guesswork to infer how this works as it is, but that >understanding does not result in gaps, just pseudo filesystems (with >no actual files). I¹ve added wording to clarify both this and the one below. > > >1.5.3.3 - The description of namespaces is singularly unhelpful in >conveying any information about the mechanism. The benefits of this >snake oil are clearly splendrous. Every time I read this, I lose sight of your point due to the beauty of that least sentence. I suspect that the reason it is vague is that it is an informational guidance and not a mandate. Each server vendor can (and will) implement it differently based on the OS and even the filesystem. > >s/alternate/alternative/ - I know that alternate is acceptable US >English, but alternate means something else to other English speakers. done > >1.5.5 - NLM could probably use a citation. Done > >3.1 - "The registered port 2049 [RFC3232] for the NFS protocol SHOULD >be the default configuration." This is an implementation requirement, >not a protocol one. We have presented other implementation requirements (e.g., Kerberos is mandatory to implement, not to deploy). > > - "this will allow NFS to transit firewalls" is only true if the >firewall is configured to recognize NTP using port numbers and allow >its passage. The requirement to use NTP is dependent on whether the site is utilizing Kerberos or not. If it is not, then time skew is not an important issue. The point here goes to that with NFSv3 there were necessary side band protocols (Mount, NLM, NSM) that required open ports. Some servers hard coded these and some picked their own to sit on. What this line is stating is that if a site wants to deploy NFS, they need only be concerned with one port for the protocol. If they add in Kerberos, then they will have to open up for communicating with the KDC, for NTP, etc. > > - "To date, all NFSv4 implementations are TCP based." Is this >really an appropriate statement for an RFC? Fixed. > > - "UDP by itself is not sufficient as a transport for NFSv4, >neither is UDP in combination with some other mechanism (e.g., DCCP >[RFC4340], NORM [RFC5740])." This statement is strange, given that >UDP is specifically prohibited. I understand that something layered >on UDP (SCTP perhaps) might be able to meet the requirements, but this >statement implies that neither congestion control (DCCP), nor >reliability (NORM) is sufficient. The draft should enumerate what >properties it expects to obtain from its transport before making such >statements. This section is really fuzzy on reliability expectations >(and 3.1.1 doesn't really help with that). There is no need to discuss UDP here, I put this more in line with what 5661 has in place. > >3.2 - "For NFSv4, the RPCSEC_GSS security flavor MUST be used to >enable the mandatory security mechanism." Which aspect of security is >mandatory? I find this confusing. Earlier statements were pretty >clear: you must implement it, but you don't have to use it. Changed to ³mandatory to implement². > >3.3 - s/may/can/ Done. > > - The SHOULD here (which is also a SHOULD in S17) could be >stronger. I can understand why you need to allow for deployments that >don't use TLS (or equivalent integrity protection), but this Incomplete thought? Let me try to tackle this anyway. It is a SHOULD because some sites still use NIS and that protocol may or may not be hardened for integrity protection. > >3.3.3 - "For Kerberos V5, the following form is RECOMMENDED: >nfs/hostname" - Q: how do you ensure interoperability if you don't >mandate a format? Comment: This doesn't provide information on why an >implementation might choose a different form. > >4.2 - The definitions of "persistent" and "volatile" are not known, Section 1.5.3.1? Provided a little more clarification on the ³semantic² we cared about for this discussion. >which makes this introduction to filehandle types hard to place in >context. In many respects the discussion on the history of the >protocol is unnecessary at this point. This would flow better if the >entire subsection was replaced with: "There are two[*] types of >filehandle: persistent and volatile. Servers select the type that >they provide clients and signal the choice using the mandatory >"fh_expire_type" attribute (see Section 4.2.3)." The rationale for >having both belongs in the respective sections (4.2.2 and 4.2.3). The first and second paragraphs lay down the foundation for the upcoming sections and also highlight the implementation burden on clients. > > * There aren't two types of filehandle, there are six different > persistence guarantees for filehandles. > >4.2.1 - "Clients MUST use filehandle comparisons only to improve >performance, not for correct behavior. All clients need to be >prepared for situations in which it cannot be determined whether two >filehandles denote the same object and in such cases, avoid making >invalid assumptions which might cause incorrect behavior." This seems >a little confused. The context establishes the fact that equal >filehandles always refer to the same file, and sometimes two different >filehandles can refer to the same file (though they SHOULDn't). So, >what value does the 'MUST' provide here? (I can't see how this would >be testable at the protocol level.) Given that there is more >discussion in 10.3.4, I'd recommend dropping these two sentences. The MUST clarifies the change in behavior from NFSv3 to NFSv4 that client implementors need to be aware of. And while I agree that it may not be testable at the protocol level, it might be testable at the client application layer. > >5.1 - Use of lowercase "must" and "may" are confusing. Consider using >alternatives. ³must² was fixed in a later edit to ³MUST² ³may² -> ³can" > >5.2 - is that lowercase "must" in the third sentence really a MUST? Yes > > - "A client MAY ask for the set of attributes the server supports >and SHOULD NOT request attributes the server does not support." How >does the client ask for the supported set? The assumption is that attributes are supported or not supported at a file system boundary. Thus, if a client queries the set of attributes via a GETATTR on one file, it will be aware of those attributes not supported in that file system. Thereafter, it would not ask for those attributes in accessing other files. > > - "A server should provide attributes whenever they don't have to >"tell lies" to the client." This statement is odd. Even with an >edit: "Rather than manufacturing fake attribute values, a server is >encouraged to avoid sending attributes that it can't support." This >seems closer implementation magic than protocol specification. Agreed, but this line is here in reference to file systems which have had to lie about an attribute in the past. > >12.1.2 - LATIN SMALL LETTER E is not U+0xxx > > - This section includes basically the same list twice. The >first instance is on Page 179 (the first two-item list), the second on >the top of 180. > >12.4.2 - What do you do with servers that are identified by server >names? The last paragraph mentions that IP addresses are just ASCII. >Are server names are subject to the IDNA rules? Do you want to >reference Section 12.6? > >12.7.3 - none of the last four points uses normative 2119 language, >though "must" and "should" appear. The text looks pretty normative to >me. > > - This looks less predictable than it could be. Is it >reasonable to require that a server use the same normalization every >time it returns a name? (That stability would only be required for a >given filehandle, of course.) Section 12 is a rewrite at this point. > _______________________________________________ Gen-art mailing list [email protected] https://www.ietf.org/mailman/listinfo/gen-art
