[Lsr] Yangdoctors last call review of draft-ietf-lsr-yang-isis-reverse-metric-01
Reviewer: Ladislav Lhotka Review result: Ready with Issues General comments - YANG module "ietf-isis-reverse-metric" is in a good shape except for one issue described below. - I appreciate the three examples of instance data in appendices (two in XML representation, one in JSON). Specific comments - In three places, the module uses "when" expressions with plain string equality test applied to identityref values, such as: when "../rt:type = 'isis:isis'" This is known to be fragile, which can demonstrated on the JSON example in appendix A.3: It has "type": "ietf-isis:isis" which makes the "when" expression false because of the different prefix, and the corresponding augment doesn't happen. The above "when" expression should use the XPath function "derived-from-or-self" that is defined in RFC 7950: when "derived-from-or-self(../rt:type, 'isis:isis')" - The "tlv16-reverse-metric" grouping is used only once in the module. Unless it is expected to be used elsewhere, I would recommend to remove this grouping and use its contents directly. - The phrase "YANG XML data" is somewhat misleading, although it is probably often used in informal discussions. I would suggest "XML instance data" instead. Nits - Abstract & Introduction: s/routeing/routing/ - Appending A.3: s/YANG XML data/JSON instance data/ ___ Lsr mailing list Lsr@ietf.org https://www.ietf.org/mailman/listinfo/lsr
Re: [Lsr] Benjamin Kaduk's Discuss on draft-ietf-ospf-yang-26: (with DISCUSS and COMMENT)
iffering LSDBs will not yield the same checksum. However,
>> if the checksums are different, you can assure the databases are different.
>
> Thanks for the extra text -- it helps confirm my guess as to what's going
> on.
>
>>leaf transmit-delay {
>> type uint16;
>> units seconds;
>> description
>>"Estimated time needed to transmit Link State Update
>> (LSU) packets on the interface (seconds). LSAs have
>> their age incremented by this amount on advertised
>> on the interface. A sample value would be 1 second.";
>>
>> nit: "on advertised on" does not seem grammatical.
>>
>> Fixed - "when advertised on".
>>
>>leaf lls {
>>[...]
>>container ttl-security {
>>
>> Should these have a default value?
>>
>> I think 1 would be appropriate since only virtual-links and sham-links
>> require more.
>>
>>case ospfv3-auth-ipsec {
>> when "derived-from-or-self(../../../../../../rt:type, "
>>+ "'ospfv3')" {
>>description "Applied to OSPFv3 only.";
>> }
>> if-feature ospfv3-authentication-ipsec;
>> leaf sa {
>> type string;
>>
>> I don't see RFC 4552 talking about names for SAs; where would this be
>> discussed (and, are they guaranteed to be human-readable)?
>>
>> If it is not human readable, how would it be configured?
>
> I wasn't sure if this was config or operational state, but you make a good
> point.
>
>>leaf poll-interval {
>> type uint16;
>> units seconds;
>> description
>>"Neighbor poll interval (seconds) for sending OSPF
>> hello packets to discover the neighbor on NBMA
>> networks. This interval dictates the granularity for
>> discovery of new neighbors. A sample would be 2 minutes
>> for a legacy Packet Data Network (PDN) X.25 network.";
>>
>> Maybe "2 minutes (120 seconds)" since the units are seconds?
>>
>> Fixed.
>>
>>container preference {
>> description
>>"Route preference configuration In many
>> implementations, preference is referred to as
>> administrative distance.";
>>
>> nit: missing sentence break?
>>
>> Fixed.
>>
>> For the spf- and lsa-logs, do we require that the 'id' is assigned in
>> any particular order, or do we just rely on the included timestamp(s)
>> for any time-ordering required by the consumer?
>>
>> I added ordering to the description of the spf-log and lsa-log. However, the
>> id is a purely internal value to provide a uniqueness key for the YANG list.
>
> Understood, and thanks.
>
>> grouping notification-neighbor {
>>[...]
>>leaf neighbor-ip-addr {
>> type yang:dotted-quad;
>> description "Neighbor address.";
>>
>> neighbors can only be identified by IPv4 addresses?
>>
>> Changed to IP address type.
>>
>>leaf packet-source {
>> type yang:dotted-quad;
>>
>> packet sources, too? (multiple times)
>>
>> Changed to IP address type.
>>
>>description
>> "This notification is sent when aa neighbor
>> state change is detected.";
>>
>> nit: s/aa/a/
>>
>> Fixed.
>>
>> Section 4
>>
>>Additionally, local specificationn of OSPF authentication keys and
>>the associated authentication algorithm is supported for legacy
>>implementations that do not support key-chains [RFC8177] for legacy
>>implementations that do not support key-chains. It is RECOMMENDED
>>that implementations migrate to key-chains due the seamless support
>>of key and algorithm rollover, as well as, the encryption of key
>>using the Advanced Encryption Standard (AES) Key Wrap Padding
>>Algorithm [RFC5649].
>>
>> (Roman caught the nits, so I won't duplicate that.)
>> I expected to see something about keeping the actual key material
>> secret, as well.
>>
>> Can you suggest some text?
>
> How's this?
>
> The actual authentication key data (whether locally specified or part of a
> key-chain) is sensitive and needs to be kept secret from unauthorized
> parties; compromise of the key data would allow an attacker to forge OSPF
> traffic that would be accepted as authentic, potentially compromising the
> entirey OSPF domain.
>
> Thanks,
>
> Ben
>
> ___
> Lsr mailing list
> Lsr@ietf.org
> https://www.ietf.org/mailman/listinfo/lsr
--
Ladislav Lhotka
Head, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67
___
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr
Re: [Lsr] WGLC: draft-ietf-isis-yang-isis-cfg (and remaining IPR replies)
I am also not aware of any IPR related to the document. Lada On Tue, 2019-07-30 at 08:06 +, stephane.litkow...@orange.com wrote: > I'm not aware of any IPR related to this document. > > -Original Message- > From: Christian Hopps [mailto:cho...@chopps.org] > Sent: Monday, July 29, 2019 19:22 > To: lsr@ietf.org > Cc: Christian Hopps; lsr-cha...@ietf.org; lsr-...@ietf.org; > draft-ietf-isis-yang-isis-...@ietf.org > Subject: WGLC: draft-ietf-isis-yang-isis-cfg (and remaining IPR replies) > > Hi Folks, > > > As mentioned in the meeting in Montreal, due to mixup (IPR call but no WGLC) > the IS-IS yang module was submitted to the IESG prior to a WGLC being done. We > are belated doing a WGLC on this document now. Please voice any objections to > the continued progression of this document. Prior to objecting though please > do review any comments received during IESG reviews. > > https://datatracker.ietf.org/doc/draft-ietf-isis-yang-isis-cfg/ > > The WGLC will complete in 2 weeks on Aug 12th. > > Authors (Stephane, Derek and Jeffery), > > Please indicate if you are aware of any IPR related to this document in a > reply to the mailing list (already received from Acee and Ladislav). > > > Thanks, > Chris & Acee. > > __ > ___ > > 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. > -- Ladislav Lhotka Head, CZ.NIC Labs PGP Key ID: 0xB8F92B08A9F76C67 ___ Lsr mailing list Lsr@ietf.org https://www.ietf.org/mailman/listinfo/lsr
[Lsr] Yangdoctors last call review of draft-ietf-ospf-yang-23
Reviewer: Ladislav Lhotka Review result: Ready with Nits I reviewed already revision 09 of this module [1]. All substantial objections and suggestions expressed in that review are addressed in revision 23 and I am satisfied with the result. I especially appreciate that descriptions were considerably expanded and references added in many places. I tested validity of the ietf-ospf module with pyang and Yangson tools, and found no issues. The comments below are non-substantial and do not affect practical use of the module. In summary, I think this YANG module and document is a remarkable piece of work demonstrating that it is possible to build quite complex vendor-neutral data model that can be used equally well with several router plaforms. Comments: - names of locally-defined identities as parameters of XPath functions derived-from and derived-from-or-self sometimes have the 'ospf:' prefix, sometimes don't. I suggest to be consistent, and the option without a prefix looks better to me. - RFC 8407 suggests this format of references to RFC: RFC : Title of the Document This draft uses a hyphen instead of a colon. I suggest to follow the 8407 convention so as to make parsing easier. - the title of Sec. 2.8 should be "OSPF Notifications" (plural and capitalization) - enumerations "nssa-translator-state-type" and "restart-status-type" define the value parameter for two of their enums but not for the third. This should be avoided. [1] https://datatracker.ietf.org/doc/review-ietf-ospf-yang-09-yangdoctors-lc-lhotka-2017-12-06/ ___ Lsr mailing list Lsr@ietf.org https://www.ietf.org/mailman/listinfo/lsr
Re: [Lsr] Shepherd review comments on draft-ietf-isis-yang-isis-cfg-29
Hi Stéphane, the problem is in the source of the "ietf-isis" module inside : the chevrons enclosing email addresses should be encoded as XML entities and but what I see there is lt; and gt; Lada On Tue, 2019-01-22 at 16:18 +, stephane.litkow...@orange.com wrote: > Hi Tom, > > This is plaintext. You can see it in the XML source of the draft, there is no > link in the YANG module. > Is your concern coming from the "[" "]" characters ? Just a copy/paste from > the text version of the boilerplate, but there is no link behind. > > > -Original Message- > From: Lsr [mailto:lsr-boun...@ietf.org] On Behalf Of tom petch > Sent: Tuesday, January 22, 2019 13:48 > To: Acee Lindem (acee); Yingzhen Qu; lsr@ietf.org; > draft-ietf-isis-yang-isis-...@ietf.org > Subject: Re: [Lsr] Shepherd review comments on draft-ietf-isis-yang-isis-cfg- > 29 > > -32 has the RFC2119 boilerplate in the YANG module - thank you - but... > YANG modules must be plain text and > > "are to be interpreted as described in BCP 14 [RFC2119] > [RFC8174] when, and only when, they appear in all capitals, as > shown here." > > looks rather like XML/HTML anchors and not plain text. > > Time idnits learnt what a YANG module looks like! > > Tom Petch > > - Original Message - > From: "Acee Lindem (acee)" > To: "Yingzhen Qu" ; ; > > Sent: Monday, January 21, 2019 9:07 PM > Subject: Re: [Lsr] Shepherd review comments on > draft-ietf-isis-yang-isis-cfg-29 > > > > Hi Yingzhen, > > All the idnits warnings are fixed in the -32 version of the drafts. > > Thanks, > > Acee > > > > From: Yingzhen Qu > > Date: Friday, January 18, 2019 at 5:35 PM > > To: "lsr@ietf.org" , > "draft-ietf-isis-yang-isis-...@ietf.org" > > > Subject: Shepherd review comments on draft-ietf-isis-yang-isis-cfg-29 > > Resent-From: > > Resent-To: Stephane Litkowski , Derek > Yeung , Acee Lindem , Jeffrey Zhang > > ___ > Lsr mailing list > Lsr@ietf.org > https://www.ietf.org/mailman/listinfo/lsr > > __ > ___ > > 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. > -- Ladislav Lhotka Head, CZ.NIC Labs PGP Key ID: 0xB8F92B08A9F76C67 ___ Lsr mailing list Lsr@ietf.org https://www.ietf.org/mailman/listinfo/lsr
Re: [Lsr] [yang-doctors] Yangdoctors last call review of draft-ietf-isis-yang-is-is-cfg-29
r-isis-adjacency'
>> > > - Section 4: Notification name mismatch in this section from actual
>> > naming
>> > > within the module (e.g. 'adjacency-change' should rather be
>> > > 'adjacency-state-change')
>> > > - Section 7: Security Considerations will need updating to be
>> > patterned after
>> > > the latest version of the template at
>> > > https://trac.ietf.org/trac/ops/wiki/yang-security-guidelines per
>> > >
>> >
>> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.7
>> > > - Section 12: All modules imported within this module MUST be
>> > referenced
>> > > within this section per
>> > >
>> >
>> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.9.
>> > > There are quite a few missing from this section right now
>> > > - Appendix A: Some of the XML elements are off in alignment
>> > > - Appendix A: Examples must be validated. The example given has the
>> > following
>> > > issues:
>> > > - /routing[name='SLI'] and /routing/description are invalid data
>> > nodes and
>> > > do not exist. I'm not sure why they are in the XML example here
>> > > - The example is meant to reference configuration however
>> > > /routing/interfaces is a r/o container
>> > > - The control-plane-protocol 'type' needs to be qualified - e.g.
>> > > > > xmlns:isis="urn:ietf:params:xml:ns:yang:ietf-isis">isis:isis
>> > > - The area-address does not validate against the pattern regex and
>> > must end
>> > > with a '.' e.g.
>> > > 49.0001..
>> > > - metric-type/value is set to 'wide' which is invalid. This
>> should
>> > rather
>> > > be 'wide-only'
>> > > - isis/afs/af/af is set to 'ipv4-unicast' which is invalid. This
>> > should
>> > > rather be 'ipv4' per iana-routing-types
>> > > - /interfaces/interface/type must be populated and is invalid.
>> This
>> > should
>> > > rather be qualified as such:
>> > > > >
>> xmlns:ianaift="urn:ietf:params:xml:ns:yang:iana-if-type">ianaift:softwar
>> > eLoopback
>> > > > >
>> xmlns:ianaift="urn:ietf:params:xml:ns:yang:iana-if-type">ianaift:etherne
>> > tCsmacd
>> > > - /interfaces/interface/link-up-down-trap-enable must have a value
>> > > associated as such:
>> > > enabled
>> > > - NP container 'priority' has a must statement checking if an
>> > interface-type
>> > > is set to 'broadcast' however if you take the XML example from
>> > this
>> > > section, it will fail to validate even if is not
>> > defined
>> > > underneath an interface-type of 'point-to-point'. It seems to
>> me
>> > that
>> > > this logic may need to be readjusted or not exist at all
>> (priority
>> > can
>> > > still be set on implementations on loopback interfaces - which
>> > would
>> > > default to 'broadcast' in the example here). Could you not
>> solve
>> > this
>> > > with use of 'when' vs. 'must' as such:
>> > >
>> > > when '../interface-type = "broadcast"' {
>> > > description "Priority can only be set for broadcast
>> > interfaces.";
>> > > }
>> > >
>> > >
>> >
>> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-4.18
>> > ..2.
>> > >
>> > > - /interfaces/interface/ipv4/mtu must contain a valid value (and
>> > likely not
>> > > need to be defined for Loopback0)
>> > > - 'isis/mpls-te/ipv4-router-id' is invalid and should rather be
>> > > 'isis/mpls/te-rid/ipv4-router-id'
>> > > - 'isis/afs/af/enabled' is invalid and should rather be
>> > 'isis/afs/af/enable'
>> > > - Examples should use IPv6 addresses where appropriate per
>> > >
>> >
>> https://tools.ietf.org/html/draft-ietf-netmod-rfc6087bis-20#section-3.12
>> >
>> >
>>
>> > _
>> >
>> > 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.
>> >
>> > ___
>> > Lsr mailing list
>> > Lsr@ietf.org
>> > https://www.ietf.org/mailman/listinfo/lsr
>>
>>
>> _
>>
>> 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.
>>
>> ___
>> yang-doctors mailing list
>> yang-doct...@ietf.org
>> https://www.ietf.org/mailman/listinfo/yang-doctors
>
> --
> Juergen Schoenwaelder Jacobs University Bremen gGmbH
> Phone: +49 421 200 3587 Campus Ring 1 | 28759 Bremen | Germany
> Fax: +49 421 200 3103 <https://www.jacobs-university.de/>
--
Ladislav Lhotka
Head, CZ.NIC Labs
PGP Key ID: 0xB8F92B08A9F76C67
___
Lsr mailing list
Lsr@ietf.org
https://www.ietf.org/mailman/listinfo/lsr
