Re: [Gen-art] Genart last call review of draft-ietf-rtgwg-backoff-algo-07

[Alissa Cooper <[e-mail]>]

Elwyn, thanks for your review. Authors, thanks for your responses. I have entered a No Objection ballot.

Alissa

> On Feb 20, 2018, at 5:18 AM, Elwyn Davies <[e-mail]> wrote:
> 
> Hi.
> 
> Thanks for the quick response.
> 
> I think the message I just sent to Acee covers most of this.  I'll await the final text on HOLDDOWN_INTERVAL etc.
> 
> I would just say that making it even clearer that the example values for timer intervals are not defaults.. implementers may understand the language but they can be lazy.
> 
> Cheers,
> Elwyn
> 
> 
> 
> Sent from Samsung tablet.
> 
> -------- Original message --------
> From: [e-mail] <mailto:[e-mail]>
> Date: 16/02/2018 14:00 (GMT+00:00) 
> To: "Acee Lindem (acee)" <[e-mail] <mailto:[e-mail]>>, Elwyn Davies <[e-mail] <mailto:[e-mail]>> 
> Cc: [e-mail] <mailto:[e-mail]>, [e-mail] <mailto:[e-mail]>, [e-mail] <mailto:[e-mail]>, [e-mail] <mailto:[e-mail]>
> Subject: RE: Genart last call review of draft-ietf-rtgwg-backoff-algo-07 
> 
> Hi Elwyn, Acee,
> 
> Thanks for your review and comments.
> Please see inline [Bruno]
> 
> > -----Original Message-----
> > From: Acee Lindem (acee) [mailto:[e-mail] <mailto:[e-mail]>]
> > Sent: Friday, February 16, 2018 1:31 AM
> > To: Elwyn Davies; [e-mail] <mailto:[e-mail]>
> > Cc: [e-mail] <mailto:[e-mail]>; [e-mail] <mailto:[e-mail]>; [e-mail] <mailto:[e-mail]>
> > Subject: Re: Genart last call review of draft-ietf-rtgwg-backoff-algo-07
> > 
> > Hi Elwyn,
> > 
> > On 2/15/18, 2:12 PM, "Elwyn Davies" <[e-mail] <mailto:[e-mail]>> 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 <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/ <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 &lt;= SHORT_SPF_DELAY and SHORT_SPF_DELAY &lt;= 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 &lt;= SHORT_SPF_DELAY, SHORT_SPF_DELAY &lt;= LONG_SPF_DELAY and HOLDDOWN_TIMER greater than INITIAL_SPF_DELAY, SHORT_SPF_DELAY, LONG_SPF_DELAY.
> 
> 
> 
> > 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.
> 
> _______________________________________________
> Gen-art mailing list
> [e-mail] <mailto:[e-mail]>
> https://www.ietf.org/mailman/listinfo/gen-art <https://www.ietf.org/mailman/listinfo/gen-art>