Subject: Re: [nfsv4] Comments on draft-ietf-nfsv4-rpcrdma-bidirection-02.txt

From: Chuck Lever <[email-masked]>
Date: Mon, 2 May 2016 11:46:41 -0400

> On Apr 12, 2016, at 11:33 AM, Chuck Lever <[email-masked]> wrote:
> 
>> 
>> On Apr 11, 2016, at 5:34 PM, Tom Talpey <[email-masked]> wrote:
>> 
>> I reviewed the update to the RPC/RDMA Bidirection draft and have the following comments,
>> and I do envision making more, but these are my responses for now.
>> 
>> 
>> 1) Wording nit:
>> 
>> The words "the repertoire of" and "For various reasons, " in the following paragraphs
>> seem to make implications which are not actually stated in this document. I suggest
>> simply deleting them here.
>> 
>>  1.2.2.  NFSv4.1 Callback Operation
>> 
>>  NFSv4.1 supports file delegation in a similar fashion to NFSv4.0, and
>>  extends the repertoire of callbacks to manage pNFS layouts, as
>>  discussed in Chapter 12 of [RFC5661].
>> 
>>  For various reasons, NFSv4.1 requires that all transport connections
>>  be initiated by NFSv4.1 clients.  Therefore, NFSv4.1 servers send
>>  callbacks to clients in the backward direction on connections
>>  established by NFSv4.1 clients.
>> 
>> 
>> 2) Clarity question:
>> 
>> What does "should interoperate correctly" mean in the first paragraph below? There
>> is no discussion why this is expected, or whether a specific requirement is being made.
>> 
>>  1.3.1.  Backward Compatibility
>> 
>>  Existing clients that implement RPC-over-RDMA Version One should
>>  interoperate correctly with servers that implement RPC-over-RDMA with
>>  backward direction support, and vice versa.
>> 
>> 
>> 3) Similar question:
>> 
>> Is the following "should" a requirement, an observation, or something else?
>> 
>>  1.3.2.  Performance Impact
>> 
>>  Support for operation in the backward direction should never impact
>>  the performance or scalability of forward direction operation, where
>>  the bulk of ONC RPC transport activity typically occurs.
>> 
>> 
>> 4) Typo "Absence":
>> 
>> 2.2.3.  In the Absense of Backward Direction Support
>> 
>> 
>> 5) Meaning of "legacy" in same section 2.2.3:
>> 
>> Versions prior to NFSv4.0? Because this section does not mention anything but 4.1,
>> it's a bit unclear what this reference is trying to say.
>> 
>>  Legacy versions of NFS never send backchannel operations.
>> 
>> 6) I disagree with this section 2.5 text as a requirement:
>> 
>> This was discussed on the call, and while there are a number of good reasons why
>> an implementation needs to be careful in the use of chunks in the backward
>> direction, there is no actual protocol-specific reason to deny it. Receivers are
>> free to ignore any chunks, and can readily detect and skip them (although I do
>> agree there may be performance and other implications).
>> 
>> Section 1.3.4 discusses a number of these implications, and makes no such
>> requirement. Therefore I believe the sentence quoted below should be removed.
>> 
>>  2.5.  Sending A Backward Direction Call
>> 
>>  ...
>> 
>>  The rdma_proc field in the RPC-over-RDMA header MUST contain the
>>  value RDMA_MSG.  All three chunk lists MUST be empty.
> 
> Tom, thanks for your review.
> 
> I can likely adjust the text in straightforward ways
> to address all your comments and questions above,
> except for the last point. To address the last point,
> I propose the following major changes to the document:
> 
> 1. Remove "Size-Limited" from the title.
> 
> 2. Allow chunks in the backward direction by
> adjusting text throughout the document, possibly
> removing discussion about message size constraints.
> 
> 3. Stipulate that if a server sends a chunk in a
> backward direction call to a client implementation
> that does not support backward direction chunks,
> the client MUST reply RDMA_ERROR / ERR_CHUNK.
> 
> 4. Because there are no current ULPs that require
> the use of backward direction chunks, introduce a
> section of guidance for (client) implementations
> that choose not to support backward direction
> RDMA Read/Write. It would recommend the mechanism
> implemented in current prototypes:
> 
> a. BD chunks need not be supported if there are
> no implementations of ULPs that have DDP-eligible
> data items in the backward direction.
> 
> b. Each supported ULP that may employ backward
> direction operation must have a mechanism to
> advertise the maximum size of backward direction
> messages, and must set that maximum to the size
> of the largest RPC message that can be sent within
> the inline threshold.
> 
> As long as the message size maximum is observed,
> there would be no need for Position Zero Read
> chunks or Reply chunks. Thus servers would never
> include chunk lists in backward direction
> RPC-over-RDMA messages, and clients would never
> encounter them.
> 
> In this way, the specification will not prohibit
> backward direction chunks, and existing
> implementations can continue to be used. The size
> limitation is then a characteristic of specific
> implementations, and not of the protocol itself.

