On 11/12/2015 2:50 PM, Alvaro Retana (aretana) wrote:
Hi!
I just finished reading this document.
[Eric] Alvaro, thanks for your review. I just posted revision -04,
with a number of edits made in response to your comments.
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.
[Eric] Often, reviewers from various directorates are assigned documents
that they are not qualified to review. This is certainly a problem, but
not a problem with the documents. There is also a problem with certain
ADs (NOT routing area ADs of course) who don't know nearly as much as
they think they do. Well, luckily for them, bidirectional multicast is
not discussed in this document, as it really seems to freak them out!
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).
[Eric] I think the table of contents is as good a road map as we are
going to get. Adding additional pointers to the overview doesn't
provide additional value, and most likely would just introduce errors.
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.
[Eric] I think you are right about this case; I've fixed the text.
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.
[Eric] I looked at each of the 200 or so occurrences of "must",
"should", etc., and made some changes. I think it's more consistent
now, but the criteria for when to say "MUST" and when to say "must" have
never been very clear.
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.
[Eric] I changed "this document REQUIREs" to "it is REQUIRED by this
document". Hopefully, the RFC editor won't ding me for unnecessary use
of the passive voice ;-)
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.
[Eric] A "VPN security violation" is just any situation in which a
packet gets delivered to a VPN where it isn't supposed to go. I added a
definition in the terminology section, and also in the Security
Considerations section.
* Misconfiguration is a significant risk, for example assigning
the wrong RTs to the wrong routes. I think that risk should be
mentioned.
[Eric] This is generally true of any technology based upon RFC4364. I
added a sentence about this to the Security Considerations.
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.
[Eric] I don't see the ambiguity here. A given IP address matches a
given route in the context of a given VRF if the route's address prefix
is the longest match in that VRF for that address. A host matches a
route if the host's IP address matches the route.
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.
[Eric] I think it is explained only once. There is also an explanation
of the "unique RD per VRF" condition, which is different than the
"unique VRF per RD" condition.
* ""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.
[Eric] You're right, the term "default RD" should not have been
introduced without explanation. When a VRF is configured with only one
RD, we call that the "default RD" for that VRF. When a VRF is
configured with two RDs, one is configured to be the "default RD", and
one is configured to be the "extranet RD". I've tried to clear this up
in the text.
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.
[Eric] I've modified the text in Section 4.1 to fix this problem. (The
new text is based on your suggestion, but is slightly different.)
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"?
[Eric] Yes, fixed.
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?
[Eric] The CE puts the Extranet Source EC on a given IP route in order
to tell the PE that when the route is propagated as a VPN-IP route, it
should carry the set of RTs that have been provisioned for extranet
sources. If the CE is using a different mechanism (such as described in
section 4.3.1 of RFC4364) to tell the PE exactly which RTs the route
should carry, there is no point in using the Extranet Source EC. So it
doesn't make sense to use both mechanisms. I've put in some explanatory
text saying characterizing the circumstances under which the CE SHOULD
NOT put on the EC, and saying that the PE SHOULD ignore the EC in those
circumstances.
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.
[Eric] I put in some text defining "Inclusive Tunnel", and making it
clear that it is the same thing that is called "Inclusive Tree" in
RFC6513. (Unfortunately, terminology is not entirely consistent across
the entire set of MVPN documents. RFC6625, e.g., uses "Inclusive
P-tunnel".)
* 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
[Eric] Okay, fixed.
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
[Eric] Okay, fixed.
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.
[Eric] The section numbers are inserted by means of the <xref
target="whatever"/> mechanism. I experimented with adding the section
names by adding "<xref target="whatever" format="title">"
However, I found that xml2rfc often did not render the section names
properly, so I had to back this out. I don't think it's a good practice
to put the section names in by hand.
[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.
[Eric] Just an accident.
9. In several places the text reads: "This document provides
procedures…" Please include a reference to the section where those
procedures are.
[Eric] I'd like to be able to say "this document covers X but does not
cover Y" without having to stop and give a complete set of references
to every place in the document where something relevant to X is
mentioned. Leaving out a reference would just introduce an error, and
in any event the proper sections can be found in the ToC.
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)
[Eric] Done.
11. References: Update the reference to rfc4601 to
draft-ietf-pim-rfc4601bis.
[Eric] The last time I included a normative reference to an
internet-draft, my document got stuck on the RFC Editor's queue for
about 5 years! If 4601bis becomes an RFC before the extranet draft does
(which I admit does seem likely), this change can be made during the
AUTH48 period (probably the RFC Editor will do it automatically).
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
[Eric] Fixed.
_______________________________________________
BESS mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/bess
