Re: [spring] Chair Review of draft-ietf-spring-srv6-srh-compression-11

Francois Clad <fclad.ietf@gmail.com> Thu, 29 February 2024 17:50 UTC

Return-Path: <fclad.ietf@gmail.com>
X-Original-To: spring@ietfa.amsl.com
Delivered-To: spring@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 88922C14F685; Thu, 29 Feb 2024 09:50:50 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -7.105
X-Spam-Level:
X-Spam-Status: No, score=-7.105 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_HI=-5, RCVD_IN_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=ham autolearn_force=no
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id Mlm23glwEh8C; Thu, 29 Feb 2024 09:50:48 -0800 (PST)
Received: from mail-ej1-x634.google.com (mail-ej1-x634.google.com [IPv6:2a00:1450:4864:20::634]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 8E4F5C14F5FC; Thu, 29 Feb 2024 09:50:47 -0800 (PST)
Received: by mail-ej1-x634.google.com with SMTP id a640c23a62f3a-a3e85a76fa8so179418366b.1; Thu, 29 Feb 2024 09:50:47 -0800 (PST)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20230601; t=1709229045; x=1709833845; darn=ietf.org; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=JDDqu7jekLqtwSAZbncfTzJznHh5EGj1CKccrWWayK8=; b=OI3U1/O7i4pVAW94qPZWtPHT97BAuLO9LF4NxJlyxwhDbyjGHAqpanq0XV66QM10O0 HI9wck6TURuIOOrgknlmQ2Egj95Oz1QHgKiSIMP9QhPP1CH49FCFn8PyN14qmJ04xYKN sJvUsluomLte6nByc7gWzSQnFp2cqiB8hhs4K+YP8K1R0hAjUe6xNB5TrHfdAyI4Plp/ tb6CGXZKEdG+O+v20DQIPRX9G2x1JkKd1nZYnCq0kCudSf5qWsJRmHyOz25qZylV6/fV Ek9fJliwnAZokKfEegmbss+6n6rSc3Uf9QA3VIaoFwaASzEjywEbBbLqDiT6wY63xQhy m13Q==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1709229045; x=1709833845; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=JDDqu7jekLqtwSAZbncfTzJznHh5EGj1CKccrWWayK8=; b=byVynlpbswqRUDNF3Duxy4lk0BFSMHi2laPvDc4UTCuGs+hf0NApfnuiWIdVdz3IhJ tSDcmK9DERyAcjK3hUq7wry7Xm+bjLmqFl75N4UECpIYHxUUvTrs3q1btRR3lFm82nnY uj3SoaE1yys4gylYuA5ulnWNKzju9MI0F0/T+lCzzdhOYeOyvZn3kcty5ns+n4fVbulr GBUJhc5ZDKvrpHY5WSDbw+2BuEQjSR9jUdUKLdoylQaf7n/eYCYouTn6lMLRMMMpHBPm u2vRgppwAY2SGIxAKiuMzD+5OiBxuUJ+gsjqGs6wukkz+G9YmKK/8nzgPXqhRX4dwGiN HIhg==
X-Forwarded-Encrypted: i=1; AJvYcCVQbs//KlNCbr8woGdfTOfSDWVoTiS4DKEyS1xWTMgTotCJLD824Wh7NoD1ynyvCPByfmElVDoEjBbfwzT4xABNvR6x7m64TLFrncgJg04L7a9XGSc6QiRg/RnVU0JqRVpmjxFcIAf9XleDCN7OtnO/QeBWk+clRQ==
X-Gm-Message-State: AOJu0YwZSRpZRJa0/Coko4EXmN2Xy6fpShWIt8xfasHXuxBfoRroj2bK tdeQj2lfJNnZubIDDQAFFdxay8eDcS9TT+LbVpdYPxJA5MqQGOz6DPyvl1aJiMPnxKoEG70SB20 +7PxIjzYPwbC29kNEjPWfEgpwry3sJIRexw==
X-Google-Smtp-Source: AGHT+IEMjkG+ZGDWUgLKBA6gnBlE+rUBqvztYWFMZx3wXropLffuRMYkmnFHEhfCXL4bQkqspmdHJCQ9SRpM73H9OSY=
X-Received: by 2002:a17:906:cd1f:b0:a43:db94:cac4 with SMTP id oz31-20020a170906cd1f00b00a43db94cac4mr2149279ejb.21.1709229045251; Thu, 29 Feb 2024 09:50:45 -0800 (PST)
Received: from 1064022179695 named unknown by gmailapi.google.com with HTTPREST; Thu, 29 Feb 2024 11:50:44 -0600
Received: from 1064022179695 named unknown by gmailapi.google.com with HTTPREST; Thu, 29 Feb 2024 11:50:41 -0600
MIME-Version: 1.0 (Mimestream 1.2.6)
References: <CAMMESsw=PihfkO3nECiBnCALfCC=vTRn6c1_OYPK-jT5=yHFZA@mail.gmail.com>
In-Reply-To: <CAMMESsw=PihfkO3nECiBnCALfCC=vTRn6c1_OYPK-jT5=yHFZA@mail.gmail.com>
From: Francois Clad <fclad.ietf@gmail.com>
Date: Thu, 29 Feb 2024 11:50:44 -0600
Message-ID: <CAHT6gR9ytPWVDBdSnoKmofzN2cfEQhS5siV-i405XMP-_=27hw@mail.gmail.com>
To: Alvaro Retana <aretana.ietf@gmail.com>
Cc: SPRING WG List <spring@ietf.org>, "spring-chairs@ietf.org" <spring-chairs@ietf.org>, "draft-ietf-spring-srv6-srh-compression@ietf.org" <draft-ietf-spring-srv6-srh-compression@ietf.org>
Content-Type: multipart/alternative; boundary="0000000000001503ea061288e663"
Archived-At: <https://mailarchive.ietf.org/arch/msg/spring/llb0waePAtXTtN1DJ3y3sQqo8_4>
Subject: Re: [spring] Chair Review of draft-ietf-spring-srv6-srh-compression-11
X-BeenThere: spring@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: "Source Packet Routing in NetworkinG \(SPRING\)" <spring.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/spring>, <mailto:spring-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/spring/>
List-Post: <mailto:spring@ietf.org>
List-Help: <mailto:spring-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/spring>, <mailto:spring-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 29 Feb 2024 17:50:50 -0000

 Hi Alvaro,

Thank you for your review and comments.

We have integrated those changes as part of revisions -12 and -13 of the
document. Please find our detailed replies inline.

Thanks,
Francois

> Dear authors:
>
> In parallel with the WGLC I have reviewed this document.  Thank you
> for the work you've put into it so far.
>
> I have several comments (in-line below) that I would like to see
> addressed.  In general, I think my comments should be relatively easy
> to address.  I want to highlight one point up-front:
>
> Operational Considerations/Guidance
>
>    Dhruv brought up [1] the point of providing "guidance on when to use
>    which flavor and with which C-SID lengths".  I fully agree!  The
>    document contains (mostly in §4) recommendations, for example, about
>    LBL and C-SID lengths, even if any other value is possible.  IOW, the
>    possibilities are endless!  Please provide more operational
>    considerations on how an operator can evaluate their needs and select
>    the appropriate flavor/value for their deployment.

We added a paragraph providing such operational guidance in revision -12,
and further clarified it in revision -13.

| SRv6 is intended for use in a variety of networks that require
| different prefix lengths and SID numbering spaces.  Each of the two
| flavors introduced in this document comes with its own
| recommendations for Locator-Block and C-SID length, as specified in
| Section 4.1 and Section 4.2.  These flavors are best suited for
| different environments, depending on the requirements of the network.
| For instance, larger C-SID lengths may be more suitable for networks
| requiring ample SID numbering space, while smaller C-SID lengths are
| better for compression efficiency.  The two compression flavors allow
| the compressed segment list encoding to adapt to a range of
| requirements, with support for multiple compression levels.  Network
| operators can choose the flavor that best suits their use case,
| deployment design, and network scale.

>    I included some in-line comments below.
>
>
> Thanks!
>
> Alvaro.
>
>
> [1]
https://mailarchive.ietf.org/arch/msg/spring/p3GGcuoqOjHaLjrJJaQpKzURTn0
>
>
>
> [Line numbers from idnits]
>
> ...
> 142 1.  Introduction
> ...
> 160   The flavors defined in this document leverage the SRv6 data plane
> 161   defined in [RFC8754] and [RFC8986], and are compatible with the SRv6
> 162   control plane extensions for IS-IS [RFC9352], OSPF [RFC9513], and
BGP
> 163   [RFC9252].
>
> [minor] rfc9252 is about SRv6-based BGP services -- is it the best
> (general) BGP reference to use at this point?

RFC 9252 defines the SRv6 Service TLVs that are used to advertise the
End.DX6, End.DX4, End.DT6, End.DT4, End.DT46, End.DX2, End.DX2V, End.DT2U,
and End.DT2M SID with a C-SID flavor.

> 165 2.  Terminology
> ...
> 188   *  C-SID sequence: A group of one or more consecutive segment list
> 189      entries carrying the common Locator-Block and at least one C-SID
> 190      container.
>
> [minor] It seems to me that this definition is a little off -- looking
> at it from the point of view of a "C-SID sequence" having to be a
> sequence of C-SIDs, not a of segment list entries.
>
> Suggestion:
>
>    *  C-SID sequence: A group of one or more consecutive C-SIDs sharing
>       a common Locator-Block and at least one C-SID container.

The term "C-SID sequence" denotes the encoding of a sequence of C-SIDs as
one or more consecutive elements in a compressed segment list. This
encoding comprises the shared Locator-Block and the C-SIDs themselves.

> 192   *  Uncompressed SID sequence: A group of one or more uncompressed
> 193      SIDs in a segment list.
>
> [minor] The definition of a "C-SID sequence" pointed at the fact that
> a sequence is "consecutive".  Is that the case here too?

Correct. We clarified this in revision -13.

| *  Uncompressed SID sequence: A group of one or more consecutive
|    uncompressed SIDs in a segment list.

> ...
> 229 3.  Basic Concepts
> ...
> 246   A segment list can be encoded in the packet header using any
> 247   combination of compressed and uncompressed sequences.  The C-SID
> 248   sequences leverage the flavors defined in this document, while the
> 249   uncompressed sequences use behaviors and flavors defined in other
> 250   documents, such as [RFC8986].  An SR source node constructs and
> 251   compresses the SID-list depending on the capabilities of each SR
> 252   segment endpoint node that the packet should traverse, as well as
its
> 253   own compression capabilities.
>
> [minor] "An SR source node constructs and compresses the SID-list
> depending on the capabilities of each SR segment endpoint node that
> the packet should traverse..."
>
> Which "capabilities of each SR segment endpoint node" are you
> referring to?  How does the SR source node learn about them?  Later in
> the text you talk about advertising the C-SIDs and the SRv6 SID
> Structure, but those are not "node capabilities".

The term "capabilities" here refers to the SIDs instantiated on each of
these nodes, with their respective behavior, flavor(s), and structure. We
swapped the term "capabilities" with "SIDs instantiated on" to avoid any
confusion with node capabilities.

| An SR source node constructs and
| compresses the SID-list depending on the SIDs instantiated on each SR
| segment endpoint node that the packet should traverse, as well as its
| own compression capabilities.

> ...
> 261 4.  SR Segment Endpoint Flavors
> ...
> 291   The SIDs of both flavors can co-exist in the same SR domain, on the
> 292   same SR segment endpoint node, and even in the same segment list.
> 293   However, it is RECOMMENDED, for ease of operation, that a single
> 294   compressed encoding flavor be used in a given routing domain.  In a
> 295   multi-domain deployment, different flavors MAY be used in different
> 296   routing domains of the SR domain.
>
> [major] s/MAY/may
>
> The fact that two flavors exist allows for different ones being used
> in different routing domains.  IOW, the use of "MAY" doesn't add any
> normative value.

Fixed in revision -13.

> ...
> 311 4.1.  NEXT-C-SID Flavor
>
> 313   A C-SID sequence using the NEXT-C-SID flavor comprises one or more
> 314   C-SID containers.  Each C-SID container is a fully formed 128-bit
> 315   SID.  It carries a Locator-Block followed by a series of C-SIDs.
The
> 316   Locator-Node and Function of the C-SID container are those of the
> 317   first C-SID, and its Argument is the contiguous series of subsequent
> 318   C-SIDs.  The second C-SID is encoded in the most significant bits of
> 319   the C-SID container Argument, the third C-SID is encoded in the bits
> 320   of the Argument that immediately follow the second C-SID, and so on.
> 321   When all C-SIDs have the same length, a C-SID container can carry up
> 322   to K C-SIDs, where K is computed as floor((128-LBL)/LNFL).  Each
> 323   C-SID container for NEXT-C-SID is independent, such that contiguous
> 324   C-SID containers in a C-SID sequence can be considered as separate
> 325   C-SID sequences.
>
> [major] It should be specified that any remaining bits MUST be 0-padded.

It is correct that the SIDs are 0-padded. However, this rule is already
specified with a normative "MUST" in section 3.1 of RFC 8986 and applies to
all SRv6 SIDs.

> [major] Please add a reference or explanation of the floor function.
> I know it may be clear to many, but no assumptions should be made
> about future readers.

We added a definition and reference for the floor function in revision -13.

| When all C-SIDs have the same
| length, a C-SID container can carry up to K C-SIDs, where K is
| computed as floor((128-LBL)/LNFL) (floor(x) is the greatest integer
| less than or equal to x [GKP94]).
...
| [GKP94]    Graham, R., Knuth, D., and O. Patashnik, "Concrete
|            Mathematics: A Foundation for Computer Science",
|            ISBN 9780201558029, 1994.

> 327   The last C-SID in the C-SID sequence is not required to have the
> 328   NEXT-C-SID flavor.  It can be bound to any behavior and flavor(s),
> 329   including the REPLACE-C-SID flavor, as long as it meets the
> 330   conditions defined in Section 6.
>
> [minor] Which conditions exactly?  Pointing to a specific part of §6 may
help.

We clarified those conditions and added a more specific pointer in revision
-13.

| 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), including the REPLACE-
| C-SID flavor, as long as the updated destination address resulting
| from the processing of the previous C-SID in the sequence is a valid
| form for that last SID.  Line S12 of the first pseudocode in
| Section 6.2 provides sufficient conditions to ensure this property.

