Last Call Review of draft-ietf-bess-vpls-multihoming-05
review-ietf-bess-vpls-multihoming-05-genart-lc-halpern-2020-03-18-00

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.