AD review of draft-ietf-rddp-sctp-03

         There are a number of comments I made on -02 that have not been
         reflected in -03 and for which I have seen no explanation from
         the authors as to whether my comments was wrong. (I am totally
         willing to be convinced that I make no sense, but not by  
silence
         on a comment.) I am hence including the more important once  
again
         below. I expect the authors to also check the comments to  
-02 to
         confirm they have addressed those that I did not include  
below as
         well.

         (The paragraph numbers may be off in the comments below, I'm  
beta-
         testing a tools script for Henrik.)


INTRODUCTION, paragraph 1:

 > Remote Direct Data Placement                                  R.  
Stewart
 > Working Group                                        Cisco  
Systems, Inc.
 > Internet-Draft                                                C.  
Bestler
 > Expires: December 2, 2006                           Broadcom  
Corporation
 >                                                             J.  
Pinkerton
 >                                                                 
Microsoft
 >                                                               S.  
Ganguly
 >                                                                
Consultant
 >                                                                   
H. Shah
 >                                                     Broadcom  
Corporation
 >                                                               V.  
Kashyap
 >                                                                       
IBM
 >                                                             May  
31, 2006

         Six authors?


INTRODUCTION, paragraph 2:

Stream Control Transmission Protocol (SCTP) Remote Direct Memory Access
 >              (RDMA) Direct Data Placement (DDP) Adaptation
 >                       draft-ietf-rddp-sctp-03.txt

         To what degree does this document still talk about RDMA? Should
         the title maybe change?


Section 1., paragraph 0:

1.  Definitions

         Nit: Would move this after the introduction, as usual.


Section 1., paragraph 2:

 >    DDP Source Stream Sequence (DDP-SSN) - A stream specific sequence
 >       number assigned by the Adaptation Layer for each SCTP Data  
Chunk
 >       sent.  This is the order that chunks were submitted to SCTP, no
 >       matter what order they are actually sent or received in.
 >       The smallest unit of data transfer for the DDP protocol.  It
 >       includes a DDP Header and ULP Payload (if present).  A DDP  
Segment
 >       should be sized to fit within the Lower Layer Protocol MULPDU.
 >       An SCTP Payload Data Chunk that encapsulates the DDP Stream
 >       Sequence (DDP-SSN) and a DDP Segment.

         Does the text starting with "The smallest unit of data  
transfer..."
         really belong under this definition? Editing mistake? (Same  
for the
         text starting with "An SCTP Payload Data Chunk...")


Section 1., paragraph 4:

 >    DDP Stream Session Control Message - DDP Stream Session Control
 >       messages are used to control the association of the DDP  
Endpoint
 >       with the DDP Stream.
 >       A common local interface convention to control which  
Steering Tags
 >       (STags) are valid with which DDP Endpoints.  Under this  
convention
 >       both the Steering Tag and DDP Endpoint are created within the
 >       context of a Protection Domain, and the Steering Tag may  
only be
 >       enabled for DDP Endpoints created under the same Protection
 >       Domain.

         Does the text starting with "A common local interface..."  
really
         belong to this definition? Editing mistake?


Section 2., paragraph 1:

 >    This document describes a method to adapt Direct Data Placement  
[DDP]
 >    [I-D.ietf-rddp-ddp] to Stream Control Transmission Protocol (SCTP)
 >    [RFC2960].

         References are messed up, e.g., [DDP] should probably be  
removed
         and just [I-D.ietf-rddp-ddp] be kept. Check for all references
         throughout the document.


Section 3., paragraph 1:

 >    The keywords MUST, MUST NOT, REQUIRED, SHALL, SHALL NOT, SHOULD,
 >    SHOULD NOT, RECOMMENDED, NOT RECOMMENDED, MAY, and OPTIONAL, when
 >    they appear in this document, are to be interpreted as  
described in
 >    [RFC2119].

         Nit: This is not the variant of the RFC2119-blurb that  
idnits likes.


Section 5.1., paragraph 2:

 >    It is presumed that the handling of incoming data chunks for DDP
 >    enabled associations is sufficiently different than for routine  
SCTP
 >    associations that it is undesirable to require support for  
mixing DDP
 >    and non-DDP streams in a single association.  More than a single
 >    association is required if an application desires to utilize  
both DDP
 >    and non-DDP traffic with the same remote host.

         Should this be an RFC2119 "MUST" or "SHOULD" instead
         of "is undesirable to require support for?"


Section 5.2.1., paragraph 1:

 >    All SCTP Payload Data Chunks used by this Adaptation layer  
include a
 >    DDP Source Sequence Number (DDP-SSN). the DDP-SSN tracks the  
sequence
 >    the messages were submitted to the SCTP layer for the SCTP  
stream in
 >    use.  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.

         Nit: s/(DDP-SSN). the DDP-SSN/(DDP-SSN). The DDP-SSN/


Section 0, paragraph 1:

 >    The following function code values are defined for DDP in
 >    this document:

         Not included in the IANA considerations section.


