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

[Ketan Talaulikar <[e-mail]>]

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


On Thu, Feb 27, 2025 at 12:16 PM Ketan Talaulikar <[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]> 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.
>


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