> On Dec 18, 2023, at 12:28, tom petch <[email protected]> wrote: > > From: Acee Lindem <[email protected]> > Sent: 18 December 2023 13:14 > To: tom petch > Cc: [email protected]; Routing Directorate; > [email protected]; Lsr > Subject: Re: [Lsr] RtgDir Last Call Review: draft-ietf-ospf-sr-yang > > Tom, > > > On Dec 18, 2023, at 07:47, tom petch <[email protected]> wrote: > > I have yet to catch up with -24 but still on -23, Ithink that you should > explain where the OSPFv3 YANG augments came from with a Informative > Refeerence to draft-acee-lsr-ospfv3-sr-yang. It has taken me since last > Thursday to work it out:-(. Unadopted individual drafts do not rate highly > in the datatracker. > > This draft includes both the SR MPLS augmentations for OSPFv2 and OSPFv3. > draft-acee-lsr-ospfv3-sr-yang is not obsolete. They were separate drafts at > one time since the latter was dependent on the OSPFv3 Extended LSA YANG model > which some day will be reviewed out AD and progressed. > > <tp> > All very true but it is ospf-sr-yang that is now progressing - draft-acee is > not - and which grew 50% in size when the thirteen augments from draft-acee > were incorporated so I see that needing a mention in ospf-sr-yang along with > an Informational Reference. > > The nature of the model and the form that that imposes on the augments makes > it hard to follow but AFAICT twelve of the augments came across unchanged, > one, relating to nssa, changed its augment point. Whether this fixed a > fault, introduced a fault or ... I cannot tell. > > It stills needs a mention IMHO
I disagree - you don’t see the history of every draft merger and split in the finished RFCs since they are all “Work in Progress” until published. Thanks, Acee > > I am aware of and have reviewed extended-lsa-yang > > Tom Petch > > > <https://datatracker.ietf.org/doc/draft-ietf-lsr-ospfv3-extended-lsa-yang/> > [ietf-logo-card.png] > YANG Model for OSPFv3 Extended > LSAs<https://datatracker.ietf.org/doc/draft-ietf-lsr-ospfv3-extended-lsa-yang/> > datatracker.ietf.org<https://datatracker.ietf.org/doc/draft-ietf-lsr-ospfv3-extended-lsa-yang/> > > > > The fact that is was never adopted may have implications, such as IPR and the > like, I do not know, but think it needs stating if only by implication > (darft-acee..!). > > We still have a WG last call to do on this draft so you needn’t worry. > > > > I had noticed and reviewed draft-acee and was waiting for a call for adoption > to make m comments - e.g. perfix - but the call never came. I think my > comments are addressed in -21, when ospfv3 was added, but I will check again > in -24 > > This problem is fixed in ietf-ospf-sr-mpls.yang > > Thanks, > Acee > > > Tom Petch > > ________________________________________ > From: Acee Lindem <[email protected]> > Sent: 12 December 2023 22:25 > To: tom petch > Cc: [email protected]; Routing Directorate; > [email protected]; Lsr > Subject: Re: [Lsr] RtgDir Last Call Review: draft-ietf-ospf-sr-yang > > Hi Tom, > > On Dec 11, 2023, at 7:45 AM, tom petch <[email protected]> wrote: > > 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 <[email protected]> on behalf of [email protected] > <[email protected]> > 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, [email protected] 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 [email protected] https://www.ietf.org/mailman/listinfo/lsr
