Re: [babel] Yangdoctors early review of draft-ietf-babel-yang-model-03

[Mahesh Jethanandani <[e-mail]>]

Hi Radek,

Thank you first of all for your thorough review. Please see comments inline.

> On Oct 14, 2019, at 7:01 AM, Radek Krejčí via Datatracker <[e-mail]> wrote:
> 
> Reviewer: Radek Krejčí
> Review result: Ready with Nits
> 
> There is a single module in the draft: [e-mail].
> 
> There are 2 warnings reported by pyang 2.0.2, but both warnings seem to be
> false.
> 
> [e-mail]:53: warning: RFC 8407: 3.1: The IETF Trust Copyright
> statement seems to be missing (see pyang --ietf-help for details).
> [e-mail]:208: warning: the module seems to use RFC 2119
> keywords, but the required text from RFC 8174 is not found (see pyang
> --ietf-help for details).

I have addressed them as part of my discussion with Martin.

> 
> The draft is well written with an informative overview of the YANG module
> sections. Maybe some set of example configuration data (some Appendix) would be
> useful.

I am not sure of the ask. Could you elaborate?

> 
> Regarding the YANG module, I have just a few notes:
> 
> - In multiple description texts, nodes are referenced by a path in format with
> dashes (e.g. babel-mac-default-apply). This format is confusing especially when
> some element in the path contains dash(es). Why the common format with slashes
> is not used?

Ok. I can see that the text can be confusing. The text is from the information model (I-d.ietf-babel-information-model), where the name of the node just happens to follow the path in the YANG model. I can modify the text to use the node names in the YANG model itself.

> 
> - Description and reference statements in containers and lists are placed as
> the last statement (after children nodes). To increase readability and allow
> people to decide if the content of the container/list is interesting for
> detailed view, please move the descriptions / references to the beginning.

Done.

> 
> - grouping routes
>  - just think if the grouping is reusable - in the module itself, the grouping
>  is used just once and needless creation of groupings decrease module
>  readability

It is a grouping so that it can be used with routing policy module.

> 
> - routes/routes/received-metric
>  -  ... indicate a the route ... -> indicate the route …

Fixed

> 
> - babel/interfaces/metric-algorithm
>  - the text "The value MUST be one of those listed in
>  'metric-comp-algorithm'." in description just duplicates the type's
>  information (identityref with metric-comp-algorithms base).

How about if it says “The value MUST be one of those listed in ‘metric-comp-algorithm’ typedef.”?

> 
> - babel/interfaces/mac-key-sets
>  - what does the babel-mac-key-sets in description refer to?

It now says:

            "List of references to the mac entries that apply
             to this interface. When an interface instance is
             created, all mac instances with default-apply 'true'
             will be included in this list.”


> 
> - babel/interfaces/stats/reset
>  - I'm confused, is it per-interface reset (as placed in the module) or
>  system-wide interfaces stats reset (as information model defines it?). If it
>  is the system-wide reset, why tha action cannot be placed in babel container?

The action statement defines an operation connected to a specific container. What would it mean to have an action statement on the whole babel container? Reset everything?

Even though the information model calls for a system-wide reset of stats, the model defines it at a per-interface level. It can be called recursively for every interface instance to realize a system-wide reset of stats.

> 
> - babel/mac/test/output/resulting-hash
>  - according to the content, it seems to me that the description is actually
>  the missing description of the test action.

> 
> - babel/dtls/test/output
>  - seems to me more as a description of the action itself (where the
>  description is missing)
> 

Fixed.

Thanks.

> 

Mahesh Jethanandani
[e-mail]