Thank you, Alvaro.

We will address other comments in later versions.

Best Regards,
Shuping


发件人: Alvaro Retana <aretana.i...@gmail.com>
发送时间: 2024年9月13日 20:23
收件人: Pengshuping (Peng Shuping) <pengshup...@huawei.com>; 
draft-peng-spring-pmtu-sr-pol...@ietf.org
抄送: SPRING WG <spring@ietf.org>
主题: Re: Updates made on draft-ietf-spring-pmtu-sr-policy-01 RE: Chair Review of 
draft-peng-spring-pmtu-sr-policy-03

Hi Shuping!

I looked at the diffs and the changes look good to me.

Thanks for addressing my comments.

Alvaro.


On September 12, 2024 at 9:56:11 PM, Pengshuping (Peng Shuping) 
(pengshup...@huawei.com<mailto:pengshup...@huawei.com>) wrote:
Hi Alvaro, folks,

Many thanks to the Chair's such thorough reviews!
Please find in line starting with Shuping>.
We also updated the draft accordingly. Please let us know if you have any 
further comments.

HTMLized: https://datatracker.ietf.org/doc/html/draft-ietf-spring-pmtu-sr-policy
Diff: 
https://author-tools.ietf.org/iddiff?url2=draft-ietf-spring-pmtu-sr-policy-01

Thank you!

Best Regards,
Shuping


-----Original Message-----
From: Alvaro Retana 
[mailto:aretana.i...@gmail.com<mailto:aretana.i...@gmail.com>]
Sent: Wednesday, July 10, 2024 2:03 AM
To: 
draft-peng-spring-pmtu-sr-pol...@ietf.org<mailto:draft-peng-spring-pmtu-sr-pol...@ietf.org>
Cc: SPRING WG <spring@ietf.org<mailto:spring@ietf.org>>
Subject: Chair Review of draft-peng-spring-pmtu-sr-policy-03

Dear authors:

As part of the adoption call I have comments related to this draft.
Please see details in-line.

Thanks!

Alvaro.


[Line numbers from idnits.]


...
137 3.  Terminology

139      Link MTU: As per [RFC4821], the Maximum Transmission Unit, i.e.,
140      maximum IP packet size in bytes, that can be conveyed in one piece
141      over a link.  This includes the IP header, but excludes link layer
142      headers and other framing that is not part of IP or the IP
143      payload.  In case of MPLS, it also includes the label stack and in
144      case of IPv6, it includes IPv6 extension headers (including SRH).

[major] rfc4821's definition of "Link MTU" was Updated by rfc8899 
(Packetization Layer Path MTU Discovery for Datagram Transports).  If referring 
to that definition, please use the updated reference.  Also, please don't 
repeat the definition (unless doing so in quotations) to avoid possibly getting 
out of sync.

Shuping> The RFC is updated to RFC8899. The definition related to the IP part 
is changed accordingly.


[major] The definition in rfc4821/rfc8899 refers to IP networks, not MPLS 
networks. I looked and found a definition in rfc3988 (MTU Signalling Extensions 
for LDP), an Experimental RFC. §3/rfc3032 (MPLS Label Stack Encoding) covers 
"Fragmentation and Path MTU Discovery", but doesn't have a definition for Link 
MTU.

Shuping> Yes, it is true. So we add the cases of MPLS and IPv6 as well.



146 Path MTU, or PMTU: The minimum link MTU of all the links in a path
147 between a source node and a destination node. In the scope of
148 this document, this is also called SR-PMTU for the SR paths and
149 policies. Note that the link MTU takes the SR overhead (label
150 stack or SRH) into consideration.

[major] This definition matches the one in rfc4821. Same comment as above about 
the Update in rfc8899 and the definition by reference.

Shuping> The RFC is updated to RFC8899. It simply states "See RFC8899". After 
this, I kept the following sentence "In the scope of this document..." to state 
the related part of this draft.

[minor] "Note that the link MTU takes the SR overhead (label stack or
SRH) into consideration."

This sentence seems unnecessary as Link MTU was defined above. Also, even 
though this is the only place where "SR overhead" is used, it may be worth 
defining it as a term for this document.

Shuping> Ok, I moved the SR overhead as a separate term.
" SR overhead: The SR-MPLS label stack or SRH. The link MTU takes the SR 
overhead into consideration."

