Re: Genart telechat review of draft-ietf-ippm-twamp-yang-11

[Mahesh Jethanandani <mjethanandani@xxxxxxxxx>]

Hi Pete,

On Jun 19, 2018, at 9:02 PM, Pete Resnick <presnick@xxxxxxxxxxxxxxxx> wrote:

Reviewer: Pete Resnick
Review result: Ready with Issues

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 wait for direction from your
document shepherd or AD before posting a new version of the draft.

For more information, please see the FAQ at

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

Document: draft-ietf-ippm-twamp-yang-11
Reviewer: Pete Resnick
Review Date: 2018-06-19
IETF LC End Date: 2018-04-27
IESG Telechat date: 2018-06-21

Summary:

Ready with maybe Issues, but probably just Nits. Not my area of expertise by
any means, but the document looks generally solid. Could definitely use a bit
of copy editing.

Major issues:

None.

Minor issues:

I don't think there are any issues here. However, some of the things I've got
as Nits in the below section could amount to actual issues if I've
misunderstood what you meant. The editorial suggestions I give below should be
fine if they are nits, but do make sure that I haven't identified a real issue.

Nits/editorial comments:

3.1 - s/The test session name that MUST be identical/The test session name,
which MUST be identical (Unless you mean something really weird that I don't
think you mean. If you don't see the difference, then trust me, you mean
"which", not "that”.)

You mean in Section 3.3. How about s/The test session name that MUST be identical with the/The test session name MUST be identical to the/?

4.1 -

OLD
  Specifically, mode-preference-chain lists the
  mode and its corresponding priority, expressed as a 16-bit unsigned
  integer, where zero is the highest priority and subsequent integers
  increase by one.

This is a bit confusing. I think you mean:

NEW
  Specifically, mode-preference-chain lists the mode and its
  corresponding priority, expressed as a 16-bit unsigned integer.
  Values for the priority start with zero, the highest priority, and
  subsequent priority value increases by one.

I can see why this can be confusing. How about ...

NEW

   Specifically, mode-preference-chain lists the
   mode and its corresponding priority as a 16-bit unsigned
   integer. Values for the priority start with zero, the highest priority, and 

   decreasing priority value is indicated by every increase of value by one.

OLD
  In turn, each ctrl-connection holds a list of test-session-request.
  test-session-request holds information associated with the Control-
  Client for this test session.

A bit awkward. I suggest:

NEW
  In turn, each ctrl-connection holds a test-session-request list. Each
  test-session-request holds information associated with the
  Control-Client for this test session.

Ok.

OLD
  The Control-Client is also responsible for scheduling TWAMP-Test
  sessions so test-session-request holds information related to these
  actions (e.g. pm-index, repeat-interval).

The word "so" in there is weird. Do you mean "therefore", or "such that", or
something else? I just had a bit of trouble understanding what you meant.

We meant “therefore”. Will make the change.

4.2 - In the penultimate paragraph, change "key-id" to either "The key-id" or
"The KeyID”.

Will change it to “The key-id”.

Please note: I did not thoroughly review the YANG in section 5.2 or the
examples in Section 6 or Appendix A. I gave them a quick run through, but did
not check for complete consistency with the rest of the text. The below two
items are simply things I happened to spot because I was looking at particular
pieces of the module.

5.2 -

          leaf priority {
            type uint16;
            description
              "Indicates the Control-Client Mode preference priority
               expressed as a 16-bit unsigned integer, where zero is
               the highest priority and subsequent values
               monotonically increasing.";
          }

I am almost positive that you don't mean "monotonically increasing". I'm
guessing you mean "increase by one”.

Will update this description to match the comment you made above or whatever we agree to.

             Depending on the Modes available in the TWAMP Server
             Greeting message (see Fig. 2 of RFC 7717), the
             this Control-Client MUST choose the highest priority
             Mode from the configured mode-preference-chain list.";

Typo: "the this Control-Client”

Will fix it to say “the Control-Client”.

Thanks.

Mahesh Jethanandani

mjethanandani@xxxxxxxxx