Re: [Lsr] AD review of draft-ietf-lsr-ospfv3-srv6-extensions-09

Ketan Talaulikar <ketant.ietf@gmail.com> Thu, 27 April 2023 16:35 UTC

Return-Path: <ketant.ietf@gmail.com>
X-Original-To: lsr@ietfa.amsl.com
Delivered-To: lsr@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 2C2B4C15153F; Thu, 27 Apr 2023 09:35:36 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.094
X-Spam-Level:
X-Spam-Status: No, score=-2.094 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_ZEN_BLOCKED_OPENDNS=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=0.001, 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 Qg_GskOKAh4O; Thu, 27 Apr 2023 09:35:31 -0700 (PDT)
Received: from mail-ej1-x636.google.com (mail-ej1-x636.google.com [IPv6:2a00:1450:4864:20::636]) (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 28424C151999; Thu, 27 Apr 2023 09:35:31 -0700 (PDT)
Received: by mail-ej1-x636.google.com with SMTP id a640c23a62f3a-94f4b911570so1370559766b.0; Thu, 27 Apr 2023 09:35:31 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20221208; t=1682613329; x=1685205329; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc:subject:date:message-id:reply-to; bh=Oap3MAaPe8nSvJ42KlCtYnozREvE6vHBykQOOqZXH4k=; b=NpS0qlC8iPmGNJNsgZpFH/N5M5KI94jBnFmlKRPKarTkEEGME/YzJnuckvyURMbhUF XmIPEJY6Gi+0jIrZiw8xE1LpKXGPsdhQB5zxDUzakm/mvhkJzJW4+DZnfBRL7VoQ+8ml cQ0uY2vQdCeyFq98mhwlF8xZSVDPZoNE8+KW5H+lmUe+tov3dRXMCCkpG9LCaRJGdh1T vJ5ien/l3ytHcNqNScsDfZXe8jETCwajUh546LkzRcZ3smHZSTe/cq5rZn3iQvgR+ds9 1P52bf5Su5A/FtePi0ND+LJB+hDOsiUdS6ruU1+3zlHspjfhigpfk0fy/vuPrL3liunJ WJPw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20221208; t=1682613329; x=1685205329; 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=Oap3MAaPe8nSvJ42KlCtYnozREvE6vHBykQOOqZXH4k=; b=LKTeatE2fWAHvm28nrPESl6prmyerMiSzTFIEW1+aSpGE1/9a7ZQhcZOT1IYtxGMm4 vvL+j2/fXVpVksgv6oN5iNW8RRm3CZ0d3X2B/c9YPBtg+NgIlYJvs93/uY2QRlOnY3iY rcq9Zt/gGIP86PNWmfYvMQkpFcbBGZH41KR9TIM1u2qc7EWGPDmG2hPxZ1aU+y7+CAhX xUDwR43N1G3zKGU1aSyle4wq5F9S3DgU0jhJOFm7RMvLwTjBJAAsaJSOfknxK8wqlof9 4G1roVtSeeAZ/97xwEBcNId62RqnnC2kVk0hEtnRq6R/ynfQeF2V1ftUnIIpfebL1f4n k3Ew==
X-Gm-Message-State: AC+VfDxDG9xJ2243+KqpGGKLz9CwNr8jdmsEbZeHCYky6AK5Xk2Se+oZ /jeFCFWSxkuOCYjX2dQ/xIBUrn9k0Z7GZeB7kv8=
X-Google-Smtp-Source: ACHHUZ4PMNKmqVSduZBn4npqF88f9s2iTn+46+Yuf3YbcjuSgWSBuKD70RM8ECO6b/GvPsGLt1VkOPaUywshVO1AqV8=
X-Received: by 2002:a17:907:360b:b0:94a:8f3a:1a77 with SMTP id bk11-20020a170907360b00b0094a8f3a1a77mr2401908ejc.8.1682613328842; Thu, 27 Apr 2023 09:35:28 -0700 (PDT)
MIME-Version: 1.0
References: <C5A44360-1B9D-4BC6-8F66-D4A61341EE53@juniper.net>
In-Reply-To: <C5A44360-1B9D-4BC6-8F66-D4A61341EE53@juniper.net>
From: Ketan Talaulikar <ketant.ietf@gmail.com>
Date: Thu, 27 Apr 2023 22:05:18 +0530
Message-ID: <CAH6gdPy9KWxBoBXH=YTBWOUAmkyL2u7d==1Ad3hAWb1Lgp7LxA@mail.gmail.com>
To: John Scudder <jgs@juniper.net>
Cc: "draft-ietf-lsr-ospfv3-srv6-extensions@ietf.org" <draft-ietf-lsr-ospfv3-srv6-extensions@ietf.org>, lsr <lsr@ietf.org>, "huzhibo@huawei.com" <huzhibo@huawei.com>, Lizhenbin <lizhenbin@huawei.com>, Peter Psenak <ppsenak@cisco.com>
Content-Type: multipart/alternative; boundary="000000000000c2925a05fa53f109"
Archived-At: <https://mailarchive.ietf.org/arch/msg/lsr/NZJulYNjjgRMWhkAlV8ElgI0NVk>
Subject: Re: [Lsr] AD review of draft-ietf-lsr-ospfv3-srv6-extensions-09
X-BeenThere: lsr@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: Link State Routing Working Group <lsr.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/lsr>, <mailto:lsr-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/lsr/>
List-Post: <mailto:lsr@ietf.org>
List-Help: <mailto:lsr-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/lsr>, <mailto:lsr-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 27 Apr 2023 16:35:36 -0000

Hi John,

Thanks for your detailed review and suggestions. Please check inline below
for responses. We've incorporated the editorial nits.

An updated version of the draft has just been posted:
https://datatracker.ietf.org/doc/html/draft-ietf-lsr-ospfv3-srv6-extensions-10


On Fri, Apr 21, 2023 at 11:57 PM John Scudder <jgs@juniper.net> wrote:

> Hi Authors, WG,
>
> Thanks for this spec.
>
> I’ve supplied my questions and comments in the form of an edited copy of
> the draft. Minor editorial suggestions I’ve made in place without further
> comment, more substantive questions and comments are done in-line and
> prefixed with “jgs:”. You can use your favorite diff tool to review them;
> I’ve attached the iddiff output for your convenience if you’d like to use
> it. I’ve also pasted a traditional diff below in case you want to use it
> for in-line reply.
>
> Thanks,
>
> —John
>
> --- draft-ietf-lsr-ospfv3-srv6-extensions-09.txt        2023-04-21
> 11:29:59.000000000 -0400
> +++ draft-ietf-lsr-ospfv3-srv6-extensions-09-jgs-comments.txt   2023-04-21
> 14:21:35.000000000 -0400
> @@ -116,9 +116,9 @@
>
>  1.  Introduction
>
> -   Segment Routing (SR) architecture [RFC8402] specifies how a node can
> -   steer a packet through an ordered list of instructions, called
> -   segments.  These segments are identified through Segment Identifiers
> +   The Segment Routing (SR) architecture [RFC8402] specifies how a node
> can
> +   steer a packet using an ordered list of instructions, called
> +   segments.  These segments are identified using Segment Identifiers
>     (SIDs).
>
>     Segment Routing can be instantiated on the IPv6 data plane through
> @@ -144,7 +144,7 @@
>     1.  An SRv6 Capabilities TLV to advertise the SRv6 features and SRH
>         operations supported by an OSPFv3 router
>
> -   2.  Several sub-TLVs are defined to advertise various SRv6 Maximum
> +   2.  Several sub-TLVs to advertise various SRv6 Maximum
>         SID Depths.
>
>     3.  An SRv6 Locator TLV using an SRv6 Locator Link-State
> @@ -179,6 +179,9 @@
>     be advertised by an SRv6-enabled router.
>
>     This TLV SHOULD be advertised only once in the OSPFv3 Router
> +---
> +jgs: What's the reason the above is a SHOULD and not a MUST?
> +---
>

KT> Changed to MUST.


>     Information LSA.  When multiple SRv6 Capabilities TLVs are received
>     from a given router, the receiver MUST use the first occurrence of
>     the TLV in the OSPFv3 Router Information LSA.  If the SRv6
> @@ -194,6 +197,10 @@
>     defined flooding scopes (link, area, or Autonomous System (AS)).  For
>     the purpose of SRv6 Capabilities TLV advertisement, area-scoped
>     flooding is REQUIRED.
> +---
> +jgs: I infer that link and AS scoped flooding is OPTIONAL. While this
> +is implicit, might it be worth adding something to that effect?
> +---
>

KT> Ack - added text.


>
>     The format of OSPFv3 SRv6 Capabilities TLV is shown below:
>
> @@ -208,6 +215,14 @@
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>                        Figure 1: SRv6 Capabilities TLV
> +---
> +jgs: In this and all subsequent figures, might it be worth filling in
> +the type value in the figure, as in "Type = 20" in this case? FWIW I
> +was first prompted to think about this by your Figure 3, where you
> +embed the literal "1" for the U-bit instead of just relying on body
> +text to tell the reader how to set it. ISTM if it's worth embedding one
> +constant in the figure, it's probably worth doing so whenever feasible.
> +---
>

KT> Embedding constants in the figures is not the convention that I've seen
followed for OSPF specs. Better to avoid repetition. Please check further
below about Figure 3 - we've removed all embedding for consistency.


>
>     Where:
>
> @@ -245,6 +260,12 @@
>
>     The SRv6 Capabilities TLV may contain optional Sub-TLVs.  No Sub-TLVs
>     are defined in this specification.
> +---
> +jgs: Thank you for setting up registry creation for this flags field.
> +However, I noticed that the registry says the O-flag has value 0x0001,
> +whereas in the diagram you show it as having value 0x4000. Please
> +correct one or the other, to be consistent.
> +---
>

KT> Thanks for catching that. We've switched from using "value" to using
"bit positions" in the IANA section.


>
>  3.  Advertisement of Supported Algorithms
>
> @@ -254,7 +275,7 @@
>  4.  Advertisement of Maximum SRv6 SID Depths
>
>     An SRv6-enabled router may have different capabilities and limits
> -   related to SRH processing and this needs to be advertised to other
> +   related to SRH processing and these need to be advertised to other
>     OSPFv3 routers in the SRv6 domain.
>
>     [RFC8476] defines the means to advertise node and link specific
> @@ -269,6 +290,10 @@
>     OSPFv3.  These MSD Types are allocated under the IGP MSD Types
>     registry maintained by IANA that are shared by IS-IS and OSPF.  They
>     are described below:
> +---
> +jgs: Please update the reference for srv6-extensions throughout, to
> +refer to [RFC9352].
> +---
>
>
KT> Ack


>
>
> @@ -310,8 +335,8 @@
>  4.4.  Maximum End D MSD Type
>
>     The Maximum End D MSD Type specifies the maximum number of SIDs
> -   present in an SRH when performing decapsulation.  These include, but
> -   not limited to, End.DX6, End.DT4, End.DT46, End with USD, End.X with
> +   present in an SRH when performing decapsulation.  These include, but
> are
> +   not limited to, End.DX6, End.DT4, End.DT46, End with USD, and End.X
> with
>     USD as defined in [RFC8986].  If the advertised value is zero or no
>     value is advertised, then the router cannot apply any behavior that
>     results in decapsulation and forwarding of the inner packet when the
> @@ -354,20 +379,58 @@
>     in the SRv6 Locator TLV, the calculation of its reachability, and the
>     installation in the forwarding plane follows the OSPFv3 [RFC5340]
>     specifications for the respective route types.
> +---
> +jgs: in the above paragraph you talk about "route types" and define them
> +in terms of RFC 5340's "specifications for the respective route types".
> +RFC 5340 isn't written in terms of route types, it talks about LSAs and
> +LSA types. These are used to compute routes, but are not themselves
> +referred to as routes. Please align your terminology with RFC 5340,
> +especially since you lean heavily on RFC 5340 to define your behavior.
> +---
>

KT> Updated the text.


>
> -   Locators associated with algorithms 0 and 1 SHOULD be advertised
> +   Locators associated with algorithms 0 and 1 SHOULD also be advertised
>     using the respective OSPFv3 Extended LSA types with extended TLVs
>     [RFC8362] so that routers that do not support SRv6 will install a
>     forwarding entry for SRv6 traffic matching those locators.  When
>     operating in Extended LSA sparse-mode [RFC8362], these locators
>     SHOULD be also advertised using the respective legacy OSPFv3 LSAs
>     [RFC5340].
> +---
> +jgs: I interpolated an "also" into the first sentence, to make it
> +clearer that such locators should be advertised in *both* the SRv6 Locator
> +LSA *and* the traditional LSA. Please check that this is correct.
> +---
>

KT> This is correct.


> +---
> +jgs: While I see the attraction of using the "respective... LSA types"
> +style of writing here, because it relieves you of writing (and the
> +reader of reading) quite a few words, I'm a little concerned that it
> +leaves something to the imagination; the reader has to glean what you
> +intended. I encourage you to think about whether you can improve on
> +this without overly burdening the document, for example I wonder if
> +you could have a paragraph earlier on that says something along the
> +lines of,
> +
> +       In this specification we introduce a new LSA type, the SRv6 Locator
> +       LSA. This has analogous use to other LSA types -- OSPFv3 Extended
> +       LSAs, Intra-Area-Prefix-LSAs, and E-Intra-Area-Prefix-LSAs. In the
> +       text that follows, we sometimes refer to "respective LSA types";
> +       this means [and then say just what it is that you mean].
> +
> +Or maybe there's an even simpler solution. Maybe I'm trying to read too
> +much into "the respective". If we simply removed those words, would the
> +meaning still be as you intend it? As in, "Locators associated with
> +algorithms 0 and 1 SHOULD also be advertised using OSPFv3 Extended
> +LSA types with extended TLVs"?
> +---
>

KT> Removed "respective" since it is not really necessary in this context.


>
>     When SRv6 Locators are also advertised as Intra-Area-Prefix-LSAs and/
>     or E-Intra-Area-Prefix-LSAs, the SRv6 Locator MUST be considered as a
>     prefix associated with the router and the referenced LSA type MUST
>     point to the Router LSA of the advertising router as specified in
>     Section 4.4.3.9 of [RFC5340].
> +---
> +jgs: what would be expected to happen/be done if the MUST is violated?
> +---
>

KT> Using something other than the Router LSA will affect the usability of
the Locator. E.g., if it is associated with a link and that link goes down,
the Locator becomes unusable. These implications are covered in the
reference - sec 4.4.3.9 of RFC5340. This MUST is for the advertising router
alone.


>
>     In cases where a locator advertisement is received both in a prefix
>     reachability advertisement (i.e., via legacy OSPFv3 LSAs and/or
> @@ -382,7 +445,7 @@
>
>     SRv6 SIDs are advertised as Sub-TLVs in the SRv6 Locator TLV except
>     for SRv6 End.X SIDs/LAN End.X SIDs which are associated with a
> -   specific Neighbor/Link and are therefore advertised as Sub-TLVs of E-
> +   specific Neighbor/Link and are therefore advertised as Sub-TLVs of the
> E-
>     Router-Link TLV.
>
>
> @@ -399,7 +462,7 @@
>     Reachability to SRv6 SIDs depends upon the existence of a covering
>     locator.
>
> -   Adherence to the rules defined in this section will assure that SRv6
> +   Adherence to the rules defined in this section will ensure that SRv6
>     SIDs associated with a supported algorithm will be forwarded
>     correctly, while SRv6 SIDs associated with an unsupported algorithm
>     will be dropped.  NOTE: The drop behavior depends on the absence of a
> @@ -408,6 +471,30 @@
>     For forwarding to work correctly, the locator associated with SRv6
>     SID advertisements must be the longest prefix match installed in the
>     forwarding plane for those SIDs.  To ensure correct forwarding,
> +---
> +jgs: I don't think the above is strictly correct -- you've stated a
> +sufficient condition for forwarding to work correctly, but not a
> +necessary one. For example, suppose the locator in question is some /64.
> +If there were also /128s in the forwarding plane, for the individual
> +SIDs, pointing to the same router, surely forwarding would still work?
> +One can also posit innumerable more baroque cases where forwarding
> +still works, either by design or by accident.
> +
> +If you agree with this, a simple rewrite to assuage my mania for
> +precision might go like,
> +
> +   If the locator associated with SRv6 SID advertisements is the longest
> +   prefix match installed in the forwarding plane for those SIDs, this
> +   will ensure correct forwarding.
> +
> +And then start the next sentence with "Network operators should..." as
> +it stands.
> +
> +I also wonder if this advice to network operators should go in an
> +Operational Considerations section or at least in its own subsection.
> +It seems misplaced in the current section which otherwise seems to be
> +speaking to implementors.
> +---
>

KT> I've reworded based on your suggestion - however, keeping it in the
current section.


>     network operators should take steps to make sure that this
>     requirement is not compromised.  For example, the following
>     situations should be avoided:
> @@ -428,11 +515,18 @@
>     represents a Flexible Algorithm, the procedures described in section
>     14.2 of [I-D.ietf-lsr-flex-algo] are followed for the programming of
>     those specific SRv6 Locators.
> +---
> +jgs: Please update all references to lsr-flex-algo to point to [RFC9350].
> +---
>

KT> Ack


> +---
> +jgs: Instead of "the same applies" may I suggest "analogous procedures
> +apply"?
> +---
>

KT> Ack


>
>     Locators associated with Flexible Algorithms SHOULD NOT be advertised
>     in the base OSPFv3 prefix reachability advertisements.  Advertising
>     the Flexible Algorithm locator in a regular prefix reachability
> -   advertisement would make them available for non-Flexible Algorithm
> +   advertisement would make it available for non-Flexible Algorithm
>     forwarding (i.e., algorithm 0).
>
>     The procedures for OSPFv3 Flexible Algorithm for SR-MPLS, as
> @@ -456,6 +550,11 @@
>     such the same value can be advertised by multiple routers.  It is
>     useful for other routers to know that the advertisement is for an
>     anycast identifier.
> +---
> +jgs: "It is useful" is an interesting claim to drop in without further
> +explanation or support. I expect it *is* useful or you wouldn't have said
> +it, but can you cite a reference here?
> +---
>

KT> RFC8402 covers some aspects of anycast. Then there was some work done
in draft-ietf-spring-mpls-anycast-segments and more recently
draft-ietf-spring-segment-protection-sr-te-paths. However, this is besides
the point and not in scope of this draft.


>
>     A new bit in OSPFv3 PrefixOptions [RFC5340] is defined to advertise
>     the anycast property:
> @@ -473,6 +572,9 @@
>
>     When the prefix/SRv6 Locator is configured as anycast, the AC-bit
>     SHOULD be set.  Otherwise, this flag MUST be clear.
> +---
> +jgs: Why SHOULD and not MUST?
> +---
>

KT> Changed to MUST.


>
>     The AC-bit MUST be preserved when re-advertising the prefix/SRv6
>     Locator across areas.
> @@ -484,6 +586,11 @@
>     The same prefix/SRv6 Locator can be advertised by multiple routers.
>     If at least one of them sets the AC-bit in its advertisement, the
>     prefix/SRv6 Locator SHOULD be considered as anycast.
> +---
> +jgs: Again, why SHOULD and not MUST, or alternately, why a 2119 keyword
> +at all and not "is", for example? If kept as SHOULD, what would be the
> +implications and consequences of disregarding the SHOULD?
> +---
>

KT> Changed to use non-normative "is".


>
>     A prefix/SRv6 Locator that is advertised by a single node and without
>     an AC-bit is considered node-specific.
> @@ -516,7 +623,19 @@
>  7.  SRv6 Locator LSA
>
>     The SRv6 Locator LSA has a function code of 42 while the S1/S2 bits
> +---
> +jgs: Congratulations on getting 42, the best number, assigned for your
> +code point. :-)
> +---
>

KT> I think I am missing something significant with this number - perhaps
you will share what that is? ;-)