152 4. SR-PMTU Definition for SR Policy

154 Segment Routing policy architecture is specified in [RFC9256]. An SR
155 Policy is associated with one or more candidate paths. A candidate
156 path is selected when it is valid and it is determined to be the best
157 path of the SR Policy. The selected path is referred to as the
158 "active path" of the SR policy. A candidate path is either dynamic,
159 explicit, or composite. The related concepts with the SR-PMTU
160 definition in this document are listed as follows.

[nit] s/Segment Routing policy architecture/The Segment Routing policy 
architecture

Shuping> Added.

[minor] The second and third sentences ("A candidate path is selected..."active 
path" of the SR policy.") are a word-by-word copy of text in rfc9256. Given 
that "active path" is only used here and in
§4.3 (where the same text is copied), it seems that you don't need to copy 
these sentences -- the reference to rfc9256 should be enough.

Shuping> Sure, these two sentences are removed and keep the reference RFC9256.

162 An explicit/dynamic candidate path is expressed as a Segment-List or
163 a set of Segment-Lists directly or by computation. If a candidate
164 path is associated with a set of Segment-Lists, each Segment-List is
165 associated with weight for weighted load balancing. The default
166 weight is 1.

168 A composite candidate path acts as a container for grouping SR
169 Policies. The composite candidate path construct enables the
170 combination of SR Policies, each with explicit candidate paths and/or
171 dynamic candidate paths with potentially different optimization
172 objectives and constraints, for load-balanced steering of packet
173 flows over its constituent SR Policies [RFC9256].

[minor] These two paragraphs are the same as in §2.2/rfc9256. Does the text 
need to be copied here? Even though the previous paragraph said that "related 
concepts with the SR-PMTU definition", that is not the case.

Shuping> These two paragraphs aim to explain the concepts of " An 
explicit/dynamic candidate path " and " A composite candidate path ". The first 
paragraph is not exactly the same as RFC9256. So the first paragraph can be 
kept.
The second paragraph is indeed the same, where the reference to RFC9256 is also 
added. I have changed the second paragraph as " A composite candidate path is 
defined in RFC9256."


175 4.1. SR-PMTU of a Segment List

177 A Segment-List represents a specific source-routed path to send
178 traffic from the headend to the endpoint of the corresponding SR
179 policy [RFC9256]. The SR-PMTU of a segment list is defined as the
180 minimum link MTU of all the links in a path between a source node and
181 a destination node. Refer Section 5.2 for specific handling for
182 Node, Adjacency and Binding SID (as well as their combinations)

[nit] s/link MTU/Link MTU/g

To refer to the definition.

Shuping>Changed.

[nit] s/Refer Section 5.2/Refer to Section 5.2

Shuping>Changed.



184 4.2. SR-PMTU of a Candidate Path
...
196 In the case of a composite candidate path, then the SR-PMTU of the
197 composite candidate path is defined as the minimum SR-PMTU of all the
198 constituent SR Policies of this composite candidate path. The SR-
199 PMTU of each SR Policy is defined in Section 4.3.

[nit] s/In the case of a composite candidate path, then the SR-PMTU of
the composite candidate path is defined as the minimum SR-PMTU of all
the constituent SR Policies of this composite candidate path./In the
case of a composite candidate path, then the SR-PMTU is defined as the
minimum SR-PMTU of all the constituent SR Policies.

Shuping>Changed.

...
239 5.1. Link MTU Collection

241 SR-PMTU is defined as the minimum link MTU of all the links in a path
242 between a source node and a destination node. The link MTU needs to
243 be first collected. The link MTU can be collected through various
244 protocols such as IGP [I-D.hu-lsr-igp-path-mtu] and BGP-LS
245 [I-D.ietf-idr-bgp-ls-link-mtu], etc.

[nit] The definition from §4.1 is repeated here. The problem with
repeating normative text (or any text) is that it may fall out of
sync. A better solution is to reference it.

Suggestion>

The SR-PMTU is defined in function of the Link MTU (Section 4.1). The
Link MTU can be collected through various mechanisms such as the ones
defined in [I-D.hu-lsr-igp-path-mtu] and [I-D.ietf-idr-bgp-ls-link-mtu].

