Subject: [RTG-DIR] Rtgdir early review of draft-ietf-rtgwg-routing-timer-param-sync-00

From: Michael Richardson <[email-masked]>
Date: Tue, 16 Jan 2018 08:32:37 -0800

Reviewer: Michael Richardson
Review result: Not Ready

RtgDir Early review: draft-ietf-rtgwg-routing-timer-param-sync-00.txt

Hello

I have been selected to do a routing directorate "early" review of this draft.
  https://datatracker.ietf.org/doc/draft-ietf-rtgwg-routing-timer-param-sync-00.txt

The routing directorate will, on request from the working group chair,
perform an "early" review of a draft before it is submitted for publication
to the IESG. The early review can be performed at any time during the draft's
lifetime as a working group document. The purpose of the early review depends
on the stage that the document has reached.

   * As this document has recently been adopted by the working group, my
     focus for the review is on providing a new perspective on the work, with
     the intention of catching any issues early on in the document's life
     cycle.

For more information about the Routing Directorate, please see
=E2=80=8Bhttp://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Comments as I read:

1) while the table of contents hints that this is about ISIS and OSPF, and
   perhaps other link-state algorithms, this should probably go into the
   abstract and intro.

2) On first read, I think that the "routing convergence timer value" is not
   the same value as the "network wide convergence time value".  Perhaps it is?

3) please give the Timer Param Sync protocol a clear name. Not crazy about
   that name.

Followup comments:

* While the document tried to describe the Timer Parameter functionality
  seperate from the first use of the parameter (fast-reroute), it failed to
  tell me anything about the new protocol other than bits on the wire.
  I would like the ISIS/OSPF diagrams to more cleary refer subtype to the new
  "Routing Timer Parameter Synchronization Registry".

  I'm unclear what a router does when it sees one of these parameters in the
  flood.  Does it flood the same value?  How does it's preference value
  interact with the value presented?

I think that this document might be better split up into two documents, one
explaining the Timer Parameter Sync protocol, and the other explaining how to
use it to implement the fast-reroute value.

I think that there are values where the converged value is MAX(values-seen),
and some that might be MIN(values-seen), and both might have hard coded upper
and lower bounds.  I wonder if the Timer Param Sync shouldn't describe the
parameter processing with another value?  Would it be useful for intermediate
routers to perform the MAX() or MIN() operation even if they don't understand
the parameter being synchronized?  Or should they drop these TLVs with
unknown sub-types?

I would feel happier with two documents as well because then for each
parameter being synchronized, the security considerations could more
reasonably explain what unreasonable values are, and how to recognize silly
values.  Security does not just defend against malicious actors, but also
just mis-configured (fat-fingered) ones.

Nits
pg2:
        s/parameter is fraught for two reasons/
          parameter is fraught with danger for two reasons/

pg3:
          Such consistency may be
          ensured by deploying automated means such as enforcing the new value
          by invoking the management interface of all involved routers.
   --> seems like a word might be missing?

section5.1:
        s/new router in introduced/new router is introduced/

---

From: Stewart Bryant <[email-masked]>
Date: Tue, 16 Jan 2018 17:12:38 +0000

Thank you for your review Michael.

>     perhaps other link-state algorithms, this should probably go into the
OK
>     the same value as the "network wide convergence time value".  Perhaps it is?
Yes. I think that is residue from a previous version. I will look at it.
> 3) please give the Timer Param Sync protocol a clear name. Not crazy about
It is a protocol to synchronize the value of timers. I suppose we could 
call it "Timer value synchronization protocol". Note it synchronizes the 
value of the timer so that a common timeout is used across the network 
rather than synchronizing the protocols. Would the WG prefer 
coordination to synchronization?

>    "Routing Timer Parameter Synchronization Registry".

I am not sure I understand your concern here. Are you concerned with the 
general definition
(which follows the tradition in the LS WGs) or with the application?

>    interact with the value presented?
This is link-state routing. Routers MUST flood link-state packets 
unchanged, so they are unchanged.
To change a value would break a protocol invariant of these routing 
protocols.
At the end of flooding all routers can see the preference of all other 
routers and use this
to pick min/max/something else as specified by the application from the 
set of values provided
by the set of routers.

> use it to implement the fast-reroute value.
I defer to the chairs on this.

> parameter processing with another value?
That is application specific, and I expect application to describe how 
the routers derive
the value from the data set.
> Would it be useful for intermediate
> the parameter being synchronized?
They don't and cannot.

> Or should they drop these TLVs with
> unknown sub-types?

It is a parameter of the TLV that it is to be flooded if unknown. The 
link state protocols
automatically flood LPS. That is a feature baked into the base protocol 
(ISIS and OSPF).

> just mis-configured (fat-fingered) ones.

Again, up to the Chairs, I can easily split if that is what the WG 
wants, but would hope
we do not have to go all the way back to individual submission.

>            parameter is fraught with danger for two reasons/
I am not sure danger is the right word here. No one is going to get 
physically harmed.
I will see if we can find a better expression to express this.

>     --> seems like a word might be missing?
Thanks, there is an "or" missing.
>          s/new router in introduced/new router is introduced/
Yes will fix.

Please can the chairs advice how they think I should proceed?

Best Regards

Stewart

---

From: "Les Ginsberg (ginsberg)" <[email-masked]>
Date: Tue, 16 Jan 2018 19:42:19 +0000

