[Teas] Yangdoctors early review of draft-ietf-teas-te-service-mapping-yang-10

Xufeng Liu via Datatracker <noreply@ietf.org> Tue, 15 March 2022 00:25 UTC

Return-Path: <noreply@ietf.org>
X-Original-To: teas@ietf.org
Delivered-To: teas@ietfa.amsl.com
Received: from ietfa.amsl.com (localhost [IPv6:::1]) by ietfa.amsl.com (Postfix) with ESMTP id 4A1AA3A12BD; Mon, 14 Mar 2022 17:25:53 -0700 (PDT)
MIME-Version: 1.0
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: 8bit
From: Xufeng Liu via Datatracker <noreply@ietf.org>
To: yang-doctors@ietf.org
Cc: draft-ietf-teas-te-service-mapping-yang.all@ietf.org, teas@ietf.org
X-Test-IDTracker: no
X-IETF-IDTracker: 7.46.0
Auto-Submitted: auto-generated
Precedence: bulk
Message-ID: <164730395314.8331.2676797734526513659@ietfa.amsl.com>
Reply-To: Xufeng Liu <xufeng.liu.ietf@gmail.com>
Date: Mon, 14 Mar 2022 17:25:53 -0700
Archived-At: <https://mailarchive.ietf.org/arch/msg/teas/OCWVkQBxrRd2bjO-v_ysDb_9RUI>
Subject: [Teas] Yangdoctors early review of draft-ietf-teas-te-service-mapping-yang-10
X-BeenThere: teas@ietf.org
X-Mailman-Version: 2.1.29
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: Tue, 15 Mar 2022 00:25:54 -0000

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?

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.

1.3) Similar to 1.2), “te-mapping-template-ref” can be improved by dropping the
prefix “te-mapping”.

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?

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.

1.6) In this YANG module, there are many lines of  “//grouping”. What are they
used for?

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.

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.

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.

2.2) Why is the container te-service-mapping “presence”? What does an empty
container te-service-mapping indicate?

2.3) Why is there container te-mapping under te-service-mapping? Are the two
containers duplicated?

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.

2.5) In the YANG module, there are a few lines of “//augment”. What do they
mean?

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.

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?

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?

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.

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”.

5.2) Sec 1.4. “Section 5 of this this document” should be “Section 6 of this
document”

5.3)  “YANG codes” is better changed to “YANG modules”.

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.

5.5) “Import network model” should be “Import network module”. The "import"
statement makes definitions from one module available inside another module.

Thanks,
- Xufeng