> 332   The structure of a SID with the NEXT-C-SID flavor is shown in
> 333   Figure 1.  The same structure is also that of a C-SID container
> 334   carrying NEXT-C-SID SIDs.
>
> [nit] It would be nice to point at Figure 1 earlier in this section.

Fixed in revision -13.

| A C-SID sequence using the NEXT-C-SID flavor comprises one or more
| C-SID containers.  Each C-SID container is a fully formed 128-bit SID
| structured as shown in Figure 1.

> [] I don't understand what the second sentence is trying to say.

This sentence says that the structure of a C-SID container for NEXT-C-SID
SIDs is the same as that of an individual, uncompressed NEXT-C-SID SID.
However, this is now redundant with the first paragraph so we removed the
sentence in revision -13.

> 336   +------------------------------------------------------------------+
> 337   |     Locator-Block      |Loc-Node|            Argument            |
> 338   |                        |Function|                                |
> 339   +------------------------------------------------------------------+
> 340    <-------- LBL ---------> < LNFL > <------------- AL ------------->
>
> 342        Figure 1: Structure of a NEXT-C-SID flavor SID (scaled for a
> 343     48-bit Locator- Block, 16-bit combined Locator-Node and Function,
> 344                            and 64-bit Argument)
>
> 346   An implementation SHOULD support a 32-bit Locator-Block length (LBL)
> 347   and a 16-bit C-SID length (LNFL) for NEXT-C-SID flavor SIDs, and MAY
> 348   support any other Locator-Block and C-SID length.  A deployment
> 349   SHOULD use a consistent Locator-Block length and C-SID length for
all
> 350   SIDs of the SR domain.
>
> [major] "SHOULD support a 32-bit Locator-Block length (LBL) and a
> 16-bit C-SID length (LNFL) for NEXT-C-SID flavor SIDs, and MAY support
> any other Locator-Block and C-SID length"
>
> What are you trying to specify here?
>
> By only recommending the LBL/LNFL lengths, there is no requirement
> that implementations will support that.  IOW, if you're trying to
> define a mandatory-to-implement minimum, then s/SHOULD/MUST
>
> The "MAY" has no normative value: s/MAY/may

