Last Call Review of draft-ietf-ippm-stamp-07
review-ietf-ippm-stamp-07-genart-lc-worley-2019-09-03-01

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document:  draft-ietf-ippm-stamp-07
Reviewer:  Dale R. Worley
Review Date:  2019-09-03
IETF LC End Date:  2019-09-03
IESG Telechat date:  2019-09-19

Summary:

       This draft is on the right track but has open issues, described
       in the review.

Minor technical issue:

4.4.  Interoperability with TWAMP Light

The consequences of using a TWAMP Session-Sender with a STAMP
Session-Reflector aren't adequately described.  I suspect that the WG
understands the technical solution, but the text does not make that
clear.

   If the Server Octets field is present in the TWAMP
   Session-Sender packet, STAMP Session-Reflector will not copy the
   content starting from the Server Octets field but will transmit the
   reflected packet of equal size.

The final phrase appears to not be true.  In both unauthenticated and
authenticated mode, the size of the reflected packet is fixed, and so
in this case, it will not have equal size with the incoming packet.
Also, there is no requirement that a STAMP Session-Reflector even
accept incoming packets that are not of the standard length.

It seems that there is an unwritten requirement that a STAMP
Session-Reflector accept longer packets than are expected for its
configured operation mode and simply ignore the trailing excess
octets.

There are also some interactions with how the HMAC is verified for
over-length incoming authenticated packets -- how does a STAMP
Session-Reflector authenticate an incoming packet that is longer than
the STAMP format?

Editorial issues: 

1.  Introduction

   and inherit separation of control (vendor-specific configuration or
   orchestration) and test functions.

Is "inherit" intended to be "inherent"?

   One of such is Performance
   Measurement from IP Edge to Customer Equipment using TWAMP Light from
   Broadband Forum [BBF.TR-390] used as the reference TWAMP Light that,
   according to [RFC8545], includes sub-set of TWAMP-Test functions in
   combination with other applications that provide, for example,
   control and security.

This sentence is awkward because of the length of the title in the
middle of the sentence.  Perhaps

   One of such is [BBF.TR-390] (Performance Measurement from IP Edge
   to Customer Equipment using TWAMP Light from the Broadband Forum),
   a the reference TWAMP Light, that includes a sub-set of TWAMP-Test
   functions along with control and security functions.

--

   This document defines an active performance measurement test protocol

This sentence doesn't explicitly say that STAMP is connected to TWAMP,
despite that the preceding sentence does explicitly say that
BBF.TR-390 is connected to TWAMP.  Perhaps, "This document defines
another such mechanism ..."  Perhaps also precede with a paragraph
break for clarity.

2.1.  Terminology

   AES Advanced Encryption Standard
   CBC Cipher Block Chaining
   ECB Electronic Cookbook
   KEK Key-encryption Key

These four terms are not used in the document.

3.  Softwarization of Performance Measurement

   The configuration and management of the STAMP Session-
   Sender, Session-Reflector, and management of the STAMP sessions can
   be achieved through various means.

You should probably revise to add "... sessions are outside the scope
of this document and can be ...".

   Command Line Interface, OSS/BSS
   (operations support system/business support system as a combination
   of two systems used to support a range of telecommunication services)
   using SNMP or controllers in Software-Defined Networking using
   Netconf/YANG are but a few examples.

This sentence is long and awkward in a number of ways.  One version
that I think is clearer is:

    A few examples are:  Command Line Interface, telecommunication
    services' OSS/BSS systems, SNMP, and Netconf/YANG-based SDN
    controllers.

4.  Theory of Operation

About half of the text in section 4 is about port assignments, but
that isn't really part of the "Theory of Operation".  I think the
exposition would be easier to read if the text about port assignments
was extracted and put into a separate subsection (probably placed
between 4 and 4.1).

4.1.1.  Session-Sender Packet Format in Unauthenticated Mode

   o  Sequence Number is four octets long field.  For each new session
      its value starts at zero and is incremented with each transmitted
      packet.

