[Idr] Re: Shepherd's review of draft-ietf-idr-sr-policy-path-mtu-09

[Cheng Li <[e-mail]>]

Hi Sue,

Sorry for my delay, and thank you for your valuable review comments! Please see my reply below.

We have updated the draft according to your suggestions! Thank you again!


URL:      https://www.ietf.org/archive/id/draft-ietf-idr-sr-policy-path-mtu-10.txt

Status:   https://datatracker.ietf.org/doc/draft-ietf-idr-sr-policy-path-mtu/

HTMLized: https://datatracker.ietf.org/doc/html/draft-ietf-idr-sr-policy-path-mtu

Diff:     https://author-tools.ietf.org/iddiff?url2=draft-ietf-idr-sr-policy-path-mtu-10

Respect,
Cheng


From: Susan Hares <[e-mail]>
Sent: Tuesday, September 3, 2024 8:47 PM
To: [e-mail]
Cc: Cheng Li <[e-mail]>; [e-mail]; [e-mail]; Lizhenbin <[e-mail]>
Subject: Shepherd's review of draft-ietf-idr-sr-policy-path-mtu-09

Cheng, YongQing, Ahmed, and Robin:


Below is a Shepherd's Review on draft-ietf-idr-sr-policy-path-mtu-09.
This Shepherd report suggests minor text changes for technical clarity
plus a few minor editorial changes.

Due to the quality of the draft, I have a few questions about the status:



  1.  Do you plan any technical changes?
[Cheng]We may not have technical change in our plan.


  1.  Did your implementation calculate the PATH MTU
Based on BGP-LS data or data from other sources?
[Cheng]It can be BGP-LS or other source.



  1.  Does this WG draft have two implementations?
   I noticed you mentioned one in the text.
[Cheng]I will send an email to our list, to request for implementation report after updating this revision.



  1.  Are you planning WG LC in the next 3 months?

[Cheng]If two implementations are reported, I believe we can issue a WGLC for this document.

A version (-10) addresses the issue below, and
2 implementations are required before WG LC.

Thank you for your hard work on this draft!
[Cheng]Thank you so much for your review and suggestions!

Cheerily, Sue Hares


==============

Shepherd Review for:
draft-ietf-idr-sr-policy-path-mtu-09

status: WG Draft
Status: Proposed Standard
version: revise for -10 suggested
Implementation status: 1 implementation in draft (? Any more)
WG LC:


Technical:

Issue-1: Abstract, Missing link with RFC9012

Old text:/
   An SR policy is a set of candidate SR paths consisting of one or more
   segment lists with necessary path attributes. However, the path
   maximum transmission unit (MTU) information for SR path is not
   available in the SR policy since the SR does not require signaling.
   This document defines extensions to BGP to distribute path MTU
   information within SR policies./

New text:/
   An SR policy is a set of SR Policy candidate paths consisting of one or
   more segments with the appropriate SR path attributes. BGP distributes
   each SR Policy candidate path as combination of an prefix plus a
   the BGP Tunnel Encapsulation(Tunnel-Encaps) attribute containing
   an SR Policy Tunnel TLV with information on the SR Policy candidate
   path as a tunnel.  However, the path maximum
   transmission unit (MTU) information for a segment list for SR path
   is not currently passed in the BGP Tunnel-Encaps attribute. .
   This document defines extensions to BGP to distribute path MTU
   information within SR policies./

[Cheng]Thank you, updated accordingly!

Issue-2: Introduction, paragraph 1,

Old text:/
   In order to distribute SR policies to the headend,
   [I-D.ietf-idr-sr-policy-safi] specifies a mechanism by using BGP./

New text:/
   In order to distribute SR policies to the headend,
   [I-D.ietf-idr-sr-policy-safi] specifies a BGP mechanism
   to pass SR Policies and Candidate SR Policies in BGP UPDATE
   message.  Each SR Candidate Path is passed as combination of a
   specific type of NLRI and BGP Tunnel Encapsulation Attribute (Tunnel-Encaps)
   with SR Policy Tunnel type tunnel.  The NLRI must contain
   either be the IPv4 Unicast AFI with SR Policy SAFI (AFI=1/SAFI=73),
   the IPv6 Unicast AFI with the SR Policy SAFI (AFI=2/SAFI=73)./

[Cheng]Ack, thank you so much for providing text even! Respect!

2. Introduction, paragraph 4, ingress needs to be ingress router

Old text:/ But the ingress still needs to
   examine the packet size for dropping too large packets to avoid
   malicious traffic or error traffic.  Also, the packet size may
   exceeds the PMTU because of the new encapsulation of SR-MPLS or SRv6
   packet at the ingress. /

New text:/  But the ingress router still needs to
   examine the packet size for dropping too large packets to avoid
   malicious traffic or error traffic.  Also, the packet size may
   exceeds the PMTU because of encapsulation of the original packet in
   SR-MPLS or SRv6 packet at the ingress router./
[Cheng]Ack. Thanks.


3. Introduction, paragraph 4, last sentence.

Old text:/
   However, the path maximum transmission unit (MTU)
   information for SR path is not available since the SR does not
   require signaling.

   This document defines extensions to BGP to distribute path MTU
   information within SR policies.  The Link MTU information can be
   obtained via BGP-LS [I-D.ietf-idr-bgp-ls-link-mtu] or some other
   means.  With the Link MTU, the controller can compute the PMTU and
   convey the information via the BGP SR policy./

New text:/
   However, the path maximum transmission unit (MTU)
   information for SR path is not currently distributed in the
   BGP Tunnel-Encaps attribute TLV for the SR Policy Tunnel.
   This document defines a new sub-TLV for the BGP Tunnel-Encaps
   attribute for the SR Policy Tunnel type to specify Maximum Path
   MTU for a Segment list (Sub-TLV)./

   The Maximum Path MTU can be calculated as the maximum of
   individual Link MTU information. The Link MTU information can be
   obtained via BGP-LS [I-D.ietf-idr-bgp-ls-link-mtu] or some other
   means.  based on all Link MTUs, the controller can compute the PMTU and
   convey the information via the BGP SR policy./

[Cheng]ACK. Updated accordingly.


 4. Section 3.1, length definition


 Old text:/
   Length: the total length of the value field not including Type and
   Length fields. /

New text:/   Length: the total length in octets the value field not including Type and
   Length fields.  The value must be 6./


5. Please add a security section.

This security section should refer to [draft-ietf-idr-sr-policy-safi].
One addition needs to be made in this text.

This security section should specifically state that the new sub-TLV PATH MTU
contains critical information for the network. Critical information is a place
where modifying the information could cause problems. Care should be taken to
protect the collection of Link MTU, the generation of PATH MTU, and the
distribution of PATH MTU.

[Cheng]Thanks, added as your suggestion.


Editorial:

  1. Introduction, paragraph 3, English tense + spelling

  current:/
   [RFC3209] specify the mechanism of MTU signaling in RSVP.
   Likewise, SRv6 pakcets will be dropped if the
   packet size is larger than path MTU, since IPv6 packet can not be
   fragmented on transmission [RFC8200] ./


  new:/
  [RFC3209] specifies the mechanism of MTU signaling in RSVP.
  Similarly, the SRv6 packets will be dropped if the packet
  size is larger than the path MTU, since IPv6 packet cannot be
   fragmented on transmission [RFC8200]./

[Cheng]Thank you so much!