[Gen-art] FW: [i2rs] Genart last call review of draft-ietf-i2rs-rib-info-model-14

["Susan Hares" <[e-mail]>]

Alissa:

 

Here’s Nitin’s response to Peter. 

 

Sue Hares 

 

From: i2rs [mailto:[e-mail]] On Behalf Of Nitin Bahadur
Sent: Friday, March 23, 2018 2:55 AM
To: [e-mail]
Cc: [e-mail]
Subject: Re: [i2rs] [Gen-art] Genart last call review of draft-ietf-i2rs-rib-info-model-14

 

Hi Peter,

 

      Thanks for the Gen-art review. Sorry it missed my attention back when you sent it. Please see NB> below for comments.

 

 

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%3E.> 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.
 
NB> You are right. I’m updating the draft to replace “external entities” to “RIB client”.
 
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.
 
NB> That sentence was requested by some folks to ensure that ROUTER_ID based conflicts do not occur.
 
The term "ethernet" is used in several places in the document.  Except in the
grammar of section 6, change these to the capitalized "Ethernet". 
 
NB> Done
 
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.
 
NB> In Section 2.4.1, when talking about the IEEE MAC we say that “The egress interface 
must be an Ethernet interface.” So the MAC is to be used only in the context of Ethernet interfaces.
 
NB> With regards to short and long form of MAC addresses, these should get specified in the data-model, 
since the data-model will need to validate things.
 
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? 
 
NB> It should be 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?
 
NB> Good point. I’ll change the figure to indicate that the RIB manager handles 1 or more 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).
 
NB> Removed the sentence related to “connection”. You are right that this sentence causes more confusion and little value.
 
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.
 
NB> How notifications are sent, whether using a subscription model or something else is left to the Data Model. And it’s left un-fleshed to allow the data-model writers to decide what to do here.
 
Nits/editorial comments:
 
NB> I’m addressing all your nits in version 15. For nits not being addressed, see NB> below.
 
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?
 
NB> Modified it to “screen scraping”
 
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.
 
NB> Done
 
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.
 
NB> Updated
 
Page 6, 1st paragraph, 1st sentence: delete this sentence as it is redundant
with information given in the previous paragraph on Page 5.
 
NB> While it is redundant, it adds clarity where the item is being defined.
 
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. ;-)
 
Thanks
Nitin