Skip to main content

Early Review of draft-ietf-nfsv4-rpcrdma-version-two-06
review-ietf-nfsv4-rpcrdma-version-two-06-tsvart-early-iyengar-2022-03-13-00

Request Review of draft-ietf-nfsv4-rpcrdma-version-two-06
Requested revision 06 (document currently at 07)
Type Early Review
Team Transport Area Review Team (tsvart)
Deadline 2022-02-25
Requested 2022-01-03
Requested by David Noveck
Authors Chuck Lever , David Noveck
I-D last updated 2022-03-13
Completed reviews Tsvart Early review of -05 by Jana Iyengar (diff)
Tsvart Early review of -06 by Jana Iyengar (diff)
Comments
Considrable progress has been made, since the earlier review, based in large part on the reviewer's helpful comments.   We would like to be sure that the current draft deals with flow-conrrol issues adequately so that we can move on to other matters.
Assignment Reviewer Jana Iyengar
State Completed
Request Early review on draft-ietf-nfsv4-rpcrdma-version-two by Transport Area Review Team Assigned
Posted at https://mailarchive.ietf.org/arch/msg/tsv-art/min55EFcuAX4bxffqyhr50VPhXE
Reviewed revision 06 (document currently at 07)
Result On the Right Track
Completed 2022-03-13
review-ietf-nfsv4-rpcrdma-version-two-06-tsvart-early-iyengar-2022-03-13-00
Chuck, all,

Thanks much for the updates to the draft to reflect a working flow control
mechanism. I think the spirit is correct, but the description is unclear, my
comments below are an attempt to clarify the text. Otherwise, this is almost
there! Thanks and apologies for the delay in reviewing.

Comments:

1/ “A peer tracks a few critical values for each connection.” —> “A sender
tracks a few critical values for each connection.”

2/ For each of the variables described, replace “peer” with “sender” or
receiver” as appropriate. Flow control functions require actions at the send
side and at the receive side, and articulating which side is maintaining a
variable or performing an action is very important to not introduce ambiguity
and potential deadlocks.

3/ “Remote credits” —> I would suggest changing this to “Advertised Credits”,
to avoid confusion about which endpoint uses this.

4/ “The sender MUST NOT post this message if the sender's "Send message
counter" is greater than the current "Remote credits" value.” —> Two issues:
(i) What is the “send message counter”? Is this supposed to be “Sent message
count”? Or a new variable? (ii) each endpoint has its “Remote credits” value,
which one does this sentence refer to?

5/ “A receiver MAY adjust its credit limit” —> “A receiver MAY adjust its
advertised credit limit”

6/ “For instance, a peer may” —> “For instance, a receiver may”

- jana