Early Review of draft-ietf-i2rs-rib-info-model-08
review-ietf-i2rs-rib-info-model-08-rtgdir-early-singh-2016-05-19-00

Request Review of draft-ietf-i2rs-rib-info-model
Requested rev. no specific revision (document currently at 17)
Type Early Review
Team Routing Area Directorate (rtgdir)
Deadline 2016-05-19
Requested 2016-05-03
Other Reviews Rtgdir Early review of -11 by Henning Rogge (diff)
Opsdir Early review of -12 by Mahesh Jethanandani (diff)
Genart Last Call review of -14 by Peter Yee (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 Ravi Singh
Review review-ietf-i2rs-rib-info-model-08-rtgdir-early-singh-2016-05-19
Posted at https://www.ietf.org/mail-archive/web/rtg-dir/current/msg02930.html
Reviewed rev. 08 (document currently at 17)
Review result Has Issues
Draft last updated 2016-05-19
Review completed: 2016-05-19

Review
review-ietf-i2rs-rib-info-model-08-rtgdir-early-singh-2016-05-19






Hi




I had been designated the RTG-DIR QA-reviewer for


https://tools.ietf.org/html/draft-ietf-i2rs-rib-info-model-08




 




I reviewed this doc.




Overall, the doc is clear and does a decent job of creating a RIB model.




However, I have a minor concern with the tone of the doc at certain places.





The document, at places, reads like a requirements doc specifying what an implementation of the RIB "SHOULD"/MUST do.





I am not sure if that is correct form for an informational draft documenting a specific RIB model.





Examples of such instances would be:







A.

     


Section 8







B.

     


Section 9







C.

     


Wherever in the doc a "SHOULD" or "MUST" shows up stating desirability of certain behavior of an external entity accessing the RIB.




 




An aspect that has not been touched-upon in the document, that however might be worthy of consideration is about how this RIB model accommodates
 an external input about traffic-statistics-monitoring desired for the various constructs.







Specific comments on the various sections in the text:







1.

      


Introduction:







a.

      


First 2 paras: some typos and sentences with redundant words.




 







2.

      


2.1:







a.

      


"type" is somewhat ambiguous. Suggest reword "type" as "address-family"




 







3.

      


2.2: 







a.

      


Some sentences could be made shorter/broken-up to improve readability of this section.







b.

     


Interface_list and router-id: For a functioning routing-instance, can't think of a routing-instance without either of those defined. So, either the optionality aspect needs to be changed to "required"
 or specify how a routing-instance would work with either missing.







c.

      


Interface-list: per-interface parameters could also be listed (since the interface-list is called out in a RIB model): address, families, MTU, extensibility-consideration-for-other-interface-attributes




 







4.

      


2.3:







a.

      


ROUTE_PREFERENCE: The text is mixing-up route-preference with "route-metric". Administrative-distance (the route metric) is the IGP cost of a route.







Both route_preference and route-metric would be attributes of the route.







b.

     


An additional attribute that should be included is "installing protocol". That would require defining a list of protocols that may install a route.




 







5.

      


2.4:







a.

      


Second paragraph could use rewording to enhance clarity. Specifically:







                                      


i.

            

Need to mention about "(appearing to be) directly connected IP" to distinguish between:







1.

      


Nexthops that don't need to be resolved (by other RIB events) to be installable







2.

      


Nexthops that need to be resolved (by other RIB events/properties) to be installable:







a.

      


Those that are currently resolved







b.

     


Those that are currently not-resolved







b.

     


Next-hop property should also include IP of (appearing to be) locally-connected device for which to ARP




 







6.

      


2.4.1:







a.

      


Last paragraph: "preceded by" would be more accurate than "followed by"





 







7.

      


2.4.3:







a.

      


Under "tunnel encap": The following text








"







An optional







      egress interface can be chained to the tunnel encap to indicate







      which interface to send the packet out on.  The egress interface







      is useful when the network device contains Ethernet interfaces and







      one needs to perform address resolution for the IP packet."








appears a bit incorrect.







If one wishes to do resolution for the tunnel-remote-dst then specifying an interface serves no purpose. Either that address does not need resolution and this specified interface is
 a p2p interface or there is a need for resolution (without needing to specify an interface-name). Can't be both.







 







8.

      


Sections 4 & 5 can be merged. What is the point of having a separate section 5 when it is not really saying anything new beyond what text exists in section 4.







 







9.

      


Section 6:







a.

      


Not repeating remarks made about specific attributes (listed above) for each item in the BNF. Eg. Route-metric/preference related remark made above about 2.3.







 







b.

     


In-label is not logically a nexthop attribute. It is infact a route. This should be fixed.







  <mpls-label-operation> ::= (<MPLS_PUSH> <MPLS_LABEL> [<S_BIT>]







                                          [<TOS_VALUE>] [<TTL_VALUE>]) |







                             (<MPLS_SWAP> <IN_LABEL> <OUT_LABEL>







                                         [<TTL_ACTION>])







c.

      


VXLAN headers needs to have a way to specify src/dst MAC in inner header, since it is possible to use VXLAN as a general-purpose encapsulation without L2-learning semantics.







 







10.

  


Section 6 describes the RIB grammar. The nexthop grammar is a part of that. However, some of that sub-grammar appears under section 7.







 







11.

  


Section 7 "Using the RIB grammar" starts out by explaining how the complex nexthops maybe used. However, it ends up being a listing of the nexthop sub-grammar which should really have been listed in section
 6 along with the RIB grammar.







I'd suggest either take the entirety of the next-hop grammar listing to the section 6, or break section 7 so that the next-hop grammar is listed in section 7 & the "using the rib" grammar
 is a purely text only description of Rib/NH grammar maybe used.







 







12.

  


Syntax for <nexthop-replicate>  needs to be reconciled beween section 7.2.3 and section 6 where








there is an syntax mismatch,







Doesn’t section 6 need to say:








<nexthop-replicate> ::= <NEXTHOP_REPLICATE> <nexthop> <nexthop> ...




 




Regards




Ravi