Hi Gunter, Many thanks for your detailed review.
I will work on your comments with the co-authors and hope to provide a reply during the week of Oct/14. Thanks, Francois On 2 Oct 2024 at 15:58:10, Gunter van de Velde (Nokia) < gunter.van_de_ve...@nokia.com> wrote: > # Gunter Van de Velde, RTG AD, comments for > draft-ietf-spring-srv6-srh-compression-18 > > # line numbers are derived with the idnits tool > https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-spring-srv6-srh-compression-18.txt > > > #GENERIC COMMENTS > #================ > # This document is an intense read. It took time to process and get > familiar with the documented procedures and concepts. During my review > process i had the unique experience to review the document through the > looking glass as a person "new to C-SID details". This is important as > that gave me a greenfield understanding of the documented C-SID details. > > # terminology: the codepoint registrations use "CSID" in the flavor name > but everywhere in the draft "C-SID" is used. Is that by intention? > > # The documented interpretation of the terms "flavor" and "behavior" > appears to be unusual. In RFC 8986, behaviors such as End, End.X, etc., are > defined, and flavors like PSP, USP, and USD provide additional information > on how to process those behaviors. Specifically, Section 4.16 of RFC 8986 > describes flavors as variants: > > "The Penultimate Segment Pop (PSP) of the SRH, Ultimate Segment Pop (USP) > of the SRH, and Ultimate Segment Decapsulation (USD) flavors are variants > of the End, End.X, and End.T behaviors. The End, End.X, and End.T behaviors > can support these flavors either individually or in combinations." > > RFC 8986 suggests that the End, End.X, and End.T behaviors can be combined > with flavors. This document defines new flavors that appear to exhibit > different characteristics from what RFC 8986 considers to be a flavor. For > example, the End behavior can be combined with the PSP and USP flavors, > whereas the End behavior with the NEXT-C-SID together with REPLACE-C-SID > flavors cannot be combined. This difference causes confusion regarding the > understanding and distinction between "behavior" and "flavor" and needs > clarification. > > # The draft specifies that C-SIDs should be used as the destination > address of an IPv6 packet, with or without an SRH. However, it does not > provide guidance on whether using a C-SID construct or container as a > source address is valid, invalid, or forbidden. > > # This document contains valuable operational C-SID guidelines scattered > throughout its sections, which may make it challenging for operators to > locate relevant recommendations. Collecting all SRv6 C-SID guidelines, > recommendations, and considerations into a single "Operational > Considerations" section or, alternatively, into a separate SRv6OPS document > would be a reasonable approach. > > # section "7. Inter-Domain Compression" describes a procedure to swap > Locator-Blocks. This seems rather different from procedures about > NEXT-C-SID and REPLACE-C-SID and is maybe better discussed in its own > separate document. > > #DETAILED COMMENTS > #================= > > > 150 The Segment Routing (SR) architecture [RFC8402] describes two data > 151 plane instantiations of SR: SR over MPLS (SR-MPLS) and SR over IPv6 > 152 (SRv6). > > GV> RFC8402 talks about the instantiation of SR *on* MPLS/IPv6 > *data-plane*, and not *over*. > A more accurate text blob, using RFC8402 text would be: > > " > The Segment Routing (SR) architecture [RFC8402] describes two data > plane instantiations of SR: SR applied to the MPLS-architecture > (SR-MPLS) and SR applied to the IPv6-architecture (SRv6). > " > > 154 SRv6 Network Programming [RFC8986] defines a framework to build a > 155 network program with topological and service segments (also referred > 156 to by their Segment Identifier (SID)) carried in a Segment Routing > 157 Header (SRH) [RFC8754]. > > GV> This statement is not fully accurate. RFC8986 also describe Reduced > SRH where a SID is carried outside the SRH to reduce the size of the SRH. > > 159 Some SRv6 applications such as strict path traffic engineering may > 160 require long segment lists. Compressing the encoding of these long > 161 segment lists in the packet header can significantly reduce the > 162 header size. This document specifies new flavors to the SR segment > 163 endpoint behaviors defined in [RFC8986] that enable a compressed > 164 encoding of the SRv6 segment list. > > GV> This document seems to define additional behaviors, not only new > flavors of existing behaviors. > For example END.PS and END.XPS appear to be new behaviors, not new > flavors of behaviors defined in RFC8986. > > 183 * Locator-Node: The least significant bits of a SID locator that > 184 identify the SR segment endpoint node instantiating the SID. The > 185 Locator-Node is referred to as "N" in Section 3.1 of [RFC8986]. > > GV> section 3.1 describe "N" as the "N is the identifier of the parent > node instantiating the SID". > Is this difference in terminology "endpoint" vs "parent node" by intent? > > 187 * Compressed-SID (C-SID): A compressed encoding of a SID. The > C-SID > 188 includes the Locator-Node and Function bits of the SID being > 189 compressed. > > GV> I wonder if this is accurate. When a CSID is a global node-level > identifier, the function bits are set to zero and are therefore omitted. > Similarly, when the CSID is a local identifier without an associated global > node-level CSID, the Locator Node bits are also zero and omitted. > > 191 * C-SID container: A 128-bit container holding a list of one or > more > 192 C-SIDs. > > GV> This may need some additional clarification for an accurate > description. > rfc8402 describe a SRv6 segment as an IPv6 address, in addition RFC8754 > describe a segment within the SRH as an 128-bit IPv6 address. The above > CSID container says its simply a 128-bit thing. A more accurate description > would be: > > " > * C-SID container: A 128-bit IPv6 address that functions as a container > holding a list of one or more C-SIDs. > " > > 194 * C-SID sequence: A group of one or more consecutive SID list > 195 entries carrying the common Locator-Block and at least one C-SID > 196 container. > > GV> Note that the verb "carry" is not entirely clear in above context. > > " > * C-SID sequence: A group of one or more consecutive SID list > entries encoding the common Locator-Block and at least one C-SID > container. > " > > 187 * Compressed-SID (C-SID): A compressed encoding of a SID. The > C-SID > 188 includes the Locator-Node and Function bits of the SID being > 189 compressed. > > GV> a *compressed* SID list can contain *uncompressed* SIDs > > 245 The compressed segment list encoding is fully compatible with and > 246 builds upon the mechanisms specified in [RFC8754] and [RFC8986]. > The > 247 compressed encoding leverages a SID list compression logic on the SR > 248 source node (Section 6) in combination with new flavors of the base > 249 SRv6 segment endpoint behaviors that process the compressed SID list > 250 (Section 4). > > GV> In the draft -18 there are 56 instances of the term "SR source node" > without an explicit definition of "what is" a SR source node from a C-SID > perspective. This should be clarified and defined in terminology section 2 > > GV> What about an editorial rewrite to make the text more accurate: > "Building upon and fully compatible with the mechanisms specified in > [RFC8754] and [RFC8986], the compressed segment list encoding leverages SID > list compression logic at the SR source node (see Section 6) in combination > with both new and existing flavors of the base SRv6 segment endpoint > behaviors that process the compressed SID list (see Section 4)." > > 252 A segment list can be encoded in the packet header using any > 253 combination of compressed and uncompressed sequences. The C-SID > > GV> Line194 in terminology section introduce C-SID sequence. How does > C-SID sequence correlate with uncompressed sequence? Could uncompressed > sequence description be added in the terminology section? > > 252 A segment list can be encoded in the packet header using any > 253 combination of compressed and uncompressed sequences. The C-SID > 254 sequences leverage the flavors defined in this document, while the > 255 uncompressed sequences use behaviors and flavors defined in other > 256 documents, such as [RFC8986]. An SR source node constructs and > 257 compresses the SID list depending on the SIDs instantiated on each > SR > 258 segment endpoint node that the packet should traverse, as well as > its > 259 own compression capabilities. > > GV> There is an inconsistency. In the text is mentioned "leverage the > flavors defined in this document", however in section 4.1 can be found: > > " > When a C-SID sequence comprises at least two C-SIDs, the last C-SID > in the sequence is not required to have the NEXT-C-SID flavor. ***It > can be bound to any behavior and flavor(s)*** > " > > GV> s/packet should traverse/packet intended to traverse/ > > 274 behaviors of [RFC8986]. A counterpart NEXT-C-SID flavor is not > 275 defined for these SIDs because they can be included within a C-SID > 276 sequence that uses the NEXT-C-SID flavor without any modification of > 277 the procedure defined in [RFC8986]. > > GV> What does "they can be included within a C-SID sequence" exactly mean? > Can this be explained how this is intended to function? > > 297 SRv6 is intended for use in a variety of networks that require > 298 different prefix lengths and SID numbering spaces. Each of the two > 299 flavors introduced in this document comes with its own > 300 recommendations for Locator-Block and C-SID length, as specified in > 301 Section 4.1 and Section 4.2. These flavors are best suited for > 302 different environments, depending on the requirements of the > network >
_______________________________________________ spring mailing list -- spring@ietf.org To unsubscribe send an email to spring-le...@ietf.org