Hi Adrian,
> On Mar 25, 2026, at 11:24 AM, Adrian Farrel <[email protected]> wrote: > > 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. I'm not sure where you've been over the last ten years, but the IETF YANG models have not been widely implemented. In LSR, we have chosen to publish them without requiring implementation. Luckily, vendors and others have chosen to copy parts of them (although often without acknowledgment). There are some IGP YANG model in implementations in the holo open source project - https://github.com/holo-routing/holo We can check if they've done anything with flex algo. > > == 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 don’t see anywhere in the model where it implies that we are going to define Identities for the user metric. It is simply a range unavailable for assignment of new metric. > > 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) After a conversation with the RFC 9350 authors, I confirmed again that flex-algo and calc-type are independent. While it is unfortunate that both are drawn from the same IANA registry, this isn't something that we're going to fix with the YANG model. > > Further, shouldn't algo-number have a range clause? It does have a range for the configuration. container ospf-flex-algo { list flex-algo { key "algo-number"; leaf algo-number { type uint8 { range "128..255"; } description "An identifier associated with the Flexible Algorithm Definition."; } One normally wouldn’t have a range on the read-only leaves in OSPF TLVs. > > Further, the description for calc-type is surely wrong. algo-type is a base > identity not an integer. calc-type and algo-type are independent. Having said that, this is good catch that the wrong base identity is used for calc-type. We will fix. > > Lastly, in flex-algo-not-supported, the leaf algo-number is a uint8 which > clashes with the use of the identity. There are no identities for flex algo-number - only the range 128-255. > > --- > > 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. You expect to see this done per unique notification? Please point me to a YANG model where this is done? A better solution would to define a generic YANG group for referencing and controling notifications so this is done uniformly across notifications in IETF YANG models. > > --- > > What happens if more flags are added to the SABM registry? Shouldn't you be > using an IANA-managed registry for that, too? Then the draft the adds the flag(s) should add the IANA registry. This is clearly out of scope for these YANG models. > > == 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. We do have descriptions of the modules. For example: This document defines a YANG module for OSPF Flexible Algorithm as defined in [RFC9350]. It is an augmentation of the OSPF base model. The module augments OSPF router configuration with support flexible algorithms. In addition, both the OSPFv2 and OSPFv3 link-state databases are augmented to include the TLVs defined in[RFC9350] What more would you expect? I think it is more important to have ccturate "description" statements in the YANG model than redundant repletion of the contents. > > --- > > 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) We''ll add any that are missed. We have a way to do that, e.g., https://datatracker.ietf.org/doc/html/rfc9129#name-ospf-yang-module. > > == Nits == > > All of the copyright statements appear to be ood for document posted this > year. We'll fix, > > --- > > 1. > OLD > Four YANG modules defined in this document. > NEW > Four YANG modules are defined in this document. > END Sure. > > --- > > 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. Will fix. > > --- > > 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)? I'll reword. Thanks, Acee > > -----Original Message----- >
_______________________________________________ Lsr mailing list -- [email protected] To unsubscribe send an email to [email protected]
