[Last-Call] Re: Tsvart last call review of draft-ietf-pce-pcep-yang-26

["Scharf, Michael" <Michael.Scharf@xxxxxxxxxxxxxxx>]

Inline [ms]

 

From: Dhruv Dhody <dhruv.ietf@xxxxxxxxx>
Sent: Tuesday, November 12, 2024 6:18 PM
To: Scharf, Michael <Michael.Scharf@xxxxxxxxxxxxxxx>
Cc: tsv-art@xxxxxxxx; draft-ietf-pce-pcep-yang.all@xxxxxxxx; last-call@xxxxxxxx; pce@xxxxxxxx
Subject: Re: Tsvart last call review of draft-ietf-pce-pcep-yang-26

 

Hi Michael,

 

On Tue, Nov 12, 2024 at 10:00 PM Michael Scharf via Datatracker <noreply@xxxxxxxx> wrote:

Reviewer: Michael Scharf
Review result: Ready with Nits

This document has been reviewed as part of the transport area review team's
ongoing effort to review key IETF documents. These comments were written
primarily for the transport area directors, but are copied to the document's
authors and WG to allow them to address any issues raised and also to the IETF
discussion list for information.

When done at the time of IETF Last Call, the authors should consider this
review as part of the last-call comments they receive. Please always CC
tsv-art@xxxxxxxx if you reply to or forward this review.

This document specifies a YANG data model for PCEP as application protocol.
This document is well-written and I have not found any specific issues related
to transport protocols.

However, I'd like to note that it is quite hard to review in detail a complex
YANG data model that has apparently not been implemented and tested at all.

From a high-level point of view, there may be two nits:

1. It is not clear to me why Section 3.3 is needed.

 

Dhruv: It is a practice that is quite common as it helps track the references as they move from draft to RFCs. It also helps avoid the idnits warning one would get saying that the reference is not used as idnits does not consider the use in the YANG. 

 

[ms] That sounds reasonable as long as the document is an I-D. My comment was about whether Section 3.3 is really needed in the final RFC.

 

2. Some design choices for data types in the YANG model are not really obvious.
For instance, "init-back-off-time" is uint16, but "max-back-off-timer" is
uint32. There are also quite a number of timer limites defined only as unit8.
In some cases, the chosen value ranges seem to originate from PCEP standard,
but I haven't been able to track down some other ones (well, as a
non-PCEP-expert). Similarly, it is hard to know if "counter32" will be
sufficient for some stat values.

 

Dhruv: I understand how this could be non-obvious. You are right some of it is based on the standard such as keep alive and dead timer are 8 bits fields in the Open Object. Some are done to keep alignment with the choice made in the PCEP-MIB RFC 7420 which had Unsigned32 (1..65535) for all the uint16 leaves. For the stats, the YANG counter32 maps to MIB as well. It wraps around and thus it should be okay! Thank you for checking.

 

[ms] Regarding uint16: OK, thanks for that context. I now understand that design choice a bit better. Yet, to me, it still seems a bit strange to refer to the same timer with an uint16 and uint32 leaf. The solution in the MIB (i.e., uint32 with a range limit) looks quite reasonable to me. But, well, probably one can argue either way. Maybe that detail is somewhat a matter of taste, and I don’t ask for any change.

 

[ms] Regarding counter wrap: In a similar case (TCP MIB) the community feedback was that the YANG model does not have to be fully backward compatible to an old MIB. Potential counter wrap around was a reason to change some counters from counter32 to counter64. Of course, at the end of the day the PCEP experts have to make the call whether counter wrap around would be an operational issue, or not, and how relevant full type compatibility with the MIB is.

 

Michael

 

Thanks!
Dhruv

 

Michael

-- 
last-call mailing list -- last-call@xxxxxxxx
To unsubscribe send an email to last-call-leave@xxxxxxxx