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%2Fdata
> > tracker.ietf.org%2Fdoc%2Fhtml%2Fdraft-ietf-netmod-rfc8407bis-
> 22%23name
> > -template-for-ietf-
> modules&data=05%7C02%7Cmohamed.boucadair%40orange.c
> >
> om%7Cdd93fc8f55524169027408dd6f83b410%7C90c7a20af34b40bfbc48b9253b6f5d
> >
> 20%7C0%7C0%7C638789333642674106%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcG
> >
> kiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoy
> >
> fQ%3D%3D%7C0%7C%7C%7C&sdata=o1APFE%2FrerQ%2FsiTEyKAY9z9n5tXn8v4kymYEDf
> > 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%2Fdata
> > tracker.ietf.org%2Fdoc%2Fhtml%2Fdraft-ietf-netmod-rfc8407bis-
> 22%23name
> > -security-considerations-
> sect&data=05%7C02%7Cmohamed.boucadair%40orang
> >
> e.com%7Cdd93fc8f55524169027408dd6f83b410%7C90c7a20af34b40bfbc48b9253b6
> >
> f5d20%7C0%7C0%7C638789333642691638%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1
> >
> hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUI
> >
> joyfQ%3D%3D%7C0%7C%7C%7C&sdata=928yJgTF2gV2E%2BVUOkMzQ57WY0F1p3%2Fry0H
> > 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

Reply via email to