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

From: Hongji Zhao <[email-masked]>
Date: Sat, 2 May 2020 12:39:46 +0000

Hi Alvaro,

We have already addressed all your comments and uploaded the latest version as draft-ietf-pim-igmp-mld-snooping-yang-10.
Below are the answers to your questions in your comments. Please check inline.

1)
175           augment /rt:routing/rt:control-plane-protocols/rt:control-plane-protocol:
176             +--rw igmp-snooping-instance {feature-igmp-snooping}?
177             |  +--rw scenario?                            snooping-scenario-type
178             |  +--rw enable?                              boolean
179             |  +--rw forwarding-mode?                     enumeration
180             |  +--rw explicit-tracking?                   boolean
{explicit-tracking}?
181             |  +--rw exclude-lite?                        boolean {exclude-lite}?
182             |  +--rw send-query?                          boolean
183             |  +--rw immediate-leave?                     empty {immediate-leave}?
184             |  +--rw last-member-query-interval?          uint16
185             |  +--rw query-interval?                      uint16
186             |  +--rw query-max-response-time?             uint16

[minor] Why are timer-value-* (rfc8294) not used?  Please consider using them for the variables above and others...
   [Authors] Regarding last-member-query-interval or query-interval, it should not be set infinity or not-set. So timer-value-* is not fit for it.

2)
    leaf igmp-version {
      type uint8 {
        range "1..3";
      }
      default 2;
      description "IGMP version.";
    }
[major] If the previous guess is correct...  What does this version mean?  Given that the default is 2, does it mean that only IGMPv2 packets should be snooped?  I would assume that deployments already supporting IGMPv3 are common...
  [Authors] The version here refers to the igmp version.  By default is 2, it means only IGMPv2 packets could be snooped. In rfc8652, igmp default version is also 2.   If IGMPv3 packets need to be snooped, it should be set 3. So does the MLD.

BR/Hongji

From: Alvaro Retana <[email-masked]>
Sent: Sunday, April 26, 2020 8:11 PM
To: Hongji Zhao <[email-masked]>
Cc: [email-masked]; Stig Venaas <[email-masked]>; [email-masked]
Subject: RE: AD Review of draft-ietf-pim-igmp-mld-snooping-yang-09

Hi!

Yes, as long as there is a plan and progress is being made. :-)

Thanks!

Alvaro.

On April 25, 2020 at 10:06:03 PM, Hongji Zhao ([email-masked]<mailto:[email-masked]>) wrote:
We will finish addressing the comments by the end of May. Is it ok?

---

From: Hongji Zhao <[email-masked]>
Date: Tue, 5 May 2020 03:52:48 +0000

Hi Alvaro,

When RFC 4541 is referenced as Normative reference,  it will raise following Downref error.
** Downref: Normative reference to an Informational RFC: RFC 4541

So I have to set it as informative reference.  Is it ok?  Or it still needs to be set as normative reference?

BR/Hongji

From: Hongji Zhao
Sent: Saturday, May 2, 2020 8:40 PM
To: Alvaro Retana <[email-masked]>
Cc: [email-masked]; Stig Venaas <[email-masked]>; [email-masked]
Subject: RE: AD Review of draft-ietf-pim-igmp-mld-snooping-yang-09

Hi Alvaro,

1)

2)
    leaf igmp-version {
      type uint8 {
        range "1..3";
      }
      default 2;

BR/Hongji

From: Alvaro Retana <[email-masked]<mailto:[email-masked]>>
Sent: Sunday, April 26, 2020 8:11 PM
To: Hongji Zhao <[email-masked]<mailto:[email-masked]>>
Cc: [email-masked]<mailto:[email-masked]>; Stig Venaas <[email-masked]<mailto:[email-masked]>>; [email-masked]<mailto:[email-masked]>
Subject: RE: AD Review of draft-ietf-pim-igmp-mld-snooping-yang-09

Hi!

Thanks!

Alvaro.

---

From: Alvaro Retana <[email-masked]>
Date: Mon, 11 May 2020 17:13:40 -0400

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 [email-masked]
...
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.

---

From: Hongji Zhao <[email-masked]>
Date: Sat, 16 May 2020 08:40:06 +0000

Hi Alvaro,

Thanks a lot for your comments. We will address them ASAP and post the new version.

BR/Hongji

-----Original Message-----
From: Alvaro Retana <[email-masked]> 
Sent: Tuesday, May 12, 2020 5:14 AM
To: Hongji Zhao <[email-masked]>
Cc: Stig Venaas <[email-masked]>; [email-masked]; [email-masked]
Subject: RE: AD Review of draft-ietf-pim-igmp-mld-snooping-yang-09

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.

Thanks!

Alvaro.

...
134	1.2. Tree Diagrams

138	[RFC8340].

...

[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.

...

[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.

[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.

...

...

...

...

[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.

...

...

[major] s/IESG/IETF

...

...

...

[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.