Hi Ebben, Thanks much for your review and comments. We have uploaded a new revision ver-11 that addresses your comments. The diff is at https://tools.ietf.org/rfcdiff?url2=draft-ietf-teas-yang-rsvp-11.txt Please look inline for on resolution details and do let us know if you have further comments. On 5/3/19, 8:11 PM, "Teas on behalf of Ebben Aries via Datatracker" <teas-bounces@xxxxxxxx on behalf of noreply@xxxxxxxx> wrote: Reviewer: Ebben Aries Review result: On the Right Track 2 modules in this draft: - ietf-rsvp@xxxxxxxxxxxxxxx - ietf-rsvp-extended@xxxxxxxxxxxxxxx No YANG compiler errors or warnings (pyang 2.0, yanglint 1.1.16, confdc 6.6.1) Module ietf-rsvp@xxxxxxxxxxxxxxx: - Remove WG Chairs from contact information per https://tools.ietf.org/html/rfc8407#appendix-B [TS]: done. - Use of 'state' containers. It is stated in Section 2.3 that 'Derived state data is contained under a "state" container...'. My only comments here are: a) Should use caution when using 'state' containers in NMDA compliant modules. Rob put together a nice doc here that I won't reiterate: https://github.com/netmod-wg/FAQ/wiki/NMDA-Modelling-FAQ Using such nomenclature locks you into r/o nodes only. [TS]: removed reference to "state" container for consistency. b) In some cases, the hierarchy is a bit redundant (statistics/state). Other NMDA compliant modules will not introduce another 'state' hierarchy for instance (see ietf-interfaces) [TS]: removed 'state' container. - leaf 'packet-len' is abbreviated while most other leafs are not [TS]: fixed. - authentication-key is of type string. Is this leaf mean to be clear-text? There is nothing associated with this type that would imply special treatment in handling. [TS]: Yes, this is entered in clear text. This is analogous with the 'keystring' defined in rfc8177. We discussed this amongst authors, and we think there may be a gap in YANG/NMDA on retrieving applied secret string configuration (e.g. passwords). I'm not sure if this was discussed before for other protocols or if there are any extensions in NMDA or YANG to allow retrieving/displaying it e.g. "****" instead of the clear text that was entered. - crypto-algorithm: Are all identities from ietf-key-chain supported? [TS]: We are not adding any restriction on the support of any of the algorithms defined in ietf-key-chain standard model. However, a vendor model can apply a strict check using a deviation. - Pluralization on counters: e.g. 'in-error' vs. 'in-errors'. Maintain consistency with other modules (see ietf-interfaces) [TS]: done. - Normative references are missing for some of the modules imported. These must be added per https://tools.ietf.org/html/rfc8407#section-3.9 [TS]: done. Module ietf-rsvp-extended@xxxxxxxxxxxxxxx: - Remove WG Chairs from contact information per https://tools.ietf.org/html/rfc8407#appendix-B [TS]: done. - Use of 'state' containers. It is stated in Section 2.3 that 'Derived state data is contained under a "state" container...'. My only comments here are: a) Should use caution when using 'state' containers in NMDA compliant modules. Rob put together a nice doc here that I won't reiterate: https://github.com/netmod-wg/FAQ/wiki/NMDA-Modelling-FAQ Using such nomenclature locks you into r/o nodes only. b) In some cases, the hierarchy is a bit redundant (statistics/state). Other NMDA compliant modules will not introduce another 'state' hierarchy for instance (see ietf-interfaces) - Pluralization on counters: e.g. 'in-error' vs. 'in-errors'. Maintain consistency with other modules (see ietf-interfaces) - 9 features are defined with no 'if-feature' statements. Where are these feature conditions meant to be used? [TS]: The defined features are for extended RSVP feature support that other modules (outside the scope of this document) may choose to use to control access to specific data nodes. - Normative references are missing for some of the modules imported. These must be added per https://tools.ietf.org/html/rfc8407#section-3.9 [TS]: Added. General comments on the draft/modules: - The state container and list key designs appear very familiar to that of OpenConfig modules however not consistent with IETF module design. Consistency is key from each producing entity (e.g. IETF in this case) [TS]: addressed and removed instances of 'state' container. - The draft mentions RPCs and Notifications however none are defined within the modules [TS]: We added two new RPCs to the draft: - one to clear rsvp session (single or all) - one to clear rsvp neighbor (single or all) - Section 2.3: Has examples of RPCs and Notifications that are non-existant in the modules [TS]: We added a new section for notifications and suggested leveraging I-D.ietf-netconf-subscribed-notifications] and I-D.ietf-netconf-yang-push in general to register on specific data nodes. - Abstract: s/RVSP/RSVP/ - Abstract: s/remote procedural/remote procedure/ - Section 2: s/extensiion/extension/ - Section 4: Update the security considerations section to adhere to https://tools.ietf.org/html/rfc8407#section-3.7 and https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines [TS]: Fixed above. - Various (missing) wording/pluralization cleanup throughout that I'll defer to the RFC Editor. A once over proofread should solve this. [TS]: in progress. Regards, Tarek _______________________________________________ Teas mailing list Teas@xxxxxxxx https://www.ietf.org/mailman/listinfo/teas
