Hi Tom, > On Jan 1, 2024, at 07:34, tom petch <ie...@btconnect.com> wrote: > > From: Acee Lindem <acee.i...@gmail.com> > Sent: 12 December 2023 22:25 > > Hi Tom, > >> On Dec 11, 2023, at 7:45 AM, tom petch <ie...@btconnect.com> wrote: > > Acee > > top posting since most of my comments are addressed in -25 (which I have > reviewed) > > Renaming the YANG module is a pain but probably needs doing on the assumption > that there will be a ospf-srv6-yang at some stage and that you will not want > to merge the details of that into the module specified here. Of course it > may then be that just as OSPFv2 and OSPFv3 have much in common and have been > merged into a single module, so SRv6 may get merged in in which case the > original name would be better. Sometimes you cannot win!
Yes - there are separate SRv6 models for OSPFv3 and IS-IS. They aren’t really to progress and there is a different set of authors. Thanks, Acee > > Tom Petch > I do see routing-ti-lfa progressing this week so hopefully Normative will be > ok. > > > > > >> >> A convenient addressee list so top posting my first thoughts on >> ospf-sr-yang, I hope to find time to have a more detailed look, at least at >> ospfv2. >> >> I have looked at ospf-sr-yang and have some queries. >> >> Is this all flavours of SR or just some? Most discussion I see these days >> relates to SRv6 I guess because SR-MPLS is mature in many respects but >> think that this I-D needs to spell out the scope (like its lsr twin) > > This specifies OSPF SR for the MPLS data plane. I’m considering renaming the > data module to ietf-ospf-sr-mpls.yang as well. > >> I note the import from sr-mpls and think it a mistake. The routing RFC says >> that new protocols should have a presence container to switch the protocol >> on and off which sr-mpls does not do but I think that ospf-sr-yang should >> follow the guidelines. > > We need to follow the sr-mpls model. We can’t change it in the OSPF SR model. > > >> >> There are mentions of vendor augmentations but no indications of what they >> might be and, importantly, where they would go. Other I-D, anticipating >> augments, include containers explicitly for augments so that different >> vendors put the same information in the same place. >> >> I am used to ospfv2 and ospfv3 being derived identities from ospf which >> makes reference to one of the other or both simple, as ospf-yang does. Why >> not here? > > I’ve updated these to use derived-from() and the current path. > > >> >> I-D references seems to lack >> RFC8102 >> "draft-ietf-rtgwg-segment-routing-ti-lfa - >> Latter needs to be Normative since a feature > > I hate making the latter normative but I guess it needs to be hopefully the > authors of this draft will finally bring it to completion. > > > >> >> s.1.1 is ood > > This has been removed. > > >> >> router-id is provided by RFC8294 so it should be imported and not be >> reinvented here > > Okay - I have used this definition. > > >> >> import ietf-ospfv3-extended-lsa { >> lacks a reference clause >> >> leaf preference { >> type uint8; >> description >> "SRMS preference TLV, value from 0 to 255."; >> >> so what? what difference soes it make to be 0 or 255 or 42? > > The description has been updated to indicate that an SR Mapping Server with a > higher preference is preferred. > > Thanks, > Acee > > > >> >> Tom Petch >> >> ________________________________________ >> From: Lsr <lsr-boun...@ietf.org> on behalf of julien.meu...@orange.com >> <julien.meu...@orange.com> >> Sent: 05 December 2023 08:15 >> >> Hi Acee, >> >> I've looked at the diff: the new version looks good to me. Thanks to the >> update. >> >> Regards, >> >> Julien >> >> >> On 01/12/2023 18:05, Acee Lindem wrote: >>> Hi Julien, >>> >>> Thanks much for your review. I’ve incorporated almost all of your comments >>> in the -23 version. >>> >>> See inline. >>> >>>> On Nov 29, 2023, at 11:03 AM, julien.meu...@orange.com wrote: >>>> >>>> Hello, >>>> >>>> I have been selected as the Routing Directorate reviewer for this draft. >>>> The Routing Directorate seeks to review all routing or routing-related >>>> drafts as they pass through IETF last call and IESG review, and sometimes >>>> on special request. The purpose of the review is to provide assistance to >>>> the Routing ADs. For more information about the Routing Directorate, >>>> please see https://wiki.ietf.org/en/group/rtg/RtgDir >>>> <https://wiki.ietf.org/en/group/rtg/RtgDir> >>>> >>>> Although these comments are primarily for the use of the Routing ADs, it >>>> would be helpful if you could consider them along with any other IETF Last >>>> Call comments that you receive, and strive to resolve them through >>>> discussion or by updating the draft. >>>> >>>> Document: draft-ietf-ospf-sr-yang-22 >>>> Reviewer: Julien Meuric >>>> Review Date: 2023-11-29 >>>> Intended Status: Standard Tracks >>>> >>>> >>>> *Summary:* >>>> >>>> This document is basically ready for publication but has nits that should >>>> be considered prior to publication. >>>> >>>> >>>> *Comments:* >>>> >>>> - The very first paragraph of the introduction/overview section summarizes >>>> the basis of YANG, XML, JSON, data models... I believe we are now far >>>> beyond those general considerations and we could skip that paragraph. >>> Removed - thanks. >>> >>> >>>> - In the grouping "ospfv3-lan-adj-sid-sub-tlvs" (p23), the leaf >>>> "neighbor-router-id" uses type "dotted-quad". This is consistent with RFC >>>> 8666 which specifies the associated OSPFv3 TLV, but we had a discussion >>>> about the type for router-id in the TE YANG models. The current resolution >>>> on TEAS side will be to consider a union of dotted-quad and ipv6-address. >>>> I wonder how much RTGWG would be ready to consider a superset of the >>>> existing OSPFv3 TLVs. >>> This is the OSPF Router-ID which is different from the OSPF TE Router-ID. >>> The two should not be confused as the OSPF Router ID is simply a 32 bit >>> unsigned integer that is typically represented in dotted quad format. It >>> only need be unique within the OSPF Routing Domain. Conversely, the OSPF TE >>> Router ID is a routable IPv4 or IPv6 address. >>> >>>> From RFC 2328 (which was inherited by RFC 5340): >>> >>> Router ID >>> A 32-bit number assigned to each router running the OSPF >>> protocol. This number uniquely identifies the router within >>> an Autonomous System. >>> >>>> >>>> *Nits:* >>>> >>>> - Multiple times in description: s/SR specific/SR-specific/ >>> Fixed. >>> >>> >>>> - Multiple times in description: s/flag bits list/flag list/ >>>> - Multiple times in description: s/flags list/flag list/ >>> I changed these to either just “bits” or “flags” - the fact that it is a >>> YANG list need not be included in the description. >>> >>> >>>> - The description fields use a mix of "Adj sid", "adj sid", "Adj SID"... >>>> sometimes with hyphens (not to mention the full expansions). A single >>>> phrase should be chosen and used all along the module. >>> Changed them all to “Adj-SID” consistent with RFC8665. >>> >>>> - A few description starts with "The..." (e.g., in >>>> "ospfv2-extended-prefix-range-tlvs" on p 19, or v3 on p 22) while most of >>>> them don't. For consistency, it should be dropped from every brief >>>> description. >>> I removed “The “ from all the brief descriptions. I left it in two of the >>> TLV description that were written as complete sentences. >>> >>>> - In the grouping "ospfv3-prefix-sid-sub-tlvs" (p 21 and all resulting >>>> pieces of tree): s/perfix-sid-sub-tlvs/prefix-sid-sub-tlvs/ >>>> - In the same grouping, the description of the container should be "Prefix >>>> SID sub-TLV *list*." (and "Prefix SID sub-TLV." reserved for the following >>>> list element). >>> Fixed both in the module and tree (which was regenerated from tree). >>> >>> >>>> - In the container "ti-lfa" (p 25): s/Enables TI-LFA/Enable TI-LFA/ [Not >>>> wrong, but should be consistent with others.] >>> Fixed. >>> >>>> - In the same container (p 26): "s/Topology Independent Loop Free >>>> Alternate/Topology-Independent Loop-Free Alternate/ >>> Fixed in this place and in another. >>> >>>> - In section 3 (p 37): s/The YANG modules [...] define/The YANG module >>>> [...] defines/ >>> Fixed. >>> >>>> - In the same section: s/in the modules/in the module/ >>> Fixed. >>> >>>> - In the same section: s/Module ietf-ospf-sr/The module ietf-ospf-sr/ >>> Fixed. >>> >>> Thanks, >>> Acee >>> >>> >>>> >>>> Thanks, >>>> >>>> Julien >>>> >> >> _________________________________ > > _______________________________________________ Lsr mailing list Lsr@ietf.org https://www.ietf.org/mailman/listinfo/lsr