[storm] idnits on rdmap-ext draft

["Black, David" <[e-mail]>]

> We are not sure what to do with these at this point:
> 
> == Missing Reference: 'RFCXXXX' is mentioned on line 1266, but not
>      defined
>      'RFC Editor: Please replace XXXX in all instances of [RFCXXXX] abo...'
> == Missing Reference: 'RFC5226' is mentioned on line 1264, but not
>      defined
> 'Allocation Policy: Standards Action ([RFC5226])...'

Ignore the first nit - that's a typical nit in IANA Considerations when
defining a new registry.  When Tom or I prepare the writeup on this draft
for the IESG, that'll be noted as ok to ignore.

The second nit is indicating that RFC5226 needs to be added as a Normative
Reference - do that, please.

Thanks,
--David

> -----Original Message-----
> From: [e-mail] [mailto:[e-mail]] On Behalf Of
> Sharp, Robert O
> Sent: Wednesday, September 11, 2013 5:03 PM
> To: Tom Talpey; [e-mail]
> Subject: Re: [storm] Review comments on updated draft-ietf-storm-rdmap-ext-05
> 
> Hi Tom,
> 
> Again, thanks for the review and comments.  Responses below...
> 
> Thanks,
> Bob
> 
> > -----Original Message-----
> > From: [e-mail] [mailto:[e-mail]] On Behalf
> > Of Tom Talpey
> > Sent: Friday, September 06, 2013 2:07 PM
> > To: [e-mail]
> > Subject: [storm] Review comments on updated draft-ietf-storm-rdmap-ext-
> > 05
> >
> > 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.
> >
> 
> The Swap operation has been removed and a reference to OFA Verbs extensions
> for masked atomics has been added.
> 
> >
> > 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.
> >
> 
> Added reference to RFC 5042 and the ULP Buffer address reference to the 5042
> security model.
> 
> >
> > 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.
> >
> 
> The intention was to note that since this is a wire protocol, it is important
> to note that it is an implementation decision to provide greater scope of
> atomicity than just the RNIC.  Changed MAY NOT to may not.  We considered
> making this a positive MAY requirement, but that didn't seem like a good idea
> when we discussed it.
> 
> >
> > 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."
> >
> 
> This text is descriptive and intended to describe the expected usage, but
> indicate that the expected usage is not absolutely required.   For example,
> there is no error checking if immediate data is not proceeded by an RDMA
> Write.   RDMA Completion is defined in RFC5040.
> 
> >
> >
> > 5) Nits:
> >
> > Section 1.1 could use a paragraph break for improved readability.
> >
> 
>  Fixed.
> 
> >
> > 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.
> 
> Thanks.  Seems like we can just delete the definition then.  Do you agree?
> 
> >
> > 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.
> >
>  Fixed.
> 
> 
> > In section 4.1, the sentence beginning "This extension defines RDMAP use
> > Queue Number 3..." appears to be missing a word ("use of"?).
> >
> 
> Thanks!  Fixed
> 
> > 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.
> 
> Long lines have been fixed.
> 
> We are not sure what to do with these at this point:
> 
> == Missing Reference: 'RFCXXXX' is mentioned on line 1266, but not
>      defined
>      'RFC Editor: Please replace XXXX in all instances of [RFCXXXX] abo...'
> == Missing Reference: 'RFC5226' is mentioned on line 1264, but not
>      defined
> 'Allocation Policy: Standards Action ([RFC5226])...'> 
> >
> > _______________________________________________
> > storm mailing list
> > [e-mail]
> > https://www.ietf.org/mailman/listinfo/storm
> _______________________________________________
> storm mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/storm