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

[Benoit Claise <[e-mail]>]

Dear draft-ietf-teas-yang-te-topo authors,
> Hi Xufeng,
>
> OK, by tooling, I don't mean the pyang plugins that I have been 
> working on to convert between different types of models.  As you 
> aware, the TE YANG models can easily be converted to NMDA style since 
> I have already done it 
> (https://github.com/rgwilton/ietf-models-to-combined).
>
> My comment actually relates to the fact the structure used by TE YANG 
> modules don't match any other YANG modules - they are using their own 
> unique style of structure.
This is an important issue to resolve.
>
> Today, there are three common styles of modules:
> (1) IETF style split config/state trees (e.g. ietf-interfaces).
> (2) IETF NMDA style combined config/state trees (i.e. where all IETF 
> modules are heading to).
> (3) OpenConfig style modules with the config/state containers 
> immediately above the config leaves.
>
> Tooling is likely to be optimized to work with these model structures, 
> but the TE modules do not fit into any of the three styles above.  
> They are a yet another OpenConfig-like style, but that is different 
> enough that tooling that is designed to work with OpenConfig style 
> YANG modules would likely not work with the TE YANG modules.
>
> Specifically, for clients that expecting to work with an OpenConfig 
> style YANG module, then if it knows the path to config leaf X, then 
> they would expect the applied config value to be available at the path 
> "../state/X".  But, this doesn't hold for the structure being used in 
> the TE YANG models.
>
> I believe strongly that the models being produced by organizations 
> should be structures in a consistent way, hence why I think that the 
> published standard version of the TE YANG modules should immediately 
> align to the NMDA style.
Agreed.
Here is the OPS and RTG AD message: 
https://www.ietf.org/mail-archive/web/netmod/current/msg18252.html

I understand that the I2RS topology YANG modules will be improved to the 
NMDA style.

Regards, Benoit
>
> Thanks,
> Rob
>
>
> On 22/06/2017 04:16, Xufeng Liu wrote:
>> 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:[e-mail]]
>>> Sent: Wednesday, June 21, 2017 5:34 AM
>>> To: Xufeng Liu <[e-mail]>; Mahesh Jethanandani
>>> <[e-mail]>; [e-mail]
>>> Cc: [e-mail]; [e-mail]; 
>>> [e-mail]
>>> 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:[e-mail]]
>>>>> Sent: Wednesday, May 24, 2017 11:44 AM
>>>>> To: [e-mail]
>>>>> Cc: [e-mail]; [e-mail];
>>>>> [e-mail]
>>>>> 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
>
> .
>