Path Segment Identifier in MPLS Based Segment Routing Network
draft-ietf-spring-mpls-path-segment-22

# Internet AD comments for draft-ietf-spring-mpls-path-segment-17
CC @ekline

* comment syntax:
  - https://github.com/mnot/ietf-comments/blob/main/format.md

* "Handling Ballot Positions":
  - https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/

## Nits

### S1.2

* "a part of the a path" ->
  "a part of the path"

# John Scudder, RTG AD, comments for draft-ietf-spring-mpls-path-segment-19
CC @jgscudder

Thanks for this document. I have a few non-blocking comments, below.

## COMMENTS

### Applicability to SRv6

Since SRv6 also doesn't guarantee to propagate the SID list all the way to the egress node, might PSID functionality potentially also be required in SRv6? The way it's defined in this document, it's tightly bound to SR-MPLS, might this create added complication in the future if an SRv6 instantiation is desired?

I'm not at all suggesting you define such an SRv6 instantiation in this document. At most, maybe you would revise some of the text in Section 2, to leave open the possibility for such an instantiation in the future.

### Terminology

It's probably worth saying somewhere that you assume knowledge of the definitions in Section 2 of RFC 8402. For example, "Local Segment" isn't defined in your document, presumably you're relying on the RFC 8402 definition. I didn't carefully track whether there are other terms similarly in need of definition.

### Section 1.2, orphan and near-orphan definitions

Some abbreviations are never used, maybe these were used in a previous version of the document, I don't know. Anyway, I think they can be removed from the section now.

Some abbreviations are used only once. In my opinion, it would be better to just spell them out where used, instead of introducing an abbreviation and then only using it once.

Never used:
DM
LM
SL

Used once:
PM
SRLB

Used twice, in the same paragraph, so IMO not needed to be listed in the abbreviations section:
MSD

### Section 2, "below MPLS label"

I don't understand what precisely you mean by, "The addition of the PSID will require the egress to read and process the PSID label in addition to the regular processing (such as a below MPLS label or the MPLS payload)." What's a "below MPLS label"? I also think "MPLS payload", would be clearer as just "payload" -- using "MPLS" as an adjective doesn't seem to add value in this context. Ironically, I would find the sentence perfectly clear if the portion inside parentheses were removed.

### Section 2, is PSID always the bottom label, or not?

Figure 1 implies PSID is always the bottom label. A few paragraphs about that, though, you have "If a PSID is the bottom label, the S bit MUST be set." The "if" implies the PSID doesn't have to be the bottom level.

It seems that either Figure 1 or the quoted text should be updated, to make them consistent. Based on Section 3.4, I guess the text is right and Figure 1 is wrong, or at least misleading. One possible fix could be,

OLD:
The label stack with PSID is shown in Figure 1:

NEW:
An example label stack with PSID is shown in Figure 1:

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. 

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments

Thanks for working on this specification. I have no objection from transport protocol point of view.

# Éric Vyncke, INT AD, comments for draft-ietf-spring-mpls-path-segment-17

Thank you for the work put into this document.

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education), and some nits.

Special thanks to Bruno Decraene (one WG chair) for the shepherd's detailed write-up including the WG consensus *and* the justification of the intended status.

I hope that this review helps to improve the document,

Regards,

-éric

# COMMENTS (non-blocking)

## Abstract and title

If the goal is to identify a specific SR path, then why not adding "identification" in the title and the abstract ? PSID is used in the rest of the document and would benefit from being used also in the abstract/title.

In `A sub-set of segments from the segment list cannot distinguish one SR path from another as they may be partially congruent`, suggest s/cannot distinguish/cannot be leveraged to distinguish/

## Lack of extensibility ?

As PSID seems to be solely identified by its position (the last label), does it prevent the use of a similar mechanism for future extension of SR list ? Should there be a syntax in this last label to identify it as a PSID?

## Section 2

`However, one PSID can be used to identify multiple Segment Lists in some use cases if needed.` seems like an oxymoron to me (bear with my lack of expertise in SR-MPLS)... Usually "identify" refers to a single identity, i.e., a single segment list in this case.


## Section 2.1

Suggest to introduce ECMP in the title and not in the 3rd paragraph *after* its first use.

## Section 3

Please use PSID rather than "path segment".

## Section 3.1

s/existing implementation on the egress node can be leveraged for measuring/existing implementation on the egress node can leverage PSID for measuring/ ?

## Section 3.2

An informative reference will be useful to the reader about `obile backhaul transport networks, there are requirements to support bidirectional paths`.

## Section 3.4

What is meant by `in a trusted domain` ? Is it rather a common operation ?

To be honest, I was about to ballot a DISCUSS on the following point... but I am trusting the RTG AD ;), how can a global PSID be used for perf measurement (and other use cases) when the sub-domains can expenad the BSID at their will ? I.e., PSID does not represent a real hop-by-hop path but more a sub-domain to sub-domain path...