There is no definition of what a STAMP session is in this document.
The idea is more or less obvious, but it would be useful to call out
its primary properties in section 4 "Theory of Operation".  It seems
like the primary properties are that a session is between one
Session-Sender and one Session-Reflector and a session contains a
potentially unlimited number of packets sent between the two.

   o  Timestamp is eight octets long field.  STAMP node MUST support
      Network Time Protocol (NTP) version 4 64-bit timestamp format
      [RFC5905], the format used in [RFC5357].  STAMP node MAY support
      IEEE 1588v2 Precision Time Protocol truncated 64-bit timestamp
      format [IEEE.1588.2008], the format used in [RFC8186].

The specification of how the time format for a particular packet is
chosen is weak.  I think you want to add to the above paragraph that
the choice of format is determined either by configuration or the Z
bit in the Error Estimate field.

4.1.2.  Session-Sender Packet Format in Authenticated Mode

   Also, MBZ fields are used to align the packet on 16 octets
   boundary.

You can't align the packet itself using a field within the packet.
You want to say "are used to align the fields within the packet on 16
octets boundaries."  Or perhaps "to make the packet length a multiple
of 16 octets."  Similarly in 4.2.1 and 4.2.2.

4.2.  Session-Reflector Behavior and Packet Format

   Two modes of STAMP Session-Reflector characterize the expected
   behavior and, consequently, performance metrics that can be measured:

   o  Stateless - STAMP Session-Reflector does not maintain test state
      and will reflect the received sequence number without
      modification.  As a result, only round-trip packet loss can be
      calculated while the reflector is operating in stateless mode.

   o  Stateful - STAMP Session-Reflector maintains test state thus
      enabling the ability to determine forward loss, gaps recognized in
      the received sequence number.  As a result, both near-end
      (forward) and far-end (backward) packet loss can be computed.
      That implies that the STAMP Session-Reflector MUST keep a state
      for each accepted STAMP-test session, uniquely identifying STAMP-
      test packets to one such session instance, and enabling adding a
      sequence number in the test reply that is individually incremented
      on a per-session basis.

This seems important enough -- the mode determines what data STAMP
can measure -- to be promoted to part of section 4, "Theory of
Operation".

4.3.  Integrity and Confidentiality Protection in STAMP

   To provide integrity protection, each STAMP message is being
   authenticated by adding Hashed Message Authentication Code (HMAC).

Of course, this is only regarding authenticated mode.  So you want to
phrase this "Authenticated mode provides integrity protection to each
STAMP message by adding ...".

4.4.  Interoperability with TWAMP Light

   For example, a TWAMP Light
   Session-Reflector may not support the use of UDP port 862 as defined
   in [RFC8545].  Thus STAMP Session-Sender MAY use port numbers as
   defined in Section 4.

The connection between these two sentences is unclear.  I think it
means:

   For example, a TWAMP Light
   Session-Reflector may not support the use of UDP port 862 as specified
   in [RFC8545].  Thus Section 4 permits a STAMP Session-Sender to use
   alternative ports.

--

   The
   Session-Sender SHOULD use the default format for its timestamps -
   NTP.  And it MAY use PTPv2 timestamp format

The first sentence could be made more specific.  But the second
sentence seems to be redundant -- SHOULD is never mandatory, so you
don't have to add a MAY.  So perhaps,

   When interoperating with a TWAMP Light Session-Reflector, the
   Session-Sender SHOULD use the default format for its timestamps -
   NTP.

--

   In the latter scenario, if a TWAMP Light Session-Sender does not
   support the use of UDP port 862, the test management system MUST set
   STAMP Session-Reflector to use UDP port number as defined in
   Section 4.

The phrase "to use UDP port number as defined in Section 4" isn't
clear, since second 4 doesn't define what UDP port number should be
used in this situation.  I think the meaning is "the test management
system MUST set STAMP Session-Reflector to use an acceptable UDP port
number, as permitted by Section 4".

6.  Security Considerations

   STAMP test packets can be transmitted with the destination UDP port
   number from the User Ports range, as defined in Section 4, that is
   already or will be assigned by IANA.

This sentence seems to be more part of the port-number discussion (see
comments on section 4) than security considerations -- unless the
usage of port numbers is a topic in security.

Additionally, it appears that section 4 requires that a STAMP service
can be configured to sent packets to any port within the User Ports
range, regardless of whether IANA assigns a User Port specifically to
STAMP.  With this consideration, the phrase "that is already or will
be assigned by IANA" seems to be either redundant or too narrow.

[END]