Hi Mahesh, Thank you for your review. We just submitted version -30 and addressed your comments. Please see detailed answer below. Thanks, Yingzhen > On Jul 18, 2021, at 9:07 PM, Mahesh Jethanandani via Datatracker <noreply@xxxxxxxx> 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. > > -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
