Re: [Pce] TSV-ART review of draft-ietf-pce-stateful-sync-optimizations

[Dhruv Dhody <[e-mail]>]

Hi Adrian, 

Thanks for your reply. See inline. 

> -----Original Message-----
> From: Adrian Farrel [mailto:[e-mail]]
> Sent: 26 February 2017 23:01
> To: Dhruv Dhody <[e-mail]>; [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail];
> [e-mail]
> Subject: RE: [Pce] TSV-ART review of
> draft-ietf-pce-stateful-sync-optimizations
> 
> Thanks Dhruv, speedy as usual!
> 
> > > == Major issues
> > >
> > > I found it quite hard to work out the objects and flags to include
> > > on an Open. The problem (for me) is that there are three levels of
> > > function a. no support for this I-D b. support for synchronisation
> > > avoidance if DBvs match with full resynch otherwise c. support for
> > > synchronisation avoidance if DBvs match with partial
> > >    resynch otherwise
> > > and three indicators:
> > > - S flag on Open
> > > - D flag on Open
> > > - presence of LSP-DB-VERSION TLV on Open
> > >
> > > S flag seems to serve two purposes:
> > > - indicates that LSP-DB-VERSION TLV must be used on PCRpt messages
> > > - indicates that resynch avoidance can be attempted
> > >
> > > D flag seems to mean:
> > > - Partial (incremental) synchronisation can be attempted
> > >
> > > Presence of LSP-DB-VERSION TLV on Open seems to indicate:
> > > - value of DBv to compare to see if resynch can be avoided
> > > - value of DBv to compare to perform partial resynch
> > >
> > > I can't find a description of the behavior at startup with no saved
> DB.
> > > I think it is to omit the LSP-DB-VERSION TLV but still set the S flag.
> > > Is the only reason you need the S flag that you don't support the
> > > value of zero for a DBv? That is, you could use the presence of the
> > > LSP-DB-VERSION TLV as indication that synchronisation avoidance is
> > > supported if you had a value you could transmit to indicate "I have
> no DB".
> > >
> > > I am missing a statement in Section 3 that makes it clear that in
> > > the absence of the LSP-SYNC-CAPABILITY (D bit) from section 4, all
> > > synchs are either NOP or full.
> > >
> > > Finally, it is not clear (to me) why the PCC needs to include an
> > > LSP-DB- VERSION TLV in its Open message. I can see that it helps the
> > > PCE to predict what will happen next, but it isn't actually a
> > > necessary part of the protocol AFAICS.
> >
> > [[Dhruv Dhody]] This document resulted from merging of sync avoidance
> > from the stateful-pce draft and others from an individual draft. The
> > idea was to
> keep
> > all these procedures together (in one document), but separate so that
> > an implementation/deployment is free to pick and choose the various
> > optimizations based on the need. The structure of the document reflect
> > that, it does have
> the
> > side effect as you noted.
> >
> > Let me try to state the purpose of each flag and the TLV in open
> > message -
> >
> > - LSP-DB-VERSION TLV: The last LSP State Database Version Number on
> > this session (useful only for re-establishment of PCEP session, ex
> > session flap)
> 
> Presumably, "last sent by PCC" is in Open from PCC, and "last seen by PCE"
> is in Open from PCE.
> Only the value in the Open from the PCE actually causes any action.
> Ah no, I see the case that when the two value match, there will be no Sync
> so the PCE does not have to wait for SYNC = 0.
> You could make that explicit!
> 

[Dhruv] Done! 

> > - S (INCLUDE-DB-VERSION):  indicates that LSP-DB-VERSION TLV must be
> > used on the *to be sent* PCRpt messages on this session (useful in
> > future re- establishment when current session goes down)
> > - D (DELTA-LSP-SYNC-CAPABILITY): Allow incremental state sync during
> > the state synchronization phase *now* (useful during re-establishment)
> > - T (TRIGGERED-RESYNC): Allowed to trigger re-sync by PCE in *future*
> > on this session
> > - F (TRIGGERED-INITIAL-SYNC): Wait for PCE to trigger state sync
> > procedure
> > *now*
> >
> > From your comments I get that, the S flag was the reason for confusion.
> > The S flag does not play any role during the current state sync, it
> facilitates future
> > state sync. This is shown in figure 3, where even though the S flag is
> > unset
> but
> > state synchronization is skipped anyway.
> 
> Yes, I think that was the confusion.
> Also just a little confusion around start of day when the PCE has no record
> of a DBv and so must send an Open without a LSP-DB-VERSION TLV rather than
> one with value zero.
> 