>     are dependent on the desired flooding scope for the LSA.  The
> +---
> +jgs: The use of "while" above creates some ambiguity. Are you using it
> +to simply stitch together two statements? If so consider rewriting as
> +two shorter sentences, as in
> +
> +   The SRv6 Locator LSA has a function code of 42. The S1/S2 bits
> +   are dependent on the desired flooding scope for the LSA.
> +---
>

KT> Ack


>     flooding scope of the SRv6 Locator LSA depends on the scope of the
>     advertised SRv6 Locator and is under the control of the advertising
>     router.  The U-bit will be set indicating that the LSA should be
> @@ -546,6 +665,13 @@
>      |                             ...                               |
>
>                           Figure 3: SRv6 Locator LSA
> +---
> +jgs: For consistency, if you don't elect to put the literal type values
> +into your figures where relevant, perhaps consider removing the literal
> +"1" above and replacing with "U"?
> +
> +Otherwise, indicate Function Code is 42?
> +---
>

KT> Ack - changed to use "U"


>
>     The format of the TLVs within the body of the SRv6 Locator LSA is the
>     same as the format used by [RFC3630].  The variable TLV section
> @@ -568,13 +694,19 @@
>      |              Type             |             Length            |
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>      |                            Value                              |
> -                                   o
> -                                   o
> -                                   o
> +                                   .
> +                                   .
> +                                   .
>      |                                                               |
>      +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>
>                     Figure 4: SRv6 Locator LSA TLV Format
> +---
> +jgs: I suggested changing out the lower-case 'o' letters in your figure
> +for periods because the column of ohs didn't scan as a vertical ellipsis
> +for me when I read it, I had to stop and think about it for an instant.
> +It's obviously not a big deal though.
> +---
>

