I2RS WG and draft authors,
Below is some feedback on the text of draft-ietf-i2rs-rib-data-model-03.
Thanks,
Chris
------------------------
It would be good to provide a way to get the yang model text file without ietf
draft headers interspersed throughout, so that people can run it through pyang
more easily.
One way to do this is to publish the xml of the draft via the draft submission
tool.
Another common way is github, but I couldn't find it there.
------------------------
The description of the nexthop-preference-def (see below) is confusing. First,
I assume there is an error in the text since the example of downloading
primary/standby/tertiary nexthops to the FIB should presumably select three
nexthops, but the text refers to selecting the two resolved nexthops with the
highest preference . More fundamentally, this example seems to imply that
only two next-hops will get downloaded to the FIB, whereas one could imagine
an implementation that uses three, four, five or more nexthops of different
preferences. If hardware supports more than two next-hop preferences being
installed in the FIB, then what is the mechanism for the client to learn how
many preference values are supported? This should all be made clearer in the
text.
typedef nexthop-preference-def {
type uint8 {
range "1..99";
}
description
"Nexthop-preference is used for protection schemes.
It is an integer value between 1 and 99. A lower
value indicates higher preference. To download a
primary/standby/tertiary group to the FIB, the
nexthops that are resolved and have two highest
preferences are selected.";
}
------------------------
If I understand the yang syntax and semantics correctly, the "nh-add" RPC
creates a nexthop in a given RIB that is not associated with any match
condition on a route. I assume the intention is to create a nexthop with a
nexthop-id but not associated with any prefix that can then be referenced by
multiple other route match conditions. This seems like a reasonable thing to
do. However, I can see two possible issues with this.
The first issue is that the structure of the data model doesn't seem to not
allow this. "grouping route" uses "grouping route-prefix" next to "container
next-hop". It appears to me that as currently written "container match" in
"grouping route-prefix" is a mandatory node based on section 3.1 of RFC6020 ,
since the "mandatory" statements below choice/cases cascade up to the
container. So the current form of the "nh-add" RPC may not be consistent
with the data model as currently defined.
The second issue is that creating a next-hop without an associated match
appears to differ from the RIB grammar defined in section 6 of
draft-ietf-i2rs-rib-info-model-08. In the RIB info model, it seems that the
only way for a <nexthop> to appear is together with a <match>.
<route> ::= <match> <nexthop>
[<route-attributes>]
[<route-vendor-attributes>]
It would be good to address these inconsistencies in some way.
------------------------
In the definition of nh-add, "rpc nh-add { input { " uses "grouping next-hop"
which has the leaf nexthop-id as a mandatory element. Based on section 7.13.2
of RFC 6020, it seems that the nexthop-id would need to be present in the RPC
invocation defined by "rpc nh-add". This seems inconsistent with the
description of nh-add in section 2.5 of this document where no nexthop-id is
supplied in the input, and instead the network element allocates a nexthop-id
and returns it in the output of the rpc.
It would be good to address this inconsistency.
------------------------
Treating tunnel-decap as a type of next-hop doesn't make sense to me. I assume
the desire to include tunnel-decap as well as tunnel-encap is to have a usable
stand-alone data model module which to deal with some use cases without having
to rely on another module that defines tunnel-decap. However, the result
doesn't make sense. I think the most common scenario involving routers would
be to install a route on router A for prefix P whose nexthop is a tunnel-encap
whose destination is router B. One router B one would need to install a
corresponding tunnel-decap whose source is router A. The most common scenario
is then that router B does a normal route lookup on the decapsulated packet
independent of the interface it entered on. I don't see how this most common
use case would be handled with the tunnel-decap next-hop in this document.
Section 7.2.5. of draft-ietf-i2rs-rib-info-model-08 gives an example usage for
tunnel-decap where the tunnel-decap nexthop is used to pop an MPLS label and
send the traffic out an egress interface next-hop. This example is not the
most common scenario. And if we want to accomplish this scenario of
decapsulating and sending to a particular nexthop, it makes more sense to treat
tunnel-decap as a route match condition similar to an interface-route in the
existing data model. However, I think the model should be able to handle the
more common scenario described above when traffic needs to be decapsulated and
routed based on a normal route lookup.
Treating tunnel-decap as a next-hop type really makes no sense to me. I think
this aspect of the model should be changed.
------------------------
The description of "identity ttl-action" on p.22 is missing the action
"decrease-and-copy-to-next". I suggest either adding it to the description or
getting rid of the other 3 from the description. Listing only 3 of 4 options
is confusing.
------------------------
These are some typos that I noticed as well.
------------------------
s/Struture/Structure/
Figure 2: Routing Instance Stuture
------------------------
s/nexhtop/nexthop/
There are several occurrences of this misspelling.
------------------------
s/tunnel-decap-nexthp/tunnel-decap-nexthop/
------------------------
s/attribtes/attributes/
------------------------
s/nexhop/nexthop/
------------------------
s/usesd/used/
on p34.
------------------------
_______________________________________________
i2rs mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/i2rs
