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

[Alissa Cooper <[e-mail]>]

Brian, thank you for your review. Olivier, thank you for your response. I have balloted No Objection.

Alissa

> On May 22, 2017, at 3:50 AM, Olivier Dornon <[e-mail]> wrote:
> 
> 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]".
>> 
>> 
>> 
>> 
> 
> _______________________________________________
> Gen-art mailing list
> [e-mail]
> https://www.ietf.org/mailman/listinfo/gen-art