Skip to main content

Early Review of draft-ietf-pce-stateful-pce-vendor-04
review-ietf-pce-stateful-pce-vendor-04-opsdir-early-min-2024-08-06-00

Request Review of draft-ietf-pce-stateful-pce-vendor
Requested revision No specific revision (document currently at 08)
Type Early Review
Team Ops Directorate (opsdir)
Deadline 2024-08-18
Requested 2024-07-23
Requested by Dhruv Dhody
Authors Cheng Li , Haomian Zheng , Siva Sivabalan , Samuel Sidor , Zafar Ali
I-D last updated 2024-08-06
Completed reviews Opsdir Early review of -04 by Xiao Min (diff)
Rtgdir Early review of -05 by Mike McBride (diff)
Opsdir Last Call review of -08 by Xiao Min
Assignment Reviewer Xiao Min
State Completed
Request Early review on draft-ietf-pce-stateful-pce-vendor by Ops Directorate Assigned
Posted at https://mailarchive.ietf.org/arch/msg/ops-dir/GMcK_tk5rTHqWJiF2IwnM9A1i-A
Reviewed revision 04 (document currently at 08)
Result Has issues
Completed 2024-08-06
review-ietf-pce-stateful-pce-vendor-04-opsdir-early-min-2024-08-06-00
Summary: I've reviewed this document and I believe this document is on the
right track. I have no major concern but several minor ones. Besides, there are
a number of nits and ungrammatical sentences, I'm also not good at this, so
just to name a few.

Major issues: None.

Minor issues: As below.
Section 2, it says "Different instances of the object can have different
Enterprise Numbers". I believe a normative language is more suitable than
*can*, MUST or MAY? It's supposed to be MAY. Section 3, my first feeling is
that this section should list all Stateful PCEP objects in which the Vendor
Information TLV may be contained, however after checking Section 3 of RFC 7470,
I found it says "Further specifications are needed to define the position and
meaning of the Vendor Information TLV for specific PCEP objects". Then I think
this section should either define the Vendor Information TLV for each Stateful
PCEP object or state something like what's said in Section 3 of RFC 7470.
Section 4.2, it says "Any standard YANG module will not include details of
vendor-specific information", and then it provides  a suggestion on how the
standard YANG module MAY be extended. I assume the mentioned extension applies
only to a proprietary YANG module, if that's the case, then I don't see much
value to mention the extension. Section 4.6, compared to what's said in Section
6.6 of RFC 7470, it seems what's said here is a little bit too simple.
Considering that multiple Vendor Information Objects/TLVs of multiple LSPs can
be carried in the Stateful PCEP messages, it can be imagined that in some cases
the amount of Vendor Information would become too huge to be processed by the
receiver timely. In other words, some kind of congestion may happen due to the
added Vendor Information. So it's helpful to the reader/implementer if some
mitigation method can be provided here.

Nits/editorial comments: As below.
Abstract Section, s/may then be/may be then.
Section 1, s/(LSP-DB)/(LSP-DB)); s/added new messages in PCEP/add new messages
to PCEP; s/[RFC7470] defined/[RFC7470] defines; s/It also defined/It also
defines; s/to also include/to include. Section 2, s/be used on a single PCRpt
message/be contained in a single PCRpt message. Section 3, SRP needs expansion
in first use; s/All the procedures as per/All the procedures are as per;
s/defines the Enterprise Numbers are allocated by IANA/defines the Enterprise
Numbers allocated by IANA; s/clarifies that the IANA registry described
is/clarifies that what the IANA registry describes is. Section 4.4, s/Verify
Correct Operations/Verifying Correct Operations. Section 7, s/PCEP also
support/PCEP also supports.