Subject: Re: [Lsr] AD Review of draft-ietf-isis-yang-isis-cfg-35

From: Alvaro Retana <[email-masked]>
Date: Tue, 30 Jul 2019 15:29:16 -0400

On June 27, 2019 at 8:36:46 AM, [email-masked] (
[email-masked]) wrote:

Stephane:

Hi!

Sorry it took me while to get to your reply.

Thanks for your comments. We are working on updating the document
accordingly.

Please find some replies inline that may require more discussion.

I have stripped the comments that will be fixed in the next revision.

Just leaving some text with answers.

Thanks!

Alvaro.

...

474         Some parameters like "overload bit" and "route preference" are
not

475         modeled to support a per level configuration.  If an
implementation

476         supports per level configuration for such parameter, this

477         implementation SHOULD augment the current model by adding both

478         level-1 and level-2 containers and SHOULD reuse existing

479         configuration groupings.

[major] "...SHOULD augment the current model by adding both level-1 and
level-2 containers"  What other way would that be done?  I think that
Normative language is not needed in this case.

[SLI] Using YANG there are multiple ways to do things. People may create
new containers, use different namings… and we want to keep the modeling
consistency even in the future augmentations.

Ok…why not use MUST then?  Are there cases where it would be ok to not be
consistent?  IOW, the current wording doesn’t guarantee consistency…

...

978            sequence-number-skipped: This notification is sent when the
system

979            receives a PDU with its own system ID and different
contents.  The

980            system has to reissue the LSP with a higher sequence number.

[nit] That's the last thing I would have guessed that this action would
have been called...  Maybe it's just me...

[SLI] This is inherited from RFC4444

982            authentication-type-failure: This notification is sent when
the

983            system receives a PDU with the wrong authentication type
field.

985            authentication-failure: This notification is sent when the
system

986            receives a PDU with the wrong authentication information.

[minor] Why do we need both of these?  Given that they both provide the
same information (including the raw PDU), and that
authentication-type-failure is a specific case of receiving "a PDU with the
wrong authentication information"

[SLI] This is inherited from RFC4444

You used this reply in several places.

Inheriting things from rfc4444 doesn’t mean it is ok…or the best solution…

In either case, most are minor comments, so no big deal.  I just want to
make sure that some of the points were considered.

...

1239         feature nsr {

1240           description

1241             "Non-Stop-Routing (NSR) support.";

1242         }

[minor] Reference?

[SLI] NSR is a local well known and deployed mechanism. We may enhance the
description if required, however we cannot really provide a reference.

Yes, please do.  See the description in draft-ietf-ospf-yang.

...

3995         grouping tlv242-router-capabilities {

3996           container router-capabilities {

3997             list router-capability {

3998                 leaf flags {

3999                   type bits {

4000                     bit flooding {

4001                       position 0;

4002                       description

4003                         "If the S bit is set, the IS-IS Router
CAPABILITY

4004                          TLV MUST be flooded across the entire routing

4005                          domain. If the S bit is clear, the TLV MUST
NOT

4006                          be leaked between levels. This bit MUST NOT

4007                           be altered during the TLV leaking.";

4008                     }

[major] This is a description of the behavior (copied from rfc7981!), not a
description of the field.

[SLI] Yes, but this provides an accurate description on the conditions of
the flag setting.

But this document is not Normative in the behavior above…. If anything,
Normative language should not be used unless making it clear that it is a
direct quote.

...

4540         notification lsp-too-large {

4541           uses notification-instance-hdr;

4542           uses notification-interface-hdr;

4544           leaf pdu-size {

4545             type uint32;

4546             description "Size of the LSP PDU";

4547           }

4548           leaf lsp-id {

4549             type lsp-id;

4550             description "LSP ID";

4552           }

4553           description

4554             "This notification is sent when we attempt to propagate

4555              an LSP that is larger than the dataLinkBlockSize for the

4556              circuit.  The notification generation must be throttled

4557              with at least 5 seconds between successive

4558              notifications.";

4559         }

...

[major] "must be throttled"  Why is this text not Normative?  It seems to
me that throttling is a good practice...in fact, it may be a good idea to
specify it for all notifications.  There are 12 total instances of the same
text.

[SLI] The text is mirrored from RFC4444.

I think this was the only “major” point what you replied with rfc4444….
Just leaving it here…. No further comments. ;-)