Early Review of draft-ietf-ippm-stamp-yang-01
review-ietf-ippm-stamp-yang-01-yangdoctors-early-jethanandani-2018-04-22-00

Request Review of draft-ietf-ippm-stamp-yang
Requested rev. no specific revision (document currently at 05)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2018-04-27
Requested 2018-03-21
Requested by Brian Trammell
Authors Gregory Mirsky, Min Xiao, Wei Luo
Draft last updated 2018-04-22
Completed reviews Yangdoctors Early review of -01 by Mahesh Jethanandani (diff)
Assignment Reviewer Mahesh Jethanandani
State Completed
Review review-ietf-ippm-stamp-yang-01-yangdoctors-early-jethanandani-2018-04-22
Reviewed rev. 01 (document currently at 05)
Review result Not Ready
Review completed: 2018-04-22

Review
review-ietf-ippm-stamp-yang-01-yangdoctors-early-jethanandani-2018-04-22

Document reviewed: draft-ietf-stamp-yang-01

Status: Not ready.

Summary:

This document specifies the data model for implementations of Session-Sender and Session-Reflector for Simple Two-way Active Measurement Protocol (STAMP) mode using YANG.

Minor

- Please update references of RFC6020 with RFC7950, as this is a YANG 1.1 model.
- Please add a separate paragraph or section of notes to the RFC editor, instead of scattering it across the document. RFC Editors appreciate not having to look for instructions all over the document.
- Also what does “ref to STAMP draft” imply? This draft, some other draft, or RFC?
- Please update references to NMDA architecture document.

Major:

If the data model is for STAMP, how is that RFC (or I-D) not referenced anywhere in the model? At the minimum most of data nodes should have references to the sections that describe them in the STAMP RFC.

Please fix indentation issues in the model. I see use of 1 space, 2 spaces, 4 spaces and even 8 spaces in the model. It makes reading of the model very difficult.

If statements carry into a subsequent line, consider indenting the second line.

s/stamp/STAMP/g

This document is NOT NMDA compliant. The whole idea of NMDA is that you do not need a separate -state container. NDMA does not advocate having a separate container with exactly the same nodes from config. An example is the nodes ‘packet-padding-size’ and ‘interval’, which are duplicated in both the config and -state container. On the other hand ‘duplicate-packets’ and ‘reordered-packets’ make sense. Once the duplicate nodes have been removed, rather than calling the container stamp-state, call it stamp-statistics, or something else that does not imply the same meaning as pre-NMDA models containers did.

Section 2.1

- The draft says it “describes all the parameters of the stamp data model.” I do not think you are describing the parameters of the STAMP data model. What you are describing is the block diagram (Fig 1) or the containers within the model, not the individual parameters.

Section 2.1.1

Once the -state container has been updated/removed, update the second paragraph.

s/referenced by session-id STAMP/referenced session by the session-id of the STAMP/

Section 3

I am not sure on the use of MAY. Is it that the use of IP and UDP port is sufficient? If so, when it is okay? There is also a use of ‘may’ w.r.t. to STAMP test sessions. How are the two mays related?

Finally, even if one was to use 5-tuple as a key, which includes a DSCP value that can be remarked, how would one uniquely identify a session?

Section 3.1

Please add a informative reference to the Tree Diagram RFC.

Section 3.2

Please add references to any RFCs referenced in the model up front, before the model.

Remove the comment “//namespace need to be assigned by IANA”. Instead update the IANA section with the information of what namespace needs to be assigned.

Could a shorter prefix be used, e.g. “stamp”?

For reference to RFC 6991 and 8177, please add the title of those RFCs.

For contact, please add the name of individual authors instead of using a mailing list, per Section 4.8 of rfc6087bis.

The description is very sparse. Consider adding more text. Also the document is missing the IETF Copyright statement, per section 4.8 of rfc6087bis.

The revision statement should say something like “Initial version” instead of 00, as those numbers are not relevant once the document is published. Also consider replacing “” for the reference statement with something like “RFC XXXX: STAMP Data Model”, and have a note of the RFC editor to replace XXXX with the assigned RFC number.

Consider updating descriptions for each leaf. A description “Packets received” for ‘rcv-packets’ is not descriptive. Also add references to sections in the STAMP RFC.

Why is there a need for feature statements for ‘session-sender’ and ‘session-reflector’. Would there be a case that a customer would configure a ‘session-reflector’ without a ‘session-sender’ or vice-versa?

What is ’type enable’? Did the authors mean ‘type boolean”?

Would it not be simpler to have ‘number-of-packets’ or ‘repeat’ as a choice statement, where the choice is between a set number of test packets or ‘forever’? Same for the other union statements.

Consider using ‘derived-from’ or ‘derived-from-or-self’ in when statements, instead of a string comparison.