Re: [nfsv4] Tsvart early review of draft-ietf-nfsv4-rpcrdma-version-two-05

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

Hi Jana, thanks for your expert comments. A few responses
are inline below.


> On Nov 2, 2021, at 5:21 AM, Jana Iyengar via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Jana Iyengar
> Review result: Not Ready
> 
> I've read up through Section 4, specifically to understand and comment on the
> Flow Control aspects of this draft, as requested by Chuck Lever.
> 
> As I understand it, FC works as follows:
> 
> - A receiver advertises the number of unacknowledged messages it is willing to
>  receive (`credit window`), and also indicates the number of messages received so far
>  (`received`). The underlying transport is assumed to provide ordered and
>  reliable delivery.
> 
> - A sender is required to not send more than receiver-advertized `received +
>  credit window` messages.
> 
> - These advertisements are typically piggy-backed on payload-bearing
>  messages. Optionally, a receiver can send a message with a `Grants Credit`
>  header type to advertise its window.
> 
> - The receiver can increase or decrease the advertised window.
> 
> - Each message has a max message size (`Inline Threshold`) which is indicated
>  via `Transport Properties`, and defaults to 4096 bytes.
> 
> 
> I have a few comments specific to Flow Control:
> 
> - The document allows for advertised window to increase or decrease, meaning
>  that a receiver can renege on credit it advertises. Specifically, this allows
>  a receiver to say it will accept a particular message and then reduces its
>  window to cause that message to be now out of its window.

It was not my intention for a credit decrease to force messages
out of the window. I hope this oversight can be addressed by
simply adding a sentence or two restricting the Receiver's
behavior.


>  This choice is rife with pain. I would urge, in the strongest manner possible,
>  to avoid allowing reneging. That is, do not allow a receiver to arbitrarily
>  reduce advertised credit so as to allow reneging. If you really really need
>  it, prepare for some blood and sweat, and eventually, inevitable tears.
> 
>  To clarify, I do not mean that the window size must remain the same. A
>  receiver can reduce its window size, but do it so that if a message was
>  in-window before, it remains in-window after.
> 
>  For example, here is a sequence of FC messages that reduces the window and
>  reneges on a previously allowed message (this is bad):
> 
>  received = 10, window = 5  (allowed = 11-15)
>  received = 11, window = 2  (allowed = 12-13, reneged on 14, 15)
> 
>  And here is a sequence of FC messages that reduces the window without
>  reneging (this is good):
> 
>  received = 10, window = 5  (allowed = 11-15)
>  received = 11, window = 4  (allowed = 12-15)
>  received = 12, window = 3  (allowed = 13-15)
> 
> - The current mechanism has a receiver advertising both `received` and `credit
>  window` and the sender computes the limit. It is more direct for a receiver to
>  simply advertise the limit -- basically the maximum message that the sender
>  can send. This makes for fewer fields in the exchange, is more intuitive, and
>  importantly, it is easier to describe a mechanism that avoids reneging
>  (advertise the total message limit, and never advertise a lower number).

A single field is preferable to retain better compatibility
with RPC/RDMA v1. I wasn't able to think of a way to combine
the fields. We can discuss this offline.


> - Are the expected limits at a receiver in bytes or in operations? I imagine
>  that the answer is bytes.

The second paragraph of Section 4.2.1 explains that the
connection's credit limits are expressed in messages, not
bytes.

   An RPC-over-RDMA version 2 credit represents the capability to convey
   exactly one RPC-over-RDMA version 2 message, regardless of its size,
   via an RDMA Send/Receive pair.


>  If so, the FC mechanisms should be tied to this
>  resource. At the moment, a receiver has to figure out what to advertise based
>  on available memory and the maximum message size it advertised. Note that
>  there will be inefficiency because messages are likely to be smaller than the
>  max limit. It is likely to be more direct and efficient if the unit is all
>  bytes.

True, messages are typically smaller than the connection's
inline thresholds. Unfortunately message bytes are not
fungible among RDMA Receive buffers.

A Receiver with 32 4096-byte Receive buffers will use exactly
the same resources to receive 32 400-byte messages as it will
to receive 32 3500-byte messages. Sending small RDMA messages
does not enable a Receiver to then receive additional messages.

At one point the document text was clear that the inline
threshold is actually the RDMA Receive buffer size, but that
over time has been removed in favor of generality.


Regarding efficiency: We had considered building a capability
in RPC/RDMA of conveying multiple short RPC messages in a
single RDMA Send/Receive, but holding up those RPC messages to
coalesce them usually adds unwanted latency.


> - When are transport properties exchanged? FC credit needs to be available at
>  the beginning of a connection for any progress to be made at all, so either
>  both endpoints need to advertise it right at the start, or some default needs
>  to be assumed. I don't see either in the document.

The second paragraph of Section 4.3 explains that both peers
must send one-at-a-time until credit limits are established.


> - Finally, `credit window` is a strange term. What the document currently uses
>  is arguably a window, and the term `window` here applies to messages that can
>  be received, not to flow control credit.
> 
> General comments (I skimmed the rest of the document):
> 
> - I understand the value of the XDR descriptions for the protocol, but I haven't
>  encountered this in the past. Is this adequate for an IETF RFC? This is not a
>  request to change anything, so perhaps all I'm looking for is that the ADs are
>  ok with this.

Fwiw, XDR describes RPC protocols throughout the RFCs pub-
lished by the nfsv4 WG, starting with RFC 2623. Previous to
the creation of the nfsv4 WG, XDR was originally defined in
RFC 1014 and used to describe NFSv2 in RFC 1094 in 1989.


> - It would be useful to have forward references from the earlier sections to the
>  later wire format and detailed error sections. For example, when a connection
>  might be closed due to FC violations.
> 
> - I might recommend a little text in the intro section laying out the structure
>  of the document.
> 
> - I see lowercase and uppercase MUSTs and SHOULDs. For clarity, consider
>  rewriting the text to avoid the lowercase keywords.

Issues regarding readability, terminology, and document
structure will be addressed in subsequent revisions. Thanks
for pointing them out!


--
Chuck Lever