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

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

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!

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

> 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?

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

> > === Minor issues
> >
> > The phrase "reliable state synchronization mechanism" is interesting as it
> > is not used in draft-ietf-pce-stateful-pce-18.txt where synchronization is
> > described only in terms of "checkpointing state".
> > Is any implication to be drawn from this use of "reliable"?
> >
> [[Dhruv Dhody]] Removed "reliable".

:-)

[snip]

> > It would help IANA and the RFC editor, and avoid any issues if you used
> > different TBD flags (such as TBD1, TBD2,...) for the different cases of
> > code point assignments to be made.
> >
> [[Dhruv Dhody]] Ack.
> 
> > Why do you feel the need to recommend values for assignment in the IANA
> > considerations section?
> >
> [[Dhruv Dhody]] There are multiple Stateful PCE documents (some with early
> code point allocation), we wanted to make sure the alignment between those
> assignments and existing implementation. The IANA review has noted these
> suggestions.

Hmmm. Mutter, mutter, mutter.

[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.

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?

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.

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

[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!

[snip]

Cheers,
Adrian