Re: [yang-doctors] Yangdoctors early review of draft-ietf-ippm-stamp-yang-01

[Greg Mirsky <[e-mail]>]

Hi Mahesh,
thank you for the thorough review and the most detailed and thoughtful
comments, suggestions. I'll work on the update in steps and will consult
with you to address all issues you've pointed to.

Best regards,
Greg

On Sun, Apr 22, 2018 at 10:14 PM, Mahesh Jethanandani <
[e-mail]> wrote:

> Reviewer: Mahesh Jethanandani
> Review result: Not Ready
>
> 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.
>
>
>