## Section 4

`The intermediate nodes of this path will not learn it.` is it true for the first node ? I.e., it knows where it is coming from and have the full view on the segment list.

## Operational considerations

The last paragraph of section 4 should rather be in an "operational considerations" section.

# NITS (non-blocking / cosmetic)

## Section 1.2

s/A sub-path is a part of the a path/A sub-path is a part of a path/ ?

## Section 2

s/intermedate node/intermediate node/

## Section 3

s/can leveage/can leverage/

Many thanks for addressing the previous discuss that can be found in the history in such a prompt manner!

# GEN AD review of draft-ietf-spring-mpls-path-segment-20

CC @larseggert

Thanks to Stewart Bryant for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/8Y8zdymWO5b2jAqASpybK3TbVHU).

## Comments

### Section 1, paragraph 3
```
     However, to support various use-cases in SR-MPLS networks, such as
     Performance MeasurementSection 3.1, bidirectional path Section 3.2,
     and end-to-end 1+1 path protection (Live-Live case) Section 3.3, the
     ability to implement path identification on the egress node is a pre-
     requisite.
```
This paragraph is really incomprehensible.

### Section 2, paragraph 2
```
     A PSID is used to identify a Segment List.  However, one PSID can be
     used to identify multiple Segment Lists in some use cases if needed.
     For example, one single PSID MAY be used to identify some or all
     Segment lists in a Candidate path or an SR policy, if an operator
     would like to aggregate these Segment Lists in operation.
```
This is confusing. Either a PSID (uniquely?) identifies *one* segment
list, or it does not. If it can identify multiple (disjoint?) segment
lists, the details of how this works need to be described much more
clearly.

### Section 3, paragraph 0
```
  3.  Use cases
```
I assume this section is informational? It would be good to say so
explicitly.

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### URLs

These URLs in the document did not return content:

 * https://www.fiberhome.com/operator/product/products/294.aspx.htm

### Grammar/style

#### Section 3.2, paragraph 3
```
 trusted domain that spans three sub-domains (Access, Aggregation and Core do
                                 ^^^^^^^^^^^
```
This word is normally spelled as one.

#### Section 3.2, paragraph 4
```
 of three sub-paths, one in each sub-domain (sub-path (A->B), sub-path (B->C
                                 ^^^^^^^^^^
```
This word is normally spelled as one.

#### Section 3.3, paragraph 1
```
ub-path is associated with a BSID and a s-PSID. The SID list of the end-to-e
                                      ^
```
Use "an" instead of "a" if the following word starts with a vowel sound, e.g.
"an article", "an hour".

#### Section 3.4, paragraph 8
```
 PSID from an egress nodes to an ingress nodes is performed within an SR tru
                              ^^^^^^^^^^^^^^^^
```
The plural noun "nodes" cannot be used with the article "an".

#### Section 5.1, paragraph 3
```
auses have been implemented in above mentioned New H3C Products(running Versi
                               ^^^^^^^^^^^^^^^
```
The adjective "above-mentioned" is spelled with a hyphen.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

Hi,

Thanks for this document.  I found is pretty easy to read, and have a couple of very minor (editorial) comments:

Nit level comments:

(1) p 6, sec 3.1.  PSID for Performance Measurement

   PSID can also be used for In-situ OAM[RFC9197] for SR-MPLS to
   identify the SR Path associated with the in-situ data fields in the
   data packets on the egress node.

   PSID can also be used for In-band PM for SR-MPLS to identify the SR
   Path associated with the collected performance metrics.

Just cosmetic, but given the repetition, perhaps put the last 3 sentences in this section into a list, e.g., starting with "PSID can also be used:"


(2) p 7, sec 3.4.  Nesting of PSIDs

   Binding SID (BSID) [RFC8402] can be used for SID list compression.
   With BSID, an end-to-end SR path in a trusted domain can be split                                                                                                                         
   into several sub-paths, each sub-path is identified by a BSID.  Then                                                                                                                      
   an end-to-end SR path can be identified by a list of BSIDs,                                                                                                                                                                                               
   therefore, it can provide better scalability.                                                                                                                                                                                                             
   BSID and PSID can be combined to achieve both sub-path and end-to-end                                                                                                                                                                                     
   path monitoring.  A reference model for such a combination in                                                                                                                                                                                             
   (Figure 2) shows an end-to-end path (A->D) in a trusted domain that                                                                                                                                                                                       
   spans three sub-domains (Access, Aggregation and Core domain) and                                                                                                                                                                                         
   consists of three sub-paths, one in each sub-domain (sub-path (A->B),                                                                                                                                                                                     
   sub-path (B->C) and sub-path (C->D)).  Each sub-path is associated                                                                                                                                                                                        
   with a BSID and a s-PSID.                                                                                                                                                                                                                                 
                                                                                                                                                                                                                                                             
As a minor nit, s-PSID is used above, but not defined until the following paragraph.

Regards,
Rob