On 9/27/13 8:53 AM, "Alvaro Retana (aretana)"
<[email protected]<mailto:[email protected]>> wrote:
We want to hear from people who have read and understood the draft (besides the
authors!) about this topic. Please provide some explanation as to why you
support or not the adoption of the draft — avoid "+1".
<WG Chair Hat Off>
Hi!
I reviewed the draft and put some comments below. In general, I think the
draft is well written and surprisingly (given the potential complexity of the
topic) relatively easy to follow..but not in one sitting!
I do need the authors to clarify one thing before I "cast my vote" about
adoption: it's Intended Status. The draft has an intended status of
Informational. However, the Introduction reads: "This document defines the
MRT Lowpoint algorithm to be used as a standard in the default MRT profile for
MRT-FRR." Which one is it?
IMHO, there's a significant difference in saying "this is an algorithm than can
be used" (informational) vs "this is the algorithm that MUST be used"
(standard). For the record, I have no issues with adopting this draft with an
intended status of Informational.
Thanks!
Alvaro.
Major Issues:
1. The draft has an intended status of Informational. However, the
Introduction reads: "This document defines the MRT Lowpoint algorithm to be
used as a standard in the default MRT profile for MRT-FRR." I would like to
see a clarification on what the intended status is.
2. I found it interesting that, even though the document is defining an
algorithm that when implemented will need to have consistency across nodes,
there was no reference to RFC2119 — but some of the words were capitalized
anyway. Even then, only 4 "MUST" and 1 "SHOULD" appeared. I realize that the
count of MUST/SHOULD is no indication of anything, but it doesn't feel right
for a 40+ page document. I would appreciate if the authors would not just add
the 2119 reference (nit), but if they would also comb the text for parts where
a little more normative direction might help (which doesn't necessarily
translate into MUSTs). For example (all the text is from Section 4 (Algorithm
Sections)):
* "The different parts of this algorithm are described below. These
work on a network graph after, for instance, its interfaces are ordered as per
Figure 14." It sounds as if Figure 14 is optional or maybe ordering the
interfaces is not mandatory.. Later it is clarified: "To ensure consistency
in computation, all routers MUST order interfaces identically. . . The required
ordering . . . Is given in Figure 14." But the first read sounds not normative
enough to me.
* Figure 14 is not deterministic. The text says that the order doesn't
matter in some cases, but then offers an example of a potential tie-breaker.
* Root Selection. No algorithm is provided. There's a reference to
I-D.atlas-ospf-mrt, where a suggestion is made ("..the router with the highest
GADAG Root Selection Priority is picked to be the GADAG Root"). IMHO, the
algorithm should be specified in this draft, where the requirement to carry the
Priority is defined so that the extensions draft(s) can show how to implement
it in OSPF (or any other protocol)..not the other way around.
* "Initially, all interfaces must be initialized to UNDIRECTED."
Should that be "MUST"?
* "It is possible that some links and nodes will be marked as
unusable.." Same comment as the one above for the root selection: this draft
should define the requirements ("Due to operational or other issues, the
capability to xxx SHOULD....") so that other docs (like I-D.atlas-ospf-mrt) can
then implement the extensions.. The text right now says: "because OSPF can
mark links as unusable then we'll do this.."
* "There are two methods to find ears; both are required." REQUIRED?
3. Algorithm Alternatives and Evaluation. There are a couple of
alternatives described in the appendices, but there is no evaluation as to why
the Lowpoint Algorithm is better. In fact, the text seems to read as if the
appendices describe options to the main algorithm (?) -- begging the question,
are there instances where these options would/could be used?
Minor Issues:
1. Some important terms are not properly defined, or at least not defined in
Section 2.
* "cut-edge" The term is not defined in Section 2. Later in the
document it is described as "The algorithm identifies cut-edges simply as links
where both ends of the link are cut-vertices.", which is the definition for
cut-link. It looks like cut-edges is used more through out.
* "MRT Island" I know it is defined in the architecture draft. It
would be nice to carry the definition over to Section 2.
* "UNDIRECTED/OUTGOING/INCOMING" These are used in the algorithm in
various points, but are not really defined as to
2. I think that the algorithm in Figure 6 is not right — of course, I may be
misunderstanding it. When talking about the endpoints of the ear (X and Y) it
says "if Y is root"..but a couple of paragraphs above the text reads: "Once a
partial ADAG is already present, we can always add ears to it: just select a
non-ready neighbor N of a ready node Q, such that Q is not the root.." It then
sounds like the starting point can't be the root..
3. The explanation of the motivation of the low-point values (after Figure
9) needs either better examples or a better explanation — or maybe I got lost.
You first start explaining about when 'L(c) >= D(x)', but the example mentions
the 'L(x) < D(x)' case. An example for the second reason would be nice.
4. Algorithm Evaluation. While it is interesting to see MRT compared to
rLFA, it would be much more interesting to see it compared to the options in
the appendices or to ARC. I would move the comparison to rLFA to an appendix.
Nits:
1. Section 2. "standard pseudo-node representation" Reference?
2. Section 2; in the definition of MRT-Red and MRT-Blue:
s/described/describe
3. Section 2; ADAG definition: "whith" ??
4. "strictly greater/lower" These terms are used in a couple of places,
but the << or >> notation is used most of the time. It would be nice to be
consistent to avoid confusion.
5. Section 3.2: "If in the partial order where an ear's two ends are X and
Y, X << Y, then there must already be a directed path from X to Y already in
the ADAG." Too many "already".
6. It would be nice to include "partial ADAG" in the definitions section.
Yes, it is defined elsewhere, but a central place is nice in case you forget.
;-)
7. I know sometimes it's hard, but please try to format the text so that the
figures are not split between pages.
8. Section 3.4, after Figure 10. "This observation means that if we want to
find a pair of MRTs rooted at R, then we need to build up a pair of RTs in
block 3 with K as a root. Similarly, we need to find another one in block 2
with C as a root, and finally, we need the last one in block 1 with R as a
root." When talking about blocks 1 and 2, you really mean a pair to RTs (just
like in block 3), not just "one", right?
9. Section 3.5: "The method for local-root computation is used in the MRT
Lowpoint algorithm for computing a GADAG using Low-Point inheritance and the
essence of it is given in Figure 12." s/is used/used and s/inheritance
and/inheritance, and To improve readability.
10. There are a couple of out of date references.
_______________________________________________
rtgwg mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/rtgwg
