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

["Adrian Farrel" <[e-mail]>]

Thanks Dhruv,

Looks like you have all points covered.
I will try to look again when you post the new version, but if you are to be
trusted ;-) then my concerns are cleared.

Adrian

> -----Original Message-----
> From: Dhruv Dhody [mailto:[e-mail]]
> Sent: 27 February 2017 14:24
> To: [e-mail]; [e-mail]
> Cc: [e-mail]; [e-mail]; [e-mail];
draft-ietf-pce-stateful-sync-
> [e-mail]
> Subject: RE: [Pce] TSV-ART review of
draft-ietf-pce-stateful-sync-optimizations
> 
> 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