Re: [pim] AD Review of draft-ietf-pim-igmp-mld-snooping-yang-09

[Alvaro Retana <[e-mail]>]

On May 2, 2020 at 8:39:48 AM, Hongji Zhao wrote:


Hongji:


Hi!  How are you?


> We have already addressed all your comments and uploaded the latest version
> as draft-ietf-pim-igmp-mld-snooping-yang-10.

First, to answer your question about rfc4541:  Yes, it must be a
Normative reference as that is where snooping is defined.  We will
take care of the DownRef registry as we get the document approved.

I need this reference in the right place so that the IETF Last Call
call its out.

Along with that, I have some comments on -10 (see below).


Thanks!

Alvaro.



...
134	1.2. Tree Diagrams

136	Tree diagrams used in this document follow the notation defined in

138	[RFC8340].

[nit] There's an extra line above...


...
180	+----------+-----------------------+---------------------------------+

182	| l2vpn    | ietf-l2vpn            | [draft-ietf-bess-l2vpn-yang]    |
183	+----------+-----------------------+---------------------------------+

185	| dot1q    | ieee802-dot1q-bridge  | [dot1Qcp]                       |

187	+----------+-----------------------+---------------------------------+

189	      Table 1: Prefixes and Corresponding YANG Modules

[nit] This table seems to also have extra lines.


191	2. Design of Data Model

[major] It may be confusing to other readers the reason for not
augmenting, or even requiring, the IGMP/MLD model (rfc8652).  Please
include some text to explain the relationship between snooping and the
IGMP/MLD protocols themselves.  Specifically, the fact that the
switches don't really need to run the protocols.


...
197	In recent years, a number of commercial vendors have introduced products
198	described as "IGMP snooping switches" to the market. These devices do
199	not adhere to the conceptual model that provides the strict separation
200	of functionality between different communications layers in the ISO
201	model, and instead utilize information in the upper level protocol
202	headers as factors to be considered in processing at the lower levels
203	[RFC4541].

[major] This paragraph is a copy from rfc4541...which was written in
2006, so the "in recent years" phrase doesn't really apply anymore.
Please remove it.


205	IGMP Snooping switches utilize IGMP, and could support IGMPv1, IGMPv2,
206	and IGMPv3. IGMP snooping switches may maintain forwarding tables based
207	on either MAC addresses or IP addresses [RFC4541]. MLD Snooping switches
208	utilize MLD, and could support MLDv1 and MLDv2.

[major] This paragraph is a great opportunity to include appropriate
references for all the protocols above.  If they are not Normative
already, any new references can be Informative.

[minor] "IGMP snooping switches may maintain forwarding tables based
on either MAC addresses or IP addresses [RFC4541]."  This sentence
also comes directly from rfc4541.  Do we need it?  What about MLD
Snooping?  Given that the model includes a mac-address for both the
IGMP and MLD instances, it seems to me that we can simply take it out
to avoid confusion.


...
229	2.2. Optional Capabilities

231	This model is designed to represent the capabilities of IGMP and MLD
232	switches with various specifications, including the basic capability
233	subsets of IGMP and MLD Snooping. The main design goals of this document
234	are that the basic capabilities described in the model are supported by
235	any major now-existing implementation, and that the configuration of all
236	implementations meeting the specifications is easy to express through
237	some combination of the optional features in the model and simple vendor
238	augmentations.

[minor] This is not the IGMP/MLD module:

   s/This model is designed to represent the capabilities of IGMP and MLD
   switches with various specifications, including the basic capability
   subsets of IGMP and MLD Snooping./This model is designed to represent the
   basic capability subsets of IGMP and MLD Snooping.


...
258	2.3. Position of Address Family in Hierarchy
...
271	*  The structure is consistent with other YANG data models such as
272	[RFC8344], which uses separate branches for IPv4 and IPv6.

[minor] s/[RFC8344]/[RFC8652]    This would be a more appropriate justification.


...
540	3.3. Using IGMP and MLD Snooping Instances
...
550	It also augments /dot1q:bridges/dot1q:bridge/dot1q:component/
551	dot1q:bridge-vlan/dot1q:vlan to use igmp-snooping-instance. It means
552	IGMP Snooping is enabled in the certain VLAN of the bridge.

[nit] s/in the certain VLAN of the bridge/in the specified VLAN on the bridge


554	     augment /dot1q:bridges/dot1q:bridge:
555	       +--rw igmp-snooping-instance?   igmp-mld-snooping-instance-ref
556	       +--rw mld-snooping-instance?    igmp-mld-snooping-instance-ref
557	     augment /dot1q:bridges/dot1q:bridge/dot1q:component
558	               /dot1q:bridge-vlan/dot1q:vlan:
559	       +--rw igmp-snooping-instance?   igmp-mld-snooping-instance-ref
560	       +--rw mld-snooping-instance?    igmp-mld-snooping-instance-ref

[nit] Add a space between the 2 augmentations...for readability.


...
594	4. IGMP and MLD Snooping YANG Module

596	This module references
597	[RFC2236],[RFC3376],[RFC3810],[RFC4286],[RFC4541],[RFC4604],[RFC4607],
598	[RFC6020],[RFC6241],[RFC6636],[RFC6991],[RFC7950],[RFC8040],[RFC8342],
599	[RFC8343],[RFC8340],[RFC8529],[RFC8652],[dot1Qcp], and [draft-ietf-bess-
600	l2vpn-yang].

[major] RFC2236, RFC3810, RFC4286, RFC4604, RFC4607, RFC6020, RFC6241,
RFC7950, RFC8040, RFC8342. RFC8340 and RFC8652 are not referenced
*inside* the model.   Please don't include them in this sentence.


602	<CODE BEGINS> file [e-mail]
...
1707	<CODE ENDS>
1708	5. Security Considerations

[nit] Add an extra line for readability.


...
1730	/rt:routing/rt:control-plane-protocols

1732	  /rt:control-plane-protocol:/ims:igmp-snooping-instance

1734	/rt:routing/rt:control-plane-protocols

1736	  /rt:control-plane-protocol:/ims:mld-snooping-instance

[minor] The locations can be summarized:

   Under /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol:/

   ims:igmp-snooping-instance

   ims:mld-snooping-instance


Please apply the same format to other entries in this section.



...
1791	6. IANA Considerations
...
1804	Registrant Contact: The IESG.

[major] s/IESG/IETF



...
1826	7.1. Normative References

1828	   [dot1Qcp] Holness, M., "IEEE 802.1Qcp-2018 Bridges and Bridged
1829	             Networks - Amendment: YANG Data Model", 2018.

[major] This reference is not complete.  Let's try this:

   [dot1Qcp] IEEE, "Standard for Local and metropolitan area networks--Bridges
             and Bridged Networks--Amendment 30: YANG Data Model", IEEE Std
             802.1Qcp-2018 (Revision of IEEE Std 802.1Q-2014), September 2018,
             <https://ieeexplore.ieee.org/servlet/opac?punumber=8467505>


...
1872	   [RFC7951] L. Lhotka, "JSON Encoding of Data Modeled with YANG", RFC
1873	             7951, August 2016.

[minor] This reference can be Informative.


...
1920	7.2. Informative References
...
1926	   [RFC4541] M. Christensen, K. Kimball, F. Solensky, "Considerations
1927	             for Internet Group Management Protocol (IGMP) and Multicast
1928	             Listener Discovery (MLD) Snooping Switches", RFC 4541, May
1929	             2006.

[major] This reference must be Normative as that is where snooping is
described.  We will take care of the downref registry after the
document is approved.