Last Call Review of draft-ietf-i2rs-rib-info-model-14
review-ietf-i2rs-rib-info-model-14-genart-lc-yee-2018-02-23-00

Request Review of draft-ietf-i2rs-rib-info-model
Requested rev. no specific revision (document currently at 17)
Type Last Call Review
Team General Area Review Team (Gen-ART) (genart)
Deadline 2018-02-23
Requested 2018-02-09
Other Reviews Rtgdir Early review of -08 by Ravi Singh (diff)
Rtgdir Early review of -11 by Henning Rogge (diff)
Opsdir Early review of -12 by Mahesh Jethanandani (diff)
Secdir Last Call review of -14 by Paul Wouters (diff)
Opsdir Last Call review of -12 by Mahesh Jethanandani (diff)
Review State Completed
Reviewer Peter Yee
Review review-ietf-i2rs-rib-info-model-14-genart-lc-yee-2018-02-23
Posted at https://mailarchive.ietf.org/arch/msg/gen-art/TZmSF1rYTn8hSn29ebRbjzwg62s
Reviewed rev. 14 (document currently at 17)
Review result Ready with Issues
Draft last updated 2018-02-23
Review completed: 2018-02-23

Review
review-ietf-i2rs-rib-info-model-14-genart-lc-yee-2018-02-23

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

<https://trac.ietf.org/trac/gen/wiki/GenArtfaq>.

Document: draft-ietf-i2rs-rib-info-model-14
Reviewer: Peter Yee
Review Date: 2018-02-22
IETF LC End Date: 2018-02-23
IESG Telechat date: 2018-03-08

Summary: This Informational draft specifies an information model for routing information bases, providing modeling of the internal information of a router or similar network device.  The draft is mostly ready, but has some issues that should be considered. [Ready with issues]

Major issues: None

Minor issues: 

Page 4, 3rd full paragraph, 1st sentence: the draft introduces the concept of "RIB clients" in Figure 1, notes that they are generally routing protocols, and then never uses the term again.  All other references to the what must be the users of the northbound interface are then called "external entities" for the most part.  This is confusing because the term "external entity" is not defined nor fully equated with "RIB client".  The term also seems to indicate that the "external entity" is not necessarily running on the network device.  While that might be one way of looking at the feeding of data into the RIB via NETCONF or RESTCONF, that doesn't seem to be the case for a routing protocol.  A fuller explanation of the users of the northbound interface and a revision to Figure 1 might help clarify this situation.

Page 7, 1st paragraph after bullet list, last sentence: Routing instances are identified by ROUTER_IDs anyhow, so this sentence seems superfluous.  Perhaps you are trying to get across the point that the ROUTER_ID (which is definitely present for the router) is not *used* by this routing instance.

The term "ethernet" is used in several places in the document.  Except in the grammar of section 6, change these to the capitalized "Ethernet".  This brings up a larger point, however.  Not all IEEE MAC addresses are associated with Ethernet interfaces and I believe this document is expected to be applicable to other IEEE 802 MACs such as IEEE 802.11 (WLAN) and IEEE 802.15 (WPAN).  IEEE 802.15 has long and short forms of MAC addresses, so you may want to give some additional thought to what you want to say with this term and pick something more appropriate.

I think there's a discussion missing that may or may not be within scope of this document.  RIBs appear to be typically divided according to the protocol for which they are providing routing (IPv4, MPLS, etc.)  Section 7.1 discusses routing preferences, with an example of OSPF route and a route from some other protocol.  When the OSPF route is withdrawn, the other route is installed in the FIB.  What's not clear is what makes the decision to do this and cause a specific RIB to push its route into the FIB.  Is that the routing instance or the RIB manager?  A routing instance is described as a set of interfaces, RIBs, and routing parameters.  It's description does not make it clear that it arbitrates among the RIBs or the routing protocols which are using the northbound interface to talk to the RIB manager.  Figure 1 makes it seem like there is a RIB manger per RIB, in which case how can the RIB manager make decisions between multiple RIBs?

Page 14, Section 3, 2nd paragraph, 2nd sentence: a "connection" is mentioned here.  This document purports to deal with an API (and one that would mostly be used by local routing protocols if the figures are to be believed) and hasn't otherwise made any mention of a connection, let alone what constitutes a connection and defines its lifetime.  More discussion is needed of this concept instead of just (possibly) resting the whole thing on brief mentions of NETCONF and RESTCONF (which aren't even brought into the picture until the Security Considerations section later on in the document).

Page 15, 1st partial paragraph: there are unstated assumptions about needing a subscription mechanism for subscribing to notifications, particularly notifications from RIBs that were not created by the entity.  (This goes back to the concept on the previous page that entities may possibly read to or write from RIBs they did not create.)  The discussion of notifications could use some fleshing out here.

Nits/editorial comments: 

General:

Append a comma after "i.e." and "e.g."  Make all uses of "e.g." lower case.  Some uses of "e.g." have double spaces after them and those double spaces should be replaced with single spaces.

Change "use-cases" to "use cases" throughout the document.  Or use the other way around.  Just be consistent in the usage.  Non-hyphenate usage appears to be preferred.

Insert a blank line before and after bullet lists for readability.  Consider adding a blank line between entries to aid readability as well.

Specific: 

Page 1, Abstract, 2nd sentence: delete the semicolon.  

Page 1, Abstract, 4th sentence: replace the space between "higher" and "level" with a hyphen.

Page 3, Section 1, 2nd sentence: change "config" to "configuration information" (or something similar).

Page 3, Section 1, 3rd sentence: change "north-bound" to "northbound".  Append a comma after each of "clients", "i.e.", and "protocols".

