Hi Francesca, Thanks for your review. Few thoughts... On Mon, Aug 26, 2019 at 5:44 PM Francesca Palombini via Datatracker <noreply@xxxxxxxx> wrote: > > Reviewer: Francesca Palombini > Review result: Ready with Issues > > 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://trac.ietf.org/trac/gen/wiki/GenArtfaq>. > > Document: draft-ietf-pce-lsp-control-request-08 > Reviewer: Francesca Palombini > Review Date: 2019-08-26 > IETF LC End Date: 2019-08-28 > IESG Telechat date: Not scheduled for a telechat > > Summary: This draft is almost ready for publication, but has minor issues/open > points that should be fixed before publication. > > Major issues: N/A > > Minor issues / questions: > > * Section 3: At the end of season 3, you indicate that this new flag has no > meaning in PCRpt and PCInitiate messages. RFC8231 defines that the SRP Object > MAY be carried in PCErr as well, shouldn't there be such requirements (MUST be > set to 0, MUST be ignored on reception) for PCErr? > I agree. I suggest to make this generic, something like - "The C Flag has no meaning in other PCEP messages that carry SRP object and the flag MUST be set to 0 on transmission and MUST be ignored on receipt." > * In the following text (Section 4): "The PCE SHOULD NOT > send control request for LSP which is already delegated to the PCE, > i.e. if the D Flag is set in the PCUpd message, then C Flag SHOULD > NOT be set." Why is there a SHOULD NOT instead of MUST NOT? In which > situation could it be acceptable or useful to request control for a > delegated LSP? > It wont be useful, but if received it would be silently ignored. It does not rise up to a high level of error and I suspect that is why authors used SHOULD. > Nits/editorial comments: > Thanks for these, just one comment ... > * Terminology should also include a sentence about the reader being familiar > with at least RFC8231. > > * Terminology could also include what SRP stand for. > > * Section 3. When introducing SRP, it would have been helpful to the reader to > reference section 7.2 of RFC8231. > > * Section 3. "PCE sets the C Flag to 1 to indicate that, it wishes" -- remove > "," > > * Section 3. "MUST be ignored on receipt" -- "MUST be ignored on reception" > I have noticed 'on receipt' in many of our documents. We should leave this one for the RFC-EDITOR maybe... > * Section 4. When introducing the D flag, it would have been helpful to the > reader to reference section 7.3 of RFC8231. > > * Section 4. "Note that, the PCUpd message with C Flag set is received" -- > "Note that, if the PCUpd message with C Flag set is received" > > (Please keep my address in the To: field if you want to make sure I see any > response to this thread) > > Thanks, > Francesca > Thanks again for your review. Regards, Dhruv