[Pce] Re: Opsdir early review of draft-ietf-pce-stateful-pce-vendor-04

[[e-mail]]

OK, no problem. :-)
Looking forward to your new version and more discussion if needed.

Cheers,
Xiao Min


Original


From: SamuelSidor(ssidor) <[e-mail]>
To: 肖敏10093570;
Cc: [e-mail] <[e-mail]>;[e-mail] <[e-mail]>;[e-mail] <[e-mail]>;
Date: 2024年08月07日 20:40
Subject: RE: Opsdir early review of draft-ietf-pce-stateful-pce-vendor-04


Thanks a lot Xiao for review and comments.
 
We are discussing changes required to the draft with co-authors. We will get back to you soon.
 
Regards,
Samuel
 
-----Original Message-----
From: Xiao Min via Datatracker <[e-mail]>  
Sent: Tuesday, August 6, 2024 9:42 AM
To: [e-mail]
Cc: [e-mail]; [e-mail]
Subject: Opsdir early review of draft-ietf-pce-stateful-pce-vendor-04
 
Reviewer: Xiao Min
Review result: Has Issues
 
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.