Hi there I have performed a WG Shepherd review of this draft. Generally I found it a well-written draft with no major issues. There are a few minor issues that I think should be resolved.
I have divided my comments into three groups: * technical - comments that affect the meaning of the document * editorial - comments that do not affect the meaning but I think would significantly improve the clarity * nits - editorial comments that are minor and obvious. I would like to have at least the technical comments resolved before forwarding the document to the IESG. Please could the authors review my comments and reply either by email or by submitting a new draft version? Best regards Jon Technical s3.2 para 6. "If the PCC attempts to skip state synchronization (i.e., the SYNC Flag = 0 on the first LSP State Report from the PCC as per [I-D.ietf-pce-stateful-pce])," What if the PCC is synchronizing an empty LSPDB? Then the first message it sends is LSP State Report with SYNC=0 and PLSP-ID=0. This is legitimate and yet the text says to treat it as an error. I think this paragraph should stipulate that the error condition is met only if SYNC=0 and PLSP-ID is not 0. -- s4.2: Please rewrite the following text using RFC 2119 keywords. If a PCC has to force full LSP DB synchronization due to reasons including but not limited: (1) local policy configured at the PCC; (2) no sufficient LSP state caches for incremental update, the PCC can set the D flag to 0. Note a PCC may have to bring down the current session and force a full LSP-DB synchronization with D flag set to 0 in the subsequent open message. I would simply say "The PCC MAY force a full LSP DB synchronization by setting the D flag to zero in the open message." And then later, when you say this: If a PCC finds out it does not have sufficient information to complete incremental synchronisation after advertising incremental LSP state synchronization capability, it MUST send a PCErr with Error-Type 20 and Error-Value 5 'A PCC indicates to a PCE that it can not complete the state synchronization' (defined in [I-D.ietf-pce-stateful-pce]) and terminate the session. add a sentence: "The PCC SHOULD re-establish the session with the D bit set to zero in the open message." -- s4.2 "So a PCC needs to remember the LSP state changes that happened when an existing PCEP session to a stateful PCE goes down in the hope of doing incremental synchronisation when the session is re-established." Even this might not be sufficient as the PCC has no idea how many messages the PCE managed to process before the session goes down. I think it would be more correct to say "A PCC needs to remember at least the LSP state changes that happened after an existing PCEP session to a stateful PCE goes down to have any chance of doing incremental synchronisation when the session is re-established." -- s5.2 Two concerns about the following text: A stateful PCE MAY choose to control the LSP-DB synchronization process. To allow PCE to do so, PCEP speakers MUST set T bit to 1 to indicate this (as described in Section 7). Why are you discussing the T bit here? I thought that the behaviours controlled by the F bit and the T bit were logically independent (although similar). Are you saying that the PCE must set the F bit and the PCC set the T bit for the procedures in section 5 to work? You have written a lot of the text in the passive voice, which makes it unclear who is supposed to set which bit (see editorial comments below for more specific pointers.) Also, I think your cross-reference should be to section 6, not 7. -- s8.2 Please rewrite this as a request to IANA to make an allocation from a specific registry (your text in 8.1 is a good example to copy.) s8.3 ditto. -- s9.2 The manageability section ought to describe requirements on implementations, not requirements on the PCE working group. I think your text ought to read "An implementation SHOULD allow the operator to view the stateful capabilities advertised by each peer, and the current synchronization status with each peer." (Note that I am not disagreeing with the idea of extending the PCEP MIB.) Editorial s3.2 para 10: "If state synchronization avoidance is enabled..." This would perhaps fit better alongside para 2, which discusses the other legitimate reasons to increment the DB-VERSION. s4.1 final para feels superfluous - suggest deleting. s5.1 I like the explanation of why PCC:PCE bandwidth might be limited and I suggest moving this explanation forwards to s4.1 (s4.1 tries to explain the same thing but is not as elegant.). s5.2 "Support of PCE-triggered state synchronization is advertised" - is advertised by whom? Also, I think you mean "initial state synchronization." Please rewrite in the active voice (for example, "Both PCEP speakers advertise their support of PCE-triggered initial state synchronization during session startup by...") Ditto "If the TRIGGERED-INITIAL-SYNC capability is not advertised" -> "If the PCE has not advertised the TRIGGERED-INITIAL-SYNC capability" s6.2 "Support of PCE-triggered state synchronization is advertised" - please use active voice. Nits s3.2 para 4: "the PCE SHOULD ignore it were to receive one" - I think you mean "the PCE SHOULD ignore it were it to receive one"
_______________________________________________ Pce mailing list [email protected] https://www.ietf.org/mailman/listinfo/pce
