Telechat Review of draft-ietf-lime-yang-connectionless-oam-13
review-ietf-lime-yang-connectionless-oam-13-genart-telechat-davies-2017-10-23-00

Request Review of draft-ietf-lime-yang-connectionless-oam
Requested rev. no specific revision (document currently at 18)
Type Telechat Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2017-10-24
Requested 2017-10-11
Draft last updated 2017-10-23
Completed reviews Secdir Telechat review of -11 by Charlie Kaufman (diff)
Yangdoctors Early review of -05 by Carl Moberg (diff)
Genart Telechat review of -13 by Elwyn Davies (diff)
Assignment Reviewer Elwyn Davies
State Completed
Review review-ietf-lime-yang-connectionless-oam-13-genart-telechat-davies-2017-10-23
Reviewed rev. 13 (document currently at 18)
Review result Ready with Issues
Review completed: 2017-10-23

Review
review-ietf-lime-yang-connectionless-oam-13-genart-telechat-davies-2017-10-23

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-lime-yang-connectionless-oam-13
Reviewer: Elwyn Davies
Review Date: 2017-10-23
IETF LC End Date: 2017-10-25
IESG Telechat date: 2017-10-26

Summary:Not really ready.  There are several missing references and the English needs cleaning up to make the document comprehensible. I found s3 to be almost totally opaque.  The fundamental concept of a Test Point needs a proper definition in s2 and a clearer introduction in s3.  The concept of 'neighboring test points' confused me for some time: I was thinking of neighboring nodes in the network whereas what seesm to be meant is a possibility of a multiplicity of 

Major issues:
None

Minor issues:
Title and description of model:
The title refers to 'connectionless networks'.  In practice the YANG model could be used with both connectionless and connection-oriented communication technologies.  I think the intention is to be able to support the management of OAM protocols that operate in a connectionless manner (i.e., using connectionless *technologies*, as per RFC 7276) rather than connectionless networks.
In the title -
OLD:
      Generic YANG Data Model for Operations, Administration, and
         Maintenance(OAM) protocols for Connectionless networks
NEW:
      Generic YANG Data Model for the Management of Operations, 
         Administration, and Maintenance (OAM) Protocols that
                     use Connectionless Communications
END

Similarly, in s1, para 1, s/connections/communications/.
In next to last para of s1: 
OLD:
   Note that the Connection-Oriented OAM YANG DATA model is defined in
   [I-D.ietf-lime-yang-connection-oriented-oam-model].

NEW:
   Note that the YANG DATA model for OAM protcols using connection-oriented 
   communications is defined in [I-D.ietf-lime-yang-connection-oriented-oam-model].
END 

s2.1: The term 'Test point' needs some actual definition - It appears from the body of the document that a TP is effectively equated to an interface together with an associated stack layer (MAC, IP, etc) or superimposed application technology (VPN end point, etc.).  One query that came into my mind around this was what happens if the IP address associated with an interface is changed dynamically (e.g., when using IPv6 privacy addresses).  Can the YANG manager understand that it is still dealing with the same interface although the IP address has changed?  I wondered if the interfaces really needed some sort of identifier (e.g., interface number) that would tie all the pieces together as well as the intra-/inter-layer pointers.

s3.3:
>    OAM
>    neighboring test points are referred to a list of neighboring test
>    points in the same layer that are related to the current test point.
>    This allows users to easily navigate between related neighboring
>    layers to efficiently troubleshoot a defect.  In this model, the
>    'position' leaf defines the relative position of the neighboring test
>    point corresponding to the current test point in the same layer, and
>    is provided to allow correlation of faults at different locations.
I don't understand what is going on here.  Doesn't fault correlation require association of test points in adjacent layers up amd down the stack for the same interface rather than the same layer?  The before/after story then allows the manager to go up and down the stack looking at wat is going on in the different layers.  I can't see any likelihood of there being multiple test points in the same layer in a given interface (unless this has something to do with possible different administrative domains. Help! If this is altered, the similar text in the descriptions of oam-neighboring-tps (in s4) will need to be made consistent.

Sources of imported models:  It would be useful to list the RFCs/I-Ds that define the models that are imported.  Currently draft-ietf-netmod-schema-mount, draft-ietf-rtgwg-ni-model and draft-ietf-rtgwg-routing-types that are under development are not mentioned; the existing standards of RFC 6021 and RFC 7223 should also be referenced (7223 is).  They should all be normative.

Nits/editorial comments: 
========================

idnits: complains about some overlong lines... probably ones with 'when "derived-from-or-self('
General: As mentioned by other reviews, there area considerable number of places where it appears that " '" should really be "' " and there are missing spaces after single quotes. 

General:  The document is inconsistent in its use of connectionless/connection-less/connection less.  The preferred usage should be connectionless as is used in most cases.  Thus:
Short title: s/Connection-Less/Connectionless/
s4:
OLD:
  feature connection-less {
    description
      "This feature indicates that OAM solution is connection less.";
  }

NEW:
  feature connectionless {
    description
      "This feature indicates that OAM solution is connectionless.";
  }
