Last Call Review of draft-ietf-pals-vpls-pim-snooping-05
review-ietf-pals-vpls-pim-snooping-05-genart-lc-carpenter-2017-05-14-00

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
<http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.

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]".