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. > > * We need to explain what Downstream Replication SID is in the replication > state. The text says <Downstream Node> represents the reachability but says > nothing about <Downstream Replication SID>. > [RP] Will add an explanation in the next revision. > Also Section 2.1 talks of optional segment list which is missing here! > > * The IANA allocation is already made under FCFS, I think what you are > asking is not a new allocation but the change control to be moved to IETF. > Thus, please rephrase. > [RP] Will do. > > * I don't think that there is any additional security risk because of the > replication segment. It can surely be exploited to replicate traffic to be > sent to an unauthorized party, or incorrectly multiply packets in the > network etc. > [RP] We meant no additional security considerations beyond those mentioned in RFCs 8402, 8754 and 8986. There was already a comment earlier about this - will rephrase in next revision. > > > ## 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. > > * The section 1 introduction is very bare. I suggest authors to provide > some more background. > [RP] Based on an earlier comment, we are going to add a terminology sub-section to Introduction. What else do you want us to add there? > > * For the bud node, you should state clearly what is the expected > replication state. > [RP] There was a similar comment by Sasha. Will try to add more explanation. > > * 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. > > ## Nits > > * Update The requirement language with RFC 8174 boilerplate text instead > of RFC 2119. > [RP] This was also brought up in an earlier comment. Will fix it in next revision. > > * I think we should avoid writing styles like - "We define a new type.." > [RP] I did not know first person pronouns were prohibited :). Will rephrase. > > * Expand SRv6, MVPN, EVPN, P2MP, RIR on first use. > [RP] I do not understand what you mean here. > > * Section A.1, what does the "->" in "<R-SID2->L12>" indicate? This should > be clearly stated in the text. > [RP] This means a packet has to be replicated with outgoing SID R-SIDs and sent on link L12. Will add an explanation in the next revision.
_______________________________________________ spring mailing list spring@ietf.org https://www.ietf.org/mailman/listinfo/spring
