Early Review of draft-ietf-pim-yang-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
Authors Xufeng Liu, Pete McAllister, Anish Peter, Mahesh Sivakumar, Yisong Liu, fangwei hu
Draft last updated 2017-03-17
Completed reviews Yangdoctors Early review of -00 by Dean Bogdanović (diff)
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)
Assignment Reviewer Dean Bogdanović 
State Completed
Review review-ietf-pim-yang-00-yangdoctors-early-bogdanovic-2017-03-17
Reviewed rev. 00 (document currently at 17)
Review result Ready with Issues
Review completed: 2017-03-17



As this is part of the early YANG doctor process, let me copy the YANG 
doctors, WG chairs, and ADs (as described at 

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