Gen-ART Last Call review of draft-ietf-pals-vpls-pim-snooping-05 I am the assigned Gen-ART reviewer for this draft. The General Area Review Team (Gen-ART) reviews all IETF documents being processed by the IESG for the IETF Chair. Please treat these comments just like any other last call comments. For more information, please see the FAQ at . Document: draft-ietf-pals-vpls-pim-snooping-05.txt Reviewer: Brian Carpenter Review Date: 2017-05-15 IETF LC End Date: 2017-05-19 IESG Telechat date: 2017-05-25 Summary: Ready with issues -------- Comment: -------- "This document isn't defining a new protocol, but rather methods to optimize the use of PIM-based multicast in a VPLS environment." [Shepherd's writeup] Also, the document uses RFC2119 terminology. So why not BCP? Major issue: ------------ > 2.11. Directly Connected Multicast Source ... > o The PE would have to do ARP snooping to determine if a source is > directly connected. What about IPv6? It may be sufficient to add Neighbor Discovery snooping, but you need to say something. Nits: ----- > 1. Introduction ... > o B. Replication on PWs on shared physical path. I realise this is declared out of scope a few lines later, but it's very "telegraphic" and hard to understand. I think you mean o B. Multicast traffic may be replicated when several PWs share a physical path. ... > While this document is in the context of VPLS, the procedures apply > to regular layer-2 switches interconnected by physical connections as > well, albeit this is outside of the scope of this document. In that > case, the PW related concept/procedures are not applicable and that's > all. That is rather unclear. How about: While this document is written in the context of VPLS, the procedures also apply to regular layer-2 switches interconnected by physical connections, except that the PW related concept and procedures do not apply in the case. > 2.2. General Rules for PIM Snooping in VPLS BPDU is used but not defined. > 2.2.1. Preserving Assert Trigger > > In PIM-SM/DM, there are scenarios where multiple routers could be > forwarding the same multicast traffic on a LAN. When this happens, > using PIM Assert election process by sending PIM Assert messages, > routers ensure that only the Assert winner forwards traffic on the > LAN. Either I have misunderstood the intention, or the second sentence is written half backwards. I *think* you mean In PIM-SM/DM, there are scenarios where multiple routers could be forwarding the same multicast traffic on a LAN. When this happens, these routers start the PIM Assert election process by sending PIM Assert messages, to ensure that only the Assert winner forwards future multicast traffic on the LAN. > 2.3.2. IPv6 What's so special about IPv6, or why isn't this section titled "IPv4"? Or better, stay neutral: 2.3.2. IP Versions > 2.3.3. PIM-SM (*,*,RP) > > This document does not address (*,*,RP) states in the VPLS network. > Although [PIM-SM] specifies that routers must support (*,*,RP) > states, there are very few implementations that actually support it > in actual deployments, and it is being removed from the PIM protocol > in its ongoing advancement process in IETF. Given that, this > document omits the specification relating to (*,*,RP) support. If I understand things correctly, you should say ... it has been removed from the PIM protocol [RFC7761]. > 2.4.3. When to Snoop and When to Proxy ... > Therefore, the general rule is that if Join Suppression is enabled on > CEs then proxying or relay MUST be used and if Suppression is known > to be disabled on all CEs then either snooping, relay, or proxying > MAY be used while snooping or relay SHOULD be used. I had to read this a few times. I think you mean Therefore, the general rule is that if Join Suppression is enabled on one or more CEs then proxying or relay MUST be used, but if Suppression is known to be disabled on all CEs then either snooping, relay, or proxying MAY be used while snooping or relay SHOULD be used. (as I understand it, even one CE with Join Suppression breaks snooping for the whole PE.) > 7. References As an RFC user, I find references like [IGMP-SNOOP] instead of [RFC4541] quite impractical. It wastes bits to use constructs like "RFC4541 [IGMP-SNOOP]", which the RFC Editor will change to "RFC 4541 [IGMP-SNOOP]".