Subject: [mpls] MPLS-RT review of draft-saad-mpls-static-yang-02

From: "Carlos Pignataro (cpignata)" <[email-masked]>
Date: Sun, 3 Apr 2016 02:19:08 +0000

Authors,

I have been selected as an MPLS Review Team reviewer for draft-saad-mpls-static-yang-02.

This document specifies two separate YANG data models: “base MPLS” and “static LSPs”.

http://www.yangvalidator.com/ <http://www.yangvalidator.com/> gives not warnings or errors, and idnits is clean.

I have one generic and some specific comments about it:

First, it’s not clear why this document combines two seemingly separate sub-documents. Unless there are strong technical reasons, I’d recommend the authors consider splitting this into two independent documents: MPLS Base, and Statis LSP respectively.

Specifically, if:
   Note, it is expected that other MPLS YANG models to augment the
   "ietf-mpls" base model to define specific data to other MPLS
   technologies (e.g.  MPLS LDP or MPLS RSVP-TE).

Then it makes sense to have an independent -base.

The document is (i.e., the sub-documents are) useful, and coherent.

Now, some specific comments:

2.2.1.  MPLS Base Module

     typedef percent {
       type uint16 {
         range "0 .. 100";
       }
       description "Percentage";
     }

RFC 3031 does not speak to percentages of usage. I’m not sure how this is used.

     grouping interface-mpls {
       description "MPLS interface properties grouping";
       leaf enabled {
         type boolean;
         description
           "'true' if mpls encapsulation is enabled on the
            interface. 'false' if mpls encapsulation is enabled
            on the interface.";
       }
     }

Seems like a “not” is missing :-)

2.2.2.  MPLS Static LSP YANG Module

     revision "2016-03-21" {
       description
         "Initial revision";
       reference
         "RFC 3031: A YANG Data Model for MPLS Static";
     }

RFC 3031 does not contain the word “Static”. I know what static LSPs are, but what’s the Normative reference to it?

       leaf loadshare {
         type mpls:percent;
         description
           "The percentage of total load tto carry on this path";
       }

It is not clear to me how this % is used.

Also a typo, “tto".

         leaf index {
           type uint32;
           description
             "The index of the label, lower indices are closer to
              the top";
         }

Question: Does this model allow the creation of a Label Stack, in which there is a label with index 1, a label with index 5, and nothing defined in between?

Also, what is an index of value 4294967295 (uint32)?

I hope these are useful!

— Carlos.