Peter, thanks for your review. Authors, thanks for addressing some of Peter’s comments. I tried to pick out those of his comments that remain unaddressed and put them in my No Objection ballot.
Alissa > On Feb 23, 2018, at 8:35 PM, Peter Yee <pe...@akayla.com> wrote: > > Reviewer: Peter Yee > Review result: Ready with Issues > > 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. ;-) > > _______________________________________________ > Gen-art mailing list > gen-...@ietf.org > https://www.ietf.org/mailman/listinfo/gen-art _______________________________________________ i2rs mailing list i2rs@ietf.org https://www.ietf.org/mailman/listinfo/i2rs
