[spring] AD Review for draft-ietf-spring-sr-replication-segment-13

[James Guichard <[e-mail]>]

Dear authors,

Thank you for this document. Some comments before I move the document to the next step (the line numbers are from idnits):

COMMENTS:

22           Requirements Language

24              The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
25              "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
26              document are to be interpreted as described in [RFC2119] [RFC8174]
27              when, and only when, they appear in all capitals, as shown here.

Jim> please correct the above requirements language section to conform to RFC 8174. This RFC provides new text to replace the above text (insert BCP 14 before [RFC2119]).

101        1.1.  Terminology

109           *  Replication Node: A node in SR domain which replication packets
110              based on Replication Segment

Jim> Please correct the above to 'Replication Node: A node in an SR domain which replicates packets based on the Replication Segment' or something similar.
Jim> Further I notice that the document uses both 'Replication segment' and 'Replication Segment'. Please choose one and update the document use of the chosen term for consistency.

115           *  Replication-ID: Identifier of a Replication Segment at Replication
116              Node

Jim> insert 'a' between 'at' and 'Replication'

118           *  Replication State: This is state of Replication Segment at a
119              Replication Node.  It is conceptually a list of replication
120              branches to Downstream nodes.  The list can be empty.

Jim> The first sentence does not parse. Suggest changing to 'Replication State: State held for a Replication Segment at a Replication Node'.

125        2.  Replication Segment

131           programmed by a PCE.  Replication segments apply equally to both

Jim> Expand 'PCE' on first use.

134           A Replication segment is identified by the tuple <Replication-ID,
135           Node-ID>, where:

137           *  Replication-ID: An identifier for a Replication segment that is
138              unique in context of the Replication Node.

Jim> Replication-ID is now defined twice; in the terminology section and in this section. If you want to keep the definition in both places then please make the text consistent
as currently it is not.

144           Replication-ID is a variable length field.  In simplest case, it can
145           be a 32-bit number, but it can be extended or modified as required
146           based on specific use of a Replication segment.  When the PCE signals

Jim> You do not specify how it can be extended or modified, neither do you specify any specific use cases. If this is out of scope for the document then please say so.

165           Replication SID identifies the Replication segment in the forwarding
166           plane.  At a Replication node, the Replication SID operates on local
167           state of Replication segment and the resulting behavior MAY be
168           similar to a Binding SID [RFC9256] of a Segment Routing Policy.

Jim> the above paragraph mention of Binding SID still bothers me. The text says that it MAY behave like a Binding SID but does not specify any guidelines as to when it might or
how it might. Is it necessary to even mention Binding SID here? if it is then you need to expand on the text to give the reader some guidance.

199           incoming Replication SID is NEXT.  At an egress node, the Replication

Jim> please provide a reference for NEXT as you cannot assume that the reader knows what that is.

214        2.1.  SR-MPLS data plane

242           set of receivers.. For some use cases, there MAY be SIDs after the

Jim> remove the additional '.' above.

249        2.2.  SRv6 data plane
251           In SRv6 [RFC8986], the "Endpoint with replication" behavior
252           (End.Replicate for short) replicates a packet and forwards the packet
253           according to a Replication state.

Jim> Please reword the above paragraph as it reads like End.Replicate is defined in RFC8986 when in fact it is defined in this document.

262           segment list may be used on some branches using H.Encaps.Red (while

Jim> put a reference above for where H.Encaps.Red is defined.

641        9.2.  Informative References

652           [I-D.ietf-pim-sr-p2mp-policy]
653                      Voyer, D., Filsfils, C., Parekh, R., Bidgoli, H., and Z.
654                      J. Zhang, "Segment Routing Point-to-Multipoint Policy",
655                      Work in Progress, Internet-Draft, draft-ietf-pim-sr-p2mp-
656                      policy-05, 2 July 2022,
657                      <https://datatracker.ietf.org/doc/html/draft-ietf-pim-sr-
658                      p2mp-policy-05>.

Jim> please update this reference to the latest version (v-06)