Re: [Teas] Yangdoctors early review of draft-ietf-teas-te-service-mapping-yang-10
Dhruv Dhody <dhruv.ietf@gmail.com> Mon, 11 July 2022 14:04 UTC
Return-Path: <dhruv.ietf@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 039DBC15791D; Mon, 11 Jul 2022 07:04:51 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.104
X-Spam-Level:
X-Spam-Status: No, score=-2.104 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_ZEN_BLOCKED_OPENDNS=0.001, 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 GiZxT26fVmDt; Mon, 11 Jul 2022 07:04:48 -0700 (PDT)
Received: from mail-io1-xd2d.google.com (mail-io1-xd2d.google.com [IPv6:2607:f8b0:4864:20::d2d]) (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 B2DE5C159486; Mon, 11 Jul 2022 07:04:45 -0700 (PDT)
Received: by mail-io1-xd2d.google.com with SMTP id r76so4965780iod.10; Mon, 11 Jul 2022 07:04:45 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=0YZohNehQJNKBNIEu9WHif8WanIQErF6UH8IRRqzBhs=; b=R/Yu4chgdMUF2WBCaAEfHS3zooJd7Mu/nL9zlIh6mae43z6LmMOLQqdDEyZ+mFbEnk TFAC73/eIvA9D/bUdffloHqp68OPvGfn//byVDL1zE+kNkoiHZdfB9+dfKF7aLHdPepk xp4efYJwm8/ooJCrUKu+/EgoB3kt0r8+DW7ttV71Qjs1Gt6aAN8oUcuUONsEajzmoh88 D+3zx33vidbCX66dNisnJqElRW3XN0IC1XHWpb9Lpzg3btMODemCg7WBeAOjyPU9xR5n dCZhAuAYwb7mH2XMFcybmlAbiRG9Z4m3RAZAgPkxkH1DFEa55q+W6nmXEUWT7dQJKRRA x5Lw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=0YZohNehQJNKBNIEu9WHif8WanIQErF6UH8IRRqzBhs=; b=QerbMJU32zRvOPU7CKIYN0yDG8MiMwmkMQiMO0tQXv+EAgDQroJKJoNnz+s6hMeHw0 wA0LECMh4w4N68PJT94TPh1Wtu/G4uuO4UdOFxkiafjZpSH0yPH8bMbmfKtZC53gsMAs ulwEE4Sgt4oIblZ+lg5O1Zgz3udNvunDGkYE4YepdJmKxWh3OW3/JnuqR3a99i+YC6e7 g6UktocoyQfok0NwGgh7g4U/8VQz8746KsvYQXvegC+RdJ1OfD5y821T/UBh42CfGg4j lJ9law/nbLUfg/Qi+xOjO5d849tLqZs4LjZP/ItYNdUPKNiLcToLwKlNaBt4fhpWvtlD 1rYg==
X-Gm-Message-State: AJIora9xNFZcaowDL1sVfVSsXnI2eClgRz04YraV6PiiwjNR61KUINdh doWzs3uWR315805u+xKxT7EDHsIIRKDJQAYsQVF6Ebj7dAN9tw==
X-Google-Smtp-Source: AGRyM1v6tZyrUp4ClpM0g94L127CCBRBqvBhT7u+pYFUudgHrmATu4vf0IbgJ0P4+d+u+Ns/Jkwbory3V8UfGidFMF0=
X-Received: by 2002:a02:606f:0:b0:335:ae88:68c6 with SMTP id d47-20020a02606f000000b00335ae8868c6mr9986248jaf.320.1657548284473; Mon, 11 Jul 2022 07:04:44 -0700 (PDT)
MIME-Version: 1.0
References: <164730395314.8331.2676797734526513659@ietfa.amsl.com>
In-Reply-To: <164730395314.8331.2676797734526513659@ietfa.amsl.com>
From: Dhruv Dhody <dhruv.ietf@gmail.com>
Date: Mon, 11 Jul 2022 19:34:08 +0530
Message-ID: <CAB75xn4RuNKKTCk0-+ZikHuTDcHE5YB2vEzUonQQkxvgV00-QQ@mail.gmail.com>
To: Xufeng Liu <xufeng.liu.ietf@gmail.com>
Cc: yang-doctors@ietf.org, draft-ietf-teas-te-service-mapping-yang.all@ietf.org, "TEAS WG (teas@ietf.org)" <teas@ietf.org>
Content-Type: multipart/alternative; boundary="000000000000b1b35905e38808da"
Archived-At: <https://mailarchive.ietf.org/arch/msg/teas/PWy0inU8WPODYY_JZqKYtVurhAk>
Subject: Re: [Teas] Yangdoctors early review of draft-ietf-teas-te-service-mapping-yang-10
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 14:04:51 -0000
Hi Xufeng, Thanks for your early Yangdoctors review. I have posted an update - https://www.ietf.org/rfcdiff?url2=draft-ietf-teas-te-service-mapping-yang-11 Please see inline. On Tue, Mar 15, 2022 at 5:55 AM Xufeng Liu via Datatracker <noreply@ietf.org> wrote: > Reviewer: Xufeng Liu > Review result: Almost Ready > > This is a review of the YANG modules in > draft-ietf-teas-te-service-mapping-yang-10. > > 1) ietf-te-service-mapping-types@2022-03-07.yang > > 1.1) In list sr-policy, there is the line: “/*Headend should also be > there!*/”. > I would agree with the comment since the color and (tail)-endpoint only > uniquely identify an sr-policy within the headend node. Does this mean that > something still needs to be done here? > > The correct thing to do is the headend is part of the draft-ietf-spring-sr-policy-yang-01 -- this way the model works on the controller as well as the device! I gave that comment some while back, and the document has not been updated in a while. For now, I will add the headend in our model (but keep the comment so that this could be tracked)! > 1.2) As described in Sec 4.3.1. of RFC8407, child nodes within a container > or > list SHOULD NOT replicate the parent identifier. The prefix “policy-” in > “policy-color-ref” and “policy-endpoint-ref” should be dropped. > > Done > 1.3) Similar to 1.2), “te-mapping-template-ref” can be improved by > dropping the > prefix “te-mapping”. > > Done > 1.4) The leaf color under the container te-policy has no reference. Is > this the > same as the “admin-group” defined in RFC 8776, RFC 7308, RFC 5305, and > RFC > 3630? > > Thanks for pointing this out. Changed to generic-path-affinities from te-types. > 1.5) In “map-type”, is the “map” the same as the “te-mapping” and > “te-service-mapping” in the parent containers? If these terms are the > same, it > should be considered to drop the prefix “map”. Furthermore, these three > terms > should be consistent, or their differences are explained. > > Removed te-mapping and updated to type only! > 1.6) In this YANG module, there are many lines of “//grouping”. What are > they > used for? > > To mark the end of grouping, but they can be removed and it is done! > 1.7) In this YANG module, there are section headers like “Typedef”, and > “Groupings”, etc., but there is no section header for “container > te-mapping-templates”, so it falls into the “Groupings” section, which is > misleading. > > Updated > 1.8) One identity is named “detnet-hard-isolation”. Does it mean to use > “detnet” architecture and implementation as defined in the DETNET Working > Group? If so, more detailed descriptions on how to use would be good. If > not, > it would be better to avoid the same term as used in > https://datatracker.ietf.org/doc/html/draft-ietf-detnet-yang. > > Removed! > 2) ietf-l3sm-te-service-mapping@2022-03-07.yang > > 2.1) This document uses the term “Layer 3 Service Model (L3SM)” which has > been > changed to “L3VPN Service Delivery” by RFC 8299. The term “L3SM” is not > used in > RFC 8299 any more, but is still used in this document extensively. Some > explanations in the document would be useful to avoid confusion. > > Good catch! Updated the terminology section. > 2.2) Why is the container te-service-mapping “presence”? What does an empty > container te-service-mapping indicate? > > It is marked that the VPN service is relying on the TE techniques, even though the mapping is not yet populated. Updated the description to say - "Indicates L3 service is relying on underlying TE" > 2.3) Why is there container te-mapping under te-service-mapping? Are the > two > containers duplicated? > > Removed. > 2.4) In the augmentation to site-network-access, why is vn-ap a list, but > ltp > single? It is notable that site-network-access is a list by itself, so we > can > support multiple parallel access points by using multiple entries of the > site-network-access list. > > Changed to a list. > 2.5) In the YANG module, there are a few lines of “//augment”. What do they > mean? > > Removed. > 2.6) The module l3vpn-svc supports two types of vpn-topology: any-to-any > and > hub-spoke. For hub-spoke, there are site-roles of hub-role and > spoke-role. It > would be beneficial for this document to describe how each type of > vpn-topology > is mapped, and how the site-roles are mapped. > > I don't think there is a change in the YANG model required -- each site belonging to a VPN has a "site-role" and mapping of the site to TE takes care of it. I have added text in the document to explain this! > 3) All modules > > 3.1) All leaves are optional. Please double-check and/or add meaningful > explanations. For example, for a template, what does an empty template do? > For > a l3-tsm, what does an empty mapping do? Are there any implications when a > leaf > is not configured? > > Good suggestion! Updated. > 3.2) There is no default on any leaf. Please double-check and/or add > meaningful > explanations. For example, if availability-type is not configured, what is > the > expected behavior? > > Updated. > 4) Examples > > 4.1) Sec 3.12 in RFC 8407 requires that “Example modules MUST be > validated”. > This can be done either by a validation tool or on a NETCONF/RESTCONF > server. > Some notable issues with the current examples are: 4.1.1) There are no > namespaces on any of the data nodes. 4.1.2) Some leafref values need to > appear > in the referenced module. > > I used yanglint to verify. The xml one is still giving me an error, there might be something off in my setup. > 5) Editorial > > 5.1) Use of the term “YANG model”. The document mostly uses correct terms > of > “YANG data model” and “YANG module”, but there are still occurrences of > “YANG > model” that should be changed to “YANG data model”. > > Ack > 5.2) Sec 1.4. “Section 5 of this this document” should be “Section 6 of > this > document” > > Updated > 5.3) “YANG codes” is better changed to “YANG modules”. > > Ok > 5.4) In each subsection of Sec 7, the document text does not have the > references to the RFCs cited in the YANG module. These references also > need to > be listed in the document. Sec 5 of RFC7317 provides a good example. > > Added in section 1.6 > 5.5) “Import network model” should be “Import network module”. The > "import" > statement makes definitions from one module available inside another > module. > > Ack. Thanks! Dhruv > Thanks, > - Xufeng > > > >
- [Teas] Yangdoctors early review of draft-ietf-tea… Xufeng Liu via Datatracker
- Re: [Teas] Yangdoctors early review of draft-ietf… Dhruv Dhody
- Re: [Teas] Yangdoctors early review of draft-ietf… Xufeng Liu