Yes, the intent is to define a minimum common ground for interoperability.
We changed the "SHOULD" to "MUST" and "MAY" to "may" in revision -13.

> [major] "SHOULD use a consistent Locator-Block length and C-SID length
> for all SIDs of the SR domain"
>
> When is it ok to not be consistent?  IOW, why is this recommended and
> not required?  What are the effects of not being consistent?

Deployments may use any Locator-Block and C-SID length. However, the C-SID
compression relies on common Locator-Block and consistent C-SID length, so
using inconsistent ones, while possible, will lead to reduced compression
efficiency.

> ...
> 355   When processing an IPv6 packet that matches a FIB entry locally
> 356   instantiated as a SID with the NEXT-C-SID flavor, the SR segment
> 357   endpoint node applies the procedure specified in the one following
> 358   subsection that corresponds to the SID behavior.  If the SID also
has
> 359   the PSP, USP, or USD flavor, the procedure is modified as described
> 360   in Section 4.1.7.
>
> [nit] s/in the one following subsection/in the following subsection

Fixed in revision -13.

> 362   An SR segment endpoint node instantiating a SID with the NEXT-C-SID
> 363   flavor MUST accept any Argument value for that SID.
>
> [major] Does this also mean that any future behavior cannot make use
> of an Argument?  IOW, behaviors like End.DT2M cannot be used with the
> NEXT-C-SID flavor.  If so, please be explicit about it.

