Mahesh Jethanandani has entered the following ballot position for draft-ietf-pce-pcep-yang-26: Discuss
When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.) Please refer to https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/ for more information about how to handle DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-pce-pcep-yang/ ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- Section 1, paragraph 3 > This document contains a specification of the PCEP YANG module, > "ietf-pcep" which provides the PCEP [RFC5440] data model. Further, > this document also includes the PCEP statistics YANG module "ietf- > pcep-stats" which provides statistics, counters and telemetry data. I realize that I had provided Early Reveiw from a YANG doctors point of view for this module. And if the module had some of the same structure that I had reviewed, I must apologize for not catching this earlier. It is fairly unusual to split the statistics into a separate module away from the rest of the module. It is also unusual to have the separate module augment the main module in the same document. Could this not have been achieved simply by having a container called 'statistics' under 'peer' and 'session'? What is achieved by having a complete separate namespace for statistics. Also, why use 'rpcs' and not the 'action' statement to clear the statistics? See Section 7.15.3 of RFC 7950 for an example and why 'action' should be used instead of 'rpcs'. Hint, rpcs have no node in the datastores tied to them. That context has to be somehow brought in. Also, how would one clear stats for a particular peer or session? For all these reasons, I feel there must have been a strong motivation for coming up with a separate module for statistics, and that motivation along with the use of rpcs should be clearly described, otherwise it would be better for the module to be collapsed into a single module, and the rpcs converted to actions. Section 4, paragraph 1 > The PCEP YANG module defined in this document has all the common > building blocks for the PCEP protocol. For the rest of the YANG module, I would like to thank Jan Lindblad for providing YANG doctor review on the document. But I have not seen any response to the review, or how it will be addressed. Holding a DISCUSS to make sure those comments are addressed. ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- "Abstract", paragraph 1 > This document defines a YANG data model for the management of the > Path Computation Element communications Protocol (PCEP) for > communications between a Path Computation Client (PCC) and a Path > Computation Element (PCE), or between two PCEs. The data model > includes configuration and state data. Maybe it is just me, but most YANG data models today have both configuration and state data, even if the state data reflects the state of any of the conifg leafs. As such, the last sentence seems redundant to me. Section 9, paragraph 1 > The YANG modules defined in this document are designed to be accessed > via a network management protocol such as NETCONF [RFC6241] or > RESTCONF [RFC8040]. The lowest NETCONF layer is the secure transport > layer and the mandatory-to-implement secure transport is SSH > [RFC6242]. The lowest RESTCONF layer is HTTPS, and the mandatory-to- > implement secure transport is TLS [RFC8446] > The NETCONF access control model [RFC8341] provides the means to > restrict access for particular NETCONF or RESTCONF users to a pre- > configured subset of all available NETCONF or RESTCONF protocol > operations and content. Please update this template to match the recent changes in the template as described in rfc8407bis Section 3.7.1. The IANA review of this document seems to not have concluded yet. Check whether the "Implementation Status" section is reasonable. If there is no implementation status to report, should this section be removed? DOWNREF [I-D.ietf-teas-yang-te] from this Proposed Standard to draft-ietf-teas-yang-te of unknown standards level. (For IESG discussion. It seems this DOWNREF was not mentioned in the Last Call and also seems to not appear in the DOWNREF registry.) Found terminology that should be reviewed for inclusivity; see https://www.rfc-editor.org/part2/#inclusive_language for background and more guidance: * Term "native"; alternatives might be "built-in", "fundamental", "ingrained", "intrinsic", "original" Found IP block or address not inside RFC5737/RFC3849 example ranges: "0.0.0.0". ------------------------------------------------------------------------------- NIT ------------------------------------------------------------------------------- All comments below are about very minor potential issues that you may choose to address in some way - or ignore - as you see fit. Some were flagged by automated tools (via https://github.com/larseggert/ietf-reviewtool), so there will likely be some false positives. There is no need to let me know what you did with these suggestions. Section 1, paragraph 3 > This document defines a YANG [RFC7950] data model for the management > of PCEP speakers. It is important to establish a common data model > for how PCEP speakers are identified, configured, and monitored. The > data model includes configuration data and state data. This is a YANG 1.1 model by virtue of the fact you are referencing RFC 7950. You might want to just say that it is a YANG 1.1 module. Document references draft-ietf-netconf-tls-client-server, but that has been published as RFC9645. Document references draft-ietf-pce-pcep-srv6-yang-05, but -06 is the latest available revision. Reference [RFC5246] to RFC5246, which was obsoleted by RFC8446 (this may be on purpose). "Abstract", paragraph 1 > the state data reflects the state of any of the conifg leafs. As such, the la > ^^^^^^^^^ Consider simply using "of" instead. _______________________________________________ Pce mailing list -- pce@ietf.org To unsubscribe send an email to pce-le...@ietf.org
