Re: [yang-doctors] Yangdoctors early review of draft-ietf-detnet-yang-12
Xufeng Liu <xufeng.liu.ietf@gmail.com> Tue, 06 July 2021 20:46 UTC
Return-Path: <xufeng.liu.ietf@gmail.com>
X-Original-To: yang-doctors@ietfa.amsl.com
Delivered-To: yang-doctors@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 904F83A0B1C; Tue, 6 Jul 2021 13:46:34 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -2.097
X-Spam-Level:
X-Spam-Status: No, score=-2.097 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, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, URIBL_BLOCKED=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 ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id QkSWhJx5GgnG; Tue, 6 Jul 2021 13:46:29 -0700 (PDT)
Received: from mail-ej1-x62e.google.com (mail-ej1-x62e.google.com [IPv6:2a00:1450:4864:20::62e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 9CED23A0B16; Tue, 6 Jul 2021 13:46:28 -0700 (PDT)
Received: by mail-ej1-x62e.google.com with SMTP id hc16so36255156ejc.12; Tue, 06 Jul 2021 13:46:28 -0700 (PDT)
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=RTgIxzpSpix6yNTB7yfRoL6Fk0KJrJmdcrrAR9S8KMI=; b=rrZRDL/xURwIUnBcdtPI+gvGoqEE2LK6JY6oV5yQ5Eg1X+aflwbh2ERpKRBJ8PmZLG EiA/WlQ0OW5CchB6Mn6pg9X0xh4pFanDlV2sOcOpC/uvrYHnFHEJltpOX98C0j34Ll3Q JKWHa/qZ4aaAGmblkpE2D4dVQm21XBBV9g1ydN5D8Y0bea7okHx7BRALYZg9BvD9Jasg DTdoiMPhdrWpgJd+48FMhD2PULYWR9uf+pCrJdE/GcO3IDwuI+l0Wai9pomW2FEcTap3 Rke61n5xx1tKXaZ+7IFPNn6KtMckvpSD4t5Z+Pfqr//qOXT1/XWLDry9uji2r906Zde5 rwaw==
X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=RTgIxzpSpix6yNTB7yfRoL6Fk0KJrJmdcrrAR9S8KMI=; b=XLJJLRn/TRFsFNrKHagbQj5RCnJXm/dR0wuQBY7wnf5rjXOh6Qmb8oAph1d4tq120h Nipnzqg8nCRFKDQH7I2DkZ2w7uonzkHRK43P+ysPFjVXjJpdrYCzs3BzQLHxRwIdQIao gTbvtoiE3d/COEneMNmY+hMgdX+ACXU7G2LYNLAfssHAw6ilAKv+d6r6RuukzWnErRD8 mtU4dL0aIorfQJejOEjwwWqeMxO4YfWAT4XKzgVUzM29RRL75LjfT670r6JgZJpycoby ZKV/e+U7c3tYdWJVMmhWVVqgMHS+FujbizheAhFOt9TjTD/kPVVhdLXdHEbZ2YkF+B6X UxKw==
X-Gm-Message-State: AOAM5326ukCTIOG5OJ4hHvltBI3q1jYgbV7VYVm4hmC4dehQMaz/3x9t okxCCcO8m+04BGrO/B4ayRhcFlZStbwHkZlF35I=
X-Google-Smtp-Source: ABdhPJwDYSyJBjPxD7mhAbvO5wuW9iTUN8WEdbhOu+85s5Wpb0P095P2PSQnuuBi7LyXBG/NjesKH1MtT/s3eaphiX4=
X-Received: by 2002:a17:907:d03:: with SMTP id gn3mr20943919ejc.516.1625604385209; Tue, 06 Jul 2021 13:46:25 -0700 (PDT)
MIME-Version: 1.0
References: <162483184905.11951.18366537346797539633@ietfa.amsl.com> <MN2PR14MB40304BBE8F9D17405F4031DFBB029@MN2PR14MB4030.namprd14.prod.outlook.com>
In-Reply-To: <MN2PR14MB40304BBE8F9D17405F4031DFBB029@MN2PR14MB4030.namprd14.prod.outlook.com>
From: Xufeng Liu <xufeng.liu.ietf@gmail.com>
Date: Tue, 06 Jul 2021 16:46:13 -0400
Message-ID: <CAEz6PPQoueOJm7eFSsk8rkkMN=a6GQzETX9feWo5CeWkKMWQVA@mail.gmail.com>
To: Don Fedyk <dfedyk@labn.net>
Cc: "yang-doctors@ietf.org" <yang-doctors@ietf.org>, "detnet@ietf.org" <detnet@ietf.org>, "draft-ietf-detnet-yang.all@ietf.org" <draft-ietf-detnet-yang.all@ietf.org>
Content-Type: multipart/alternative; boundary="000000000000ed023c05c67a831e"
Archived-At: <https://mailarchive.ietf.org/arch/msg/yang-doctors/gO_b1BN9NAJAfAxww74DQMNEE1M>
Subject: Re: [yang-doctors] Yangdoctors early review of draft-ietf-detnet-yang-12
X-BeenThere: yang-doctors@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Email list of the yang-doctors directorate <yang-doctors.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/yang-doctors/>
List-Post: <mailto:yang-doctors@ietf.org>
List-Help: <mailto:yang-doctors-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/yang-doctors>, <mailto:yang-doctors-request@ietf.org?subject=subscribe>
X-List-Received-Date: Tue, 06 Jul 2021 20:46:42 -0000
Hi Don, Thanks for addressing my comments. Look forward to the updated version. Best, - Xufeng On Tue, Jun 29, 2021 at 7:44 PM Don Fedyk <dfedyk@labn.net> wrote: > Hi Xufeng > > > Thanks for your detailed comments. > > Comments inline [Don] > > -----Original Message----- > From: Xufeng Liu via Datatracker <noreply@ietf.org> > Sent: Sunday, June 27, 2021 6:11 PM > To: yang-doctors@ietf.org > Cc: detnet@ietf.org; draft-ietf-detnet-yang.all@ietf.org > Subject: Yangdoctors early review of draft-ietf-detnet-yang-12 > > Reviewer: Xufeng Liu > Review result: On the Right Track > > This is a review of the YANG module in draft-ietf-detnet-yang-12. > > 1) Missing diagrams in the PDF and HTML versions of the document. > Sec A.1. says “Please consult the PDF or HTML versions for the Case A-1 > Diagram”, but the this diagram and other diagrams are not present. Because > of the missing diagrams, it is unclear how these examples work, so the > following comments are based on some assumptions that may or may not be > what the authors intend to hold. > [Don] raised as a Tools issue to the editor. > > 2) Implementation locations and the users of the model. > Is this a network model implemented on a controller or a device model > implemented on a network element? The abstract section states “The > model > allows for provisioning of end-to-end DetNet service”, but the YANG > module > and all examples use “ietf-interfaces”, which is a device model. The > interface name such as “eth0” is unique within the scope of a device. > There > is no explanation in the YANG module and any of these examples. > If the model is intended to be used on a network-wide controller, any > device > module should not be imported. A network-wide module may be used on a > device, and a device module may then be imported. > > [Don] Will Clarify. The model is per device. Either an operator or a > Controller aligns the configuration of the devices. > [Xufeng]: Thanks for the clarification, which also solves some of my confusion below. It would be beneficial to add some description to the document. > > 3) In Sec 5., in the diagram, “TR” is used, but not defined. Amusing that > “Ref” > means “referencing”, the abbreviation also needs to be defined, especially > if this is using the YANG leafref mechanism. > [Don] TR should be Traffic Profile. I have expanded this acronym. > > 4) In Sec 5., the last paragraph states “four parts of DetNet functions > defined in section 3”, but “section 3” does not define the “four” parts. > [Don] Wil fix this. This is referring to App-flow, Service Sub-layer, > Forwarding Sub-layer and traffic Profile (as 4 but this this confusing.) > The Modle has 4 main section but supports 3 layers (not instances) > > 5) In Sec 5., the last paragraph describes “three instances in DetNet YANG > Model”. The term “instance” is not clear here. A YANG data model does not > usually consist of “instances”. RFC7950 uses “instance” to indicate the > configuration data or operational data, so “instance” means “data > instance”, which is not part of a YANG data model that “describes how data > is represented and accessed”. > > In Sec A.1, Example A-1 provides one data instance that contains all three > “parts” (or components) of the DetNet system, so there is no such a concept > of “App-flow instance”, “service sub-layer instance”, or “forwarding > sub-layer instance”, because one single “data instance” may contain all > three parts. > [Don] App-flow is a layer. A node for example an edge node is a data > instance that has the 3 layers. > > 6) In Sec 6. in the sub-tree app-flows, > 6.1) Does this sub-tree model App-flow or NetNet flow, or both? > draft-ietf-detnet-flow-information-model-14 defines both terms and > describes the distinctions between them. > [Don] App-Flow is a DetNet flow it all depends on the granularity and > where you are. At an edge a flow or set of flows come from an application. > It could be a single applications i.e. a voice call or a bunch of flows - > another DetTet network (with voice and video flows). > Note it is all unidirectional. > > 6.2) Except the leaf "name", all other leaves are optional. When a user > creates an instance of app-flow with only the name specified, what > does the > system do? > [Don] Will check but in essence if the Application is an ingress interface > there is not much there. I think we can make interface mandatory (Multiple > apps can aggregate multiple interfaces.) Traffic flow qualifiers can be > refine to less than an interface. > [Xufeng]: Right. Currently, even the interface is optional. > > 6.3) Containers “ingress” and “egress” are specified. However, based > on Sec > 2.1 of draft-ietf-detnet-flow-information-model-14, An app-flow > consists of > source and destination, not ingress and egress. “DN Ingress” and “DN > Egress” are parts of a “DetNet flow”, not app-flow. > [Don] DetNet flow equals App-Flow. Source and destination are used as > traffic qualifiers for IP flows. > > 6.4) Container “ingress” should not contain both source and > destination. > Sec 2.1. of draft-ietf-detnet-flow-information-model-14 defines the > “ingress” as the location where DetNet flow starts, so the ingress is > related only to the source, not the destination. > [Don] Ingress is for aggregation / filtering of traffic in a > unidirectional. It is like an Access control list for the traffic. Will > add a note. But source and destination is correct. > [Xufeng]: A note would be appreciated. Your above clarification that the model is per device also helps me here and for some questions below. > > 6.5) Based on Sec 5.4. of draft-ietf-detnet-flow-information-model-14, > the > choice data-flow-type under the container “ingress” is how a DN flow is > identified, so this choice is not just related to the ingress of the > flow, > and should be outside of the container “ingress”. > [Don] This is Unidirectional. Ingress is the correct placement. The other > direction is Ingress on the remote end. > > 6.6) Based on Sec 5.6. of draft-ietf-detnet-flow-information-model-14, > the > ingress section of the model should identify the ingress node(s) and/or > starting reference points. It would be good to have some description > on how > the ingress node(s) is (are) identified, especially in the case of > mpls-app-flow. The description on the leaf name currently is "Ingress > DetNet application", which is not consistent with the definition in > draft-ietf-detnet-flow-information-model-14. > [Don] These are identified by interfaces and next-hops (interfaces). > > 6.7) Why is app-flow-status under the container ingress? > app-flow-status is > related to an instance of app-flow, not only the “ingress” part of the > app-flow, right? > [Don] The status is unidirectional. The remote side has the other > direction status. > > 6.8) The leaf interface in the container ingress would only work when > this > model is implemented on a device (but not on a controller) because > ietf-interfaces is a device model. Based on my previous assumption that > this model may be used as a network-wide model on a controller, the > referencing to ietf-interfaces would not be appropriate. > [Don] This is a device model. The controller configures multiple devices. > If the configuration does not align the flow will not work. > > 6.9) The relationship between app-flow and service-sub-layer needs > better > described. When an app-flow is served by a DN service, the packets of > this > app-flow are processed by this particular ND service, so there is only > one > supporting service. Why are there the “incoming-service” and > “outgoing-service”? Are the terms “incoming-service” and > “outgoing-service” > defined and/or described in any document? > [Don] We support point to point and point to multipoint. (Replication is > an example.) > > 7) In Sec 6. in the sub-tree traffic-profile: > The container flow-spec is not consistent with > draft-ietf-detnet-flow-information-model-14, which defines the leaves > under > this container as TrafficSpecification in Sec 4.1. > [Don] It is up the to WG if they want to change the name. I will ask the > work Group. The flow specification is a functional model. The DetNet model > does not map 1:1. > > 8) Before Sec 6, the document does not have an informative reference to > the YANG tree diagrams specification RFC8340. Refer to Section 2.2 of > [RFC8349] for an example of such a reference. > [Don] Added. Tom Petch Commented. > > 9) In 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. > [Don] Added. Tom Petch Commented. > > 10) In Sec 9, the Security section is missing. > [Don] Added. Tom Petch Commented. > > 11) In Sec 8, the IANA Considerations section is incorrect, with missing > IANA requests. > [Don] Added. Tom Petch Commented. > > 12) For the IP address or prefix examples in the Appendix, a mix of either > IPv4 and IPv6 addresses or IPv6 addresses exclusively SHOULD be used. > [Don] The model supports it but it is a royal pain to align the sample > config and the SVG diagrams. I would much rather say IPv4 is shown and IPv6 > is supported. I already did some conversion of the from Tom Petch's > comments apparently there is a Ipv4 IETF space we have to use. You can see > the updates here: > https://github.com/detnet-wg/draft-ietf-detnet-yang/blob/master/draft-ietf-detnet-yang-13.pdf > The issue is IPv6 examples do not clarify anything as far as I'm > concerned. They just burn more text and diagram real estate (and editorial > time). > [Xufeng]: Your opinion is understandable, but the consensus was documented in the guidelines [RFC8407], and it is a checkpoint during IESG review. > > 13) The module prefix ietf-detnet is better to be changed to detnet. > IETF-defined YANG modules usually do not use “ietf-” as part of the prefix > name. > [Don] Changed to dnet per Tom Petch Comment. > > 14) As described in Sec 4.3.1. of RFC8407, child nodes within a container > or list SHOULD NOT replicate the parent identifier. There are several > occasions in the YANG module, for example, profile-name, > app-flow-bidir-congruent, service-protection, and service-operation-type. > [Don] Perhaps the YANG tools should prevent this! We evolved the naming > several times. Personally. I prefer to make the container profile and the > child name for example which works when reading the tree diagram and in the > config examples. > [Xufeng]: It could be raised to the tools' authors, but I'm not sure that they would be able to achieve this level of sophistication. > > 15) The node name in the list or leaf-list statements should not contain > the suffix “-list”, or in the plural format. For example, > service-sub-layer-list is better to be changed to service-sub-layer; > member-services should be member-service. The structure of > app-flows/app-flow* is the common convention used in IETF. It would be good > to be consistent within this module, so it will be a good idea to add a > container "traffic-profiles" to the list "traffic-profile". > [Don] Already removed -type. Will remove -list. > > 16) Most of the leaves in this module are defined as optional, but only > one leaf has a default statement in the entire module. It is generally > needed to either provide a default value or an explanation in the > description for such an optional leaf. > [Don] Correct the leaves are optional - the model support various levels > of traffic refinement (6-tuple) aggregation etc. > [Xufeng]: If a default statement is not appropriate, some clarification in the description statement would be appreciated. > > 17) There is no configuration node and operational state node to indicate > whether DetNet is enabled on the system. Please confirm that this is the > intention. The fact that this module is supported on a system may not mean > that DetNet is enabled. > [Don] I don't know I don't see a system enable/disable being useful. > Perhaps a feature statement? Then if a system does not support the feature > it is disabled. > [Xufeng]: If such a capability is not useful, you don't have to model it. A feature statement would not help in specifying the run-time system behavior. The system identifies a YANG feature when it loads a module. > > Thanks again. > Don > > Thanks, > - Xufeng > > > >
- [yang-doctors] Yangdoctors early review of draft-… Xufeng Liu via Datatracker
- Re: [yang-doctors] Yangdoctors early review of dr… Don Fedyk
- Re: [yang-doctors] Yangdoctors early review of dr… Don Fedyk
- Re: [yang-doctors] Yangdoctors early review of dr… Don Fedyk
- Re: [yang-doctors] Yangdoctors early review of dr… Xufeng Liu