Hi Swadesh,

Thanks for the reply and the updated draft.
Looks good to me.
Please see inline [BD] some further comments

Thanks
Bruno

From: Swadesh Agrawal (swaagraw) <swaag...@cisco.com>
Sent: Friday, October 4, 2024 1:34 AM
To: DECRAENE Bruno INNOV/NET <bruno.decra...@orange.com>; SPRING WG 
<spring@ietf.org>
Subject: Re: [spring] draft-agrawal-spring-srv6-mpls-interworking

Hi Bruno,

Thanks for your review and the comments. Please find my response inline 
starting with[SA].
Sorry for the delay.

Regards
Swadesh

From: bruno.decra...@orange.com<mailto:bruno.decra...@orange.com> 
<bruno.decra...@orange.com<mailto:bruno.decra...@orange.com>>
Date: Thursday, July 11, 2024 at 6:30 AM
To: SPRING WG <spring@ietf.org<mailto:spring@ietf.org>>
Subject: [spring] draft-agrawal-spring-srv6-mpls-interworking
Dear authors,

Thank you for your draft.

I’ve done a review of your document. (with multiple interruptions, hopefully 
the comments will be consistent…)
Please find below some comments.


Abstract

“   This document describes SRv6 and MPLS/SR-MPLS interworking and co-
   existence procedures. »


Could you please expand the abstract to better give an overview of the whole 
document? (e.g., looking at the top titles in the table content and reflecting 
those subjects in the abstract)
Is this only description or does the document also specifies some technical 
procedures? (please rephrase and/or adjust the document intended status 
accordingly)
I’m not seeing much “co-existence procedures”. (more comments on this below)
[SA] Please see if new version provide better overview of the whole document.
[SA] Also removed the co-existence procedures as they are not currently 
specified in the document. Will pick it as separate work.

---
§2

The topology/scenario covers the case of inter-domain in the form of an Area 
Border Router (ABR) with a single node belongings in both IGP/domain. Another 
common scenario is the form of a couple of ASBR, each one belonging in a single 
IGP/domain.
Would it be possible to cover both cases?
[SA] Intention is to show generalized procedures that work in different IW 
scenarios. ASBR topologies with single hop eBGP sessions works in same manner. 
For 6oM, IPv6 SAFI (2/1) advertise locators across such sessions. Similarly for 
Mo6, its option C between the ASBRs. If you think it will be helpful, I can add 
them as appendix in future versions.

[BD] A priori, I think it would be useful to add. Since there would be two 
separate domains -hence ASBRs- (one MPLS, one SRv6) it’s a priori less obvious 
to me which one would to the IW procedures.

---
§2.1.1


“SRv6 over MPLS (6oM)”



At this point in the draft, it’s not clear that anything is needed as SRv6 
packets can cross IPv6 (non-SRv6) networks. This would also allow aggregation, 
while §7.1 seems to create one FIB entry per egress PE.

May be you are assuming that the Core network doesn’t support IPv6 forwarding 
(even with 6PE) or that non-shortest path are needed in the Core. Clarification 
would be useful IMO.



[SA] This section is describing generalized IW scenarios and not the existing 
or new procedures required to solve it. Yes these are 6PE procedures through 
MPLS core domain but for infrastructure locators. Section 7.1.2 states it 
“Procedures described below build upon BGP 3107  [I-D.ietf-mpls-seamless-mpls] 
and [RFC4798]”.

[SA] Regarding aggregation, section 7.1.2.1 does mention in bullet 2: “Note: 
Egress border router may advertise summary prefix covering all PE locators in 
LE domain”.

[SA] Yes details are for one FIB entry per egress PE since here it shows BGP 
control plane end to end and keeps it in sync with Mo6 procedures i.e. to show 
remote IPv6 locator tunneled in ingress SRv6 domain that avoids remote locator 
being learnt on internal ingress domain routers (Advantage is end to end metric 
propagation etc.)


---

“4.1<https://datatracker.ietf.org/doc/html/draft-agrawal-spring-srv6-mpls-interworking-07#section-4.1>.
  End.DTM »



The End.DTM SID MUST be the last segment in a SR Policy

