Hi Med, I'll do this in two revisions with the first one addressing the DISCUSS comments and a second one incorporating more of the editorial comments.
I've completed addressing the DISCUSS points - https://www.ietf.org/archive/id/draft-ietf-ospf-sr-yang-35.txt > On Mar 27, 2025, at 12:45 PM, mohamed.boucad...@orange.com wrote: > > Hi Acee, > > Thank you for the follow-up. > > Please see inline. > > Cheers, > Med > >> -----Message d'origine----- >> De : Acee Lindem <acee.i...@gmail.com> >> Envoyé : jeudi 27 mars 2025 00:38 >> À : 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 Med, >> >> Let me respond to the high level DISCUSS points first. I'll address >> the other comments in a subsequent Email. >> >>> On Mar 26, 2025, at 12:13 PM, Mohamed Boucadair via Datatracker >> <nore...@ietf.org> wrote: >>> >>> Mohamed Boucadair has entered the following ballot position for >>> draft-ietf-ospf-sr-yang-34: 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.) >>> >>> >>> -------------------------------------------------------------------- >>> DISCUSS: >>> -------------------------------------------------------------------- >>> >>> Hi Yingzhen, Acee, Jeffrey, and Helen, >>> >>> Thank you for the effort put into this specification, especially for >>> your perseverance to progress the spec for almost ten years. I hope >> we >>> could produce device YANG modules quicker. >>> >>> The augmentations are overall adhering to the base specifications >>> defined in RFC8665/RFC8666. There are minor issues (but many, >> though) >>> that can be easily fixed. I found it unfortunate that we are mixing >>> message format vs. what can be managed (e.g., data nodes are >> mirroring TLVs, etc.). >>> >>> I will be definitely balloting “Yes” for this specification, but I’d >>> like to DISCUSS some few points. >>> >>> # Lack of narrative text >>> >>> I would expect a minimum of narrative text to explain the set of >>> management function supported by the model. For example, I can’t >> tell >>> whether we can store the algo capabilities of adj SR routers, etc. >>> >>> Please remember that RFC8407 says the following: >>> >>> 3.5. Narrative Sections >>> >>> The narrative part MUST include an overview section that describes >>> the scope and field of application of the module(s) defined by the >>> specification and that specifies the relationship (if any) of >> these >>> modules to other standards, particularly to standards containing >>> other YANG modules. The narrative part SHOULD include one or more >>> sections to briefly describe the structure of the modules defined >> in >>> the specification. >> >> This has proved to be a good exercise. >> > > [Med] I hope you meant include narrative text not omitting it :-) Yes - I mean including it. And I actually found some augmentations in the model that we'ren't needed. > >> >>> >>> # Implicit SID types >>> >>> CURRENT: >>> container sid-sub-tlv { >>> description >>> "Used to advertise the SID/Label associated with a >>> prefix or adjacency."; >>> leaf length { >>> type uint16; >>> 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 a label. If the length >>> is set to 4, then the value represents a 32-bit SID."; >>> >>> It is unfortunate to mix the wire encoding vs manageability >> function. >>> Why a type is not used here instead of implicitly inferring it form >> a length? >> >> For the LSAs, we've mirrored the encodings in most cases. However, I >> guess we could add a type. >> > > [Med] Thanks. > >>> >>> BTW, should that type (or length in the current version) be >> mandatory? >>> especially that the description says "is necessary"? >> >> These are read-only state data and we normally don't indicate >> mandatory fields in read-only data. I intend to update the model to >> make it much clearer as to which augmentations are read-write and >> which are read-only. >> > > [Med] Great. > >> >> >>> >>> # Ordering control of SID/Label >>> >>> How to control the order of SID/Label to follow this part from >> RFC8665? >>> >>> * The originating router decides the order in which the set of >> SID/ >>> Label Range TLVs are advertised inside the Router Information >>> Opaque LSA. >> >> This is left to the implementation. This is read-only data as well. >> > > [Med] ACK. Maybe say that in a description or in the narrative part. Sure - but this will be for my augmentations. > >> >>> >>> # Overlapping prefixes >>> >>> RFC8665 states that: >>> The originating router MUST NOT advertise overlapping ranges. >>> >>> How the model enforces this check on SID/Label Range? >> >> This is read-only data as well. >> >> If you wanted, you could file an Errata on RFC 9020 to enforce non- >> overlapping ranges. >> Offhand, I can't think of a way to enforce with YANG constraints other >> than in the text. >> > > [Med] Agree that it is not straightforward. Let's go for a simple fix and > simply remind this constraint in a description statement. The configuration is imported from RFC 9020 but I'll put the description on my augmentations. Note that even if there were a way to check the overlapping with a YANG statement, adding one to the ietf-sr-mpls module would not be backward compatible. > >> >>> >>> # SR capabilities >>> >>> It seems that this is defined only for OSPFv3 in the module, but >>> rfc8665#section-3 says the following: >>> >>> These SR capabilities are advertised in the Router Information >> Opaque >>> LSA (defined in [RFC7770]). The TLVs defined below are applicable >> to >>> both OSPFv2 and OSPFv3; see also [RFC8666]. >> >> The augmentations to the OSPFv2 RI LSA Opaque and OSPFv3 RI LSA are in >> the draft. For example: >> >> augment "/rt:routing/" >> + "rt:control-plane-protocols/rt:control-plane-protocol/" >> + "ospf:ospf/ospf:areas/" >> + "ospf:area/ospf:database/" >> + "ospf:area-scope-lsa-type/ospf:area-scope-lsas/" >> + "ospf:area-scope-lsa/ospf:version/ospf:ospfv2/" >> + "ospf:ospfv2/ospf:body/ospf:opaque/ospf:ri-opaque" { >> when "derived-from(/rt:routing/rt:control-plane-protocols/" >> + "rt:control-plane-protocol/rt:type, 'ospf:ospfv2')" { >> description >> "This augmentation is only valid for OSPFv2."; >> } >> description >> "SR-specific TLVs for OSPFv2 type 10 opaque LSA."; >> uses sr-algorithm-tlv; >> uses sid-range-tlvs; >> uses local-block-tlvs; >> uses srms-preference-tlv; >> } >> >> >> >> augment "/rt:routing/" >> + "rt:control-plane-protocols/rt:control-plane-protocol/" >> + "ospf:ospf/ospf:areas/" >> + "ospf:area/ospf:database/" >> + "ospf:area-scope-lsa-type/ospf:area-scope-lsas/" >> + "ospf:area-scope-lsa/ospf:version/ospf:ospfv3/" >> + "ospf:ospfv3/ospf:body/ospf:router-information" { >> when "derived-from(/rt:routing/rt:control-plane-protocols/" >> + "rt:control-plane-protocol/rt:type, 'ospf:ospfv3')" { >> description >> "This augmentation is only valid for OSPFv3."; >> } >> description >> "SR-specific TLVs for OSPFv3 Router Information LSA."; >> uses sr-algorithm-tlv; >> uses sid-range-tlvs; >> uses local-block-tlvs; >> uses srms-preference-tlv; >> } >> > > [Med] Thanks. I consider this one resolved. > > I rechecked my notes and I think that I was confused by the comment at > https://github.com/boucadair/IETF-Drafts-Reviews/blob/5a3ae50407e3e470c132f268db6767cbf948c42a/YANG/ietf-ospf-sr-mpls.yang#L1141 > -only for OSPFv3 Thanks, 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. _______________________________________________ Lsr mailing list -- lsr@ietf.org To unsubscribe send an email to lsr-le...@ietf.org
