Dhruv, All of your suggestions make sense. I will incorporate them in the next revision.
Thanks, -Rishabh On Tue, Jan 10, 2023 at 8:34 PM Dhruv Dhody <dhruv.i...@gmail.com> wrote: > Hi Rishabh, > > Thanks for your response, snipping... > > On Tue, Jan 10, 2023 at 11:14 PM Rishabh Parekh <risha...@gmail.com> > wrote: > >> Dhruv, >> Thanks for the review. Comments inline @ [RP] >> >> On Mon, Jan 2, 2023 at 10:00 PM Dhruv Dhody <dhruv.i...@gmail.com> wrote: >> >>> >>> ## Major >>> * For a standard track document, IMHO you need to clearly specify the >>> size of Replication-ID. >>> ```` >>> In simplest case, Replication-ID can be a 32-bit number, but it can >>> be extended or modified as required based on specific use of a >>> Replication segment. When the PCE signals a Replication segment to >>> its node, the <Replication-ID, Node-ID> tuple identifies the segment. >>> Examples of such signaling and extension are described in >>> [I-D.ietf-pim-sr-p2mp-policy]. >>> ```` >>> If for some use 32-bit is not enough we need to make sure we have a >>> bigger size assigned from the start or have a mechanism for variable size. >>> But saying that replication ID size is based on the specific use of the >>> replication ID could be problematic IMHO because of the tight coupling with >>> the usecase. >>> >>> BTW, the reference in the above text (which says Tree-ID is mapped to >>> Replication-ID) assumes it is always 32-bits! >>> >> >> [RP] The actual identifier is indeed dependent on the use-case – a simple >> 32-bit id is sufficient for Ingress Replication use-case whereas for >> SR-P2MP tree, the <Root, Tree-ID, Instance-ID> are mapped to the >> Replication-ID. The intent of the document is just to define the concept of >> Replication Segment and the use cases are specified in detail in other >> documents. >> >> > > Dhruv: My suggestion would be to say that the Replication-ID is a variable > length field (so that it is clear from the start), and then say that in its > simplest form can be a 32-bit number. This would make sure any signaling > extension that includes Replication-ID field considers it as a variable > length field. I understand that the approach taken right now is to signal > <Root, > Tree-ID, Instance-ID> and not Replication-ID as a field but still for > future cases, that would make more sense! > > <snip > >> ## Minor >>> * I am wondering about the usage of the term "multi-point" on its own - >>> usually, it is P2MP or MP2MP, I am unsure about just MP! If it is needed >>> maybe a definition can be provided early on. >>> >> [RP] P2MP or MP2MP distinguishes unidirectional and bi-directional trees. >> Here "multi-point" is referring to a service that reaches multiple >> destinations. >> >>> > Dhruv: Please add that in the Introduction or terminology section. > > <snip> > >> * Section A.1 >>> >>> ```` >>> Replication segment at R2: >>> >>> Replication segment <R-ID,R2>: >>> Replication SID: R-SID2 >>> Replication State: >>> R2: <Leaf> >>> ```` >>> What does R2: <Leaf> indicate? It does not align with the text in >>> section 2, which states that there is no replication state for egress >>> nodes. Perhaps we need a better description in section 2 for replication >>> state at leaf and bud nodes. >>> >> [RP] A pure leaf node does not have any replication state, but it still >> needs an indicator that it is a leaf node. That is what <Leaf> indicates in >> the example. >> >>> > Dhruv: In the text if we say that the leaf node has no replication state, > but in the figure the replication state includes an indicator for the leaf > node <leaf>, that does not align. I think you should state that in the case > of leaf or bud nodes, the replication state includes an indicator for the > leaf node only. > > <snip> > >> >>> * Expand SRv6, MVPN, EVPN, P2MP, RIR on first use. >>> >> [RP] I do not understand what you mean here. >> >>> > Dhruv: We expand all abbreviations (unless they are well known as > identified by the RFC Editor). See > https://www.rfc-editor.org/materials/abbrev.expansion.txt > > Thanks! > Dhruv >
_______________________________________________ spring mailing list spring@ietf.org https://www.ietf.org/mailman/listinfo/spring
