RE: [Teas] Yangdoctors last call review of draft-ietf-teas-yang-te-topo-

Hi Rob,

While the tooling is very nice to have, especially for writing new models or 
converting models, we do not have to use it for every model. It seems not 
necessary to use the tooling on this model for now. We already know that we can 
manually convert it to NMDA style if needed.

Thanks,
- Xufeng

-----Original Message-----
From: Robert Wilton [mailto:rwilton(_at_)cisco(_dot_)com]
Sent: Wednesday, June 21, 2017 5:34 AM
To: Xufeng Liu <Xufeng_Liu(_at_)jabil(_dot_)com>; Mahesh Jethanandani
<mjethanandani(_at_)gmail(_dot_)com>; yang-doctors(_at_)ietf(_dot_)org
Cc: ietf(_at_)ietf(_dot_)org; teas(_at_)ietf(_dot_)org; 
draft-ietf-teas-yang-te-topo(_dot_)all(_at_)ietf(_dot_)org
Subject: Re: [Teas] Yangdoctors last call review of 
draft-ietf-teas-yang-te-topo-
08

Hi Xufeng,

On 12/06/2017 22:28, Xufeng Liu wrote:

Hi Mahesh,

Thank you much for the review. We have submitted an updated draft

(https://tools.ietf.org/html/draft-ietf-teas-yang-te-topo-09) to address these
issues. More detailed explanations are put below inline.


If the responses and updates are satisfactory, we are ready for the last 
call.

Best regards,
- Xufeng

-----Original Message-----
From: Mahesh Jethanandani [mailto:mjethanandani(_at_)gmail(_dot_)com]
Sent: Wednesday, May 24, 2017 11:44 AM
To: yang-doctors(_at_)ietf(_dot_)org
Cc: ietf(_at_)ietf(_dot_)org; teas(_at_)ietf(_dot_)org;
draft-ietf-teas-yang-te-topo(_dot_)all(_at_)ietf(_dot_)org
Subject: Yangdoctors last call review of
draft-ietf-teas-yang-te-topo-08

Reviewer: Mahesh Jethanandani
Review result: Ready with Issues

Document reviewed: draft-ietf-teas-yang-te-topo-08

Status: Ready with Issues

I am not an expert in Traffic Engineering. This review is looking at
the draft from a YANG perspective. With that said, I have marked it as 
“Ready

with Issues”

because of some of the points discussed below.

Summary:

This document defines a YANG data model for representing, retrieving
and manipulating TE Topologies. The model serves as a base model that
other technology specific TE Topology models can augment.

Comments:

Almost all the containers in the model are presence containers. Is
there a reason why they have to be presence containers? Note, that
presence containers are containers whose existence itself represents
configuration data. What particular configuration data is each container

representing in itself?

[Xufeng] Containers that use “presence” are:
    - Container “underlay”
      o  We have changed 13 occurrences of such containers to be not

presence container.

    - Container “te” under augmentation
      o  To indicate that “TE” is enabled (configuration data)
      o  Also used to do augmentation. The “presence” statement can

prevent the mandatory child from affecting augmented base model.

    - /nw:networks/nw:network/nw:network-types/te-topology!
      o  A mechanism required by I2RS topology model to specify the

topology type.

It is difficult to co-relate the diagram with the model itself
because of different terms being used to define different parts of the 
model.
There is “TE Topology Model” and then there is “Generic TE Topology

Model”.

Are these one and the same models? If so, a common term for both of
them would be helpful.

[Xufeng] Yes. These two terms are the same. Figure 12, Figure 13, and 
relevant

descriptions have been updated to make the document consistent.

There is extensive use of groupings in the document. However, not all
instances of groupings are used multiple number of times. Where they
are not being repeated, it would be better to move the grouping
directly where the uses statement resides. Case in point the grouping

connectivity-label-restriction-list.

[Xufeng] We have removed the following groupings
     te-link-augment
     te-node-augment
     te-termination-point-augment
     te-topologies-augment
     te-topology-augment
     te-link-state-underlay-attributes
     te-node-state-derived-notification
     te-topology-type

The remaining groupings have been kept so that we can:
    - Share the groupings in this model
    - Prepare to be shared by a model augmenting this model
    - Prevent a grouping or configuration section from being too long
    - Improve readability

The split between config and state containers does not seem to follow
any particular pattern.

[Xufeng] The pattern is clear:
For each manageable entity (object), there is a config container and state

container. The configurable properties go into the config container and state
properties go into the state container. Such objects are identified by a list 
item
or a presence container so that the “create”, “delete”, and “modify” 
operations
can be performed on them. The non-presence containers do not represent
configuration data so they do not introduce such objects.

It is neither a top level split, as is the case with existing IETF
models,

[Xufeng] We could not do top level split because the base I2RS network

topology model (https://tools.ietf.org/html/draft-ietf-i2rs-yang-network-topo-
12) that we augment does not have the top-level split (for its own reasons).

nor do they follow the OpenConfig style of splitting config and state
under each relevant leaf,

[Xufeng] The pattern is consistent with this style in principle, with some

adjustments to fit to our multiple levels of hierarchy.
This is effectively a new forth style of YANG models that is not consistent 
with
any of the three existing styles, i.e.:
  - Current IETF config/state split model style
  - NMDA combined config/state tree
  - OpenConfig split config/state containers immediately above the config true
leaves.

Tooling that it designed to work with OpenConfig models will need
customization to work with these TE models because the config/state
containers will not be where the tooling expects them to be.

Thanks,
Rob

RE: [Teas] Yangdoctors last call review of draft-ietf-teas-yang-te-topo-08