Re: [mpls] Shepherd review of draft-ietf-mpls-spring-inter-domain-oam-10

[Shraddha Hegde <[e-mail]>]

Adrian,

Thank you for careful review and comments. Version -11 addresses your comments
Pls see inline for responses.


Juniper Business Use Only
-----Original Message-----
From: Adrian Farrel <[e-mail]>
Sent: Wednesday, February 7, 2024 10:23 PM
To: [e-mail]; [e-mail]
Cc: [e-mail]
Subject: RE: [mpls] Shepherd review of draft-ietf-mpls-spring-inter-domain-oam-10

[External Email. Be cautious of content]


Oh, I should add...

Please be sure to clean up all of the nits reported by idnits https://urldefense.com/v3/__https://author-tools.ietf.org/api/idnits?url=https:**Awww.ietf.org*archive*id__;Ly8vLw!!NEt6yMaO-gk!B7Ff8868TaqCp1zMGGfPx37gxIVnvcUdZgT9M5drdwRi3aDww_0OxUTpWY_su3WVw7CS63GNDuYRdgSqJw$
/draft-ietf-mpls-spring-inter-domain-oam-10.txt

The only things you can get away with are the things that look like references, but aren't.
<SH> Worked on IDnits and the above mentioned ones are the only ones remaining.

Cheers,
Adrian

-----Original Message-----
From: mpls <[e-mail]> On Behalf Of Adrian Farrel
Sent: 06 February 2024 21:18
To: [e-mail]
Cc: [e-mail]
Subject: [mpls] Shepherd review of
draft-ietf-mpls-spring-inter-domain-oam-10

Hi,

I have taken over as shepherd for your document. That necessitates me jumping in with a "slightly late" review. But I guess all reviews are helpful.

If you can spin a new version, I can pass the document on to the AD.

Thanks,
Adrian

===

Throughout the document there are a number of cases of missing spaces before or after round brackets or commas. Please check and fix.

---
<SH> done
Abstract

s/Segment Routing (SR)/The Segment Routing (SR)/ s/Autonomous Systems(AS)/Autonomous Systems (ASes)/ s/with simple/with a simple/
<SH> Done
---

Please move the section on Requirements Language down to become 1.2.
<SH> Done

---

1.

I think this section really should start with some text and not leap in with a figure. Figure 1 is not referenced until the 3rd paragraph, so this should be easy to do.

<SH> done
The figure could use a key for:
AS
ASBR
P
PE
PMS

---
<SH> done

1. para 1

s/Autonomous Systems either/Autonomous Systems (ASes) either/ s/Segment Routing/Segment Routing (SR)/ s/Autonomous systems(AS)./ASes./ s/Identifiers(SID)/Identifiers (SIDs)/

---

1. para 2

s/with MPLS/with an MPLS/
s/Autonomous system/AS/
s/Autonomous Systems(AS)/ASes/

---
<SH> Done

1. para 3

s/(EPE-SID)/(EPE-SIDs)/
s/For example in Figure 1/For example, in Figure 1,/

---
<sH> done

1. para 3

   It is advantageous for operations to be
   able to perform LSP ping and traceroute procedures on these inter-AS
   SR-MPLS paths.

- s/operations/operators/  ???
- "It is advantageous" feels a bit unsupported. What are the advantages?
  Perhaps "... in order to detect and diagnose failed deliveries and to
  determine the actual path that traffic takes through the network."

---
<SH> Changed to below

"It is useful for operators to be able to perform LSP ping and traceroute
procedures on these inter-AS SR-MPLS paths, in order to detect and diagnose
 failed deliveries and to determine the actual path that traffic takes
 through the network."

1.

You have both 'traceroute' and 'Traceroute'. Please pick one and apply it to the whole document.

---
<SH> Done

1. para 4

   That is because there is no IP connectivity to the source address of
   the ping packet, which is in a different AS from the packet's
   destination.

I know what you mean, but as stated this is not completely true (and not always true, in any case). Maybe...

   That is because there might not always be IP connectivity from a
   responding node back to the source address of the ping packet when
   the responding node is in a different AS from the source of the ping.

---
<SH> Thanks for the text. I have updated.

1. para 5

s/out the MPLS/out MPLS/
s/requires PMS/requires the PMS/

---
<SH> done

1. para 5

   This mechanism is operationally
   very heavy and requires PMS to be capable of building a huge number
   of GRE tunnels, which may not be feasible.

Presumably...

   This mechanism is operationally
   very heavy and requires the PMS to be capable of building a huge
   number of GRE tunnels or installing the necessary static routes,
   which may not be feasible.

