Subject: Re: [Last-Call] Genart last call review of draft-ietf-nfsv4-rpcrdma-cm-pvt-data-06

From: Chuck Lever <[email-masked]>
Date: Tue, 28 Jan 2020 10:50:54 -0500

Hi Suhas -

Thanks for your review and comments! My responses are below.

> On Jan 27, 2020, at 2:37 PM, Suhas Nandakumar via Datatracker <[email-masked]> wrote:
> 
> Reviewer: Suhas Nandakumar
> Review result: Ready with Nits
> 
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair.  Please treat these comments just
> like any other last call comments.
> 
> For more information, please see the FAQ at
> 
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
> 
> Document: draft-ietf-nfsv4-rpcrdma-cm-pvt-data-??
> Reviewer: Suhas Nandakumar
> Review Date: 2020-01-27
> IETF LC End Date: 2020-01-27
> IESG Telechat date: Not scheduled for a telechat
> 
> Summary: Thanks for the work. This document is clear in the problem to be
> solved . This document is ready to be published as it-is, however I do have few
> clarification questions for my own understanding
> 
> Major issues:
> 
> Minor issues:
> 
> Nits/editorial comments:
> 1. The draft doesn't specify normative procedures on sender/receiver behavior
> when certain fields are missing (say size of all zeroes). Should the draft say
> recommended procedures for handling these scenarios ?

Section 4 defines the format, which is fixed in size. Section 4.1.1 in
particular mandates the behavior when a perfectly-formed RPC-over-RDMA
private message is not received.

Zero is a permitted value for the size fields. Section 5.2 explains how
to compute the actual buffer size. If those fields contain zero, the
actual send and receive buffer sizes would be 1024 octets.

"Recommended procedures" are scattered about, but IMO the cases are
covered appropriately. If you see one that isn't, or one that could be
made more clear, please let me know.

> 2. Also i didn't see fallback procedures to be followed when the server
> reported size isn't of much use to the sender of the data . In such case the
> sender might decide to go with existing explicit RDMA data transfer operations
> instead of failing the connection ?

If I understand your question, you mean when an RPC message to be
transmitted is larger than the buffer sizes reported in the private
data. Section 3.5 of RFC 8166 explains how the RPC-over-RDMA protocol
handles that situation.

I see the confusion: Section 3.1 of the current document could be more
precise about the risks of exceeding the inline threshold size. The
second paragraph could instead read:

"If an incoming RDMA message exceeds the size of a receiver's inline
threshold, the receive operation fails, and the RDMA provider typically
terminates the connection.  To convey an RPC message larger than a
receiver's inline threshold without risking receive failure, the sender
must use explicit RDMA data transfer operations, which are more expensive
than RDMA Send."

--
Chuck Lever

---

From: Suhas Nandakumar <[email-masked]>
Date: Tue, 28 Jan 2020 21:03:55 -0800

Thanks Chuck for the response. Please see inline

> actual send and receive buffer sizes would be 1024 octets.
[Suhas] I am not sure if i am reading it right here. Section 5.2 would
result in the
value of -1 if the min of the values is Zero (0/1024 - 1). Isn't it so ?

> must use explicit RDMA data transfer operations, which are more expensive

[Suhas] This works great. Thanks for adding the clarification

---

From: Chuck Lever <[email-masked]>
Date: Wed, 29 Jan 2020 10:23:43 -0500

> On Jan 29, 2020, at 12:03 AM, Suhas Nandakumar <[email-masked]> wrote:
> 
> [Suhas] I am not sure if i am reading it right here. Section 5.2 would result in the

Section 5.2 says:

   Inline threshold sizes from 1KB to 256KB can be represented in the
   Send Size and Receive Size fields.  A sender computes the encoded
   value by dividing the actual value by 1024 and subtracting one from
   the result.  A receiver decodes this value by performing a
   complementary set of operations.

Here, "actual value" means the real size of the buffer. A 1024-octet
buffer would result in (1024 / 1024) - 1 = 0.

The computation done by the receiver is the inverse:

   (0 + 1) * 1024 = 1024

I could replace "actual value" by "buffer size, in octets". Would it
help if the text also spelled out the inverse computation?

--
Chuck Lever

---

From: Suhas Nandakumar <[email-masked]>
Date: Wed, 29 Jan 2020 07:35:52 -0800

Thanks Chuck for the additional clarification. That helped me to understand
better.
 Sorry that I missed it in my reading.

Yes the suggested changes look great.

Cheers
Suhas

---

From: Chuck Lever <[email-masked]>
Date: Wed, 29 Jan 2020 10:43:26 -0500

OK. The next submitted revision will include changes resulting from
the Gen-ART review. I'm waiting for resolution of SecDir comments
before submitting a fresh revision.

Meanwhile, the changes so far are available to view as a diff with -06:

https://chucklever.github.io/i-d-rpcrdma-cm-pvt-data/#go.draft-ietf-nfsv4-rpcrdma-cm-pvt-data.diff

> On Jan 29, 2020, at 10:35 AM, Suhas Nandakumar <[email-masked]> wrote:
> 
> Thanks Chuck for the additional clarification. That helped me to understand better.

--
Chuck Lever