Shuping>Changed as suggested, thank you.


247 5.2. SR-PMTU Computation

249 The collected link MTU of all the related links are sent to the
250 network controller where the SR-PMTU is computed. Depending upon the
251 path type, the computation methods are different, which are described
252 in the following subsections.

[major] Figure 1 and §5.1 imply that the collection can be done "in
the network": using an IGP and collecting the information at the
headend. But the description above only talks about using a
controller. Either the general framework, the examples in §5.1, or
this section need to be updated.

Is using a controller the only valid place to compute the SR-PMTU?

Shuping> " or the headend " is added into the sentence, so it is like "The 
collected link MTU of all the related links are sent to the network controller 
or the headend where the SR-PMTU is computed."

254 5.2.1. Loose TE Path

256 In a loose TE path [RFC7855], only Node SIDs are used along the path.
257 Between two adjacent Node SIDs, generally, there are equal-cost
258 multipaths (ECMP). The SR-PMTU of the loose TE path is computed by
259 finding out the minimum SR-PMTU of all the ECMPs between two adjacent
260 Node SIDs along the loose TE path.

[] It's sad that none of the SR documents talk about loose and strict paths. :-(

The use of the SR-PMTU is not limited to TE applications. Reading
into §3.3/rfc7855, the TE requirements refer explicitly to loose and
strict route options...so I think we can safely use "loose/strict
path" (no TE).

Shuping> "TE" is removed.


[minor] s/Node SID/Node-SID/g [rfc8402]
Shuping>Changed

[minor] s/finding out/finding/g
Shuping>Changed


262 Note that an implementation could maintain the SR-PMTU value
263 associated with Node SIDs at the time of best path computation. The
264 details of which are out of the scope of this document.

[?] I don't know what you mean by "an implementation could maintain".

Shuping> I have removed the paragraph.


266 5.2.2. Strict TE Path

268 In a strict TE path [RFC7855], only Adj SIDs are used along the path.
269 Since the link MTU of all the links being indicated by the Adj SIDs
270 of the strict TE path are known to the network controller, the SR-
271 PMTU of the strict SR-TE path is computed by finding out the minimum
272 link MTU of all the links in the strict SR-TE path between its source
273 node and destination node.

[minor] s/Adj SID/Adj-SID/g [rfc8402]
Shuping>Changed


275 5.2.3. Mixed Path

277 In a mixed path, both Node SIDs and Adj SIDs are used along the path.
278 The PMTU of the mixed TE path is computed by finding out the minimum
279 value of all the ECMPs between two adjacent Node SIDs and the link
280 MTU of all the links indicated by the Adj SIDs.

[major] s/minimum value of all the ECMPs/minimum SR-PMTU of all the ECMPs
Shuping>Changed


282 5.2.4. Binding Path

284 The Binding SID (BSID) [RFC8402] is bound to an SR Policy,
285 instantiation of which may involve a list of SIDs. The SR-PMTU of
286 the binding path is the same as that of an SR Policy as specified in
287 the above section modulo that it also includes the encapsulation
288 overhead associated with it (i.e. in case of SR-MPLS, the additional
289 label stack pushed in case of SR-MPLS and the outer IPv6 header with
290 its own SRH in case of SRv6). This is done to make sure the headend
291 of the SR path that includes a BSID is able to compute the SR-PMTU
292 correctly by taking the correct SR-PMTU of the binding path into
293 consideration along with other SIDs in the SR path.

[nit] s/in case of SR-MPLS, the additional/the additional
Shuping>Changed


295 5.2.5. TI-LFA

297 Topology Independent Loop-free Alternate Fast Re-route (TI-LFA)
298 [I-D.ietf-rtgwg-segment-routing-ti-lfa], aimed at providing
299 protection of node and adjacency segments within the SR framework.
300 The repair path is to pre-compute SPT_new(R,X) for each destination,
301 that is, the Shortest Path Tree rooted at node R in the state of the
302 network after the resource X has failed. An implementation is free
303 to use any local optimization to provide smaller SID lists by
304 combining Node SIDs and Adjacency SIDs. In addition, the usage of
305 Node-SIDs allows to maximize ECMPs over the repair path. Note that
306 while the PMTU of repair path might be different from the original
307 path, which could lead to fragmentation while the repair path is in
308 use. When the controller has computed the new path, its new PMTU
309 would be updated to the headend.

