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
_______________________________________________
BESS mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/bess