SR Replication segment for Multi-point Service Delivery
draft-ietf-spring-sr-replication-segment-19

### S2.2.1

    S01.   If (Upper-Layer header type == 4(IPv4) OR
               Upper-Layer header type == 4(IPv6) ) {

seems suspicious.  I think the second "4" should probably be "41"?

### S5

ULAs are not "non-routable"; they're "non-globally-routable".

Thank you to Mohit Sethi for the SECDIR review.

Thanks for the refined Security Considerations language.

I support Andrew Alston DISCUSS position.

# Éric Vyncke, INT AD, comments for draft-ietf-spring-sr-replication-segment-15

Thank you for the work put into this document. It is quite dense and not too easy to read though, perhaps adding some graphics?

Please find below some non-blocking COMMENT points (but replies would be appreciated even if only for my own education), and one nit.

Special thanks to Mankamana Prasad Mishra for the shepherd's detailed write-up including the WG consensus and the justification of the intended status. 

I hope that this review helps to improve the document,

Regards,

-éric

# COMMENTS

## Section 1

The reader would probably welcome use case of this protocol: is it for multicast ? or more like a span port for monitoring/troubleshooting ?

Waiting until section 3 is not really reader friendly.

## Section 2

`When the PCE signals a Replication segment to its node` what is 'its node' ?

## Section 2.2

In the 2nd paragraph, is the segment left field also decremented ?

## Section 2.2.3

Thanks for this section (no need to reply).

# NITS

## Section 4.2

s/has all the Must and SHOULD clause/has all the MUST and SHOULD clauses/ ?

Moving this to a new objection thanks to the update to the security text.  My thanks to the authors for making the effort to clearly document the security considerations - and while I still feel that there are major problems with srv6 security in general - I don't feel that they can be further dealt with in the context of this document.

# GEN AD review of draft-ietf-spring-sr-replication-segment-16

CC @larseggert

Thanks to Thomas Fossati for the General Area Review Team (Gen-ART) review
(https://mailarchive.ietf.org/arch/msg/gen-art/WF6_i6kgEP9J8_frlekZtnm_6sQ).

## Comments

### Section 2, paragraph 18
```
     In principle it is possible for different Replication segments to
     replicate packets to the same Replication segment on a Downstream
     node.  However, such usage is intentionally left out of scope of this
     document.
```
What was the intent of leaving this out? There seems to be complexity
here that can be abused, in which case I would have expected this to
either be explicitly forbidden or discussed in sufficient detail to
understand (and mitigate) the issues.

### Section 2.2.3, paragraph 2
```
     An implementation of Replication segment for SRv6 MUST enforce these
     same restrictions and exceptions.  Though this specification does not
     use any extension header a future extension may do so and MUST
     support the exception (2) above.
```
It is unusual for a spec to limit what a future extension can do in
this way (and often it turns out to be too limiting) - why is this
content needed in this document?

## Nits

All comments below are about very minor potential issues that you may choose to
address in some way - or ignore - as you see fit. Some were flagged by
automated tools (via https://github.com/larseggert/ietf-reviewtool), so there
will likely be some false positives. There is no need to let me know what you
did with these suggestions.

### Section 2, paragraph 18
```
     an indicator role of the node is Leaf.  The operation performed on
     incoming Replication SID is NEXT https://www.rfc-editor.org/rfc/
     rfc8402#section-2.  At an egress node, the Replication SID MAY be
```
Document uses `eref` elements that convert into plaintext URLs in the
document (above and elsewhere), which make things difficult to read.
Convert to proper references?

### Grammar/style

#### Section 1.1, paragraph 5
```
des the ingress (Root) node of a multi-point service and the egress (Leaf) no
                                 ^^^^^^^^^^^
```
This word is normally spelled as one. (Also elsewhere.)

#### Section 2.1, paragraph 8
```
ote that this H.Encaps.Red is independent from the replication segment – it i
                              ^^^^^^^^^^^^^^^^
```
The usual collocation for "independent" is "of", not "from". Did you mean
"independent of"?

#### Section 2.2, paragraph 7
```
is variant of End behavior. The pseudo-code in this section follows the conv
                                ^^^^^^^^^^^
```
This word is normally spelled as one. (Also elsewhere.)

#### Section 2.2.3, paragraph 1
```
this draft does not specify any inter-operable elements of Replication segme
                                ^^^^^^^^^^^^^^
```
This word is normally spelled as one.

#### Section 8.2, paragraph 4
```
6, using N-SID6, steers packet via shortest path to that node. Replication to
                                   ^^^^^^^^
```
A determiner may be missing.

#### "A.1.", paragraph 11
```
b8:cccc:6:F6::0, steers packet via shortest path to that node. Replication to
                                   ^^^^^^^^
```
A determiner may be missing.

## Notes

This review is in the ["IETF Comments" Markdown format][ICMF], You can use the
[`ietf-comments` tool][ICT] to automatically convert this review into
individual GitHub issues. Review generated by the [`ietf-reviewtool`][IRT].

[ICMF]: https://github.com/mnot/ietf-comments/blob/main/format.md
[ICT]: https://github.com/mnot/ietf-comments
[IRT]: https://github.com/larseggert/ietf-reviewtool

Thanks to Wes Eddy for the TSVART review.

Hi,

(1) p 2, sec 1.  Introduction

   Replication segment is a new type of segment for Segment Routing(SR)
   [RFC8402], which allows a node (henceforth called a Replication node)
   to replicate packets to a set of other nodes (called Downstream
   nodes) in a Segment Routing Domain.  Replication segments provide
   building blocks for Point-to-Multipoint Service delivery via SR
   Point-to-Multipoint (SR P2MP) policy.  A Replication segment can
   replicate packets to directly connected nodes or to downstream nodes
   (without need for state on the transit routers).  This document
   focuses on the Replication segment building block.  The use of one or
   more stitched Replication segments constructed for SR P2MP Policy
   tree is specified in [I-D.ietf-pim-sr-p2mp-policy].  This document
   focuses on specifying replication behavior in an SR domain.  The
   management of IP multicast groups, building IP multicast trees, and
   performing multicast congestion control are out of scope of this
   document.

This isn't directly my technology area, but I found this document, at least up to section 2.2.1 to be quite heavy going.  I don't have any great suggestions on how to significantly improve this, but found the examples helpful and wished that I had looked at them first.  So, perhaps include a forward reference to them at the end of the introduction section.



Minor level comments:

(2) p 5, sec 2.1.  SR-MPLS data plane

   SIDs MAY be added before the downstream SR-MPLS Replication SID in
   order to guide a packet from a non-adjacent SR node to a Replication
   node.  A Replication node MAY replicate a packet to a non-adjacent
   Downstream node using SIDs it inserts in the copy preceding the
   downstream Replication SID.  The Downstream node may be a leaf node
   of the Replication segment, or another Replication node, or both in
   case of bud node.  A Replication node MAY use an Anycast SID or BGP
   PeerSet SID in segment list to send a replicated packet to one
   downstream Replication node in an Anycast set if and only if all
   nodes in the set have an identical Replication SID and reach the same
   set of receivers.  For some use cases, there MAY be SIDs after the
   Replication SID in the segment list of a packet.  These SIDs are used
   only by the Leaf/Bud nodes to forward a packet off the tree
   independent of the Replication SID.  Coordination regarding the
   absence or presence and value of context information for Leaf/Bud
   nodes is outside the scope of this document.

As a minor tweak to readability, I did wondering whether the above paragraph could be split into something more like a list, i.e., something like:

SIDs MAY ...

  - A Repliaction node MAY ...
  - A Replication node MAY ...
  - For some use cases, there MAY ...

Regards,
Rob