[Idr] Re: AD review of draft-ietf-idr-bgp-ls-sr-policy-13

[John Scudder <[e-mail]>]

Hi Ketan,

I’ve briefly reviewed the diff and it looks like you have addressed my concerns. Thanks!

—John

On Feb 28, 2025, at 8:48 AM, Ketan Talaulikar <[e-mail]> wrote:



Hi John,

Thanks for the offline discussion yesterday to discuss your suggestions. Please find inline below with KT3 a summary of the changes made.

An update has been posted that captures all these changes (in addition to a few LC comments): https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-ls-sr-policy-15<https://urldefense.com/v3/__https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-ls-sr-policy-15__;!!NEt6yMaO-gk!DAydtdWOAbenRUhcaQAcFb681smgE2xFOYPY0lLQQLVVCTTVNucA9ubMxyWlpoZQ09gEzzCLQTjGUXMmjw$>


On Thu, Feb 27, 2025 at 12:16 PM Ketan Talaulikar <[e-mail]<mailto:[e-mail]>> wrote:
Hi John,

Thanks for your responses. Please check inline below for further clarifications with KT2


On Thu, Feb 27, 2025 at 7:08 AM John Scudder <[e-mail]<mailto:[e-mail]>> wrote:
Hi Ketan,

This mostly looks fine. I have a few residual points below; trimmed for brevity, cuts indicated by ellipses.a

On Feb 17, 2025, at 7:08 AM, Ketan Talaulikar <[e-mail]<mailto:[e-mail]>> wrote:

Hi John,

Thanks for sharing the next part of your document review and suggestions. Please check inline below for responses.

We've also posted an update with the changes as discussed below: https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-ls-sr-policy-14<https://urldefense.com/v3/__https://datatracker.ietf.org/doc/html/draft-ietf-idr-bgp-ls-sr-policy-14__;!!NEt6yMaO-gk!GcANokg-oBRf06lI8IW_5LhkGS9mnhtDdT7LLCGeMKWznZ9hGnIZqr3veu4jWXSVJJMXZZ6z4NcGjZ5Few$>


On Sat, Feb 15, 2025 at 8:34 AM John Scudder <[e-mail]<mailto:[e-mail]>> wrote:
...
@@ -1406,7 +1429,27 @@
       identifier for a set of disjoint paths.  A PCEP Association Object
       [RFC8697] (including its optional TLVs) MAY also be advertised to
       convey the disjoint group identifier.
++--
+jgs: I'm a little lost at sea here.

+First: The diagram shows this as "Disjoint Group Identifier" but here
+you call it "Disjointness Group Identifier". Please pick one.

KT> Fixed

+
+Second: The diagram says "(variable)" for size but here you say it's
+four octets. Which is it? I'm guessing 4-octet is right, in which case
+please fix the diagram.
+

KT> It is a 4-octet value unless the path was set up via PCEP in which cases the PCEP Association Object (and its TLVs) can be reported.

Oh! You’re saying that I can embed a PCEP Association Object in this field? If so that was not at all clear to me and I think we need an update, something like,

OLD:
   *  Disjoint Group Identifier: 4-octet value that is the group
      identifier for a set of disjoint paths.  A PCEP Association Object
      [RFC8697] (including its optional TLVs) MAY also be advertised to
      convey the disjoint group identifier.

NEW:
   *  Disjoint Group Identifier: 4-octet value that is the group
      identifier for a set of disjoint paths.  Alternatively, this field
      MAY contain a PCEP Association Object
      [RFC8697] (including its optional TLVs).

Is that what you mean?

KT2> Yes. I've updated with your suggested text that is more clear.


Some other things:

- Please add the section reference (Section 6) to the RFC 8697 xref, unless there’s some reason not to.
- I assume you mean exactly the Association object depicted in 8697 §6 Figures 3 and 4, reserved field, flags, and all.

KT2> Yes, section 6.1 to be precise (now added in the reference). Yes, it includes the entire object.

- If so, what’s the receiver meant to do if it gets an R flag? (Not to mention any future TBD flags.)
- If not, and there are restrictions on the content or semantics of the object compared to what’s in 8697, please detail what the valid profile of the object is.

KT2> I assume you mean the BGP-LS Consumer when you say "receiver" because BGP-LS Speakers do not need to do semantic validation of the TLV contents. The specifications of the actions of BGP-LS Consumers is outside the scope of BGP-LS documents. However, if we keep that aside, and specifically talk about the R flag, it is something between the PCC and the PCE - while in this case we are providing information from the network to controllers and so the R flag will most likely be ignored. If the LSP is removed from the association group, there will be a follow-on update. This is based on my limited knowledge of PCEP so I may be wrong.