[nit] s/while the PMTU of/the PMTU of
Shuping>Changed

[] "...which could lead to fragmentation while the repair path is in
use. When the controller has computed the new path, its new PMTU
would be updated to the headend."

This sounds counter to the motivation (§1.1): "avoid fragmentation"
and "When fragmentation is unavoidable, the ability to do it correctly
at the headend." When the repair path is in use neither goals are
met.

Shuping> The quoted text is about when PMTU calculation does not take repair 
paths into consideration, or when the repair paths are calculated without 
considering the original PMTU.
This paragraph is changed as
" Topology Independent Loop-free Alternate Fast Re-route (TI-LFA)
[I-D.ietf-rtgwg-segment-routing-ti-lfa], aims to provide protection
for node and adjacency segments within the SR framework. The PMTU of
the repair path might be different from the original path's, which
could lead to fragmentation while the repair path is in use."

311 Note that it is possible for the headend implementation to take an
312 FRR overhead into consideration when determining if fragmentation
313 would be needed for the SR Path with TI-LFA enabled. If this is
314 used, an implementation SHOULD allow the value to be configured by an
315 operator.

[major] "SHOULD allow the value to be configured"

Which value? In other cases the information seems to be collected
from the path, not manually configured. Why is this change needed?

Why is the ability to configure not required? Or is the configuration
beyond the collection?

Shuping> This paragraph is changed as
" To avoid fragmentation, it is possible for the headend (or controller)
to consider the FRR overhead when computing the SR-PMTU of the
original path."

317 5.2.6. Others

319 All other types of path can be considered here in future updates.

[] This section is superfluous.

Shuping> Has been removed.

321 5.3. SR-PMTU Enforcement
...
349 When there are multiple paths that can be selected, the one with the
350 highest SR-PMTU will be enforced in order to avoid fragmentation on
351 the headend.

[minor] s/multiple paths that can be selected, the one with the
highest SR-PMTU will be enforced/... used

Shuping> Changed.

...
356 5.4. Handling behaviors on the headend

358 After the SR-PMTU is computed and enforced on the headend, the
359 headend is going to perform the handling behaviors such as
360 encapsulation, fragmentation, etc. Note that this behavior is
361 similar to the existing behavior of MPLS and IPv6 dataplane.

[] Enforcing the SR-PMTU already implies fragmentation.

Suggestion>

After the SR-PMTU is computed, the headend performs the handling
behaviors such as encapsulation and fragmentation, if needed. Note
that this behavior is similar to the existing behaviors of MPLS
and IPv6 dataplane.

Shuping> Updated, thank you!

363 5.4.1. SR-PMTU Constraints and Optimization

365 Generally, considering its services being carried, the operators set
366 an SR-PMTU limit aiming for a proper path selection that fulfills
367 packet size requirements hence avoiding fragmentation. Furthermore,
368 the encapsulation on the headend will introduce the overhead on top
369 of the packet to be encapsulated. Generally, the encapsulation
370 overhead has to be estimated according to the possible path hops and
371 sometimes the repair paths. Therefore, the SR-PMTU constraint is set
372 considering both the carried services and the encapsulation overhead.

[nit] s/considering its services being carried/considering the
services being carried

Shuping> Changed.


[] "the operators set an SR-PMTU limit"

In §5.1 the collection is implied to be done through network
mechanisms (IGP, BGP-LS, etc.). Perhaps include the possibility of
manual configuration.

The text should be updated to reflect the in-network collection. For
example, an IGP is going to report the MTU on an interface without
knowledge of the services.

Shuping> As above suggested, this paragraph is further changed to
" The SR-PMTU of a segment list is defined as the
minimum link MTU of all the links in a path, see Section 4.1. The
Link MTU can be collected in network through various mechanisms such as the ones
defined in [I-D.hu-lsr-igp-path-mtu] and [I-D.ietf-idr-bgp-ls-link-mtu] without 
the knowledge of the services."



374 When SR-PMTU-based path optimization is done, PCE will select the
375 path with the highest SR-PMTU among all the possible paths.

