Re: [i2rs] Adam Roach's No Objection on draft-ietf-i2rs-rib-data-model-10: (with COMMENT)

["Susan Hares" <[e-mail]>]

Adam:

Good catch.  If you see any others, please let me know.  This initially was a "hack" to provide references.  We'll go with the yang:mac-address for the MAC Address.  

 I will also ask the RTG-DIR whether we should make this an index into the actual MACS.  I do not think that is appropriate, but it is worth a question to the RTG-DIR and OPS-DIR. 

We will move this references to the standard references that Martin mentioned in his email. 

   inet:ipv6-flow-label
   yang:mac-address

I really appreciate all the new reviews on this draft.  Due to the history on this draft, please mention any questionable references you might have notice. I will track them down. 

Susan Hares 

-----Original Message-----
From: Adam Roach [mailto:[e-mail]] 
Sent: Thursday, April 5, 2018 12:39 AM
To: The IESG
Cc: [e-mail]; Susan Hares; [e-mail]; [e-mail]; [e-mail]
Subject: Adam Roach's No Objection on draft-ietf-i2rs-rib-data-model-10: (with COMMENT)

Adam Roach has entered the following ballot position for
draft-ietf-i2rs-rib-data-model-10: No Objection

When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.)


Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
for more information about IESG DISCUSS and COMMENT positions.


The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-i2rs-rib-data-model/



----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------


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?