[…]

  S02.   If (Segments Left != 0) {


This does not seem to cover draft-ietf-spring-srv6-srh-compression.
Is there any plan to cover it or is End.DTM not compatible with srh-compression?
[SA] pseudo code is defined as per RFC 8986. Sine DTM is last SID, NEXT pseudo 
code is same. Will cover REPLACE option in future versions.


S06.    Process as per [ietf-spring-srv6-network-programming] section 4.

Draft has been published as RFC more than 3 years ago so now may be a good time 
to update the reference.
[SA] Done

“as per section 4” is not very specific as this is essentially the whole RFC 
(17 pages from the PDF). Please be more specific.
[SA] Looks like your version got truncated. Its section 4.1.1 as per 
https://datatracker.ietf.org/doc/draft-agrawal-spring-srv6-mpls-interworking/

---
§4.2 End.DPM

Terminology question.
End.DPM means "Endpoint with decapsulation and MPLS label push"
End.BM means “"Endpoint bound to an SR-MPLS Policy"”
Why not using the same terminology? (ie. SR-MPLS Policy rather than MPLS label 
push”
[SA] The difference is “decapsulation of outer SRv6 header” and label push. 
There is no decap for End.BM. End.BM is bound to SR MPLS policy. It can be non 
last segment and allows scalable SRv6 policy across MPLS domain. DPM is the 
last segment.

At minimum, it’s not clear to me whether End.BM pushes a label or a label stack:

1.       “MPLS label push » seems to refer to a single label

2.       “Push the MPLS label stack” seems to refer to a label stack (of 
arbitrary size)
[SA] Its not limited to single label. Label or stack to which FEC is bounded.

[BD] OK. Thanks for the clarification.
What about clarifying this in the first bullet? E.g. :s/MPLS label push/MPLS 
label stack push

It’s not clear to me whether End.DPM may be used for any type IPv6 payload? 
(e.g., IPv4?, IPv6?, MPLS?, Ethernet?)
If so, is the Upper-Layer header type checked when decapsulation is performed? 
What is being done if there is not a match between the type and the FEC of the 
inner label being pushed?
[SA] End.DPM results in decap of IPv6 and its extension headers and push of 
MPLS label stack. This is similar to MPLS LSR swap operation and it does not 
need the upper layer header processing. This behavior is not exposed at edge 
and type of payload does not matter, similar to MPLS LSR.
[SA]  Yes, End.DPM may be used for any type of IPv6 payload. Will try to state 
either the generic or case by case mismatch handling between the type and the 
FEC of label in next version.


[BD] OK. Thanks. Yes possible mismatch between the IPv6 payload type (Next 
Header) and MPLS FEC type was my point. A priori, before ‘S01 Remove the outer 
IPv6 Header with all its extension headers“ one need to check the IPv6 Next 
Header value and possible push an additional MPLS explicit null label if there 
is a mismatch with the MPLS FEC


---

5.1<https://datatracker.ietf.org/doc/html/draft-agrawal-spring-srv6-mpls-interworking-07#section-5.1>.
  H.Encaps.M: H.Encaps applied to MPLS label stack


“   The H.Encaps.M behavior encapsulates a received MPLS Label stack

   [RFC3032<https://datatracker.ietf.org/doc/html/rfc3032>] packet in an IPv6 
header with an SRH.  Together MPLS label

   stack and its payload becomes the payload of the new IPv6 packet.

   The Next Header field of the SRH MUST be set to 137 
[RFC4023<https://datatracker.ietf.org/doc/html/rfc4023>].”


I would welcome more details. Such as:

1.       What is being done on and with the top label of the received MPLS 
packet? (current text seems to assume that it is not read nor swapped/poped. Ie 
the node performing H.Encaps is not an MPLS LSR)

2.       How do you set Hop Limit, Flow labels, QoS fields
[SA] It depends on the control plane programming of the top label. For example, 
if the top label is MPLS Binding SID to SRv6 policy, it results in pop of top 
label and rest of MPLS stack being encapsulated in IPv6 header. Another example 
is BGP LU next hop self, in that case received top label is swapped and new top 
label is encapsulated in IPv6 header to cross SRv6 domain.

[BD] Could this be specified in the draft?

[SA] MPLS TTL is propagated to IPv6 header hop limit; similarly transfer the 
MPLS TOS to IPv6 QOS field. Will add the above details to the section in the 
draft.

---
§7.1.1.1

“B:4:BM-C1-7:: », « B:10::DT4 » are not valid IPv6 addresses (notation, “M” and 
“T” are not an hexadecimal digit.

---
§7.1.1.1

“Node 7 performs a native IPv6 lookup on due PHP behavior for 16007”
 [SA] Idea was to show the behavior of the SID in the function part of the SID 
for better readability/interpretation. It does not show allocated function 
value in the packet. If required, will change to valid IPv6 addresses in the 
draft.

That does not seem to work in the general case. You assumed that Core and node 
5 are SR-MPLS IPv4, possibly not supporting IPv6. With PHP, node 5 would see an 
IPv6 packet that it does not know how to forward.
[SA] Node 4 encaps MPLS segment list has IPv6 explicit NULL. This will make 
sure IPv6 payload is only exposed at node 7.
“Packet leaving node 4 MPLS (16005,16007,2)((A:1::, B:8:E::)
      (B:10::DT4, B:8:E::, B:4:BM-C1-7-:: ; SL=1))”

---
§7.1.2

May be  :s/BGP 3107/BGP Label Unicast
Especially since RFC3107 has been obsoleted.
In any case, please be consistent across the document. (which currently 
sometimes uses BGP 3107 and sometimes BGP LU)
[SA] Done

“BGP-LU(1/4, 2/4)”
I’m not sure that this is a normative terminology, especially outside of the 
IDR WG. Please expand. E.g. BGP-LU (SAFI 4, AFI 1 or 2).
Also the rest of the network does not use this notation. E.g. §7.1.2.2.1 uses 
“BGP AFI=1, SAFI=4”. Please be consistent.
[SA] Done

“They are also applicable to color-aware routes [I-D.ietf-idr-bgp-car] 
recursing over intent aware intra-domain paths.”
If you refer to BGP-CAR, please also refer to the alternative solution. We are 
not taking side.
Also note that you are creating a Normative Reference to an Experimental RFC. 
If the reference is needed, to avoid a downref, this document could to move to 
experimental or may be even informational.
[SA] Those are informative references. Alternative solution uses 
draft-salih-spring-srv6-inter-domain-sid hence didn’t reflect it here. Please 
let me know if you think otherwise.

----
§7.1.2.1

Egress border router seems to behave as per rfc4798 (6PE). May be adding a 
reference would help from a spec standpoint. May be referring to the term “6PE” 
would also help the reader as in this scenario (MPLS core) I would assume that 
the term 6PE is well known to the reader
 [SA] Done

---
§7.1.2.1


“Alternatively, leak remote locators or their summary in LI IGP”

Redistribution from BGP into IGP is usually from upon in operational networks. 
E.g. if you make a mistake with your filters, you may end of redistributing 
full routing in the IGP.
 [SA] Yes. Right filters need to be in place. Also its expected to be done only 
for infrastructure prefixes since full table is not expected in the 
infrastructure ABRs in general.

FYI, in ‘FIB state’ at the end of the section, the end of lines are truncated 
in the PDF and txt version. With no indication of this for the reader, so it’s 
a bit difficult to read. I’m don’t know whether the blame in on the authors of 
the xml tools.
Also this FIB state seems to be specific to a scenario (“Alternatively, 
B:10::/48 or summary route reachability is learned through ISIS”). Please 
detail which scenario is covered with this example of FIB states.
Also you seem to use END with PSP while PSP is not indicated in the text.
  [SA] DOne
---
§7.1.2.1

When introducing the example (at the end of the section) it would help the 
reader if you could describe which options are taken as the preceeding text 
cover multiple options (e.g  remote locators learned with or without SRv6 SID 
encapsulation, egress PE originating route in BGP IPv6 or not, ingress BR 
leaking in IGP or not, Egress BR aggregating prefix or not…). Currently the 
reader has to figure it out itself while an example is supposed to help.
 [SA] Done. (“Example to advertise node 10 locator to node 1 with SRv6 SID 
encapsulation”)

---
§7.1.2.1

“label explicit null”
Please use the real name. (in particular since there are two ones. I would 
assume that you are referring to "IPv6 Explicit NULL Label")
 [SA] Done

---
§7.1.2.1 - Example

At this point in the draft, the reader may not recall what nodes 7, 4, 1, are. 
May be adding a summarized topology at the begging odf the example would help. 
E.g.

|  :                   |                      |                     : |
|----+    IGP1       +---+        IGP2      +---+      IGP3      +----|
| 1  | ............. | 4 | ...............  | 7 | .............  | 10 |
|----+               +---+                  +---+                +----|
|                      |                      |                       |
iPE                   iBR                    eBR                     ePE


[SA] Added
7.1.2.2.2.

This section seems a bit short for something which is new. Especially compared 
to other sections which had more details, including on existing technology 
(e.g. on 6PE).
For example,


1.       For each BGP LU route
ok


2.       Advertise BGP route to PE address

It’s not obvious to me whether you are referring to a SAFI 1 or SAFI 4 route. A 
priori label is not used so SAFI 4 does not seem needed. OTOH, translating 
route from one SAFI to another seems like something that would need to be 
called out explicitly.
 [SA] Those details are added in BESS draft as indicated in the section.
“Section 2.2 of [I-D.agrawal-bess-bgp-srv6-mpls-interworking]
   describes how existing BGP advertisement can signal SRv6 SID
   associated with DPM behavior from egress to ingress border router.”


---
7.2.1
I guess that this is similar to inter-as option A. To help readers, could this 
be stated at the beginning? (just like §7.2.2 says that it’s similar to option 
B)
[SA] Added

---
7.2.2

“Certain L2 service specific information (eg. control word) translation is out 
of the scope. It will be covered in separate document. »
Any progress on this ?
[SA] Planning to update the BESS draft by next IETF.

---
8

Essentially this section has no content. “These procedures would be detailed in 
a future revision” is there from -00 which is 6 years old. Is there any real 
plan to fill that section or should it be deleted?
[SA] As already stated, will delete this for now and start a separate work for 
it.

--- implementation section

It would be good to add an implementation section as per 
https://wiki.ietf.org/en/group/spring/WG_Policies
 [SA]Ack

---
9. Availability

This section does not seem cover LE to LI propagation of egress PE failure for 
BGP procedure.
In particular when address summarization done on ingress border router as 
proposed in 7.1.2.1
[SA] It only covers the failures of IW nodes. Will add reference to BGP PIC 
draft. Egress PE is the existing failure and out of scope for current draft.
---
10. IANA Considerations



IANA is requested to assign identifier value

[…]

| TBD         |  TBD   |    End.DTM              | <this document>  |


IANA registry already has an entry for End.DTM
73
0x0049
End.DTM
[draft-agrawal-spring-srv6-mpls-interworking-05<https://www.iana.org/go/draft-agrawal-spring-srv6-mpls-interworking-05>]

Are you asking for a second one, which would be different (hence incompatible) 
with the existing one?
- If so, may be choosing a different name would avoid issues
- If not, please update the draft to use the existing one.
 [SA] Will update the draft for the DTM. Will request for DPM.

---

11. Security Considerations

[blank]

I don’t think that this is good enough.
I’m seeing significant security considerations. In particular the security 
model of MPLS networks relies on the inability for any attack to spoof/inject 
packets with an arbitrary label. Some of the functions define in this document 
do exactly this hence they introduce risk by their nature.
Also the border nodes (4 ,7 ) seems to belongs to two domains and 2 IGPs and 
hence needs to handle both the MPLS and SRv6 security measures.
Please clarify whether SRv6 security measures are only necessary in C or also 
in LI and LE. Please clarify the perimeter of the “SR domain” (as used in 
RFC8754 and in particular in its section 5.1)
 [SA] Ack. Will work in the security consideration in future versions.

Regards,
--Bruno

____________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations 
confidentielles ou privilegiees et ne doivent donc

pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce 
message par erreur, veuillez le signaler

a l'expediteur et le detruire ainsi que les pieces jointes. Les messages 
electroniques etant susceptibles d'alteration,

Orange decline toute responsabilite si ce message a ete altere, deforme ou 
falsifie. Merci.



This message and its attachments may contain confidential or privileged 
information that may be protected by law;

they should not be distributed, used or copied without authorisation.

If you have received this email in error, please notify the sender and delete 
this message and its attachments.

As emails may be altered, Orange is not liable for messages that have been 
modified, changed or falsified.

Thank you.
____________________________________________________________________________________________________________
Ce message et ses pieces jointes peuvent contenir des informations 
confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce 
message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages 
electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou 
falsifie. Merci.

This message and its attachments may contain confidential or privileged 
information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete 
this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been 
modified, changed or falsified.
Thank you.
_______________________________________________
spring mailing list -- spring@ietf.org
To unsubscribe send an email to spring-le...@ietf.org

Reply via email to