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.