ENDS

s1, last para:
OLD:
   In this document, we presents a base YANG Data model for
   connectionless OAM protocols.  The generic YANG model for
   connectionless OAM only includes configuration data and state data.
   It can be used in conjunction with data retrieval method model
   [I-D.ietf-lime-yang-connectionless-oam-methods], which focuses on
   data retrieval procedures like RPC.  However it also can be used
   independently of data retrieval method model.
NEW:
   This document documents a base YANG Data model for
   connectionless OAM protocols.  This generic YANG model for
   connectionless OAM only includes configuration data and state data.
   It can be used in conjunction with data retrieval method model
   described in [I-D.ietf-lime-yang-connectionless-oam-methods], which focuses on
   data retrieval procedures such as RPC.  However it also can be used
   independently of this data retrieval method model.
ENDS  

s2.1:  As mentioned above, TP needs some definition.  MAC is primarily concerned with MAC address in this document - definition: address for data link layer interface.  BFD should have a reference probably to RFC 5880.  It would probably be sensible to split the section into expanded modertely well-known abbreviations  (MAC, BFD, RPC*) and new terms (TP, CC).

s2.1, last para: s/e.g. /e.g., /

s3: Maybe the usage "is/are augmented to" is accepted YANG jargon but it isn't good English. "Augments"  will be good instead.
 
s3, para 1: The 'nd' prefix is part of the YANG specification in s4 and isn't known at this point. 

s3, para 3: s/eg.,/e.g.,/

s3, last para: s/test- point-locations/test-point-locations/

s3, most of the section, but especially the last para: I found this to be almost totally unreadable and useless. 

s3.1:
This needs to be clarified.
OLD:
   In connectionless OAM, the TP address is defined with the following
   type:

   o  MAC address [RFC6136]

   o  IPv4 or IPv6 address

   o  TP-attribute

   o  System-id to represent the device or
      node.[I-D.ietf-spring-sr-yang]
NEW:
   With connectionless OAM protocols, the TP address can be one of the following
   types:

   o  MAC address [RFC6136] for link layer TPs

   o  IPv4 or IPv6 address for IP layer TPs

   o  TP-attribute identifying a TP associated with an application layer function

   o  System-id to represent the device or
      node.[I-D.ietf-spring-sr-yang]
ENDS 

s3.1, last para: s/'tp-address'grouping/'tp-address' grouping/ 

s3.3:
I found this a little confusing - suggest:
OLD;
   As typical networks have a multi-layer architecture, the set of OAM
   protocols similarly take a multi-layer structure; each layer may have
   its own OAM protocol [RFC7276] corresponding to a specific
   administrative domain and has associated test points.
NEW:
   As typical network communication stacks have a multi-layer architecture, 
   the set of associated OAM protocols may similarly have a multi-layer 
   structure; each communication layer in the stack may have its own OAM 
   protocol [RFC7276] that may also be linked to a specific administrative 
   domain.  Management of these OAM protocols will necessitate associated 
   test points in the nodes accessible by appropriate management domains.

   Accordingly, a given network interface may present several test points  
ENDS

s3.5: s/e.g.,VRF/e.g., VRF/

s3.: s/per- hop/per-hop/

s4, Module/description:
Also needs the IETF copyright and redistribution boiler plate.
OLD:
  description
    "This YANG module defines the generic configuration,
     data model, statistics for connectionless OAM to be
     used within IETF in a protocol independent manner.
     It is assumed that each protocol maps corresponding
     abstracts to its native format. Each protocol may
     extend the YANG model defined here to include protocol
     specific extensions";
NEW:
  description
    "This YANG module defines the generic configuration,
     data model, and statistics for OAM protocols using 
     connectionless communications, described in a 
     protocol independent manner.
     It is assumed that each protocol maps corresponding
     abstracts to its native format. Each protocol may
     extend the YANG model defined here to include protocol
     specific extensions";
ENDS

s4, module/contact, module/organization:  These need to be 'future proofed' - the WG and the draft authors are not appropriate for a standard.

s4, grouping session-jitter-statistics/description: s/e.g.,Packet/e.g., Packet/

s5, multiple places: s/bfd/BFD/g

s5, para 1: s/"ietf-connectionless-oam" model/The "ietf-connectionless-oam" model/; 
s/technology-independent/a technology-independent/

s5, para 2:
OLD:
 Note that, in this section, we only present several
   snippets of technology-specific model extensions for illustrative
   purposes.
NEW:
 Note that, in this section, several snippets of technology-specific 
   model extensions are presented for illustrative purposes.
ENDS

s5.1: I notice that RFC 7276 defines BFD as a connection-oriented protocol (that is used to monitor a connectionless protocol in the case of basic BFD for IP)! Some explanation may be appropriate.

s5.1.1, para 2:
OLD:
 Note that in BFD WG, there is a BFD YANG data model
   [I-D.ietf-bfd-yang] to be produced.  Users can choose to use "ietf-
   connectioless-oam" as basis and augment the "ietf-connectionless-oam"
   model with bfd specific details.  The bfd specific details can be the
   grouping defined in the BFD model.
