Hi Yingzhen,
Thank you for the update. Glad you found the PR with yang changes useful :-)
I checked the diff draft-ietf-isis-sr-yang-25 vs. draft-ietf-isis-sr-yang-28
and also sid encoding in the OSPF spec. All looks good, except one easy-to-fix
point: please move RFC9000 from normative to informative. FYI (as this may be
useful for future document), the guidance on that matter is as follows:
RFC8407bis:
Note: [RFC8341] (or a future RFC that replaces it) MUST be listed as
normative references.
By default, [RFC4252], [RFC6241], [RFC8040], [RFC8446], [RFC9000],
and RFC AAAA (or future RFCs that replace any of them) are listed
as informative references unless normatively cited in other
sections of the document that specifies the YANG module.
Please let me know when a new version is available so that I clear my DISCUSS.
Cheers,
Med
De : Yingzhen Qu <yingzhen.i...@gmail.com>
Envoyé : mercredi 16 avril 2025 23:41
À : BOUCADAIR Mohamed INNOV/NET <mohamed.boucad...@orange.com>
Cc : The IESG <i...@ietf.org>; draft-ietf-isis-sr-y...@ietf.org;
lsr-cha...@ietf.org; lsr@ietf.org; Christian Hopps <cho...@chopps.org>
Objet : Re: Mohamed Boucadair's Discuss on draft-ietf-isis-sr-yang-25: (with
DISCUSS and COMMENT)
Hi Med,
Sorry for the late reply. We focused on the changes of OSPF SR YANG first, now
we made corresponding changes in the ISIS model as well. Version -27 has been
published to address your comments.
Please see my detailed response below.
Thanks,
Yingzhen
On Tue, Mar 25, 2025 at 7:18 AM Mohamed Boucadair via Datatracker
<nore...@ietf.org<mailto:nore...@ietf.org>> wrote:
Mohamed Boucadair has entered the following ballot position for
draft-ietf-isis-sr-yang-25: Discuss
When responding, please keep the subject line intact and reply to all
email addresses included in the To and CC lines. (Feel free to cut this
introductory paragraph, however.)
Please refer to
https://www.ietf.org/about/groups/iesg/statements/handling-ballot-positions/
for more information about how to handle DISCUSS and COMMENT positions.
The document, along with other ballot positions, can be found here:
https://datatracker.ietf.org/doc/draft-ietf-isis-sr-yang/
----------------------------------------------------------------------
DISCUSS:
----------------------------------------------------------------------
Hi Stéphane, Yingzhen, Pushpasis, Helen, and Jeff,
Thank you for the effort put into this well-written document.
I appreciate your perseverance to push this specification for almost 10 years.
I have a DISCUSS point and a set of comments that I’d like we address. Main
comments are tagged with (*).
# DISCUSS
CURRENT:
leaf length {
type uint8;
description
"Length of the SID value. YANG model specification
is necessary since it dictates the semantics of the
SID.";
}
leaf sid {
type uint32;
description
"Segment Identifier (SID) - A 20 bit label or 32 bit SID.
If the length is set to 3, then the 20 rightmost bits
represent an MPLS label. If the length is set to 4, then
the value is a 32-bit index.";
}
This construct mirrors how the object is encoded on the wire, but when it comes
to management a type is better instead fo inferring its from the length. I
suggest to use an explicit type here instead of an implicit type.
[Yingzhen]: an explicit type is defined, the same as changes done is the OSPF
module.
----------------------------------------------------------------------
COMMENT:
----------------------------------------------------------------------
# Abstract
OLD:
This document defines a YANG data module that can be used to
configured and manage IS-IS Segment Routing for MPLS data plane.
NEW:
This document defines a YANG data module that can be used to
manage IS-IS Segment Routing for MPLS data plane.
Delete “configure” as this is covered by “manage”. Remember the FCAPS ;-)
[Yingzhen]: fixed.
# Section 2
Clarify this is a device model + Cite where the base model is defined when
first cited + avoid restricting the model to configuration. We claim anyway
that the module is NMDA compliant.
OLD:
This document defines a YANG model for IS-IS Segment Routing for the
MPLS data plane. It is an augmentation of the IS-IS base model.
The IS-IS SR MPLS YANG module requires support for the base segment
routing module [RFC9020], which defines the global segment routing
configuration independent of any specific routing protocol
configuration, and support of IS-IS base model [RFC9130] which
defines basic IS-IS configuration and state.
NEW:
This document defines a device YANG model for IS-IS Segment Routing for the
MPLS data plane. It is an augmentation of the IS-IS base model [RFC9130].
The IS-IS SR MPLS YANG module requires support for the base segment
routing module [RFC9020], which defines the global SR
management structure independent of any specific routing protocol,
and support of IS-IS base model [RFC9130] which
defines basic IS-IS configuration and state.
[Yingzhen]: fixed.
# Section 3 (*)
I would split the long tree into small snippets and help readers go through to
understand the rationale for each augment.
[Yingzhen]: The tree is moved to Appendix B as OSPF.
# Section 4 (*)
I have several comments about the module:
* Please run "pyang -f yang --yang-canonical” as the current order does not
follow a canonical order
[Yingzhen]: done.
* Not sure we can claim that the module covers extension that is “which is
common across all of the vendors”. I would simply delete that mention.
[Yingzhen]: done.
* The boilerplate is not needed
* It is unfortunate that the naming in the module reasons about TLV objects
* There are missing reference statements
* Need to clarify the mapping with the flags as defined in the base spec (e.g.,
af-flag/F-flag, sf-flag/S-Flag)
* There are several layers without clear explanation (e.g.,
sr-capability/sr-capability may question why we need two layers, do we need the
tree levels prefix-sid-sub-tlvs/prefix-sid-sub-tlvs/prefix-sid-sub-tlv?)
* No need to repeat the parent prefix
* Use of plural for lists and leaf-list, while singular is recommended in the
guidelines.
* There are no reference to justify the default values
[Yingzhen]: all the above all fixed.
* Weird to have “Configuration” while we claim the module is NMDA compliant.
A version that fixes many of these can be found at:
https://github.com/boucadair/IETF-Drafts-Reviews/blob/master/YANG/ietf-isis-sr-mpls.yang.
Please double check/complete. Thanks.
[Yingzhen]: thanks for providing this. really helpful!
A diff to track the changes can be seen here:
https://github.com/boucadair/IETF-Drafts-Reviews/commit/395b6ece9e7939c33af7d150a71bfd3948bb5ba1
# Section 5
## Please update this section to follow the template defined in 8407bis (*)
## I don’t find “node-msd-tlv” defined in the model
CURRENT:
Some of the readable data nodes in the modules may be considered
sensitive or vulnerable in some network environments. It is thus
important to control read access (e.g., via get, get-config, or
notification) to these data nodes.
/isis:router-capabilities/sr-capability
/isis:router-capabilities/sr-algorithms
/isis:router-capabilities/local-blocks
/isis:router-capabilities/srms-preference
/isis:router-capabilities/node-msd-tlv
[Yingzhen] : this was from module ietf-isis-msd.yang, which used to be in this
file as well. Removed now.
## nit
OLD:
Unauthorized access to any data node of these subtrees can disclose
the operational state information of IS-IS protocol on this device.
NEW:
Unauthorized access to any data node of these subtrees can disclose
the operational state information of IS-IS protocol on a device.
[Yingzhen] : fixed.
# Section 7
Consider updating as follows
OLD:
The IANA is requested to assign one new URI from the IETF XML
registry ([RFC3688]). Authors are suggesting the following URI:
URI: urn:ietf:params:xml:ns:yang:ietf-isis-sr-mpls
Registrant Contact: The IESG.
XML: N/A, the requested URI is an XML namespace
This document also requests one new YANG module name in the YANG
Module Names registry ([RFC6020]) with the following suggestion :
name: ietf-isis-sr-mpls
namespace: urn:ietf:params:xml:ns:yang:ietf-isis-sr-mpls
prefix: isis-sr-mpls
reference: RFC XXXX
NEW:
The IANA is requested to assign a new URI from the IETF XML
registry [RFC3688]:
URI: urn:ietf:params:xml:ns:yang:ietf-isis-sr-mpls
Registrant Contact: The IESG.
XML: N/A, the requested URI is an XML namespace
This document also requests a new YANG module name in the YANG
Module Names registry ([RFC6020]):
name: ietf-isis-sr-mpls
namespace: urn:ietf:params:xml:ns:yang:ietf-isis-sr-mpls
prefix: isis-sr-mpls
maintained by IANA? N
reference: RFC XXXX
[Yingzhen]: fixed.
# References (*)
RFC8446, RFC8340, RFC8040, RFC6242, and RFC6241 are listed as Normative, while
they shouldn’t. Please move those to be listed as Informative.
[Yingzhen]: fixed.
# Appendix A (*)
The JSON example is broken. Please update and validate using yanglint or
yangson, for example.
OLD:
{
"routing": {
"router-id": "1.1.1.1",
"control-plane-protocols": {
"control-plane-protocol": {
"type": "isis:isis",
"name": "isis",
"isis": {
"system-id": "1111.2222.3333",
"interfaces": {
"interface": {
"name": "",
"segment-routing": {
"adjacency-sid": {
"adj-sids": {
"value": 38888
}
}
}
}
},
"segment-routing": {
"enabled": true
},
"protocol-srgb": {
"srgb": {
"lower-bound": 4000,
"upper-bound": 5000
}
}
}
}
}
}
}
NEW:
{
"ietf-routing:routing": {
"router-id": "1.1.1.1",
"control-plane-protocols": {
"control-plane-protocol": {
"type": "ietf-isis:isis",
"name": "isis",
"ietf-isis": {
"system-id": "1111.2222.3333",
"interfaces": {
"interface": {
"name": "",
"ietf-isis-sr-mpls:segment-routing": {
"adjacency-sid": {
"adj-sids": {
"value": 38888
}
}
}
}
},
"ietf-isis-sr-mpls:segment-routing": {
"enabled": true
},
"ietf-isis-sr-mpls:protocol-srgb": {
"srgb": {
"lower-bound": 4000,
"upper-bound": 5000
}
}
}
}
}
}
}
[Yingzhen]: the example is fixed.
Cheers,
Med
____________________________________________________________________________________________________________
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
To unsubscribe send an email to lsr-le...@ietf.org
