Hi John, 

All your comments regarding duplication of TLVs got me thinking.  We have 
covered this in recent RFCs in a more generalized manner. For example, see 
https://datatracker.ietf.org/doc/rfc7684/ and search for “same”. However, 
handling this general case of duplicate information isn’t completely addressed 
in the base RFCs (at least not for OSPF with which I’m infinitely more familiar 
with than IS-IS). For example, duplicate links in router-LSAs is not covered in 
RFC 2328. And, with OSPFv3 (RFC 5340) where the LSA ID is decoupled from the 
advertised prefix or router link, it is not covered in the base documents. It 
was just assumed that no rationale implementations wouldn’t do this… I’m not 
sure if we need a document to fully explain this but this document shouldn’t be 
burdened with this specification.

Peter can comment for IS-IS as to whether this duplication is addressed in 
IS-IS specifications.  

Thanks,
Acee

> On Apr 28, 2023, at 9:13 AM, Peter Psenak 
> <[email protected]> wrote:
> 
> Hi John,
> 
> Shradha and I have worked to address your comments.
> The new version of the draft has been published.
> 
> thanks,
> Peter
> 
> On 20/04/2023 02:27, John Scudder wrote:
>> Hi Authors, WG,
>> Thanks for this spec and for your patience as it waited for me to review it.
>> 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-ip-flexalgo-08.txt   2023-04-19 19:07:41.000000000 -0400
>> +++ draft-ietf-lsr-ip-flexalgo-08-jgs-comments.txt      2023-04-19 
>> 20:24:36.000000000 -0400
>> @@ -97,6 +97,10 @@
>>  1.  Introduction
>>     An IGP Flex-Algorithm as specified in [I-D.ietf-lsr-flex-algo]
>> +---
>> +jgs: All references to [I-D.ietf-lsr-flex-algo] should be updated to
>> +reference [RFC9350].
>> +---
>>     computes a constraint-based path to:
>>     *  All Flex-Algorithm specific Prefix Segment Identifiers (SIDs)
>> @@ -104,7 +108,7 @@
>>     *  All Flex-Algorithm specific SRv6 Locators [RFC8986].
>> -   Therefore, Flex-Algorithm cannot be deployed in the absence of SR and
>> +   Therefore, Flex-Algorithm cannot be deployed in the absence of SR or
>>     SRv6.
>> @@ -128,7 +132,14 @@
>>  3.  Use Case Example
>>     Mobile networks are becoming more and more IP centric.  Each end-user
>> -   session from a gNB (gNodeB) can be destined to a specific UPFs (User
>> +   session from a gNB (gNodeB) can be destined to a specific UPF (User
>> +---
>> +jgs: thanks for expanding gNB, but if you think about it I think you'll
>> +realize that "gNodeB" isn't any more informative to a reader who isn't
>> +already conversant with 3GPP terminology. Please either supply a reference,
>> +rewrite this section in more accessible terms, or remove the section, your
>> +choice.
>> +---
>>     Plane Function) based on the session requirements.  For example, some
>>     sessions require high bandwidth, others need to be routed along the
>>     lowest latency path.  Each UPF is assigned a unique IP address.  As a
>> @@ -136,14 +147,14 @@
>>     destination IP address.
>>     By associating the prefix that contains the UPF addresses with a
>> -   specific IP algorithm and routing the algorithm specific traffic
>> -   according to a certain constraints, e.g., low latency, a session
>> +   specific IP algorithm and routing the algorithm-specific traffic
>> +   according to a certain constraints, e.g., low latency, a session's
>>     traffic is routed according to the SLA (Service Level Agreement)
>>     appropriate for such session.
>>  4.  Advertising Flex-Algorithm Definitions (FAD)
>> -   To guarantee loop free forwarding, all routers that participate in a
>> +   To guarantee loop-free forwarding, all routers that participate in a
>>     Flex-Algorithm MUST agree on the Flex-Algorithm Definition (FAD).
>>     Selected nodes within the IGP domain MUST advertise FADs as described
>> @@ -183,13 +194,19 @@
>>     Advertisement of participation in IP Flex-Algorithm MUST NOT impact
>>     the router participation signaled for other data-planes.
>> +---
>> +jgs: would it be correct to rewrite the two MUST NOT above as "will not"
>> +or "does not"? If so, please do that. If you are actually using MUST NOT
>> +advisedly in the RFC 2119 sense, I don't understand what you're doing
>> +with these paragraphs -- please help me out.
>> +---
>>     The following sections describe how the IP Flex-Algorithm
>>     participation is advertised in IGP protocols.
>>  5.1.  The IS-IS IP Algorithm Sub-TLV
>> -   The ISIS [ISO10589] IP Algorithm Sub-TLV is a sub-TLV of the IS-IS
>> +   The IS-IS [ISO10589] IP Algorithm Sub-TLV is a sub-TLV of the IS-IS
>>     Router Capability TLV [RFC7981] and has the following format:
>>          0                   1                   2                   3
>> @@ -226,7 +243,7 @@
>>  Internet-Draft              IP Flex-Algorithm              December 2022
>> -   The use of IP Algorithm Sub-TLV to advertise support for algorithms
>> +   The use of the IP Algorithm Sub-TLV to advertise support for algorithms
>>     outside the Flex-Algorithm range (128-255) is outside the scope of
>>     this document.
>> @@ -253,6 +270,13 @@
>>                        Figure 2: OSPF IP Algorithm TLV
>>     *  Type: IP Algorithm TLV (Value TBD by IANA)
>> +---
>> +jgs: it's a common, and pleasant, convention for authors to stub in
>> +distinguished TBD1, TBD2, ... , TBDn for the different pending code
>> +points. This is a desirable thing from point of view of making sure
>> +the final RFC is error-free; please consider updating your different
>> +placeholders this way.
>> +---
>>     *  Length: Variable
>> @@ -286,6 +310,10 @@
>>     flooding scopes (link, area, or Autonomous System (AS)).  For the
>>     purpose of IP Algorithm TLV advertisement, area-scoped flooding is
>>     REQUIRED.
>> +---
>> +jgs: I take it then, that link and AS-scoped flooding are OPTIONAL?
>> +Although this is implicit, I think it wouldn't hurt to say so.
>> +---
>>     The IP Flex-Algorithm participation advertised in the OSPF IP
>>     Algorithm TLV is topology independent.  When a router advertises
>> @@ -293,14 +321,18 @@
>>     all topologies in which the advertising node participates.
>>  6.  Advertising IP Flex-Algorthm Reachability
>> +---
>> +jgs: Please do a global search-and-replace of "algorthm" by "algorithm".
>> +I count seven instances of this misspelling.
>> +---
>>     To be able to associate the prefix with the Flex-Algorithm, the
>> -   existing prefix reachability advertisements can not be used, because
>> +   existing prefix reachability advertisements cannot be used, because
>>     they advertise the prefix reachability in default algorithm 0.
>> -   Instead, a new IP Flex-Algorithm reachability advertisements are
>> +   Instead, new IP Flex-Algorithm reachability advertisements are
>>     defined in IS-IS and OSPF.
>> -   The M-flag in FAD is not applicable to IP Algorithm Prefixes.  Any IP
>> +   The M-flag in the FAD is not applicable to IP Algorithm Prefixes.  Any IP
>>     Algorithm Prefix advertisement includes the Algorithm and Metric
>>     fields.  When an IP Algorithm Prefix is advertised between areas or
>>     domains, the metric field in the IP Algorithm Prefix advertisement
>> @@ -308,8 +340,8 @@
>>  6.1.  The IS-IS IPv4 Algorithm Prefix Reachability TLV
>> -   A top-level TLV is defined for advertising IPv4 Flex-Algorithm Prefix
>> -   Reachability in IS-IS - IPv4 Algorithm Prefix Reachability TLV.
>> +   The IPv4 Algorithm Prefix Reachability top-level TLV is defined for 
>> advertising IPv4 Flex-Algorithm Prefix
>> +   Reachability in IS-IS.
>>     This new TLV shares the sub-TLV space defined for TLVs Advertising
>>     Prefix Reachability.
>> @@ -328,6 +360,15 @@
>>     *  Type: IPv4 Algorithm Prefix Reachability TLV (Value 126).
>>     *  Length: Variable.
>> +---
>> +jgs: "Variable" is a bit terse. I looked at a few other IS-IS RFCs.
>> +There doesn't seem to be a single standard pattern, but they all say
>> +something more helpful than 'variable'. For example, RFC 6232 says,
>> +
>> +      LENGTH - total length of the value field.
>> +
>> +Please update the description to be something a little more helpful.
>> +---
>> @@ -340,6 +381,12 @@
>>     *  R bits (4 bits): Reserved for future use.  They MUST be set to
>>        zero on transmission and MUST be ignored on receipt.
>> +---
>> +jgs: It's a little unusual in my experience to see this drawn as
>> +four one-bit fields all named "R" instead of one four-bit field
>> +named "Rsvd" or similar. It's fine to leave it if you prefer it
>> +this way, but know that it may surprise others too.
>> +---
>>     *  MTID (12 bits): Multitopology Identifier as defined in [RFC5120].
>>        Note that the value 0 is legal.
>> @@ -400,12 +447,47 @@
>>     the lowest-numbered LSP and ignore any subsequent IPv4 Algorithm
>>     Prefix Reachability advertisements for the same prefix for any other
>>     Algorithm.
>> +---
>> +jgs: The way you've written this ("each with a different Algorithm") I
>> +don't think you get the outcome you want. Consider if a router receives
>> +four advertisements for the same prefix, with algorithms 128, 129, 130,
>> +and 130, respectively. They are not *each* with a different algorithm,
>> +the last two have the same algorithm, therefore this paragraph doesn't
>> +apply. I don't think that's what you want, in fact in this case I
>> +the algorithm is a red herring. I think you mean something like
>> +
>> +   If a router receives multiple IPv4 Algorithm Prefix Reachability
>> +   advertisements for the same prefix from the same originator, it
>> +   MUST select the first advertisement in
>> +   the lowest-numbered LSP and ignore any subsequent IPv4 Algorithm
>> +   Prefix Reachability advertisements for the same prefix.
>> +
>> +Or is there some other defined behavior for what the router should do
>> +if the originator sends two or more advertisements for a given prefix
>> +in the same algorithm?
>> +---
>>     A router receiving multiple IPv4 Algorithm Prefix Reachability
>>     advertisements for the same prefix, from different originators, each
>>     with a different Algorithm, MUST ignore all of them and MUST NOT
>>     install any forwarding entries based on these advertisements.  This
>>     situation SHOULD be logged as an error.
>> +---
>> +jgs: A similar problem exists here. In this case I don't think you can
>> +ignore talking about the algorithm, because of course it's
>> +normal and expected for different routers to sometimes advertise the
>> +same prefix. So I think you want something like,
>> +
>> +   If a router receives multiple IPv4 Algorithm Prefix Reachability
>> +   advertisements for the same prefix, from different originators, that
>> +   do not all use the same Algorithm, it MUST ignore all of them and
>> +   MUST NOT install any forwarding entries based on these
>> +   advertisements.  This situation SHOULD be logged as an error.
>> +
>> +I think you have six occurrences of this "each with a different
>> +Algorithm" problem. I'll flag the others I find, but please double-check
>> +that I didn't miss any.
>> +---
>>     In cases where a prefix advertisement is received in both a IPv4
>>     Prefix Reachability TLV and an IPv4 Algorithm Prefix Reachability
>> @@ -416,7 +498,7 @@
>>     The IS-IS IPv6 Algorithm Prefix Reachability TLV is identical to the
>>     IS-IS IPv4 Algorithm Prefix Reachability TLV, except that it has a
>> -   unique type.  The type is 127.
>> +   distinct type.  The type is 127.
>>     A router receiving multiple IPv6 Algorithm Prefix Reachability
>>     advertisements for the same prefix, from the same originator, each
>> @@ -424,19 +506,25 @@
>>     the lowest-numbered LSP and ignore any subsequent IPv6 Algorithm
>>     Prefix Reachability advertisements for the same prefix for any other
>>     Algorithm.
>> +---
>> +jgs: here's another
>> +---
>>     A router receiving multiple IPv6 Algorithm Prefix Reachability
>>     advertisements for the same prefix, from different originators, each
>>     with a different Algorithm, MUST ignore all of them and MUST NOT
>>     install any forwarding entries based on these advertisements.  This
>>     situation SHOULD be logged as an error.
>> +---
>> +jgs: and another
>> +---
>>     In cases where a prefix advertisement is received in both an IPv6
>>     Prefix Reachability TLV and an IPv6 Algorithm Prefix Reachability
>>     TLV, the IPv6 Prefix Reachability advertisement MUST be preferred
>>     when installing entries in the forwarding plane.
>> -   In cases where a prefix advertisement is received in both IS-IS SRv6
>> +   In cases where a prefix advertisement is received in both an IS-IS SRv6
>>     Locator TLV and in IS-IS IPv6 Algorithm Prefix Reachability TLV, the
>>     receiver MUST ignore both of them and MUST NOT install any forwarding
>>     entries based on these advertisements.  This situation SHOULD be
>> @@ -496,6 +584,9 @@
>>     each with a different Algorithm, MUST ignore all of them and MUST NOT
>>     install any forwarding entries based on these advertisements.  This
>>     situation SHOULD be logged as an error.
>> +---
>> +jgs: and another
>> +---
>> @@ -562,19 +653,24 @@
>>  Internet-Draft              IP Flex-Algorithm              December 2022
>> -      Metric (4 octets): The algorithm specific metric value.  The
>> +      Metric (4 octets): The algorithm-specific metric value.  The
>>        metric value of 0XFFFFFFFF MUST be considered as unreachable.
>>     When the OSPFv3 IP Algorithm Prefix Reachability Sub-TLV is present,
>>     the NU-bit in the PrefixOptions field of the parent TLV MUST be set.
>> -   This is needed to avoid the OSPFv3 IP Algorithm Prefix Reachability
>> -   advertisement to contribute to the base algorithm reachability.  If
>> +   This is needed to prevent the OSPFv3 IP Algorithm Prefix Reachability
>> +   advertisement from contributing to the base algorithm reachability.  If
>>     the NU-bit in the PrefixOptions field of the parent TLV is not set,
>>     the OSPFv3 IP Algorithm Prefix Sub-TLV MUST be ignored by the
>>     receiver.
>>     The metric value in the parent TLV is RECOMMENDED to be set to
>>     LSInfinity [RFC2328].
>> +---
>> +jgs: if the RECOMMENDATION is ignored, what will be the outcome?  In
>> +other words, why is this not a MUST? Under what circumstances would it
>> +be OK for an implementor to ignore the recommendation?
>> +---
>>     An OSPFv3 router receiving multiple OSPFv3 IP Algorithm Prefix
>>     Reachability Sub-TLVs in the same parent TLV, MUST select the first
>> @@ -586,6 +682,9 @@
>>     each with a different Algorithm, MUST ignore all of them and MUST NOT
>>     install any forwarding entries based on these advertisements.  This
>>     situation SHOULD be logged as an error.
>> +---
>> +jgs: and another
>> +---
>>     In cases where a prefix advertisement is received in any of the LSAs
>>     advertising the prefix reachability for algorithm 0 and in an OSPFv3
>> @@ -607,9 +706,9 @@
>>     advertise a Flex-Algorithm specific metric associated with the
>>     corresponding ASBR LSA.
>> -   As described in [I-D.ietf-lsr-flex-algo] each data-plane signals the
>> +   As described in [I-D.ietf-lsr-flex-algo] each data-plane signals its
>>     participation independently.  IP Flex-Algorithm participation is
>> -   signaled independent of the Segment Routing (SR) Flex-Algorithm
>> +   signaled independent of Segment Routing (SR) Flex-Algorithm
>> @@ -685,9 +784,9 @@
>>  7.  Calculating of IP Flex-Algorthm Paths
>>     The IP Flex-Algorthm is considered as yet another data-plane of the
>> -   Flex-Algorithm as described [I-D.ietf-lsr-flex-algo].
>> +   Flex-Algorithm as described in [I-D.ietf-lsr-flex-algo].
>> -   Participation for the IP Flex-Algorithm is signalled as described in
>> +   Participation in the IP Flex-Algorithm is signalled as described in
>>     Section 5 and is specific to the IP Flex-Algorithm data-plane.
>>     Calculation of IP Flex-Algorithm paths follows what is described in
>> @@ -734,6 +833,15 @@
>>     Algorithm, LFA paths to the prefix MUST be calculated using such
>>     Flex-Algorithm in the associated topology, to guarantee that they
>>     follow the same constraints as the calculation of the primary paths.
>> +---
>> +jgs: The above paragraph appears to be redundant with the final
>> +paragraph of Section 10, Protection. Consider removing it, or providing
>> +a forward reference to Section 10, or alternately (no pun intended) remove
>> +the redundant paragraph in Section 10 and refer back to this paragraph.
>> +
>> +If you don't remove it, please expand LFA on first use and supply
>> +a citation for it.
>> +---
>>  9.  Deployment Considerations
>> @@ -798,6 +906,10 @@
>>     This specification updates the OSPF Router Information (RI) TLVs
>>     Registry as follows:
>> +---
>> +jgs: this is where I remind you about my strong suggestion to change out
>> +"TBD" for "TBD1", "TBD2", etc.
>> +---
>>           +=======+==================+===========================+
>> @@ -808,7 +920,7 @@
>>                                   Table 1
>> -   This document also updates the ISIS "Sub-TLVs for TLV 242" registry
>> +   This document also updates the IS-IS "Sub-TLVs for TLV 242" registry
>>     as follows:
>> @@ -862,6 +974,18 @@
>>     Reachability TLV and IPv6 Algorithm Prefix Reachability TLV.  It also
>>     includes these TLVs in a table which lists the presence of Sub-TLVs
>>     in a parent TLVs as follows:
>> +---
>> +jgs: I think the above could be a little clearer. Perhaps,
>> +
>> +   Since the above TLVs share the sub-TLV space managed in the "IS-IS
>> +   Sub-TLVs for TLVs Advertising Prefix Reachability" registry, IANA is
>> +   requested to add "IPv4 Algorithm Prefix Reachability TLV (126)" and
>> +   "IPv6 Algorithm Prefix Reachability TLV (127)" to the list of TLVs in
>> +   the description of that registry.
>> +
>> +   In addition, columns headed '126' and '127' are added to that registry,
>> +   as follows:
>> +---
>>          Type  Description                          126 127
>> @@ -931,6 +1055,25 @@
>>     This document inherits security considerations from
>>     [I-D.ietf-lsr-flex-algo].
>> +---
>> +jgs: This is an excessively terse Security Considerations section. Have
>> +you carefully thought through the security implications of the current
>> +proposal and concluded that it really introduces nothing new? For
>> +example, the 'each with a different Algorithm' paragraphs could make it
>> +possible for an attacker to cause a prefix to be ignored --
>> +
>> +Consider the case where a legitimate router, A, advertises prefix P in
>> +algorithm 128. An attacker wants to prevent P from being reachable, and
>> +uses a subverted router, B, to advertise prefix P in algorithm 129.
>> +Following the paragraphs I mention, I think what happens is the attacker
>> +successfully has prevented P from being reachable. I don't think this
>> +attack exists in the underlying RFC 9350.
>> +
>> +Quite likely your conclusion after considering this would be just like
>> +RFC 9350: "... it is not different from advertising any other incorrect
>> +information through IS-IS or OSPF". But that doesn't relieve you of the
>> +need to say that.
>> +---
>>  13.  Acknowledgements
>> @@ -965,6 +1108,10 @@
>>                information exchange protocol for use in conjunction with
>>                the Protocol for providing the Connectionless-mode Network
>>                Service (ISO 8473)", August 1987, <ISO/IEC 10589:2002>.
>> +---
>> +jgs: "IANA" can't possibly be the correct author, I'm sure you mean "ISO".
>> +Please check the above citation carefully.
>> +---
>>     [RFC2119]  Bradner, S., "Key words for use in RFCs to Indicate
>>                Requirement Levels", BCP 14, RFC 2119,
>> _______________________________________________
>> Lsr mailing list
>> [email protected]
>> https://www.ietf.org/mailman/listinfo/lsr 
>> <https://www.ietf.org/mailman/listinfo/lsr>
> 
> _______________________________________________
> Lsr mailing list
> [email protected]
> https://www.ietf.org/mailman/listinfo/lsr

_______________________________________________
Lsr mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lsr

Reply via email to