YANG Doctor review of draft-ietf-pim-igmp-mld-snooping-yang-05 (by Reshad Rahman)

This is my YD review of -05, in June 2018 I had done an early review of -03.

1 module defined in this draft:
-	ietf-igmp-mld-snooping@2018-10-11.yang

Errors are shown at https://datatracker.ietf.org/doc/draft-ietf-pim-igmp-mld-snooping-yang/ but that seems to be tools related (yang modules can’t be found)

All major issues raised in previous review have been addressed.

Main issues/questions:
-	igmp-snooping-instances (mld- also) are top level containers, I believe they should be under rt:control-plane-protocol (RFC8349). I should have raised this in the previous review.
-	Should there be a dependency on draft-ietf-pim-igmp-mld-yang, e.g. should we allow IGMP/MLD snooping configuration only if IGMP/MLD is enabled (leaf “enable”) or supported (feature-mld and feature-igmp)? E.g. I don’t think it makes sense to configure igmp-snooping if igmp is not supported.

Minor comments, suggestions and nits:
-	Section 2, I don’t see the point of last sentence “This document provides freedom….”.
-	Section 2.1 has a reference to draft-dsdt-nmda-guidelines which has expired
-	YANG indentation is off, please correct
-	Comment “replace with IANA namespace” applies to what line?
-	In YANG module, s/Refrence/Reference/ (1 instance)
-	Leaf “type” in snooping-instances, why not have an identity for type (l2vpn and bridge only for now). Right now there’s an enum which is defined twice (for mld and igmp)
-	Similar comment as above for leaf “host-filter-mode”
-	Send-query: s/topo/topology/, s/param/parameter/
-	For all feature definitions, add references (RFC + relevant sections)
-	Leaf exclude-lite, in description should there an explanation or reference of what exclude lite means?
-	Grouping statistics-sent-received:
1.	remove -sent-received from name
2.	Description of counters, some have messages and some don’t
3.	For all counters for messages, add a reference for the messages
4.	Counter names, e.g. “pim” is too short, it should be something along the lines of num-pim-messages or num-pim.
5.	In pim description s/pim/PIM/
-	RPCs clear-xxx-snooping-groups, rename to clear-xxx-snooping-cache (as per description)?
-	Security Considerations, you should also mention the xxx-snooping-instance leaf nodes under vlan and l2vpn
-	Examples (Appendix A)
1.	Indentation is off, please correct
2.	Were the examples validated with a tool? You can use yanglib
3.	bridge-mrouter-interface should be “eth1/1” instead of “1/1”. Likewise for interface, bridge-outgoing-interface
4.	The example is for bridge, there should be an example for l2vpn also
-	Discrepancy in affiliation of Mahesh (1st page v/s author’s addresses).