Section 0, paragraph 3:

 >    ULP supplied Private Data MUST be included for DDP Stream Session
 >    Initiate DDP Stream Session Accept and DDP Stream Session Reject
 >    messages.  However, the ULP supplied Private DATA MAY be of zero
 >    length.

         Nit: s/DATA/Data/
         Nit: s/included for/included in/


Section 0, paragraph 5:

 >    Private Data MUST NOT be included for the DDP Stream Session
 >    Terminate message.

         Nit:    Nit: s/included for/included in/


Section 6., paragraph 1:

 >    A DDP Endpoint is the logical sender/receiver of DDP Segments.   
A DDP
 >    Stream connects two DDP Endpoints using a matched pair of SCTP
 >    Streams.

         How are they "matched?"


Section 6., paragraph 2:

 >    A DDP Stream Session defines the sequence of Data Chunks exchanged
 >    between two DDP Endpoints over a DDP Stream that has a distinct
 >    beginning and end.  Data Chunks from one DDP Stream Session are  
never
 >    carried over to the next session.

         What indicates the "distinct beginning and end?"
         What does "never carried over" mean? Under what  
circumstances could
         this "carrying over" occur?


Section 6., paragraph 3:

 >    The local interface MAY associate a DDP Endpoint with the DDP  
Stream
 >    based upon the initial exchanges of a DDP Session, and  
terminate that
 >    association at the session's end.

         Is that an RFC2119 "MAY", or maybe a MUST or SHOULD?


Section 6., paragraph 4:

 >    A DDP Stream is associated with at most one Protection Domain  
during
 >    a single DDP Stream Session.

         So it can be associated with no Protection Domain?
         When is that useful?


Section 6.1., paragraph 2:

 >    The Payload Data Chunks for a given session, when sequenced by  
their
 >    DDP-SSN, MUST follow one of the patterns defined in this section.

         The sequence patterns in the remainder of this section don't
         include any DDP-SSNs? How can compliance thus be established?


Section 6.2., paragraph 2:

 >       Active Side sends a DDP Stream Session Initiate message.
 >       Passive Side sends a DDP Stream Session Accept message.
 >       Each side may then send zero or more DDP Segments with  
increasing
 >       DDP-SSNs, subject to various layers of flow control.

         What does "various layers of flow control" mean?
         What happens when the DDP-SSN wraps? It's no longer  
increasing then.


Section 8., paragraph 2:

 >    The adaptation layer should request the maximum number of SCTP  
stream
 >    it will wish to use over the lifetime of the association.  SCTP
 >    streams must still be bound to DDP Endpoints, and a DDP or DDP 
+RDMA
 >    enabled SCTP association does not support ordered Data Chunks.
 >    Therefore the mere existence of an SCTP stream is unlikely to  
require
 >    significant supporting resources.

         This section still refers to "DDP+RDMA", which has been removed
         from the rest of the document.


Section 9., paragraph 2:

 >    As a Lower Layer Protocol (LLP) for DDP, the SCTP adaptation layer
 >    MUST inform the DDP layer of the maximum DDP Segment size that  
will
 >    be supported.  This should be the largest value that can be  
supported
 >    without use of IP or SCTP fragmentation, or 516 bytes,  
whichever is
 >    larger.

         Why a lower bound of 516 bytes?


Section 9., paragraph 5:

 >    The SCTP adaptation layer SHOULD set the maximum DDP Segment size
 >    below the theoretical maximum in order to allow bundling of  
Control
 >    Chunks in the same SCTP packet.

         How much lower? How large are control chunks?


Section 10., paragraph 5:

 >    The receiver MAY perform a validity check on received DDP-SSNs to
 >    ensure that any gap could be accounted for by unreceived Data  
Chunks.
 >    Implementations are advised against allocating resources on the
 >    assumption that DDP-SSNs are valid without first performing such a
 >    validity check.  An invalid DDP-SSN MAY result in termination  
of the
 >    DDP Stream.

         "are advised against" or "SHOULD NOT?"


Section 12., paragraph 0:

12.  IANA considerations

         Does not satisfy RFC2434. Need to define each new namespace,
         register values used here, and define how future allocations  
are
         to be handled. Also see comment on function codes above.


Section 12., paragraph 1:

 >    This document defines one new Adaptation Layer Indication  
codepoints
 >    with the recommended value of:

         Namespace defined in draft-ietf-tsvwg-addip-sctp, point there.


Section 13., paragraph 1:

 >    Any direct placement of memory could pose a significant  
security risk
 >    if adequate local controls are not provided.  These threats  
should be
 >    addressed in the appropriate DDP [DDP-Draft] [I-D.ietf-rddp-ddp],
 >    RDMA [RDMA-Draft] [I-D.ietf-rddp-rdmap] or Security [RDMA- 
Security]
 >    [I-D.ietf-rddp-security] drafts.  This document does not add any
 >    additional security risks over those found in [RFC2960].

         "Should be addressed" - _are_ they addressed there?


Section 15., paragraph 0:

15.  References

         Needs to split references in normative and informative.

-- 
Lars Eggert                                     NEC Network Laboratories

_______________________________________________
rddp mailing list
[email protected]
https://www1.ietf.org/mailman/listinfo/rddp