Subject: [storm] idnits on rdmap-ext draft

From: "Black, David" <[email-masked]>
Date: Sat, 14 Sep 2013 01:13:48 -0400

> 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: [email-masked] [mailto:[email-masked]] On Behalf Of
> Sharp, Robert O
> Sent: Wednesday, September 11, 2013 5:03 PM
> To: Tom Talpey; [email-masked]
> 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: [email-masked] [mailto:[email-masked]] On Behalf
> > Of Tom Talpey
> > Sent: Friday, September 06, 2013 2:07 PM
> > To: [email-masked]
> > 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
> > [email-masked]
> > https://www.ietf.org/mailman/listinfo/storm
> _______________________________________________
> storm mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/storm

---

From: "Sharp, Robert O" <[email-masked]>
Date: Mon, 16 Sep 2013 20:12:48 +0000

Hi David,

We've added the reference go RFC5226.  I don't know why I was thinking that this section would be moved to another RFC that held the registry values, but we've fixed that now.

Thanks,
Bob

> From: Black, David [mailto:[email-masked]]
> Sent: Saturday, September 14, 2013 12:14 AM
> To: Sharp, Robert O; [email-masked]
> Subject: idnits on rdmap-ext draft
> 
> 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.
> 
> > Of Sharp, Robert O
> > > From: [email-masked] [mailto:[email-masked]] On
> > > Behalf Of Tom Talpey
> > > 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:
> > >
> > > 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.
> >
> > >
> > > 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"?
> > >
> > > 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
> > > 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
> >
> > > reference. It should be referenced, perhaps from section 5 in the
> > > discussion of the target of Atomic operations.
> > >