A YANG Data Model for the Routing Information Base (RIB)
draft-ietf-i2rs-rib-data-model-15

This is similar enough to Suresh's DISCUSS that I don't see the point in
making it a DISCUSS also (which it would be otherwise). I'll let him make sure
field size issues are taken care of.

>        case mac-route {
>          description
>            "MAC route case.";
>          leaf mac-address {
>            type uint32 ;
>            mandatory true;
>            description
>              "The MAC address used for matching.";
>          }
>        }

The intention here is to use IEEE EUI-48 and/or EUI-64 identifiers here, right?
These don't fit into a uint32.

This problem arises elsewhere in the module; e.g.:

>          leaf ieee-mac-address {
>            type uint32;
>            mandatory true;
>            description
>              "The nexthop points to an interface with
>               a specific mac-address.";
>          }

===========================================================================

§2.6:

>    Nexthops can be fully resolved or an unresolved.

Nit: remove "an"

---------------------------------------------------------------------------

§3:

>  import ietf-interfaces {
>    prefix if;
>        reference "RFC 7223";
>  }
>
>  import ietf-yang-types {
>    prefix yang;
>        reference "RFC 6991";
>  }

The indenting of the "reference" fields seems odd.


>  identity ipv6-decapsulation {
>    base "tunnel-decapsulation-action";
>    description
>      "IPv4 tunnel decapsulation.";
>  }

The description here appears to be incorrect (should say "IPv6")


>  identity decrease-and-copy-to-next {
>    base "ttl-action";
>    description
>      "Decrease TTL by one and copy the TTL
>       to the next header.For example: when
>       MPLS label swapping, decrease the TTL
>       of the inner label and copy it to the
>       outer label.";
>  }

Nit: insert a space before "For".


>  identity resolved {
>    base "nexthop-state";
>    description
>      "Reolved nexthop state.";
>  }

Nit: "Resolved" rather than "Reolved."


>  typedef nexthop-lb-weight-definition {
>    type uint8 {
>      range "1..99";
>    }
>    description
>      "Nexthop-lb-weight is used for load-balancing.
>       Each list member MUST be assigned a weight
>       between 1 and 99. The weight determines the
>       proportion of traffic to be sent over a nexthop
>       used for forwarding as a ratio of the weight of
>       this nexthop divided by the weights of all the
>       nexthops of this route that are used for forwarding.
>       To perform equal load-balancing, one MAY specify
>       a weight of 0 for all the member nexthops.  The
>       value 0 is reserved for equal load-balancing
>       and if applied, MUST be applied to all member nexthops.";
>  }

To match the text (which allows 0 as a special case), the range needs to be
updated to be "0..99" rather than "1..99"


>    leaf hop-limit {
>      type uint8;
>      description
>        "The hop limit the header.";
>    }

Nit: "The hop limit of the header."


>    choice nvgre-type {
>      description
>        "NvGRE can use eigher IPv4
>         or IPv6 header for encapsulation.";

Nit: "either"


>    leaf flow-id {
>      type uint16;
>      description
>        "The flow identifier of the NvGRE header.";
>    }

Why is this a uint16 rather than a uint8?

Sec 1.2:

"YANG tree diagrams provide a concise representation of a YANG module,
   and SHOULD be included to help readers understand YANG module
   structure."

This document does not seem like an appropriate place to have normative guidance about this. And if this sentence is removed, I don't see the point of including Section 1.2 otherwise. This would also imply deleting the reference to I-D.ietf-netmod-yang-tree-diagrams.

Sec 2.1: Again here I'm confused about the use of normative language. Why do you need to specify normative requirements for what this very document is specifying? Or are these supposed to be requirements on implementations?

Sec 2.5: s/causes/caused/

(1) "This document defines a YANG data model for Routing Information Base (RIB) that aligns with the I2RS RIB information model."  That and the multiple references to the information model (in the text and on the e-mail archive) make it a required document to understand for the implementation of the data model.  draft-ietf-i2rs-rib-info-model should then be a Normative reference.

I am not making this point a DISCUSS because I think it is easy to solve: just move the reference.

(2) In Figure 1: s/route-reason-definition/route-change-reason-definition

(3) For completeness: in S2.3, the Reason attribute is missing (from S4 in draft-ietf-i2rs-rib-info-model).

I agree with Alissa's comments.

Requirements Language: There are at least a few instances of lower case versions of 2119 keywords. Please consider using the boilerplate from RFC 8174.

Abstract: Missing article before "Routing Information Base"

§1, first paragraph: Missing article before "Routing Information Base"

I agree with Alissa's comment.

A couple nits:

In Section 2.6:
    Nexthops can be fully resolved or an unresolved.
I don't think the "an" is needed.

In the module itself:
   typedef nexthop-lb-weight-definition {
     type uint8 {
       range "1..99";
     }
     description
       "Nexthop-lb-weight is used for load-balancing.
        Each list member MUST be assigned a weight
        between 1 and 99. The weight determines the
        proportion of traffic to be sent over a nexthop
        used for forwarding as a ratio of the weight of
        this nexthop divided by the weights of all the

"sum of the weights", presumably.

        nexthops of this route that are used for forwarding.
        To perform equal load-balancing, one MAY specify
        a weight of 0 for all the member nexthops.  The
        value 0 is reserved for equal load-balancing
        and if applied, MUST be applied to all member nexthops.";
   }

Also, there's a mismatch between the MUST (1-99) and MAY (0).

I2RS use cases document seems to be abandoned, and generally use case documents best fit the purpose of tracking the work on specification documents. Is the reference really needed? 

Please address RTG-DIR comments on RIB/Rib/rib consistency, encap/encapsulation, decap/decapsulation consisteny. 

s/nexthop-replicates/nexthop-replicate, or change the description of base nexthop. 

s/blow/below

s/VxLAN/VXLAN throughout the document. 

nexthop-lb-weight-definition: divided by the sum of weights. Or, to simplify the text, representing a proportion. Value of 0 is not in the range.

I agree with Alissa's comments on use of normative language.

Thanks for addressing my DISCUSS.