# OPSDIR review of draft-ietf-mpls-rfc6374-sr-11

I have reviewed this document as part of the Operational directorate's ongoing effort to review all IETF documents being processed by the IESG.  These comments were written with the intent of improving the operational aspects of the IETF drafts. Comments that are not addressed in the last call may be included in AD reviews during the IESG review.  Document editors and WG chairs should treat these comments just like any other last-call comments.

The document describes the Performance Delay and Loss Measurement techniques in the SR-MPLS network. 

The document does not include an explicit manageability & operational considerations section. Authors could consider adding it. Both RFC 6374 and RFC 7876 have it and it would make sense to call out considerations that are specific to SR-MPLS (if any). 

## Comments

- Perhaps it was a mistake to include "rfc6374" in the filename. 

- The document uses the term "policies" without any qualifier. Suggest to use the explicit term SR Policy.  

- Section 1, "Segment Routing (SR) leverages the source routing paradigm for Software Defined Networks (SDNs)"; what is the purpose of "for SDN"? SDN term is not used in this document and is not a common framing in the spring RFCs either. 

- Section 2.2, consider adding references where appropriate. Add LSE, DM, LM

- Section 3, "The packet loss measurement using Alternate-Marking Method defined in [RFC9341] MAY use Block Number (BN) for data correlation."; Since you are restating from RFC 9341, either do a quote or avoid using normative MAY. (also in section 6.4)

- Section 4.1.2, In "The query messages MUST be transmitted using each SL of the SR-MPLS Policy Candidate-Path", IMHO each SL is unclear. The next sentence "A query message for an end-to-end SR-MPLS Policy, used for delay and/or loss measurement, contains SR-MPLS label stack of the Segment List(s) of the Candidate-Path, with the G-ACh Label (GAL) at the bottom of the stack (with S=1) as shown in Figure 2" gives an impression that a single query message contains label stack of all segment list. How about "every SL" in the first sentence and rephrase the 2nd? Also, it would make sense for Figure 2 to be more explicit on what 0x0001, version, reserved, and channel Type is and what follows it (ACH).

- Section 4.2.2, I don't think that "[I-D.ietf-pce-sr-bidir-path]" is the correct reference for "return SR-MPLS path". I suggest removing the reference. If you prefer you can add a separate sentence with the role of PCE in SR bidirectional path setup. 

- Section 4.2.3, same comment as above! Also, I wonder if this is the correct usage of the normative keyword in "MUST NOT be punted out of the fast path". BTW I don't see similar text in RFC 6374 FWIW. It would also be a good idea to add an example showing a label stack with forward and reverse paths. 
 
- Section 5.1, what do you mean by the same value in "the same MPLS DM ACH value MUST be used"? Is it value 0x000C for delay measurement? The use of MUST feels off. (similar phrases in 6.1 and 6.2).

- Section 6.3, should this document explicitly state that PSID MUST be used for LM? Should Figure 3 also include other labels, this gives the impression that it is just PSID and GAL. Should add text to explain how the loss measurement works when the same PSID is used for all SL and candidate paths and the packet loss accounting can be done on the SR policy level?

- Section 7.1, the text "The Return Path TLV is a Mandatory TLV Type." gives the impression that the TLV is mandatory. Perhaps say that the TLV belongs to the Mandatory subspace as per RFC6374? 

- Section 7.1.1, I don't think that the PCE document is the correct reference to Binding SID as currently phrased. Either use RFC 8402 and phrase the text in terms of the role of PCE in BSID allocation.  

- Section 7.2, clearly states what is the meaning of the R flag. I am also unsure on why it is needed. 

- Section 8, talks about anycast in the context of ECMP. But an SR path made up of node SIDs could also have ECMP between the nodes, why is that not called out? Also, what do you mean by "sweeping of entropy label"? You say that "loss measurement for different ECMP paths of an SR-MPLS Policy are outside the scope", what about delay?  

- Section 12, IANA no longer prefers the term "sub-registry", they ask us to use "registry group" and "registry" instead. 

## Nits

- Suggested rewording of the abstract as - “Segment Routing (SR) leverages the source routing paradigm. SR applies to the Multiprotocol Label Switching data plane (SR-MPLS) as specified in RFC 8402. RFC 6374 and RFC 7876 specify protocol mechanisms to enable efficient and accurate measurement of packet loss, one-way and two-way delay, as well as related metrics such as delay variation in MPLS networks. RFC 9341 defines the Alternate-Marking Method using Block Number (BN) as a data correlation mechanism for packet loss measurement. This document utilizes mechanisms from RFC 6374, RFC 7876, and RFC 9341 for performance delay and loss measurements in SR-MPLS networks, covering both links and end-to-end SR-MPLS paths, including SR Policies.”

- s/requirements to measure network performance/requirements for measuring network performance/

- s/an UDP return path/a UDP return path/

- s/well-suited in SR-MPLS/well-suited to SR-MPLS/

- s/are sent as following:/are sent as follows:/

- s/to collect transmit and receive/to collect, transmit, and receive/g

- s/include or more optional TLVs/include one or more optional TLVs/

- Please run through a grammar check, various issues such as article missing.

Thanks! 
Dhruv