Subject: [Gen-art] Gen-ART LC review of draft-ietf-rddp-sctp-06

From: "Gray, Eric" <[email-masked]>
Date: Wed, 9 Aug 2006 15:29:06 -0400

I am the assigned Gen-ART reviewer for this document. For background

on Gen-ART, please see the FAQ at:

	http://www.alvestrand.no/ietf/gen/art/gen-art-FAQ.html

Please consider these comments along with any other last call comments you 
may receive.

Document: draft-ietf-rddp-sctp-06 ("Stream Control Transmission Protocol 
(SCTP) Direct Data Placement (DDP) Adaptation")

Authors: Caitlin Bestler, Randall R. Stewart
Shepherding AD: Lars Eggert

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+
 
	I have one minor question/issue and a few editorial NITs to point
out as part of this last call review.

Comments/Questions/Issues
=========================

Section 7.2 seems incomplete.  From the text, it appears that support for 
multiple IP addresses is allowed by this proposal.  However, for this to 
work, this would need to be something that both SCTP end-points are able 
to handle, or it should not be done by either.  How is this "capability" 
negotiated?

++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
+

NITs
----

One term is not used consistently in this document: DDP-SSN.  It is expanded
as (DDP) "Source Stream Sequence" (in its definition), as "Stream Sequence"
(in the defintion of "DDP Segment Chunk") and "Source Sequence Number" (in 
sections 5.2.1 and 6).  In all of these places, it probably should have been
(DDP) "Stream Sequence Number" as it is obviously analogous to "STCP Stream
Sequence Number" (which - AFAIK - is used consistently this way in the
draft)
based on the following excerpt:

"The DDP-SSN MUST have the same value that the SCTP Stream Sequence Number 
 (SSN) would have been assigned had ordered SCTP Payload Data Chunks been 
 used rather than unordered."

On that basis, I suggest changing all expansions of DDP-SSN to either omit
the expansion (relying on the fact that is is provided in the definitions 
section) or replace the expansion with "DDP Stream Sequence Number".

----------------------------------------------------------------------------
---

The following terms and/or acronyms should be added to section 2
(Definitions):

