Hello,
please find my comments on the pending items.
On 10/26/2015 10:10 PM, Ina Minei wrote:
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?
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?
3. Can you please review the comments that were not incorporated and
let us know if you agree?
Thank you,
Ina
On Fri, Oct 16, 2015 at 7:40 AM, Julien Meuric
<[email protected] <mailto:[email protected]>> wrote:
Dear authors,
To prepare the upcoming move to the IESG, please find below my
review of the aforementioned I-D (at last!).
_Summary_
Main qualities:
- core specification is clear;
- wording is smooth, with very few typos;
- the manageability and security sections have been included with
relevant text.
Main issues to point out:
- RFC 2119 keywords (picky me, but so is the IESG: ask JP about 5440);
- a few corner/error cases;
- consistency with existing RFCs (5440, 5886).
I also take the opportunity to remind the WG that including
codepoint values from existing registries before allocation by the
IANA (which can be requested early) is a very bad idea, whatever
the WG.
_Detailed Comments_
[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
In this case PCRpt differs from PCReq. The PCE needs to know the ERO
object for each LSP, as may have been pushed by a different PCE.
Reporting RRO is not sufficient, as that contains the effective LSP
path, e.g. with loose hops expanded by the PCC. That is why ERO is a
mandatory object in PCRpt (as part of intended_path), hence we specify
an empty object for the end-of-sync marker.
- 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
The association between a PCEP session and PCE processes is something
which I would consider an internal PCE detail, and it should be covered
by the next sentence (e.g. raise PCErr 20/1).
- When mentioning errors, adding a sentence reminding that RFC
5440 already defines a set of applicable error codes would be
valuable.
### XXX Pending
I agree, this is an extension, so implementations should reuse RFC5440
errors when appropriate.
- 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
It is not, as that would be seen as a request to modify the LSP setup to
empty. Such an acknowledgement would have to include full configuration
as previously reported -- which would be handled as a normal update.
- 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.
- 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
It could, if we want to be strict about it, but it does not really have
an impact on protocol operation: delegate=0 should kick in first, which
means the PCC can safely discard any extra payload.
- s/<ERO><attribute-list>/<RRO><attribute-list>/ [Per RFC 5440, a
report from PCC to PCE is RRO.]
### XXX Pending
- 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
The idea here is that ERO contains the path as pushed by the PCE. It may
contain loose hops, which the PCC can expand as it sees fit. The RRO
contains the effective path the LSP is currently taking, e.g. any loose
hops are resolved. Since a backup PCE is not required to share state
with the primary PCE, and there is no way to derive ERO from RRO, the
PCEP session needs to communicate both, so a backup PCE can pick up the
previous PCE's policy decision as well as the current LSP path.
- 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
Current wording is based on the assumption that the PCE has to have a
consistent point-in-time view of the PCC's state. In this regard a PCRpt
of a new LSP which exceeds PCE implementation-internal limit on the
number of LSPs it supports would break that assumption, hence we chose
PCErr. This makes it consistent with what would happen if that LSP is
reported during initial state resynchronization.
- 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
The MUST is there to maintain a single global identifier for the LSP.
PLSP-ID is then used as a shorthand. I do not recollect the exact
reasoning as to why the TLV can be in SRP, as the placement and
semantics of that TLV has changed quite a bit over the past couple of
years. If I were to venture a guess, I think it was retrofitted to allow
the PCE to update the symbolic path name.
Thanks,
Robert
_______________________________________________
Pce mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/pce
