Hi Med,
See inline.
> On Apr 18, 2025, at 6:59 AM, mohamed.boucad...@orange.com wrote:
>
> Hi Acee,
>
> Thank you for handling the comments in a timely manner and implementing the
> changes.
>
> I made a check using diff draft-ietf-ospf-sr-yang-34 vs
> draft-ietf-ospf-sr-yang-46. Great to see that both ISIS/OPSF spec follow the
> same approach for the sid encoding part.
>
> There are still some very few comments:
>
> (1) I don't find Inter-Area Flag in 8666:
>
> CURRENT:
> identity ia-flag {
> base extended-prefix-range-flag;
> description
> "Inter-Area flag.";
> reference
> "RFC 8665: OSPF Extensions for Segment Routing, Section 4
> RFC 8666: OSPFv3 Extensions for Segment Routing, Section 5";
> }
It is not needed for OSPFv3 since the SR extensions can be added to the
Extended-LSAs and there is are separate LSAs for inter-area advertisement.
I'll remove the reference to RFC 8666.
>
> Can you please double check? If that flag is only defined in 8665, we may
> consider controlling its use with a feature.
Let's not get carried away - this is a bit in read-only data. Adding a feature
would be ludicrous.
>
> (2) I suggest we delete as this is not allowed with the range statement
> (multiple occurrences):
>
> "Topologies range from 0-127 and return of any other value would indicate an
> error."
Sure.
> (3) Please complete this part with the sensitive data nodes:
>
> CURRENT:
> Some of the readable data nodes in this YANG module 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. Specifically, the following
> subtrees and data nodes have particular sensitivities/
> vulnerabilities:
Adding every data node in every LSA is just a waste of time and space. I'll
address them generically as is done in
https://datatracker.ietf.org/doc/draft-ietf-idr-bgp-model/
>
> (4) Please fix the references to follow this guidance from 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.
RFC 8341 does have a normative reference in the current version -
https://datatracker.ietf.org/doc/draft-ietf-ospf-sr-yang/
What do you mean?
Thanks,
Acee
>
> Please let me know when a new version is available so that I can clear my
> DISCUSS.
>
> Thank you.
>
> Cheers,
> Med
>
>> -----Message d'origine-----
>> De : BOUCADAIR Mohamed INNOV/NET
>> Envoyé : dimanche 30 mars 2025 15:22
>> À : Acee Lindem <acee.i...@gmail.com>
>> Cc : The IESG <i...@ietf.org>; draft-ietf-ospf-sr-y...@ietf.org;
>> lsr-cha...@ietf.org; lsr <lsr@ietf.org>; Christian Hopps
>> <cho...@chopps.org>
>> Objet : RE: Mohamed Boucadair's Discuss on draft-ietf-ospf-sr-yang-
>> 34: (with DISCUSS and COMMENT)
>>
>> Hi Acee,
>>
>> Thanks. Will do a full check together with the IS-IS spec.
>>
>> One quick comment about the example, I confirm that it has many
>> pbs: it does not adhere at least to the following from RFC7951:
>>
>> A namespace-qualified member name MUST be used for all members
>> of a
>> top-level JSON object and then also whenever the namespaces of
>> the
>> data node and its parent node are different. In all other
>> cases, the
>> simple form of the member name MUST be used.
>>
>> +
>>
>> An "identityref" value is represented as a string -- the name of
>> an
>> identity. If the identity is defined in a module other than the
>> leaf
>> node containing the identityref value, the namespace-qualified
>> form
>> (Section 4) MUST be used. Otherwise, both the simple and
>> namespace-
>> qualified forms are permitted.
>>
>> Better to validate this using tools. Noted that you will do.
>>
>> As a general note, I hope we can have this integrated as part of
>> idnits because otherwise identifying issues depends on reviewers
>> eyes ;-)
>>
>> Cheers,
>> Med
>>
>>> -----Message d'origine-----
>>> De : Acee Lindem <acee.i...@gmail.com> Envoyé : dimanche 30 mars
>> 2025
>>> 14:09 À : BOUCADAIR Mohamed INNOV/NET
>> <mohamed.boucad...@orange.com>
>>> Cc : The IESG <i...@ietf.org>; draft-ietf-ospf-sr-y...@ietf.org;
>> lsr-
>>> cha...@ietf.org; lsr <lsr@ietf.org>; Christian Hopps
>>> <cho...@chopps.org> Objet : Re: Mohamed Boucadair's Discuss on
>>> draft-ietf-ospf-sr-yang-34:
>>> (with DISCUSS and COMMENT)
>>>
>>>
>>> Hi Mohamed,
>>>
>>> I've addressed the non-DISCUSS level comments in -36. I've
>>> incorporated most but not all of your comments.
>>>
>>>>
>>>>
>>>>
>>>> ---------------------------------------------------------------
>> -----
>>> --
>>>> COMMENT:
>>>> ---------------------------------------------------------------
>> -----
>>> --
>>>>
>>>> # Abstract: "configure" is covered by "manage" (remember the
>> FCAPS
>>>> functions)
>>>>
>>>> OLD:
>>>> This document defines a YANG data module that can be used to
>>>> configure and manage OSPF Extensions for Segment Routing for
>> the
>>> MPLS
>>>> data plane.
>>>>
>>>> NEW :
>>>> This document defines a YANG data module that can be used to
>>>> manage OSPF Extensions for Segment Routing (SR) for the MPLS
>>>> data plane.
>>>>
>>>> # Introduction: Simplify + remove "configure" as this is
>> covered by
>>> "manage"
>>>>
>>>> OLD:
>>>> This document defines a YANG data model [RFC7950] that can be
>> used
>>> to
>>>> configure and manage OSPFv2 extensions for Segment Routing
>>> [RFC8665]
>>>> and OSPFv3 extensions for Segment Routing [RFC8666] for the
>> MPLS
>>> data
>>>> plane. It is an augmentation to the OSPF YANG data model
>>> [RFC9129].
>>>>
>>>> NEW:
>>>> This document defines a YANG data model [RFC7950] that can be
>> used
>>> to
>>>> manage Segment Routing (SR) for OSPFv2 [RFC8665]
>>>> and OSPFv3 [RFC8666] for the MPLS data
>>>> plane. It is an augmentation to the OSPF YANG data model
>>> [RFC9129].
>>>
>>> Fixed for both.
>>>
>>>
>>>>
>>>> # Section 2
>>>>
>>>> ## (redundant) I would delete the following as this was stated
>> few
>>> lines above.
>>>>
>>>> CURRENT:
>>>> It is an augmentation of the OSPF base model.
>>>>
>>>> ## The module is not only for configuration, but for state
>>> retrieval.
>>>> The document says that it adheres to the NMDA ;-)
>>>>
>>>> Please update the following accordingly:
>>>>
>>>> CURRENT:
>>>> The OSPF SR 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 OSPF base model [RFC9129] which
>>> defines
>>>> basic OSPF configuration and state.
>>>
>>> Fixed to a separate section with narrative text.
>>>
>>>>
>>>> ## Long tree diagram
>>>>
>>>> Please move the full tree to an appendix but consider snippets
>> to
>>> help
>>>> readers go through the model.
>>>>
>>>> Refer to Section 3.4 of 8407bis for more guidance.
>>>
>>> Done.
>>>
>>>>
>>>> # Section 2.1: Check references
>>>>
>>>> CURRENT:
>>>> [RFC2328], [RFC4750], [RFC4915], [RFC5340], [RFC5643],
>> [RFC5838],
>>>> [RFC6991], [RFC8102], [RFC8294], [RFC8343], [RFC8476],
>> [RFC8349],
>>>> [RFC9587], and [I-D.ietf-rtgwg-segment-routing-ti-lfa] are
>>> referenced
>>>> in the YANG data model.
>>>>
>>>> For example, I failed to find where these ones are cited
>> [RFC4750],
>>>> [RFC5643], [RFC5838], [RFC8343 ], [RFC8476].
>>>
>>> Fixed.
>>>
>>>
>>>>
>>>> # Section 2.2
>>>>
>>>> ## Please use the template at
>>>>
>>>
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fd
>> ata
>>>> tracker.ietf.org%2Fdoc%2Fhtml%2Fdraft-ietf-netmod-rfc8407bis-
>>> 22%23name
>>>> -template-for-ietf-
>>> modules&data=05%7C02%7Cmohamed.boucadair%40orange.c
>>>>
>>>
>> om%7Cdd93fc8f55524169027408dd6f83b410%7C90c7a20af34b40bfbc48b9253b6
>> f5d
>>>>
>>>
>> 20%7C0%7C0%7C638789333642674106%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1
>> hcG
>>>>
>>>
>> kiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUI
>> joy
>>>>
>>>
>> fQ%3D%3D%7C0%7C%7C%7C&sdata=o1APFE%2FrerQ%2FsiTEyKAY9z9n5tXn8v4kymY
>> EDf
>>>> Iw%2FPg%3D&reserved=0
>>>>
>>>> ## Description issues
>>>>
>>>> CURRENT:
>>>> This YANG module defines the generic configuration
>>>> and operational state for OSPF Segment Routing (SR),
>> which
>>>> is common across all of the vendor implementations. It
>> is
>>>> intended that the module will be extended by vendors to
>>>> define vendor-specific OSPF Segment Routing
>> configuration
>>>> and operational parameters for the MPLS data plane.
>>>>
>>>> * How you checked this part "is common across all of the vendor
>>>> implementations"?
>>>>
>>>> * I would remove the last sentence. It is weird to make
>> assumption
>>>> about what proprietary extensions in a STD spec!
>>>
>>> Ok.
>>>
>>>
>>>>
>>>> ## Consistent referencing style: The module include a mix of
>> styles
>>>> for the reference statements. Please update accordingly. For
>>> example,
>>>>
>>>> OLD: "RFC 6991 - Common YANG Data Types";
>>>> NEW: "RFC 6991: Common YANG Data Types";
>>>
>>> Fixed
>>>
>>>>
>>>> ## Lack of references:
>>>>
>>>> For example, consider adding the following for all prefix-sid-
>> flag
>>>>
>>>> NEW:
>>>> reference
>>>> "RFC 8665: OSPF Extensions for Segment Routing, Section 5
>>>> RFC 8666: OSPFv3 Extensions for Segment Routing, Section
>> 6";
>>>
>>> This module already had more references than most but I've added
>> more.
>>>
>>>
>>>>
>>>> ## Some description are copied from the base spec, while the
>>> statement
>>>> does not make sense for a given type.
>>>>
>>>> For example, the following is included for an identity, but 'if
>> set,
>>>> ..' does not make sense for an identity. This is not a flag
>> bit. An
>>>> example is provided below (but other similar constructs are
>> included
>>>> in the module)
>>>>
>>>> OLD:
>>>> "Inter-Area flag. If set, advertisement is of
>>>> inter-area type.";
>>>> NEW:
>>>> "Inter-Area flag.";
>>>
>>> Fixed.
>>>
>>>>
>>>> ## Lack of mapping between some identities and the flag names
>> as
>>>> defined in the base spec. For example, the description for vi-
>> flag
>>>> should indicate that this corresponds to the V-Flag as there is
>> no
>>> VI-Flag in 8665/8666:
>>>
>>> This is due to pyang not being able to disambiguate the duplicate
>>> identities with different base identities.
>>>
>>>
>>>>
>>>> OLD:
>>>> "Value/Index flag.";
>>>>
>>>> NEW:
>>>> "Value/Index flag. Corresponds to the Adj-SID V-Flag.";
>>>> reference
>>>> "RFC 8665: OSPF Extensions for Segment Routing, Section
>> 6.1
>>>> RFC 8666: OSPFv3 Extensions for Segment Routing, Section
>>> 7.1";
>>>>
>>>> Idem, please indicate that lo-flag corresponds to the L-Flag
>> defined
>>>> in
>>>> 8665/8666 as there is no LO-Flag in 8665/8666.
>>>>
>>>> OLD:
>>>> "Local/Global flag.";
>>>>
>>>> NEW:
>>>> "Local/Global flag. Corresponds to the Adj-SID L-Flag.";
>>>> reference
>>>> "RFC 8665: OSPF Extensions for Segment Routing, Section
>> 6.1
>>>> RFC 8666: OSPFv3 Extensions for Segment Routing, Section
>>> 7.1";
>>>
>>> Fixed.
>>>
>>>>
>>>> ## No need to repeat the parent node prefix. There are many
>>>> occurrences of this in the module.
>>>
>>> I'm going to leave these as is. In these cases, the list elements
>> are
>>> TLVs.
>>>
>>>
>>>>
>>>> OLD: list prefix-sid-sub-tlv {
>>>> NEW: list prefix-sid {
>>>>
>>>> OLD: list extended-prefix-range-tlv {
>>>> NEW: list extended-prefix-range {
>>>>
>>>> OLD: container extended-prefix-range-flags {
>>>> NEW: container flags {
>>>>
>>>> Etc.
>>>>
>>>> ## Use Singular for list/leaf-list per rfc8407bis, for example:
>>>>
>>>> OLD: leaf-list flags {
>>>> NEW: leaf-list flag {
>>>>
>>>> There are many occurrences in the text.
>>>
>>> Fixed.
>>>
>>>
>>>
>>>
>>>>
>>>> ## Defaults: Please provide an authoritative reference where
>> the
>>>> default is defined + indicate the meaning of this default.
>>>>
>>>> For example, it is not clear for the definition the meaning of
>> this
>>> value:
>>>>
>>>> CURRENT:
>>>> leaf priority {
>>>> type uint8;
>>>> default "128";
>>>
>>> In the context of the container, the default should be obvious.
>>>
>>>
>>>>
>>>> ## Restrict using range: restrict the range rather than having
>> this
>>> in
>>>> the description.
>>>>
>>>> CURRENT:
>>>> leaf mt-id {
>>>> type uint8;
>>>> description
>>>> "Multi-topology ID. Topologies range from 0-127 and
>>>> return of any other value would indicate an
>> error.";
>>>>
>>>> # Section 3: Security template
>>>>
>>>> Please follow the template defined in
>>>>
>>>
>> https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fd
>> ata
>>>> tracker.ietf.org%2Fdoc%2Fhtml%2Fdraft-ietf-netmod-rfc8407bis-
>>> 22%23name
>>>> -security-considerations-
>>> sect&data=05%7C02%7Cmohamed.boucadair%40orang
>>>>
>>>
>> e.com%7Cdd93fc8f55524169027408dd6f83b410%7C90c7a20af34b40bfbc48b925
>> 3b6
>>>>
>>>
>> f5d20%7C0%7C0%7C638789333642691638%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0
>> eU1
>>>>
>>>
>> hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIl
>> dUI
>>>>
>>>
>> joyfQ%3D%3D%7C0%7C%7C%7C&sdata=928yJgTF2gV2E%2BVUOkMzQ57WY0F1p3%2Fr
>> y0H
>>>> NiL533kk%3D&reserved=0
>>>
>>> Fixed
>>>
>>>>
>>>> # Section 5: Indicate whether this is maintained by IANA or not
>>>>
>>>> OLD:
>>>> name: ietf-ospf-sr-mpls
>>>> namespace: urn:ietf:params:xml:ns:yang:ietf-ospf-sr-
>> mpls
>>>> prefix: ospf-sr-mpls
>>>> reference: RFC XXXX
>>>>
>>>> NEW:
>>>> name: ietf-ospf-sr-mpls
>>>> namespace: urn:ietf:params:xml:ns:yang:ietf-ospf-sr-
>> mpls
>>>> prefix: ospf-sr-mpls
>>>> maintained by IANA? N
>>>> reference: RFC XXXX
>>>>
>>>> # Section 6: Issues with classifying references
>>>>
>>>> RFC6241, RFC6242, RFC8040, RFC8343, RFC8446, and RFC8476 are
>> cited
>>> as
>>>> normative, while they shouldn't. Please move those to be listed
>> as
>>> Informative.
>>>
>>> Fixed. RFC8343 and RFC8476 were removed.
>>>
>>>>
>>>> # Appendix
>>>>
>>>> ## Redundant examples
>>>>
>>>> Do we really need to have the xml example given that the JSON
>>> encoding
>>>> for the same example is provided?
>>>
>>> I'd be glad to only have json. Leaving in -36
>>>
>>>
>>>>
>>>> ## Broken JSON example
>>>>
>>>> Please validate the example using yanglint, etc.
>>>
>>> We'll discuss among the co-authors but I believe this was
>> validated.
>>> It looks like all you did was replace the module prefixes with
>> the
>>> module names.
>>>
>>> Thanks
>>> Acee
>>>
>>>
>>>
>>>>
>>>> OLD:
>>>> {
>>>> "routing": {
>>>> "router-id": "1.1.1.1",
>>>> "control-plane-protocols": {
>>>> "control-plane-protocol": {
>>>> "type": "ospf:ospfv2",
>>>> "name": "OSPFv2",
>>>> "ospf": {
>>>> "areas": {
>>>> "area": {
>>>> "area-id": "0.0.0.0",
>>>> "interfaces": {
>>>> "interface": {
>>>> "name": "eth0",
>>>> "segment-routing": {
>>>> "adjacency-sid": {
>>>> "adj-sids": {
>>>> "value": 3888
>>>> }
>>>> }
>>>> }
>>>> }
>>>> }
>>>> }
>>>> },
>>>> "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-ospf:ospfv2",
>>>> "name": "OSPFv2",
>>>> "ietf-ospf:ospf": {
>>>> "areas": {
>>>> "area": {
>>>> "area-id": "0.0.0.0",
>>>> "interfaces": {
>>>> "interface": {
>>>> "name": "eth0",
>>>> "ietf-ospf-sr-mpls:segment-routing": {
>>>> "adjacency-sid": {
>>>> "adj-sids": {
>>>> "value": 3888
>>>> }
>>>> }
>>>> }
>>>> }
>>>> }
>>>> }
>>>> },
>>>> "ietf-ospf-sr-mpls:segment-routing": {
>>>> "enabled": true
>>>> },
>>>> "ietf-ospf-sr-mpls:protocol-srgb": {
>>>> "srgb": {
>>>> "lower-bound": 4000,
>>>> "upper-bound": 5000
>>>> }
>>>> }
>>>> }
>>>> }
>>>> }
>>>> }
>>>> }
>>>>
>>>> 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