[Dhruv] This is stated as - 

   If a PCEP speaker's LSP state database did not survive the
   restart of a PCEP session or at startup when the database is empty,
   the PCEP speaker MUST NOT include the LSP-DB-VERSION TLV in the OPEN
   object.

> > I agree that the text in section 3.2 does not make this fact clear, I
> > have
> updated
> > that in the working copy.
> 
> Thanks.
> 
> > To further clarify, the presence of LSP-DB-VERSION TLV indicate that
> > this is a
> case
> > of re-establishment of PCEP session and the peer remembers the LSP-state.
> > If the version match, state sync can be skipped.
> > If version don't match and D flag is set, we do Incremental sync;
> > otherwise
> full
> > sync.
> >
> > Please have a look at the working copy and if further changes are needed.
> 
> Erm, where's that then?


[Dhruv] It was(/is) included in the email.
Also at - https://github.com/dhruvdhody-huawei/ietf/raw/master/draft-ietf-pce-stateful-sync-optimizations-09.txt 

> 
> > > ---
> > >
> > > The function introduced for 3.3.2 has far reaching implications for
> > > a PCE system. I wonder whether the WG noticed it and considered
> > > those implications.
> > >
> > > Up until now a PCEP session has been known in the context of the
> > > PCE/PCC address pair. You are changing this by introducing a
> > > SPEAKER-ENTITY-ID that represents the PCEP speaker for all time.
> > > You are not saying anything about how the assignment of this value
> > > is made consistent across the domain (indeed, you seem to offer a
> > > range of implementation choices that could clash), or how uniqueness
> > > might be policed. Furthermore, you only give a "SHOULD" as the
> > > mechanism to identify interdomain PCEs, and that doesn't seem to
> > > provide a reliable mechanism (actually, even the mechanism for this
> "SHOULD" is a bit vague).
> > >
> > > You are saying that one of the PCE/PCC can change its address and
> > > re- assume a PCEP session without any further impact.
> > >
> > > Now, if the PCC changes its address there will be "interesting"
> > > consequences for the LSPs for which it is the head end. Renumbering
> > > events under the feet of live TE-LSPs are not well examined, but it
> > > looks like the LSPs might cease to be.
> > >
> > > So, perhaps you are looking at what happens when a PCE changes its address.
> > > This *might* be a more likely event if the PCE is virtualised and
> > > moves around in a DC.
> > >
> > > But, how does a PCC/PCE pairing get discovered and authorised? If
> > > you are relying on discovery through the routing protocol, then you
> > > will need to add the SPEAKER-ENTITY-ID to the advertisement or else
> > > it is hit or miss for the PCC to find the right PCE. If you are
> > > relying on configuration, then I suspect that the SPEAKER-ENTITY-ID
> > > is not needed because the configuration actions can achieve the right
> effect.
> > >
> > > Perhaps the use case you have in mind is when:
> > > - PCEP sessions are initiated by the PCE
> > > - The PCC is configured to accept any PCE
> > > - The PCE might wander taking its DB with it This has a some
> > > utility, but a lot of security concerns and error cases.
> > > What if a new PCE comes alive using a SPEAKER-ENTITY-ID that is
> > > already in use? What if a bogus PCE claims to have the same
> > > SPEAKER-ENTITY-ID as a PCE that has just failed? (And, of course,
> > > what stops a bogus PCE from randomly latching on to PCCs in the network).
> > >
> > > Maybe your plan is that the PCC will be configured with acceptable
> > > SPEAKER-ENTITY-IDs?
> > >
> > > You might also have had some ideas about PCE redundancy here. For
> > > example, a backup PCE with access to the DB used by the primary PCE
> > > could want to step in and take over seamlessly. In this case, of
> > > course, you would have two PCEs with the same SPEAKER-ENTITY-ID.
> > >
> > > In short, I don't think you have described this use case and its
> > > consequences, and you are trying to optimise a terribly small corner
> case.
> > >
> > > (Actually, this would all have been a lot easier if I had known
> > > about
> > > D=0 all the way through section 3, but that is just a comment about
> > > how hard the document is to parse for a slow learner like me.)
> >
> > [[Dhruv Dhody]] The origin of this idea is from PREDUNDANCY-GROUP-ID
> > TLV in
> >
> https://tools.ietf.org/html/draft-ietf-pce-stateful-pce-03#section-7.1
> > .3 When we moved this to this draft, we made it generic as
> > SPEAKER-ENTITY-ID to allow two cases -
> >
> > - In case of PCC, TCP client can choose any IP address to connect to the
> PCE.
> And
> > in case a physical interface IP address is used and for example
> > because of
> line
> > card change, the PCEP session might go down and come back up again but
> > with a different IP address used by the TCP client. We wanted a way to
> > identify the same speaker even if this IP address has changed.
> > - PCE redundancy as you describe.
> >
> > The key use-case for us is to allow us to skip state sync if only IP
> > address
> has
> > changed and limited to within the State Timeout Interval, till we have
> > the
> state
> > data to associate with the same session.
> >
> > I have added a separate section in the working copy to drive this point.
> >
> > An error-code is added when there is a clash of this ID.
> >
> > Please have a look at the working copy and see if further details
> > should be
> added.
> 
> OK. The first use case is important, and the error code will help.
> I am still unsure about the wider impact.
> 
[Dhruv]: Will recheck with our shepherd/AD on this point. 