NEW:
 Note that a dedicated BFD YANG data model [I-D.ietf-bfd-yang] is also 
   standardized.  Augmentation of the "ietf-connectionless-oam" model 
   with BFD specific details provides an alternative approach that 
   provides a unified view of management information across various OAM 
   protocols.  The BFD specific details can be the grouping defined in 
   the BFD model avoiding duplication of effort.
ENDS

s5.1.1.1, para 2:
OLD:
The snippet below depicts an example of augmenting "bfd" type into
   the ietf-connectionless-oam":
NEW:
The snippet below depicts an example of adding the "bfd" type as an 
   augment to the ietf-connectionless-oam" model:
ENDS

s5.1.1.2:
OLD:
To support bfd technology, the "ietf-connectionless-oam" model can be
   extended and add bfd specific parameters under "test-point-locations"
   list and/or add new location type such as "bfd over MPLS-TE" under
   "location-type".
NEW:
To support BFD technology, the "ietf-connectionless-oam" model can be
   extended by adding specific parameters into the "test-point-locations"
   list and/or adding a new location type such as "BFD over MPLS-TE" under
   "location-type".
ENDS

s5.1.1.2.1, para 1:
OLD:
 In
   this section, we reuse some groupings which are defined in
   [I-D.ietf-bfd-yang] as following:
NEW:
 In this section, some groupings which are defined in
   [I-D.ietf-bfd-yang] are reused as follows:
ENDS

s5.1.1.2.2, para 2:
OLD:
In this section, we add a new "location-
   type" case and reuse some groupings which are defined in
   [I-D.ietf-bfd-yang] as follows:
NEW:
In this section, a new "location-type" case is added and some groupings that are defined in
   [I-D.ietf-bfd-yang] are reused as follows:
ENDS

s5.1.2:
OLD:
   And another alternative method is using schema mount mechanism
   [I-D.ietf-netmod-schema-mount] in the "ietf-connectionless-oam".
   Within the "test-point-locations" list, a "root" attribute is defined
   to provide a mounted point for models mounted per "test-point-
   locations".  Therefore, the "ietf-connectionless-oam" model can
   provide a place in the node hierarchy where other OAM YANG data
   models can be attached, without any special extension in the "ietf-
   connectionless-oam" YANG data models [I-D.ietf-netmod-schema-mount].
   Note that the limitation of the Schema Mount method is it is not
   allowed to specify certain modules that are required to be mounted
   under a mount point.

   The snippet below depicts the definition of "root" attribute.
NEW:
   Another alternative method is using the schema mount mechanism
   [I-D.ietf-netmod-schema-mount] in the "ietf-connectionless-oam" model.
   Within the "test-point-locations" list, a "root" attribute is defined
   to provide a mount point for models mounted per "test-point-
   locations".  Therefore, the "ietf-connectionless-oam" model can
   provide a place in the node hierarchy where other OAM YANG data
   models can be attached, without any special extension in the "ietf-
   connectionless-oam" YANG data models [I-D.ietf-netmod-schema-mount].
   Note that the limitation of the Schema Mount method is it is not
   allowed to specify certain modules that are required to be mounted
   under a mount point.

   The snippet below depicts the definition of the "root" attribute.
ENDS

s5.2.1:
OLD:
   The following sections shows how the "ietf-connectionless-oam" model
   can be extended to support LSP ping technology.  For this purpose, a
   set of extension are introduced such as technology-type extension and
   test-point attributes extension.

   Note that in MPLS WG, there is a LSP Ping YANG data model
   [I-D.zheng-mpls-lsp-ping-yang-cfg] to be produced.  Users can choose
   to use "ietf-connectioless-oam" as basis and augment the "ietf-
   connectionless-oam" model with LSP Ping specific details in the model
   extension.  The LSP Ping specific details can be the grouping defined
   in the LSP ping model.

NEW:
   The following sections shows how the "ietf-connectionless-oam" model
   can be extended to support LSP ping technology.  For this purpose, a
   set of extensions are introduced such as the "technology-type" extension and
   the test-point "attributes" extension.

   Note that a LSP Ping YANG data model
   [I-D.zheng-mpls-lsp-ping-yang-cfg] has been standardized.  As with BFD, users can choose
   to use the "ietf-connectioless-oam" as basis and augment the "ietf-
   connectionless-oam" model with LSP Ping specific details in the model
   extension to provide a unified view across different technologies.  
   The LSP Ping specific details can be the grouping defined
   in the LSP ping model to avoid duplication of effort..

ENDS

s9:  I think I-D.ietf-i2rs-yang-network-topo is normative.  One could discuss whether the various drafts mentioned in s5 are also normative.  Some additional normative references will come form listing the sources of imported modules (see minor issues). idnits complains that RFCs 6991, 7223 and 5462 are not explicitly referenced.  6991 and 7223 are import sources (see above) 5462 is used in s3.1 but isn't marked as a reference.