I'm sorry for the noise and for the current state of the tooling. Please find my *real* review below.
Adrian ======================= Hi, I have been selected as the Operational Directorate (opsdir) reviewer for this Internet-Draft. The Operational Directorate reviews all operational and management-related Internet-Drafts to ensure alignment with operational best practices and that adequate operational considerations are covered. A complete set of "Guidelines for Considering Operations and Management in IETF Specifications" can be found at https://datatracker.ietf.org/doc/draft-opsarea-rfc5706bis/. While these comments are primarily for the Operations and Management Area Directors (Ops ADs), the authors should consider them alongside other feedback received. Thank you for your work on this document. Please address these comments or feel free to contact me for clarification and/or discussion. Best regards, Adrian ====== Document: draft-ietf-lsr-ospf-flex-algo-yang-05 Reviewer: Adrian Farrel ([email protected]) Review Date: 2026-03-24 Intended Status: Standards Track Summary Has Major Issues: I have significant concerns about this document and recommend that the OPS ADs discuss these issues further with the authors. This document defines YANG models for handling flex-algo and application-specific link attributes in OSPF. As such, it depends heavily on the protocol work in RFC 9350 and RFC 9492. The document contains two IANA-managed YANG modules, one new module, and one that augments the YANG model defined in RFC 9129. This document is a pair with draft-ietf-lsr-isis-flex-algo-yang which depends on the two IANA-managed YANG modules defined here. The document shepherd write-up is deficient. In answer to question 4 (For protocol documents, are there existing implementations of the contents of the document? ) it says "N/A". But this is a Standards Track protocol document. YANG models are implementable and it would give significant credence to the completeness of this specification if this question had been answered with implementation details. (Of course, it would have been even better to see an Implementation Status section in the document.) It remains up to the WG and AD whether to pursue publication of an unimplemented YANG model. == Management Considerations == Failure cases are covered by a single Notification for flex-algo not supported. There is no equivalent for metric not supported, although that is a less critical issue. There are no controls available to turn the reporting on or off, and to throttle reporting. == Major Issues == iana-igp-metric-types has identity bandwidth-metric { base metric-type; description "Bandwidth metric."; reference "draft-ietf-lsr-flex-algo-bw-con"; } There is a set of codepoints for "user-defined metrics" and "flexible algorithms". I don't think we want them to appear in the IANA registry (for example, as user-metric-128, user-metric-129, etc. I see leaf algo-number, but nothing equivalent for metrics. But I also don't understand how algo-number, metric-type, and calc-type are used. Surely algo-number and calc-type are complementary with only one of them having meaning at once? Don't you need a Bool to say whether to use algo-number or calc-type? (and similarly for metric-type or the new user-defined-metric type) Further, shouldn't algo-number have a range clause? Further, the description for calc-type is surely wrong. algo-type is a base identity not an integer. Lastly, in flex-algo-not-supported, the leaf algo-number is a uint8 which clashes with the use of the identity. --- When I see a Notification defined, I expect to see some way to turn on and off the generation of notifications, and ideally a way to control the rate of generation. --- What happens if more flags are added to the SABM registry? Shouldn't you be using an IANA-managed registry for that, too? == Minor Issues == While the tree diagrams are useful, it is a shame that there is no text describing the two OSPF models and how they are used. --- You have a number of imports from other modules. While your reference clauses are clear with their pointers to the relevant RFCs, those RFCs don't appear in the References section. What you need to do is either provide a table of imports (as is often done - for example, section 1.3 of RFC 9731), or some text above each module saying "This module imports from RFCwxyz". Then you can add the documents to the References section. I think they are all Normative references. I see, at least: RFC 8665 RFC 9843 (once you update from draft-ietf-lsr-flex-algo-bw-con) RFC 9587 RFC 8294 RFC 8776 draft-ietf-teas-yang-te (this is already Pub Req so a normative reference is safe) == Nits == All of the copyright statements appear to be ood for document posted this year. --- 1. OLD Four YANG modules defined in this document. NEW Four YANG modules are defined in this document. END --- 1. The third module augments the IETF OSPF YANG data model to support OSPF Flexible Algorithm, and the fourth module is to support OSPF Application-Specific Link Attributes. Not very important, but the third and fourth modules are presented in the other order. --- 4. and 5. In addition, both the OSPFv2 and OSPFv3 link-state databases are augmented to include the TLVs defined in[RFC9350]. Really? Does this document augment the link-state databases? Or does it augment the YANG modules used to model the lsdb (with reference to the document that defines those modules)? -----Original Message----- _______________________________________________ Lsr mailing list -- [email protected] To unsubscribe send an email to [email protected]
