Early Review of draft-ietf-pim-yang-00
review-ietf-pim-yang-00-yangdoctors-early-bogdanovic-2017-03-17-00

Request Review of draft-ietf-pim-yang-00
Requested rev. 00 (document currently at 17)
Type Early Review
Team YANG Doctors (yangdoctors)
Deadline 2017-03-17
Requested 2017-03-17
Requested by Mehmet Ersue
Other Reviews Yangdoctors Last Call review of -12 by Jürgen Schönwälder (diff)
Rtgdir Telechat review of -12 by Adrian Farrel (diff)
Opsdir Last Call review of -12 by Jürgen Schönwälder (diff)
Genart Last Call review of -12 by Roni Even (diff)
Secdir Last Call review of -12 by Melinda Shore (diff)
Review State Completed
Reviewer Dean Bogdanović
Review review-ietf-pim-yang-00-yangdoctors-early-bogdanovic-2017-03-17
Posted at https://mailarchive.ietf.org/arch/msg/yang-doctors/oTSApawpMPULwIZgkWt9Fwf9IDI
Reviewed rev. 00 (document currently at 17)
Review result Ready with Issues
Draft last updated 2017-03-17
Review completed: 2017-03-17

Review
review-ietf-pim-yang-00-yangdoctors-early-bogdanovic-2017-03-17

Dean,

Thanks.
As this is part of the early YANG doctor process, let me copy the YANG 
doctors, WG chairs, and ADs (as described at 
https://www.ietf.org/iesg/directorate/yang-doctors.html)

Regards, Benoit

> Authors,
>
> I don’t have deep knowledge of PIM, so if some protocol specifics 
> haven’t been modeled right, I missed them. For application comparison, 
> was looking at  Juniper PIM configuration. The modules are using 
> draft-ietf-netmod-routing-cfg as base, and follows the 
> routing-instance-centric model, hence didn’t have problems mapping it 
> to Junos PIM config style. The model design by using base module and 
> build for each specific variant a separate module is a good approach, 
> as it enables simpler application of the modules by vendors and users.
>
> Throughout the draft authors are using abbreviations (many of them not 
> widely known) and the terminology section is not complete for PIM.  It 
> would be good to write them out when first time used in the text
>
> example,
>
> the configuration for PIM-SM that is not relevant for an SSM-only implementation is collected in an ASM container.
>
> Same thing is in the YANG module descriptions
>
> enum new-dr {
>             description
>               "A new DR was elected on the connected network.";
>           }
>           enum new-df {
>             description
>               "A new DF was elected on the connected network.";
>           }
>
> DR and DF should be spelled out in the description
>
> Make the descriptions in the code consistent, like in following example
> typedef pim-mode {
>         type enumeration {
>           enum none {
>             description
>               "PIM is not operating.";
>           }
>           enum ssm {
>             description
>               "Source-Specific Multicast (SSM) with PIM Sparse Mode.";
>           }
>           enum asm {
>             description
>              "Any Source Multicast (ASM) with PIM Sparse Mode.";
>           }
>
>
> Why are the PIM related RFC not listed in the introduction section, as 
> there are clearly relations between the model and PIM related RFCs
>
> In chapter 2.2, why are you stating vendors will augment with required 
> restrictions, but features might be added
>
> It is expected that vendors
>     will augment the model with any specific restrictions that might be
>     required.  Vendors may also extend the features list with proprietary
>     extensions.
>
> It is expected that vendors will augment the model with any specific 
> extensions and restrictions needed to adapt it to their vendor 
> specific implementation.
>
> In chapter 3.1 bullet 2, the chapter finishes with statement
>
> which does not make sense for PIM.
>
> It would be nice to explain why does it not make sense for PIM. Why is 
> there only 1 instances of PIM per VRF
>
> From YANG perspective, the authors followed recommendations in the 
> draft-ietf-netmod-rfc6087-bis-08
>
> Hope this helps
>
> Dean
>
>
>
> _______________________________________________
> netmod mailing list
> netmod@ietf.org
> https://www.ietf.org/mailman/listinfo/netmod