Page 3, Figure 1 caption: append a comma after "clients".

Page 4, 1st partial paragraph, 1st complete sentence: change "which" to "that".  Append a comma after "policies".

Page 4, 1st partial paragraph, 4th complete sentence: replace the space between "publicly" and "documented" with a hyphen and then append a comma after "publicly-documented".

Page 4, 2nd full paragraph, 1st sentence: I'm not sure what "show output screen scraping" is.  I'm familiar with screen scraping, but could not find a good source for your term.  Perhaps you could explain or modify it?

Page 5, Section 1.1: you may wish to reference RFC 8174, which updates RFC 2119 and makes it applicable across more than Standards Track documents.

Page 5, Section 2, 4th sentence: delete the comma after "single".

Page 5, Section 2, 5th sentence: make "Section" lower case.

Page 5, Section 2, 6th sentence: change the space between "high" and "level" to a hyphen.

Page 5, Figure 2: remove the space between "routing-instance" and "(s)".

Page 5, Section 2.1, 3rd sentence: change "intance" to "instance".  Also fix the sentence or Figure 2: the sentence says 1 or more RIBs, the diagram shows 0 or more.  I'm not sure how a zero RIB routing instance is useful, but make the two ranges consistent.

Page 6, 1st paragraph, 1st sentence: delete this sentence as it is redundant with information given in the previous paragraph on Page 5.

Page 6, 2nd paragraph, 1st sentence: Change "a" to "an" before "ENABLE_IP_RPF_CHECK".  Capitalize each word in "Reverse path forwarding".

Page 6, 2nd paragraph, 2nd sentence: delete "Reverse path forwarding", insert the word "The" at the beginning of the sentence and remove the parentheses around "RPF".

Page 6, 2nd paragraph, 3rd and 4th sentences: change "rpf" to "RPF".

Page 6, Section 2.2, 1st paragraph, 3rd sentence: delete both semicolons.

Page 6, "interface-list" bullet item, 3rd sentence: I think it reads better with "in" inserted before "on".

Page 7, "ROUTER_ID" bullet item: change "router-id" to "ROUTER_ID".  Or if you want a descriptive term, change it to "router identification".

Page 8, MPLS bullet item: "change "a" to "an".

Page 8, paragraph after "route-vendor-attributes" bullet item, 1st sentence: change "Nexthop" to "nexthop".

Page 9, 1st partial paragraph, 4th full sentence: change "a" to "an" before "MPLS".

Page 9, 1st partial paragraph, 7th full sentence: append a comma after "Conversely".  Insert "the" before "case".

Page 10, 1st paragraph, 1st sentence: put a comma after "extensible".

Page 10, 1st paragraph, 5th sentence: change "it's" to "its".

Page 11, "EGRESS_INTERFACE" sub-bullet item: append a comma after "logical".

Page 11, "EGRESS_INTERFACE and IP address" sub-bullet item: append a comma after "cases".

Page 11, "Tunnel nexthops" bullet item: change "Various" to "The".

Page 11, "tunnel encap" sub-bullet item: change "tunnel encap" to "tunnel-encap" in the sub-bullet title to match Figure 4.  Change "encap" to "encapsulation" in the first sentence.  Change "tunnel encap" in the 2nd sentence to "tunnel-encap".

Page 11, "tunnel decap" sub-bullet item: change "tunnel decap" to "tunnel-decap" in the sub-bullet title to match Figure 4.  Change "decap" to "decapsulation" in the second sentence.  Change the "a" before "EGRESS_INTERFACE" to "an" in the third sentence.

Page 11, "logical tunnel" sub-bullet item: change "logical tunnel" to "logical-tunnel" in the sub-bullet title to match Figure 4.  Change the "a" before "MPLS" to "an".

Page 11, last (partial) paragraph, 2nd partial sentence: change "end-point" to "endpoint".

Page 12, Section 2.4.1.1, 1st sentence: change "drop" to "discard" to match the following discussion.

Page 12, Section 2.4.2, 1st paragraph after bullet items, 1st sentence: delete the comma.

Page 12, Section 2.4.2, 1st paragraph after bullet items, 3rd sentence: delete the comma.

Page 12, Section 2.4.2, 1st paragraph after bullet items, 6th sentence: change "and" to "or".  Make "header" plural.

Page 13, Section 2.4.2.1, "NEXTHOP_PREFERENCE" bullet item, 4th sentence: insert "the" before "two".

Page 13, Section 2.4.2.1, "NEXTHOP_PREFERENCE" bullet item, last sentence: delete the asterisk and join "(Section 6)" with the rest of the sentence.

Page 14, Section 3, 1st paragraph, 1st sentence: delete the comma.

Page 14, Section 3, 2nd paragraph, 2nd sentence: change "agent" to "entity" to at least be consistent with prior usage in the document.  But also refer back to the issue listed above about use of the term "external entity".

Page 14, Section 4, 1st paragraph, 1st sentence: delete the comma.

Page 18, Section 6.1, 2nd sentence: append a comma after "preference".  Change "multicasted" to "multicast" (the preferred form of the word).

Page 18, Section 7.2.1, last sentence: change "encap" to "encapsulation".  Change "decap" to "decapsulation".

Page 21, Section 7.2.6, 2nd sentence: delete the comma.

Page 21, Section 7.2.6, 5th sentence: change "Lets" to "Let's".

Page 23, Section 8, 2nd sentence: delete the comma.

Page 24, Section 11, 1st sentence: append a comma after "co-chair".  Change the first occurrence of "on" to "for".

Page 24, Section 11, 2nd sentence: append a comma after "Hares".  Yeah, yeah, I know, no one is going to require the Oxford comma here to figure out what the sentence means. ;-)