Re: [nfsv4] I-D Action: draft-talpey-rdma-commit-02.txt

[Chuck Lever III <[e-mail]>]

> On Jan 28, 2023, at 12:23 PM, Tom Talpey <[e-mail]> wrote:
> 
> On 1/26/2023 5:23 PM, Chuck Lever III wrote:
>> I read through this document to get a flavor. A couple of observations/comments follow:
> 
> Thanks for the early review!
> 
>> The Requirements Language section needs to be updated to include a reference to RFC 8174. For reference, the most recent version of the RL boilerplate looks like this:
> 
> Interesting, the id-nits tool does not flag this. Because this is still an individual draft, I didn't update any of the boilerplate from the earlier versions. Obviously this will be done if it becomes a WG document.
> 
>> Section 1 is lengthy and contains considerable repetition.
> 
> Yes, this informational section was lightly updated, but for this review it was left in place. We can certainly discuss what part(s) remain useful in a normative standards-track version, and either edit, move or delete.
> 
> One note however:
> 
>> 	• The definition of "Persistence" in Section 1.1 is confusing.
>> 		• Perhaps a short discussion of the terms "persistence" and "durability" (or a mention that they are synonyms) might be appropriate.
> 
> I intended to remove the no-longer-preferred term "durable", but oops I did neglect to delete the single reference to the term.
> 
> I also intended to remove "commit" where it was used to mean "flush", and there is at least one vestige. Its use in the filename is purely to maintain continuity and will be renamed if adopted.
> 
>> 	• Is there a purpose for "Global" in the definition of "Global Visibility" (as opposed to "Local Visibility")? A brief discussion of that distinction would help orient readers; or perhaps it is covered elsewhere in the document and I missed it.
> 
> Yes, "global" is an important qualifier and it's in the first sentence of the glossary. But it could be made clearer. In a nutshell, persistence is for storage semantics, while global visibility is for shared memory semantics. Upper layer processing is required to implement these over traditional RDMA.

I was also thinking that there seems to be some distinction between
when data becomes visible/durable when dealing with local persistent
memory versus the same for remote memory -- that might be missing from
the discussion here (and I'm not sure it's completely relevant to the
protocol itself, but it helps motivate it) -- or perhaps it is covered
in the Introduction in the area where I started skimming <bad Chuck,
no biscuit>.


>> Section 3 is marked "INCOMPLETE". What is the plan to complete this section? Is there other remaining significant work planned for this document?
> 
> It's intended to mirror the format of RFC5040 and RFC7306, but there are some new requirements that make the old table format difficult to reuse. I have an idea, but thought it was better to get the ball rolling now.
> 
> And, I don't plan other additions, apart from things that a WG discussion might generate.

Thanks, this is good to know.


>> Is there any implementation status to document via an "Implementation Status" section? I would like to see one... or at least an informal report on the mailing list about current implementation plans, as part of the planned IETF 116 discussion of this document.
> 
> There have been experimental implementations, which are not for me to cite in a standards-track document. There is also at least one non-IETF protocol which has implemented a similar set of functionalities, in fact whose semantics were based on earlier versions of this draft.

An "Implementation Status" section is often removed before
publication. I don't feel it's inappropriate as part of a WG
document. See RFC 7942 for a detailed discussion.


>> The third paragraph of "6. Security Considerations" touches on this, but what is the trust relationship between the peers in push mode? How do push mode peers authenticate to each other?
> 
> This is entirely the domain of the upper layer. It needs to determine whether the peer is trusted, prior to creating and advertising a push-mode memory handle for that peer. Perhaps this can be stated more forcefully.

Stating this would be very pertinent to Security Considerations,
I feel.


--
Chuck Lever