Hi Ahmed,
Thanks for posting version 15, it was helpful.
In the interest of keeping things constructive, I'm top-posting my comments on
the DISCUSS points, rather than attempting a point-by-point reply. Other points
are replied to in line. Agreed points (or “I disagree but it’s not a blocking
issue and I don’t want to drag it out” points) are snipped (“…”) for brevity.
### Whole document, is post-convergence of the essence, or not?
First and foremost, thank you for clarifying/confirming that the
authors/working group intends that following the post-convergence path is a
nice-to-have, not a required feature of the solution ("post-convergence is not
a constrain or a requiremehnt”). I’m surprised by this, because as I noted in
the DISCUSS, Sections 5-8 appear to mandate the use of the post-convergence
path, not just suggest it, and for that matter many other parts of the document
focus closely on post-convergence. But at least this lets us focus on the point
of disagreement.
I would normally request the authors clarify the document at this point. Since
you don't agree that the document is internally inconsistent, we are at an
impasse. I can do your work for you, suggesting rewrites that would resolve my
concerns, but I might not be able to do this as quickly as you could. Also, to
remove the impression that post-convergence is required would be a fairly huge
rewrite!
Finally, although I regret having to get this far down in the weeds, it seems
necessary that I cite a definition of the English adjective "key”.
Dictionary.com<http://dictionary.com/> defines “key (adjective)” as “essential;
central; important”. Other sources give similar definitions. It seems like you
are leaning hard into the third and weakest sense, of “important”. The problem
is that the first sense of “essential” shouldn't be ignored. Even if I concede
for the purpose of discussion that there is room for debate, our goal with our
specifications should be maximum clarity, not to squeeze through on a
technicality.
I’m open to discussing this on the phone if it would help us move forward.
### Section 10, multiple unrelated failures
You ask,
I do not understand what is your concern?
To pare the point down to the essentials, it's that RFC 5714 is a framework,
not a specification, and it doesn't make sense to say implementations should be
“in accordance” with it since it has no normative requirements. An implementor
looking at RFC 5714 would find nothing to tell them what code to write. A
minimal fix would be to delete the sentence in question.
(Also, to the extent a framework places requirements on anyone, it isn't
implementers, but rather authors of solutions. So, we might ask if this TI-LFA
specification is “in accordance” with RFC 5714. But I think we probably don't
want to go there, and the better solution is to delete the sentence and move
on.)
So, my proposal here is:
OLD:
Implementations of TI-LFA should deal with the
occurrence of multiple unrelated failures in accordance to the IP
Fast Reroute Framework [RFC5714].
NEW:
<nothing>
To repeat, the reason for this is that the sentence is not actionable and
implies something incorrect about the status and content of RFC 5714.
On May 12, 2024, at 11:02 PM, Ahmed Bashandy <abashandy.i...@gmail.com> wrote:
Thanks for the thorough review
I am in the process of uploading version 15 of the document. So whenever I say
"fixed", "changed", "modified",..., etc, I am referring to version 15
See inline comments at #Ahmed
Ahmed
On 4/16/24 1:14 PM, John Scudder via Datatracker wrote:
John Scudder has entered the following ballot position for
draft-ietf-rtgwg-segment-routing-ti-lfa-13: Discuss
When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)
Please refer to
https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/<https://urldefense.com/v3/__https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/__;!!NEt6yMaO-gk!Cfbrr6ZaOgH0backvKwwo9W-jNxM5JZuxAPIY97sFKYLeYAJyik34ClQcFp65uGyh8kjs7Ku6A9t0nfYAdFBpA$>
for more information about how to handle DISCUSS and COMMENT positions.
The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-rtgwg-segment-routing-ti-lfa/<https://urldefense.com/v3/__https://datatracker.ietf.org/doc/draft-ietf-rtgwg-segment-routing-ti-lfa/__;!!NEt6yMaO-gk!Cfbrr6ZaOgH0backvKwwo9W-jNxM5JZuxAPIY97sFKYLeYAJyik34ClQcFp65uGyh8kjs7Ku6A9t0nfqpCMjug$>
----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------
# John Scudder, RTG AD, comments for draft-ietf-rtgwg-segment-routing-ti-lfa-13
CC @jgscudder
Thanks for this document. The technology is valuable, and the underlying
techniques sound. Despite what you might guess from my abundance of comments, I
like this document. I suspect it has suffered from having been edited
repeatedly by the same set of experts, it becomes hard to see it as a new
reader might. So, please accept my comments in the spirit of a new set of eyes
looking over some long-established text.
Although most of my remarks are non-blocking COMMENTs, I am putting two in as
DISCUSS points. Some of my other comments are of a similar nature, but I think
they're less serious for the overall coherence of the document.
## DISCUSS
### Whole document, is post-convergence of the essence, or not?
#Ahmed No. THere are many aspects in the documents that indicates that the
whole document is not post convergence, although post-covergence aspect is a
very important aspect of the document. In fact the sentence starting with
"Although not a Ti-LFA requirement or constraint," that you quoted in the
comment after the next one explicitly mentions that post-convergence is not a
constrain or a requiremehnt
The document seems to be arguing with itself about whether following the
post-convergence path is, or is not an essential/required feature. In the
Abstract:
A key aspect of
TI-LFA is the FRR path selection approach establishing protection
over the expected post-convergence paths from the point of local
repair
It's a *key* aspect! OK! But then, in the Introduction:
Although not a Ti-LFA requirement or constraint, TI-LFA also brings
the benefit of the ability to provide a backup path that follows the
expected post-convergence path
#Ahmed: I do not understand why you think that the work "key" implies "only" or
"must".
Wait, it's "key" but "not a requirement or constraint"? Moving on to Section 6,
#Ahmed: Again I do not understand how you came to the conclusion that "key"
implies "requirement" or "constraint" even though the document explicitly
mentions that post-convergence is neither a requirement nor a contraint
The repair list encodes the
explicit post-convergence path to the destination
So it "encodes the explicit post-convergence path". "Encodes", not "might
encode" or "can encode". So the Abstract is right and Section 2 is wrong. But
wait there's more! Later in Section 11,
#Ahmed: I really do not understand why you want to push "must be post
convergence" or "only post-convergence" down the throat of this draft.
Does the sentence say "the repair list ****only**** encodes"??.
traffic can
be steered by the PLR onto its expected post-convergence path during
the FRR phase
So it "can"... which implies "doesn't have to be".
#Ahmed: Of course it implies "doesn't have to be". If there is any sentence in
the draft that says that a ti-lfa repair path must be on post convergence, I
would be more than happy to re-word it.
There's more, for example, all of Section 5 talks about post-convergence paths,
and there are many more mentions in Section 6 too.
#Ahmed: Of course. It is a "key" attribute of the post convergence path and big
advantage of ti-lfa, but not a "must-have", "a constraint", nor a "requirement"
of ti-lfa
Given that Sections 5-8 seem to come closest to being the normative ones
(though the document is sadly not very precise in this regard) I'm left with
the impression that the Abstract is right ("key"), and the quoted passages of
Sections 2 and 11 are wrong. In any case, I think this needs to be resolved in
some way.
#Ahmed: IMO is no problem to solve as I mentioned in the replies to your above
comments, which are hinged on a the **very big** assumption that "key" implies
"constraint", "requirement" and/or "must-have"
### Section 10, multiple unrelated failures
Implementations of TI-LFA should deal with the
occurence of multiple unrelated failures in accordance to the IP Fast
Reroute Framework [RFC5714].
(Nit, you misspelled "occurrence".)
#Ahmed: Fixed in version 15 (coming soon)
Can you explain what you mean by this sentence? I haven't reviewed it carefully
but RFC 5714 is a framework, and I don’t know what it means to be "in
accordance to" it. The only relevant text I was able to find in it was RFC 5714
Section 5.2.6,
However, it is important that the occurrence of
a second failure while one failure is undergoing repair should not
result in a level of service which is significantly worse than that
which would have been achieved in the absence of any repair strategy.
Putting that together with the quote from your specification, I come up with an
interpretation like "it's important to behave reasonably in the face of
multiple failures, we aren't going tell you how to do it, this other document
we are citing isn't going to tell you how to do it either, it's just going to
tell you that our specification was supposed to cover this, but we didn't.”
#Ahmed: The paragraph starting with "for each destination" near the bottom of
page 5 indicates clearly mentioned that this document covers only a single
failure. The above sentence refers the implementer to the framework of FRR.
I do not understand what is your concern?
----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------
## COMMENTS
### General Gripe, Y U no XML?
It looks like you must have uploaded the TXT rendering instead of the XML
source. Whenever it is you upload your next revision, please consider uploading
the XML source instead.
…
#Ahmed: I used
https://author-tools.ietf.org/<https://urldefense.com/v3/__https://author-tools.ietf.org/__;!!NEt6yMaO-gk!Cfbrr6ZaOgH0backvKwwo9W-jNxM5JZuxAPIY97sFKYLeYAJyik34ClQcFp65uGyh8kjs7Ku6A9t0nc4d2cvvQ$>
OK. Next time you upload a revision, please upload XML instead of TXT.
### Abstract, for want of a hyphen the meaning was lost
…
I'm not sure the concept of two-connectedness is universally understood enough
that it's suitable for use in an Abstract, so I'd support further editing to
get rid of the graph theory term entirely, but at least this edit makes it
correct.
#Ahmed: 2-connectness is an important requirement for ti-lfa to be successful.
I am 100% sure that all network engineers know what "two-connected means, and,
if not are quite capable of looking up the term "two-connected graph”
If you are a betting man, I would happily wager stakes that your 100%
prediction is not accurate.
Be that as it may, this is only a COMMENT; even though I think avoidance of the
graph theory terminology would be preferable for readability, I don't insist,
it’s not a big deal.
…
### Section 2, can't parse this sentence
By relying on SR this document provides a local repair
mechanism for standard link-state IGP shortest path capable of
restoring end-to-end connectivity in the case of a sudden directly
connected failure of a network component.
I can’t parse this sentence. I am *guessing* that what you mean is something
like,
NEW:
This document leverages Segment Routing (SR) to provide a local
repair mechanism for a shortest path computed by a standard
link-state IGP. The local repair is capable of restoring end-to-end
connectivity in the case of a failure of a directly connected
network component.
If this is what you mean, you’re welcome to use this text if you want. If it’s
not what you mean, please explain?
#Ahmed: The text has been changed to be very close to what you are suggesting
in version 14. I hope the latest text is satisfactory
Note I removed “sudden” in my NEW text because I’m guessing you don’t mean to
exclude a gradual or a foreseeable failure. A failure is a failure, after all.
The new version looks good, except the comment about “sudden” still stands for
the new version which is “abrupt”. Isn’t the technology also applicable in the
event of, for example, a planned failure? It seems like the sentence would be
just fine without the adjective.
…
### Section 2, “primary link”
You mention the “primary link” in a few places here (and nowhere else). What is
the “primary link”? Please clarify or re-word. For example, maybe you mean “the
link whose failure is detected”.
#Ahmed: I added definitions to the terms "primary link" and "primary interface"
to the terminology section
Thanks! Since the uses in Section 2 precede the definitions, maybe insert a
forward reference, as in “primary link [Section 2.1]”.
…
### Section 6, terminology is inconsistent and unclear
I can understand NodeSID(R1) well enough even though you haven't supplied a
definition, it’s the node SID for router R1. But what is “Node_SID(P)”? P isn’t
a router, it’s a set of routers, or a space. Please clarify, whether with a
definition or otherwise. Probably your clarification will fix both this and the
previous point.
#Ahmed:The sentence where "Node_SID(P)" says right before using the term
"Node-SID(P)" says "from P to Q when these nodes". Pay special attention to
"these nodes". AFAIK a "these" refers to last nouns and the last nouns are "P"
and "Q". In addition I added definitions to "P node" and "Q node" to the
terminology section based on your comment
Thanks for the improvements, they are good but don’t eliminate the issue. The
underlying problem is that although you’ve defined “a P node” (which is
helpful, thanks), “P” as you’ve used it here, without qualification, can refer
in this document to either “a P node” or “P-space”, and either one potentially
makes sense in this context, but only one is right.
There are various ways you could fix this. One fairly straightforward one goes
like this,
OLD:
The TI-LFA repair path is found by intersecting P(S,X) and Q(D,X)
with the post-convergence path to D and computing the explicit SR-
based path EP(P, Q) from P to Q when these nodes are not adjacent
along the post convergence path. The TI-LFA repair list is expressed
generally as (Node-SID(P), EP(P, Q)).
NEW:
The TI-LFA repair path is found by intersecting P(S,X) and Q(D,X)
with the post-convergence path to D and computing the explicit SR-
based path EP(P, Q) from a node P to a node Q when these nodes are not
adjacent
along the post-convergence path. The TI-LFA repair list is expressed
generally as (Node-SID(P), EP(P, Q)).
…
### Sections 6.1, 6.2, 6.3, SHOULD NOT use SHOULD
What work are the SHOULDs doing here? Considering that in other parts of the
document you leave the computation of the repair path up to the implementation,
why are you mandating it here? And, if you’re mandating it, why not mandate it
all the way with a MUST?
#Ahmed: Every time the terms "should" or "should not" is/are used, it is clear
what these terms are referring to. If there is a place where the terms
"should" or "should not" are not clean, please point these places out so that I
can correct them.
Second, we are not mandating anything in these sections because, as you
mentioned, computation are part of the implementation. Hence we did not use
"MUST" or "MUST NOT"
I think you’ve misunderstood the use of RFC 2119 keywords. SHOULD is a weak
mandate, it’s not just a casual suggestion. This is evident from the title of
RFC 2119: "Key words for use in RFCs to Indicate Requirement Levels”. It’s
about *requirements*. From RFC 2119:
3. SHOULD This word, or the adjective "RECOMMENDED", mean that there
may exist valid reasons in particular circumstances to ignore a
particular item, but the full implications must be understood and
carefully weighed before choosing a different course.
It’s essentially a MUST with an exception. If you can’t articulate the
exception, the two things you should ask yourself are “is this actually a
MUST?” and “is this actually a ’should’?” If you don’t want to specify
requirements, you shouldn’t use RFC 2119 keywords here.
It seems to me that if you don’t want to mandate implementation, it would be
sensible to take the RFC 2119 language out altogether. If you do want to
mandate implementation, I don’t see why you wouldn’t make this a MUST. If you
really do want it to be SHOULD, Please explain your reasoning.
#Ahmed: Disagree. There are certain properties that should exist in an
implementation. However the implementation itself is out of scope.
Saying “disagree” isn’t an explanation [1]. You can give details of properties
just fine using plain old lowercase “should”, etc. Once you reach for the
Requirement Levels keywords you’re invoking the specific semantics defined by
BCP 14, and you say you don’t want that. The all-caps keywords are not merely a
way of providing emphasis, even though people often use ALL CAPS that way in
other contexts — BCP 14 makes it very clear they have specified meaning in
documents that cite it (which includes your document!), and that meaning is
about “requirements”.
(I was expecting you to say that you used SHOULD the way RFC 2119 expects,
which I think of as a kind of “MUST…UNLESS” pair (obviously “UNLESS” isn’t a
real 2119 keyword), and to talk about what some reasonable exception cases
might be.)
### Section 7, primary outgoing interface
The existence of a "primary outgoing interface" seems to imply the existence of
a secondary outgoing interface, tertiary outgoing interface, etc. Please define
primary outgoing interface, or if this isn't an important distinction, consider
whether you can simplify to just say "outgoing interface".
#Ahmed: I added definitions to the terms "primary link" and "primary interface"
to the terminology section
Thanks! Consider either updating your definition to something like,
OLD:
Primary Interface: It is one of the outgoing interface towards a
destination according the IGP link-state protocol
NEW:
Primary Interface, Primary Outgoing Interface: One of the outgoing
interfaces towards a
destination according to the IGP link-state protocol
(I made grammatical fixes while I was there)
Or, drop “outgoing” from the quoted text.
…
### Section 8.2 and others, description of SRv6 behaviors
RFC 8754 describes forwarding behaviors using a kind of line-numbered
pseudocode, and later documents that modify forwarding behaviors specify
updates to the pseudocode. (Examples: RFC 8986,
draft-ietf-spring-srv6-srh-compression-15,
draft-ietf-rtgwg-srv6-egress-protection-16,
draft-ietf-spring-sr-redundancy-protection-03,
draft-ietf-spring-srv6-path-segment-07)
You don’t do this, you use a descriptive approach instead. I'm ok with this in
isolation, but I’d like to know if you made an affirmative decision to diverge
from the usual SRv6 way of doing things, and if so, why, and if the SPRING
working group specifically considered this and is OK with it.
#Ahmed: When it comes to implementation specifics, we explicitly avoid to
mandate anything. Pseudo code may appear to be a mandate
But yet, the rest of the SRv6 document set (which also presumably doesn’t want
to mandate implementation details) uses pseudocode. I’m not saying I love that
approach, but the question stands as to whether there is some reason for the
divergence other than author preferences (which is what I’m inferring so far).
### Section 8.2, shorter than what?
In such case, there is no need for a preceding
Prefix SID and the resulting repair list is likely shorter.
Shorter than what? This is the first place in this document the string “Prefix
SID” occurs, so I’m confused.
#Ahmed: shorter than preceding the adjacency SID with a prefix SID.
For the term prefix-SID, it is defined in the SR architecture RFC [RFC8402]. I
added a reference to RFC8402 beside the 1st mention of prefix-SID
My point wasn’t that I don’t know what a Prefix SID is (though defining it is a
good idea) but rather that mentioning “a preceding Prefix SID” without anyone
having suggested there should be one, to begin with, raises more questions than
answers.
It wouldn’t hurt to spend a few more words to spell out what you’re trying to
convey here. "Brevity is the soul of wit" is a good maxim but so is “everything
should be made as simple as possible, but not simpler.”
### Section 11, limit the implementation of local FRR policies
Based on this assumption, in order to facilitate the operation of
FRR, and limit the implementation of local FRR policies
Do you mean "limit the need for implementation of local FRR policies"? (And you
could drop “implementation of” for that matter.)
I didn’t see a response to this question.
### Section 11, TI-LFA and SR policies don't mix?
...
Also, although I appreciate that you provided some rudimentary parameterization
of the topologies in Table 1, I think it would be helpful to at least say what
time period the topologies reflect —
draft-francois-rtgwg-segment-routing-ti-lfa-00 dates to summer of 2015; are we
talking about the topologies that were in vogue in 2015? Those of 2023? Etc.
This comment wasn’t responded to.
### Section 12, granularity wut
We do not cover the
case for 2 SIDs (Section 6.3) separately because there was no
granularity in the result.
I don’t understand what this means. Can you rephrase it? Generally, I find the
words “granular”. “granularity”, and related have almost zero descriptive
power. :-(
This comment wasn’t responded to.
### Section 12, "2 or more" or "2", "3", and no more?
In your description of the table, you say,
The convention that we use is as follows
* 0 SIDs: the calculated repair path starts with a directly
...
* 1 SIDs: the repair node is a PQ node, in which case only 1 SID is
...
* 2 or more SIDs: The repair path consists of 2 or more SIDs as
described in Section 6.3 and Section 6.4. We do not cover the
case for 2 SIDs (Section 6.3) separately
...
But the table headers show:
+-------------+------------+------------+------------+------------+
| Network | 0 SIDs | 1 SID | 2 SIDs | 3 SIDs |
+-------------+------------+------------+------------+------------+
I.e. the table headers don’t show “2 or more” they show 2, and 3, broken out
distinctly, and no "or more" case. Seems like these need to be reconciled.
This comment wasn’t responded to.
…
## Notes
This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues.
[ICMF]:
https://github.com/mnot/ietf-comments/blob/main/format.md<https://urldefense.com/v3/__https://github.com/mnot/ietf-comments/blob/main/format.md__;!!NEt6yMaO-gk!Cfbrr6ZaOgH0backvKwwo9W-jNxM5JZuxAPIY97sFKYLeYAJyik34ClQcFp65uGyh8kjs7Ku6A9t0ndAEcolfQ$>
[ICT]:
https://github.com/mnot/ietf-comments<https://urldefense.com/v3/__https://github.com/mnot/ietf-comments__;!!NEt6yMaO-gk!Cfbrr6ZaOgH0backvKwwo9W-jNxM5JZuxAPIY97sFKYLeYAJyik34ClQcFp65uGyh8kjs7Ku6A9t0nfFpEkUFg$>
[1] https://en.wikipedia.org/wiki/Argument_Clinic
_______________________________________________
rtgwg mailing list -- rtgwg@ietf.org
To unsubscribe send an email to rtgwg-le...@ietf.org
