Hi Authors,
Please find my review of your I-D. The core idea is clear but I feel
the document needs some cleanup and tightening of specifications at
par standards track document.
==
Major
******
(1) We need to use normative text in this standards track document.
Section 3 and section 5 could benefit from it. To illustrate my point
-
~ Section 3.1, the use of 'may' in -
The originating endpoint (PCC) node may report/ delegate the forward
and reverse direction LSPs to a PCE. The remote endpoint (PCC) node
may report its forward direction LSP to a PCE.
- Similar text in section 3.2
~ There is only 1 normative MUST in section 5.1 and 5.2.
~ My overall suggestion: Since you have multiple scenarios
(single-sided & double-sided) and (PCE-Initiated & PCC-Initiated) and
each requiring different processing, the current text ends up being
generic (and thus not possible to use Normative). It is better to
break the test for the 4 cases (or sub-sections) -
~ single-sided PCE-Initiated
~ single-sided PCC-Initiated
~ double-sided PCE-Initiated
~ double-sided PCC-Initiated
and use the normative language for message exchange (as well as the
use of ASSOCIATION) for each case separately. That should improve the
document clarity for interoperability.
(2) Section 4,
o An LSP (forward or reverse) cannot be part of more than one
Bidirectional LSP Association Group. More than one forward LSP
and/ or reverse LSP can be part of a Bidirectional LSP Association
Group.
~ Rewrite to use normative text!
~ Are you sure about the 2nd sentence? Isn't this association
between only 2 LSPs - one forward, one backward? In my reading, RFC
7551 is about 2 LSPs. If not, the use-case and handling need to be
explained.
o The Tunnel (as defined in [RFC3209]) of forward and reverse LSPs
of the Single-sided Bidirectional LSP Association on the
originating node MUST be the same.
~ Do you mean the Tunnel ID (as carried in LSP-IDENTIFIERS TLV
[RFC8231])? I ask because the source and destination would be opposite
for the forward and reverse LSPs in the LSP-IDENTIFIERS TLV? If yes,
then also add a restriction on the endpoint mismatch.
(3) We should be clear that existing forward or reverse LSP can be
bi-directionally associated. In this document, the focus seems to be
on initiation only. You have some text in section 5.1/5.2 but it would
be good to make it prominent something like Section-3.2 of RFC 7551.
(4) Include text for exchanging association types in the open message
[RFC8697] specifies the mechanism for the capability advertisement of
the Association types supported by a PCEP speaker by defining an
ASSOC-Type-List TLV to be carried within an OPEN object. This
capability exchange for the Bidirectional LSP Association Types MUST be
done before using the Bidirectional LSP Association. Thus, the PCEP
speaker MUST include the Bidirectional LSP Association Types in the
ASSOC-Type-List TLV and MUST receive the same from the PCEP peer
before using the Bidirectional LSP Association in PCEP messages.
(5) Error Reporting that needs to be added -
~ Bidirectional LSP Association types are not supported
~ Both LSPs marked as forward
~ Both LSPs marked as reverse
~ One direction LSP says co-routed, another direction non-co-routed
~ More than 2 LSPs in the group (if you agree with the comment in (2))
(6) For single-sided Bidirectional LSP, is the association information
configured on D? We claim this to be an operator-configured
association and thus want to make sure that is the case here!
=
Minor
******
- Global comment: Change RSVP to RSVP-TE in this I-D.
- Section 1:
The MPLS Transport Profile (MPLS-TP) requirements document [RFC5654]
specifies that MPLS-TP MUST support associated bidirectional point-
to-point LSPs. [RFC7551] defines RSVP signaling extensions for
binding two reverse unidirectional LSPs [RFC3209] into an associated
bidirectional LSP. The fast reroute (FRR) procedures for associated
bidirectional LSPs are described in [RFC8537].
~ The use of normative MUST in the first sentence is incorrect.
~ Reference to RFC 3209 seems out of place as that term is not used in the RFC
- Section 1.1: This summary feels out of place as we are yet to talk
about the single-sided and the double-sided bidirectional LSPs. You
can think about placing it after section 3.3. I was also wondering if
it makes sense to break the starting sentence (in the first 3 items)
into two, one for single-sided and another for double-sided. Also, the
last item about co-routed is mentioned in the context of stateless PCE
only - why?
- At the end of section 1, you should mention that this document
defines two association types (and corresponding association group)
and a Bidirectional LSP Association Group TLV with forward-references.
So that when they are mentioned in section 3 it is not out of the
blue.
- Section 3.1, Since we expect both endpoints to be PCC; better to
break it into 2 sentences.
OLD:
As specified in [RFC7551], in the single-sided case, the
bidirectional tunnel is provisioned only on one endpoint node (PCC)
of the tunnel.
NEW:
As specified in [RFC7551], in the single-sided case, the
bidirectional tunnel is provisioned only on one endpoint node
of the tunnel. Both endpoints act as a PCC.
END
- Section 3.1, do you mean PathTear below?
Similarly, the remote endpoint node deletes the
reverse LSP when it receives the RSVP Path delete message [RFC3209]
for the forward LSP.
~ It might be better to just refer to the RSVP-TE RFCs without
mentioning the message as the state could also be cleared in case of
other messages like PathErr.
- Section 3, for the figures, please add a legend so that the readers
understand what does F,R means? Maybe also the '0' as PLSP-ID=0.
- Section 4.2, cleaning up the text and adding MUST
OLD:
The Bidirectional LSP Association Group TLV is defined for use with
the Single-sided and Double-sided Bidirectional LSP Association Group
Object Types.
NEW:
The Bidirectional LSP Association Group TLV an OPTIONAL TLV for use with the
Bidirectional LSP Associations (ASSOCIATION object with Association
type = TBD1 for Single-sided or TBD2 for Double-sided).
END
- Section 4.2, Reserved should be called Flags field. You should say
unassigned flags MUST be set to zero and ignored on receipt. You would
then also align this section with the IANA section.
- Section 5.3, add a reference to RFC 8697. Do we need some mechanism
to tell computation failure, especially for the co-routed case?
=
Nits
******
- Expand PCEP in Title to Path Computation Element Communication Protocol
- Section 1: s/Path Computation Element Protocol (PCEP)/Path
Computation Element Communication Protocol (PCEP)/
- Section 1: To align with RFC 8697
OLD:
[RFC8697] introduces a generic mechanism to create a grouping of LSPs
which can then be used to define associations between a set of LSPs
and/or a set of attributes, for example primary and secondary LSP
associations, and is equally applicable to the active and passive
modes of a Stateful PCE [RFC8231] or a stateless PCE [RFC5440].
NEW:
[RFC8697] introduces a generic mechanism to create a grouping of LSPs.
This grouping can then be used to define associations between
sets of LSPs or between a set of LSPs and a set of attributes,
and it is equally applicable to the stateful PCE (active and
passive modes) and the stateless PCE.
END
- Section 3:
As shown in Figure 1, two reverse unidirectional LSPs can be grouped
to form an associated bidirectional LSP.
~ Shouldn't this be 'forward and reverse' instead of 'two reverse'?
- Section 4: As per changes made in other association draft by the RFC-Editor
OLD:
This document defines two new Association Types for the ASSOCIATION
Object (Object-Class value 40) as follows:
o Association Type (TBD1) = Single-sided Bidirectional LSP
Association Group
o Association Type (TBD2) = Double-sided Bidirectional LSP
Association Group
NEW:
This document defines two new Association types, called
"Single-sided Bidirectional LSP" (TBD1) and "Double-sided
Bidirectional LSP" (TBD2), based on the generic ASSOCIATION
object.
END
- Section 4.2
OLD:
o For co-routed LSPs, this TLV MUST be present.
o For reverse LSPs, this TLV MUST be present.
NEW:
o For co-routed LSPs, this TLV MUST be present and C flag set.
o For reverse LSPs, this TLV MUST be present and R flag set.
END
- Section 5.7, can the same error handling for PCC and PCE be changed
to use terms PCEP speaker and PCEP peer instead?
~ Applicable to other errors as well
- Section 9.1
~s/defined [RFC8697]/defined in [RFC8697]/
OLD:
This document adds new Association Types for the ASSOCIATION Object
(Object-class value 40) defined [RFC8697]. IANA is requested to make
the assignment of values for the sub-registry "ASSOCIATION Type"
[RFC8697], as follows:
NEW:
This document defines two new Association types, originally described in
[RFC8697]. IANA is requested to assigned the following new values in the
"ASSOCIATION Type Field" subregistry [RFC8697] within the "Path
Computation Element Protocol (PCEP) Numbers" registry:
END
~ Remove 'group' from the name.
- Section 9.2.1, add 0-28 as Unassigned
- Acknowledgments: You are thanking me twice :)
==
Thanks!
Dhruv
_______________________________________________
Pce mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/pce
