Reviewer: Dave Thaler Review result: Ready with Issues As assigned INT directorate reviewer for draft-ietf-rift-rift my comments were written primarily for the benefit of the Internet Area Directors. Document editors and shepherd(s) should treat these comments just like they would treat comments from any other IETF contributors and resolve them along with any other Last Call comments that have been received. For more details on the INT Directorate, see <https://datatracker.ietf.org/group/intdir/about/>. A summary of my main feedback is in the list below, but a bunch of other editorial nits are called out in comments or redlines at: PDF: https://1drv.ms/b/s!Aqj-Bj9PNivcn-5XwvmeGhtn6fM2pQ?e=tsUrGY DOCX: https://1drv.ms/w/s!Aqj-Bj9PNivcn-5VtiY6w4aezViPGw?e=q0ebxd 1. Figure 1 does not mention the address family, nor does the text discussing it. But the text says "A/32 is node A's loopback" which would seem to imply that this figure is IPv4-only. Clarify. 2. There are some gratuitous differences in labels between ASCII diagrams and SVG diagrams in the HTML/PDF versions. Some differences are understandable but some are just gratuitous. E.g., Figure 10 "ToF" vs "Top-of-Fabric" and "(depth)" vs "(in depth)", etc. 3. The draft says that messages go over link-scoped multicast (or in one variant, broadcast). But it allows both TTL=1 and TTL=255. I'd have expected it to say that the mitigation against things counting down to TTL=1 is to drop packets that were received on a destination address other than the link scoped multicast/broadcast address (i.e., drop unicast/anycast packets), but the spec has no such security protection. Since sending to multicast/broadcast is a MUST, why not say that receivers should check that? 4. Section 6.3.1 says "TIEs MUST be sent with an IPv4 Time to Live (TTL) or an IPv6 Hop Limit (HL) of either 1 or 255 and also MUST be ignored if received with values different than 1 or 255. This prevents RIFT information from reaching beyond a single L3 next-hop in the topology." No it doesn’t "prevent" it… Since a sender could send with TTL=255 and it arrives with TTL=1. It’s the fact that you use link-scoped multicast addresses and only accept packets that arrive on that address (NOT unicast, for example) that actually “prevents” it. 5. Section 9.2 says "RIFT explicitly requires the use of a TTL/HL value of 1 *or* 255 when sending/receiving LIEs and TIEs so that implementors have a choice between the two." On receiving, it seems like there is no choice, one has to support both in order to ensure interoperability with senders that get a choice. The text implies that receivers can choose to just support one of the two, which I don’t think is the case." 6. Page 37 says "A simplified version MAY be implemented on platforms with … no multicast support … by … sending and receiving LIE frames on IPv6 all routers multicast address". This sentence doesn’t make sense. It says if you have “no” multicast support then you can send and receive on this multicast address, which is a contradiction. Perhaps delete “or no” and let “limited” cover the intended meanings 7. Are rows in Table 1 symmetric? e.g., there’s a row for Local=IPv4,IPv6 and Remote=IPv6 but not vice versa. Also, what about a row for Local=IPv4,IPv6 and Remote=IPv4 (including vice versa)? 8. The last row in Table 2 says "If IPv4 and IPv6 LIEs advertise conflicting _ipv4_forwarding_capable_ flags, the behavior is unspecified." How can that happen? The top of page 38 says “If IPv4 forwarding is supported on an interface, _ipv4_forwarding_capable_ MUST be set to true for all LIEs advertised from that interface”. So when could it be false in this row? Maybe just delete this sentence due to that MUST. 9. Section 6.3.3 says "The TIEHEader can also carry an _origination_time_ schema element (for fabrics that utilize precision timing) which contains the absolute timestamp". Section 6.8.4 does talk about clock synchronization so I'd suggest providing a forward reference to that section. 10. Section 6.3.3.1.5 says "On a periodic basis all TIEs with lifetime left > 0 MUST be sent out on the adjacency, removed from TIES_TX list and requeued onto TIES_RTX list. The specific period is out of scope for this document." Are there any constraints? Is once a century considered compliant? What’s the point of having a MUST if there’s no way to tell whether an implementation complies? Similarly section 9.1 says "a node SHOULD implement a strategy of discarding contents of all TIEs that were not present in the SPF tree over a certain, configurable period of time". Are there any recommendations on constraints (whether max or min) on this period?" 11. Section 6.3.9 says "nodes at the same level do *not* have to agree on a specific algorithm to select the FRs, but overall load balancing should be achieved so that different nodes at the same level should tend to select different parents as FRs". Perhaps informatively cite RFC 2991 here, which discusses several potential algorithms? 12. Section 10.1 requests assignments in the "Service Name and Transport Protocol Port Number Registry" with: > Assignee: Tony Przygienda (prz@xxxxxxxxxxx) > Contact: Jordan Head (jhead@xxxxxxxxxxx) RFC 6335 states: “For assignments done through RFCs published via the "IETF Document Stream" [RFC4844], the Assignee will be the IESG <iesg@xxxxxxxx>.“ and “For assignments done through RFCs published via the "IETF Document Stream" [RFC4844], the Contact will be the IETF Chair <chair@xxxxxxxx>.“ 13. Table 9 has AddressFamilyMinValue. What does this mean? It’s not used anywhere in the main protocol description, so what does it mean to have the min value not be IPv4? 14. Many of the tables in the IANA considerations section have blanks in the description column. 15. RFC 8126: Guidelines for Writing an IANA Considerations Section in RFCs (rfc-editor.org) has requirements for defining registries, which this document does. Among the requirements is "Size, format, and syntax of registry entries" which in my view this document does not do adequately. For example, is it ok for Description to be empty? What about "Max Schema Version"? Must a contact or assignee be provided? Only the fields that are columns in table 35 and no more? Any constraints on the "name" field in Table 35, e.g., can it contain spaces, punctuation, or non-ASCII characters? 16. Table 51 has "bandwidth", but in what units? Would using different units require registering another name? 17. A couple rows in table 53 have "optional" in the text of the description column. So everything that doesn’t have “optional” in the description column is mandatory? Why not have a separate column for that then? 18. Table 59 "from_link" description has "Link to which the address belongs to." Besides the obviously poor grammar of redundant "to", it doesn't say how the link is identified. Should it be "Link ID of the link to which the address belongs"? Table 71 has "Remaining lifetime" but has no units. Is it "in seconds"? Thanks, Dave -- last-call mailing list last-call@xxxxxxxx https://www.ietf.org/mailman/listinfo/last-call
