Subject: Yangdoctors early review of draft-ietf-lime-yang-connectionless-oam-05

From: Carl Moberg <[email-masked]>
Date: Thu, 01 Jun 2017 10:45:22 -0700

Reviewer: Carl Moberg
Review result: On the Right Track

I am the assigned YANG Doctor for the LIME YANG modules, this is the
notes from my initial review of
draft-ietf-lime-yang-connectionless-oam done with the authors on a
call last week:

The module is intended to be implemented as a service model, per the
module classification draft
(https://tools.ietf.org/html/draft-ietf-netmod-yang-model-classification-05)
with augments to topologies, I suggest it needs more description on
the general concepts and how to use it.

I suggest to remove the 'OAM data hierarchy' section. It does not add
any information.

The current version YANG module is all configuration except the 'oper'
subtree. And there are no mandatory nodes (except list keys) which
seems to allow for many, many combinations that likely make no sense.
Please re-review and make sure this is an informed choice.

Suggest to rename tp-address-type-value to
tp-address-technology-type.

Why does 'grouping tp-technology' only allow 'technology-null' or
'technology-string'/'leaf ipv4-icmp' and what is the meaning of the
leaf ipv4-icmp content?

There are a couple of occasions of 'ordered-by user' that does not
have a description of the meaning of the order. Please review.

Add an if-feature on path-discovery in *-methods draft.

Generally I'd suggest adding more information about the various types
and which specifications you are picking them up from (e.g. RD, LSP
ID, etc)

The specification defines a typedef that is a path in another module
("routing-instance-ref"). Suggest to reach out to authors of
draft-ietf-rtgwg-ni-model-02 to see if they want the -ref type in that
module for reuse.

Generally check whitespace and comments. Use the '-f yang' option to
YANG to fix whitespace.

Downcase "IPv4-Multicast-Group-Address", and
"IPv6-Multicast-Group-Address", "IP-Multicast-Group-Address"

Name of top container ('oper') not very helpful, suggest renaming to
'session-statistics'?

Spelling of grouping (and uses) cc-session-statsitics

Filename should include revision in CODE BEGINS

Don't reference drafts, pleas check RFC6087 bis and well-formed drafts
for header stmts
  
https://datatracker.ietf.org/doc/html/draft-ietf-netmod-rfc6087bis#section-4.9'