Re: [i2rs] Robert Wilton's Discuss on draft-ietf-i2rs-yang-l2-network-topology-14: (with DISCUSS and COMMENT)
Susan Hares <shares@ndzh.com> Thu, 09 July 2020 14:44 UTC
Return-Path: <shares@ndzh.com>
X-Original-To: i2rs@ietfa.amsl.com
Delivered-To: i2rs@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id 1C32B3A0BFD; Thu, 9 Jul 2020 07:44:41 -0700 (PDT)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: 0.948
X-Spam-Level:
X-Spam-Status: No, score=0.948 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, DOS_OUTLOOK_TO_MX=2.845, SPF_HELO_NONE=0.001, SPF_NONE=0.001, URIBL_BLOCKED=0.001] autolearn=no autolearn_force=no
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 87u4ph3PpllI; Thu, 9 Jul 2020 07:44:38 -0700 (PDT)
Received: from hickoryhill-consulting.com (50-245-122-97-static.hfc.comcastbusiness.net [50.245.122.97]) (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 2C2223A0C02; Thu, 9 Jul 2020 07:44:38 -0700 (PDT)
X-Default-Received-SPF: pass (skip=loggedin (res=PASS)) x-ip-name=50.107.91.217;
From: Susan Hares <shares@ndzh.com>
To: 'Robert Wilton' <rwilton@cisco.com>, 'The IESG' <iesg@ietf.org>
Cc: draft-ietf-i2rs-yang-l2-network-topology@ietf.org, i2rs@ietf.org, i2rs-chairs@ietf.org
References: <159429679610.11105.2067062212106537820@ietfa.amsl.com>
In-Reply-To: <159429679610.11105.2067062212106537820@ietfa.amsl.com>
Date: Thu, 09 Jul 2020 10:44:28 -0400
Message-ID: <002101d655ff$73f1b490$5bd51db0$@ndzh.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
X-Mailer: Microsoft Outlook 14.0
Content-Language: en-us
Thread-Index: AQLfG8fY9Xjp0/0dViApAf34v9+Emqbtw+VA
X-Antivirus: AVG (VPS 200709-4, 07/09/2020), Outbound message
X-Antivirus-Status: Not-Tested
X-Authenticated-User: skh@ndzh.com
Archived-At: <https://mailarchive.ietf.org/arch/msg/i2rs/kgmUt3si0c6DrwcOxEHfx9pigPI>
Subject: Re: [i2rs] Robert Wilton's Discuss on draft-ietf-i2rs-yang-l2-network-topology-14: (with DISCUSS and COMMENT)
X-BeenThere: i2rs@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "Interface to The Internet Routing System \(IRS\)" <i2rs.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/i2rs>, <mailto:i2rs-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/i2rs/>
List-Post: <mailto:i2rs@ietf.org>
List-Help: <mailto:i2rs-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/i2rs>, <mailto:i2rs-request@ietf.org?subject=subscribe>
X-List-Received-Date: Thu, 09 Jul 2020 14:44:41 -0000
Rob: Thank you for raising the process discussion regarding IEEE-IETF coordination. Qin, his co-authors, and I desire to have a model which uses the best common practices of the IEEE and IETF. Qin and his co-authors will work through all the specific description changes below and the auto-neg description in his responses. We were pointed to draft-ietf-netmod-sub-intf-vlan-model and worked to adjust to this model. Specific suggestions for changes would be helpful at this point. I have the following two higher level questions as the shepherd/co-chair: 1) This document was repeated reviewed by the Yang doctors prior to submission. We were pointed to draft-ietf-netmod-sub-intf-vlan-model and worked to adjust to this model. What happened in the process of Yang model review that caused you to have so many comments at the last moment? Where did we miss the clues that there were problems in the email from the Yang doctors? 2) Policy questions regarding IEEE and IETF focus on QinQ versus VLAN I have one question for you and the rest of the IESG. The IEEE terminology for QinQ versus VLAN does not always match the industry use. IETF has always maintained running code for operational networks is the most important factor. IEEE has maintained consistency of their own terminology was most important. These differences cause the switches two have both terminologies. How should we support both? Susan Hares -----Original Message----- From: i2rs [mailto:i2rs-bounces@ietf.org] On Behalf Of Robert Wilton via Datatracker Sent: Thursday, July 9, 2020 8:13 AM To: The IESG Cc: draft-ietf-i2rs-yang-l2-network-topology@ietf.org; i2rs@ietf.org; i2rs-chairs@ietf.org Subject: [i2rs] Robert Wilton's Discuss on draft-ietf-i2rs-yang-l2-network-topology-14: (with DISCUSS and COMMENT) Robert Wilton has entered the following ballot position for draft-ietf-i2rs-yang-l2-network-topology-14: Discuss When responding, please keep the subject line intact and reply to all email addresses included in the To and CC lines. (Feel free to cut this introductory paragraph, however.) Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html for more information about IESG DISCUSS and COMMENT positions. The document, along with other ballot positions, can be found here: https://datatracker.ietf.org/doc/draft-ietf-i2rs-yang-l2-network-topology/ ---------------------------------------------------------------------- DISCUSS: ---------------------------------------------------------------------- I support Magnus's discuss regarding IEEE 802.1Q WG review. I also feel that the YANG model could benefit from another editorial pass: - In many places the descriptions are very terse, and references are missing. - The way that auto-neg is defined doesn't really match the 802.3 specification, probably splitting it into two separate leaves (one for whether auto-neg is on/off, and separate one for the duplex setting would be better). - The use of terminology for VLAN vs QinQ might not be acceptable to IEEE. Finding names that are more closely aligned with the terms in 802.1Q may be helpful (although if I understand it correctly, 802.1Q bridges don't directly expose double VLAN tags). Possibly some of the terminology/description from draft-ietf-netmod-sub-intf-vlan-model (which has been reviewed by IEEE 802.1Q WG) may be helpful here. ---------------------------------------------------------------------- COMMENT: ---------------------------------------------------------------------- Mostly minor/editorial comments (###) on the YANG model, but I do think that it would be helpful for these to be discussed and addressed as appropriate: 4. Layer 2 Topology YANG Module import ietf-inet-types { prefix inet; reference "Section 4 of RFC 6991"; } import ietf-yang-types { prefix yang; reference "Section 3 of RFC 6991"; } ### These references should probably both just be: "RFC 6991: Common YANG Data Types" revision 2020-06-29 { description "Initial revision"; reference "RFC XXXX: A YANG Data Model for Layer 2 Network Topologies"; } ### I would reorder these sections to be (which will solve the forward reference issue mentioned by Ben): feature-stmt identity-stmt typedef-stmt I'm surprised that pyang didn't flag this. /* * Typedefs */ typedef vni { type uint32 { range "0..16777215"; } description "VXLAN Network Identifier or VXLAN Segment ID. It allows up to 16 M VXLAN segments to coexist within the same administrative domain. The use of value '0' is implementation-specific."; reference "RFC 7348: Virtual eXtensible Local Area Network (VXLAN): A Framework for Overlaying Virtualized Layer 2 Networks over Layer 3 Networks"; } typedef l2-flag-type { type identityref { base flag-identity; } description "Base type for L2 flags. One example of L2 flag type is trill which represents trill topology type."; } ### This isn't really a base type. Given where this flag is used, would this be better as an "network-flag-type", with a description more similar to the ones below? typedef node-flag-type { type identityref { base flag-identity; } description "Node flag attributes. The physical node can be one example of node flag attribute."; } typedef link-flag-type { type identityref { base flag-identity; } description "Link flag attributes. One example of link flag attribute is the pseudowire."; } ### Should there be identities defined for l2-flag, node-flag and link-flag that derive from flag-identity? That would them make these three typedefs reference different things rather than all referencing the same base flags. typedef l2-network-event-type { type enumeration { enum add { value 0; description "A Layer 2 node or link or termination-point has been added."; } enum remove { value 1; description "A Layer 2 node or link or termination-point has been removed."; } enum update { value 2; description "A Layer 2 node or link or termination-point has been updated."; } } description "Layer 2 network event type for notifications."; } ### Since these are events, I would suggest renaming these "add" -> "addition", "remove" -> "removal", "update" typedef neg-mode { type enumeration { enum full-duplex { description "Indicates full-duplex mode."; } enum auto-neg { description "Indicates auto-negotiation mode."; } enum half-duplex { description "Indicates half-duplex mode."; } } description "Indicates the type of the negotiation mode."; } ### What negotiation do you mean? Does this mean IEEE 802.3 auto-negotation? If so, it would be more correct to have a separate leaf for duplex vs auto-negotation. /* * Features */ feature VLAN { description "Indicates that the system supports the vlan functions (also known as an IEEE 802.1Q tag)."; } feature QinQ { description "Indicates that the system supports the qinq functions (also known as IEEE 802.1ad double tag)."; } ### I think that the description should be more clear on what is supported here: 2 VLAN tags, as defined by IEEE 802.1ad. The features should also include reference statements. Possibly we need to find a better name for these features (IEEE will possibly comment on this). feature VXLAN { description "Indicates that the device supports VXLAN functions."; reference "RFC 7348: Virtual eXtensible Local Area Network (VXLAN): A Framework for Overlaying Virtualized Layer 2 Networks over Layer 3 Networks"; } /* * Identities */ identity flag-identity { description "Base type for flags."; } ### Probably the name should be just "flag" rather than "flag-identity" identity eth-encapsulation-type { base ift:iana-interface-type; description "Base identity from which specific Ethernet encapsulation types are derived."; reference "RFC 7224: IANA Interface Type YANG Module"; } ### I would rename the base identity to probably "l2-encapsulation-type". It also probably doesn't help to derive from ift:iana-interface-type. identity ethernet { ... identity vxlan { base eth-encapsulation-type; description "VXLAN MAC in UDP encapsulation."; } ### Please add references for protocols defined in the identities. /* * Groupings */ grouping l2-network-type { Dong, et al. Expires December 31, 2020 [Page 12] Internet-Draft Layer 2 Network Topology Data Model June 2020 description "Indicates the topology type to be L2."; container l2-network { presence "Indicates L2 Network"; description "The presence of the container node indicates L2 Topology."; } } grouping l2-network-attributes { description "L2 Topology scope attributes."; container l2-network-attributes { description "Contains L2 network attributes."; leaf name { type string; description "Name of the L2 network."; ### Perhaps just "Name of the network". Are there any restrictions here on how long the name can be, can it contain spaces, etc? } leaf-list flag { type l2-flag-type; description "L2 network flags."; } } } ### Perhaps expand the description to "Flags that can be associated with the network" I would suggest changing the name of all "flag" to "flags" where they are leaf-lists. grouping l2-node-attributes { description "L2 node attributes"; container l2-node-attributes { description "Contains L2 node attributes."; leaf name { type string; description "Node name."; } leaf description { type string; description "Node description."; } leaf-list management-address { type inet:ip-address; description "System management address."; } leaf sys-mac-address { type yang:mac-address; description "System MAC address."; } leaf management-vid { if-feature "VLAN"; type dot1q-types:vlanid; description "System management VID."; ### Please change "management-vid" to "management-vlan-id", and fix the description. Probably could expand on all the descriptions of the "System" properties to explain what these system properties are and how they are used (e.g. to manage the device). } leaf-list flag { type node-flag-type; description "Node operational flags."; } } } grouping l2-link-attributes { description "L2 link attributes"; container l2-link-attributes { description "Contains L2 link attributes."; leaf name { type string; description "Link name."; } leaf-list flag { type link-flag-type; description "Link flags."; } leaf rate { type decimal64 { fraction-digits 2; } units "Mbps"; description "Link rate."; ### Please expand on this description. Does this describe how the link is operating at the physical layer, or its bandwidth? } leaf delay { type uint32; units "microseconds"; description "Link delay in microseconds."; ### Is this uni-directional delay, or bi-directional? Please clarify. leaf maximum-frame-size { type uint32; description "Maximum L2 frame size. If L2 frame is an Ethernet frame, the Ethernet header should be included; if L2 frame is other type (e.g., PPP), the L2 header should be included."; ### This needs to clarify whether the 4 byte CRC is included in the frame size. leaf eth-encapsulation { type identityref { base eth-encapsulation-type; } ### As per comments on the identities, I think that this should be "encapsulation". description "Encapsulation type of this termination point."; } leaf lag { type boolean; default "false"; description "Defines whether lag is supported or not."; ### Does this leaf indicate whether LAG is supported, or enabled on the link? } leaf-list member-link-tp { when "../lag = 'true'" { description "Relevant only when the lag interface is supported."; } type leafref { path "/nw:networks/nw:network/nw:node/" + "nt:termination-point/nt:tp-id"; } description "Member link termination points."; } leaf mode { type neg-mode; default "auto-neg"; description "Exposes the negotiation mode."; } ### Suggest changing the name to "ethernet-auto-neg-mode", and refining the description. I would also suggest splitting duplex and auto-negotiation into 2 separate leaves to more accurately represent leaf port-vlan-id { when "derived-from-or-self(../eth-encapsulation" + ", 'l2t:vlan')" { description "Only applies when the type of the Ethernet encapsulation is 'vlan'."; } if-feature "VLAN"; type dot1q-types:vlanid; description "Port VLAN ID is the VLAN identifier that will be assigned to any untagged frames entering the switch on the specific port."; } ### Probably naming this as the "default-untagged-vlan" may be more helpful. list vlan-id-name { when "derived-from-or-self(../eth-encapsulation" + ", 'l2t:vlan')" { description "Only applies when the type of the Ethernet encapsulation is 'vlan'."; } ### I think that I would name this list "vlans" leaf vlan-name { ### I would suggest renaming this leaf to just "name" "Interface configured SVLANs and CVLANs."; leaf svlan-id { type dot1q-types:vlanid; description "SVLAN ID."; } leaf cvlan-id { type dot1q-types:vlanid; description "CVLAN ID."; } } ### Suggest "SVLAN" => "S-VLAN" and "CVLAN" => "C-VLAN" container vxlan { when "derived-from-or-self(../eth-encapsulation" + ", 'l2t:vxlan')" { description "Only applies when the type of the Ethernet encapsulation is 'vxlan'."; } if-feature "VXLAN"; leaf vni-id { type vni; description "VXLAN Network Identifier (VNI)."; } description "Vxlan encapsulation type."; } } //case ethernet case legacy { leaf layer-2-address { type yang:phys-address; description "Interface Layer 2 address."; } leaf encapsulation { type identityref { base ift:iana-interface-type; } description "Other legacy encapsulation type of this termination point."; } } //case legacy such as atm, ppp, hdlc, etc. } //choice termination-point-type leaf tp-state { type enumeration { enum in-use { value 1; description "The termination point is in forwarding state."; } enum blocking { value 2; description "The termination point is in blocking state."; } enum down { value 3; description "The termination point is in down state."; } enum others { ### Please rename others => other value 4; description "The termination point is in other state."; ### Please modify the description to something like "The termination point is in another state, not listed in the enumeration." } } config false; description "State of the termination point."; } } } _______________________________________________ i2rs mailing list i2rs@ietf.org https://www.ietf.org/mailman/listinfo/i2rs
- [i2rs] Robert Wilton's Discuss on draft-ietf-i2rs… Robert Wilton via Datatracker
- Re: [i2rs] Robert Wilton's Discuss on draft-ietf-… Susan Hares
- Re: [i2rs] Robert Wilton's Discuss on draft-ietf-… Qin Wu