KT> Fixed to use what is there in RFC3630


>
>     The Length field defines the length of the value portion in octets
>     (thus, a TLV with no value portion would have a length of 0).  The
> @@ -594,6 +726,11 @@
>     flooding scope, the LSA flooding scope MUST satisfy the application-
>     specific requirements for all the locators included in a single SRv6
>     Locator LSA.
> +---
> +jgs: Isn't the MUST above almost tautological? It doesn't really seem like
> +something actionable for protocol conformance, would it be reasonable to
> +replace it with "has to"?
> +---
>

KT> Correct - changed to using "has to".


>
>     When multiple SRv6 Locator TLVs are received from a given router in
>     an SRv6 Locator LSA for the same Locator, the receiver MUST use the
> @@ -658,14 +795,26 @@
>                4 - AS External Type 2
>                5 - NSSA External Type 1
>                6 - NSSA External Type 2
> +---
> +jgs: I take it this list was created ab initio and isn't taken from
> +some existing code point space. Should it have a registry or is it
> +certain (or nearly certain) to never need extension?
> +---
>

KT> I would wager "nearly certain" to not need an extension. If we
introduce a new type, it would need significant updates to several OSPF
specifications.


>
>        Algorithm: 1-octet field.  The algorithm associated with the SRv6
>        Locator.  Algorithm values are defined in the IGP Algorithm Type
>        registry.
> +---
> +jgs: please add a reference to the registry
> +---
>

