Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-29
Adrian Farrel <adrian@olddog.co.uk> Mon, 11 July 2022 17:55 UTC
Return-Path: <adrian@olddog.co.uk>
X-Original-To: teas@ietfa.amsl.com
Delivered-To: teas@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 9E71EC15A747; Mon, 11 Jul 2022 10:55:23 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.893
X-Spam-Level:
X-Spam-Status: No, score=-1.893 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_BLOCKED=0.001, SPF_HELO_NONE=0.001, SPF_NONE=0.001, T_KAM_HTML_FONT_INVALID=0.01, T_SCC_BODY_TEXT_LINE=-0.01, URIBL_BLOCKED=0.001, URIBL_DBL_BLOCKED_OPENDNS=0.001, URIBL_ZEN_BLOCKED_OPENDNS=0.001] autolearn=ham autolearn_force=no
Received: from mail.ietf.org ([50.223.129.194]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id LLc95bYrYpWX; Mon, 11 Jul 2022 10:55:19 -0700 (PDT)
Received: from mta5.iomartmail.com (mta5.iomartmail.com [62.128.193.155]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 4E37CC15790B; Mon, 11 Jul 2022 10:55:17 -0700 (PDT)
Received: from vs2.iomartmail.com (vs2.iomartmail.com [10.12.10.123]) by mta5.iomartmail.com (8.14.7/8.14.7) with ESMTP id 26BHtEVp031551; Mon, 11 Jul 2022 18:55:14 +0100
Received: from vs2.iomartmail.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 4ADED46050; Mon, 11 Jul 2022 18:55:14 +0100 (BST)
Received: from vs2.iomartmail.com (unknown [127.0.0.1]) by IMSVA (Postfix) with ESMTP id 3CC1F46052; Mon, 11 Jul 2022 18:55:14 +0100 (BST)
Received: from asmtp3.iomartmail.com (unknown [10.12.10.224]) by vs2.iomartmail.com (Postfix) with ESMTPS; Mon, 11 Jul 2022 18:55:14 +0100 (BST)
Received: from LAPTOPK7AS653V ([148.252.133.43]) (authenticated bits=0) by asmtp3.iomartmail.com (8.14.7/8.14.7) with ESMTP id 26BHtBYK029373 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NO); Mon, 11 Jul 2022 18:55:12 +0100
Reply-To: adrian@olddog.co.uk
From: Adrian Farrel <adrian@olddog.co.uk>
To: 'Tarek Saad' <tsaad.net@gmail.com>
Cc: 'TEAS WG' <teas@ietf.org>, teas-chairs@ietf.org, draft-ietf-teas-yang-te@ietf.org
References: <409bd958-008c-76df-1692-221ab8dfbab0@labn.net> <06c501d84d22$e975ade0$bc6109a0$@olddog.co.uk> <DS0PR19MB6501B33D8C8A0AC822544E52FCCA9@DS0PR19MB6501.namprd19.prod.outlook.com> <DS0PR19MB6501B9942732A1C5D53FEF77FC879@DS0PR19MB6501.namprd19.prod.outlook.com>
In-Reply-To: <DS0PR19MB6501B9942732A1C5D53FEF77FC879@DS0PR19MB6501.namprd19.prod.outlook.com>
Date: Mon, 11 Jul 2022 18:55:11 +0100
Organization: Old Dog Consulting
Message-ID: <033a01d8954f$5ef79af0$1ce6d0d0$@olddog.co.uk>
MIME-Version: 1.0
Content-Type: multipart/alternative; boundary="----=_NextPart_000_033D_01D89557.C0CCCBD0"
X-Mailer: Microsoft Outlook 16.0
Thread-Index: AQHvSVN9onMQ0gC94Mob8o440099MwGbk2GnAVRlc+8BbcmgQq0pHO6g
Content-Language: en-gb
X-Originating-IP: 148.252.133.43
X-Thinkmail-Auth: adrian@olddog.co.uk
X-TM-AS-GCONF: 00
X-TM-AS-Product-Ver: IMSVA-9.1.0.2090-9.0.0.1002-27010.001
X-TM-AS-Result: No--23.783-10.0-31-10
X-imss-scan-details: No--23.783-10.0-31-10
X-TMASE-Version: IMSVA-9.1.0.2090-9.0.1002-27010.001
X-TMASE-Result: 10--23.783400-10.000000
X-TMASE-MatchedRID: jFqw+1pFnMxfsB4HYR80ZnFPUrVDm6jtkYC3rjkUXRJcZUa13Wf//0Ij Ip4EpSqCkprVpplFRlgtZ+Yu4uYPVEPPi0oqTn8ElVHM/F6YkvQ2LwvzxRX0gI2cB1+t/hle0Ra 6PezBhkAKoiVTDA+B7JTIMHQYuXBtr8SWmHOl/UtQsahOMIZ15XEJxqEF0kDO2Qpm+FTHj9MXdu dYOKS8OlWYI+V0xqhufT79No1E+AVjDV//SvkH3uScxJMb2Uf4NWN4cIQzhfZvNs77F4nIWhkhC hIO6vg8tQsQjGYTOpEXB7/m9NWCpsVQ1jrs5s2SDZs/Kgmqdkus4IQYg+G3CDYzQ2g9isXl07za VZEg+DPQY+Vxne94JdCGuesANG2NtE4rffipOm7RCUFHAmq1I0yQ5fRSh265shUUKa5wa6VexHV WV7Fz76Iria44tVY76JBhFChuHBL+Emqe+OrfJATtNEP9j0LPBFF0/8xJ1RMmAcWAOshCoDlCRr 8Hb3qie0wUcH160FTDcuzAdMciB+/i00nUfc4wNuAqqzqy7pEXivwflisSrP4DDXoaCqk7qy49Z AU+sqVdxAn8q9TnRXc0GoR2aQuYgp4XG6WY1bNH4a2iJdV4MVkN0eJOT05w7BS7GFkJ1jLc4g8j wT7+bk2qC9aSduX2U6TDR6ZPvE1I0iy+6/jfH83WPbtc5QfmlQszlmkWAikyr+EEjz9hsfto1vM dVRxRmk4WTYuTAYQHMSexFgmXDm/cHuU/JlpMoS0guoV6SZfTAe9FsgbYx0xqTmWcX8+mn9svyU wUlFXvOPZOIImsUXMZvgn+O+j0X0LCk9pDuRCeAiCmPx4NwGmRqNBHmBvevqq8s2MNhPDOG2o4D tJIL5vLE5hS3p8WU6baA36eiawgbhiVsIMQKxZ5+8y352uC
X-TMASE-SNAP-Result: 1.821001.0001-0-1-22:0,33:0,34:0-0
Archived-At: <https://mailarchive.ietf.org/arch/msg/teas/_pE1CJevmhvWlocpCB244BW43t8>
Subject: Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-29
X-BeenThere: teas@ietf.org
X-Mailman-Version: 2.1.39
Precedence: list
List-Id: Traffic Engineering Architecture and Signaling working group discussion list <teas.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/teas>, <mailto:teas-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/teas/>
List-Post: <mailto:teas@ietf.org>
List-Help: <mailto:teas-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/teas>, <mailto:teas-request@ietf.org?subject=subscribe>
X-List-Received-Date: Mon, 11 Jul 2022 17:55:23 -0000
Hi Tarek, Thanks for the status update. This all seems to be going in the right direction, so thanks for the work and sorry for dumping so many comments on you. Adrian From: Tarek Saad <tsaad.net@gmail.com> Sent: 11 July 2022 16:17 To: Adrian Farrel <adrian@olddog.co.uk> Cc: TEAS WG <teas@ietf.org>; teas-chairs@ietf.org; draft-ietf-teas-yang-te@ietf.org Subject: Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-29 Hi Adrian, Thank you very much for thoroughly reviewing this draft and for providing all the comments. The authors have tried to address some of your comments (some remain outstanding and pending closure). We have published a new revision of the draft (rev -30) that includes the changes. The diff is at: https://www.ietf.org/rfcdiff?url2=draft-ietf-teas-yang-te-30.txt Note, we intend to send a follow-up email to close on the outstanding points soon. Please see inline below for responses to your comments ([TS] for closed comments, and [TS/OPEN-ISSUE] for the outstanding ones). Regards, Tarek (for the co-authors) From: Teas <teas-bounces@ietf.org <mailto:teas-bounces@ietf.org> > on behalf of Adrian Farrel <adrian@olddog.co.uk <mailto:adrian@olddog.co.uk> > Date: Sunday, April 10, 2022 at 5:35 PM To: 'Lou Berger' <lberger@labn.net <mailto:lberger@labn.net> >, 'TEAS WG' <teas@ietf.org <mailto:teas@ietf.org> > Cc: 'TEAS WG Chairs' <teas-chairs@ietf.org <mailto:teas-chairs@ietf.org> > Subject: Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-29 Hi, I have reviewed this draft as it goes through WG last call. I think there are a few nits to resolve before this is ready for publication. Cheers, Adrian === I think Xufeng has changed affiliation --- [TS]: Fixed 1. s/describes YANG data model/describes a YANG data model/ [TS]: Fixed --- 1. The document describes a high-level relationship between the modules defined in this document, as well as other external protocol YANG modules. I think it would be nice to introduce (in this section, and before this paragraph) the modules defined in this document. --- [TS]: Fixed 1. The model covers data applicable to generic or device-independent, device- specific, and Multiprotocol Label Switching (MPLS) technology specific. Something wrong with this sentence. Maybe... The model includes data that is generic, device-independent, device-specific, and specific to Multiprotocol Label Switching (MPLS) technology. [TS]: Fixed --- Throughout the document. It isn't necessary to represent "optional plural(s)". In English, the plural includes the singular so you can write (for example) "it is expected other YANG module that model TE signaling protocols" [TS]: Fixed --- The section headings in Section 2 seem mixed up. I think you need... 2. Terms and Conventions 2.1. Requirements Language 2.2. Terminology 2.3. Prefixes in Data Node Names 2.4. Model Tree Diagrams [TS]: Fixed --- 3. You begin with "This document describes a generic TE YANG data model that is independent of any dataplane technology." I feel that this point is quite important and is missing from the Abstract and the Introduction. [TS]: Fixed. --- 3. s/signaling protocol/signaling protocols/ s/respect data organization/respect to data organization/ [TS]: Fixed. --- 3. I don't know what this means: "can be used to model data off a device" Is your point that this is not just for reading or writing information from/to a device, but is also to allow a third party to hold a representation of the modeled entity? If so, great, but I think you have only discovered the whole point of data modeling. If you mean something else, it is not clear. [TS]: A TE system can reside on the device or off the device (TE controller). We intend to add this clarification in next revision: NEW: When the model is used to manage a specific device, the model contains the TE Tunnels originating from the specific device. When the model is used to manage a TE controller, the 'tunnels' list contains all TE Tunnels and TE tunnel segments originating from device(s) that the TE controller manages. --- 3. The device-specific TE data is defined in module 'ietf-te-device' as shown in Figure 1, Do you mean Figure 1? Doesn't seem like the right figure. Should end with a period not a comma. [TS]: Figure 1 is correct as it shows the relationship of the different modules. Corrected the punctuation. --- 3. Aren't bullets 2 and 4 saying the same thing? [TS]: not necessarily. In YANG optional/mandatory can be specified for a specific leaf. A YANG feature can be negotiated between server and client to hide/show certain nodes of the data model. --- 4. The support of extended or vendor specific TE feature(s) is expected to be in either augmentations, or deviations to the model defined in this document. s/be in either/either be in/ However, the text is ambiguous because it could be read to mean that this document defines augmentations or deviations. [TS]: Fixed --- 4.1 s/The TE data model/The TE data models/ (twice) s/in a separate YANG module(s)/in separate YANG modules/ [TS]: Fixed --- 4.1 defined in another document Missing a citation [TS]: reworded to avoid citation to work outside the scope of this document. OLD: For example, the MPLS-TE module "ietf-te-mpls.yang" is defined in another document and augments the TE generic model as NEW: The TE data models for specific instances of signaling protocols are outside the scope of this document and are defined in other documents. --- In Figure 1, is the text "o: augment" intended to be specific to the one relationship it appears next to, or is it a general key explaining the meaning of the "o" symbol. I think you could tidy up Figure 1 as TE generic +---------+ module | ietf-te |o-------------+ +---------+ \ o \ |\ \ | \ TE device module \ | +----------------+ \ | | ietf-te-device | \ | +----------------+ \ | o o \ | / \ \ | / \ \ RSVP-TE +--------------+ +---------------+ module | ietf-rsvp-te |o | ietf-te-mpls^ | +--------------+ \ +---------------+ | \ MPLS-TE module | \ | \ | \ | \ o +-------------------+ RSVP +-----------+ | ietf-rsvp-otn-te^ | module | ietf-rsvp | +-------------------+ +-----------+ Module for OTN extensions to RSVP-TE X---oY indicates that module X augments module Y ^ indicates a module defined in another document [TS]: Fixed. --- 5. The generic TE YANG module ('ietf-te') is meant to manage and operate a TE network. Ah, but does it succeed? Possibly... The generic TE YANG module ('ietf-te') is meant for management and operation of a TE network. [TS]: Fixed --- 5. This includes creating, modifying and retrieving TE Tunnels, LSPs, and interfaces and their associated attributes (e.g. Administrative-Groups, SRLGs, etc.). I don't think you retried Tunnels, LSPs, etc. Just information about them. [TS]: Fixed/reworded to: NEW: This includes creating, modifying and retrieving information about TE Tunnels, LSPs, and interfaces and their associated attributes (e.g. Administrative-Groups, SRLGs, etc.). --- 5. The detailed tree structure is provided in Figure 2. Not a lot of detail in Figure 2. Did you mean a different figure? [TS]: Fixed --- Section 5.1 is all a bit jumbled and would be a lot better as... 5.1. Module Structure The 'te' container is the top-level container in the 'ietf-te' module. The presence of the 'te' container enables TE function system wide. There are three further containers grouped under the 'te' container as shown in Figure 2 and described below. globals: The 'globals' container maintains the set of global TE attributes that can be applied to TE Tunnels and interfaces. tunnels: The 'tunnels' container includes the list of TE Tunnels that are instantiated. Refer to Section 5.1.2 for further details on the properties of a TE Tunnel. lsps: The 'lsps' container includes the list of TE LSPs that are instantiated for TE Tunnels. Refer to Section 5.1.3 for further details of the properties of a TE LSP. The model also contains two Remote Procedure Calls (RPCs) as shown in Figure 2 and described below. tunnels-path-compute: An RPC to request path computation for a specific TE Tunnel. The RPC allows requesting path computation using atomic and stateless operation. A tunnel may also be configured in 'compute-only' mode to provide stateful path updates - see Section 5.1.2 for further details. tunnels-action: An RPC to request a specific action (e.g. reoptimize, or tear-and- setup) to be taken on a specific tunnel or all tunnels. Figure 2, shows the relationships of these containers and RPCs within the 'ietf-te' module. module: ietf-te +--rw te! +--rw globals . . +--rw tunnels . . +-- lsps rpcs: +---x tunnels-path-compute +---x tunnels-action Figure 2: TE Tunnel model high-level YANG tree view [TS]: Fixed. --- 5.1.1 OLD The 'globals' container covers properties that control TE features behavior system-wide, and its respective state (see Figure 3). The TE globals configuration include: NEW The 'globals' container covers properties that control a TE feature's behavior system-wide, and its respective state as shown in Figure 3 and described in the text that follows. END [TS]: Fixed. --- 5.1.1 s/list named/list of named/ s/The path constraint/The path constraints/ [TS]: Fixed. --- 5.1.1 OLD formed up of the following path constraints: NEW formed of the path constraints shown in Figure 4. END [TS]: Fixed. --- 5.1.1 OLD o setup/hold priority: A YANG leaf that holds the LSP setup and hold admission priority as defined in [RFC3209]. NEW o setup/hold priority: YANG leafs that holds the LSP setup and hold admission priority as defined in [RFC3209]. END [TS]: Fixed. --- 5.1.1 s/exclusions instruction/exclusion instructions/ (three times) [TS]: Fixed. --- 5.1.1 3. An empty 'route-object-include-exclude' list for a reverse path means it always follows the forward path (i.e., the TE Tunnel is co-routed). When the 'route-object-include- exclude' list is not empty, the reverse path is routed independently of the forward path. How would you show that the reverse path is routed independently, but that there are no constraints on the path? Do you have to include a random link that couldn't possibly be on the path just to make this point? [TS]: We have introduced a new 'co-routed' boolean leaf to explicitly specify co-routedness. We have added a YANG when check to assert it is only set for bidirectional LSPs. --- 5.1.2 TE Tunnel: A YANG container of one or more LSPs established between the source and destination TE Tunnel termination points. A TE Tunnel LSP: is a connection-oriented service provided by the network layer for the delivery of client data between a source and the destination of the TE Tunnel termination points. I had to look down at Figure 8 to find where LSPs sit in the tree. There I found that there are some mentions under 'tunnels' but mainly deeper under the various "path" branches such as 'primary-paths'. Perhaps the problem here is that you have introduced LSPs before talking about tunnel paths (5.1.2.1> so that it is necessary to read through to the end of 5.1.3 in order to see how it all hangs together. [TS]: added text to describe "TE Path" and elaborated on relationship of Tunnel -> Path -> LSP. NEW: TE Path: An engineered path that once instantiated can be used to forward traffic from the TE unnel source termination point to the TE tunnel destination termination point. --- 5.1.2 The text describing the leaf nodes after Figure 5 does not appear to be in the same order as the tree diagram. 'description' and 'admin-state' are not described. XX: Fixed. There is no mention of whether there is any scope of uniqueness for the 'alias'. [TS]: the alias is not a key in the tunnel list and need not be unique. The alias was introduced because there is a need to change the tunnel name after it has been instantiated (refer to <https://github.com/tsaad-dev/te/issues/50#issuecomment-631940823> https://github.com/tsaad-dev/te/issues/50#issuecomment-631940823). To be clear (and deducing from the reference), the 'color' is only to be used when the tunnel is exposed in BGP as described in RFC 9012 and has no other use. [TS]: yes, this color is used for services mapping to TE tunnel. Great that you have the encoding and switching type and reference RFC 3945 for their meanings. I'd note that RFC 8779 actually references RFC 3471 for the meaning of these two things. I wondered by you also didn't show the G-PID (or encapsulation type) along with encoding and switching type. One might assume that this is only of interest to the end points, but it is important for management and for getting traffic into and out of the tunnel. But I see that RFC 8779 doesn't seem to cover that either. Any thoughts? [TS/OPEN-ISSUE]: do we need the G-PID? s/inteval/interval/ I'm OK with what you have, but note that your definitions of source and destination seem to preclude ingress and egress protection schemes. The text at the top of the section... When the model is used to manage a TE controller, the 'tunnels' list contains all TE Tunnels and TE tunnel segments originating from device(s) that the TE controller manages. ...is confusing when considered in the context of the 'controller' leaf. [TS]: the controller container holds information about the controller owner. This may be useful to show on other controller(s) in the network that are 'non-owners'. Somewhere in the document you should be noting the implication of setting 'bidirectional' to true for the meaning of the various reverse-path objects. [TS/OPEN-ISSUE]: elaborate on implication of setting bidirectional in the document. You have: The TE tunnel associations can be overridden by associations configured directly under the TE Tunnel path. Is it true that they are overridden? Or are they "supplemented"? That is, if you say that "these two tunnels are associated" is there anything you can do in the configuration of the paths to say that they are not? [TS/OPEN-ISSUE]: the team has not converged on whether 'override' or 'supplement' or both options may be needed. s/(see Figure 6./(see Figure 6)./ s/as a TE links/as a TE link/ [TS]: Fixed. --- 5.1.2.1 s/can be specified a set/can be specified as a set/ s/describe further/describes further/ s/The following set common path/The following set of common path/ A primary path is identified by 'name'. So, what is the uniqueness property required here? Just unique within the tunnel? [TS]: unique within each path list. Looking at 'preference', which first appears here, it is clear from the Description of 'path-preference' how preferences are ranked, but there is nothing to say how you distinguish between to equal values. Possibly that situation isn't allowed, but there is nothing said about that. [TS]: multiple with same preference are allowed. The server is free to pick any of the equal paths and activate it. --- 5.1.2.1 A secondary path contains attributes similar to a primary path. How "similar"? What are the differences? [TS/OPEN-ISSUE]: elaborate on differences s/holds teh set/holds the set/ s/following set common path/following set of common path/ s/A path of TE Tunnel is/A path of a TE Tunnel is/ s/so that it can is/so that it can be/ s/instantiated in forwarding/instantiated in the forwarding plane/ [TS]: Fixed. --- 5.1.2.1 In some cases, a TE path may be provisioned for the only purpose of computing a path and reporting it without the need to instantiate the LSP or commit any resources. "Provisioned" may not be the best word. It usually means "provisioned in the network" which normally means "instantiated". How about "configured". [TS]: Accepted. s/for the only purpose/only for the purpose/ s/a YANG container/A YANG container/ (twice) [TS]: fixed. --- 5.1.3 The 'lsps' container includes the set of TE LSP(s) that are instantiated. Should that be s/that are/that have been/ [TS]: OK. --- 5.3 Looks like the copyright statement in the ietf-te module is old. [TS]: Fixed. --- 5.3 In path-computation-error-reason you are capturing some of (all?) of the PCE No-Path-Vector bits in the NO-PATH-VECTOR TLV Flag Field registry at <https://www.iana.org/assignments/pcep/pcep.xhtml#no-path-vector-tlv> https://www.iana.org/assignments/pcep/pcep.xhtml#no-path-vector-tlv. It is highly confusing that, although you give the same references for the explanations, you don't use exactly the same wording. It would also be really helpful to list them in the same order as in the registry - at the very least, this would help me work out which reasons you have left out so that I can ask you why you left them out. But, that brings up another point. It seems to be messy that you have defined a list of reasons here, but if new reasons get assigned in the registry, each one has to be added through a new augmentation. I think the conventional way to handle this is through an IANA managed module that you include. [TS/OPEN-ISSUE]: working with team on a way forward regarding this point. --- 5.3 Sadly (and my fault for not picking this up during the review of RFC 8776 before publication) te-types defines the base type 'association-type' which is actually a mirror of the IANA registry at <https://www.iana.org/assignments/pcep/pcep.xhtml#association-type-field> https://www.iana.org/assignments/pcep/pcep.xhtml#association-type-field This gives us all a problem when new association types are introduced. Indeed, you don't have "SR Policy Association" in the base type. That is probably the first of many new associations that will get added. Again, this should probably have been in an IANA maintained module that could be included. Not sure whether this is something you can fix here. It probably needs: - Define the IANA YANG module (in this document or a separate document with a request for IANA to maintain it) - Import the IANA module in ietf-te - Make a bis of 8776 to drop association-type-field and import the new IANA module The third of these might be optional, although it seems like a good idea. [TS/OPEN-ISSUE]: working with team on a way forward regarding this point. This all seems a lot better than what you have done here which is define a new association type to match "Disjoint Association" (annoying that you gave it a different name!) that was left out of 8776. What you have done here means that someone else who wants to also use that association type must define it themselves or import your whole module! --- 5.3 Why do you use 5512 as a reference for protocol-origin-bgp when it has been obsoleted by 9012? Why are neither 5512 nor 9012 mentioned in the preamble txt at the top of 5.3? [TS]: Fixed. --- 5.3 Wondering why primary-reverse-path gets a Reference when primary-path, secondary-path, and secondary-reverse-path don't. Even the reference for primary-reverse-path is a little thin since 7551 doesn't mention "primary" in the whole document. Why doesn't primary-reverse-path have a path-preference? [TS]: primary-reverse-path is single path, so no preference arbitration is needed. --- 5.3 leaf compute-only { type empty; description "When set, the path is computed and updated whenever the topology is updated. No resources are committed or reserved in the network."; } "computed and updated whenever the topology is updated" Do you mean this? Surely there are some thresholds and relevance tests? [TS]: compute-only tunnels are treated similar to regular tunnels. So, yes a compute-service may optimize acting on relevant topology updates as the case for any other tunnel. When compute-only is set, are a number of other leaf nodes meaningless and to be ignored? [TS]: yes, the leafs relevant to LSP instantiation are optional and will not be populated. The text "When set" should probably be "When present" [TS]: Fixed. --- 5.3 I think that the Description for use-path-computation may not cover what you intend. You have: "When 'true' indicates the path is dynamically computed and/or validated against the Traffic-Engineering Database (TED), and when 'false' indicates no validation against the TED is required."; So, if the path is incomplete (i.e., computation is required) and this field is set to false, is the path computed? Or what happens? [TS]: If set to 'false', the user-supplied path is instantiated as-is without further validation or expansion by the computation engine. The onus is on the operator to provide a full valid path. If path expansion (even partial) is required, then use-path-computation 'true' is expected. The text further up the document doesn't mention "validation". [TS]: rephrased as: description "When 'true' indicates the path is dynamically computed and/or validated against the Traffic-Engineering Database (TED), and when 'false' indicates no path expansion or validation against the TED is required."; --- 5.3 /* This grouping will be re-used in path-computation rpc */ s/will be/is/ ? (several times) [TS]: Fixed. --- 5.3 The only mention of path-scope is when it appears in the module itself. Here the Description tells use the meaning, but not how it gets set by the implementation (yes, I see it is read-only). 8776 doesn't help with this (just defining the base type). More words are needed. leaf path-scope { type identityref { base te-types:path-scope-type; } default "te-types:path-scope-end-to-end"; config false; description "Indicates whether the path is a segment or portion of of the full path., or is the an end-to-end path for the TE Tunnel."; } [TS]: Updated the description above. --- 5.3 grouping k-requested-paths { description "The k-shortest paths requests."; leaf k-requested-paths { type uint8; range 1..255 default "1"; description "The number of k-shortest-paths requested from the path computation server and returned sorted by its optimization objective. "; } } Isn't "all possible paths" a potential exposure? You seem to recognise this by limiting the requested number of paths to 255. But then you specifically allow "all" which could be a very large number of paths. I think you should cap this by allowing the "path computation server" (wow, there is some old terminology ;-) to limit how many paths it returns in all cases, and in any case saying that it must not return more than 255 paths. That said, 255 is still a lot of paths especially if you are doing recomputations every time the topology is updated. [TS/OPEN-ISSUE]: working with team to address this. --- 5.3 I think I am a little disappointed that lsp-provisioning-error-info only has a string to describe the provisioning error: no numeric code or identity. [TS/OPEN-ISSUE]: working with team to address this. We may be adding a base error identity that other path-setup technology modules can extend. --- Seems you have some things like "When set to 'True'" and "If 'False'" These should be 'true' and 'false'. [TS]: Fixed. --- 5.3 leaf lsp-protection-state { type identityref { base te-types:lsp-protection-state; } default "te-types:normal"; description "The state of the APS state machine controlling which tunnels is using the resources of the protecting LSP."; } Either "tunnel is" or "tunnels are" [TS]: Fixed. Why does this text talk about the APS state machine when 8776 makes reference to 4427? Possibly the references in 8776 are wrong and should have pointed to 7271 and 8234. If you intend those meanings of APS then you should simply add references to your document. (Raising an Errata Report against 8776 for this would be a bonus) [TS]: Added reference to RFC7271 and RFC8234. [TS/OPEN-ISSUE]: to track other suggestions. --- 5.3 I think the reference for aps-signal-id is wrong. [TS/OPEN-ISSUE]: working with team to address this. --- 5.3 Shouldn't the hold-off, wait-to-restore, and wait-to-revert timers have defaults? [TS]: the approach we followed was to specify a default when one is specified in the RFC/standard. Otherwise, we leave it as an implementation choice. --- 5.3 Why are src-tunnel-tp-id and dst-tunnel-tp-id of type binary and not te-tp-id? [TS]: for tunnel termination point, we tried to be consistent with rfc8795. --- 5.3 leaf protection-group-ingress-node-id { type te-types:te-node-id; description "When specified, indicates whether the action is applied on ingress node. By default, if neither ingress nor egress node-id is set, the action applies to ingress node only."; } leaf protection-group-egress-node-id { type te-types:te-node-id; description "When specified, indicates whether the action is applied on egress node. By default, if neither ingress nor egress node-id is set, the action applies to ingress node only."; } The "by default" language here implies that the ingress node id is known even when not set here. In that case, why do you need these to be of type te-node-id? Wouldn't boolean do just as well? [TS]: this was reworked and suggestion was incorporated. Updated below: leaf protection-group-ingress-node { type boolean; default "true"; description "When 'true', indicates that the action is applied on ingress node. By default, the action applies to the ingress node only."; } leaf protection-group-egress-node { type boolean; default "false"; description "When set to 'true', indicates that the action is applied on egress node. By default, the action applies to the ingress node only."; } BTW, this is not "by default". It is "if" since you are defining a meaning that has no other override. --- 5.3 Would you consider renaming the lsp-record-route-information-state etc. to lsp-actual-route-information-state etc. and dropping mention of RSVP? This might prove to be more useful in the general (non-RSVP-TE) case. container lsp-record-route-information { | description | "RSVP recorded route object information."; | list lsp-record-route-information { [TS/OPEN-ISSUE]: working with team to address this. But, as I read a bit further beyond this grouping, I found a lot of stuff that was expressed in terms of RSVP-TE. Surely, either that should be generalised to be in this model, or it should be moved out into the RSVP-TE augmentation? Random example... leaf destination { type te-types:te-node-id; description "The tunnel endpoint address extracted from SESSION object."; reference "RFC3209"; } [TS]: reference to RSVP specific language SESSION/SENDER_TEMPLATE was removed. --- 6.1.1 s/interface attributes/Interface attributes/ [TS]: Fixed. --- 6.2 Figure 11 shows the tree diagram of the device TE YANG model defined in modules 'ietf-te.yang'. Possibly 'ietf-te-device.yang' [TS]: Fixed. --- 6.3 Copyright date [TS]: Fixed. --- The two modules seem a bit confused about where the controls for individual LSPs go. the base module has many things for configuring and operating individual LSPs, but the device model has the LSP timers that are applicable only at the ingress nodes for LSPs. Surely, those timers are also specific to the LSPs and could be grouped in the base model. XX: move to base. That said, the Description "Applicable to ingress LSPs only" may be missing an "of" or maybe s/P/R/ And anyway, the Descriptions of the timers are a bit vague. For example, does life-time report how long the LSP has been up or say after how long it will be torn down? [TS]: updated description of timers. --- Section 8 appears to be empty! [TS]: Removed. --- Section 13 needs to use IP addresses from the documentation range not real routable addresses. [TS/OPEN-ISSUE]: working with team to address this. --- I think all three of [RFC4427], [RFC8800], and [RFC9012] are used as explanations of items in the modules and so they should be normative references. [TS]: Fixed. -----Original Message----- From: Teas <teas-bounces@ietf.org <mailto:teas-bounces@ietf.org> > On Behalf Of Lou Berger Sent: 21 March 2022 13:28 To: TEAS WG <teas@ietf.org <mailto:teas@ietf.org> > Cc: TEAS WG Chairs <teas-chairs@ietf.org <mailto:teas-chairs@ietf.org> > Subject: [Teas] WG Last Call: draft-ietf-teas-yang-te-29 All, This starts working group last call on <https://datatracker.ietf.org/doc/draft-ietf-teas-yang-te/> https://datatracker.ietf.org/doc/draft-ietf-teas-yang-te/ Given the size of the document and this week's meeting, this will be an extended LC. The working group last call ends on April 13th. 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, Lou (Co-Chair & doc Shepherd) _______________________________________________ Teas mailing list Teas@ietf.org <mailto:Teas@ietf.org> <https://www.ietf.org/mailman/listinfo/teas> https://www.ietf.org/mailman/listinfo/teas _______________________________________________ Teas mailing list Teas@ietf.org <mailto:Teas@ietf.org> <https://www.ietf.org/mailman/listinfo/teas> https://www.ietf.org/mailman/listinfo/teas
- [Teas] WG Last Call: draft-ietf-teas-yang-te-29 Lou Berger
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… Adrian Farrel
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… t petch
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… Rakesh Gandhi
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… Italo Busi
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… t petch
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… Lou Berger
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… Tarek Saad
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… Tarek Saad
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… Adrian Farrel
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… julien.meuric
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… Tarek Saad
- Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-… Tarek Saad