[storm] Review comments on updated draft-ietf-storm-rdmap-ext-05

[Tom Talpey <[e-mail]>]

Authors, thanks for the updates to the RDMA Extensions draft based on comments received.

I have some comments on the recent set of changes, which I would like to discuss before considering this version ready for Working Group Last Call.


1) Section 1.1 "Discovery of RDMAP Extensions" was added and addresses many of my earlier concerns. However, it says the following:

     " Today there are RDMA applications and/or ULPs that are aware of the  
       existence of Atomic and Immediate data operations for RDMA  
       transports such as [IB].  Today, these applications need to be aware  
       that iWARP RNICs do not support these operations."
and
     "Negotiation of Atomic Operations typically are to determine the  
       scope of atomicity guarantees, not down to the individual Atomic  
       Operations supported.  Therefore, this specification requires all  
       Atomic Operations defined within to be supported if an RNIC supports  
       any Atomic Operations."
Subsequently, section 5 now says:
     " The Atomic Operations specified in this document provide  
       equivalent functionality to the [IB] RDMA transport, to allow  
       applications that use these primitives to work interchangeably over  
       iWARP. In addition, this document specifies a basic Swap operation."

That last sentence extends Atomics beyond those of Infiniband, and this in turn contradicts the goal stated in section 1.1 that applications should not need to be aware of differences. This inconsistency needs to be addressed.


2) While the text has been updated to specify the target of Atomic operations as a registered ULP buffer, it does not discuss the fact that this is compatible with (indeed, required by) the RFC5042 security model. This should be made explicit, and the reference made explicit.


3) In section 5.4, an "Implementation note" uses the invalid term "MAY NOT". Perhaps this means to say the behavior is "OPTIONAL"? Or is it "MUST NOT"? As a general comment, when implementation notes contain normative language, they are not actually notes,. It may be safer to make specific requirements here, and also check the other notes.


4) In section 6, it is not clear what advice this statement is making. Is it intended to be normative, saying that Immediate Data operations always do this? If so, it should move to the processing text below. Also, the term "RDMA Completion" appears three times in this paragraph, but is not defined. Do you mean simply "Completion"?

     " An Immediate Data operation that is not preceded by an RDMA  
       Write operation causes an RDMA Completion."



5) Nits:

Section 1.1 could use a paragraph break for improved readability.


In the Glossary, ULP is defined as "Upper Level Protocol". The existing 504x RFCs, and I believe common usage, use "Upper Layer Protocol". Suggest using this definition.

Also in the Glossary, this statement should be moved from the definition of Atomics, to section 5:
    " Atomicity guarantees  
       across multiple RNICs or between other applications/ULPs running on  
       the Responder with access to the ULP Buffer are outside the scope of  
       this specification.

In section 4.1, the sentence beginning "This extension defines RDMAP use Queue Number 3..." appears to be missing a word ("use of"?).

RFC5042 is not referenced in the body, but it appears as a normative reference. It should be referenced, perhaps from section 5 in the discussion of the target of Atomic operations.

The document has a small number of id-nits which should be corrected.