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. 

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..!).  

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

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

Reply via email to