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