Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-29

Tarek Saad <tsaad.net@gmail.com> Thu, 21 July 2022 15:46 UTC

Return-Path: <tsaad.net@gmail.com>
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 4B408C159497; Thu, 21 Jul 2022 08:46:01 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -7.105
X-Spam-Level:
X-Spam-Status: No, score=-7.105 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, FREEMAIL_FROM=0.001, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_HI=-5, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, 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
Authentication-Results: ietfa.amsl.com (amavisd-new); dkim=pass (2048-bit key) header.d=gmail.com
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 528wWVBA3Eku; Thu, 21 Jul 2022 08:45:57 -0700 (PDT)
Received: from mail-il1-x130.google.com (mail-il1-x130.google.com [IPv6:2607:f8b0:4864:20::130]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id E83A0C14F746; Thu, 21 Jul 2022 08:45:56 -0700 (PDT)
Received: by mail-il1-x130.google.com with SMTP id f1so989349ilu.3; Thu, 21 Jul 2022 08:45:56 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=from:to:cc:subject:thread-topic:thread-index:date:message-id :references:in-reply-to:accept-language:content-language :mime-version; bh=ZJ6+TGq6/MiPCHu5fklef+kbfi8EHTsKUY7vgEfAfXU=; b=hAnn4rUVbZlJ8oV1VXPHrMEbYiUVTghnis6GkDt6F1D7kj6Xjrx/Ex6M6ugs44WwON xgWb1+4o5u6gufFT3dJpaNzkcvwfCYq/ibQ78qFU1wXwiJsvzFsodPSifgbEThCflg98 Eiu9XpHBHUJFhD2vJ9/WTUVmYdA/c1Mo7DpEm9+Qg/8RvIzY98uJzKGjm7i+1Kz+6OVw SWsJCFAo/qkJ4ym+FJQkZEF8N+fNQplD/3rfXyspXQINILIbo3lZfUeZ7Ss3kfHYwmMP oEjXToeZnXJaIDeMKzw4WBzULoE/4qW5ZCqoFetgjd+WwanubgWJFv6Mzjmkrkomzgv9 yyJg==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:from:to:cc:subject:thread-topic:thread-index :date:message-id:references:in-reply-to:accept-language :content-language:mime-version; bh=ZJ6+TGq6/MiPCHu5fklef+kbfi8EHTsKUY7vgEfAfXU=; b=o7v5W8rCgagg8TvbOhuwggA1jPHn3CT4lKr8DFIxyys9dCkVpPRTNyG9X4yD+wakgx hK5ELHa9X6iCZHd6LYrvicbOJJQcy60MsVS/X9sgGrmY7WZr7yWR/X8H85eQ6cV5MSdv Cknzgnlo2upiAJqLYFuD78tUDodp/O7jI4ACM5nqMb8zQqucKHGSNaGsO1eDj1JNbn1k fvolrQEZPdTf0MmKX63Dfsbxggl8W8nqCEmKhXqn5bF31TWVUI60kZFNQTEeqCAd0aNZ auLQcpDtfgcrpDmxJ+Y23hKMmsRpwgaakCx2PTU6qXusyc5QCW1PsRzYd+ckrcWIT4iY IWxQ==
X-Gm-Message-State: AJIora+KJMFOZp6Z8Ot491yK2qiGN4DMr92/9lxGcaC2Z6mUd0XSdjZw 1apD/QDupLfC1tGzbKoMg5On55Blyco=
X-Google-Smtp-Source: AGRyM1u76chJyPPI26+w2QFkn5yUpe/PWd2TKwlHB5o9SRs14MVH7KYUG1fT0NYXuHGGU1InZDO++g==
X-Received: by 2002:a92:cd82:0:b0:2dc:e010:e94a with SMTP id r2-20020a92cd82000000b002dce010e94amr11590090ilb.226.1658418355613; Thu, 21 Jul 2022 08:45:55 -0700 (PDT)
Received: from DS0PR19MB6501.namprd19.prod.outlook.com ([2603:1036:5:f::5]) by smtp.gmail.com with ESMTPSA id f11-20020a02a10b000000b0033fbab06b75sm911815jag.46.2022.07.21.08.45.54 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Thu, 21 Jul 2022 08:45:55 -0700 (PDT)
From: Tarek Saad <tsaad.net@gmail.com>
To: "julien.meuric@orange.com" <julien.meuric@orange.com>
CC: TEAS WG <teas@ietf.org>, "teas-chairs@ietf.org" <teas-chairs@ietf.org>, "draft-ietf-teas-yang-te@ietf.org" <draft-ietf-teas-yang-te@ietf.org>
Thread-Topic: [Teas] WG Last Call: draft-ietf-teas-yang-te-29
Thread-Index: ATBiOTc40hBzor0AyuAb8o440099M8/V014AgDKcznyAXYNiJIALFXqAgASzeBE=
X-MS-Exchange-MessageSentRepresentingType: 1
Date: Thu, 21 Jul 2022 15:45:53 +0000
Message-ID: <DS0PR19MB65018C2486DFD73A08A97679FC919@DS0PR19MB6501.namprd19.prod.outlook.com>
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> <11631_1658159270_62D580A6_11631_123_1_04486cd1-ee24-94f0-831f-bd88e40f967f@orange.com>
In-Reply-To: <11631_1658159270_62D580A6_11631_123_1_04486cd1-ee24-94f0-831f-bd88e40f967f@orange.com>
Accept-Language: en-US
Content-Language: en-CA
X-MS-Has-Attach:
X-MS-Exchange-Organization-SCL: -1
X-MS-TNEF-Correlator:
X-MS-Exchange-Organization-RecordReviewCfmType: 0
Content-Type: multipart/alternative; boundary="_000_DS0PR19MB65018C2486DFD73A08A97679FC919DS0PR19MB6501namp_"
MIME-Version: 1.0
Archived-At: <https://mailarchive.ietf.org/arch/msg/teas/qsZ0_q9Y6kvgv_qDggLiLyGqcWk>
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: Thu, 21 Jul 2022 15:46:01 -0000

Hi Julian,

Thanks for raising this.
I believe the thinking was that a ‘link’ may be uniquely identified by the 3-tuple <(Local Node ID, Local ID), Remote Node ID> -- where Local ID is local-te-link-tp-id.
Personally, I don’t mind if remote-link-tp-id is there -- which can also be unset if not available. I’ll also let others chime in if needed too.

Regards,
Tarek

From: julien.meuric@orange.com <julien.meuric@orange.com>
Date: Monday, July 18, 2022 at 11:47 AM
To: Tarek Saad <tsaad.net@gmail.com>
Cc: TEAS WG <teas@ietf.org>, teas-chairs@ietf.org <teas-chairs@ietf.org>, draft-ietf-teas-yang-te@ietf.org <draft-ietf-teas-yang-te@ietf.org>
Subject: Re: [Teas] WG Last Call: draft-ietf-teas-yang-te-29
Hi Tarek,

Thanks for the regular work in this I-D. While looking at latest
revision, I've noticed that hierarchical-link doesn't include a
remote-te-link-tp-id leaf, only a remote-te-node-id is there to describe
the remote end point. Am I missing something about that expected attribute?

Thank you,

Julien


On 11/07/2022 17:17, Tarek Saad wrote:
>
> 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> on behalf of Adrian Farrel
> <adrian@olddog.co.uk>
> *Date: *Sunday, April 10, 2022 at 5:35 PM
> *To: *'Lou Berger' <lberger@labn.net>, 'TEAS WG' <teas@ietf.org>
> *Cc: *'TEAS WG Chairs' <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).
>
>
> 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.
>
> 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
>
> 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]: Addedreference 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> On Behalf Of Lou Berger
> Sent: 21 March 2022 13:28
> To: TEAS WG <teas@ietf.org>
> Cc: TEAS WG Chairs <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/
>
> 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
> https://www.ietf.org/mailman/listinfo/teas
>
> _______________________________________________
> Teas mailing list
> Teas@ietf.org
> https://www.ietf.org/mailman/listinfo/teas
>
>
> _______________________________________________
> Teas mailing list
> Teas@ietf.org
> https://www.ietf.org/mailman/listinfo/teas


_________________________________________________________________________________________________________________________

Ce message et ses pieces jointes peuvent contenir des informations confidentielles ou privilegiees et ne doivent donc
pas etre diffuses, exploites ou copies sans autorisation. Si vous avez recu ce message par erreur, veuillez le signaler
a l'expediteur et le detruire ainsi que les pieces jointes. Les messages electroniques etant susceptibles d'alteration,
Orange decline toute responsabilite si ce message a ete altere, deforme ou falsifie. Merci.

This message and its attachments may contain confidential or privileged information that may be protected by law;
they should not be distributed, used or copied without authorisation.
If you have received this email in error, please notify the sender and delete this message and its attachments.
As emails may be altered, Orange is not liable for messages that have been modified, changed or falsified.
Thank you.