[L2tpext] draft-ietf-l2tpext-tdm-04

[Ignacio Goyret <[e-mail]>]

Hi Sasha,
Here are some editorial details for the TDM over L2TPv3 draft.

* All "RECOMMENDED" should be changed to the standard term
  "SHOULD".

* All "NOT RECOMMENDED" should be changed to the standard
  term "SHOULD NOT".

* For the sake of clarity and readability, consider using
  the term "structure-aware" instead of "CESoPSN".

* Page 3, second paragraph:
  Old:
   [PWE3-CESoPSN] and [RFC4553] describe how to transfer the Attachment
   Circuit (AC) status via the data plane. This is therefore NOT
                                           ^^^^^^^^^^^^^^^^^^^^^
   RECOMMENDED to use the Set-Link-Info (SLI) message described in
   ^^^^^^^^^^^
   [RFC3931] for conveying this status with the PWs in question.
                                       ^^^^

  New:
   [PWE3-CESoPSN] and [RFC4553] describe how to transfer the Attachment
   Circuit (AC) status via the data plane. Therefore,
   the Set-Link-Info (SLI) message described in [RFC3931]
   SHOULD NOT be used for conveying this status for the PWs in question.

* Page 3, fourth paragraph:
  Old:
   The next sections describe the extensions to the L2TP for
                                                ^^^
   establishment and validation of TDM pseudowire sessions.

  New:
   The next sections describe the extensions to L2TP for
   establishment and validation of TDM pseudowire sessions.

* Page 3, fifth paragraph:
    s/2/two/
    s/One AVP describe/One AVP describes/
    s/The second AVP describe/The second AVP describes/

    s/Session Connection Messages/Session Management messages/

* The CAS and SP fields on the TDM PW AVP: should there be
  some space for growth, just in case something new comes
  along the way? How about making both of them 3 bits wide?

* There should be an indication of the values for CAS and SP
  for the structure-agnostic case (only structure-aware seems
  to be defined).

* Page 4:
      b) In addition to that, for trunk-specific NxDS0 with CAS,
         (Payload Bytes/number of DS0 channels) MUST be an integer
         factor of the number of frames per corresponding trunk
         multiframe.

  Can we rewrite this condition so it is easier to read?
  Sorry, I can't really suggest an alternative here as I'm not
  working on TDM PWs.

* CAS bits definition:
 OLD:
   The CAS bits define the trunk type for trunk-specific CESoPSN
   services with CAS. These bits:
  1) MUST be set to 0 for all pseudowire types excluding trunk-specific
     CESoPSN with CAS
  2) For trunk-specific CESoPSN with CAS these bits bust be set to:
     a) '01' in the case of an E1 trunk
     b) '10' in the case of a T1/ESF trunk
     c) '11' in the case of a T1/SF trunk.

  NEW:
   The CAS bits define the trunk type for trunk-specific CESoPSN
   services with CAS. These bits MUST be set as follows:
    '01' - For E1 trunk-specific CESoPSN with CAS
    '10' - For T1/ESF trunk-specific CESoPSN with CAS
    '11' - For T1/SF trunk-specific CESoPSN with CAS
    '00' - For all other pseudowire types.

* Page 5:
  OLD:
   PT is the payload type expected in the RTP header.  Value of zero
                                                      ^^
   indicates that the payload type is ignored and will not be used to
                                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
   detect malformed packets.

  NEW:
   PT is the payload type expected in the RTP header.  A value of zero
   indicates that the payload type will not be checked to
   detect malformed packets.

* Top of page 6:
  OLD:
   misconnections. Since L2TP provides an alternative security mechanism
   via the cookies, if the cookie length is larger then zero the SSRC
   ^^^^^^^                                         ^^^^^^^^^^^
   SHOULD be zero.

  NEW:
   misconnections. Since L2TP provides an alternative security mechanism
   using cookies, if the cookie length is larger than zero then the SSRC
   SHOULD be zero.

* Section 2.3:
  OLD:
   Control Connection that support TDM MUST add the appropriate PW Type
   value to the list in the Pseudowire Capabilities List AVP. The exact
   value is TBA by IANA and is listed in the next section.

  NEW:
   Control Connections that support TDM MUST add the appropriate PW Type
   value(s) to the list in the Pseudowire Capabilities List AVP. The valid
   values are listed in the next section.


* The IANA section does not list the new PW types. Also, I suggest moving
  the assignment suggestions from section 2.4 to this section.

* In the IANA section, there should be a note that any new bits or values
  should be defined at least with Expert Review.

BTW, thanks for taking the editor job for this draft!
-Ignacio

_______________________________________________
L2tpext mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/l2tpext