[Gen-art] Genart last call review of draft-ietf-pals-vpls-pim-snooping-05

[Brian Carpenter <[e-mail]>]

Reviewer: Brian Carpenter
Review result: Ready with Issues

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