Re: [RTG-DIR] Rtgdir last call review of draft-ietf-isis-sr-yang-17

[Yingzhen Qu <[e-mail]>]

Hi Shuping,

Thanks for the review. Please see my response below inline.

Thanks,
Yingzhen

On Fri, Nov 24, 2023 at 12:37 AM Shuping Peng via Datatracker <
[e-mail]> wrote:

> Reviewer: Shuping Peng
> Review result: Has Issues
>
> Hello,
>
> I have been selected as the Routing Directorate reviewer for this draft.
> The
> Routing Directorate seeks to review all routing or routing-related drafts
> as
> they pass through IETF last call and IESG review, and sometimes on special
> request. The purpose of the review is to provide assistance to the Routing
> ADs.
> For more information about the Routing Directorate, please see
> http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir
>
> Although these comments are primarily for the use of the Routing ADs, it
> would
> be helpful if you could consider them along with any other IETF Last Call
> comments that you receive, and strive to resolve them through discussion
> or by
> updating the draft.
>
> Document: draft-ietf-isis-sr-yang
> Reviewer: Shuping Peng
> Review Date: 2023-11-24
> IETF LC End Date: 2023-11-30
> Intended Status: Standards
>
> Summary:
> I have some minor concerns about this document that I think should be
> resolved
> before publication.
>
> Major Issues:
>  "No major issues found."
>
> Minor Issues:
> 1. Page 3, when configure adjacency-sid, do we need to indicate the
> neighbor's
> systemid or IP in order to differentiate the different neighbors in the
> case of
> having multiple neighbors?
>
> augment /rt:routing/rt:control-plane-protocols
>           /rt:control-plane-protocol/isis:isis/isis:interfaces
>           /isis:interface:
>     +--rw segment-routing
>        +--rw adjacency-sid
>           +--rw adj-sids* [value]
>           |  +--rw value-type?   enumeration
>           |  +--rw value         uint32
>           |  +--rw protected?    boolean
>

[Yingzhen]:  thanks for catching this. We didn't consider LAN interfaces,
will fix this in the next version.

>
> 2. Page 4, since LFA, RLFA and TI-LFA are the three algorithm for computing
> backup paths, why they are not in sibling relationship?
>
>   augment /rt:routing/rt:control-plane-protocols
>           /rt:control-plane-protocol/isis:isis/isis:interfaces
>           /isis:interface/isis:fast-reroute:
>     +--rw ti-lfa {ti-lfa}?
>        +--rw enable?   boolean
>   augment /rt:routing/rt:control-plane-protocols
>           /rt:control-plane-protocol/isis:isis/isis:interfaces
>           /isis:interface/isis:fast-reroute/isis:lfa/isis:remote-lfa:
>     +--rw use-segment-routing-path?   boolean {remote-lfa-sr}?
>
> [Yingzhen]: the assumption here is that LFA is preferred when
available.  Although in the ti-lfa draft, it says that LFA may not be
preferred over ti-lfa, however if there is LFA route available, the chance
of it also being post-convergence path is very high. I'll check with the
ti-lfa authors and some implementations.

3. Page 4, the keys of the global-block and local-block are not clear.
>
>   augment /rt:routing/rt:control-plane-protocols
>           /rt:control-plane-protocol/isis:isis/isis:database
>           /isis:levels/isis:lsp/isis:router-capabilities:
>     +--ro sr-capability
>     |  +--ro sr-capability
>     |  |  +--ro sr-capability-bits*   identityref
>     |  +--ro global-blocks
>     |     +--ro global-block* []
>     |        +--ro range-size?    uint32
>     |        +--ro sid-sub-tlv
>     |           +--ro sid?   uint32
>     +--ro sr-algorithms
>     |  +--ro sr-algorithm*   uint8
>     +--ro local-blocks
>     |  +--ro local-block* []
>     |     +--ro range-size?    uint32
>     |     +--ro sid-sub-tlv
>     |        +--ro sid?   uint32
>     +--ro srms-preference
>        +--ro preference?   uint8
>

[Yingzhen]: these are read-only data, so key is not a must.

>
> 4. Currently there is no configuration node for the micro loop avoidance
> (
> https://datatracker.ietf.org/doc/draft-bashandy-rtgwg-segment-routing-uloop/
> ),
> any thoughts or plan on it?
>
> [Yingzhen]: we can do an augmentation when the mentioned draft is ready to
progress.