Re: [Pce] Shepherd's review of draft-ietf-pce-stateful-sync-optimizations-03

["Zhangxian (Xian)" <[e-mail]>]

Hi, Jon,

Thank you for the detailed review. Please see my reply inline:

I will update/upload the draft once we reach agreement on all points, right after the upload re-opening.

Regards,
Xian

From: Jonathan Hardwick [mailto:[e-mail]]
Sent: 2015Äê10ÔÂ19ÈÕ 18:18
To: [e-mail]
Cc: [e-mail]
Subject: Shepherd's review of draft-ietf-pce-stateful-sync-optimizations-03

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 ¨C comments that affect the meaning of the document

*         editorial ¨C comments that do not affect the meaning but I think would significantly improve the clarity

*         nits ¨C 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.

[Xian]: Agreed. Will update as:
¡°If the PCC attempts to skip state synchronization, by setting the SYNC flag to 0 and PLSP-ID set to non-zero, as per [I-D.ietf-pce-stateful-pce],¡±

--

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.¡±
[Xian]: point taken, with s/open/OPEN twice.

--

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.¡±

[Xian]: point taken, with s/to/with.

--

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.

[Xian]: This text is wrong and it should refer to the F bit. In version 01, we used the same bit (F) for both functions (Section 5 and 6), but latter we decided to separate them. How about update it as below?
¡°In order to allow a stateful PCE to control the LSP-DB synchronization after establishing a PCEP session, both PCEP speakers MUST set F bit to 1 in the OPEN message.¡±


--

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.
[Xian]: Sure, will do.

One issue though: The STATEFUL-PCE-CAPABILITY TLV is defined in draft-ietf-pce-stateful-pce-12, thus no registry yet but being requested there. Am I ok to write the following text to provide a linkage?

¡°The STATEFUL-PCE-CAPABILITY TLV is defined in [draft-ietf-pce-stateful-pce-12] and a registry is requested to be created to manage the flags in the TLV.  IANA is requested to make the following allocation in the aforementioned registry.
¡­
¡±

--

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.)
[Xian]: Ok. Expand the Information and Data Models with the sentence provided to make it more general.

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.
[Xian]: point taken.

s4.1 final para feels superfluous ¨C suggest deleting.
[Xian]: ok.
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.).
[Xian]: Section 4 and 5 used to one. So when splitting, we also put emphasis on different aspects. Section 4 focuses on the need of delta sync. driven by small changes of LSP-DB. This probably has wider utility. Section 5 focuses on the constraints a particular network may have (optical as the main use case) so as to allow for PCE control of initial sync.  Of course, implementations on both help better address the optical network CC constraints. So I do not really want to move the whole sentence up, but to address your comment, I suggest to add something like (e.g., in-band control channel for optical transport networks) in Section 4.

s5.2 ¡°Support of PCE-triggered state synchronization is advertised¡± ¨C 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¡± ¨C please use active voice.

[Xian]: Point taken; will be more explicit in the next update.

Nits

s3.2 para 4: ¡°the PCE SHOULD ignore it were to receive one¡± ¨C I think you mean ¡°the PCE SHOULD ignore it were it to receive one¡±

[Xian]: accepted.