[pim] Re: Mohamed Boucadair's No Objection on draft-ietf-p im-sr-p2mp-policy-15: (with COMMENT)
Rishabh Parekh <[email protected]>
| Newsgroups | gmane.ietf.pim |
|---|---|
| Message-ID | <CABjMoXazTgAPZy4mEa=ruiMr=-MSK-Vd+WHqz7apP6BOPVbBrw@mail.gmail.com> |
Mohamed, Thanks for the review. I have addressed some of the comments in revision 16 . Additional comments inline @ [RP] -Rishabh. On Wed, Aug 6, 2025 at 1:12 AM Mohamed Boucadair via Datatracker < [email protected]> wrote: > > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > Hi Rishabh, Dan, Clarence, Hooman, and Jeffrey, > > Thank you for the effort put into this specification, which leverages > RFC9524 > and RFC9256. > > Please find some comments below: > > # Check > > Section 2 has the following: > > It is similar to SR Policy > [RFC9256]. Like SR Policy, SR P2MP Policy has one or more Candidate > Paths and uses same criteria to select the Active Candidate Path. > > I’d like to check this as I’m not sure that all parameters are inherited. > For > example, do we still have Discriminator for the P2MP case as well? > [RP] Yes, all the elements of a Candidate Path of SR Policy are inherited for SR P2MP Policy as well. > > # Active instance of a Candidate Path?! > > Section 2.3 says: > The controller designates an active instance of a CP at the Root node > of SR P2MP Policy by signalling this state through the protocol used > to instantiate the Replication segment of the instance. > > (1) What is meant by “an active instance of a CP”? > [RP] An "instance" of a CP is the tree computed based on the constraints/objective of a CP. A CP can have more than one instance at a given time, e.g. for make-before-break. However, only instances of a CP carries the traffic from Root to Leaves and that is the active instance. > > (2) How this behavior interacts with the tie-breaking rules? > [RP] The tie-breaking rules are used to select the Active Candidate Path. The controller specifies the active instance of that Active Candidate path. > > As I’m there, please fix this: s/The controller/A controller > [RP] Done. > > # Section 2.4 > > CURRENT: > The Tree-SID of the active > instance of the active Candidate Path SHOULD be used as the Binding > SID of the SR P2MP Policy. > > Why this is not MUST? (see also next comment) > [RP] This is not a MUST because it is possible to assign a fixed well known Binding-SID to the SR P2MP Policy (that is different from Tree-SID of the active instance) so that the Binding-SID does not change when the active CP and/or active instance of the policy changes. This is useful when the traffic ingresses SR domain on a border node which is not the Root node of P2MP policy. The ingress node steers the traffic to the Root node using a segment-list in which the last segment is the Binding-SID of SR P2MP policy. > # Steering behavior > > CURRENT: > The Root node can steer an incoming packet into a SR P2MP Policy in > one of following methods: > > * Local Policy-Based Routing: The Root node selects the active P2MP > tree instance of the active Candidate Path of the SR P2MP Policy > based on local policy. The procedures to map an incoming packet > to a SR P2MP Policy are out of scope of this document. > > * Tree-SID Based Routing: The Binding SID (Tree-SID) in the incoming > packet is used to map the packet to the appropriate P2MP tree > instance. > > (1) Should the behavior of the root node be part of the instructions > received > from the controller? > [RP] I am not sure I understand your question clearly. But, the controller does not dictate what and how traffic is mapped into the P2MP policy. > > (2) As discussed earlier in the document, there is room for a case where > BID!=Tree-SID. What is the expected behavior in such cases? > [RP] In this case the Root node replaces the Binding-SID in incoming packet with the Tree-SID of active CP and active instances, and replicates the packet. > > (3) s/Local Policy-Based Routing/Local Policy-Based forwarding and > s/Tree-SID > Based Routing/Tree-SID Based forwarding > [RP] Done. > > # Section 3.1 > > CURRENT: > The Tree-SID SHOULD also > serve as the Replication-SID for the Replication segments at > intermediate Replication nodes and Leaf nodes. > > Please provide the rationale for this one. What are the implications if > this > SHOULD is not followed? > [RP] A controller computes a P2MP tree instance of a CP of a SR P2MP policy and instantiates the tree via Replication segments. It is easier to assign one unique SID as the Tree-SID and use that for all Replication segments at Root, intermediate and leaf nodes. However, this is not mandatory. A controller can decide to allocate unique SIDs for different Replication Segments on the tree as long as it ensures the Replication Segment replication branches replace the incoming SID correctly. But obviously SID resource management on the controller is more complex. > > # Section 3.2 > > (1) > > CURRENT: > A shared Replication Segment SHOULD be identified using a Root-ID set > to zero (0.0.0.0 for IPv4 and :: for IPv6) along with a Replication- > ID that is unique within the context of the node where the > Replication segment is instantiated. > > Idem as previous point, why this isn’t a MUST? At least the rationale > should be > called out. > [RP] Good point. I can't think of any reason for SHOULD. I will change it to MUST. > > (2) > > CURRENT: > However, a shared Replication segment MUST NOT be associated with an > SR P2MP tree. > > Does this apply even if this is shared only between a subset and not all > instances? > [RP] Yes, since this is shared across two or more instances, it cannot be associated with a particular P2MP tree. > > # Section 3.3 > > ## Transport/Service Context > > CURRENT: > For multi-point services, the transport identifier which is the Tree- > SID or Replication SID at a Leaf node is also associated with the > service context because it is not always feasible to separate the > transport and service context with efficient replication in core > since a) multi-point services may have differing sets of end-points, > and b) downstream allocation of service context cannot be encoded in > packets replicated in the core. > > I guess I understand what is meant here by these contexts, but it would be > better to introduce these first. > [RP] I have added a couple of sentences explaining service context. > > ## Deployment matter? > > CURRENT: > However, for SR-MPLS deployments, if it is known a priori that multi- > point services mapped to a P2MP tree can be uniquely identified > within the SR domain, a controller MAY opt not to instantiate > Replication Segments at Leaf nodes. > > How is that made known to the controller? Also, shouldn’t this better > handled > by a policy? > [RP] This can be by provisioning the controller or by some dynamic discovery mechanisms for example when Domain-wide Common Block (DCB) is used. But the exact mechanism is out of scope of this document. > > # Section 4.2 > > CURRENT: > A controller performs the following functions in general: > > * Topology Discovery: A controller discovers network topology across > Interior Gateway Protocol (IGP) areas, levels or Autonomous > Systems (ASs). > > * Capability Exchange: A controller discovers a node's capability to > participate in SR P2MP tree as well as advertise it’s capability > to compute P2MP trees. > > It also need to retrieve installed tree instances in the underly domain > when it > first bootstraps. > [RP] I expect most controller implementations would discover existing instances on startup, but IMO, this is an implementation detail and not a general requirement. > > # Section 4.3 > > ## Loops > > CURRENT: > A controller MUST compute a P2MP tree such that there are no loops in > the tree at steady state as required by [RFC9524]). > > I guess this should be conditional: IF the controllers computes a tree, > then it > must be forwarding loop-free. > [RP] Isn't this implied? If the controller does not or cannot compute a tree, there is no tree and therefore no need to satisfy the loop-free condition. > > (nit) delete the extra “)”. > [RP] Done. > > ## Policy-based > > CURRENT: > A controller SHOULD modify a P2MP tree of a Candidate Path on > detecting a change in the network topology or in case a better path > can be found based on the new network state. In this case, the > controller MAY create a new instance of a P2MP tree and remove the > old instance of the tree from the network in order to minimize > traffic loss. > > The SHOULD is scoped vaguely. No every topology change will trigger a > modification of the tree. > [RP] I have changed the text to make it conditional when the topology change impacts a given tree. > > Also, I guess some policy is needed to drive the controller behavior for > migrating to a new path/instance. > [RP] I am not sure I understand what you mean by "some policy is needed to drive the controller behavior". Do you mean a policy to decide whether to do a "in-place repair" by modify the tree around the affected part of the topology or do a "make-before-break" by computing a new instance, provisioning it and then switching traffic to the new instance on the Root? > > # Section 4.5.2 > > CURRENT: > It is possible for a controller create a disjoint backup tree > instance for providing end-to-end path protection. > > Well, this depends on the underlying topology. > > Not sure this statement (even if adjusted) adds much to the discussion. > [RP] Agree. But this is added for the sake of completeness in terms of local and path protection . AFAIK, this was added due to a comment during WG adoption. I can definitely add a conditional based on topology. > > # Additional Operational Considerations > > I was expecting some discussion about scalability matters and how to test > an > active CP. > [RP] The controller section is meant as a guidance and high level requirements to support SR P2MP policy. It is not a formal specification; I am not sure if these should be really in the scope of this document. > > Please consider adding some discussion about these. Adding > readily-available > pointers (if any) would work as well. Thanks > [RP] I can add pointers to other documents if you are aware of any. > > # Minor points > > ## Abstract > > (1) Circular definition > > CURRENT : > A SR P2MP Policy consists of Candidate Paths (CP) which > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > define the topology of P2MP tree instances in each Candidate Path. > ^^^^^^ ^^^^^^^^^^^^^^^^^^^^^^ > [RP] I am not sure how the above is circular definition. The top level construct is a SR P2MP Policy which has one or more CPs. Each CP can have one more P2MP tree instance. > > (2) (nit) s/Paths and and how P2MP trees/Paths and how P2MP trees > [RP] Fixed. > ## Introduction > > (1) Cite an authoritative reference > > CURRENT: > A Multi-point service delivery can be realized with P2MP trees in a > Segment Routing domain. > ^^^^^^^^^^^^^^^^^^^^^^ > [RP] Done. > > (2) There might be multiple roots > > OLD: > A controller computes P2MP tree instances, from the Root to Leaf > nodes, > > NEW: > A controller computes P2MP tree instances, from a Root to Leaf > nodes, > [RP] A SR P2MP Policy can have only one Root. I have modified the text to say "A controller computes P2MP tree instances of a SR P2MP Policy ...." > > (3) nit > > OLD: Once computed, the controller instantiate a P2MP tree instance > NEW: OLD: Once computed, the controller instantiates a P2MP tree instance > > [RP] Fixed > (4) Missing references > > CURRENT: > The Replication segments of a P2MP tree can be instantiated for both > SR-MPLS and SRv6 dataplanes, enabling efficient packet replication > ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ > within an SR domain. > > [RP] Added references. > ## Section 2.2: point the reader to Section 4 where these matters are > zoomed > into > > OLD: A SR P2MP Policy is provisioned on a controller. > > NEW: An SR P2MP Policy is provisioned on a controller (see Section 4). > > [RP] Done. > ## Section 2.3 > > CURRENT: > The Root > node selects the active Candidate Path based on the tie breaking > rules defined in [RFC9256]. > ^^^^^^^^^^^^ > > Please add the exact section to look at. > > [RP] Done. > ## Section 4 > > OLD: > A controller is provisioned with SR P2MP Policy and it's Candidate > ^^^^^^ > Paths to compute and instantiate P2MP trees in SR domain. Once > ^^^^^^^^^^^^ > computed, the controller instantiates the Replication segments that > compose the P2MP in the SR domain nodes using signalling protocols > such as PCEP, BGP, NetConf etc. The procedures for provisioning a > ^^^^^^^^^^^^ > controller and the instantiation Replication segments in SR domain > ^^^^ ^^^^^^^^ > are outside the scope of this document. > > NEW: > A controller is provisioned with SR P2MP Policy and its Candidate > Paths to compute and instantiate P2MP trees in an SR domain. Once > computed, the controller instantiates the Replication segments that > compose the P2MP in the SR domain nodes using signalling protocols > such as PCEP, BGP, NETCONF, etc. The procedures for provisioning a > controller and the instantiation of Replication segments in an SR domain > are outside the scope of this document. > > [RP] Done. > ## Section 4.2 > > s/ASs/ASes > s/it’s capability/its capability > > [RP] Done. > ## Appendix > > Replication SIDs formatted as SRv6 Segment Identifier (SID). The textual > representation of those has to adhere to RFC 5952, especially this part: > > The characters "a", "b", "c", "d", "e", and "f" in an IPv6 address > MUST be represented in lowercase. > [RP] Fixed. > > Cheers, > Med > > > > _______________________________________________ > pim mailing list -- [email protected] > To unsubscribe send an email to [email protected] > _______________________________________________ pim mailing list -- [email protected] To unsubscribe send an email to [email protected]