Let me comment on some things from the perspective of someone who provided a good deal of feedback on the draft, based on which the authors very kindly made significant revisions.
Inline.

> -----Original Message-----
> From: rtgwg [mailto:[email-masked]] On Behalf Of Stewart Bryant
> Sent: Tuesday, January 16, 2018 9:13 AM
> To: Michael Richardson <[email-masked]>; [email-masked]
> Cc: [email-masked]; [email-masked];
> [email-masked]
> Subject: Re: Rtgdir early review of draft-ietf-rtgwg-routing-timer-param-
> sync-00
> 
> 
> > perform an "early" review of a draft before it is submitted for
> > publication to the IESG. The early review can be performed at any time
> > during the draft's lifetime as a working group document. The purpose
> > of the early review depends on the stage that the document has reached.
> >
> It is a protocol to synchronize the value of timers. I suppose we could call it
> "Timer value synchronization protocol". Note it synchronizes the value of the
> timer so that a common timeout is used across the network rather than
> synchronizing the protocols. Would the WG prefer coordination to
> synchronization?
> 

[Les:] Calling this a "protocol" is inappropriate - and one of the things I originally objected to.
In fact the current version of the document does NOT make that claim - rather it is simply defining a modest extension to existing link state protocols.
What is being defined is the ability to advertise a class of parameters ("timers"). For each particular "timer" there will be defined a behavior as to how routers make use of the set of advertised values.
Let's please keep the scope of what is being done appropriate.

> general definition (which follows the tradition in the LS WGs) or with the
> application?
> 
> This is link-state routing. Routers MUST flood link-state packets unchanged,
> so they are unchanged.
> At the end of flooding all routers can see the preference of all other routers
> and use this to pick min/max/something else as specified by the application
> from the set of values provided by the set of routers.
> 
> >
> > I think that this document might be better split up into two
> > documents, one explaining the Timer Parameter Sync protocol, and the
> > other explaining how to use it to implement the fast-reroute value.

[Les:] I think this isn't necessary. The content of the draft is quite modest - splitting it into two drafts just makes more work for the authors and does not help the readers.
This is a simple protocol extension - which defines the ability to advertise a class of parameters - and it defines one use case.
If the text is unclear, I am sure the authors will be responsive to comments aimed at clarifying the text - but I really don't see the need for two documents.

> >
> > I think that there are values where the converged value is
> > MAX(values-seen), and some that might be MIN(values-seen), and both
> > might have hard coded upper and lower bounds.  I wonder if the Timer
> > Param Sync shouldn't describe the parameter processing with another
> value?
> That is application specific, and I expect application to describe how the
> routers derive the value from the data set.
> > routers to perform the MAX() or MIN() operation even if they don't
> > understand the parameter being synchronized?
> They don't and cannot.
> 

[Les:] Unknown TLVs/sub-TLVs are NEVER dropped - such action would break link state flooding.
They are silently ignored.

   Les

> It is a parameter of the TLV that it is to be flooded if unknown. The link state
> protocols automatically flood LPS. That is a feature baked into the base
> protocol (ISIS and OSPF).
> 
> >
> _______________________________________________
> rtgwg mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/rtgwg

---

From: Michael Richardson <[email-masked]>
Date: Tue, 16 Jan 2018 15:02:21 -0500

Stewart Bryant <[email-masked]> wrote:
    >> 3) please give the Timer Param Sync protocol a clear name. Not crazy
    >> about that name.

    > call it "Timer value synchronization protocol". Note it synchronizes
    > the value of the timer so that a common timeout is used across the
    > network rather than synchronizing the protocols. Would the WG prefer

I have particular opinion, but coordination does seem like a better term.

    >> Followup comments:
    >>
    >> * While the document tried to describe the Timer Parameter
    >> functionality seperate from the first use of the parameter
    >> (fast-reroute), it failed to tell me anything about the new protocol
    >> other than bits on the wire.  I would like the ISIS/OSPF diagrams to
    >> more cleary refer subtype to the new "Routing Timer Parameter
    >> Synchronization Registry".

    > I am not sure I understand your concern here. Are you concerned with
    > the general definition (which follows the tradition in the LS WGs) or
    > with the application?

I am concerned with the general definition. I found it abruptly short.

    >> I'm unclear what a router does when it sees one of these parameters in
    >> the flood.  Does it flood the same value?  How does it's preference
    >> value interact with the value presented?

    > unchanged, so they are unchanged.  To change a value would break a
    > protocol invariant of these routing protocols.  At the end of flooding
    > all routers can see the preference of all other routers and use this to
    > pick min/max/something else as specified by the application from the
    > set of values provided by the set of routers.

okay, but it's unclear in the document if a router that spoke ISIS on one
interface and OSPF on another would do something magic.  I think not, but
there is also discussion about devices flooding different values on different
interfaces... so maybe that's where I began to be confused.

    >> reasonably explain what unreasonable values are, and how to recognize
    >> silly values.  Security does not just defend against malicious actors,
    >> but also just mis-configured (fat-fingered) ones.

    > wants, but would hope we do not have to go all the way back to
    > individual submission.

Perhaps if there were seperate security considerations sections for the base
protocol, and then for the specific application which being proposed.

--
Michael Richardson <[email-masked]>, Sandelman Software Works
 -= IPv6 IoT consulting =-