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

Reply via email to