Re: Yangdoctors last call review of draft-ietf-rtgwg-policy-model-29

[tom petch <[e-mail]>]

From: rtgwg <[e-mail]> on behalf of Yingzhen Qu <[e-mail]>
Sent: 30 July 2021 06:18

Hi Mahesh,

Thank you for your review. We just submitted version -30 and addressed your comments. Please see detailed answer below.

<tp>
I agree with you that it is a bit late to create IANA maintained modules.  It is the sort of thing that the rtgwg or lsr might have done years ago and then everyone could have used them, as NETMOD created RFC6991, but to do it now would just tread on the toes of those who have done it for themselves and confuse those yet to come,

Tom Petch

Thanks,
Yingzhen

> On Jul 18, 2021, at 9:07 PM, Mahesh Jethanandani via Datatracker <[e-mail]> wrote:
>
> Reviewer: Mahesh Jethanandani
> Review result: Almost Ready
>
> This review is looking at the draft from a YANG perspective. With that said, I
> have marked it as almost ready, because of some of the points discussed below.
>
> Summary:
>
> This document defines a YANG data model for configuring and managing routing
> policies in a vendor-neutral way. The model provides a generic routing policy
> framework which can be extended for specific routing protocols using the YANG
> 'augment' mechanism.
>
> The description in the document and in the model is well written and easy to
> understand.
>
> Nits
>
> - Repeat of parent as a prefix. It is not necessary to repeat the parent name
> in child attributes, e.g. routing-policy -> policy-definitions ->
> policy-definition. This can be shortened to routing-policy -> definitions >
> definition.
[Yingzhen]: We didn’t change this unless you have a strong opinion about it.
>
> s/domian/domain/
> s/suspectable/susceptible/
[Yingzhen]: thanks for catching these. Fixed.
>
> - Consistency in how the reference statements are written. Most of the
> reference statements start on a new line, except for a few places where they do
> not.
[Yingzhen]: fixed.
>
> Comments:
>
> Section 1 - Introduction:
>
> The document does not mention whether the model is YANG a 1.1 model. It
> includes RFC 7950 which would imply a 1.1 module, and the YANG model has a
> yang-version is 1.1., but it would be nice to state it explicitly.
[Yingzhen]: My understanding is RFC 7950 means YANG 1.1. If this has to be explicitly stated, please let us know.
>
> Section 7.2
>
> - Consider moving identity 'metric-type' and 'route-level' and their derived
> identities into an IANA maintained module, e.g. 'iana-policy-types', so that
> module can be updated separately from the rest of the module (much more easily).
[Yingzhen]: I think it’s a bit too late to make this change unless it’s really necessary.
>
> - The leaf 'mode' is defined as an enumeration with enum values of ipv4 and
> ipv6. The description however says:
>
>              "Indicates the mode of the prefix set, in terms of
>               which address families (IPv4, IPv6, or both) are
>               present."
>
> How does a user indicate both?
[Yingzhen]: I removed “both”.

> The model uses a lot of groupings, most of them used only once in the model. It
> does identify that prefix sets, neighbor sets and tag sets as reusable
> groupings. Is that the case for the rest of the groupings? Unless these
> groupings are meant for use by other models, they should be folded into the
> main container.
>
[Yingzhen]: we removed all the groupings not necessary.

> Please drop <mailto:....> and just keep the e-mail address. That tag works only
> when embedded within a HTML document. (This is a leftover item from the early
> review, and if there was a discussion about it already, just ignore it).
[Yingzhen]: I’ll leave this to RFC editor.
>
> Section 8 - Security Considerations:
>
> The security considerations section lists /routing-policy as one of the nodes
> as being sensitive from a write operation perspective. That would imply the
> whole module is sensitive. It however, goes onto identifying specific nodes
> within the module. Not clear if the whole module was intended to be identified
> or specific nodes.
>
> Similarly a sub-tree of the module is identified in
> /routing-policy/policy-definitions.
[Yingzhen]: removed these two sub-tree.
>
> If the idea of having a node without a description is to identify
> (sub-)sections of the module where the nodes occur (the path already indicates
> so), some words to that effect might help. E.g. In the
> /routing-policy/policy-definitions section of the module the following nodes
> are considered vulnerable.
>
> The last paragraph is a fairly generic, and seems to repeat what is already
> identified above. Moreover, it is not clear what is meant by "related model
> carries potential risk". What is "related model”?
[Yingzhen]: modified the text. Hope it’s clear now.
>
> General
>
> A pyang compilation of the model with —ietf and —lint option was clean. A
> validation of the model and the example in Appendix B also succeeded. Thank you
> for providing an example.
>
> An idnits run of the draft was generally clean.
>
>

_______________________________________________
rtgwg mailing list
[e-mail]
https://www.ietf.org/mailman/listinfo/rtgwg