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 <bruno.decra...@orange.com> Date: Thursday, July 11, 2024 at 6:30 AM To: SPRING WG <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. --- §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. 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. --- 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. [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 1. 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.
_______________________________________________ spring mailing list -- spring@ietf.org To unsubscribe send an email to spring-le...@ietf.org
