Re: [Teas] WG Last Call: draft-ietf-teas-yang-path-computation-18

Hi Dhruv,
Thanks for your review and comments.
Please see in line our replies.
We have fixed most of the comments in github , see PR #111<https://github.com/rvilalta/ietf-te-path-computation/pull/111>

Thanks
Italo and Sergio (on behalf of authors/contributors)

From: Teas <teas-bounces@ietf.org> On Behalf Of Dhruv Dhody
Sent: Friday, June 24, 2022 4:06 PM
To: Vishnu Pavan Beeram <vishnupavan@gmail.com>
Cc: TEAS WG <teas@ietf.org>; TEAS WG Chairs <teas-chairs@ietf.org>
Subject: Re: [Teas] WG Last Call: draft-ietf-teas-yang-path-computation-18

Hi WG,

Please find my review comments -

# Review of draft-ietf-teas-yang-path-computation-18

## Minor

### Abstract

* Abstract only mentions "intra-domain paths", whereas the documents do talk about inter-layer and inter-domain path computation as well.
[Belotti, Sergio (Nokia - IT/Vimercate)] fixed

### Section 1.3

* Remove inet from the table
[Belotti, Sergio (Nokia - IT/Vimercate)] fixed

### Section 2

* You state "Use cases d) and e)..." without stating what d) and e) is. Perhaps use section numbers instead?
[Belotti, Sergio (Nokia - IT/Vimercate)] fixed

### Section 2.1

* Figure 1 and Figure 2 are completely different network topologies, but the text gives an impression that they are related. I suggest making it clear that Figure 2 shows a different network topology.
[Belotti, Sergio (Nokia - IT/Vimercate)] fixed

### Section 2.4

* Please update "[RFC5441] has defined the Virtual Source Path Tree (VSPT) TLV within PCE Reply Object..." to "[RFC5441] has defined the Virtual Source Path Tree (VSPT) flag within RP (Request Parameters) object..."
[Belotti, Sergio (Nokia - IT/Vimercate)] fixed

### Section 5.3.1

* Could you explain the rationale for the fact that the requested path can only be a primary path for non-transit cases?
[Belotti, Sergio (Nokia - IT/Vimercate)] Rephrased to clarify that in non-transit case the only path must be a primary path.

### Section 5.3.2

* The phrase "empty ERO" makes sense for PCEP, but not for YANG. Please rephrase.
[Belotti, Sergio (Nokia - IT/Vimercate)] fixed

### Section 6.2 (YANG Model)

* WG Web: should point to datatracker instead of tools page
[Belotti, Sergio (Nokia - IT/Vimercate)] fixed

* Authors/Editors mismatch between the front page and YANG
[Belotti, Sergio (Nokia - IT/Vimercate)] Our understanding is that there are no requirements for an exact match between the editors of YANG and the front page editors of the RFC

* Copyright is incorrect, it should be Revised BSD. https://trustee.ietf.org/documents/trust-legal-provisions/tlp-5/
[Belotti, Sergio (Nokia - IT/Vimercate)] fixed

* Reference for svec-metric-cumul-hop should be RFC5541
[Belotti, Sergio (Nokia - IT/Vimercate)] fixed

* Is the use of the term "configuration" inside grouping requested-state correct? After all, this is part of the RPC input, can that be called
configuration?
[Belotti, Sergio (Nokia - IT/Vimercate)] We think it is correct since the grouping contains the attributes to configure how the transient state should be reported (e.g., the expiration timer)

* For objective-function-type in grouping synchronization-optimization, the use of "default "te-types:of-minimize-cost-path" is a problem, because this OF is not applicable to SVEC. I don't think you need a default here!
[Belotti, Sergio (Nokia - IT/Vimercate)] We need to further investigate whether the SVEC objective functions the same or different from the path objective functions: see issue #107<https://github.com/rvilalta/ietf-te-path-computation/issues/107>

* I am not able to make sense of this when statement. Is this when the statement suppose to tell it is transit? How?
```
          choice path-role {
            when 'not (./source) and not (./destination) and
                  not (./src-tunnel-tp-id) and
                  not (./dst-tunnel-tp-id)' {
```
[Belotti, Sergio (Nokia - IT/Vimercate)] The choice applies only to transit tunnels (i.e., when the source, src-tp-id, destination and dst-tp-id attributes are empty). See description in section 5.3.1 of this draft and in section 5.2.1 of draft-ietf-ccamp-transport-nbi-app-statement-15. Added some more details to the YANG description.

* This description is incorrect in the context of the container, it is the description of the leaf primary-path-name that follows -
```
            container secondary-path {
              presence
                "Indicates that the requested path is a secondary
                path.";
              description
                "The name of the primary path which the requested
                primary reverse path belongs to.";
```
[Belotti, Sergio (Nokia - IT/Vimercate)] OK: fixed c/primary reverse path/secondary-path/

## Appendix A

* The IP address in the example should be from the range reserved for documentation. See RFC5737. I see this issue exists in draft-ietf-teas-yang-te-29 as well.
[Belotti, Sergio (Nokia - IT/Vimercate)] Ok, to be fixed in #108<https://github.com/rvilalta/ietf-te-path-computation/issues/108> (aligning the changes to the IP address within the TE tunnel draft)

## Nits

* s/PCE request message/PCReq message/
* s/PCE Response message/PCRep message/
* Section 3.2.1, Figure 8 has cost 65, the text states the cose as 60
* s/srlgs/SRLGs/
[Belotti, Sergio (Nokia - IT/Vimercate)] ok, fixed

Thanks,
Dhruv

On Tue, May 24, 2022 at 11:15 PM Vishnu Pavan Beeram <vishnupavan@gmail.com<mailto:vishnupavan@gmail.com>> wrote:
All,

This starts working group last call on
https://datatracker.ietf.org/doc/draft-ietf-teas-yang-path-computation/

Given the size of the document, this will be an extended
LC (3 weeks). The working group last call ends on June 14th.
Please send your comments to the working group mailing list.

Positive comments, e.g., "I've reviewed this document
and believe it is ready for publication", are welcome!
This is useful and important, even from authors.

Thank you,
Pavan and Lou
_______________________________________________
Teas mailing list
Teas@ietf.org<mailto:Teas@ietf.org>
https://www.ietf.org/mailman/listinfo/teas