Subject: Re: [RTG-DIR] Routing directorate QA review of draft-ietf-i2rs-rib-info-model

From: "Susan Hares" <[email-masked]>
Date: Mon, 6 Jun 2016 20:09:12 -0400

Thank you on behalf of I2RS WG and the authors. 

Sue 

From: rtg-dir [mailto:[email-masked]] On Behalf Of Ravi Singh
Sent: Tuesday, May 17, 2016 12:55 PM
To: [email-masked]
Cc: 'Jonathan Hardwick'; Zhangxian (Xian); Susan Hares; 'Jon Hudson'
Subject: Re: [RTG-DIR] 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