Dear authors,
I have reviewed draft-ietf-spring-cs-sr-policy-05 and have the following
comments, listed below from <comment 1> to <comment 44>.
These comments mainly include the following:
a) Definition of CS-SR Policy is suggested to be more clear <comment 17>
b) Inconsistent description style used when describing SR-MPLS and SRv6,PCEP
and BGP,PCC-initiated mode and PCE-initiated mode.
c) Some references are lost or not be updated to the latest status
d) Key words("MUST", "SHOULD", "RECOMMENDED"...) are suggested to be added in
some places
e) Discussion on the terms used for recovery scheme in section 7.
f) Some texts are suggestion to be reconstructed for ease of reading
g) Editorial nits
Line numbers is included from nits
(https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-spring-cs-sr-policy-05.txt)
to help identify where in the document the comment applies.
Regards,
Yao
13 Circuit Style Segment Routing Policies
14 draft-ietf-spring-cs-sr-policy-05
<comment 1><technical> "Circuit Style Segment Routing Policies" or ""Circuit
Style Segment Routing Policy" ?
I understand that when we are talking able more than one CS-SR Policy, the term
"CS-SR Policies" should be used. But when refering to the new concept (a new
type of SR policy), using "CS-SR Policy" may be better and the usage should be
consistent throughout the document, taking examples from RFC8402.
94 However transport services (commonly referred to as "private lines")
95 are delivered via pseudowires (defined by the PWE3 and PALS
96 workgroups) and require:
<comment 2><technical> Suggest to use RFCs(e.g RFC3985) or documents as
references for pseudowires instead of names of working groups.
141 * LSPA : LSP attributes
<comment 3><editorial> "attributes" should be "Attributes"
195 When using PCEP as the communication protocol on the headend routers,
196 the centralized entity is a stateful PCE defined in [RFC8231]. When
197 using a MPLS data plane [RFC8660], PCEP extensions defined in
198 [RFC8664] will be used. When using a SRv6 data plane [RFC8754],
199 [RFC8986], PCEP extensions defined in [RFC9603] are used.
<comment 4><editorial> Inconsistent description style used when describing
SR-MPLS and SRv6. Use "MPLS"&"IPv6", or "SR-MPLS"&"SRv6". And RFC8986 seems
unnecessary. Similar comments apply for line 230.
Suggested text:
When using the SR-MPLS data plane [RFC8660], PCEP extensions defined in
[RFC8664] are used. When using the SRv6 data plane [RFC8754],PCEP extensions
defined in [RFC9603] are used.
192 CS-SR Policies can be instantiated in the headend routers using
193 configuration, PCEP or BGP.
195 When using PCEP as the communication protocol on the headend routers,
196 the centralized entity is a stateful PCE defined in [RFC8231]. When
197 using a MPLS data plane [RFC8660], PCEP extensions defined in
198 [RFC8664] will be used. When using a SRv6 data plane [RFC8754],
199 [RFC8986], PCEP extensions defined in [RFC9603] are used.
201 When using BGP as the communication protocol on the headend routers,
202 the BGP extensions defined in [I-D.ietf-idr-sr-policy-safi] are used.
204 When using configuration, the YANG model defined in
205 [I-D.ietf-spring-sr-policy-yang] does apply.
<comment 5><editorial> Suggest to adjust the structure of the above text to
combine them as a whole paragraph.
Suggested text:
CS-SR Policies can be instantiated in the headend routers using PCEP,BGP or
configuration.
* When using PCEP as the communication protocol on the headend routers, the
centralized entity is a stateful PCE defined in [RFC8231]. When using the
SR-MPLS data plane [RFC8660], PCEP extensions defined in [RFC8664] are used.
When using the SRv6 data plane [RFC8754], PCEP extensions defined in [RFC9603]
are used.
* When using BGP as the communication protocol on the headend routers, the BGP
extensions defined in [I-D.ietf-idr-sr-policy-safi] are used.
* When using configuration, the YANG model defined in
[I-D.ietf-spring-sr-policy-yang] does apply.
210 * An adjacency-SID which is:
212 - Manually allocated or persistent: to ensure that its value does
213 not change after a node reload
<comment 6><technical><discussion> "Manually allocated or persistent" or
"Manually allocated and persistent" ?
224 In a network with link bundles an adjacency-SID SHOULD be assigned to
225 each member-link ([RFC8668], [RFC9356]) to ensure deterministic
226 traffic placement onto physical links.
<comment 7><technical> Some reference/example may help to understand what link
bundle is.
Suggested Text:
In a network with link bundles(i.e,Link Aggregation Group[IEEE802.1AX]), an
adjacency-SID SHOULD be assigned to each member-link ([RFC8668], [RFC9356]) to
ensure deterministic traffic placement onto physical links.
227 adjacency-SIDs representing parallel adjacencies Section 4.3 of
228 [RFC8402] SHOULD also be avoided.
<comment 8><technical> RFC8402 doesn't have section 4.3
235 When using a SRv6 data plane [RFC8754], [RFC8986] the IGP extensions
236 defined in [RFC9352], [RFC9513] and BGP-LS extensions in [RFC9514]
237 apply.
<comment 9><editorial> RFC8986 seems unnecessary. A few places in this document
use "a SR...", should be "an SR..."
Suggested Text:
When using an SRv6 data plane [RFC8754], the IGP extensions defined in
[RFC9352], [RFC9513] and BGP-LS extensions in [RFC9514] apply.
242 In a circuit switched network such as SONET/SDH, OTN or
243 DWDM resources (timeslots or a wavelength) are allocated
<comment 10><technical> The terms SONET/SDH, OTN and DWDM are neither listed in
the terminology section nor with explanation attached.
245 In a packet switched network resources are
246 only allocated when communication is present, i.e. packets are to be
247 sent.
<comment 11><editorial> Comma is lost in a few places, such as line 250,line
349,line 406,line 429,line547,605,610
Suggested Text:
In a packet switched network, resources are only allocated when communication
is present, i.e. when packets are to be sent.
280 * Use of dedicated manual unprotected adjacency-SIDs that are used
281 solely by CS-SR traffic. Features like TI-LFA
282 [I-D.draft-ietf-rtgwg-segment-routing-ti-lfa] and microloop
283 avoidance [I-D.draft-bashandy-rtgwg-segment-routing-uloop] can use
284 dynamic unprotected adjancency-SIDs.
<comment 12><editorial> Start with an imperative sentence to keep it consistent
with the previous graphes.
Suggested Text: Use dedicated persistent unprotected adjacency-SIDs solely for
CS-SR policies.
<comment 13><technical> I didn't quite understand what is the "dynamic
unprotected adjancency-SIDs" here. If TI-LFA is used, it is a protected adj-sid.
286 The approach of allocating a Diffserv codepoint can leverage any of
287 the following Per Hop Behavior (PHB) strategies:
289 * Use a Assured Forwarding (AF) class queue for CS-SR Policies and
290 limit the total utilization across all other queues to bandwidth O
291 by traffic policing or shaping and ensure that P - N - O >= C
293 * Use a Expedited Forwarding (EF) class queue for CS-SR Policies and
294 limit the total utilization across all other EF queues of higher
295 or equal priority to bandwidth O by traffic policing or shaping
296 and ensure that P - N - O >= C
298 * Use a Expedited Forwarding (EF) class queue for CS-SR Policies
299 with a priority higher than all other EF queues and limit the
300 utilization of the CS-SR Policy EF queue by traffic policing to C
301 <= P - N
<comment 14><editorial> Per Hop Behavior--> Per-Hop Behavior as mentioned
in line220 and RFC3246.
<comment 15><technical> reference should be added for AF(RFC2597) and
EF(RFC3246)
<comment 16><technical> considering that the whole text is the expansion of the
Diffserv codepoint allocating method, and the meaning P, N and C are not
mentioned in the previous Diffserv codepoint allocating method part. It is
suggestion to add the explanation for them at the beginning of this part for
ease of understanding.
OLD:
The approach of allocating a Diffserv codepoint can leverage any of the
following Per Hop Behavior (PHB) strategies:
Suggested NEW Text:
The approach of allocating a Diffserv codepoint can leverage any of the
following Per Hop Behavior (PHB) strategies below, where P is the the bandwidth
of physical link, N is the bandwidth allocated for network control and C is
the bandwidth reserved for CS-SR policies.
120 SR Policies that satisfy those requirements are called "Circuit-
121 Style" SR Policies (CS-SR Policies).
311 5. CS-SR Policy Characteristics
313 A CS-SR Policy has the following characteristics:
317 * Bidirectional co-routed: a CS-SR Policy between A and Z is an
318 association of an SR Policy from A to Z and an SR Policy from Z to
319 A following the same path(s)
<comment 17><technical><discussion> Just to confirm that a CS-SR Policy is
consist of two unidirection SR policies that are co-routed, right?
If the above understanding is right,let's look back at the definition of CS-SR
Policy in the introduction section(line 120, line 121), the definition is not
that accurate.
A "normal" SR Policy is unidirectioanal, so there wouldn't be an single SR
Policy can satisfy all the requirements listed in the introduction section
along and can be called a CS-SR Policy.
It is suggested to re-consider the definition of the CS-SR Policy in the
introduction section to make it more clear and aligned with the characteristics
in section 5, such as, A "Circuit-Style" SR Policy (CS-SR Policy)" is an
association of two co-routed unidirectioanal SR Policies, and it satisfies the
above requirements.
328 * Multiple candidate paths in case of protection/restoration:
<comment 18><technical> This description may lead to misunderstanding that
there must be multiple candidate paths in a CS-SR Policy.
Suggested NEW Text:
More than one candidate path may appear in case of protection/restoration
342 * Connectivity verification and performance measurement is activated
343 on each candidate path (Section 8)
<comment 19><editorial>
NEW
Connectivity verification and performance measurement are activated on each
candidate path (Section 8)
353 Both nodes A and Z act as PCC and delegate path computation to the
354 PCE using PCEP with the extensions defined in [RFC8664] and the
355 procedure described in Section 5.7.1 of [RFC8231]. SRv6 specific
356 extensions are defined in [RFC9603].
<comment 20><editorial> re-construct the paragraph to make parallel
descriptions for SR-MPLS and SRv6
Suggested new text
Both node A and Z act as PCC and delegate path computation to the
PCE using PCEP with the procedure described in Section 5.7.1 of
[RFC8231].
For SR-MPLS, the extensions defined in [RFC8664] are used. And SRv6
specific
extensions are defined in [RFC9603].
358 The PCRpt message sent from the headends to the PCE contains the
359 following parameters:
<comment 21><technical> Is a "SHOULD" needed here?
Suggested New Text
The PCRpt message sent from the headends to the PCE SHOULD contain the
following parameters:
347 6.1. Policy Creation when using PCEP
413 In addition to the above described PCC-initiated mode of operation,
414 The SR Policies can be instantiated in the network by a PCE using
415 PCE-initiated procedures.
<comment 22><technical><major> The PCC-initiated procedures are described
detailly and takes up most of section 6.1, while the PCE-initiated procedures
are only mentioned in the above paragraph, which seems not clear enough as a
guidance for implementing PCE-initiated mode. It is suggested to expand the
PCE-initiated procedures, either as paragraphs in section 6.1 or as a
sub-section (e.g section6.1.1 for PCC-initiated mode and section6.1.2 for
PCE-initiated mode). For the description of PCE-initiated mode, if some
procedures are just the same as those for PCC-initiated mode,a concise
description can be used or just refering to the PCC-initiated mode, but it
should not be omitted.
423 The centralized controller is instructed (i.e. by an application via
424 an API call)
<comment 23><technical> By using "i.e", it indicates that the only way to
instruct the controller is by an application via an API call. "e.g" seems
better since there may be other choices such as direct configuration on the
controller.
423 The centralized controller is instructed (i.e. by an application via
424 an API call) to create the CS-SR Policy, for which the controller
425 does perform path computation and is requesting headend A via BGP to
426 instantiate a SR Policy (with Z as endpoint) and requesting headend Z
427 via BGP to instantiate a SR Policy (with Z as endpoint).
<comment 24><editorial> I'm not a native English speaker, but "is requesting"
used here seems a little bit strange, and two "and" are used in one sentence.
Suggested New Text:
for which the controller performs path computation and requests the
headends via BGP to instantiate the corresponding SR Policies on them, e.g,
headend A is requested via BGP to instantiate an SR Policy with Z as the
endpoint and headend Z is requested via BGP to instantiate an SR Policy with A
as the endpoint.
460 6.3. Maximum SID Depth
<comment 25><technical> Suggest to use "Maximum SID Depth Constraint " as the
title to make the theme clear at the first glance.
466 When using SR-MPLS this constraint is called "Base MPLS Imposition
467 MSD" and is advertised via IS-IS [RFC8491], OSPF [RFC8476], BGP-LS
468 [RFC8814] and PCEP [RFC8664].
470 When using SRv6 this constraint is called "SRH Max H.encaps" and is
471 advertised via IS-IS [RFC9352], OSPF [RFC9513], BGP-LS [RFC9514] and
472 PCEP [RFC9603]
<comment 26><technical> Suggest to use "SRH Max H.encaps MSD" to be aligned
with "Base MPLS Imposition MSD" in MPLS.
473 The MSD constraint is typically resolved by leveraging a label stack
474 reduction technique, such as using Node SIDs and/or BSIDs (SR
475 architecture [RFC8402]) in a segment list, which represents one or
476 many hops in a given path.
<comment 27><technical> "label stack" ---> "segment list"
500 7. Recovery Schemes
502 Various protection and restoration schemes can be implemented. The
503 terms "protection" and "restoration" are used with the same subtle
504 distinctions outlined in section 1 of [RFC4872], [RFC4427] and
505 [RFC3386] respectively.
<comment 28><technical>When refering to the difference between "protection" and
"restoration", 3 RFCs are used for reference. Section 1 of [RFC4872] only says
that, to check [RFC4427] to see the difference, this reference is unnecessary.
And RFC4427 kind of makes the definition and use "protection" and "restoration"
more clear since RFC3386 uses these two terms interchangeably although it is
(probably)the first RFC defines them. So is it enough if RFC4427 is used as the
only reference here? And this paragraph is suggested to be re-constructed for
ease of understanding.
Suggested Text:
Various recovery(protection and restoration) schemes can be implemented
for CS-SR Policy. As described in RFC4427 section 4.3, there's subtle
distinction between the terms "protection" and "restoration", based on the
resource allocation done during the recovery path establishment. Same
definitions apply for the CS-SR Policy recovery schemes, wherein,
Protection: another candidate path is computed and fully established in
the data plane and ready to carry traffic
Restoration: a candidate path may be computed and may be partially
established but is not ready to carry traffic
513 The term "failure" is used to represent both "hard failures" such
514 complete loss of connectivity detected by connectivity verification
515 described in Section 8.1 or degradation, a packet loss ratio, beyond
516 a configured acceptable threshold.
<comment 29><editorial>
Suggested Text:
The term "failure" is used to represent both "hard failures" such as
complete loss of connectivity detected by connectivity verification
described in Section 8.1 and degradation, i.e, when the packet loss
ratio beyond
a configured acceptable threshold.
518 7.1. Unprotected
525 When using PCEP, a PCRpt message is sent from the PCC to the PCE with
526 the O field in the LSP object set to 2.
<comment 30><technical>Add reference for the O field(rfc8231#section-7.3)
<comment 31><technical>Explain the meaning of value 2 to be aligned with the
description for BGP-LS, same suggestion applies for the following sections
where the value of O field is described.
Suggested Text:
When using PCEP, a PCRpt message is sent from the PCC to the PCE with
the O field in the LSP object[RFC8231] set to 2 to indicate the candidate
path is active and carrying traffic.
545 7.2. 1:1 Protection
558 Appropriate routing of the protect path diverse from the working path
559 can be requested from the PCE by using the "Disjointness Association"
560 object (type 2) defined in [RFC8800] in the PCRpt messages. The
561 disjoint requirements are communicated in the "DISJOINTNESS-
562 CONFIGURATION TLV"
564 * L bit set to 1 for link diversity
565 * N bit set to 1 for node diversity
567 * S bit set to 1 for SRLG diversity
569 * T bit set to enforce strict diversity
571 The P bit may be set for first candidate path to allow for finding
572 the best working path that does satisfy all constraints without
573 considering diversity to the protect path.
<comment 32><technical> The term "first candidate path" in line 571, is it the
primary candidate path with the higher priority?
Suggested Text:
For the primary candidate path, the P bit may be set to allow for finding the
best working path that does satisfy all constraints without considering
diversity to the protect path.
654 7.3.1. 1+R Restoration
<comment 33><technical> Why it is called 1+R restoration since only one
candidate path is used for protection in this section
837 8. Operations, Administration, and Maintenance (OAM)
<comment 34><technical><discussion>The whole section using STAMP for OAM
purpose, can other mechanism be used, e.g, BFD for connectivity validation?
841 The proper operation of each segment list is validated by both
842 headends using STAMP in loopback measurement mode as described in
843 section 4.2.3 of [I-D.ietf-spring-stamp-srpm].
<comment 35><technical> no section 4.2.3 in [I-D.ietf-spring-stamp-srpm] now
<comment 36><technical> is there a "SHOULD"/"MAY"/“RECOMMANDED" missing ? And I
don't quite understand why use "The proper operation" ,can we just state that
it's connectivity verification operation
Suggested Text(taking “RECOMMENDED" as an example with reconstruction) :
The connectivity verification for each segment list on both headends is
RECOMMENDED to be done using STAMP in loopback measurement mode as described in
section 6 of [I-D.ietf-spring-stamp-srpm].
845 As the STAMP test packets are including both the segment list of the
846 forward and reverse path, standard segment routing data plane
847 operations will make those packets get switched along the forward
848 path to the tailend and along the reverse path back to the headend.
850 When using PCEP, the headend forms the bidirectional SR Policy
851 association using the procedure described in
852 [I-D.ietf-pce-sr-bidir-path] and receives the information about the
853 reverse segment list from the PCE as described in section 4.5 of
854 [I-D.ietf-pce-multipath]
856 When using BGP, the controller does inform the headend routers about
857 the reverse segment list using the Reverse Segment List Sub-TLV
858 defined in section 4.1 of [I-D.ietf-idr-sr-policy-path-segment].
<comment 37><technical> For SRv6, packets wouldn't get switched just forwarded.
Suggested Text:
As the STAMP test packets carry both the segment list of the forward and
reverse path, standard segment routing data plane operations will make those
packets get forwarded along the forward path to the tailend and along the
reverse path back to the headend.
<comment 38><technical> Before talking about the PCEP and BGP, it feels like
something is missing to explain why the headend need to know the reverse
segment list information.
Suggested Text:
In order to be able to send STAMP test packets for loopback measurement
mode, the STAMP Session-Sender(i.e, the headend) needs to acquire the segment
list information of the reverse path:
* PCEP part...
* BGP part....
865 8.2. Performance Measurement
867 The same STAMP session is used to estimate round-trip loss as
868 described in section 5 of [I-D.ietf-spring-stamp-srpm].
870 The same STAMP session used for connectivity verification can be used
871 to measure delay. As loopback mode is used only round-trip delay is
872 measured and one-way has to be derived by dividing the round-trip
873 delay by two.
<comment 39><editorial> The first paragraph seems redundant since the beginning
of the second paragraph repeated the same thing.
<comment 40><technical><discussion> Is the one-way delay always half of the
round-trip delay? Or in other words, is the delay of the forward path always
equal to the reverse path ?
875 8.3. Candidate Path Validity Verification
877 A stateful PCE/controller is in sync with the network topology and
878 the CS-SR Policies provisioned on the headend routers. As described
879 in Section 5 a path must not be automatically recomputed after or
880 optimized for topology changes. However, there may be a requirement
881 for the stateful PCE/controller to tear down a path if the path no
882 longer satisfies the original requirements, detected by stateful PCE/
883 controller, such as insufficient bandwidth, diversity constraint no
884 longer met or latency constraint exceeded.
<comment 41><editorial>I try to reconstruction the paragraph
Suggested Text:
A stateful PCE/controller is in sync with the headend routers in
the network topology and the CS-SR Policies provisioned on them. As
described
in Section 5, a path MUST not be automatically recomputed after or
optimized for topology changes. However, there may be a requirement
for the stateful PCE/controller to tear down a path if the path no
longer satisfies the original requirements, as detected by stateful
PCE/
controller, such as insufficient bandwidth, diversity constraint no
longer met or latency constraint exceeded.
886 The headend may measure the actual bandwidth utilization of a CS-SR
887 Policy to take local action and/or report it as requested bandwidth
888 via PCEP or BGP-LS to the stateful PCE/controller. Typical actions
889 are raising alarms or adjusting the reserved bandwidth.
<comment 42><technical> The measured bandwidth utilization(e.g,10GB/s) can be
reported to the controller to influence the bandwidth adjustment, but I don't
quite understand why the measured bandwidth can be used as the requested
bandwidth.(Its bandwidth measured is already 10GB/s, why requesting a 10GB/s
bandwidth to the controller ).
895 9. External Commands
<comment 43><technical> It's not clearly stated who would support these
external commands (I believe it's the controller) and are these commands
mandatory or optional. Maybe change the title(such as External Commands
Recommended of the controller) or add some description under the title(such
as, this section recommends some external commands on the controller).
897 9.1. Candidate Path Switchover
901
Operator triggered
902 switching between candidate paths is unidirectional and has to be
903 requested on both headends.
<comment 44><editorial> reconstruction the text for ease of reading. And is
there a "SHOULD" needed here?
Suggested New Text:
Since switch between candidate paths triggered by the operator is
unidirectional, the candidate path switch commands SHOULD be executed for both
headends of a CS-SR policy._______________________________________________
spring mailing list -- spring@ietf.org
To unsubscribe send an email to spring-le...@ietf.org
