Skip to main content

Telechat Review of draft-ietf-rift-rift-21
review-ietf-rift-rift-21-intdir-telechat-thaler-2024-04-25-00

Request Review of draft-ietf-rift-rift
Requested revision No specific revision (document currently at 24)
Type Telechat Review
Team Internet Area Directorate (intdir)
Deadline 2024-04-25
Requested 2024-04-02
Requested by Éric Vyncke
Authors Tony Przygienda , Jordan Head , Alankar Sharma , Pascal Thubert , Bruno Rijsman , Dmitry Afanasiev
I-D last updated 2024-04-25
Completed reviews Intdir Telechat review of -21 by Dave Thaler (diff)
Rtgdir Last Call review of -20 by Loa Andersson (diff)
Secdir Early review of -04 by Scott G. Kelly (diff)
Rtgdir Early review of -06 by Russ White (diff)
Genart Early review of -08 by Robert Sparks (diff)
Secdir Early review of -08 by Scott G. Kelly (diff)
Opsdir Early review of -08 by Nagendra Kumar Nainar (diff)
Rtgdir Early review of -08 by Jonathan Hardwick (diff)
Assignment Reviewer Dave Thaler
State Completed
Request Telechat review on draft-ietf-rift-rift by Internet Area Directorate Assigned
Posted at https://mailarchive.ietf.org/arch/msg/int-dir/KitnVPj8eJD9PVLcIteVFhx84o8
Reviewed revision 21 (document currently at 24)
Result Ready w/issues
Completed 2024-04-25
review-ietf-rift-rift-21-intdir-telechat-thaler-2024-04-25-00
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@juniper.net) > Contact: Jordan Head (jhead@juniper.net) RFC 6335 states:
“For assignments done through RFCs published via the "IETF Document Stream"
[RFC4844], the Assignee will be the IESG <iesg@ietf.org>.“ and “For assignments
done through RFCs published via the "IETF Document Stream" [RFC4844], the
Contact will be the IETF Chair <chair@ietf.org>.“

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