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