Hi Dhruv, Thanks for the review. I've uploaded version -01 and addressed your comments. Please see details below.
Thanks, Yingzhen On Mon, Sep 1, 2025 at 11:11 AM Dhruv Dhody via Datatracker < [email protected]> wrote: > Document: draft-ietf-lsr-ospf-flex-algo-yang > Title: YANG Data Model for OSPF Application-Specific Link Attributes and > Flexible Algorithm Reviewer: Dhruv Dhody Review result: Almost Ready > > # Early YANGDOCTORS review of draft-ietf-lsr-ospf-flex-algo-yang > > This document includes two IANA-maintained modules - "iana-igp-algo-types" > and > "iana-igp-metric-types". Further, it has two YANG modules for OSPF > Application-Specific Link Attributes "ietf-ospf-link-attr" and Flex Algo > "ietf-ospf-flex-algo". > > ## YANG > > - Please run "pyang --ietf --lint -f yang --keep-comments > --yang-line-length > 69" to fix the formatting in the YANG modules. > [Yingzhen]: done. > > ### iana-igp-algo-types and iana-igp-metric-types > > - Please update the date > > - In the description clause, please add > ``` > All revisions of IETF and IANA published modules can be found > at the YANG Parameters registry group > (https://www.iana.org/assignments/yang-parameters). > ``` > - Please make this change > ``` > OLD: > The latest version of this YANG module is available at > <mailto:[email protected]>"; > NEW: > The latest version of this YANG module is available at > https://www.iana.org/assignments/yang-parameters.";; > END > ``` > - Remove the editor note about IANA_FOO_URL > [Yingzhen]: fixed. > > - Please add a reference statement for various identities in > iana-igp-metric-types. The description for some of them also includes RFC > in > text; would it not make sense to include those in the reference statement > as > well? > [Yingzhen]: I added references as in the registry. > > ### ietf-ospf-link-attr > > - Remove BCP14 boilerplate as keywords are not used in the module. > > - Please remove 'reference "RFC XXXX";' after description. > > - Please update the RFC title in the revision clause. > > - Please add a reference statement to identities. > > - Please add reference to grouping app-specific-link-attr-sub-tlvs > > [Yingzhen]: all fixed. > > ### ietf-ospf-flex-algo > > - Please add a reference statement for imports. > > - Remove BCP14 boilerplate as keywords are not used in the module. > > - Please remove 'reference "RFC XXXX: YANG Data Model for OSPF Flexible > Algorithm.";' after description. > > - Please update the RFC title in the revision clause. > > - Please add a reference statement to identities. > [Yingzhen]: all fixed. > > - It is unclear why groupings are created for fa-ex-ag-sub-tlv, > fa-in-any-ag-sub-tlv, fa-in-all-ag-sub-tlv, fad-flags-sub-tlv, > fa-ex-srlg-sub-tlv, when they are only used once and can easily be defined > inline in the grouping fad-tlvs. Please also add a reference statement in > groupings. > [Yingzhen]: removed groupings for those sub-tvs. > > - For leaf flex-algo, consider adding "{ range "128..255" }". I note that > you > have it for leaf algo-number but not for flex-algo, any reason for it? > [Yingzhen]: the range is added only for configuration. flex-algo is a read-only from fad-tlv. > > - For extended-admin-groups, why unit64? RFC 7308 says EAG is a multiple > of 4 > bytes. Should it not be 32? > [Yingzhen]: Thanks for catching this. I don't remember why it was set to uint64. > > - In configuration for metric-type leaf, should it not have a default > statement > for IGP metric? > [Yingzhen]: Added a default to igp-metric.. > > - Why does OSPFv2 FAAM modeling differs from OSPFv3 wrt unknown-tlvs? If > there > is a good reason, perhaps describe it in text in the draft. > [Yingzhen]: They are defined at different level of TLVs. OSPFv3 inter-router-tlv already has unknown tlvs included(RFC 9587). > > ## Draft > > - Please add a brief paragraph that explains the tree in section 4. This > can be > done by listing the three places in OSPF YANG where the augmentation is > being > done and how to connect back to the ASLA feature. > > - Same for section 5. > > [Yingzhen]: done. > - It would be good to explain some design choices, such as why container > prefix-metric with presence, use of unknown-tlvs in the model when ASLA > subtlv > can be known, the idea of key-less list to allow multiple instances, etc. > > - I was wondering why the IANA IGP modules that are common for both OSPF > and > ISIS are in an OSPF document. Perhaps state that explicitly to avoid > confusion. > > [Yingzhen]: these registries are common for both OSPF and ISIS. > - Please follow the security consideration template in > > https://www.ietf.org/archive/id/draft-ietf-netmod-rfc8407bis-28.html#section-3.7.1 > > - Please update sections 7.2 and 7.3 to match the template at > > https://www.ietf.org/archive/id/draft-ietf-netmod-rfc8407bis-28.html#section-4.30.3.1 > ; > there are slight inconsistencies. > > [Yingzhen]: both updated. > ## Nits > > - s/which itself augments [RFC8349]/which itself augments the IETF Routing > model [RFC8349]/ > > [Yingzhen]: fixed. > Thanks, > Dhruv > > >
_______________________________________________ Lsr mailing list -- [email protected] To unsubscribe send an email to [email protected]
