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

[Ketan Talaulikar <[e-mail]>]

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]> 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]>
> 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]> 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.


>
> ...
>
> @@ -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 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.


>
>
>
>>        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.


>
>
>
>>
>> +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

Thanks,
Ketan


>
> Thanks,
>
> —John
>
>