Hi Gunter - Your review uncovered some needed edits and proposed clarifications. Nobody has implemented the automatic metric derivation from the bandwidth (at least if I understood the implementation poll right), so we have some leeway here. Authors can correct me if I'm wrong.
Hi Shraddha, Peter, and other Co-Authors, Can you either update the draft or respond as to why you didn’t for each comment below? Thanks, Acee > On Nov 19, 2024, at 11:30, Gunter van de Velde (Nokia) > <[email protected]> wrote: > > # Gunter Van de Velde, RTG AD, comments for draft-ietf-lsr-flex-algo-bw-con-15 > > # the referenced line numbers are derived from the idnits tool: > https://author-tools.ietf.org/api/idnits?url=https://www.ietf.org/archive/id/draft-ietf-lsr-flex-algo-bw-con-15.txt > > #GENERIC COMMENTS > #================ > ## I found the draft well written, and the procedures are well described. > Thank you for the nice document > ## This review is mostly about clarifications and consistency and nearly > ready for IETF LC > > #DETAILED COMMENTS > #================= > > 26 Requirements Language > 25 > 28 The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL > NOT", > 29 "SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" > in this > 30 document are to be interpreted as described in RFC 2119 > [RFC2119]. > > GV> This seems like an old boilerplate was used. The new one looks as follows: > > " > The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", > "SHOULD", "SHOULD NOT", "RECOMMENDED", "NOT RECOMMENDED", "MAY", and > "OPTIONAL" in this document are to be interpreted as described in BCP > 14 [RFC2119] [RFC8174] when, and only when, they appear in all > capitals, as shown here. > " > > 170 Section 3 defines additional Flexible Algorithm Definition (FAD) > 171 constraints that allow the network administrator to preclude the > use > 172 of low bandwidth links or high delay links. > > GV> Could a reference to RFC9350 be added for FAD? > > 236 0 1 2 3 > 237 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 > 0 1 > 238 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > 239 | Type | Length | metric-type | > 240 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > 241 | Value | > 242 +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ > > 244 Type : TBD1 (To be assigned by IANA) > 245 Length: 4 octets > 246 Metric-type: A value from the IGP metric-type registry > 247 Value : Metric value range (0 - 16,777,215) > > GV> The picture is slightly confusing. What about the following alternate > explanation. Similar structire could be used with the other encoding in the > sections that follow. > > " > Type (1 octet): > An 8-bit field assigned by IANA (To Be Determined as TBD1). This value > uniquely identifies the Metric TLV. > > Length (1 octet): > An 8-bit field indicating the total length, in octets, of the subsequent > fields. For this TLV, the Length is set to 4. > > Metric-Type (1 octet): > An 8-bit field specifying the type of metric. The value is taken from the > "IGP metric-type" registry maintained by IANA. > > Value (3 octets): > A 24-bit unsigned integer representing the metric value. The valid range is > from 0 to 16,777,215. > " > > 334 If the metric type indicates a standard metric type for which > there > 335 are other advertisement mechanisms (e.g., the IGP metric, the Min > 336 Unidirectional Link Delay, or the Traffic Engineering Default > 337 Metric), the Generic Metric advertisement MUST be ignored. > > GV> I am slightly confused. I may misunderstand this procedure rule. Is this > section trying to prescribe that generic metric type TLV should not carry a > standard metric? or is it trying to prescribe that when for a specific link > there is a legacy metric type advertisement (e.g. IGP metric) that the > generic TLV must be ignored? > > 355 Flexible-Algorithm-specific metric. Metrics carried in FAPM > will be > 356 equal to the metric to reach the prefix for that Flex-Algorithm > in > 357 its source area or domain. When Flex-Algorithm uses Generic > metric, > > GV> Maybe a small clarification for accuracy by adding few words: "Metrics > carried in FAPM will be equal to the metric to reach the prefix for that > Flex-Algorithm in its source area or domain ***from the ABR perspective***.". > > 430 If the Maximum Link Bandwidth is lower than the Minimum link > 431 bandwidth advertised in FAEMB sub-TLV, the link MUST be excluded > from > 432 the Flex-Algorithm topology. If a link does not have the Maximum > > GV> corner case scenario. I suspect that the existing text is written already > on purpose, but i wanted to sanity check: Is the intent to have 'lower' or > 'lower or equal'? > > 471 If the Min Unidirectional Link Delay value is higher than the > Maximum > 472 link delay advertised in FAEMD sub-TLV, the link MUST be excluded > 473 from the Flex-Algorithm topology. If a link does not have the > Min > > GV> same as above. Is this higher or higher and equal? > > 546 not aadvertised for a link but the FAD contains this sub-TLV,then > > GV> s/aadvertised/advertised/ > > 564 link advertisements. Bandwidth metric is a link attribute and > for > 565 the advertisement and processing of this attribute for Flex- > 566 algorithm, MUST follow the the section 12 of [RFC9350] > > GV> Is there special consideration for stub links? are there any special > behaviours for transit or stub links applied when a router is configured for > "overload max-metric"? (to take a router out-of-service for maintenance, or > for making an ABR less desirable during maintenance) > > 653 which may cause undesired load-balancing on the links. In such > 654 cases, a device may locally apply load-balancing factor relative > to > 655 the link bandwidth on the ECMP nexthops. > > GV> Would it be worthwhile to say explicit that these load-balancing > mechanisms are out of scope of this document (similar as a few lines earlier?) > > 683 For example, > 684 > 685 reference bandwidth = 1000G > 686 > 687 Granularity = 20G > 688 > 689 The derived metric is 10 for link bandwidth in the range 100G > to > 690 119G > > GV> Maybe more accurate that the FLOOR function is used to rounds number > down, toward zero, to the nearest multiple of significance > > 746 When granularity_bw is less than or equal to > Total_link_bandwidth > 747 > 748 Metric calculation: (Reference_bandwidth) / > 749 (Total_link_bandwidth - > 750 (Modulus of(Total_link_bandwidth, > 751 granularity_bw))) > 752 > 753 When granularity_bw is greater than Total_link_bandwidth > 754 Metric calculation: Reference_bandwidth / > 755 Total_link_bandwidth > > GV> should it be mentioned that this is an integer division? > > 769 automatically derived metric for that link. In case of Interface > 770 Group Mode, if all the parallel links have been advertised with > the > 771 Bandwidth Metric, The individual link Bandwidth Metric MUST be > used. > 772 If only some links among the parallel links have the Bandwidth > Metric > 773 advertisement, the Bandwidth Metric for such links MUST be > ignored > 774 and automatic Metric calculation MUST be used to derive link > metric. > > GV> what about following alternative writeup to make the text easier to > understand: > > " > In the case of Interface Group Mode, the following procedures apply: > * When all parallel links have advertised the Bandwidth Metric: The > individual link Bandwidth Metric MUST be used for each link. > * When only a subset of the parallel links have advertised the Bandwidth > Metric: The Bandwidth Metric advertisements for those links MUST be ignored. > In this scenario, automatic metric calculation MUST be used to derive the > link metrics for all parallel links. > " > > 847 When the computed link bandwidth is less than Bandwidth > Threshold 1, > 848 the MAX_METRIC value of 4,261,412,864 MUST be assigned as the > 849 Bandwidth Metric on the link during the Flex-Algorithm SPF > 850 calculation. > 851 > 852 When the computed link bandwidth is greater than or equal to > 853 Bandwidth Threshold 1 and less than Bandwidth Threshold 2, > Threshold > 854 Metric 1 MUST be assigned as the Bandwidth Metric on the link > during > 855 the Flex-Algorithm SPF calculation. > 856 > 857 Similarly, when the computed link bandwidth is greater than or > equal > 858 to Bandwidth Threshold 2 and less than Bandwidth Threshold 3, > 859 Threshold Metric 2 MUST be assigned as the Bandwidth Metric on > the > 860 link during the Flex-Algorithm SPF calculation. > 861 > 862 In general, when the computed link bandwidth is greater than or > equal > 863 to Bandwidth Threshold X AND less than Bandwidth Threshold X+1, > 864 Threshold Metric X MUST be assigned as the Bandwidth Metric on > the > 865 link during the Flex-Algorithm SPF calculation. > 866 > 867 Finally, when the computed link bandwidth is greater than or > equal to > 868 Bandwidth Threshold N, then Threshold Metric N MUST be assigned > as > 869 the Bandwidth Metric on the link during Flex-Algorithm SPF > 870 calculation. > > GV> What about the following procedure rewrite? > > " > Assignment of Bandwidth Metric Based on Computed Link Bandwidth > > The Bandwidth Metric for a link during the Flex-Algorithm Shortest Path First > (SPF) calculation MUST be assigned according to the following rules: > > 1. When the computed link bandwidth is less than Bandwidth Threshold 1: > * The Bandwidth Metric MUST be set to the maximum metric value of > 4,261,412,864. > > 2. When the computed link bandwidth is greater than or equal to Bandwidth > Threshold 1 and less than Bandwidth Threshold 2: > * The Bandwidth Metric MUST be set to Threshold Metric 1. > > 3. When the computed link bandwidth is greater than or equal to Bandwidth > Threshold 2 and less than Bandwidth Threshold 3: > * The Bandwidth Metric MUST be set to Threshold Metric 2. > > 4. In general, for all integer values of X such that 1≤X<N: > * When the computed link bandwidth is greater than or equal to Bandwidth > Threshold X and less than Bandwidth Threshold X+1: > ** The Bandwidth Metric MUST be set to Threshold Metric X. > > 5. When the computed link bandwidth is greater than or equal to Bandwidth > Threshold N: > * The Bandwidth Metric MUST be set to Threshold Metric N. > > Notes: > > * The term Bandwidth Threshold X refers to a predefined threshold value > corresponding to the index X. > * The term Threshold Metric X refers to the metric value associated with > Bandwidth Threshold X. > * N represents the total number of bandwidth thresholds defined in the system. > * The maximum metric value of 4,261,412,864 indicates that the link is > considered unreachable for the purposes of the Flex-Algorithm SPF calculation. > > Implementations MUST ensure that these rules are consistently applied to > maintain interoperability and optimal path computation within the network. > " > > 872 The IS-IS FADBT sub-TLV MUST NOT appear more than once in an > IS-IS > 873 FAD sub-TLV. If it appears more than once, the IS-IS FAD sub-TLV > 874 MUST MUST stop participating in such flex-algorithm. > > GV> s/MUST MUST/MUST/ > GV> Is it deliberate that here is written to stop participating in such > flex-algo, where at line 762 is written that "the IS-IS FAD sub-TLV MUST be > ignored by the receiver.". Is this intended? > > 934 When granularity_bw is less than or equal to > Total_link_bandwidth > 935 > 936 Metric calculation: (Reference_bandwidth) / > 937 (Total_link_bandwidth - > 938 (Modulus of(Total_link_bandwidth, > 939 granularity_bw))) > 940 > 941 When granularity_bw is greater than Total_link_bandwidth > 942 > 943 Metric calculation: Reference_bandwidth/ > 944 Total_link_bandwidth > 945 Figure 10: OSPF FADRB Sub-TLV > > GV> I was wondering to mention that for completeness this is an integer > division? > > 957 automatically derived metric for that link.In case of Interface > Group > 958 Mode, if all the parallel links have been advertised with the > 959 Bandwidth Metric, The individual link Bandwidth Metric MUST be > used. > 960 If only some links among the parallel links have the Bandwidth > Metric > 961 advertisement, the Bandwidth Metric for such links MUST be > ignored > 962 and automatic Metric calculation MUST be used to derive link > metric. > > GV> proposed rewrite > > " > Metric Assignment in Interface Group Mode: > When a link does not have a configured Bandwidth Metric, the automatically > derived metric MUST be used for that link. > > In the context of Interface Group Mode, the following rules apply to parallel > links: > > * If all parallel links have advertised the Bandwidth Metric: > **The individual link Bandwidth Metrics MUST be used for each link during > path computation. > * If only some of the parallel links have advertised the Bandwidth Metric: > ** The Bandwidth Metric advertisements for those links MUST be ignored. > ** Automatic metric calculation MUST be used to derive the link metrics for > all parallel links. > > This approach ensures consistent metric calculation and avoids discrepancies > caused by partial Bandwidth Metric advertisements among parallel links. > " > > 1036 When the computed link bandwidth is less than Bandwidth Threshold > 1 , > 1037 the MAX_METRIC value of 4,294,967,296 MUST be assigned as the > 1038 Bandwidth Metric on the link during the Flex-Algorithm SPF > 1039 calculation. > 1040 > 1041 When the computed link bandwidth is greater than or equal to > 1042 Bandwidth Threshold 1 and less than Bandwidth Threshold 2, > Threshold > 1043 Metric 1 MUST be assigned as the Bandwidth Metric on the link > during > 1044 the Flex-Algorithm SPF calculation. > 1045 > 1046 Similarly, when the computed link bandwidth is greater than or > equal > 1047 to Bandwidth Threshold 2 and less than Bandwidth Threshold 3, > 1048 Threshold Metric 2 MUST be assigned as the Bandwidth Metric on the > 1049 link during the Flex-Algorithm SPF calculation. > 1050 > 1051 In general, when the computed link bandwidth is greater than or > equal > 1052 to Bandwidth Threshold X AND less than Bandwidth Threshold X+1, > 1053 Threshold Metric X MUST be assigned as the Bandwidth Metric on the > 1054 link during the Flex-Algorithm SPF calculation. > 1055 > 1056 Finally, when the computed link bandwidth is greater than or > equal to > 1057 Bandwidth Threshold N, then Threshold Metric N MUST be assigned as > 1058 the Bandwidth Metric on the link during the Flex-Algorithm SPF > 1059 calculation. > > GV> From readability maybe following rewrite/restructuring proposal: > > " > Assignment of Bandwidth Metric Based on Computed Link Bandwidth: > > The Bandwidth Metric for a link during the Flex-Algorithm Shortest Path First > (SPF) calculation MUST be assigned according to the following rules: > > 1. When the computed link bandwidth is less than Bandwidth Threshold 1: > * The Bandwidth Metric MUST be set to the maximum metric value of > 4,294,967,296. > > 2. When the computed link bandwidth is greater than or equal to Bandwidth > Threshold 1 and less than Bandwidth Threshold 2: > * The Bandwidth Metric MUST be set to Threshold Metric 1. > > 3. When the computed link bandwidth is greater than or equal to Bandwidth > Threshold 2 and less than Bandwidth Threshold 3: > * The Bandwidth Metric MUST be set to Threshold Metric 2. > > 4. In general, for all integer values of X where 1≤X<N: > * When the computed link bandwidth is greater than or equal to Bandwidth > Threshold X and less than Bandwidth Threshold X+1: > ** The Bandwidth Metric MUST be set to Threshold Metric X. > > 5. When the computed link bandwidth is greater than or equal to Bandwidth > Threshold N: > * The Bandwidth Metric MUST be set to Threshold Metric N. > > Notes: > * Bandwidth Threshold X refers to the predefined bandwidth threshold > corresponding to index X. > * Threshold Metric X is the metric value associated with Bandwidth Threshold > X. > * N represents the total number of bandwidth thresholds defined in the system. > * The maximum metric value of 4,294,967,296 indicates that the link is > effectively considered unreachable during the Flex-Algorithm SPF calculation. > > Implementations MUST consistently apply these rules to ensure accurate path > computations and maintain interoperability within the network. > " > > G/ > _______________________________________________ > Lsr mailing list -- [email protected] <mailto:[email protected]> > To unsubscribe send an email to [email protected] <mailto:[email protected]>
_______________________________________________ Lsr mailing list -- [email protected] To unsubscribe send an email to [email protected]