LLP - this is expanded as "lower layer protocol" (I believe it should be
"Lower
Level Protocol entity") in section 4 but occurs in earlier text unexpanded.

ULP - this is not expanded anywhere but I assume it is meant to be "Upper
Layer
(or Level) Protocol (entity)."  This term appears in the draft almost as
often
as the definite article "the".

MPA - This term is used in the expansion of "MULPDU" (section 5.2.2) but is
not
itself expanded anywhere in the draft.

MULPDU - this is expanded as "MPA Upper Layer PDU" in section 5.2.2 but is
used
in the definition of "DDP Segment" in section 2.

----------------------------------------------------------------------------
---

In the definition of "SCTP endpoint" - in section 2 - you use "a SCTP port"
in
one line and "an SCTP endpoint" in the next.  Shouldn't it be consistently
one
or the other ("a" or "an")?

----------------------------------------------------------------------------
---

In section 5.2.3, the second paragraph - first sentence - should read:

"ULP supplied Private Data MUST be included for DDP Stream Session Initiate,

 DDP Stream Session Accept and DDP Stream Session Reject messages."

(Comma inserted after "Initiate")

----------------------------------------------------------------------------
---

It would help if the 3rd paragraph in section 6.6 were reworded to use a bit
more precise terminology.  Currently, it says:

"This MAY be determined by SCTP acking of the Data Chunk used to carry the
 DDP Stream Session Initiate message, or by receipt of a responsive DDP 
 Stream Session Control message."

----------------------------------------------------------------------------
---

I assume that it should be "Stream Identifier" (rather than simply "Stream")
in the 4th paragraph of the same section (6.6).  Currently it says:

"A DDP Stream MUST NOT be re-used for another DDP Stream Session while any 
 Data Chunk from a prior session might be outstanding."

_______________________________________________
Gen-art mailing list
[email-masked]
https://www1.ietf.org/mailman/listinfo/gen-art

---

From: "Caitlin Bestler" <[email-masked]>
Date: Thu, 10 Aug 2006 08:49:38 -0700

Gray, Eric wrote:
> Please consider these comments along with any other last call
> comments you may receive. 
> 
> Authors: Caitlin Bestler, Randall R. Stewart Shepherding AD:
> Lars Eggert
> 
> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++ +
> 
> 	I have one minor question/issue and a few editorial
> NITs to point out as part of this last call review.
> 
> Section 7.2 seems incomplete.  From the text, it appears that
> support for multiple IP addresses is allowed by this
> proposal.  However, for this to work, this would need to be
> something that both SCTP end-points are able to handle, or it
> should not be done by either.  How is this "capability"
> negotiated?
> 

Support for multiple IP addresses is provided at the SCTP
layer. There is nothing unique about the use of multiple
IP addresses in the adaptation for RDMAP/DDP.

There are numerous strategies possible here. See the
most recent SCTP documents, and the ADDIP feature.

> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
> ++++++++++++++ +
> 
> NITs
> ----
> 
> One term is not used consistently in this document: DDP-SSN.
> It is expanded as (DDP) "Source Stream Sequence" (in its
> definition), as "Stream Sequence"
> (in the defintion of "DDP Segment Chunk") and "Source
> Sequence Number" (in sections 5.2.1 and 6).  In all of these
> places, it probably should have been
> (DDP) "Stream Sequence Number" as it is obviously analogous
> to "STCP Stream Sequence Number" (which - AFAIK - is used
> consistently this way in the draft)
>  (SSN) would have been assigned had ordered SCTP Payload Data
> Chunks been  used rather than unordered."
> 
> On that basis, I suggest changing all expansions of DDP-SSN
> to either omit the expansion (relying on the fact that is is
> provided in the definitions

The latter strikes me as better, but either would work and be
consistent with the intent.
 
> LLP - this is expanded as "lower layer protocol" (I believe
> it should be "Lower Level Protocol entity") in section 4 but
> occurs in earlier text unexpanded.
> 
> ULP - this is not expanded anywhere but I assume it is meant
> to be "Upper Layer (or Level) Protocol (entity)."  This term
> appears in the draft almost as often as the definite article "the".
> 
> MPA - This term is used in the expansion of "MULPDU" (section
> 5.2.2) but is not itself expanded anywhere in the draft.
> 
> MULPDU - this is expanded as "MPA Upper Layer PDU" in section
> 5.2.2 but is used in the definition of "DDP Segment" in section 2.
> 

Agreed. I'll clip some from the MPA or DDP document later today.

> In the definition of "SCTP endpoint" - in section 2 - you use "a SCTP
> port" in
> one line and "an SCTP endpoint" in the next.  Shouldn't it be
> consistently one or the other ("a" or "an")?
>

Yes, it should be. I was relying on spell checkers and ear, but
matching the SCTP RFC should be the definitive source.

agreed.
> It would help if the 3rd paragraph in section 6.6 were
> reworded to use a bit more precise terminology.  Currently, it says:
> 
> "This MAY be determined by SCTP acking of the Data Chunk used
> to carry the  DDP Stream Session Initiate message, or by
> receipt of a responsive DDP  Stream Session Control message."
> 

The intent is to state that implementations may mix and match
transport layer pacing with higher layer pacing, as long as
some method of ensuring order is preserved. 

Could you make a suggestion on what would have been clearer?

> I assume that it should be "Stream Identifier" (rather than
> simply "Stream") in the 4th paragraph of the same section (6.6). 
> Currently it says: 
> 
> "A DDP Stream MUST NOT be re-used for another DDP Stream
> Session while any  Data Chunk from a prior session might be
> outstanding."

I was actually thinking of the abstract resource rather
than the identifier when I wrote that. But changing it to
the Identifier would have the same effect.