---
<SH> done

1. para 6

s/segment routing/SR/

---
<SH> done

1. para 7

s/[RFC7743] mechanism/The [RFC7743] mechanism/ s/in slow path/on the slow path/

---
<SH> done

1. para 8

   This mechanism uses
   MPLS path and no changes are required in the forwarding path.

I cannot parse this. Maybe s/MPLS path/MPLS LSPs/ ?
<SH> I mean to say the packet is being forwarded via MPLS so IP connectivity is not needed.
MPLS LSPs term will be too specific to LDP/RSVP I believe. SR RFCs don't use the term MPLS LSPs.

---

1. para 8

s/Reply path TLV/Reply Path TLV/   (twice)
s/segment routing/SR/  (please check the rest of the doc!)

---
<SH> done

1.1

Nothing wrong with what you have written, but RFC 8402 defines the term "SR domain" and distinguishes it from an "IGP domain". Upon opening your document, I was confused by your use of "domain" until:
- Tony Li put me straight
- I read section 1.1

I wonder whether there are ways you can make the distinction clearer sooner in the document.

---
<SH> I put a forward reference in the into the "domain definition" section. Found it difficult to move it earlier.

1.1

s/Autonomous System (AS)/AS/

---
<SH> done
2.

Please use Title Case for the section header.

---
<SH> Done

2.

Again, please don't begin the section with a figure.
The text needs to make reference to the figure.

---
<SH> Done
2.

s/Reply path TLV/a Reply Path TLV/
s/in echo reply/in an echo reply

---
<sH> done

3.

OLD
and PMS/Head-end
NEW
and a PMS/Head-end
END

---
<SH> Done

3.

s/Reply path TLV/Reply Path TLV/   (please check the rest of the doc)
s/Implementations may/Implementations MAY/  (twice) s/Command Line Interface(CLI)/Command Line Interface (CLI)/ sIPV4/IPv4/
s/dual- stack/dual-stack/

---
<SH> Done

4.

OLD
   The motivation has been to keep the definitions
   same as in [RFC9256] with minimal modifications if it is needed.
NEW
   The intention was to keep the definitions as close to
   those in [RFC9256] as possible, with modifications only when needed.
END

---
<SH> done

4.

s/segment sub-TLV/Segment Sub-TLV/   (please check whole doc)

---
<SH> done
4.2

Not all confusing to have TBD1, TBD3, and TBD4 (but no TBD2)  :-)

Aha! This is because you have a "TBA2" instead. Maybe change TBA to TBD.
Except, you also have a TBA1.

---
<SH> Assigned TBD1,TBD2, TBD3. Left TBA1, TBA2 as is. Hope that works for you.

4.1

   The following applies to the Type-1 Segment sub-TLV:

I think s/Type-1/Type-A/
Similar issues in 4.2 and 4.3.
Also, the section headings should (presumably) be s/Type A/Type-A/ etc.

---
<SH> done

4.1, 4.2, 4.3

Compare the requirements for the Reserved fields. Why are they different?

---
<SH> Updated to " MUST be set to zero when sending;
                MUST be ignored on receipt."
For all sections

4.2

When the Algo is present it is used to derive the Label. Is that the SID label? How does that conflict with the SID field (if present)?

Ditto 4.3

---
<SH> " When the MPLS SID field is present, it MUST be used for
      constructing the Reply Path"  This was there, I moved it in SID field description. Hope that helps.

"SID: optional: 4-octet field containing label, TC, S and
      TTL as defined in  <xref target="type1"/>.
           When the SID field is present, it MUST be used for
      constructing the Reply Path."

4.4 and 10.2

It's not illegal, it's just a bit odd that you are requesting bit 1 to be assigned and not but 0. Any reason?

---
<SH> Trying to mimic what is already defined. Other bits currently defined in BGP but not applicable here.

4.4

   Unused bits in the Flag octet SHOULD be set to zero upon transmission
   and MUST be ignored upon receipt.

I think you need this to be MUST be zero on transmission. Otherwise you break the definition of new bits in the future (new bit has meaning but legacy implementation sometimes sets the bit to 1).

---
<SH> agree

4.4

   A-Flag applies to Segment Types C, D.  If A-Flag appears with any
   other Segment Type, it MUST be ignored.

Is this too strong? It may confuse future extensions that want to define new Segment Types and use the A flag. Perhaps all you are concerned about is the use of the A flag with Type-A segments?

---
<SH> yes. Agree
If A-Flag
      appears with Type-A Segment Type, it MUST be ignored.