KT> Added reference to RFC8665 where the registry is defined/introduced.


>
>        Locator Length: 1-octet field.  Specifies the length of the
>        Locator prefix as the number of locator bits from the range
>        (1-128).
> +---
> +jgs: is there any need for words about what would happen if the
> +(implicitly forbidden) value 0 were present?
> +---
>

KT> Perhaps, but that would not be in this document. Value 0 would make it
a default route and that would be an illegal locator for SRv6.


>
>
>
> @@ -694,6 +843,13 @@
>     The following OSPFv3 Extended-LSA sub-TLVs corresponding to the
>     Extended Prefix LSAs are also applicable for use as sub-TLVs of the
>     SRv6 Locator TLV using code points as specified in Section 13.7:
> +---
> +jgs: Is there a need for another applicability column in the
> +OSPFv3 Extended-LSA Sub-TLVs registry table, analogous to the L3BM
> +column? The operative question is, when new sub-TLVs are introduced,
> +should the authors consider the question of whether they're applicable
> +for inclusion in the Extended Prefix LSA?
> +---
>

KT> We do not have today any indication as to applicability of a sub-TLV to
the Extended Prefix LSA types. Everything is mixed - prefix, link, node,
ASBR, etc. Everything that is applicable for Extended Prefix LSAs is also
applicable to SRv6 Locator in general. Earlier versions didn't have this
list and it was introduced since we got WG queries on how Forwarding
Address would be set/used.


