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

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