377 Even if the SR-PMTU is not considered by the PCE at the time of path
378 computation, the computed SR-PMTU is useful at the headend for the
379 reasons already stated in Section 1.1.

381 Once the SR-PMTU constraint is set on the headend, it is supposed to
382 be the lowest bound of the SR-PMTUs of all the paths being computed
383 locally or enforced by the controller in order to avoid
384 fragmentation.

[] The paragraph above says that the "lowest bound of the SR-PMTUs" is
used, but two paragraphs above it says that the "PCE will select the
path with the highest SR-PMTU".

What's the difference between the "SR-PMTU constraint" and the SR-PMTU
of the SR policy? I know I'm missing something, I just don't know
what it is. Is the "SR-PMTU limit" mentioned in the first paragraph
of this section the same as the "SR-PMTU constraint"?

Shuping> SR-PMTU constraint is like a hop-limit or delay constraint (i.e. 
during path computation make sure this limit is not crossed) whereas SR-PMTU is 
the actual value. The limit and constraint are the same things and perhaps we 
could use constraint. In the first paragraph, the "SR-PMTU limit" is changed to 
"SR-PMTU constraint".


386 5.4.2. Fragmentation processing

388 If the SR-PMTU of all the paths being computed locally or enforced by
389 the controller is smaller than the SR-PMTU constraint set on the
390 headend, the fragmentation will have to be handled. If fragmentation
391 is not possible, the headend could generate the ICMP messages to
392 notify the traffic source.

[major] "the headend could generate the ICMP messages"

Please reference the ICMP RFC.

Shuping> RFC4443 is added.


394 Over this selected path, on the headend, the packets are fragmented
395 in order to guarantee the size of the encapsulated packets is smaller
396 than the PMTU of the selected path.

[] This paragraph seems to say the same thing as the previous one.

Shuping> This paragraph is meant to say after the path is selected, and the 
fragmentation actions over this selected path

...
406 7. Security Considerations

408 [RFC9256] specifies in detail the SR Policy construct (introduced
409 [RFC8402]) and its security consideration. The additional SR-MTU
410 attribute information can be sensitive in some deployments and could
411 be used to influence SR path setup and selection with adverse effect.
412 The protocol extensions that include SR-PMTU need to take this into
413 consideration. This document does not define any new protocol
414 extensions and thus does not introduce any further security
415 considerations.

[nit] s/introduced [RFC8402]/introduced in [RFC8402]
Shuping> Changed.

[nit] s/security consideration/security considerations
Shuping> Changed.


...
474 10.2. Informative References

476 [RFC4821] Mathis, M. and J. Heffner, "Packetization Layer Path MTU
477 Discovery", RFC 4821, DOI 10.17487/RFC4821, March 2007,
478 <https://www.rfc-editor.org/info/rfc4821>.

[major] This reference should be Normative because the terminology
comes from it. Look at other related comments.

Shuping> Now RFC4821 has been removed and changed to RFC8899 according to the 
suggestions above. Now the RFC8899 has been moved to the Normative part.

...
486 [RFC8201] McCann, J., Deering, S., Mogul, J., and R. Hinden, Ed.,
487 "Path MTU Discovery for IP version 6", STD 87, RFC 8201,
488 DOI 10.17487/RFC8201, July 2017,
489 <https://www.rfc-editor.org/info/rfc8201>.

[major] This reference should be Normative.

Shuping> moved.

...
505 [I-D.ietf-idr-sr-policy-path-mtu]
506 Li, C., Zhu, Y., El Sawaf, A., and Z. Li, "Segment Routing
507 Path MTU in BGP", Work in Progress, Internet-Draft, draft-
508 ietf-idr-sr-policy-path-mtu-08, 19 October 2023,
509 <https://datatracker.ietf.org/doc/html/draft-ietf-idr-sr-
510 policy-path-mtu-08>.

[major] This reference should be Normative because it is where the
Path MTU is added to the SR Policy.

Shuping> The IDR document is only talking about SR Policy Safi NLRI though they 
use the SR Policy structure. If some text related to that is needed, it is 
better to add this in this document rather than a normative reference.

[EoR-03]

Best regards,
Shuping
_______________________________________________
spring mailing list -- spring@ietf.org
To unsubscribe send an email to spring-le...@ietf.org

Reply via email to