[bess] Shepherd review on draft-ietf-bess-bgp-vpls-control-flags-04

[Mach Chen <[e-mail]>]

Hi authors,

I was asked by the BESS WG chair to shepherd this document. Here are my review comments.

In summary, the idea is straightforward and clear, but the text need some clean-up and enhancements to make the document more readable, hence to reduce the confusion.  There are also some potential issues need to be addressed before publication. 

Below are some specific comments:

1. 
Idnits tool shows there are some 2119 language warning that needs to be addressed.

2. 
Please expand the acronyms (e.g., VPLS, PW, NLRI., etc.) when first use. 

3. 
Since this document updates the meaning of the "control flags" fields inside the "layer2 info extended community" defined RFC4761, the Abstract and Introduction sections should have some text to state this. E.g., "This document updates RFC4761. "

4. 
Section 2, s/ off multiple/of multiple/, 
And the following sentence is hard to parse, it's better to do a re-writing here.
" The behavior required off the multiple PEs identified
   by the NLRI indicates the VPLS label they should use in the VPLS
   traffic being forwarded to this PE."

5.
Section  2, Second Paragraph,

"[RFC4761] requires that if the advertising PE sets the C and S bits,
   the receiving PE MUST honor the same by inserting control word (CW)
   and by including sequence numbers respectively."

My understanding of "Honor the same" is that the receiving PE MUST set the C and S bits. But obviously, you intend to say something else. Maybe you could just directly say as below:

"[RFC4761] requires that if the advertising PE sets the C and S bits,  when forwarding VPLS traffic to the PE, the receiving PE MUST insert control word (CW) and include sequence numbers respectively. "

6. 
Section2, the third paragraph,
Remove the "the VPLS BGP NLRI" or add more clarification text.

" Thus, the behavior of BGP VPLS needs to be further specified." This sentence is too general,  how about:
" Thus, the behavior of processing CW and sequencing needs to be further specified."

7.
 Section 3.2

s/Current BGP VPLS implementation/ Current BGP VPLS specification

8. 
Section 3.2,
s/with the sequence numbers as well/with  sequence numbers as well

9.
 Section 3.2,

"If the PEs at both
   ends of the PW do not agree on the setting of the S-bit, the PW
   SHOULD NOT come up at all."

Seems this is different from the process of CW, is this the intention?


10. 
Section 4,

It suggests to use two P2MP LSPs to deliver VPLS frames with/without CW and/or sequence.  But for a specific PE, there will be 4 possibilities: 

CW   Sequence
Y       Y
Y       N
N      Y
N      N

Correspondingly, it seems that 4 LSPs needs, am I right?  If so, how do the two LSP cover the 4 situations?

11 
Section 5 and Section 6 talk how to apply the new treatment defined in this document to the multi-homing scenario.  Seems no need put them into different sections. I'd suggest to merge the section and simplify the description. Because most of the context belong to the problem statement and has been stated in the problem section.  You move some text to the problem section.

In addition, for the multi-homing scenario, it allows the backup and primary PWs having different C and S treatment, is it the intention.  

12.

I believe that the current "Security Considerations"  is too simple and cannot pass the IESG evaluation, the authors need to expand this section.

BTW, the IANA section just list "none" seems too simple as well:-), add more words would improve the completeness of the document. 


Best regards,
Mach