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

2017-05-24 Thread Alissa Cooper 
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  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
>> .
>> 
>> 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

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

2017-05-22 Thread Olivier Dornon 

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
.

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
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art

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

2017-05-14 Thread Brian Carpenter 
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
.

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
Gen-art@ietf.org
https://www.ietf.org/mailman/listinfo/gen-art