> [snip]
> 
> > > 4.2
> > >    It is not necessary for a PCC to store a complete history of LSP
> > >    Database change, but rather remember the LSP state changes (including
> > >    LSP modification, setup and deletion) that happened between the PCEP
> > >    session(s) restart in order to carry out incremental state
> > >    synchronization.
> > >
> > > Nice, but...
> > > Consider the PCRpt messages that were in flight when the PCE or the
> > > session went down.  Did they get included in the PCE's DB? That
> > > doesn't matter because the PCE will announce it via the DBv in the
> > > new Open. But does the PCC need to have remembered them? That
> > > depends on whether it thinks they arrived at the PCE and were
> > > processed. A situation made worse by the inclusion of multiple LSPs
> > > on one PCRpt message which could give rise to some being added to the
> DB and some lost.
> > >
> > > Similarly...
> > >
> > >    After the synchronization procedure finishes, the
> > >    PCC can dump this history information.
> > >
> > > How does the PCC know that this state has made it into the PCE's DB?
> > >
> > > Maybe you intend to get around this with...
> > >
> > >    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.  The PCC
> > >    SHOULD re-establish the session with the D bit set to 0 in the OPEN
> > >    message.
> > >
> > > This seems extreme, since the PCC could just do full resynch anyway.
> > >
> >
> > [[Dhruv Dhody]] You are right, we do not have an explicit ack from
> > PCE, and
> thus
> > we have to find a way around.
> > Each LSP at the PCC stores the DBv at the time this LSP was reported.
> > So based
> on
> > the PCE's DBv we could find the LSP that needs to be reported. This
> > works fine for new LSP, LSP with state change etc. The problem would
> > be when the LSP are explicitly deleted at the PCC and PCC would like
> > to delete it from its
> database. In
> > this case we need to store this information at PCC so that we could
> > report
> this
> > deleted LSP as part of incremental state synchronization. We kept
> > things
> generic
> > in the document, thinking this would be implementation details.
> 
> I agree with all this, but I don't think you have addressed for me how the
> PCC knows what information it has to retain.
> The text says it only has to retain state changes that happened while the
> session was down.
> I am saying it also has to retain state changes that might not have made
> it into the PCE's DB before the session went down or the PCE restarted.


[Dhruv] The text now says - 

   In order to carry out incremental state synchronization, it is not
   necessary for a PCC to store a complete history of LSP Database
   change for all time, but remember the LSP state changes (including
   LSP modification, setup and deletion), that the PCE did not get to
   process during the session down.  Note that, a PCC would be unaware
   that a particular LSP report has been processed by the PCE before the
   session to PCE went down.  So a PCC implementation MAY choose to
   store the LSP State Database Version Number with each LSP at the time
   its status changed, so that when a session is re-established and
   incremental synchronization could be attempted based on the PCE's
   last LSP State Database Version Number.  For LSP that is deleted at
   the PCC, the PCC implementation would need to remember the deleted
   LSP in some way to make sure this could be reported as part of
   incremental synchronization later.  The PCC would discard this
   information based on a local policy, or when it determines that this
   information is no longer needed with sufficient confidence.  

> 
> I agree that you have closed this window by saying "in the event that the
> information is not available to the PCC, do a full synch."
> It just doesn't seem very graceful, nor in the spirit of synch avoidance.
> 
> > Regarding the use of PCE triggered re-sync instead of session
> > termination, we were worried that technically this is not
> > re-synchronization, this is the
> first sync of
> > the session and considered it wise to avoid it here.
> 
> But still, why tear down the session and start again instead of just getting
> on with the synch?
> 

