Re: Yangdoctors early review of draft-ietf-rtgwg-policy-model-09

[Mahesh Jethanandani <[e-mail]>]

[Trimming the list to open issues]

Hi Yingzhen,

> On May 23, 2020, at 1:21 PM, Yingzhen Qu <[e-mail]> wrote:
> 
> Hi Mahesh,
> 
> Thanks for the review, very much appreciated.
> 
> I submitted -10 version to address your comments: https://datatracker.ietf.org/doc/draft-ietf-rtgwg-policy-model/ <https://datatracker.ietf.org/doc/draft-ietf-rtgwg-policy-model/> 
> 
> Note: There are YANG validation errors because yanglint can't find model "ietf-if-extensions", not sure why.
> yanglint SO 1.6.7: yanglint --verbose -p {rfclib} -p {draftlib} -p {tmplib} {model} -i:
> err : Data model "ietf-if-extensions" not found.
> err : Importing "ietf-if-extensions" module into "ietf-routing-policy" failed.
> err : Module "ietf-routing-policy" parsing failed.

ietf-if-extensions is part of the draft-ietf-netmod-inft-ext-yang draft, which is still a draft. It should ideally be in the {draftlib} path variable. I would suggest a open an issue with the tools team to have them track it.

I am assuming of course that you are able to validate the model locally on your machine.
…

>    The document includes a section on Terminology and Notation. However it does
>    not define what is a "routing policy" or provide a reference to it.
> [YQ]: I'm not sure there is a standard definition of "routing policy". I'll do some
> research on this and see whether we should add it to the terminology.

Looking at how the draft describes it, how about something like this?

“A routing policy defines whether a routing protocol imports a route into the routing table, modifies it, or exports a route from the routing table."

…

>    Section 10.1 - Routing policy model
> 
>    The model uses a lot of groupings, most of them used only once in the model.
>    That makes the model difficult to understand. Unless these groupings are meant
>    for use by other models, they should be folded into the main container. Take
>    this grouping for example:
> 
>         grouping policy-definitions {
>           description
>             "This grouping provides policy definitions";
> 
>           leaf name {
>             type string;
>             description
>               "Name of the top-level policy definition -- this name
>               is used in references to the current policy";
>           }
>         }
> 
>    For one the description of the grouping is not correct. Secondly, it is used
>    only once in the model and that too to define a key of a list.
> 
> [YQ]: This model was initiated by OpenConfig, we've removed some groupings 
> Since this become an IETF draft. I removed "policy-definitions" and "policy-statements" 
> groups.

The general rule is that if the grouping is used only once in the model, and not meant for use outside the module, that it should not be a grouping.

For example, ‘grouping neighbor-set’ is used only once in the model, and I doubt it is meant for use outside of this module. That grouping should be collapsed into the model where it gets used. Same for ‘grouping tag-set’. These are just two examples. There are many more.

> 
>    The YANG model imports multiple models from other drafts. It provides normative
>    references to the imported models in Section 2.2. (A nit here is that it
>    missing the cross-reference in the table for if-l3-vlan),

A nit. if-l3-vlan is still missing the cross-reference (and the hyperlink) to the draft. Also why idnits is giving you an error at the bottom.

> but does not provide
>    references to them in the model. See Section 3.9 of RFC 8407. Please add
>    something like this in the model.
> 
>    import ietf-inet-types {
>      prefix "inet";
>      reference
>        "RFC 6991: Common YANG Data Types.";
>    }
> [YQ]: fixed.
> 
>    Please drop <mailto:....> and just keep the e-mail address. That tag works only
>    when embedded within a HTML document.
> [YQ]: do you mean the "editor" in the model? 

I meant to say you should drop the word “mailto:" from the list of e-mail addresses of the editors. That tag works inside a HTML document, not inside a YANG model.

> 
>    A pyang compilation of the model with —ietf and —lint option was clean.
>    However, there seems to be an issue with the example in Section 11. Instead of
>    actually including an example, it is showing the path to what should have been
>    the example. I was therefore not able to validate the example against the model.
> [YQ]: I can't find this example in OpenConfig Github anymore, so removed this section.

Hmm. That leaves the model without any examples, and examples are important.

I was able to find the example in an earlier version of your own draft (01). I would suggest that you take that example and modify it to make it compatible with the latest version of the model. If you need help, let me know.

> 
>      Checking references for intended status: Proposed Standard
>      ----------------------------------------------------------------------------
> 
>         (See RFCs 3967 and 4897 for information about using normative references
>         to lower-maturity documents in RFCs)
> 
>      == Unused Reference: 'I-D.ietf-netmod-sub-intf-vlan-model' is defined on
>         line 1620, but no explicit reference was found in the text

The reason you are getting this error is because you are lacking a normative reference to I-D.ietf-netmod-sub-intf-vlan-model in the document. Goes back to the nit I point up earlier.

I would suggest that once you have fixed all the issues in the draft, that you re-run idnits locally on the machine.

> 
>      -- No information found for draft-ietf-netmod-intf-ext-yang - is the name
>         correct?
> 
>      -- Possible downref: Normative reference to a draft: ref.
>         'I-D.ietf-netmod-intf-ext-yang'
> 
>      -- No information found for draft-ietf-netmod-sub-intf-vlan-model - is the
>         name correct?
> 
>      -- Possible downref: Normative reference to a draft: ref.
>         'I-D.ietf-netmod-sub-intf-vlan-model'
> 
> [YQ]: there is dependency on both 'I-D.ietf-netmod-intf-ext-yang' and 'I-D.ietf-netmod-sub-intf-vlan-model'.
> 
>         Summary: 0 errors (**), 0 flaws (~~), 9 warnings (==), 5 comments (--).