KT3> As discussed, text is now elaborated to cover when a BGP-LS producer would need to encode a PCEP association object and that it would be only parsed by a BGP-LS consumer which is outside the scope of this document.



...
@@ -1535,9 +1607,14 @@
    *  Length: 12 octets

    *  Metric Type: 1-octet field which identifies the type of the metric
-      being used.  The Table 1 below lists the metric types introduced
+      being used.  Table 1 below lists the metric types introduced
       by this document along with reference for each.  The reference is
       to IS-IS (equivalent also exist for OSPF) specifications where
++--
+jgs: This doesn't appear to be anywhere close to specific enough. Please
+provide a reference for OSPF as well. For that matter, is there any
+reason not to cover both OSPFv2 and OSPFv3?
++--

KT> This document is following the BGP-LS (RFC 7752, 9552) convention of providing IS-IS references instead of all 3 protocols (as OSPFv2 and OSPFv3 are separate) for every metric type. This has been good enough for implementers to identify the equivalent for OSPF.

Against my instincts, I am going to accept this, even though it sounds a great deal like “all my friends jumped off the bridge so I’m going to jump off, too”. In general, I think it is best if our specifications need a minimum of interpretation (“creativity”) on the part of the implementor.

KT2> I understand. In this case, I tried to follow the "norm" while in some other BGP-LS documents I've tried to elaborate for all 3 IGPs (it is not exactly the same but please check https://www.rfc-editor.org/rfc/rfc9085.html#section-2.3.2<https://urldefense.com/v3/__https://www.rfc-editor.org/rfc/rfc9085.html*section-2.3.2__;Iw!!NEt6yMaO-gk!DAydtdWOAbenRUhcaQAcFb681smgE2xFOYPY0lLQQLVVCTTVNucA9ubMxyWlpoZQ09gEzzCLQTiwOX9qig$> for something on similar lines). In that latter case, it will not be in a table form but as a bullet list. Let me know if you feel strongly about this and I will make that change.

KT3> Now expanded with references to all 3 IGPs in a bullet format.




       those metric types are defined for a link while in the SR Policy
       context those relate to the candidate path or the segment list.
       The metric type code points that may be used in this sub-TLV are
@@ -1575,6 +1652,14 @@
       | Point |    Metric Type     |         Reference               |
       +-------+--------------------+---------------------------------+
       |  0    | IGP                | [RFC5305] Section 3             |
++--
+jgs: Even in the IS-IS context (never mind my OSPF related complaints)
+this doesn't appear specific enough. Looking at RFC 5305 Section 3, I
+see at least two different things called "metric", the "default metric"
+and the "TE Default metric". This feels like a place where the
+implementor is being invited to use their creativity, which is not
+something our specs should do.
++--

KT> Does it help if we say section 3.0 instead of 3?

Not at all. There’s no defined meaning for “3.0”. We have a regrettable ambiguity created by the way we use sections/subsections in our document set. Please revert to “Section 3” which is less bad. I think the “default metric” clarification in version 14 covers you sufficiently, but I’ll leave it to you to decide if it’s worth excluding the subsections; if so you could use the wordy, but unambiguous, “Section 3 (but not its subsections)”.

KT2> OK. I will revert the 3.0 to 3.


There is already a reference to section 3.7 for the TE metric. That said, added text to clarity that IGP and TE metric are called as "default metric" and "TE default metric" in the IS-IS references so as to address the concerns that you have raised.

       |  1    | Min Unidirectional | [RFC8570] Section 4.2           |
       |       | Delay              |                                 |
       |  2    | TE                 | [RFC5305] Section 3.7           |
@@ -1592,6 +1677,10 @@
       +-------+--------------------+---------------------------------+

                    Table 1 BGP-LS SR Policy Metric Types
++--
+jgs: I think all the above have to be normative references since they
+provide the necessary details about the respective values.
++--

KT> Ack - made them normative.


    *  Flags: 1-octet field that indicates the validity of the metric
       fields and their semantics.  The following bit positions are
@@ -1641,7 +1730,25 @@
       metric to accommodate for other factors such as bandwidth
       availability, minimal SID stack depth, and maximizing of ECMP for
       the SR path computed.
