[spring] Additional SRv6 Compression draft review

[Joel Halpern <[e-mail]>]

  Below is a review Boris Hassanov has provided the working group of the 
SRv6 compression draft, to complement the other reviews that we have 
received.  I hope to see comments on all the reviews from the document 
editors.

Thank you,

Joel



-------- Forwarded Message --------
Subject: 	Re: [spring] Volunteers for the SPRING SRv6 Compression draft
Date: 	Fri, 20 Oct 2023 14:38:38 +0000 (UTC)
From: 	Boris Hassanov <[e-mail]>
To: 	Joel Halpern <[e-mail]>



Hi Joel,
Thank you!

Yes, please forward it to the SPRING list, I think no reasons to hide my 
name.
I just prepared the updated version for it  with some editorial changes 
for slightly better reading, please use that one below.

SY,
Boris

>> *BEGINNING of the review*
>>
>> 1) Part 1.Introduction (p. 2) has incorrect reference:
>>
>> "These flavors enable a compressed encoding of the SRv6 Segment-List 
>> in the SRH and therefore address the requirements described in 
>> [I-D.srcompdt-spring-compression-requirement]."
>>
>> It has to be draft-ietf-spring-compression-requirement.
>>
>> 2) 2.1  Part 2. Terminology
>>
>> "Locator-Block: The SRv6 SID block (IPv6 prefix allocated for SRv6 
>> SIDs by the operator) of an SRv6 SID Locator, as defined in Section 
>> 3.1 of [RFC8986]."
>>
>> IMO, sounds a bit strange for understanding, I mean this one:"The 
>> SRv6 SID block of an SRv6 SID Locator."
>>
>> I think it has to be re-phrased: "IPv6 prefix which is used for 
>> allocation of SRv6 SIDs by the operator".
>>
>> Especially if Section 3 says:" In an SRv6 domain, the SIDs are 
>> allocated from a particular IPv6 prefix: the Locator-Block. All SRv6 
>> SIDs instantiated from the same Locator-Block share the same most 
>> significant bits."
>>
>> 2.2  Part 2. Terminology
>>
>> "Locator-Node: The identifier of the parent node instantiating a SID 
>> in an SRv6 SID Locator, as defined in Section 3.1 of [RFC8986]."
>>
>> I guess that the reference to Section 3.1 of RFC8986 might not 
>> becorrect ( It describes just format of SID, allocation is in Section 
>> 3.2)
>>
>> 3) Section 3. Basic concepts
>>
>> "The compressed Segment List encoding supports any Locator-Block 
>> allocation.While other options are supported and may provide higher 
>> efficiency, each routing domain can be allocated a /48 prefix from a 
>> global IPv6 block (see Section 6.2)."
>>
>> Which other options? Which routing domain can be allocated? Sounds 
>> confusing...
>>
>> If the authors meant IPv6 prefix length so it should be explicitly 
>> mentioned: "While other options of IPv6 prefix length allocation  for 
>> Locator-Block are supported it is RECOMMENDED to allocate a /48 
>> prefix from a global IPv6 block (see Section 6.2)."
>>
>> 4) SR Segment Endpoint Flavors
>>
>> 4.1 "This section defines several options to achieve compressed 
>> Segment List encoding in the form of two new flavors for the End, 
>> End.X, End.T, End.B6.Encaps, End.B6.Encaps.Red, and End.BM behaviors 
>> of [RFC8986].These flavors could also be combined with behaviors 
>> defined in other documents. "
>>
>> Which two flavors are mentioned here? it is not clear for the reader. 
>> I would suggest to mention those flavors at the beginning because 
>> there are many references for them in this section. For example, the 
>> authors wrote:
>>
>> "The compressed encoding can be achieved by leveraging any of these 
>> SR segment endpoint flavors."
>>
>> Next : "The NEXT-C-SID flavor and the REPLACE-C-SID flavor..."
>>
>> The reader should guess that these NEXT-C-SID and REPLACE-C-SID 
>> flavors are those which were mentioned. IMO it is not quite correct. 
>> I would change the structure of this section like this:
>>
>> " This section defines several options to achieve compressed 
>> Segment List encoding in the form of two new flavors 
>> (NEXT-C-SID, REPLACE-C-SID ) for the End, End.X, End.T, 
>> End.B6.Encaps, End.B6.Encaps.Red, and End.BM behaviors of [RFC8986]. 
>> Theseflavors could also be combined with behaviors defined in other 
>> documents.
>>
>> ... "
>>
>> 4.2 This sentence is too long and  hard for understanding: "The 
>> NEXT-C-SID flavor and the REPLACE-C-SID flavor expose the same 
>> high-level behavior in their use of the SID argument to determine the 
>> next segment to be processed, but they have different low-level 
>> characteristics that can make one more or less efficient than the 
>> other for a particular SRv6 deployment."
>>
>>  I would split and slightly change it:"The NEXT-C-SID f and the 
>> REPLACE-C-SID flavors expose the same high-level behavior in their 
>> use of the SID argument to determine which next segment will be 
>> processed. The NEXT-C-SID f and the REPLACE-C-SID flavors have 
>> different low-level characteristics that can make one of them more or 
>> less efficient than the other for a particular SRv6 deployment."
>>
>> 4.3 Here is the quite strange mapping of abbreviations and their 
>> explanations:
>>
>> " Both flavors leverage the following *variables*:
>>
>> **Variable *LBL is the Locator-Block length of the SID.
>>
>> **Variable *LNFL is the sum of the Locator-Node and the Function 
>> lengths of the SID.It is also referred to as C-SID length.
>>
>> **Variable *AL is the Argument length of the SID."
>>
>> Why do not write excluding the duplication of the word ‘variables’ 
>> (also LBL, LNFL, AL could be added to Terminology section) :
>>
>> " Both flavors leverage the following variables:
>>
>> *LBL - the Locator-Block length of the SID.
>>
>> *LNFL - the sum of the Locator-Node and the Function lengths of the 
>> SID.It is also referred to as C-SID length.
>>
>> *AL - the Argument length of the SID."
>>
>> 5) 4.1. NEXT-C-SID Flavor5.1 " A SID instantiated with the NEXT-C-SID 
>> flavor takes an argument that carries the remaining C-SIDs in the 
>> current C-SID container."
>>
>> I would change it for the better clarity like this:
>>
>> " A SID instantiated with the NEXT-C-SID flavor holds an argument 
>> that carries the remaining C-SIDs in the current C-SID container. "
>>
>> I am not native English-speaker but it looks that "takes" is too 
>> broad here (I may be mistaken of course).
>>
>> 5.2 " The length AL of the argument is equal to 128-LBL-LNFL." -> 
>> "The length of AL calculates as AL == 128-LBL-LNFL."
>>
>> 5.3 " An SR segment endpoint node instantiating a SID with the 
>> NEXT-C-SID flavor MUST accept any argument value for that SID."
>>
>> -> "An SR segment endpoint node that instantiates a SID with the 
>> NEXT-C-SID flavor MUST accept any argument value for that SID. "
>>
>> 5.4I would prefer to see an explanation diagram after that paragraph:
>>
>> "A C-SID sequence using NEXT-C-SID comprises one or more C-SID 
>> containers, each carrying the common Locator-Block followed by a 
>> series of C-SIDs.The Locator-Node and Function of the C-SID container 
>> are those of the first C-SID, and its Argument is the contiguous 
>> series of subsequent C-SIDs."
>>
>> 5.5 Some diagram is needed after this too:
>>
>> "When processing a SID with the NEXT-C-SID flavor, while the Argument 
>> value is non-zero, the SR segment endpoint node constructs the next 
>> SID by copying the SID Argument value immediately after the Locator- 
>> Block, thus overwriting the previous SID Locator-Node and Function, 
>> and filling the least significant bits of the argument with zeros."
>>
>> 5.6 Hard for understanding - why instead and instead of what?
>>
>> "When the Argument value is 0, the SR segment endpoint node *instead 
>> *copies the next 128-bit Segment List entry from the SRH to the 
>> Destination Address field of the IPv6 header. "
>>
>> Again, I may be mistaken but it looks that it should be written as : 
>> "When the Argument value is 0, the SR segment endpoint node copies 
>> the next 128-bit Segment List entry from the SRH to the Destination 
>> Address field of the IPv6 header."
>>
>> 6) 4.1.1. End with NEXT-C-SID
>>
>> 6.1 "When processing an IPv6 packet that matches a FIB entry locally 
>> instantiated as an End SID with the NEXT-C-SID flavor, the procedure 
>> described in Section 4.1 of [RFC8986] is executed with the following 
>> modifications." ->
>>
>> " When a node processes an IPv6 packet that matches a locally 
>> instantiated FIB entry as an End SID with the NEXT-C-SID flavor, the 
>> procedure described in Section 4.1 of [RFC8986] is executed with the 
>> following modifications: ..."
>>
>> 6.2
>>
>> "... with the following modifications:
>>
>> - The below pseudocode is inserted between lines S01 and S02 of the 
>> SRH processing in Section 4.1 of [RFC8986];
>>
>> -The below pseudocode is inserteda second time before line S01 of the 
>> upper-layer header processing in Section 4.1.1 of [RFC8986], or prior 
>> to processing any extension header other than Hop-by-Hop or 
>> Destination Option..."
>>
>> It is not really clear how this pseudocode should be finally 
>> inserted: 1 time between lines S01/S02 then again after line S01, so 
>> two times?
>>
>> I also think that the 2nd item should be corrected to clarify the 
>> point (when this pseudocode will be inserted 2nd time after line S01? 
>> Which conditions if any ?).
>>
>> 7)4.1.2.End.X with NEXT-C-SID
>>
>> 7.1 Same as in 6.1, I would suggest the following change:
>>
>> " When a node processes an IPv6 packet that matches a locally 
>> instantiated FIB entry as an End.X SID with the NEXT-C-SID flavor, 
>> the procedure described in Section 4.2 of [RFC8986] is executed with 
>> the following modifications: ..."
>>
>> 7.2 Same comment about pseudocode insertion as in6.2
>>
>> 8)4.1.3.End.T with NEXT-C-SID
>>
>> 8.1 Same as above, I would suggest the following change:
>>
>>  "When a node processes an IPv6 packet that matches a locally 
>> instantiated FIB entry as an End.T SID with the NEXT-C-SID flavor, 
>> the procedure described in Section 4.3 of [RFC8986] is executed with 
>> the following modifications: ..."
>>
>> 8.2 "The pseudocode in Section 4.1.1 of this document is modified by 
>> replacing line S08 as shown below."
>>
>> I think instead of reference to the section 4.1 and suggestion for 
>> changing lines S08, would be much clearer to place the link to 
>> Appendix B with whole new pseudocode (to exclude any mistake in 
>> interpretation)
>>
>> 8.3The pseudocode insertion description here is also confusing as 
>> before, so same comment as in 6.2: it must be clarified.
>>
>> 9)4.1.4.End.B6.Encaps with NEXT-C-SID
>>
>> 9.1Same as above, I would suggest the following change:
>>
>> "When a node processes an IPv6 packet that matches a locally 
>> instantiated FIB entry as an End.B6.Encaps SID with the NEXT-C-SID 
>> flavor, the procedure described in Section 4.13 of [RFC8986] is 
>> executed with the following modifications: ..."
>>
>> 9.2 Same as in 8.2, would be better to place the link to Appendix B 
>> with whole updated pseudocode
>>
>> 9.3 The pseudocode insertion description here is also confusing as 
>> before, so same comment as in 6.2: it must be clarified too.
>>
>> 10) 4.1.6.End.BM with NEXT-C-SID
>>
>> 10.1Same as above, I would suggest the following change:
>>
>> "When a node processes an IPv6 packet that matches a locally 
>> instantiated FIB entry as an End.BM SID with the NEXT-C-SID flavor, 
>> the procedure described in Section 4.15 of [RFC8986] is executed with 
>> the following modifications: ..."
>>
>> 10.2Same as in 8.2, would be better to place the link to Appendix B 
>> with whole updated pseudocode.
>>
>> 10.3 The pseudocode insertion description here is also confusing as 
>> before, so same comment as in 6.2: it must be clarified too.
>>
>> 11) 4.2.REPLACE-C-SID Flavor
>>
>> 11.1"A SID instantiated with the REPLACE-C-SID flavor takes an 
>> argument that indicates the index of the next C-SID in the 
>> appropriate C-SID container." ->
>>
>> " A SID instantiated with the NEXT-C-SID flavor holds an argument 
>> that carries the remaining C-SIDs in the current C-SID container. "
>>
>> 11.2 As in 5.2:"The length of AL calculates asAL == 128-LBL-LNFL."
>>
>> 11.3 " The index value is encoded in the least significant 
>> ceil(log_2(128/LNFL)) bits of the argument field."
>>
>> Which index? What is a ceil here? If a rounding function was meant 
>> (i.e. ceil in Python) it should be explicitly clarified in the text 
>> or better changing.
>>
>> 11.4 This item needs to be illustrated by a diagram, IMO:
>>
>> " A C-SID sequence using REPLACE-C-SID starts with an uncompressed 
>> REPLACE-C-SID flavored SID (as shown in Figure 2) carrying the common 
>> Locator-Block, the Locator-Node and Function of the first C-SID, and 
>> an argument with the index value 0.This first SID is copied into the 
>> Destination Address field of the IPv6 header either at the SR source 
>> node, if it is in the first position in the segment list, or at the 
>> previous SR segment endpoint node."
>>
>> 11.5 " When processing a SID with the REPLACE-C-SID flavor, if no SRH 
>> is present, the SR segment endpoint node ignores the index value in 
>> the SID argument and processes the upper-layer header as per [RFC8986]."
>>
>> -> "When processing a SID with the REPLACE-C-SID flavor, if no SRH is 
>> present, the SR segment endpoint node MUST ignore the index value in 
>> the SID argument and process the upper-layer header as per [RFC8986]."
>>
>> MUST or SHOULD, IMO.
>>
>> 11.6    " If an SRH is present, the SR segment endpoint node 
>> decrements the index value in the SID argument or, if it is 0, 
>> instead decrements the Segments Left value in the SRH and resets the 
>> index value in the SID argument to the C-SID container capacity minus 1."
>>
>> ->" If an SRH is present, the SR segment endpoint node MUST decrement 
>> the index value in the SID argument or, if it is 0 itMUST decrement 
>> the Segments Left value in the SRH instead and it MUST reset the 
>> index value in the SID argument to the C-SID container capacity minus 1."
>>
>> May be my proposal is not ideal but definitely the verbs like 
>> ignores/decrements/resets are too inconcrete for the such important 
>> draft, IMO.
>>
>> 11.7"The updated index value indicates the position of the next C-SID 
>> within the C-SID container at the "Segment List" index "Segments 
>> Left" in the SRH."
>>
>> I cannot get that: "... at the "Segment List" index "Segments Left" 
>> in the SRH." Surely it must be re-phrased or accompanied by some diagram.
>>
>> 11.8" The SR segment endpoint node then constructs the next SID by 
>> copyingthis next C-SID immediately after the Locator-Block in the 
>> Destination Address field of the IPv6 header, thus overwriting the 
>> previous SID Locator-Node and Function." ->
>>
>> "The SRv6 segment endpoint node then SHOULD construct the next SID by 
>> copying this next C-SID immediately after the Locator-Block in the 
>> Destination Address field of the IPv6 header, thus overwriting the 
>> previous SID Locator-Node and Function."
>>
>> 12) 4.2.1.End with REPLACE-C-SID
>>
>> 12.1 " 4.2.1.End with REPLACE-C-SID" -> "4.2.1End SID with REPLACE-C-SID"
>>
>> 12.2"When processing an IPv6 packet that matches a FIB entry locally 
>> instantiated as an End SID with the REPLACE-C-SID flavor, the SRH 
>> processing described in Section 4.1 of [RFC8986] is executed with the 
>> following modifications." ->
>>
>> " When a node processes an IPv6 packet that matches a locally 
>> instantiated FIB entry as an End SID with theREPLACE-C-SID flavor,the 
>> procedure described in Section 4.1 of [RFC8986] is executed with the 
>> following modifications: ..."
>>
>> 12.3 End.X with REPLACE-C-SID
>> 12.3.1 “When processing an IPv6 packet that matches a FIB entry 
>> locally instantiated as an End.X SID with the REPLACE-C-SID flavor, 
>> the procedure described in Section 4.2 of [RFC8986] is executed with 
>> the following modifications.“ ->
>> "When a node processes an IPv6 packet that matches a locally 
>> instantiated FIB entry as an End.X SID with the REPLACE-C-SID flavor, 
>> the procedure described in Section 4.2 of [RFC8986] is executed with 
>> the following modifications: ..."
>>
>> 12.3.2 "The pseudocode in Section 4.2.1 of this document is modified by replacing line S18 and S29 as shown below. S01. Submit the packet to the IPv6 module for transmission to the new destination via a member of J."
>>
>> I see here only line S01, where are the replacement for lines S18 and 
>> S09?
>>
>> Who is "a member of J."! is not clear from the text? RFC 8986 Section 
>> 4.2 has that information: J is a set that contains several L3 
>> adjacencies. Because reader should not jump between different 
>> documents for such details clarification, I think it will be much 
>> easier to add that info in this section of the draft:
>>
>> “ S01. Submit the packet to the IPv6 module for transmission to the 
>> new destination via a member of J.,where J is a set that contains 
>> several L3 adjacencies “
>>
>> 13) 4.2.3.End.T with REPLACE-C-SID
>>
>> 13.1" When processing an IPv6 packet that matches a FIB entry locally 
>> instantiated as an End.T SID with the REPLACE-C-SID flavor, the 
>> procedure described in Section 4.3 of [RFC8986] is executed with the 
>> following modifications." -> " When a node processes an IPv6 packet 
>> that matches a locally instantiated FIB entry as an End SID with 
>> theREPLACE-C-SID f flavor, the procedure described in Section 4.3 of 
>> [RFC8986] is executed with the following modifications: ..."
>>
>> 13.2"The pseudocode in Section 4.2.1 of this document is modified by 
>> replacing line S18 and S29 as shown below.
>>
>> S01. Set the packet's associated FIB table to T.
>>
>> S02. Submit the packet to the egress IPv6 FIB lookup for transmission 
>> to the new destination."
>>
>> So lines S18 and S29 should be replaced by S01, S02? Hm-m, sounds 
>> confusing for the reader.
>>
>> 14)4.2.4.End.B6.Encaps with REPLACE-C-SID
>>
>> 14.1 " When processing an IPv6 packet that matches a FIB entry 
>> locally instantiated as an End.B6.Encaps SID with the REPLACE-C-SID 
>> flavor, the procedure described in Section 4.13 of [RFC8986] is 
>> executed with the following modifications."
>>
>> ->"When a node processes an IPv6 packet that matches a locally 
>> instantiated FIB entry as an End.B6.Encaps SID with theREPLACE-C-SID 
>> flavor, the procedure described in Section 4.13 of [RFC8986] is 
>> executed with the following modifications: ..."
>>
>> 14.2Confusion again with pseudocode lines numbering:
>>
>> "The pseudocode in Section 4.2.1 of this document is modified by 
>> replacing line S18 and S29 as shown below."
>>
>> But again, the example has the lines S01-S05, how to map them to S18 
>> and S29? Each vendor should decide by himself?
>>
>> 15)4.2.5.End.B6.Encaps.Red with REPLACE-C-SID
>>
>> 15.1 "When processing an IPv6 packet that matches a FIB entry locally 
>> instantiated as an End.B6.Encaps.Red SID with the REPLACE-C-SID 
>> flavor, the procedure described in Section 4.14 of [RFC8986] is 
>> executed with the same modifications as in Section 4.2.4 of this 
>> document."->
>>
>> "When a node processes an IPv6 packet that matches a locally 
>> instantiated FIB entry as an End.B6.Encaps SID with theREPLACE-C-SID 
>> flavor, the procedure described in Section 4.14 of [RFC8986] is 
>> executedwith the same modifications as in Section 4.2.4 of this 
>> document."
>>
>> 16)4.2.6.End.BM with REPLACE-C-SID
>>
>> 16.1 " When processing an IPv6 packet that matches a FIB entry 
>> locally instantiated as an End.BM SID with the REPLACE-C-SID flavor, 
>> the procedure described in Section 4.15 of [RFC8986] is executed with 
>> the following modifications."    -> "When a node processes an IPv6 
>> packet that matches a locally instantiated FIB entry as an End.BM SID 
>> with theREPLACE-C-SID flavor, the procedure described in Section 4.15 
>> of [RFC8986] is executed with the following modifications: ..."
>>
>> 16.2 Again issue with pseudocode numbering (S18, S29 -> S01, S02) as 
>> I already mentioned above.
>>
>> 16.3"S01. Push the MPLS label stack for B."
>>
>> In RFC 8986 Section 4.15 is a special comment which clarifies what is 
>> B: " Any SID instance of this behavior is associated with an SR-MPLS 
>> Policy B.". But in this draft it is not clarified and looks confusing 
>> (a reference or clarification is needed.)
>>
>> 17)4.2.7.End.DX and End.DT with REPLACE-C-SID
>>
>> 7.1 " .... These new behaviors can be used to indicate the capability 
>> of compression of Node and SID, which are required in path 
>> computation and compressed SID list encoding."
>>
>> "Can be used" ? May be at least " MAY or SHOULD be used"?
>>
>> 17.2 "  As per Section 6.4, when allocating a C-SID value from a 
>> Local Identifiers Block (LIB), an extra prefix of 
>> Locator_block:FunctionID::/LBL+FL is required on the Segment 
>> Endpointnode to support LIB matching in forwarding.The new behaviors 
>> with REPLACE-C-SID flavor explicitly require the node to do so in SID 
>> initiation."
>>
>> I did not get - why an extra prefix is required from this text... :( 
>> Looks I just have to believe.
>>
>> 17.2.1 I think LIB abbreviation has to be added to Terminology part.
>>
>> 17.3 " When processing an IPv6 packet that matches a FIB entry 
>> locally instantiated as an End.DX6, End.DX4, End.DT6, End.DT4, 
>> End.DT46, End.DX2, End.DX2V, End.DT2U or End.DT2M SID with the 
>> REPLACE-C-SID flavor, the procedures described in [RFC8986] are 
>> executed. " -> " When a node processes an IPv6 packet that matches a 
>> FIB entry locally instantiated as an End.DX6, End.DX4, End.DT6, 
>> End.DT4, End.DT46, End.DX2, End.DX2V, End.DT2U or End.DT2M SID with 
>> the REPLACE-C-SID flavor, the procedures described in [RFC8986] are 
>> executed."
>>
>> 17.4The next paragraph is really confusing - it is a huge and I do 
>> not understand its logic: why by some reasons it focuses only on 
>> End.DT2M...
>>
>> It also constantly referencing Arg.FE2 without any explanation, so 
>> reader have to go to read RFC 8986, find section 4.15 there and find 
>> its meaning. So I would suggest to add Arg.FE2 to Terminology part 
>> too if it is so important (again I did not understand - why). I would 
>> also add some explanation diagram here.
>>
>> 18.4.2.8.Combination with PSP, USP, and USD flavors
>>
>> 18.1 "PSP: When combined with the REPLACE-C-SID flavor, the 
>> additional PSP flavor instructions defined in Section 4.16.1.2 of 
>> [RFC8986] are inserted after line S17 and S28 of the pseudocode in 
>> Section 4.2.1, and the first line of the inserted instructions after 
>> S28 is modified as follows." ->
>>
>> "PSP: When combined with the REPLACE-C-SID flavor, the additional PSP 
>> flavor instructions defined in Section 4.16.1.2 of [RFC8986] are 
>> inserted after *lines*S17 and S28 of the pseudocode in Section 4.2.1, 
>> and the first line of the inserted instructions after S28 is modified 
>> as follows:"
>>
>> It says that those additional instructions added after lines S17 and S28.
>>
>> But later only the instruction S.28.1 is mentioned, what about S17.1 ?
>>
>> 18.2 " S28.1.If (Segments Left == 0 and (DA.Arg.Index == 0 or Segment 
>> List[0][DA.Arg.Index-1] == 0)) { "
>>
>> I also think that final open { mightnot be needed here, better to 
>> double check.
>>
>> 19. 5.C-SID Allocation
>>
>> 19.1" In order to efficiently manage the C-SID numbering space, it 
>> may be beneficial to divide it into two non-overlapping sub-spaces: a 
>> Global Identifiers Block (GIB) and a Local Identifiers Block (LIB).  "
>>
>> GIB has to be added to Terminology part too.
>>
>> "it may be beneficial" sounds too open and inconcrete. IMO, at 
>> least:"... MAY be divided into..."
>>
>> 20) 5.1.Global C-SID
>>
>> 20.1"A Global C-SID typically identifies a shortest path to a node in 
>> the SRv6 domain.An IP route is advertised by the parent node to each 
>> of its global C-SIDs, under the associated Locator-Block.The parent 
>> node executes a variant of the End behavior." -> "A Global C-SID 
>> typically identifies a shortest path to a node in the SRv6 domain.An 
>> *IPv6 *route is advertised by the parent node to each of its global 
>> C-SIDs, under the associated Locator-Block.The parent node SHOULD 
>> execute a variant of the END behavior."
>>
>> 20.2  "A node can have multiple global C-SIDs under the same 
>> Locator-Block (e.g., one per IGP flexible algorithm).Multiple nodes 
>> may share the same global C-SID (anycast)."-> " A node MAY have 
>> multiple global C-SIDs under the same Locator-Block (e.g., one per 
>> IGP flexible algorithm).Multiple nodes MAY share the same global 
>> C-SID (anycast)."
>>
>> 21)5.2.Local C-SID
>>
>> 21.1 " No IP route is advertised by a parent node for its local C-SIDs."
>>
>> -> " No IPv6 route is advertised by a parent node for its local C-SIDs."
>>
>> 22) 7.2.Segment List Compression
>>
>> 22.1I would add some explanation diagram about compression here.
>>
>> 23) 8.Inter Routing Domains with the End.XPS behavior
>>
>> 23.1 It is great to have such solution here !
>>
>> *END of review.*
>>