>
>     *  IPv6-Forwarding-Address sub-TLV [RFC8362]
>
> @@ -766,12 +922,19 @@
>        Length: 2-octet field.  The total length (in octets) of the value
>        portion of the Sub-TLV including its further nested Sub-TLVs.
>
> -      Reserved: 1-octet field.  It MUST be set to 0 on transmission and
> -      MUST be ignored on receipt.
> -
>        Flags: 1-octet field.  It specifies the flags associated with the
>        SID.  No flags are currently defined and this field MUST be set to
>        0 on transmission and MUST be ignored on receipt.
> +---
> +jgs: I suggested reordering the "Flags" and "Reserved" descriptions to
> +match the flow of the diagram. But more substantively, if you're going
> +to define a flags field, I think you should also create an (unpopulated)
> +registry for it. If you don't want to create the registry, just make
> +the Reserved field 16 bits wide.
> +---
>

KT> Fixed the ordering of fields. We don't want to call this as a reserved
field since even though we don't yet have flags identified we know they
would be needed at some point. We have these SRv6 SID flags that are the
same in ISIS and BGP as well. Possibly we can share the same registry -
though not sure. Best to do this down the line when we start using the
flags.


> +
> +      Reserved: 1-octet field.  It MUST be set to 0 on transmission and
> +      MUST be ignored on receipt.
>
>        Endpoint Behavior: 2 octets field.  The endpoint behavior code
>        point for this SRv6 SID as defined in [RFC8986].  Supported
> @@ -795,8 +958,8 @@
>
>     The SRv6 endpoint behaviors defined in [RFC8986] include certain
>     behaviors that are specific to links or adjacencies.  The most basic
> -   of these which is critical for link-state routing protocols like
> -   OSPFv3 is the End.X behavior that is an instruction to forward to a
> +   of these, which is critical for link-state routing protocols like
> +   OSPFv3, is the End.X behavior that is an instruction to forward to a
>     specific neighbor on a specific link.  These SRv6 SIDs and others
>     that are defined in [RFC8986] which are specific to links or
>     adjacencies need to be advertised to OSPFv3 routers within an area to
> @@ -825,6 +988,9 @@
>     one neighbor.  Thus multiple instances of the SRv6 End.X SID and SRv6
>     LAN End.X SID Sub-TLVs MAY be advertised within the E-Router-Link TLV
>     for a single link.
> +---
> +jgs: why is the SHOULD above not a MUST?
> +---
>

