Re: [netmod] review of draft-litkowski-netmod-isis-cfg-00

[Ladislav Lhotka <[e-mail]>]

"t.petch" <[e-mail]> writes:

> I would consult the isis list about the use of TLVnnn as names.
>
> For me, that is very much a part of is-is, that is, when I see TLVnnn I
> know I am dealing with someone who is familiar with is-is.

Yes, it's like those monks telling jokes just by calling out numbers. :-)

Anyway, the output of "show isis" commands in the major CLIs doesn't contain these numbers.

Lada  

>
> Tom Petch
>
>
> ----- Original Message -----
> From: <[e-mail]>
> To: "Ladislav Lhotka" <[e-mail]>; <[e-mail]>
> Sent: Friday, June 20, 2014 9:05 AM
> Subject: Re: [netmod] review of draft-litkowski-netmod-isis-cfg-00
>
>
>> Hi Lada,
>>
>> Thanks for your review.
>>
>> Please find inline comments.
>>
>> Best regards,
>>
>> Stephane
>>
>>
>> -----Original Message-----
>> From: Ladislav Lhotka [mailto:[e-mail]]
>> Sent: Thursday, June 19, 2014 16:34
>> To: [e-mail]; LITKOWSKI Stephane SCE/IBNF
>> Subject: review of draft-litkowski-netmod-isis-cfg-00
>>
>> Hi,
>>
>> I reviewed the isis module and I-D. I think it is a good start,
> although some adjustments will be needed to make it work with the core
> routing data model, see below. I don't address any specific issues of
> IS-IS configuration.
>>
>> *** General Comments
>>     - As Andy already pointed out, descriptions are often missing or
>>       too brief, and the prose in the I-D is also extremely scarce.
>>     - An example instance document would be useful, e.g. a reply to
>>       <get> as in Appendix A of RFC 7277. (The "sample-skeleton"
>>       plugin of pyang can help with generating it.)
>>
>> [SLI] Sure I will take this into account.
>>
>> *** YANG module design
>>     - Entries of the "rt:routing-protocol" list (under both "routing"
>>       and "routing-state") are designed to support multiple instances
>>       of the same protocol. In configuration it means not only that
>>       the nodes "isis-cfg", "instances" and "instance" are
>>       superfluous, but this design also prevents connecting different
>>       ISIS instances to different RIBs.
>>
>> [SLI] Ok I didn't catch this point, for me there was only one protocol
> instance per type indexed by its name "isis" or "ospf" in the same
> routing instance.
>> If it's not the case, I would remove the instances for ISIS.
>> The OSPF model (draft-yeung-netmod-ospf) sounds also to implement a
> list of instances for OSPF protocol :
>> " augment "/rt:routing/rt:routing-instance/rt:routing-protocols/"
>>          + "rt:routing-protocol" {
>>      list ospf-router {
>>
>>        description
>>          "This is a top-level container for the OSPF router.";
>>
>>        when "./type=ospf";
>>
>>        key "version name";
>>        leaf version {
>>          description
>>            "OSPF version.";
>>          type uint8 {
>>            range "2..3";
>>          }
>>        }
>> "
>>
>>
>>     - Similarly, state data for each ISIS instance should go straight
>>       to the corresponding entry of "routing-protocol" under
>>       "routing-state".
>>
>> [SLI] Ack
>>
>>     - Nodes in state data should use descriptive names rather than
>>       "TLVnnn". TLV numbers are important for the protocol but IMO
>>       there is no need to expose operators to them. For instance,
>>       "dynamic-hostname" is better than "TLV137".
>>
>> [SLI] Ok, sometimes TLV numbers are more understable than descriptions
> :)
>>
>>     - Grouping "interface-cfg" is used only once, so it is not
>>       needed. Other groupings are used at least twice.
>>
>>   [SLI] Ack
>>
>>     - A type definition for ISO addresses might be useful.
>>
>> [SLI] Ack
>>
>> *** Specific YANG issues
>>     - The module is invalid, I am attaching a diff file with
>>       necessary changes. Module validity should always be verified
>>       before posting an I-D, e.g. with pyang.
>>
>> [SLI] Ok , I checked it but without importing modules, so only syntax
> checking was done.
>>
>>     - The argument of "namespace" should be an URI, e.g. urn:TBD.
>>
>> [SLI] Ok
>>
>>     - Arguments of some "augment" statements are broken, see the
>>       attached diff. In particular, they must not contain trailing
>>       slashes, see ABNF production "absolute-schema-nodeid".
>>     - The module should follow the guidelines of RFC 6087, they can
>>       be checked using
>>
>>       pyang --ietf isis.yang
>>
>> *** Formal issues
>>     - The module text should be wrapped in
>>
>>       <CODE BEGINS> file "[e-mail]"
>>       ...
>>       <CODE ENDS>
>>
>>       so that it can be easily extracted with rfcstrip.
>>     - Assuming the module is bound for standard track, its name
>>       should be "ietf-isis".
>>     - Some descriptions exceed the line length limit of 72 characters,
>>       line breaks should be inserted.
>>
>> [SLI] Will be fixed.
>>
>>
>> Lada
>>
>>
> ________________________________________________________________________
> _________________________________________________
>>
>> Ce message et ses pieces jointes peuvent contenir des informations
> confidentielles ou privilegiees et ne doivent donc
>> pas etre diffuses, exploites ou copies sans autorisation. Si vous avez
> recu ce message par erreur, veuillez le signaler
>> a l'expediteur et le detruire ainsi que les pieces jointes. Les
> messages electroniques etant susceptibles d'alteration,
>> Orange decline toute responsabilite si ce message a ete altere,
> deforme ou falsifie. Merci.
>>
>> This message and its attachments may contain confidential or
> privileged information that may be protected by law;
>> they should not be distributed, used or copied without authorisation.
>> If you have received this email in error, please notify the sender and
> delete this message and its attachments.
>> As emails may be altered, Orange is not liable for messages that have
> been modified, changed or falsified.
>> Thank you.
>>
>> _______________________________________________
>> netmod mailing list
>> [e-mail]
>> https://www.ietf.org/mailman/listinfo/netmod
>

-- 
Ladislav Lhotka, CZ.NIC Labs
PGP Key ID: E74E8C0C