Hi Chris, Currently there is a range statement when configuring the algo-number, but for the tlvs in the LSDB a uint8 is used without range so it can accommodate whatever value returned. This way, we can make sure to configure a correct algo-number, and in case something goes wrong in the database we can see it.
Are you suggesting we should remove the range for configuration? A wrong configuration may still be rejected by the router. I'll update all names to "algo-number" for consistency. Thanks, Yingzhen On Thu, Sep 18, 2025 at 4:55 AM Christian Hopps <[email protected]> wrote: > Michal Vasko <[email protected]> writes: > > > Hi Yingzhen, > > > > I do not understand why you want to keep the 2 nodes in > > `ietf-isis-link-attr` the way they are. Whether configuration or > > state data, by using statements such as `range` you can formally and > > exactly specify the allowed value space, why would you not want to > > use it? If you put this information in the `description`, it is > > understandable only by a human. Also, why not be consistent and use > > the `range` for one node but not the other. Finally, duplicating > > information is always redundant. > > > > In any case, I am only providing YANG-specific advice which you are > > essentially free not to follow but at least another opinion of a YANG > > doctor may be helpful. > > [as wg-member] > > As an aside, I'm not a big fan of range statements like these here since > they are often ripe for introducing backward incompatible changes when the > range is updated someday by another RFC. I like to, whenever possible, let > the current standard say the value (e.g., 128-255 here), and let the module > hold more values (the full uint8 which is given by the on-the-wire > encoding), then if the standard is ever updated the module continues to > work unchanged. Restricting values in the YANG modules should bring more > value to the table IMO. > > In any case, if these are updated please use the same leaf-name for all of > them since they contain the same semantic value. So, for example, name them > all `algo-number` since it's used that way under the `flex-algo` container, > but not `algo-number` `flex-algo-number`, and `flex-algo`. > > Thanks, > Chris. > > > > > > Regards, > > Michal > > > > On 18. 9. 2025 0:24, Yingzhen Qu wrote: > > > > Hi Michal, > > > > Thanks for the review. I've updated the JSON example in the > > appendix. However, I don't think those > > nits in ietf-isis-flex-alog need to be fixed. Details below > > inline. > > > > Thanks, > > Yingzhen > > > > On Wed, Sep 17, 2025 at 1:06 AM Michal Vasko <[email protected]> > > wrote: > > > > > > Hi Yingzhen, > > > > it seems the minor issues in `ietf-isis-link-attr` were fixed > > but the ones in `ietf-isis-flex-algo` were not: > > > > ietf-isis-flex-algo > > - leaf flex-algo - range in description instead of using the > > "range" statement > > > > [Yingzhen]: This is read only data. The description is only for > > information. > > > > > > - leaf algo-number - redundant range mentioned in the > > description > > > > [Yingzhen]: I don't consider this as a nit. > > > > > > Also, the XML data example now validates but the JSON one > > seems to be using syntax rules unknown to me and definitely > > not the ones in RFC 7951. I have attached a valid YANG data > > JSON example. > > > > [Yingzhen]: I've updated the JSON example. > > > > > > Regards, > > Michal > > > > On 12. 9. 2025 8:13, Yingzhen Qu wrote: > > > > Hi Michal, > > > > Thanks for the review. I have uploaded a new version to > > address your comments. > > > > I fixed all the editorial issues in your comments and > > updated the examples. Please let me know if you have any > > other questions or comments. > > > > Thanks, > > Yingzhen > > > > On Fri, Aug 29, 2025 at 12:17 AM Michal Vaško via > > Datatracker <[email protected]> wrote: > > > > Document: draft-ietf-lsr-isis-flex-algo-yang > > Title: YANG Model for IS-IS Application-Specific Link > > Attributes and Flexible > > Algorithm Reviewer: Michal Vaško Review result: Ready > > with Issues > > > > Looking at the 2 YANG modules, they are in a good > > shape and I found only a few > > nits (without having knowledge of the ISIS routing > > protocol). However, the XML > > and JSON example YANG data are invalid, which needs > > to be addressed. > > > > ietf-isis-link-attr > > - leaf udabm-length - 2nd line of description has > > extra space > > - leaf transition - 2nd line of description has extra > > space > > - leaf unidirectional-link-delay - units mentioned in > > description instead of > > "units" statement - uses > > application-specific-link-attributes-sub-tlv - second > > usage has extra indent > > > > ietf-isis-flex-algo > > - leaf flex-algo - range in description instead of > > using the "range" statement > > - leaf algo-number - redundant range mentioned in the > > description > > > > Appendix A invalid (yanglint used): > > - none of the XML prefixes of the used identities are > > defined > > - there are leafrefs to the ietf-te module data, > > which should ideally be > > included so that the examples can successfully be > > validated or at least the > > used ietf-te data referenced, if found in another RFC > > - "At least one > > area-address must be configured." > > (/ietf-routing:routing/control-plane-protocols/ > > control-plane-protocol[type='ietf-isis:isis'][name= > > 'default']/ietf-isis:isis) > > > > Appendix B invalid: > > - some of the prefixes used are not valid module > > names (iana-metric-type and > > iana-algo-types) - similar leafref problems - similar > > explicit validation error > > message > > > > > > >
_______________________________________________ Lsr mailing list -- [email protected] To unsubscribe send an email to [email protected]
