Re: [ippm] Genart last call review of draft-ietf-ippm-stamp-07

[Greg Mirsky <[e-mail]>]

Hi Dale,
thank you for your kind consideration of the proposed updates, thoughtful
and helpful suggestions. I've clipped parts of our discussion that we've
agreed on the changes. My follow-up notes are in-lined under GIM2>> tag.
Attached, please find the updated working version and its diff to -07
version of the STAMP draft.

Regards,
Greg

On Sun, Oct 13, 2019 at 6:03 PM Dale R. Worley <[e-mail]> wrote:

> > From: Greg Mirsky <[e-mail]>
> >
> > Hi Dale,
> > hope this friendly reminder finds you well. I much appreciate your
> > consideration of the responses to your helpful comments.
>
> My apologies for the delay.
>
> All of this looks good.  Although there are a couple of minor points
> that I've added below.
>
> On Mon, Sep 30, 2019 at 5:17 PM Greg Mirsky <[e-mail]> wrote:
>
> > Hi Dale,
> > thank you for your review, comments, and suggestions. Please find answers
> > and notes in-line under the GIM>> tag.
> > Attached, please find the diff between -07 and the current working
> > version, as well as, the working version of the draft.
> >
> > Regards,
> > Greg
> >
> > On Tue, Sep 3, 2019 at 6:01 PM Dale Worley via Datatracker <
> > [e-mail]> wrote:
> >
> >>   ----------------------------------------------------------------------
> >> 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.
> >>
>
> >> Editorial issues:
> >>
> >> 1.  Introduction
> >>
> >>    and inherit separation of control (vendor-specific configuration or
> >>    orchestration) and test functions.
> >>
> >> Is "inherit" intended to be "inherent"?
> >>
> > GIM>> Thank you. That was the intention. Done.
> >>
> >>    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.
> >>
> > GIM>> Thank you for the proposed text. While addressing other comments
> the
> > text has been transformed to the following:
> > CURRENT TEXT:
> >    Recent work on IP Edge to Customer Equipment using TWAMP Light from
> >    Broadband Forum [BBF.TR-390] demonstrated that interoperability among
> >    implementations of TWAMP Light is challenged because the composition
> >    and operation of TWAMP Light were not sufficiently specified in
> >    [RFC5357].  According to [RFC8545], TWAMP Light includes sub-set of
> >    TWAMP-Test functions to provide comprehensive solution requires
> >    support by other applications that provide, for example, control and
> >    security.
> > Hope the current version is clearer and more readable.
>
> I would change "interoperability among implementations of TWAMP Light
> is challenged" to "interoperability among TWAMP Light implementations
> is poor", or perhaps "... is difficult".
>
GIM2>> Will use "difficult".

>
> In the last sentence, I think there are words missing from "to provide
> comprehensive solution requires support"; perhaps "to provide a
> comprhenesive soultion with support"?
>
GIM2>> I agree, that sentence is awkward. Here's its update:
OLD TEXT:
   According to [RFC8545], TWAMP Light includes sub-set of
   TWAMP-Test functions to provide comprehensive solution requires
   support by other applications that provide, for example, control and
   security.
NEW TEXT:
   According to [RFC8545], TWAMP Light includes a sub-set of
   TWAMP-Test functions.  Thus, to have a comprehensive tool to measure
   packet loss and delay requires support by other applications that
   provide, for example, control and security.

>
>
> >>    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.
> >>
> > GIM>> In regard to the format of a timestamp, STAMP specification follows
> > the idea of RFC 8186 where extension to negotiate timestamp format for
> > TWAMP-Control and the new interpretation of Z flag were defined. For
> STAMP,
> > as defined for TWAMP, the value of the Z flag in the Error Estimate field
> > is only to reflect the format used by the node.
>
> In that case, I think you want to add to the definition of the
> timestamp field that the format it uses is part of the configuration
> of the session.  In any situation, it must be clear how the receiver
> of a STAMP packet determines how to interpret the Timestamp value.
>
GIM2>> Thank you for the suggestion. I've added the sentence to the
definition of Timestamp:
    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 (PTP) truncated 64-bit
      timestamp format [IEEE.1588.2008], the format used in [RFC8186].
      The use of the specific format, NTP or PTP, is part of
      configuration of the Session-Sender or the particular test
      session.
>
>
> >> 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.
> >>
> > GIM>> Thank you for the suggested text. I've used the second option.
> >
> >>
> >> 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".
> >>
> > GIM>> I agree. Moved the text to Section 4 and re-named this section to
> > Session-Reflector Packet Format
>
> That looks good, except that the first words of para 4 are
> "STAMP supports two modes:", just after para 2 and 3 define two
> *other* modes.  So I'd change para 4 to start "STAMP supports two
> authentication modes:".
>
GIM2>> Agree. Thank you.

>
>
> Dale
>