Since I've had no more review comments on
draft-nfsv4-rpcrdma-bidirection-02, I've posted
a new revision with the above changes. This is a
substantial reworking of the document.

Because of the extent of these changes, perhaps
the WGLC for -02 should be terminated, and a fresh
WGLC started for -03. Spencer, does that make sense
to you?

--
Chuck Lever

---

From: Spencer Shepler <[email-masked]>
Date: Mon, 2 May 2016 16:19:27 +0000

-----Original Message-----
From: Chuck Lever [mailto:[email-masked]] 
Sent: Monday, May 2, 2016 8:47 AM
To: Spencer Shepler <[email-masked]>
Cc: [email-masked]
Subject: Re: [nfsv4] Comments on draft-ietf-nfsv4-rpcrdma-bidirection-02.txt

>> I reviewed the update to the RPC/RDMA Bidirection draft and have the 
>> following comments, and I do envision making more, but these are my responses for now.
>> 
>> 
>> 1) Wording nit:
>> 
>> The words "the repertoire of" and "For various reasons, " in the 
>> following paragraphs seem to make implications which are not actually 
>> stated in this document. I suggest simply deleting them here.
>> 
>>  NFSv4.1 supports file delegation in a similar fashion to NFSv4.0, 
>> and  extends the repertoire of callbacks to manage pNFS layouts, as  
>> What does "should interoperate correctly" mean in the first paragraph 
>> below? There is no discussion why this is expected, or whether a specific requirement is being made.
>> 
>> Versions prior to NFSv4.0? Because this section does not mention 
>> anything but 4.1, it's a bit unclear what this reference is trying to say.
>> 
>> This was discussed on the call, and while there are a number of good 
>> reasons why an implementation needs to be careful in the use of 
>> chunks in the backward direction, there is no actual 
>> protocol-specific reason to deny it. Receivers are free to ignore any 
>> chunks, and can readily detect and skip them (although I do agree there may be performance and other implications).
>> 
>> Section 1.3.4 discusses a number of these implications, and makes no 
>> such requirement. Therefore I believe the sentence quoted below should be removed.
>> 
> I can likely adjust the text in straightforward ways to address all 
> your comments and questions above, except for the last point. To 
> address the last point, I propose the following major changes to the 
> document:
> 
> 2. Allow chunks in the backward direction by adjusting text throughout 
> the document, possibly removing discussion about message size 
> constraints.
> 
> 3. Stipulate that if a server sends a chunk in a backward direction 
> call to a client implementation that does not support backward 
> direction chunks, the client MUST reply RDMA_ERROR / ERR_CHUNK.
> 
> 4. Because there are no current ULPs that require the use of backward 
> direction chunks, introduce a section of guidance for (client) 
> implementations that choose not to support backward direction RDMA 
> Read/Write. It would recommend the mechanism implemented in current 
> prototypes:
> 
> a. BD chunks need not be supported if there are no implementations of 
> ULPs that have DDP-eligible data items in the backward direction.
> 
> b. Each supported ULP that may employ backward direction operation 
> must have a mechanism to advertise the maximum size of backward 
> direction messages, and must set that maximum to the size of the 
> largest RPC message that can be sent within the inline threshold.
> 
> As long as the message size maximum is observed, there would be no 
> need for Position Zero Read chunks or Reply chunks. Thus servers would 
> never include chunk lists in backward direction RPC-over-RDMA 
> messages, and clients would never encounter them.
> 
> In this way, the specification will not prohibit backward direction 
> chunks, and existing implementations can continue to be used. The size 
> limitation is then a characteristic of specific implementations, and 
> not of the protocol itself.

Since I've had no more review comments on draft-nfsv4-rpcrdma-bidirection-02, I've posted a new revision with the above changes. This is a substantial reworking of the document.

Because of the extent of these changes, perhaps the WGLC for -02 should be terminated, and a fresh WGLC started for -03. Spencer, does that make sense to you?
[Spencer] Yes, this is fine with me.  I will send out a new notice.

--
Chuck Lever