KT> It is an implementation choice based on the use-case. We don't
mandatorily need adjacency SIDs. However, for many common use-cases like
TI-LFA and SR-TE, the adjacency SIDs are required and hence the SHOULD.


>
>     All End.X and LAN End.X SIDs MUST be subsumed by the subnet of a
>     Locator with the matching algorithm which is advertised by the same
> @@ -887,6 +1053,9 @@
>              +-+-+-+-+-+-+-+-+
>              |B|S|P| Reserved|
>              +-+-+-+-+-+-+-+-+
> +---
> +jgs: this should have a registry
> +---
>

KT> Agree. In hindsight, we could have shared this between ISIS and OSPFv3.
I missed this as the ISIS spec was working through the reviews. I wonder
whether it is OK to point to ISIS registry or if we define one for OSPFv3.


>
>        -  B-Flag: Backup Flag.  If set, the SID refers to a path that is
>           eligible for protection.
> @@ -916,6 +1085,9 @@
>        Algorithm: 1-octet field.  The algorithm associated with the SRv6
>        Locator from which the SID is allocated.  Algorithm values are
>        defined in the IGP Algorithm Type registry.
> +---
> +jgs: please add a reference to the registry
> +---
>

KT> Ack


>
>        Weight: 1-octet field.  Its value represents the weight of the
>        End.X SID for load-balancing.  The use of the weight is defined in
> @@ -994,6 +1166,9 @@
>              +-+-+-+-+-+-+-+-+
>              |B|S|P| Reserved|
>              +-+-+-+-+-+-+-+-+
> +---
> +jgs: this should have a registry
> +---
>

