Dear Authors,
Please find my review below of draft-ietf-spring-mpls-path-segment-15. I have
used idnit on this version so that you can see the line numbers for my comments.
##########
24 In SR for MPLS data plane (SR-MPLS), an Egress node can not
determine
Jim> replace 'can not' with 'cannot'
26 because the segment identifiers are stripped from the label
stack as
Jim> More accurately the segment identifiers are 'swapped' or 'popped' from the
label stack. Suggest changing 'stripped' to 'removed'.
91 1. Introduction
93 Segment Routing (SR) [RFC8402] leverages the source-routing
paradigm
94 to steer packets from a source node through a controlled set of
95 instructions, called segments, by prepending the packet with an
SR
96 header in the MPLS data plane SR-MPLS [RFC8660] through a label
stack
97 to construct an SR path.
Jim> I would suggest rewording the above paragraph as it is somewhat confusing
for those who do not know the technology. Suggest:
Segment Routing (SR) [RFC8402] leverages the source-routing
paradigm
to steer packets from a source node through a controlled set
of
instructions, called segments, by prepending the packet with
an SR
header. In the MPLS data plane SR-MPLS [RFC8660] the SR
header is
instantiated through a label stack.
100 the labels in the MPLS label stack will be swapped or popped. So
101 that no label or only the last label (e.g. a service label or an
Jim> I would replace 'So that ...' with 'The result of this is that no label
...'. In addition, I would remove the text '(e.g. a service label or an
Explicit-Null label)' as it is not necessary.
106 However, to support various use-cases in SR-MPLS networks, such as
107 end-to-end 1+1 path protection (Live-Live case) Section 3.3,
108 bidirectional path Section 3.2, or Performance Measurement (PM)
109 Section 3.1, the ability to implement path identification on the
110 egress node is a pre-requisite.
Jim> Please reorder the above paragraph to list the use cases in the order in
which they appear in the document e.g. 3.1, 3.2, 3.3.
112 Therefore, this document introduces a new segment type, Path
Segment.
Jim> Change the above sentence to 'Therefore, this document defines a new
segment type, referred to herein as a Path Segment.'
114 egress node of the path. It MAY be used by the egress nodes for
path
Jim> Change 'nodes' to 'node' above.
115 identification hence to support various use-cases including SR
path
116 PM, end-to-end 1+1 SR path protection, and bidirectional SR paths
117 correlation. Note that, per-path states will be maintained in the
Jim> Remove text 'hence to support various use-cases including SR path PM,
end-to-end 1+1 SR path protection, and bidirectional SR paths correlation' as
you already stated above. Also, change 'states' to 'state'.
118 egress node due to the requirements in these use cases, though in
Jim> Change 'requirements in these use cases' to 'requirements in the
aforementioned use cases'
119 normal cases that the per-path states will be maintained in the
120 ingress node only in the SR architecture.
Jim> Change 'states' to 'state' and remove the text 'in the SR architecture.'
159 A Path Segment is a Local Segment which uniquely identify an SR
path
Jim> Change 'identify' to 'identifies'.
164 The term of SR path used in this document is a path described by a
165 Segment-List (SL). A PSID is used to identify a Segment List.
Jim> Is the first sentence above necessary? If not please remove it.
166 However, one PSID can be used to identify multiple Segment Lists
in
167 some use cases if needed. For example, one single PSID may be
used
168 to identify some or all Segment lists in a Candidate path or an SR
169 policy, if an operator would like to aggregate these Segment
Lists in
170 operation. How to use the PSID to Segment Lists depends on the
171 requirements of the use cases.
Jim> I would remove the last sentence above as it adds no value. The previous
sentence explains that a single PSID may be used to identify > 1 segment lists
and gives examples. Also should 'may' be 'MAY'?
173 When a PSID is used, the PSID can be inserted at the ingress node
and
Jim> 'Can be'? Is it possible for the PSID to be inserted anywhere else other
than the ingress node? If not then replace 'can be' with 'is'.
174 MUST immediately follow the last label of the SR path associated
to it
Jim> Remove text 'associated to it'.
178 when receiving on an intermidate node of the associated path, but
it
Jim> Replace 'receiving' with 'received'. Replace 'intermidate' with
'intermediate'.
179 can be the top label in the label stack on the penultimate node
after
180 the last forwarding label with Penultimate Hop Popping (PHP) is
Jim> I would remove the text 'after the last forwarding label' as this has
already been stated and the text makes the reading awkward. Also, remove the
text 'is popped off.'
181 popped off. Otherwise, the PSID may be processed by an
intermediate
182 node, which may cause error in forwarding because of mis-matching.
Jim> I do not think the last sentence is necessary and I would remove it. It is
obvious from the previous text that processing of the PSID by an intermediate
node will cause forwarding errors.
207 RFC8491 the MSD signals the total number of MPLS labels that can
be
Jim> Please make RFC8491 a reference and enclose with [].
241 2.1. Equal-Cost Multipath Considerations
243 If Entropy Label(EL) is also used on this egress node, as per
Jim> Replace 'this' with 'the'.
312 The mechanism of constructing bidirectional path using path
segment
313 is out of the scope of this draft and has been described in
several
Jim> Replace 'draft' with 'document'.
332 This equivalence group can be instantiated in the network by an
SDN
Jim> Remove 'SDN' from the above.
337 Binding SID (BSID) [RFC8402] can be used for SID list compression.
338 With BSID, an end-to-end SR path in a trusted domain can be
splitted
Jim> Replace 'splitted' with 'split'.
391 the ingress node of the associated path will learn the info of
PSID.
Jim> Replace 'info' with 'information'.
396 deeper label stack which representing a longer path. For example
the
Jim> Replace 'representing' with 'represents'.
397 case described in {#psid-nesting} and the related BSID is not used
Jim> Replace '{#psid-nesting}' with a reference to Section 3.4 of this document.
400 a trusted domain under the considerations defined in [RFC8402].
Jim> What specifically are these considerations and where in RFC8402 are they
documented? You need some text here or at a minimum a reference as to which
section of RFC8402 you are referring too.
581 5.6. Interoperability Test
Jim> Should this section also be removed by the RFC editor upon publication? If
so please state this specifically as you have done in the implementation
section. If it is not to be removed you need to explicitly state that the
information was at the time of publication.
Jim
_______________________________________________
spring mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/spring