[Dhruv] It is only the PCC that knows that it can't do incremental, and the PCE is expecting incremental sync. 
In this case if the PCC attempt full sync, for the new and existing LSPs there won't be a problem  but for the LSP that were deleted at PCC would not be updated at the PCE, because the PCE in case of incremental did not do stale marking and purge unreported LSP at the end. 
Thus we could not find a way around it. 

> BTW, I think the D bit MUST be 0 on the new session or the wold will end
> in an infinite loop of session thrash.
[Dhruv] This is already in the draft as - 

   The PCC
   SHOULD re-establish the session with the D bit set to 0 in the OPEN
   message.
> 
> > > ---
> > >
> > > In 5.2
> > >
> > > Why do you need the 'Attempt to trigger synchronization when the
> > > TRIGGERED-SYNC capability has not been advertised' error code?
> > > Surely it is enough that the PCE will have received F=0. Recall that
> > > the Open messages can overlap. Tearing down the session just to
> > > change the setting of the F bit is unnecessary.
> > >
> > > Then
> > >
> > >    The PCC SHOULD NOT send PCRpt messages to the stateful PCE before
> it
> > >    triggers the State Synchronization.
> > >
> > > Please tell us about the circumstances under which it MAY send PCRpt
> > > and how, given the PCE has to be able to handle those, it is helpful
> > > to have a "SHOULD NOT" here.
> > >
> > > And to be clear, you are saying that all updates to the DB,
> > > including all new delegations must be deferred and held as though
> > > the PCE was not in the session.
> > >
> > > But what happens if the PCE sends a PCUpd? Does the PCC decline to respond?
> > > Or does it respond but leave out the LSP-DB-VERSION? Or do we apply
> > > section 5.6 of draft-ietf-pce-stateful-pce
> > >    A PCE SHOULD NOT send PCUpd messages to a PCC before State
> > >    Synchronization is complete.  A PCC SHOULD NOT send PCReq messages
> to
> > >    a PCE before State Synchronization is complete.  This is to allow
> the
> > >    PCE to get the best possible view of the network before it starts
> > >    computing new paths.
> > > In which case I wonder why the PCE even opened the session since
> > > nothing can be done on the session until the PCE is ready to synch.
> >
> > [[Dhruv Dhody]] The error code "Attempt to trigger..." is not in
> > response to
> Open
> > message, it is in response to receiving an PCUpd message with SYNC
> > flag set when the capability in the open message is not advertised and
> > agreed upon, PCC does not close the session in this case.
> >
> > I have further updated this section and added an explicit error-code.
> 
> OK. And sorry for my bad reading.
> 
> I think you still have the converse of the "SHOULD NOT" to address.
> 
[Dhruv] The text was updated to - 

   If the
   TRIGGERED-INITIAL-SYNC capability is advertised by a PCE and the PCC,
   the PCC MUST NOT trigger state synchronization on its own.  If the
   PCE receives a PCRpt message before the PCE has triggered the state
   synchronization, the PCE MUST send a PCErr with Error-Type 20 and
   Error-Value TBD3 (suggested value - 3) 'Attempt to trigger
   synchronization before PCE trigger' (see Section 8.1).

> [snip]
> 
> > > 6.2
> > >
> > >    The PCUpd message MUST include an empty ERO as its
> > >    intended path
> > >
> > > I guess you have this because draft-ietf-pce-stateful-pce says an
> > > ERO is mandatory in a PCUpd. That's a bit heavy, and you could
> > > choose to say the ERO is ignored on a PCUpd if the SYNC flag is set.
> > > But anyway can you clarify that an empty ERO is:
> > > 1. sent with object length 4
> > > 2. the empty ERO is not an indication to the PCC that it can perform
> > >    local path computation and resignal the LSP according to its own
> > >    whim (i.e., even the empty ERO has to be ignored)
> >
> > [[Dhruv Dhody]] Ack. This should be added in stateful pce draft too
> 
> Oh, well, if you're going to update that draft as well, why not make it
> optional?
> 
> Anyway, please make sure the right people are triggered to make the right
> updates to the other draft!
> 
[Dhruv] They are in the cc, will do my part and remind them. 

Regards,
Dhruv

> [snip]
> 
> Cheers,
> Adrian