Early Review of draft-ietf-pim-msdp-yang-13
review-ietf-pim-msdp-yang-01-yangdoctors-early-rahman-2018-01-12-01
Request | Review of | draft-ietf-pim-msdp-yang-01 |
---|---|---|
Requested revision | 01 (document currently at 18) | |
Type | Early Review | |
Team | YANG Doctors (yangdoctors) | |
Deadline | 2017-12-13 | |
Requested | 2017-11-22 | |
Requested by | Mehmet Ersue | |
Authors | Xufeng Liu , Zheng Zhang , Anish Peter , Mahesh Sivakumar , Feng Guo , Pete McAllister | |
I-D last updated | 2020-02-12 | |
Completed reviews |
Yangdoctors Early review of -13
by Reshad Rahman
(diff)
Yangdoctors Last Call review of -16 by Reshad Rahman (diff) Yangdoctors Last Call review of -12 by Reshad Rahman (diff) Rtgdir Last Call review of -08 by Yingzhen Qu (diff) Secdir Last Call review of -12 by Vincent Roca (diff) |
|
Comments |
Requested by YANG secretary on behalf of PIM chairs. |
|
Assignment | Reviewer | Reshad Rahman |
State | Completed | |
Request | Early review on draft-ietf-pim-msdp-yang by YANG Doctors Assigned | |
Reviewed revision | 13 (document currently at 18) | |
Result | Ready w/issues | |
Completed | 2020-02-12 |
review-ietf-pim-msdp-yang-01-yangdoctors-early-rahman-2018-01-12-01
Thank you Sandy for addressing the last round of comments. I didn’t re-review the whole document, just made sure that comments were addressed. They have all been addressed for the exception of examples. There are still no examples, I believe you should have a config example, an operational example (with sa-cache) and action examples (clear-peer and clear-sa-cache) and all examples have to match. Regards, Reshad. =============================================================== YANG Doctor review of draft-ietf-pim-msdp-yang-12 by Reshad Rahman 1 module in this draft: - ietf-msdp@2020-01-24.yang No YANG validation errors or warnings Thank you for addressing comments which were provided on rev-01 of the document. - Page 21, the YANG module has the augmentation of “/rt:…/rt:control-plane-protocol” but there is a missing when statement. This means that on a device which supports this YANG model, the “msdp” container node will appear in all instances of “rt:control-plane-protocol”, even the non-MSDP ones. If you need an example of how to fix this with when statement, please take a look at OSPF and BFD YANG models. - There are no examples. Just a couple of simple XML examples would help a lot. - There are IMO still too many (14?) features for a fairly straight-forward YANG model. An explanation for this is provided in section 3.1, but this does not comply with 4.17 of RFC8407: The set of YANG features defined in a module should be considered carefully. Very fine granular features increase interoperability complexity and should be avoided. A likely misuse of the feature mechanism is the tagging of individual leafs (e.g., counters) with separate features. - Page 12, the if-feature was taken out of password (to address a comment regarding duplicate if-feature), but I believe the remaining one should be moved up from case key-chain to container authentication. - There is an as-number leaf in the YANG model, but no such thing in RFC3618. Do we need a reference here? - Page 24, RPC clear-sa-cache has source-addr using type ipv4-multi-cast-source-address. But in the operational model (container sa-cache), source-addr is a union of either * or ipv4-address. Why the difference? Same question, wrt inconsistency, for leaf group (ipv4-multicast-group-address in RPC and ipv4-address in operational model). - Page 22, rp-address is of type ip-address, should that be ipv4-address just like rpf-peer? Or am I misunderstanding this? - Many of the descriptions are still very terse, e.g. up-time, expire. - I’m not an MSDP expert, but I believe adding, where appropriate in the YANG module, more references to the appropriate sections of RFC3618 or other PIM RFCs would improve the document. e.g. this could help for RPF-related nodes. - Section 6: s/one new URIs/one new URI/