[bess] AD Review of draft-ietf-bess-mvpn-extranet-03

["Alvaro Retana (aretana)" <[e-mail]>]

Hi!

I just finished reading this document.

As mentioned by the WG chairs at the meeting in Yokohama, the technology in bess can be complex and hard to penetrate for the average (or novice) reader.  While the target of documents like this one is not always the average (or novice) reader, it is important that those readers (including the IESG and other review directorates) are able to at least understand what's going on.  Most of my comments (below) are around readability/clarity.

In general I found the document (relatively) not hard to follow, if you read it end-to-end — it may be harder for someone to jump in and read/review a specific section without the benefit of building up to it.    Because of the length of the document, it would be nice to include a "road map" to guide the reader (in general: "Section x talks about X, Section y covers Y, etc.").  It might be easier to include references to the specific sections in 1.4 (Overview).

This is a long standards track document, so it is bound to have a lot of normative language.  On one hand I don't want to go overboard with explicitly mandating every little step..but at the same time we want to be clear as to what is "actually required for interoperation or to limit behavior which has potential for causing harm" [RFC2119].  Due to the potential of bad configurations resulting in incomplete/illdefined extranets, I can see why the behavior around RTs would require normative language.  However, there are some places where the use of rfc2119 keywords may be lacking, not needed or inconsistent.  An example of inconsistency is this paragraph from Section 7.2.3.1. (When Inter-Site Shared Trees Are Used):

   If VRF-S exports a Source Active A-D route that contains C-S in the
   Multicast Source field of its NLRI, and if that VRF also exports a
   UMH-eligible route matching C-S, the Source Active A-D route MUST
   carry at least one RT in common with the UMH-eligible route.  The RT
   must be chosen such that the following condition holds: if VRF-R
   contains an extranet C-receiver allowed by policy to receive extranet
   traffic from C-S, then VRF-R imports both the UMH-eligible route and
   the Source Active A-D route.

.. If the "route MUST carry at least one RT", why isn't the condition to be met also normative?  I don't want to belabor on this specific case, it is just an example.  However, I would appreciate it if the authors would scan the document for required or unneeded normative language.  I pointed at some cases in my comments below.

I do have a couple of items that I think are Major (see below) that I would like to see addressed before starting the IETF Last Call.






Major:

  1.  Section 1.3. (Clarification on Use of Route Distinguishers) uses the word "REQUIREs" in a couple of places.  In a strict manner, the rfc2119 key words is "REQUIRED".  While I think that the meaning of using "REQUIREs" should be obvious, please rephrase the text to strictly use the rfc2119 language.
  2.  Section 10 (Security Considerations)
     *   What is a "VPN security violation"?  It is mentioned in several places, but it is not explicitly defined.  Please either add a reference or be clear in what it is.
     *   Misconfiguration is a significant risk, for example assigning the wrong RTs to the wrong routes.  I think that risk should be mentioned.

Minor:

  1.  Section 1.1. (Terminology): "We will sometimes say that a route "matches" a particular host if the route matches an IP address of the host."  Given the previous definition (in the same paragraph) of "match" ("the address prefix of the given route is the longest match in that VRF for the given IP address") and the use of match there makes it unclear whether you're talking about a host route or just the longest match.
  2.  Section 1.3. (Clarification on Use of Route Distinguishers)
     *   This section explains the "unique VRF per RD" condition a couple of times — even though the explanation seems to be the same, it would be nice if it was explained only once.
     *   ""default RD" (discussed above)"  Where this text appears is in fact the first time that a "default RD" is mentioned.  I'm guessing that this refers to the RD in the "unique VRF per RD" condition, but please don't make the reader guess.
  3.  Section 4.1. (UMH-Eligible Routes)  The "MAY" in the second paragraph appears to be part of the example.  Suggested new text:
     *   The UMH-eligible extranet C-source routes do not necessarily have to   be unicast routes, they MAY be SAFI-129 routes (see Section 5.1.1 of   [RFC6513]).  If one wants, e.g., a VPN-R C-receiver to be able   to receive extranet C-flows from C-sources in VPN-S, but one does not   want any VPN-R system to be able to send unicast traffic to those   C-sources, then the UMH-eligible routes exported from VPN-S and   imported by VPN-R may be SAFI-129 routes.  The SAFI-129 routes are    used only for UMH determination, but not for unicast routing.
  4.  Section 4.2. (Distribution of Unicast Routes Matching C-RPs and DRs)  "It follows that if a VRF contains C-S, but does not contain a C-RP for C-G, then the VRF must import a unicast route matching a C-RP for C-G." and "Similarly, if a VRF contains a C-RP for C-G, but does not contain C-S, the VRF must import a unicast route matching the DR for C-S."  Should those "must" be a "MUST"?
  5.  Section 4.4.1. (The Extranet Source Extended Community)  "…only useful when all the extranet routes from a given VRF are exported with exactly the same set of RTs.  (Cf.  Section 4.3.1 of [RFC4364], which does provide a mechanism…"   Is there any inference that should be made from this text?  Should the Extranet Source Extended Community not be used when the mechanism in rfc4364 is used?  Any downsides if both are used?
  6.  Section 5.2. (Considerations for Particular Inclusive Tunnel Types)
     *   Please include a reference (or definition) for "inclusive tunnel" (maybe in the Terminology).  I couldn't find one in rfc6513, but did in rfc7582.
     *   The third paragraph ("In order for VRF-S to set up the P2MP…") seems to explain what is needed for the setup, which is something that I assume is standardized in a different document.  If so, then the "MUST" shouldn't be normative in this document.  s/MUST/must
  7.  Section 7.2.1. (Intra-AS I-PMSI A-D Routes)  "As specified in [RFC6514]…the RTs carried by that route MUST be chosen…"  That "MUST" is really in reference to rfc6514, so it shouldn't be normative in this document.  If you want to stress the "MUST", then one solution could be to quote the relevant text; otherwise s/MUST/must
  8.  It would be nice if section names were used in addition to section numbers when referring to them in the text — that would make it easier to recall (or look forward to) the text w/out having to go look at the TOC over and over.  [This point may be able to be addressed by the RFC Editor.]  It looks like you started doing this in some places in Section 7, but it is not consistent.
  9.  In several places the text reads: "This document provides procedures…"  Please include a reference to the section where those procedures are.
  10. Please expand on first use:
     *   s/S-PMSI/Selective Provider Multicast Service Interface (S-PMSI)
     *    s/SSM/source-specific multicast (SSM)
     *    s/I-PMSI/Inclusive-PMSI (I-PMSI)
     *    s/MI-PMSI/Multidirectional I-PMSI (MI-PMSI)
  11. References: Update the reference to rfc4601 to draft-ietf-pim-rfc4601bis.

Nits:

  1.  Some replacements:
     *   s/misdelivery of data in those scenarios are outlined in those Section 2.3/misdelivery of data in those scenarios are outlined in Section 2.3
     *   (6.3.1) s/and Each such/and each such
     *   (7.1) s/"corresponds to" to/"corresponds" to