Including the authors of this draft and the WG list. Thank Ravi for the review efforts.
Cheers, Xian From: Ravi Singh [mailto:[email protected]] Sent: 2016年5月18日 0:55 To: [email protected] Cc: 'Jonathan Hardwick'; 'Jon Hudson'; Susan Hares; Zhangxian (Xian) Subject: RE: Routing directorate QA review of draft-ietf-i2rs-rib-info-model 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
_______________________________________________ i2rs mailing list [email protected] https://www.ietf.org/mailman/listinfo/i2rs
