Erik, Thanks for the review. Comments inline @ [RP]. On Wed, Jul 5, 2023 at 6:24 PM Erik Kline via Datatracker <nore...@ietf.org> wrote:
> Erik Kline has entered the following ballot position for > draft-ietf-spring-sr-replication-segment-15: Discuss > > .... > > ---------------------------------------------------------------------- > DISCUSS: > ---------------------------------------------------------------------- > > # Internet AD comments for draft-ietf-spring-sr-replication-segment-15 > CC @ekline > > * comment syntax: > - https://github.com/mnot/ietf-comments/blob/main/format.md > > * "Handling Ballot Positions": > - > https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/ > > ## Discuss > > ### S2.2.1 > > * I think there's some clarification required about what a Replication SID > does if Segments Left is > 0. > > I can't see where Segments Left is decremented prior to packet > duplication > and so it looks like Replicate() would effectively H.Encaps[.Red] packets > with an (inner) SRH that still points to the Replication SID? > [RP] Unlike most other SRv6 behaviors, a Replication node does not process the SRH (except at a Leaf node where a Context SID may be present to provide context for packet processing). A Replication node has Replication state associated with the Replication SID (in incoming packet) from which it gets the list of branches and then for each branch, it replaces the IPv6 destination address replicated packet copy with the Replication SID of the downstream node. Hence, a Replication does not decrement Segments Left (if present in a SRH). * At S20, if Segments Left is still > 0 (even after whatever S16 is supposed > to be doing), why would you discard all of the extension headers? > [RP] This is similar to say End.DT4 behavior in Section 4.7 RFC 8986 where a node with local End.DT4 SID decapsulates the outer IPv6 header (and extension headers) to process the inner payload. Similarly, an upstream Replication node (or Root of replication segment) encapsulates a payload in IPv6 header and possibly extension headers and a Leaf or Bud node removes the outer IPv6 encapsulation and extension headers to process the payload. Please note this document does not specify use of IPv6 extension headers, but a future document extending Replication segment may do so. > > I think the S14-S21 stuff is trying to say "if this Replication node is > a leaf or a bud include it in the Replication List". If that's true I > suspect there are clearer ways to express that; you could just redefine > the replication list inside Replicate() and let implementers figure out > how to apply optimizations. > [RP] I hope my above reply clarifies that S14-S21 is just describing the decapsulation action on Leaf/Bud nodes. > > ### A.2.1 > > * Please clarify which source address R6 uses to formulate the Echo Reply. > > I'm a little unclear on the mental model here. The generation of ICMP > errors is prohibited because the Replication SID is analogized to a > multicast address in this context. > > Pinging a multicast address is of course fine, but the echo requester > knows > to expect the source address of replies to be any unicast address. I'm > assuming a requester needs to be modified to know that echo replies > cannot > come from the Replication SID in this case? > > Or is the Replication SID intended to be the source address of Echo > Replies > here, as if it's some kind of conceptual "unicast whenever we want but > also > multicast whenever say" kind of address? > [RP] The source address used in Echo Reply is the Replication SID of the responder. Pinging a replication SID is similar to pinging any other SRv6 SID as described in Sections 2.2, A.1.2 of RFC 9259. Note the Replication SID is like other SRv6 SIDS with LOCATOR:FUNCTION:ARG format. The LOCATOR is an IPv6 unicast prefix and the FUNCTION has the SID value. So the Replication SID is itself an IPv6 unicast address and thus it can (actually MUST) be used as the source address of Echo Reply according to Section 4.2 of ICMPv6 RFC 4443. > > ---------------------------------------------------------------------- > COMMENT: > ---------------------------------------------------------------------- > > # Internet AD comments for draft-ietf-spring-sr-replication-segment-15 > CC @ekline > > * comment syntax: > - https://github.com/mnot/ietf-comments/blob/main/format.md > > * "Handling Ballot Positions": > - > https://ietf.org/about/groups/iesg/statements/handling-ballot-positions/ > > ## Comments > > ### S2/S2.2 > > * Given that Replicate() is implemented in terms of encapsulation in > another > SRH it's probably good to cite some text about the MTU considerations for > operators. Probably the usual "size your MTU big enough or expect > trouble" > type of advice is really all that can be said. > > [RP] Note that SRH is not mandatory or even necessary for Replication SID, since the unicast locator prefix of downstream Replication SID can take a packet directly (on IGP shortest path) to the downstream node. SRH is only required, as described in second paragraph of Section 2.2, if a replicated packet has to be steered on a specific path (not the IGP shortest path) to the downstream node. This is done using H.Encaps.Red which adds the SRH. However, your point about MTU consideration for this particular case is valid. I will add text in Section 2.2 about this. ### S2/S6 > > * It seems like nothing in the control plane representation information can > prevent a chain of replication SIDs forming a loop. It should probably > be > noted that this can occur, looping and replicating packets until the > Hop Limit stops it, if there is no function elsewhere that prevents the > formation of loops when setting up the control plane (not necessarily a > problem when a PCE is programming things, but in the "provisioned locally > on a node" case it might be easier to make a mistake). > [RP] Agreed. I will add text in Section 2 about potential looping and how MPLS TTL/IPv6 Hop Limit can break the loop. But I think this is equally applicable to other SRv6 behaviors like End/End.X. One can craft an SRH that can cause a packet to loop between a few nodes till either Segment List is exhausted or IPv6 Hop Limit is reached. > ## Nits > > ### S1.1 > > * s/IPV6/IPv6/ > > ### S2.2 > > * s/pen-ultimate/penultimate/ > > > [RP] Will fix. > > _______________________________________________ > spring mailing list > spring@ietf.org > https://www.ietf.org/mailman/listinfo/spring >
_______________________________________________ spring mailing list spring@ietf.org https://www.ietf.org/mailman/listinfo/spring
