Hi Ina,
Following our fruitful discussing in Yokohama, please find [JM]'s note
below. Just a few items are pending, I hope you will soon find an
agreement with your co-authors.
WG, if you disagree with the proposed change about discovery bit
allocation below, please let us know.
Julien
Oct. 26, 2015 - inami...@google.com:
Julien,
Thank you for the detailed review, please find answers inline below
###. I have incorporated the overwhelming majority of the comments,
explained the reason for not incorporating a couple of them, and am
still working with the co-authors on a couple of items marked
"pending", which we will close on shortly.
Two questions and one ask
1. Forward references to SRP object and SRP-ID - there are several in
the comments, though the relevant section is always mentioned. How
should such forward references be addressed?
[JM] A simple way is to add the acronym and its expansion into the
terminology section, and optionally the forward reference.
2. Section 7 - s/defined in this document/defined in that
(aforementioned) document/
The comment was not clear to me. The intention is for the flags to be
set as explained for the new objects we are defining here, can you
clarify the comment?
[JM] Just suggesting a rewording, but my proposal was not clear either;
"this current I-D" may be enough to clear the ambiguity.
3. Can you please review the comments that were not incorporated and
let us know if you agree?
[JM] See below.
Thank you,
Ina
On Fri, Oct 16, 2015 at 7:40 AM, Julien Meuric
<julien.meu...@orange.com> wrote:
<snip>
- s/Active Stateful PCE: is an extension/Active Stateful PCE: an
extension/
### Replaced as "an Active Stateful PCE that may issue
recommendations...
[JM] Guessing you actually meant "a Stateful PCE that may...", it is OK.
<snip>
- OLD
Redelegation Timeout Interval: when a PCEP session is terminated,
a PCC waits for this time period before revoking LSP delegation to
a PCE and attempting to redelegate LSPs associated with the
terminated PCEP session to an alternate PCE.
NEW
Redelegation Timeout Interval: the period of time a PCE waits for,
when a PCEP session is terminated, before revoking LSP delegation
to a PCE and attempting to redelegate LSPs associated with the
terminated PCEP session to an alternate PCE.
### Done
- OLD
State Timeout Interval: when a PCEP session is terminated, a PCC
waits for this time period before flushing LSP state associated
with that PCEP session and reverting to operator-defined default
parameters or behaviors.
NEW
State Timeout Interval: the period of time a PCE waits for, when a
PCEP session is terminated, before flushing LSP state associated
with that PCEP session and reverting to operator-defined default
parameters or behaviors.
### Done
[JM] I hope you caught my (double) typo: I was only trying to rephrase,
the waiting party remaining the PCC.
<snip>
- Section 3.1.3 includes 2 paragraphs "Scale and Performance":
they should either be merged, or the old text version should be
dropped.
### Done, new text below:
o Scale & Performance: configuration operations often have
transactional semantics which are typically heavyweight and often
require processing of additional configuration portions beyond the
state being directly acted upon, with corresponding cost in CPU
cycles, negatively impacting both PCC stability LSP update rate
capacity.
[JM] Looks fine to me.
<snip>
- The reference to draft-sivabalan-pce-disco-stateful makes the
reader wonder why is is not part of draft-ietf-pce-stateful-pce
itself. Besides, Table 1 also mentions IS-IS and OSPF
PCE-CAP-FLAGS sub-TLV, only the allocation section seems to be
missing here. Would you talk to the authors (some being common to
both I-Ds) of the former to consider using their material as a
contribution? A separate document is quite an overhead for
allocating 2 bits.
### I don't think discovery should be part of the base draft. Several
reasons: a) 5440 does not include discovery, b) the base spec is
already quite large, we want to keep only the core function and c)
the discovery draft is expired. I cleaned up table 1 and added the
following text instead of the reference to the draft:
Old text
[I-D.sivabalan-pce-disco-stateful] defines the extensions needed
tosupport autodiscovery of stateful PCEs when using OSPF ([RFC5088])
or IS-IS ([RFC5089]) for PCE discovery.
New text
Similarly to [RFC5440], no assumption is made about the discovery
method used by a PCC to discover a set of PCEs (e.g., via static
configuration or dynamic discovery) and on the algorithm used to
select a PCE. Extensions needed to support autodiscovery will be
defined in a separate document.
[JM] Along with RFC 5557 and 6006, including discovery bit allocation
with PCEP extensions in a single document would save much administrative
processing.
<snip>
- s/include an empty ERO/include an empty RRO/ [Along with RFC
5440 (section 7.10), the object sent by a PCC to report to a PCE
is an RRO: let us keep it consistent.]
### XXX Pending
[JM] As discussed, clarifying that you encode the "intended path" using
ERO (class 7) and "actual path" using RRO (class 8) will address my
comments related to xROs.
<snip>
- Avoiding "positive acknowledgements for properly received
synchronization messages" has scalability benefits in normal
situations, but the PCC is blind and may keep on sending PCRpt to
dead processes behind up PCEP sessions. Have you consider
acknowledgement, possibly using a compression mechanism like the
one defined later in the I-D?
### XXX Pending
- When mentioning errors, adding a sentence reminding that RFC
5440 already defines a set of applicable error codes would be
valuable.
### XXX Pending
<snip>
- In section 5.5.1, it is not clear if an empty LSP Update Request
with a Delegate flag to 1 is an acceptable way for a PCE to send a
delegation acknowledgement: to be clarified.
### XXX Pending
<snip>
- s/SHOULD return the LSP delegation/MUST return the LSP delegation/
### This should remain SHOULD. The nice way to do it is to return it
explicitly, but it may choose to wait until the next update and return
the delegation then by not setting the delegate flag.
[JM] As discussed together, I can live with that.
- In section 5.5.3, assuming an LSP was delegated, does the
reception by the PCC of a non empty LSP Update Request with a
Delegate Flag to 0 trigger an error?
### XXX Pending
<snip>
- At the top of page 18, "and [assuming that] PCEs' decisions are
the same" should be added.
### The comment is not clear - I am assuming you refer to the text
"assuming that the state timeout... ' on page 17, changed to: " and
assuming that the primary and redundant PCEs take similar decisions, this"
[JM] You got my point. :-)
<snip>
- In section 5.6.2, in case of new LSP, the very first message
sent by the PCC aims at getting a path. This is clearly the role
of a PCReq message, and the I-D extends it to support the LSP
object including the delegation flag (section 7.3). However,
Figure 8 treats a new LSP the same way as reporting an existing
LSP, i.e., using a PCRpt message. In this case, there is an
overlap between PCReq and PCRpt, which MUST be resolved because
interoperability is at stake. Documenting the delegation of a new
LSP deserves some new text and possibly figure, the overlapping
specification of the PCRpt should be explicitly precluded.
### This is historical confusion in the figure, from the time when
initiation was rolled up in the same document. I modified the figure
so it is clear this is for updating parameters only.
[JM] Seems addressed. Just to be confirmed when published and to
confront with draft-ietf-pce-pce-initiated-lsp.
<snip>
- s/<ERO><attribute-list>/<RRO><attribute-list>/ [Per RFC 5440, a
report from PCC to PCE is RRO.]
### XXX Pending
[JM] As above.
- The need for the optional xRO is not well documented. Its proper
naming will depend on the associated use that must be clarified.
- "SRP" is still not defined.
### Isn't it enough to reference the section in which it is defined?
[JM] Once added in terminology section, all these are solved.
<snip>
- The use of the optional xRO is mentioned, but its relationship
with the RRO (formerly ERO) is not clear. I suspect some
assumptions are made on the way the ERO/RRO are populated; RFC
5440 only says ERO for PCE->PCC and RRO for PCC->PCE.
### XXX Pending
[JM] As above.
<snip>
- The behavior associated to the resource limit per PCC rather
looks like a Notifcation than an Error (e.g., in RFC 5440,
cancelling a set of pending requests relies on PCNtf). Please
consider the use of Notification instead of Error here.
### XXX Pending
[JM] To be updated.
<snip>
- s/defined in this document/defined in that (aforementioned)
document/
### Unclear comment, the intention was for the flags to be set as
explained for the new objects in this document, not for anything in 5440.
[JM] As above: adding "current" may be enough.
<snip>
- s/on a PCRpt message/On a PCRpt or PCReq message/
### We don't support delegation through a PCReq message
[JM] Addressed as part of the resolution of the new path delegation.
<snip>
- "PCE SHOULD remove all state" is written 3 times: I wonder about
"MUST".
### SHOULD is the correct operation, the PCE is free to choose to keep
state around for as long as it wishes. The 3 removal statements refer
to different scenarios.
[JM] I can live with that.
<snip>
- It would be nice to elaborate on the reason why the
SYMBOLIC-PATH-NAME MUST be included and not SHOULD.
- I do not see why SYMBOLIC-PATH-NAME may be included in SRP
Object: defining the LSP Object as its single place seems enough
and much simpler.
### XXX Pending
<snip>
- As mentioned above, in Error-Type 19, Error-value 4 should be
considered in a PCNtf.
### XXX Pending
<snip>
- Again in section 10.4, the "resource limit exceed" information
(Error-value 4 of Error-Type 19) should be considered in a PCNtf.
### XXX Pending
<snip>
_______________________________________________
Pce mailing list
Pce@ietf.org
https://www.ietf.org/mailman/listinfo/pce
