Reviewer: Joel Halpern Review result: Ready with Issues 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 <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>. Document: draft-ietf-bess-vpls-multihoming-05 Reviewer: Joel Halpern Review Date: 2020-03-18 IETF LC End Date: 2020-03-12 IESG Telechat date: Not scheduled for a telechat Summary: This document is ready for publication as a Proposed Standard. I would recommend that the minor issues below be addressed before approval for publication. Major issues: N/A Minor issues: Section 1 seems to have some language oddities. The first paragraph appears to be technically correct that RFC 4761 and 6074 both describe using BGP for auto-discovery. As written, the text seems to imply that the same thing is defined in two places. This is going to confuse readers and tempt them into trying to understand the wrong problem. Please either simplify or explain. (I believe the distinction in the RFCs is about use with LDP. While the text refers to that, the English construction is not clear.) Section 1.1 paragraph 4 is a duplicate of the latter half of paragraph 3. VE (VPLS Edge) is defined in section 1.1 as being about a device. The introductory text then starts talking about VE NLRI and VE-IDs. It looks like the point is merely to setup the definition for CE-NLRI. If so, the text could be restructured to say: BGP VPLS NLRI as specified in section 3.2.2 in [RFC4761] includes a number of fields. This document refers to the VE-ID, VE offset, .... from that RFC. This document should reference RFC 8174 as part of BCP 14. And should use upper case usage (MUST, SHOULD, ...) consistently. It currently mixes upper and lower case usage in different places. Even though some of each usage are clearly normative. It is unclear why discarding CE-ID=0 (which is defined to be invalid) is a should rather than a MUST. The beginning of section 3.3.1 seems to say that the L2-info community may be omitted (e.g. is optional). Then the text says that it must (presumably upper case) be present. As written, this is quite confusing. Reading later, it appears that this is intended to support backwards compatibility. If so, then the sentence about handling their absence should say "Although required, this document specifies mechanisms for dealing with their absence to support backwards compatibility." Alternatively, it seems that the L2-info is allways required, but that the L2-pref is not required unless the VPLS advertisement is inter-AS. If that is the case, say that. Nits/editorial comments: I found myself misreading the definition of multi-homing: "redundant connectivity to one or more sites". What is there is technically correct. It would help I think if this were split into two pieces: multi-homing is when the provider provides redundant VPLS connectivity to a customer site. If the customer has more than one site, the service is considered to be multi-homed if at least one site is multi-homed. Section 3.3.1, the two new flags in the L2-info community are presumably defined in this document rather than proposed. Or at least, that is how they need to be described once this document is approved. _______________________________________________ BESS mailing list BESS@ietf.org https://www.ietf.org/mailman/listinfo/bess