Hi Julien - Thanks again for your review including and especially noticing perfix-sid-sub-tlvs. Now just waiting on Tom Petch’s promised WG last call review…
Acee > On Dec 5, 2023, at 3:15 AM, julien.meu...@orange.com wrote: > > 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 >>> > > ____________________________________________________________________________________________________________ > Ce message et ses pieces jointes peuvent contenir des informations > confidentielles ou privilegiees et ne doivent donc > pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu > ce message par erreur, veuillez le signaler > a l'expediteur et le detruire ainsi que les pieces jointes. Les messages > electroniques etant susceptibles d'alteration, > Orange decline toute responsabilite si ce message a ete altere, deforme ou > falsifie. Merci. > > This message and its attachments may contain confidential or privileged > information that may be protected by law; > they should not be distributed, used or copied without authorisation. > If you have received this email in error, please notify the sender and delete > this message and its attachments. > As emails may be altered, Orange is not liable for messages that have been > modified, changed or falsified. > Thank you. _______________________________________________ Lsr mailing list Lsr@ietf.org https://www.ietf.org/mailman/listinfo/lsr
