Last Call Review of draft-ietf-lsr-isis-fast-flooding-07
review-ietf-lsr-isis-fast-flooding-07-secdir-lc-leiba-2024-03-13-00
Request | Review of | draft-ietf-lsr-isis-fast-flooding |
---|---|---|
Requested revision | No specific revision (document currently at 11) | |
Type | Last Call Review | |
Team | Security Area Directorate (secdir) | |
Deadline | 2024-02-29 | |
Requested | 2024-02-15 | |
Authors | Bruno Decraene , Les Ginsberg , Tony Li , Guillaume Solignac , Marek Karasek , Gunter Van de Velde , Tony Przygienda | |
I-D last updated | 2024-03-13 | |
Completed reviews |
Tsvart Last Call review of -07
by Mirja Kühlewind
(diff)
Secdir Last Call review of -07 by Barry Leiba (diff) Genart Last Call review of -07 by Ines Robles (diff) Rtgdir Last Call review of -05 by Loa Andersson (diff) Tsvart Early review of -06 by Mirja Kühlewind (diff) |
|
Assignment | Reviewer | Barry Leiba |
State | Completed | |
Request | Last Call review on draft-ietf-lsr-isis-fast-flooding by Security Area Directorate Assigned | |
Posted at | https://mailarchive.ietf.org/arch/msg/secdir/db2YqUWtKn3660Y-JdwI5Ts3cuI | |
Reviewed revision | 07 (document currently at 11) | |
Result | Has issues | |
Completed | 2024-03-13 |
review-ietf-lsr-isis-fast-flooding-07-secdir-lc-leiba-2024-03-13-00
Only some minor things here: — Section 3 — Although modern implementations have not strictly adhered to the 33 millisecond interval, it is commonplace for implementations to limit the flooding rate to the same order of magnitude similar as the 33 ms value. This sentence seems ungrammatical. I think I know what you’re saying, so perhaps this will work?: NEW Although modern implementations have not strictly adhered to the 33 millisecond interval, it is commonplace for implementations to limit the flooding rate to the same order of magnitude: tens of milliseconds, and not the single digits or fractions of milliseconds that are needed today. END If that’s not quite right, please riff on it as appropriate. — Section 4 — For a parameter which has never been advertised, an IS SHOULD use its local default value. That value SHOULD be configurable on a per-node basis and MAY be configurable on a per-interface basis. Nit: I think the first SHOULD here ought not to be a BCP 14 key word, and only the second is. I would write the first part of the sentence as a fact, and only have the second be a directive: NEW For a parameter that has never been advertised, an IS uses its local default value. That value SHOULD be configurable on a per-node basis and MAY be configurable on a per-interface basis. END — Section 4.4 — Length: Indicates the length in octets (1-8) of the Value field. The length SHOULD be the minimum required to send all bits that are set. The SHOULD seems very odd: what would be a good reason to make it longer than necessary? Is there a real reason not to straightforwardly say, “The length is the minimum required…”? — Section 6 — Just a “thanks” comment here: I found Section 6 and its subsections to be clear and informative. — Section 8 — I think the additional implications of having the new TLV have been well thought out, and I don’t see anything missing.