5.

   SRv6 dataplane is not in the scope of this document and will be
   addressed in a separate document.

Will it, though?
Either s/will/may/ or delete from "and" to the end.

---
<SH> changed to
" SRv6 dataplane is not in the scope of this document."

6.1

   In the inter-AS scenario,the procedures described in this document
   should be used to specify the return path.

Is this "should" or "SHOULD"?
You are allowing variation (by using should not must) so you need to say why an implementation/deployment would choose to do something different.

---
<SH> The inter-AS deployment may not need procedures from this doc if ip connectivity to the initiator is available. That’s why the first statement
Is not normative.

6.1 has several acceptable uses of "MUST". But I don't find what a receiver should do if one of these is violated. For example,

   LSP ping initiator MUST
   set the Reply Mode of the echo request to 5

So, it the receiver sets a Reply Mode set to another value, what happens? 6.2 might cover this, but doesn't quite.

---
<SH> If reply mode 5 is not set, normal RFC 8029 procedures will be used and if no ip connectivity to initiator echo-reply will be dropped.
I don't think this document need to specify it.

6.3

   The responder MAY check the
   reachability of the top label in its own Label Forwarding Information
   Base (LFIB) before sending the echo reply.

A fine use of MAY, but you should indicate how/why an implementation would make this choice.

---
<SH> Tried to cover, why an implementation may do it
"The responder MAY check the reachability of the top label in its
 own Label Forwarding Information Base (LFIB) before sending the echo reply
 and provide necessary log information in case of unreachabilty."

6.3

   In certain scenarios,the
   head-end may choose to send Type C/Type D segments consisting of IPV4
   address or IPv6 address.

"may" or "MAY"?
Which scenarios and why make that choice?

<SH> changed to MAY
These are just implementation choices and I think it's not necessary to talk about
Why an implementation would choose to do it that way.
---

6.3

   Optionally SID may also be associated with
   the Type C/Type D segment.

"Optionally" or "It is OPTIONAL"?
How is his choice determined?

---
<SH> Again it's implementation choice.

6.3

   The reply path return code must be set as described in section 7.4 of
   [RFC7110].  The Reply Path TLV must be included in an echo reply
   indicating the specified return path that the echo reply message is
   required to follow as described in section 5.3 of [RFC7110].

"must" or "MUST"?  (twice)

---
<SH> These are from RFC 7110 and I was asked to remove MUST in previous review.

6.4

Is it possible that the TTL pre-increment is 255?

---
<SH> may be. If that happens, existing standards already specify what should be done.
This document is not going to change that.

7.

   Example topologies given in Figure 1 and Figure 2 will be used

I think that your examples here all use ASBRs, so you are actually limiting yourself to Figure 1 except for the final paragraph of 7.2.2?

---
<SH> yes. That is correct. Anything to be clarified about this?

7.

   IGPs like OSPF/ISIS

Are there many IGPs like OSPF and ISIS?

--
<SH> May be in future 😊. We have RIFT already

7.1

s/mpls/MPLS/

You have some non-ASCII characters

---
<SH> I fixed some but not sure I did the right way. Does IDnits catch this?

7.2.1

You have a couple of cases of "MUST" in this section. But this is an example, not normative text. So I think "will" is good enough.

---
<SH> Done
8. and subsections

Please use Title Case for the section headings

---
<SH> Done
8.1

Is "to traceroute" really a verb leading to the participle "tracerouted"?

---
<SH> This comment is too "technical" for me. Can you suggest correct sentence pls?
8.1

This seems very late to be introducing the expansion of ABR which you have used before.

---
<SH> All previous uses are names so I though this is first place its being used in text.

8.1

   In cases when SRGB is not uniform across the network,
   it is RECOMMENDED to add a Type C or a Type D segment.

Fair enough, but why might an implementation choose to do otherwise and what are the benefits/implications?

---
<SH> some implementation may come up  with other smart ways of doing it.
This document will not restrict those implementation choices.

10.1

You cite "[IANA]" but have no matching reference

---
<SH> fixed

10.2

I don't think that Figure 7 needs to be a figure.
<SH> fixed

_______________________________________________
mpls mailing list
[e-mail]
https://urldefense.com/v3/__https://www.ietf.org/mailman/listinfo/mpls__;!!NEt6yMaO-gk!B7Ff8868TaqCp1zMGGfPx37gxIVnvcUdZgT9M5drdwRi3aDww_0OxUTpWY_su3WVw7CS63GNDubL9E31Bg$