YANG Data Model for the IS-IS Protocol
draft-ietf-isis-yang-isis-cfg-42

Thanks for addressing my DISCUSS point.

Thank you for the work put into this document.  Disclaimer: I am neither an IS-IS export nor a YANG doctor ;-)

I have 2 COMMENTs below.

Regards,

-éric

== COMMENTS ==

-- Section 2.3 --
C.1) Is there a reason to have one example with a generic value of 250 that will never be used as you are either level-1 or level-2 ? (I am not an IS-IS expert of course)

-- Section 6 / YANG module --

C.2) About lsp-entry/remaining-lifetime, is there also a state about the received hold time ? It could be interesting to know whether the remaining lifetime is 3% of the original lifetime or 30% ;-) But again, I am not an IS-IS expert

Thanks for the work that went into this model. I have only a handful
of minor issues I found when reading through the module.

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

>    grouping spf-parameters {
>      container spf-control {
>          leaf paths {
>            if-feature max-ecmp;
>            type uint16 {
>              range "1..32";
>            }

Why is this a uint16 rather than a uint8?

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

>      leaf-list tag {
>        type uint32;
>        description
>          "List of 32-bit tags associated with the IPv4 prefix.";
>      }
>      leaf-list tag64 {
>        type uint64;
>        description
>          "List of 32-bit tags associated with the IPv4 prefix.";
>      }

I think this second description is meant to say "64-bit" rather than "32-bit".

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

>      leaf reason {
>        type string {
>          length "1..255";
>        }
>        description
>          "The system may provide a reason to reject the
>           adjacency. If the reason is not available,
>           an empty string will be returned.
>           The expected format is a single line text.";
>      }

This description is inconsistent with the definition: it calls for an empty
string, while the definition requires that at lest one character be present. If
you want to keep the description as-is, you need to adjust the length to be
"0..255". Alternately, you might indicate that the field is simply to be
omitted rather than empty, which appears to be the intention for other
"reason" fields in this model.

Thanks for the work on this YANG module.  My comments are all editorial, and there’s no need for a response... please just consider these, as they will help make the document clearer.

Throughout the document, please hyphenate the following as shown here:
per-topology basis
per-interface basis
per-level configuration
per-level value
level-specific parameter
interface-specific parameter
vendor-specific

When you use “like” to mean “such as”, it’s ambiguous: “like” can also introduce a comparison, so does “fruit, like an apple” mean any fruit (and an apple is an example), or are you talking specifically about fruit that is in some way similar to an apple?  It’s better to say “such as”, to avoid the ambiguity.  All instances of “like” in the document should be changed, *except* for “encoded as a MAC address like” on page 35 and “would like to thank” in the Contributors section.

Other, specific comments:

— Section 2.1 —

   The model implements features, thus some of the configuration
   statement becomes optional.

This is an odd sentence that I’m having a hard time understanding.  Do you mean this?:

NEW
   This model includes optional features, for which the corresponding
   configuration statements are optional.
END

   By advertising the feature "admin-control", a device communicates to
   the client that it supports the ability to shutdown a particular IS-
   IS instance.

The verb “shut down” is two words.

— Section 2.2 —

   Some specific parameters can be defined on a per topology basis both
   at global level and at interface level

Apart from adding a hyphen in “per-topology basis”, you need “the” before both “global” and “interface”.  There are many places throughout the document where articles (usually “the”, but sometimes “a” or “an”) are missing.  Someone familiar with the correct use of articles should take a pass through the document.

— Section 2.3 —
In the last two paragraphs of the section, one uses “should” advertise and the other uses “SHOULD” advertise.  They should either both be BCP 14 key words, or both not.

— Section 2.6 —

   The goal of this empty
   container is to allow easy augmentation with additional parameters
   like timers for example.

As I noted above, you should use “such as”, rather than “like”, and you don’t *also* need “for example”, because “such as” already has that covered.  So, “...additional parameters, such as timers.”

— Section 2.8 —

   The "candidate-enable" allows to mark an interface to be used as a
   backup.

You need a subject for the verb after “allows” or “requires”.  It has to allow <something> to mark an interface, and you can’t omit the <something>.  So what is it?

If there really is no sensible entity to put there, you can use passive voice, ‘The "candidate-enable" option allows an interface to be marked for use as a backup.’

— Section 2.9 —
Throughout the section, “information” is a collective noun; we don’t say “informations”.

— Section 4 —
The first four notification descriptions start with “raised when”, and the rest start with “This notification is sent when”.  Please make them consistent, one way or the other.

— Section 5 —

   Some IS-IS specific routes attributes are added to route objects

I think this is supposed to say, “Some IS-IS-specific route attributes are added...” (add another hyphen and take the “s” off “routes”).

Thanks for the updates in the -41!
I think that "i-e" in the following got missed, but that can probably be done with an
RFC Editor note, so I'm clearing my Discuss:

     grouping neighbor {
       description  "IS-IS standard neighbor grouping.";
       leaf neighbor-id {
         type extended-system-id;
         description "IS-IS neighbor system-id";
       }
      container instances {
         description "List of all adjacencies between the local
                      system and the neighbor system-id.";
         list instance {
           key id;

           leaf id {
             type uint32;
             description "Unique identifier of an instance of a
                          particular neighbor.";
           }
           leaf i-e {
             type boolean;
             description
               "Internal or External (I/E) Metric bit value";
           }

Also, I think that a spurious 'd' got added into "Routing Information Based".