Hi Ines, Thanks for the review. See inline - I've fixed everything other than adding a new leaf for the virtual-ipv6-link-local-address. The comments are addressed in -16.
> On May 22, 2026, at 12:44 PM, Ines Robles via Datatracker <[email protected]> > wrote: > > Document: draft-ietf-rtgwg-vrrp-rfc8347bis > Title: A YANG Data Model for the Virtual Router Redundancy Protocol (VRRP) > Reviewer: Ines Robles > Review result: Almost Ready > > I am the assigned Gen-ART reviewer for this draft. The General Area > Review Team (Gen-ART) reviews all IETF documents being processed > by the IESG for the IETF Chair. Please treat these comments just > like any other last call comments. > > For more information, please see the FAQ at > > <https://wiki.ietf.org/en/group/gen/GenArtFAQ>. > > Document: draft-ietf-rtgwg-vrrp-rfc8347bis-15 > Reviewer: Ines Robles > Review Date: 2026-05-22 > IETF LC End Date: 2026-05-22 > IESG Telechat date: Not scheduled for a telechat > > Summary: > > This document describes a YANG data model for the Virtual Router Redundancy > Protocol (VRRP). Both versions 2 and 3 of VRRP are covered. The VRRP > terminology has been updated to conform to inclusive language guidelines for > IETF technologies. This document obsoletes RFC 8347. > > The document is well written, I have some comments/questions that it would be > nice to be addressed before publication. > > Comments/Questions: > > 1- Section 4: list virtual-ipv6-address > > 1.1- The IPv6 virtual address list is limited to "max-elements 2", but RFC > 9568 > describes the IPv6 address list as "one or more IPv6 addresses" and does not > appear to limit the list to two addresses. Is there a specific justification > for restricting the list to two elements? If so, it would be helpful to > mention > the rationale in the document. This is wrong. While it was this way in the original ietf-vrrp.yang, I'm going to correct it. > > 1.2- Also, the model currently relies on descriptive text stating that the > first address must be link-local. Based on my understanding, this constraint > is > not formally enforceable by the YANG schema itself. In particular, the list is > keyed by "ipv6-address" and does not declare "ordered-by user", so the concept > of a "first" list entry may not be reliable. Would it make sense to model the > required link-local address explicitly, or otherwise add a formal schema > constraint? I guess this was a Claude or ChatGTP comment... While the terminology changes are not backward compatible, I'd rather not add a new leaf for the first address to enforce that it is link-local as this is non-backward compatible configuration while the other changes were only operational state and notifications. The constraint cannot be enforced with a schema constraint since you can't match regular expressions in constants (limited to XPath 1.0 such as "startswith()") . . > > 2- Section 4: skew-time > > 2.1- The description of "skew-time" states that it is "Calculated based on the > priority and advertisement interval configuration command parameters. See RFC > 9568." However, it is not entirely clear whether this operational value is > intended to reflect only configured parameters or the dynamically learned > advertisement interval used by Backup routers as defined in RFC 9568. It would > be helpful to clarify this. Since this is the local router's advertisement skew time, it is based on the local router's configured priority and wait interval. The "configuration command parameters" states this explicitly. > > 2.2- Additionally, RFC 9568 expresses these timer calculations in > centiseconds, > while the YANG leaf uses microseconds. Some clarification on the unit > conversion and operational precision might also be helpful. I think microseconds is more operationally useful since the computation could result in fractions of a centisecond. I'll add a note though. > > 3- Section 4: log-state-change > The description of "log-state-change" refers to transitions from "up" to > "down" > or from "down" to "up". However, RFC 9568 defines VRRP states as Initialize, > Backup, and Active. It is not completely clear whether this leaf refers to > VRRP > state changes or interface state changes. Clarification here would be helpful. Good point. I've updated the description. > > 4-Section 4: up-datetime > The description of "up-datetime" refers to the "init" state, while the YANG > identity and RFC 9568 use the term "Initialize". Is the use of "init" > intentional here? Nope - I've fixed. > > 5- The IPv6-related text refers to "VRRPv2 or VRRPv3". My understanding is > that > IPv6 VRRP support is defined only for VRRPv3. Should the text be fixed to > reflect only VRRPv3 for IPv6? Good point. I've fixed. > > Nits: > > 6- RFC 9586 --> RFC 9568 Fixed. > > 7- this sentence is repeated in the introduction: The leaf "new-master-reason" > was changed to "new-active-reason". The associated descriptive text was also > updated. Fixed. > > 8- prempt --> preempt? Fixed. > > 9- was add to --> was added to Fixed. > > 10- The effect priority --> The effective priority Fixed. > > 11- Kindly check the JSON format in Appendix A. There appear to be several > syntax issues, for example a missing comma after the "preempt" object and > malformed array entries under "virtual-ipv6-address". Fixed. > > Thanks for this document, Thanks for reviewing. Acee > > Ines. > > _______________________________________________ Gen-art mailing list -- [email protected] To unsubscribe send an email to [email protected]
