Document: draft-ietf-bess-vpls-multihoming Title: BGP based Multi-homing in Virtual Private LAN Service Reviewer: Jie Dong Review result: Ready with Nits
I am reviewing this draft on behalf of the BGP Directorate. In general, the draft technically is in a good shape, I only found a couple of "nits" to be fixed. - Section 1.1 General Terminology It is suggested to put the terminology into bullets. There was no formal definition of "VPLS site" in the RFCs quoted. while it seems the definition of "VPN site" in this section is not consistent with the related descriptions in RFC 4364 and 4761. RFC 4364 says: Each VPN site must contain one or more Customer Edge (CE) devices. RFC 4761 says: It is often desired to multi-home a VPLS site, i.e., to connect it to multiple PEs... While this document says: A VPLS site is a grouping of ports on a PE that belong to the same VPLS domain. It is suggested to rephrase and align the definition of VPLS site with those RFCs. - Section 3.2 It says: If both CE1 and CE4 connectivity to PE1 is down, remote PEs can choose based on D bit in VE NLRI. The D bit is actually carried in the L2-Info Extended Community, not in the NLRI. Suggests to correct this description. - Section 3.3.1 In "A designated forwarder must set the F bit and a non-designated forwarder must clear the F bit when sending BGP CE NLRIs for multi-homed sites.", the "must" should be capitalized. - Section 3.3.2.7 s/Route Origin (RO) community/Route Origin (RO) extended community - Section 4.1 In “The Local Administrator sub-field SHOULD be set to zero on sending”, suggest to use MUST unless there is scenario in which the sub-field can be set to non-zero. - Section 4.2 In " In addition, Local Preference value greater than or equal to 2^16 for VPLS advertisements is not valid.", suggest to clarify whether this restriction only applies to inter-AS VPLS, or it also applies to the intra-AS case. - Section 4.3.2 It currently describes that RR must set the LP to VP and VP to LP if RR is used for VPLS advertisement reflection, suggest to also add descriptions about the operation if RR is not used. And the "must" should be capitalized. - Section 5.1 The "must" should be capitalized. Section 5.2 The "recommended“ should be capitalized. Section 6.2 In “If a LDP VPLS PEs running BGP AD lacks support of multi-homing operations specified in this document, it SHOULD ignore a CE NLRI with the length field of 17.”, currently it reads like this is putting new requirements on a legacy implementation. I know this is not the intention, thus it is suggested to rephrase the text. Section 8 In the IANA considerations, the F bit is named "MAC flush indicator", while in section 5.1 both D bit and F bit are called "MAC flush indicators". It may be better to give F bit a more precise name， e.g. something related to "Designated Forwarder State" could be considered. _______________________________________________ BESS mailing list -- [email protected] To unsubscribe send an email to [email protected]
