Hi,

Apologies for a late response but since the LC has not been officially
closed, let me slip in my review! :)

--

# draft-ietf-spring-sr-replication-segment

Hi,

IMHO there are some issues in the document that require fixing before we
send the document to IESG.

## 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!

* 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>. 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.

* 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.

## 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.

* The section 1 introduction is very bare. I suggest authors to provide
some more background.

* For the bud node, you should state clearly what is the expected
replication state.

* 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.

## Nits

* Update The requirement language with RFC 8174 boilerplate text instead of
RFC 2119.

* I think we should avoid writing styles like - "We define a new type.."

* Expand SRv6, MVPN, EVPN, P2MP, RIR on first use.

* Section A.1, what does the "->" in "<R-SID2->L12>" indicate? This should
be clearly stated in the text.

--

Thanks!
Dhruv

On Mon, Nov 28, 2022 at 8:40 PM James Guichard <
james.n.guich...@futurewei.com> wrote:

> Dear WG:
>
>
>
> This email starts a 2-week Working Group Last Call for
> https://datatracker.ietf.org/doc/draft-ietf-spring-sr-replication-segment/
>
>
>
> Please read the updated document if you haven’t already and send your
> comments to the SPRING WG list no later than December 12th 2022.
>
>
>
> If you are raising a point which you expect will be specifically debated
> on the mailing list, consider using a specific email/thread for this point.
>
>
>
> Lastly, if you are an author or contributor please respond to indicate
> whether you know of any undisclosed IPR related to this document.
>
>
>
> Thanks!
>
>
>
> Jim, Joel & Bruno
>
>
>
>
> _______________________________________________
> 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

Reply via email to