This statement does not impose any requirement on future behaviors.

The argument of the NEXT-C-SID flavor SIDs defined in this document only
contains the following C-SIDs in the container, for which an endpoint node
must accept any value.

A future document defining another NEXT-C-SID flavor SID whose argument
contains other pieces of information will need to define the structure of
that argument and acceptable values. Most likely, the part of the argument
carrying the following C-SIDs will follow the same rule as stated here.

> ...
> 377 4.1.1.  End with NEXT-C-SID
> ...
> 384   The below pseudocode is inserted between lines S01 and S02 of the
SRH
> 385   processing in Section 4.1 of [RFC8986].  In addition, this
pseudocode
> 386   is executed before processing any extension header that is not an
> 387   SRH, a Hop-by-Hop header or a Destination Option header, or before
> 388   processing the upper-layer header, whichever comes first.
>
> [major] "In addition..."
>
> This sentence is not needed because S01 says "When an SRH is
> processed", so we're already processing the SRH.  Also, this sentence
> is paraphrasing the ordering in §4/rfc8200 -- which makes it
> unnecessary as the behavior is already specified elsewhere.
>
> Furthermore, Appendix A.1 shows the pseudocode being executed "before
> processing the upper-layer header".  However, that upper-layer header
> would only be processed *after* the SRH is processed (rfc8200) -- so
> doing it again is unnecessary.
>
> Please remove both the sentence above and the extra step in A.1
> *before* the upper-layer header.
>
> ** Note that other descriptions in this section also contain the same
> text and should be modified in the same way (including the
> appendices).
>

