Hi Bruno,
On 2/16/18, 9:00 AM, "[email protected]" <[email protected]>
wrote:
Hi Elwyn, Acee,
Thanks for your review and comments.
Please see inline [Bruno]
> -----Original Message-----
> From: Acee Lindem (acee) [mailto:[email protected]]
> Sent: Friday, February 16, 2018 1:31 AM
> To: Elwyn Davies; [email protected]
> Cc: [email protected]; [email protected];
[email protected]
> Subject: Re: Genart last call review of draft-ietf-rtgwg-backoff-algo-07
>
> Hi Elwyn,
>
> On 2/15/18, 2:12 PM, "Elwyn Davies" <[email protected]> wrote:
>
> Reviewer: Elwyn Davies
> Review result: Ready with Nits
>
> I am the assigned Gen-ART reviewer for this draft. The General Area
> Review Team (Gen-ART) reviews all IETF documents being processed
> by the IESG for the IETF Chair. Please treat these comments just
> like any other last call comments.
>
> For more information, please see the FAQ at
>
> <https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.
>
> Document: draft-ietf-rtgwg-backoff-algo-07.txt
> Reviewer: Elwyn Davies
> Review Date: 2018/02/15
> IETF LC End Date: 2018/02/14
> IESG Telechat date: 2016/02/22
>
> Summary: Ready with nits. The draft does not refer to OSPFv3 - i am
not sure if
> this is an oversight or because ODSPFv3 already has this mechanism -
either way
> it should be mentioned.
[Bruno] This is an oversight and has been added.
> One question that occurred to me is whether the draft
> could be considered as updating the OSPFv2/v3 and ISIS standards
(not that IETF
> has any control over ISIS).
[Bruno] I personally don't think so. I'd leave this to OSPF/IS-IS/LSR
chairs and routing AD.
> Major issues:
> None
>
> Minor issues:
> (Non-)Relation between HOLDDOWN_INTERVAL and *_SPF_DELAY values: I
notced that
> Benjamin Kaduk's SECDIR review of this document
>
(https://datatracker.ietf.org/doc/review-ietf-rtgwg-backoff-algo-07-secdir-lc-kaduk-2018-02-14/)
> was concerned that certain state transitions would never occur. I
loooked at
> this and realized that his assumption that LONG_SPF_DELAY <
HOLDDOWN_INTERVAL
> is not required by the document and s6 explicitly resiles from
offering
> suggested default values. Without this assumption, the state
machine appears
> to be correct. Not being familiar with the consequences of setting
the
> HOLDDOWN_INTERVAL relative to the *_SPF_DELAY, I am not sure if
anything could
> be said about such consequences,
[Bruno] There is no "significant" bad consequence.
Thinking about this, I could see 2 consequences:
- There could be a SPF computation scheduled after the HOLDDOWN_INTERVAL. I
don't see this as an issue as, as stated below by Acee, the HOLDDOWN_INTERVAL
is defined as related to the period with no IGP events. It's not defined as the
period with no SPF computations (not to mention further computations e.g., from
BGP).
- Perhaps more importantly, if an IGP event occurs at t1 in the interval
[LONG_SPF_DELAY;HOLDDOWN_INTERVAL], then the SPF computation would be triggered
HOLDDOWN_INTERVAL-t1 after the IGP event. While the intuition is that it should
be triggered after INITIAL_SPF_DELAY.
> but I think it would avoid other people making
> the same assumption as the SECDIR reviewer if it was explicitly
stated that
> HOLDDOWN_INTERVAL is not necessarily bigger than any of the
*_SPF_DELAY values
> and adding any advice from experience about how to choose
appropriate values.
> This might also avoid naive implementers shortcutting the state
machine
> implementation if they made the same assumption.
[Bruno] ok, I would propose the following change:
OLD: In order to satisfy the goals stated in Section 2, operators are
RECOMMENDED to configure delay intervals such that INITIAL_SPF_DELAY <=
SHORT_SPF_DELAY and SHORT_SPF_DELAY <= LONG_SPF_DELAY.
NEW: In order to satisfy the goals stated in Section 2, operators are
RECOMMENDED to configure delay intervals such that INITIAL_SPF_DELAY <=
SHORT_SPF_DELAY, SHORT_SPF_DELAY <= LONG_SPF_DELAY and HOLDDOWN_TIMER
greater than INITIAL_SPF_DELAY, SHORT_SPF_DELAY, LONG_SPF_DELAY.
[Acee] Looks good to me.
Thanks,
Acee
> The definition of HOLDDOWN_INTERVAL explicitly states:
>
> HOLDDOWN_INTERVAL: The time required with no received IGP events
> before considering the IGP to be stable again and allowing the
> SPF_DELAY to be restored to INITIAL_SPF_DELAY. e.g., 3 seconds. The
> HOLDDOWN_INTERVAL MUST be defaulted or configured to be longer than
> the TIME_TO_LEARN_INTERVAL.
>
> Perhaps, it should be restated in the third paragraph of section 6.
[Bruno] ok, added.
>
> Requirements Language: Suggest s/RFC2119/RFC8174/ as there are uses
of lower
> case versions of the reserved words.
>
> I believe this was brought up before and we will make this change. If
not, we'll change to the RFC
> 8174 language.
[Bruno] ok, changed.
> Default values for parameters: There is a possible conflict between
s3, where
> example values for the various interval parameters are given and s6
which
> states that no default values are specified in the document. The
difference in
> termnology maybe too subtle for some implementers.
>
> I would expect those implementing IGPs to know the different between an
"example" and a
> "default".
[Bruno] I agree with Acee.
In s3, I think the examples are useful for the understanding.
In s6, I think the text is clear enough:
" This document does not propose default values for the parameters
because these values are expected to be context dependent.
Implementations are free to propose their own default values."
> Aborting or otherwise of SPF calculation if an IGP event occurs
while an SPF
> calculation is in progress. A note about whether this should happen
(if it is
> possible) would be desirable.
>
> This is certainly out scope and I'm not sure why anyone would deduce
from this draft that an
> implementation should or shouldn't do this.
[Bruno] I agree with Acee. I think adding some text on this may bring
confusion.
>
> OSPFv3: Does this (not) equally apply to OSPF v3 for IPv6? If so it
should be
> mentioned and RFC 5340 included in the references.
>
> Yes. This will be added as reference.
[Bruno] done.
> s12: I suspect (although it could be arguable) that the ISIS
definition, RFC
> 2328 (OSPFv2) and (if added) RFC5340 are normative as you need to
understand
> how they work. This work could even be considered to update these
documents.
>
> While implemented from the beginning, SPF Backoff is not specified by
the IGP protocol
> specifications.
[Bruno] Personally, I'd argue that one does not really need to understand
any specific link state protocol in order to add a delay before its SPF
computation. In all cases, I'd leave this to the IESG.
> Bruno and I will look at the editorial comments and let you know which
ones we do and don't
> incorporate.
Under way....
Thanks,
--Bruno
> Thanks,
> Acee
>
> Nits/editorial comments:
> General: The term 'back-off' may not be familiar to non-Emglish
mother tongue
> speakers and on first occurrence needs a little explanation for
naive readers
> to indicate what it means and to what the back-off is being applied.
I have
> suggested some additional text to this end for the abstract and s1.
>
> Abstract:
> OLD:
> This document defines a standard algorithm to back-off link-state
IGP
> Shortest Path First (SPF) computations.
> NEW:
> This document defines a standard algorithm to temporararily
postpone or
> 'back-off' link-state IGP Shortest Path First (SPF) computations
to reduce
> the computational load on IGP nodes if network events occurring
at closely
> spaced times would otherwise lead to multiple, essentially
redundant
> recalculations of the routing tables.
> ENDS
>
> s1, para 1: s/at the same time/essentially at the same time/
>
> s1, para 2: s/new Shortest Path First (SPF)/new Shortest Path First
(SPF)
> routing table/
>
> s1, para 2:
> OLD:
> experiencing multiple temporally close failures over a short
> period of time
> NEW:
> experiencing multiple temporally close failures (that is,
eventuating over a
> short period of time)
> ENDS
>
> s1, para 2: There is a right bracket missing in the following and
starting a
> clause with 'such as' and ending it with an ellipsis ('...') is
redundant. >
> such as LDP [RFC5036], RSVP-TE [RFC3209], > BGP [RFC4271], Fast
ReRoute
> computations (e.g. Loop Free Alternates > (LFA) [RFC5286], FIB
updates...
> It is unclear to me where the bracket should go: maybe after
[RFC5286] or at
> the end. Please clarify.
>
> s1, para 2: the phrase
> > This also reduces the churn on
> > routers and in the network and.
> is useless, vague jargon. The previous sentence expresses what I
suspect is
> meant by 'churn'. so this is redundant and can be omitted.
>
> s1, para 3:
> OLD:
> To allow for this, IGPs implement an SPF back-off algorithm.
> NEW:
> To allow for this, IGPs usually implement an SPF back-off algorithm
that
> postpones or backs-off the running of the SPF calculation when the
algorithm
> predicts that a run would be essentially redundant or even
counter-productive
> because it appears that multiple closely timed routing-affecting
events can be
> expected. ENDS
>
> s1, para 3: s/choosen/chosen/
>
> s2, last bullet: SPF_DELAY is not defined at this point:
> s/SPF_DELAY timers values/values for any timers used to back-off SPF
> calculations/
>
> s2, last bullet: s/Even though/This is important even though/
>
> s3, para 1: Undesirable ellipsis:
> s/a metric change on a link or prefix.../and a metric change on a
link or
> prefix./
>
> s3:Need to expand SRLG on first use - it isn't deemed to be
well-known.
>
> s3, INITIAL_SPF_DELAY bullet: s/A very small delay to quickly handle
link
> failure/A very small delay to quickly handle a single isolated link
failure/
>
> s3, SHORT_SPF_DELAY bullet:
> OLD:
> SHORT_SPF_DELAY: A small delay to have a fast convergence in
case of
> a single failure (node, SRLG..), e.g., 50-100 milliseconds.
> NEW:
> SHORT_SPF_DELAY: A small delay to provide fast convergence in
the case of
> a single component failure (node, SRLG..) that leads to multiple
IGP events,
> e.g., 50-100 milliseconds.
> ENDS
>
> s5/s5.1: There is currently no text in s5: this is generally
considered
> inappropriate. Suggest removing the first sentence in s5.1 ("This
section
> describes the state machine.") and adding to s5: NEW: This section
describes
> the abstract finite state machine (FSM) intended to control the
timing of the
> running of SPF calculations in response to IGP events.
>
> s5.1, QUIET bullet: s/occured/occurred/
>
> s5.2: There is no need for 3 expansions of FSM - the expansion can
be moved to
> s5 as suggested above.
>
> s5.3 title: s/States/State/
>
> s6, next to last para: s/it's RECOMMENDED to play it safe/it is
recommended
> that timer intervals should be chosen conservatively/ (this is an
operational
> recommendation).
>
> s6, last para: s/RECOMMENDED/recommended/ (ditto).
>
> s7, para 1: s/is based on/is dependent on/,
s/RECOMMENDED/recommended/
> (operational again)
>
> s8: Other documents (e.g., from vendors) have used the terms SPF
wait time and
> SPF hold time. It might be useful to mention that this document
essentially
> provides ways to implement these settings.
>
>
>
_________________________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations
confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu
ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages
electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou
falsifie. Merci.
This message and its attachments may contain confidential or privileged
information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and
delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been
modified, changed or falsified.
Thank you.
_______________________________________________
rtgwg mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/rtgwg