++--
+jgs: This might be a place where I am running afoul of lack of domain
+knowledge, but I would have assumed we desire to minimize some metrics
+(e.g. latency) but maximize others (say, bandwidth). Above you're
+implying we want only to maximize metrics (you talk about "minimum
+metric"). Does this make sense in some context that pertains to this
+spec?

KT> Not all metrics are additive. There was originally a discussion on this topic in the SR Policy Architecture document that went on to be published as RFC9256. However, per the WG and chairs feedback that these topics were well-known and established in the realm of TE, they were removed to another document that was not picked up by the SPRING WG - https://datatracker.ietf.org/doc/html/draft-filsfils-spring-sr-policy-considerations-09#section-3.1<https://urldefense.com/v3/__https://datatracker.ietf.org/doc/html/draft-filsfils-spring-sr-policy-considerations-09*section-3.1__;Iw!!NEt6yMaO-gk!GcANokg-oBRf06lI8IW_5LhkGS9mnhtDdT7LLCGeMKWznZ9hGnIZqr3veu4jWXSVJJMXZZ6z4NcCjlVlcw$> ... if we step back, this draft is a specification for reporting and not path computation.

You say “not all metrics are additive”. Indeed. But the text in question implies several things, contradictory ones at that. "The metric margin loosens the criteria for minimum metric path calculation up to the specified metric”:

- “Minimum” implies that a metric MUST be greater than or equal to the given value
- “Up to” implies “the metric MUST be less than or equal to the given value

Oops. Maybe we can discuss offline what it is you really want to express here, and come up with some replacement text. (Or you can school me on why I’m confused. :-)

KT2> Indeed, it looks like this needs a discussion. We can circle back on this thread once we have concluded.


KT3> Fixed the language with the text on lines of your suggestions. Please check and let me know if this works.





+Related, "loosens the criteria... up to the specified metric" and so on
+seems insufficiently precise. That is to say, is the effective metric...
+the specified metric plus the margin? Minus? Plus-or-minus?
+

KT> We cannot preclude which metric types are introduced in the future - some may have higher values are better while others where lower values are better. This is left to the specification of the particular metric type.

Sure. See above for why I think the text as written isn’t achieving the goals you’ve described above. Let’s discuss offline.

KT2> Ack



+Come to think of it, I don't know what the "computed path metric" is,
+which means I don't know what I'm supposed to apply the percentage to.
+I guess maybe I don't need to, precisely, it just means the effective
+metric is equal to the specified metric times (1 + percentage value/100)
+(or maybe it's +/-).
++--

KT> That depends on the metric type. The point is that instead of getting the "lowest", it may be OK to get something that is 25% higher but meets other criteria.

Again, that’s fine but I think the text could be clearer; again, let’s discuss.

KT2> Ack


...
@@ -1948,9 +2064,35 @@
   +------+-------------------------------------------------------------+

                     Table 2  SR Segment Types
++--
+jgs: It makes me sad that we're defining a new registry that appears to
+need to be kept in sync with
+https://www.iana.org/assignments/segment-routing/segment-routing.xhtml#segment-types<https://urldefense.com/v3/__https://www.iana.org/assignments/segment-routing/segment-routing.xhtml*segment-types__;Iw!!NEt6yMaO-gk!GcANokg-oBRf06lI8IW_5LhkGS9mnhtDdT7LLCGeMKWznZ9hGnIZqr3veu4jWXSVJJMXZZ6z4NfHWmAwzQ$>

+Is there a good reason this can't/shouldn't be a new column in the
+existing Segment Types registry? Beyond the obvious desire to avoid
+duplication, unifying the registries might also offer a helpful hint to
+people defining new Segment Types that they should also not forget to
+define a new Segment Descriptor.
++--

KT> Good point. This one can still be augmented within the SR Parameters registry group. However, we cannot do that for BGP SR Policy SAFI (the southbound protocol) - see https://www.ietf.org/archive/id/draft-ietf-idr-sr-policy-safi-13.html#section-6.5<https://urldefense.com/v3/__https://www.ietf.org/archive/id/draft-ietf-idr-sr-policy-safi-13.html*section-6.5__;Iw!!NEt6yMaO-gk!GcANokg-oBRf06lI8IW_5LhkGS9mnhtDdT7LLCGeMKWznZ9hGnIZqr3veu4jWXSVJJMXZZ6z4NcTKwtvAQ$> and we cannot also do that for similar things in PCEP. Please let me know if you still feel strongly about putting only the BGP-LS code points in the SR Parameters registry group.

Let’s make this a topic to discuss also.

KT2> Ack

KT3> I am going with the option of introducing additional guidance for the DEs to ensure consistency between the two registries.

Thanks,
Ketan


Thanks,
Ketan


Thanks,

—John