The SRH processing is performed when the packet contains an SRH. The
sentence that you quoted (and the corresponding step described in appendix)
covers the cases where it does not. This may occur when the SRH is omitted
in the SRv6 encapsulation (section 4.1 of RFC 8754 or section 5 of RFC
8986) or when it is removed before the ultimate destination (section 4.16.1
of RFC 8986).

> 390   N01. If (DA.Argument != 0) {
> 391   N02.   If (IPv6 Hop Limit <= 1) {
> 392   N03.     Send an ICMP Time Exceeded message to the Source Address,
> 393              Code 0 (Hop limit exceeded in transit),
> 394              interrupt packet processing and discard the packet.
> 395   N04.   }
>
> [major] Why are the other checks not done?  For example, why are SL
> not checked?  I understand that if the previous node didn't change it
> then it should be ok -- but it may not!

This pseudocode specifies a dataplane behavior implemented in the
forwarding path of routers, where every operation matters. For that reason,
sanity checks are strictly limited to the fields used as part of the packet
processing. For instance, the value in the Segments Left field is only
validated when it is used or modified.

> ...
> 400   N07.   Decrement Hop Limit by 1.
>
> [nit] s/Decrement Hop Limit by 1/Decrement IPv6 Hop Limit by 1

Fixed in revision -13.

> ...
> 547 4.2.  REPLACE-C-SID Flavor
> ...
> 597   The RECOMMENDED Locator-Block lengths (LBL) for REPLACE-C-SID flavor
> 598   SIDs are 48, 56, 64, 72, or 80 bits, depending on the needs of the
> 599   operator.
>
> 601   The REPLACE-C-SID flavor supports both 16- and 32-bit C-SID lengths
> 602   (LNFL).  A C-SID length of 32-bit is RECOMMENDED.
>
> 604   Any other Locator-Block and C-SID length selection is possible, but
> 605   may lead to suboptimal C-SID encoding in the C-SID containers (e.g.,
> 606   presence of padding bits).
>
> [major] The first two of the three paragraphs above suggest the use of
> specific values, but it is not until the third paragraph that the
> reasons become clear -- and it is clarified that any length selection
> is possible.  This makes the initial paragraphs a little misleading
> because it gives the impression that only specific lengths are
> supported.
>
> Suggestion:
>
>    The REPLACE-C-SID flavor supports any Locator-Block and C-SID
>    length selection.  LBL values of 48, 56, 64, 72, or 80 bits,
>    and C-SID lengths of 16- or 32-bits are RECOMMENDED to avoid
>    suboptimal C-SID encoding in the C-SID containers (e.g.,
>    presence of padding bits).

We clarified the requirements and recommendations in revision -13.

| The REPLACE-C-SID flavor SIDs support any Locator-Block length (LBL),
| depending on the needs of the operator, as long as it does not exceed
| 128-LNFL-ceiling(log_2(128/LNFL)) (ceiling(x) is the least integer
| greater than or equal to x [GKP94]), so that enough bits remain
| available for the C-SID and Argument.  A Locator-Block length of 48,
| 56, 64, 72, or 80 bits is RECOMMENDED for address planning reasons.
|
| This document defines the REPLACE-C-SID flavor for 16-bit and 32-bit
| C-SID lengths (LNFL).  An implementation MUST support a 32-bit C-SID
| length for REPLACE-C-SID flavor SIDs.
|
| A deployment SHOULD use a consistent Locator-Block length and C-SID
| length for all SIDs of the SR domain.
|
| The Argument length (AL) for REPLACE-C-SID flavor SIDs is equal to
| 128-LBL-LNFL.  The index value is encoded in the least significant X
| bits of the Argument, where X is computed as ceiling(log_2(128/
| LNFL)).

> Open questions: How does an operator select the best LBL according to
> its needs?  I imagine that the LBL selection is global (or at least
> per routing domain or SR domain??) -- how are the nodes made aware of
> the LBL used/selected?

Each operator selects LBL based on their SRv6 SID addressing scheme and
preferences.

Indeed, the same LBL consistency recommendation given in section 4.1 also
applies here. We added the relevant text in revision -13 (see quoted text
above).

Section 6 describes how an SR source node learns the structure (including
LBL) of the SIDs in the domain.

> How do the nodes in the network know which C-SID length is in use?  Is
> there a consistency recommendation when using the REPLACE-C-SID
> flavor?

Correct, the consistency recommendation also applies to the C-SID length
(see quoted text above).

> 608   The Argument length (AL) for REPLACE-C-SID flavor SIDs is equal to
> 609   128-LBL-LNFL.  The index value is encoded in the least significant X
> 610   bits of the Argument, where X is computed as ceil(log_2(128/LNFL)).
>
> [major] Please add a reference or explanation of the ceil function.  I
> know it may be clear to many, but no assumptions should be made about
> future readers.

Fixed in revision -13 (see quoted text above).

> [major] Part of the guidance above about how to chose the LBL and
> C-SID length should include leaving enough bits in AL to encode the
> index.  There are many cases, but a simple one is using an LBL of 96
> and a 32-bit C-SID length, which leaves no room for the index.

Fixed in revision -13 (see quoted text above).

> ...
> 642 4.2.1.  End with REPLACE-C-SID
> ...
> 652   S02.   If (Segments Left == 0 and (DA.Arg.Index == 0 or
> 653              Segment List[0][DA.Arg.Index-1] == 0)) {
>
> [?] I don't understand why "DA.Arg.Index-1" (the "-1" part) is used.
> If DA.Arg.Index points at the bits containing the index, what does
> DA.Arg.Index-1 point to?

DA.Arg.Index represents the value contained in these bits and DA.Arg.Index
- 1 is this value reduced by 1. We clarified the definition of this term in
revision -13, as well as similar definitions in other parts of the document.

|    |     *  DA.Arg.Index identifies the value contained in the bits
|    |        [(128-ceil(log_2(128/LNFL)))..127] in the Destination
|    |        Address of the IPv6 header.

> ...
> 657   R01. If (DA.Arg.Index != 0) {
> 658   R02.   If ((Last Entry > max_LE) or (Segments Left > Last Entry)) {
>
> [?] Should "Segments Left > Last Entry" be "Segments Left > Last Entry+1"?

The pseudocode is correct. The difference between line R02 and R13 comes
from the fact that SL is decremented by 1 after line R13 and before
"Segment List[Segments Left]" is read on line R20, while SL is not
decremented after line R02.

> ...
> 823 4.2.7.  End.DX and End.DT with REPLACE-C-SID
> ...
> 831   These SIDs differ from those defined in [RFC8986] by the presence of
> 832   an Argument as part of the SID structure.  The Argument value is
> 833   effectively ignored by the SR segment endpoint node.
>
> [minor] s/value is effectively ignored/value is ignored

Fixed in revision -13.

> ...
> 840   The SR segment endpoint node obtains the value Arg.FE2 from the 16
> 841   most significant bits of DA.Argument if DA.Arg.Index is zero, or
from
> 842   the 16 least significant bits of the next position in the current
> 843   C-SID container (Segment List[Segments Left][DA.Arg.Index-1])
> 844   otherwise (DA.Arg.Index is non-zero).
>
> [?] Where does the 16-bit value come from?  rfc8986 doesn't specify
> the size of Arg.FE2, and the related applications don't seem to match
> in length.  What am I missing?

This document sets the length of Arg.FE2 to 16 bits for the End.DT2M with
REPLACE-C-SID SID. We clarified this in revision -13.

| The value of Arg.FE2 is 16-bit long.  The SR segment endpoint node
| obtains the value Arg.FE2 from the 16 most significant bits of
| DA.Argument if DA.Arg.Index is zero, or from the 16 least significant
| bits of the next position in the current C-SID container (Segment
| List[Segments Left][DA.Arg.Index-1]) otherwise (DA.Arg.Index is non-
| zero).

> ...
> 869 5.  C-SID Allocation
> ...
> 874   In order to efficiently manage the C-SID numbering space, a
> 875   deployment MAY divide it into two non-overlapping sub-spaces: a
> 876   Global Identifiers Block (GIB) and a Local Identifiers Block (LIB).
>
> [major] There's no normative value in "MAY": s/MAY/may

Fixed in revision -13.

> ...
> 920 5.3.  GIB/LIB Usage
> ...
> 926   The GIB number space is shared among all SR segment endpoint nodes
> 927   using SRv6 locators under a Block space.  The more SIDs assigned
from
> 928   this space, per node, the faster it is exhausted.  Therefore its use
> 929   is prioritized for global segments, such as SIDs that identify a
> 930   node.
>
> [nit] s/a Block space/a Locator-Block space

Fixed in revision -13.

> ...
> 942   Given the previous Locator-Block and C-SID length recommendations,
> 943   the following GIB/LIB usage is RECOMMENDED:
>
> [] s/RECOMMENDED/recommended

Fixed in revision -13.

> ...
> 963 5.4.  Recommended Installation of C-SIDs in FIB
>
> 965   An SR segment endpoint node instantiating a NEXT-C-SID or REPLACE-
> 966   C-SID flavor SID SHOULD install the corresponding FIB entry to match
> 967   only the Locator and Function parts of the SID (i.e., with a prefix
> 968   length of LBL + LNL + FL).  Any other mean of identifying a locally
> 969   instantiated SID is possible as long as it is compliant with
> 970   Section 4.3 of [RFC8754] and accepts all valid Argument values for
> 971   the SID.
>
> [major] §4.3/rfc8754 doesn't use normative language.  It uses a
> general statement that allows for different implementations:
>
>    Without constraining the details of an implementation, the SR segment
>    endpoint node creates Forwarding Information Base (FIB) entries for its
>    local SIDs.
>
> It seems to me that rfc8754 already covers what wants to be conveyed
> in this document: the FIB entry has to uniquely identify the segment
> endpoint.
>
> As written, the text raises several questions:
>
> When is it ok to not "install the corresponding FIB entry to match
> only the Locator and Function parts of the SID"?  IOW, why is this
> action recommended and not required?  Note that the entry has to at
> least cover "a prefix length of LBL + LNL + FL".
>
> The other means refer to §4.3/rfc8754, which (as shown above) says
> that anything (including "install the corresponding FIB entry to match
> only the Locator and Function parts of the SID") is ok.  This takes us
> back to my original point: rfc8754 already covers what this section
> wants to convey and it is not needed.
>
> "all valid Argument values" -- most of the SIDs used don't use an
> Argument, so which are the "valid Argument values"?   s/all valid
> Argument values/any Argument value

In general, an implementation could identify a locally instantiated SRv6
SID with argument by installing multiple /128 FIB entries, one for each
valid argument value. However, such method is not suited for NEXT-C-SID and
REPLACE-C-SID flavor SIDs given than any argument value is valid. We
clarified this in revision -13.

| Section 4.3 of [RFC8754] defines how an SR segment endpoint node
| identifies a locally instantiated SRv6 SID.  To ensure that any valid
| argument value is accepted, an SR segment endpoint node instantiating
| a NEXT-C-SID or REPLACE-C-SID flavor SID SHOULD install a
| corresponding FIB entry that matches only the Locator and Function
| parts of the SID (i.e., with a prefix length of LBL + LNL + FL).

> 973   In addition, an SR segment endpoint node instantiating NEXT-C-SID
> 974   flavor SIDs from both GIB and LIB MAY install combined "Global +
> 975   Local" FIB entries to match a sequence of global and local C-SIDs in
> 976   a single LPM lookup.
>
> [major] The GIB/LIB constructs are not visible to the FIB, and there's
> no requirement that one or the other, or both, be installed in it.
> IOW, the "MAY" lacks any normative value.  s/MAY/may

Fixed in revision -13.

> [minor] Please expand LPM.

Fixed in revision -13.

> ...
> 1080 6.2.  Segment List Compression
> ...
> 1086   It is out of the scope of this document to describe the mechanism
> 1087   through which an uncompressed segment list is derived.  As a
general
> 1088   guidance for implementation or future specification, such a
mechanism
> 1089   should aim to select the combination of SIDs that would result in
the
> 1090   shortest compressed segment list.  For example, by selecting a
C-SID
> 1091   flavor SID over an equivalent non-C-SID flavor SID or by
consistently
> 1092   selecting SIDs of the same C-SID flavor within each routing domain.
>
> [minor] This paragraph is a little confusing.  It starts by saying
> that "It is out of the scope of this document to describe the
> mechanism through which an uncompressed segment list is derived."
> While that is true, isn't an uncompressed segment list "normal
> operation"?  IOW, it is out of scope but also already defined
> elsewhere, right?
>
> §8 says that "the controller provides the ordered segment list
> comprising the uncompressed SIDs" and that "a node that does not
> support this specification...handles it as described in the
> corresponding control plane specification..."  I interpret this as
> meaning that there's no change.
>
> Am I missing something?
>
> The text then goes on to talk about "the shortest compressed segment
list"...

Yes, there are other specifications, such as RFC 8402, RFC 9256,
draft-ietf-rtgwg-segment-routing-ti-lfa, etc., which would work just fine.
The quoted text says that the segment list calculation algorithm could be
refined to minimize the length of the compressed segment list.

> 1094   The segment list that the SR source node pushes onto the packet
MUST
> 1095   comply with the rules in Section 6.3 and Section 6.4 and result in
a
> 1096   path equivalent to the original segment list.
>
> [major] "MUST...result in a path equivalent to the original segment list"
>
> How is a "path equivalent to the original" defined?  The next
> paragraph mentions "a compressed segment list of equal or shorter
> length than the uncompressed segment list".  What does the length
> refer to -- the number of Segment Lists in the SRH, the size of the
> SRH, or something else?

We clarified in revision -13 that this is "the same forwarding path".

The length of a segment list is the number of elements that it contains.

> 1098   If an SR source node chooses to compress the segment list, one
method
> 1099   is described below for illustrative purposes.  Any other method
> 1100   producing a compressed segment list of equal or shorter length than
> 1101   the uncompressed segment list is compliant.
>
>
>
> ...
> 1107   *  When the compression method encounters a series of one or more
> 1108      consecutive compressible NEXT-C-SID flavor SIDs, it compresses
the
> 1109      series as follows.  A SID with the NEXT-C-SID flavor is
> 1110      compressible if its structure is known to the SR source node and
> 1111      its Argument value is 0.
>
> [major] Unlike the REPLACE-C-SID flavor (below), there's no check
> equivalent to ConCheck for the NEXT-C-SID flavor SIDs.  Why isn't that
> needed?

A similar check is performed inline on line S03 of the first pseudocode in
this section.

> ...
> 1183      |  Note: When the last C-SID is an End.DT2M SID with the
REPLACE-
> 1184      |  C-SID flavor, if there is 0 or at least two C-SID positions
> 1185      |  left in the current C-SID container, the C-SID is encoded as
> 1186      |  described above and the value of the Arg.FE2 argument is
placed
> 1187      |  in the 16 least significant bits of the next C-SID position.
> 1188      |  Otherwise (if there is only one C-SID position left in the
> 1189      |  current C-SID container), the current C-SID container is
pushed
> 1190      |  onto the segment list (the value of the C-SID position 0
> 1191      |  remains zero) and the End.DT2M SID with the REPLACE-C-SID
> 1192      |  flavor is encoded in full SID format with the value of the
> 1193      |  Arg.FE2 argument in the 16 most significant bits of the SID
> 1194      |  Argument.
>
> [minor] "if there is 0 or at least two C-SID positions left"
>
> 0?

In order words, the number of C-SID positions left is different from 1.

> 1196   *  In all remaining cases (i.e., when the compression method
> 1197      encounters a SID in the uncompressed segment list that is not
> 1198      handled by any of the previous subroutines), it pushes this SID
as
> 1199      is onto the compressed segment list.
>
> [major] "In all remaining cases...pushes this SID as is onto the
> compressed segment list."
>
> One of these cases is a SID that is not a C-SID, or one without the
> structure information, etc.  Why would these be put "onto the
> compressed segment list"?
>
>
> 1201   Regardless of how a compressed segment list is produced, the SR
> 1202   source node writes it in the IPv6 packet as described in Section
4.1
> 1203   of [RFC8754].  The text is reproduced below for reference

>From Section 2:

| *  Compressed segment list encoding: A segment list encoding that
|    reduces the packet header length thanks to one or more C-SID
|    sequences.  A compressed segment list encoding also contains zero,
|    one, or more uncompressed SID sequences.