Hi Yingzhen, Thanks for addressing my comments.
Cheers. > On Jul 29, 2021, at 10:18 PM, Yingzhen Qu <[email protected]> wrote: > > 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 >> <[email protected]> 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. Mahesh Jethanandani [email protected]
_______________________________________________ rtgwg mailing list [email protected] https://www.ietf.org/mailman/listinfo/rtgwg
