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:rtgwg-bounces@xxxxxxxx] On Behalf Of Stewart Bryant > Sent: Tuesday, January 16, 2018 9:13 AM > To: Michael Richardson <mcr+ietf@xxxxxxxxxxxx>; rtg-dir@xxxxxxxx > Cc: draft-ietf-rtgwg-routing-timer-param-sync.all@xxxxxxxx; ietf@xxxxxxxx; > rtgwg@xxxxxxxx > Subject: Re: Rtgdir early review of draft-ietf-rtgwg-routing-timer-param- > sync-00 > > > Thank you for your review Michael. > > On 16/01/2018 16:32, Michael Richardson wrote: > > 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. > OK > > 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? > 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 > > that name. > 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. > > > > 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'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? > 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. > > > > > 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 defer to the chairs on this. > [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. > > Would it be useful for intermediate > > routers to perform the MAX() or MIN() operation even if they don't > > understand the parameter being synchronized? > They don't and cannot. > > > Or should they drop these TLVs with > > unknown sub-types? > [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). > > > > > 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. > > 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. > > > > > > > Nits > > pg2: > > s/parameter is fraught for two reasons/ > > 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. > > > > > 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? > Thanks, there is an "or" missing. > > section5.1: > > 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 > > > > > > > > > > _______________________________________________ > rtgwg mailing list > rtgwg@xxxxxxxx > https://www.ietf.org/mailman/listinfo/rtgwg