Last Call Review of draft-ietf-ippm-type-p-monitor-02
review-ietf-ippm-type-p-monitor-02-opsdir-lc-morton-2015-10-26-00

OPS-DIR, Authors, 

I have reviewed this document as part of the Operational directorate's 
ongoing effort to review all IETF documents being processed by the IESG.  
These comments were written primarily for the benefit of the operational 
area directors. Document editors and WG chairs should treat these comments 
just like any other last call comments.

Al

Title:
Differentiated Service Code Point and Explicit Congestion Notification
       Monitoring in Two-Way Active Measurement Protocol (TWAMP)
                   draft-ietf-ippm-type-p-monitor-02


https://tools.ietf.org/html/draft-ietf-ippm-type-p-monitor-02



Summary: Almost ready, comments and editorial suggestions follow.

The draft header should indicate that this draft updates RFC 5357.

Section 2.2.1

Figure 1 has a number of canonical references that should 
be cited to ensure IPv4 and IPv6 applicability, including 
RFC 2474, RFC 3168, and other relevant updates (of which 
there are many).

<later>
   o  if the negotiated/provisioned DSCP value is not known (e.g.  TWAMP
      Light), the choice of the DSCP is implementation specific.  For
      instance, Session-Reflector MAY copy the DSCP value from the
      received test packet and set it as DSCP in a reflected packet.
Question:
What about the ECN value?   From 3168:
      +-----+-----+
      | ECN FIELD |
      +-----+-----+
        ECT   CE         [Obsolete] RFC 2481 names for the ECN bits.
         0     0         Not-ECT
         0     1         ECT(1)
         1     0         ECT(0)
         1     1         CE
It makes little sense to copy a "11" == CE into the reflected packet,
and it may affect the measured performance if "11" were copied.
The draft needs more guidance here (there is a general problem
when the negotiated values are "not known"; this does not apply 
to the Standards Track TWAMP protocol or its extensions).

-=-=-=-=-=-=-=-=-=-=-=-=-

Editorial comments/suggestions:

OLD
2.2.  TWAMP-Test Extension

   Monitoring of DSCP and ECN requires support by the Session-Reflector
   and changes the format of its test packet format both in
   unauthenticated, authenticated and encrypted modes. 
Suggest:
   Monitoring of DSCP and ECN requires support by the Session-Reflector
   and changes the test packet format in all the original
   (unauthenticated, authenticated and encrypted) modes. 

2.2.1

OLD
   o  the first six bits of the Differentiated Service field MUST be
      copied from received Session-Sender test packet into Sender DSCP
      (S-DSCP) field;

   o  the following two bits of the ECN field MUST be copied from
      received Session-Sender test packet into Sender ECN (S-ECN) field.
Suggest:
   o  the six (least-significant) bits of the Differentiated Service field MUST be
      copied from received Session-Sender test packet into Sender DSCP
      (S-DSCP) field;

   o  the two bits of the ECN field MUST be copied from
      received Session-Sender test packet into Sender ECN (S-ECN) field.

because RFC 3168 defines:
         0     1     2     3     4     5     6     7
      +-----+-----+-----+-----+-----+-----+-----+-----+
      |          DS FIELD, DSCP           | ECN FIELD |
      +-----+-----+-----+-----+-----+-----+-----+-----+

        DSCP: differentiated services codepoint
        ECN:  Explicit Congestion Notification

      Figure 2: The Differentiated Services and ECN Fields in IP.
-=-=-=-=-=-=-=-

OLD
    0     1     2     3     4     5     6     7     8
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    |               S-DSCP              |   S-ECN   |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

                Figure 1: Sender DSCP and ECN field format

Suggest:
The extra "+" within bit positions are distracting,
and don't conform to the conventions of these diagrams.
Also, there's no "8" when numbering from 0.

Some attempt should be made to put Figure 3 on a single page in the 
final version. There's one line taken up by "+" that is unnecessary
in the modified field:

    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
    | Sender TTL  |  S-DSCP-ECN   |                               |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+                               +
    |                                                             |
 >> +                                                             + <<
    |                       MBZ (14 octets)                       |
    |                                                             |
    +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+