Re: [Pals] Genart last call review of draft-ietf-pals-vpls-pim-snooping-05

[Olivier Dornon <[e-mail]>]

Brian,

Thank you for the thorough review.

I agree on all your comments, the changes will be included in the next 
version.

Thanks,
Olivier.

On 05/15/2017 03:43 AM, Brian Carpenter wrote:
> 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]".
>
>
>
>