KT> Same as the previous comment.


>
>        -  B-Flag: Backup Flag.  If set, the SID refers to a path that is
>           eligible for protection.
> @@ -1081,6 +1256,10 @@
>        Type: 2-octet field.  The value for this type is 30.
>
>        Length: 2-octet field.  The value MUST be 4.
> +---
> +jgs: if you're filling in literal values in your figures, note you can
> +fill in length as well as type
> +---
>
>        LB Length: 1-octet field.  SRv6 SID Locator Block length in bits.
>
> @@ -1110,6 +1289,9 @@
>        BGP-LS and then be monitored for conformance to the SRv6 SID
>        allocation scheme chosen by the operator as described in
>        Section 3.2 of [RFC8986].
> +---
> +jgs: add an informational reference for BGP-LS?
> +---
>

KT> Ack


>
>     *  verification and the automation for securing the SRv6 domain by
>        provisioning filtering rules at SR domain boundaries as described
> @@ -1124,6 +1306,11 @@
>
>     The details of these potential applications are outside the scope of
>     this document.
> +---
> +jgs: that may be so, but having identified the possible use for securing
> +the domain, you might do well to mention or refer to it in your Security
> +Considerations section.
> +---
>

KT> That is already covered in RFC8754 as referenced.


>
>  11.  Advertising Endpoint Behaviors
>
> @@ -1163,9 +1350,14 @@
>     domain.  In these deployments, stronger authentication mechanisms
>     such as those specified in [RFC4552] or [RFC7166] SHOULD be used.
>
> -   Implementations MUST assure that malformed TLV and Sub-TLV defined in
> +   Implementations MUST ensure that malformed TLVs and Sub-TLVs defined in
>     this document are detected and do not provide a vulnerability for
>     attackers to crash the OSPFv3 router or routing process.  Reception
> +---
> +jgs: The preceding sentence smells a lot like "implementations MUST NOT
> +contain bugs", see [RFC9225] for a more detailed treatment. I suggest
> +removing it.
> +---
>

KT> Removed


>     of malformed TLV or Sub-TLV SHOULD be counted and/or logged for
>     further analysis.  Logging of malformed TLVs and Sub-TLVs SHOULD be
>     rate-limited to prevent a Denial of Service (DoS) attack (distributed
> @@ -1185,7 +1377,7 @@
>     Routing behaviors to enable the creation of interoperable overlays;
>     the security considerations from that document apply too.
>
> -   The advertisement for an incorrect MSD value may have negative
> +   The advertisement of an incorrect MSD value may have negative
>     consequences, see [RFC8476] for additional considerations.
>
>     Security concerns associated with the setting of the O-flag are
> @@ -1246,6 +1438,10 @@
>
>     *  Value: 0x0001.  Description: O-flag.  Reference: Section 2 of this
>        document.
> +---
> +jgs: as noted earlier, Section 2 shows the O-flag with value 0x4000.
> +Please reconcile.
> +---
>

KT> Fixed

Thanks,
Ketan


>
>  13.5.  OSPFv3 Extended-LSA Sub-TLVs
>
>
>
>